Fix some logic errors that reversed the reset choice

README.md

@@ -55,10 +55,10 @@ updating your initrd.
 
 | Vendor | Family | Common Name(s)
 |---|---|---|
-|AMD|Polaris 10| RX 470, 480, 570, 580, 590
+|AMD|Polaris 10|
-|AMD|Polaris 11| RX 460, 560
+|AMD|Polaris 11|
-|AMD|Polaris 12| RX 540, 550
+|AMD|Polaris 12|
-|AMD|Vega 10| Vega 56/64/FE |
+|AMD|Vega 10| Vega 56/64 |
 |AMD|Vega 20| Radeon VII |
 |AMD|Navi 10| 5600XT, 5700, 5700XT
 |AMD|Navi 12| Pro 5600M |

dkms.conf

@@ -1,5 +1,5 @@
 PACKAGE_NAME="vendor-reset"
-PACKAGE_VERSION="0.0.19"
+PACKAGE_VERSION="0.0.18"
 BUILT_MODULE_NAME[0]="${PACKAGE_NAME}"
 MAKE[0]="make KDIR=${kernel_source_dir}"
 CLEAN="make KDIR=${kernel_source_dir} clean"

src/amd/common.c

@@ -82,16 +82,10 @@ int amd_common_pre_reset(struct vendor_reset_dev *dev)
   pci_save_state(pdev);
   priv->saved_state = pci_store_saved_state(pdev);
   pci_read_config_word(pdev, PCI_COMMAND, &priv->cfg);
-  pci_write_config_word(pdev, PCI_COMMAND, priv->cfg | PCI_COMMAND_MEMORY | PCI_COMMAND_INTX_DISABLE);
+  pci_write_config_word(pdev, PCI_COMMAND, priv->cfg | PCI_COMMAND_MEMORY);
 
-  priv->audio_pdev = pci_get_domain_bus_and_slot(pci_domain_nr(pdev->bus),
+  if (!pci_wait_for_pending_transaction(pdev))
-                                                 pdev->bus->number, 1);
+    vr_warn(dev, "Timed out waiting for transaction to clear\n");
-  if (priv->audio_pdev)
-  {
-    pci_set_power_state(priv->audio_pdev, PCI_D0);
-    pci_clear_master(priv->audio_pdev);
-    pci_save_state(priv->audio_pdev);
-  }
 
   return 0;
 
@@ -105,8 +99,7 @@ int amd_common_post_reset(struct vendor_reset_dev *dev)
   struct amd_vendor_private *priv = amd_private(dev);
   struct pci_dev *pdev = dev->pdev;
 
-  if (priv->mmio)
+  if (priv->mmio) {
-  {
     iounmap(priv->mmio);
     priv->mmio = NULL;
   }
@@ -124,17 +117,9 @@ int amd_common_post_reset(struct vendor_reset_dev *dev)
   }
   pci_write_config_word(pdev, PCI_COMMAND, priv->cfg);
 
-  if (priv->audio_pdev)
-  {
-    pci_restore_state(priv->audio_pdev);
-    pci_set_power_state(priv->audio_pdev, PCI_D3hot);
-    pci_dev_put(priv->audio_pdev);
-    priv->audio_pdev = NULL;
-  }
-
   /* don't try to go to low power if reset failed */
-  if (!dev->reset_ret)
+  // if (!dev->reset_ret)
-    pci_set_power_state(pdev, PCI_D3hot);
+  //   pci_set_power_state(pdev, PCI_D3hot);
 
   kfree(priv);
   dev->vendor_private = NULL;

src/amd/common.h

@@ -30,52 +30,52 @@ Place, Suite 330, Boston, MA 02111-1307 USA
 #define DRM_DEBUG(fmt, args...) pr_debug("vendor-reset-drm: " fmt, ##args)
 static inline bool drm_can_sleep(void)
 {
-  if (in_atomic() || in_dbg_master() || irqs_disabled())
+	if (in_atomic() || in_dbg_master() || irqs_disabled())
-    return false;
+		return false;
-  return true;
+	return true;
 }
 
-#define RREG32(reg)                                                    \
+#define RREG32(reg)                                                          \
-  ({                                                                   \
+  ({                                                                         \
-    u32 __out;                                                         \
+    u32 __out;                                                               \
-    if (((reg)*4) < adev_to_amd_private(adev)->mmio_size)              \
+    if (((reg) * 4) < adev_to_amd_private(adev)->mmio_size)                        \
-      __out = readl(adev_to_amd_private(adev)->mmio + (reg));          \
+      __out = readl(adev_to_amd_private(adev)->mmio + (reg));                \
-    else                                                               \
+    else                                                                     \
-    {                                                                  \
+    {                                                                        \
-      writel(((reg)*4), adev_to_amd_private(adev)->mmio + mmMM_INDEX); \
+      writel(((reg) * 4), adev_to_amd_private(adev)->mmio + mmMM_INDEX);           \
-      __out = readl(adev_to_amd_private(adev)->mmio + mmMM_DATA);      \
+      __out = readl(adev_to_amd_private(adev)->mmio + mmMM_DATA);            \
-    }                                                                  \
+    }                                                                        \
-    __out;                                                             \
+    __out;                                                                   \
   })
 
-#define WREG32(reg, v)                                                 \
+#define WREG32(reg, v)                                                       \
-  do                                                                   \
+  do                                                                         \
-  {                                                                    \
+  {                                                                          \
-    if (((reg)*4) < adev_to_amd_private(adev)->mmio_size)              \
+    if (((reg) * 4) < adev_to_amd_private(adev)->mmio_size)                        \
-      writel(v, adev_to_amd_private(adev)->mmio + (reg));              \
+      writel(v, adev_to_amd_private(adev)->mmio + (reg));                    \
-    else                                                               \
+    else                                                                     \
-    {                                                                  \
+    {                                                                        \
-      writel(((reg)*4), adev_to_amd_private(adev)->mmio + mmMM_INDEX); \
+      writel(((reg) * 4), adev_to_amd_private(adev)->mmio + mmMM_INDEX);           \
-      writel(v, adev_to_amd_private(adev)->mmio + mmMM_DATA);          \
+      writel(v, adev_to_amd_private(adev)->mmio + mmMM_DATA);                \
-    }                                                                  \
+    }                                                                        \
   } while (0)
 
-#define WREG32_PCIE(reg, v)      \
+#define WREG32_PCIE(reg, v)                                                 \
-  do                             \
+  do                                                                        \
-  {                              \
+  {                                                                         \
-    WREG32(mmPCIE_INDEX2, reg);  \
+    WREG32(mmPCIE_INDEX2, reg);                                             \
-    (void)RREG32(mmPCIE_INDEX2); \
+    (void)RREG32(mmPCIE_INDEX2);                                            \
-    WREG32(mmPCIE_DATA2, v);     \
+    WREG32(mmPCIE_DATA2, v);                                                \
-    (void)RREG32(mmPCIE_DATA2);  \
+    (void)RREG32(mmPCIE_DATA2);                                             \
   } while (0)
 
-#define RREG32_PCIE(reg)               \
+#define RREG32_PCIE(reg)                                                    \
-  ({                                   \
+  ({                                                                        \
-    u32 __tmp_read;                    \
+    u32 __tmp_read;                                                         \
-    WREG32(mmPCIE_INDEX2, reg);        \
+    WREG32(mmPCIE_INDEX2, reg);                                             \
-    (void)RREG32(mmPCIE_INDEX2);       \
+    (void)RREG32(mmPCIE_INDEX2);                                            \
-    __tmp_read = RREG32(mmPCIE_DATA2); \
+    __tmp_read = RREG32(mmPCIE_DATA2);                                      \
-    __tmp_read;                        \
+    __tmp_read;                                                             \
   })
 
 /* KIQ is only used for SRIOV accesses, we are not targetting these devices so
@@ -117,8 +117,6 @@ struct amd_vendor_private
 {
   u16 cfg;
 
-  struct pci_dev *audio_pdev;
-
   struct vendor_reset_dev *vdev;
   struct pci_saved_state *saved_state;
   struct amd_fake_dev adev;

src/amd/firmware.h

@@ -25,4 +25,7 @@ Place, Suite 330, Boston, MA 02111-1307 USA
 int atom_bios_init(struct amd_fake_dev *adev);
 void atom_bios_fini(struct amd_fake_dev *adev);
 
+/* this is actually in amdgpu_bios.c */
+bool amdgpu_get_bios(struct amd_fake_dev *adev);
+
 #endif

src/amd/navi10.c

@@ -33,15 +33,149 @@ Place, Suite 330, Boston, MA 02111-1307 USA
 #include "nv.h"
 #include "psp_gfx_if.h"
 #include "smu_v11_0_ppsmc.h"
+#include "thm/thm_11_0_2_offset.h"
+#include "thm/thm_11_0_2_sh_mask.h"
 
-extern bool amdgpu_get_bios(struct amd_fake_dev *adev);
+enum navi10_reset_type
+{
+  NAVI10_RESET_NONE,
+  NAVI10_RESET_BACO,
+  NAVI10_RESET_MODE1,
+};
+
+static int navi10_needs_reset(struct vendor_reset_dev *dev, enum navi10_reset_type *type)
+{
+  struct amd_vendor_private *priv = amd_private(dev);
+  struct amd_fake_dev *adev = &priv->adev;
+  u32 smu_resp, mp1_intr, psp_bl_ready, sol;
+
+  /* collect some state info */
+  sol = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_81);
+  smu_resp = RREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_90);
+  mp1_intr = (RREG32_PCIE(MP1_Public |
+                          (smnMP1_FIRMWARE_FLAGS & 0xffffffff)) &
+              MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK) >>
+             MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED__SHIFT;
+  psp_bl_ready = !!(RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_35) & 0x80000000L);
+  vr_info(dev, "SMU response reg: %x, sol reg: %x, mp1 intr enabled? %s, bl ready? %s\n",
+          smu_resp, sol, mp1_intr ? "yes" : "no",
+          psp_bl_ready ? "yes" : "no");
+
+  if (!sol && !mp1_intr && psp_bl_ready)
+    /* okay, if we're in this state, we're probably reset */
+    *type = NAVI10_RESET_NONE;
+  else if (sol && sol != ~1L && smu_resp != 0 && mp1_intr && psp_bl_ready)
+    *type = NAVI10_RESET_BACO;
+  else
+    *type = NAVI10_RESET_MODE1;
+
+  return *type != NAVI10_RESET_NONE;
+}
+
+static int navi10_mode1_reset(struct vendor_reset_dev *dev)
+{
+  struct amd_vendor_private *priv = amd_private(dev);
+  struct amd_fake_dev *adev = &priv->adev;
+  u32 offset, tmp;
+  int ret = 0, timeout;
+
+  vr_info(dev, "begin psp mode 1 reset\n");
+  amdgpu_atombios_scratch_regs_engine_hung(adev, true);
+
+  pci_save_state(dev->pdev);
+
+  /* check validity of PSP before reset */
+  offset = SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_64);
+  tmp = psp_wait_for(adev, offset, 0x80000000, 0x8000FFFF, false);
+  if (tmp)
+    vr_warn(dev, "timed out waiting for PSP to reach valid state, but continuing anyway\n");
+
+  /* reset command */
+  WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_64, GFX_CTRL_CMD_ID_MODE1_RST);
+  msleep(500);
+
+  /* wait for ACK */
+  offset = SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_33);
+  tmp = psp_wait_for(adev, offset, 0x80000000, 0x80000000, false);
+  if (tmp)
+  {
+    vr_warn(dev, "PSP did not acknowledger reset\n");
+    ret = -EINVAL;
+    goto out;
+  }
+
+  vr_info(dev, "mode1 reset succeeded\n");
+
+  pci_restore_state(dev->pdev);
+
+  for (timeout = 100000; timeout; --timeout)
+  {
+    tmp = RREG32_SOC15(NBIO, 0, mmRCC_DEV0_EPF0_RCC_CONFIG_MEMSIZE);
+
+    if (tmp != 0xffffffff)
+      break;
+    udelay(1);
+  }
+
+out:
+  return ret;
+}
+
+static int navi10_wait_for_psp_ready(struct vendor_reset_dev *dev)
+{
+  struct amd_vendor_private *priv = amd_private(dev);
+  struct amd_fake_dev *adev = &priv->adev;
+  int timeout;
+
+  /*
+   * this takes a long time :(
+   */
+  for (timeout = 100; timeout; --timeout)
+  {
+    /* see if PSP bootloader comes back */
+    if (RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_35) & 0x80000000L)
+      return 0;
+    msleep(100);
+  }
+
+  return (RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_35) & 0x80000000L) ? 0 : -ETIMEDOUT;
+}
+
+static int navi10_baco_reset(struct vendor_reset_dev *dev)
+{
+  struct amd_vendor_private *priv = amd_private(dev);
+  struct amd_fake_dev *adev = &priv->adev;
+  int ret, tmp;
+
+  vr_info(dev, "Entering BACO\n");
+
+  /* BACO_SEQ_BACO */
+  ret = smum_send_msg_to_smc_with_parameter(adev, PPSMC_MSG_ArmD3, 0, NULL);
+  if (ret)
+    return ret;
+
+  tmp = RREG32_SOC15(THM, 0, mmTHM_BACO_CNTL);
+  tmp |= 0x80000000;
+  WREG32_SOC15(THM, 0, mmTHM_BACO_CNTL, tmp);
+
+  ret = smum_send_msg_to_smc_with_parameter(adev, PPSMC_MSG_EnterBaco, 0, NULL);
+  if (ret)
+    return ret;
+  msleep(500);
+
+  vr_info(dev, "Exiting BACO\n");
+  ret = smum_send_msg_to_smc_with_parameter(adev, PPSMC_MSG_ExitBaco, 0, NULL);
+
+  return ret;
+}
 
 static int amd_navi10_reset(struct vendor_reset_dev *dev)
 {
   struct amd_vendor_private *priv = amd_private(dev);
   struct amd_fake_dev *adev;
   int ret = 0, timeout;
-  u32 sol, smu_resp, mp1_intr, psp_bl_ready, tmp, offset;
+  u32 sol;
+  enum navi10_reset_type reset_type;
 
   adev = &priv->adev;
   ret = amd_fake_dev_init(adev, dev);
@@ -93,141 +227,37 @@ static int amd_navi10_reset(struct vendor_reset_dev *dev)
   }
 
   if (sol == ~1L)
-  {
-    vr_warn(dev, "Timed out waiting for SOL to be valid\n");
     /* continuing anyway because sometimes it can still be reset from here */
-  }
+    vr_warn(dev, "Timed out waiting for SOL to be valid\n");
 
-  vr_info(dev, "bus reset disabled? %s\n", (dev->pdev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET) ? "yes" : "no");
+  if (!navi10_needs_reset(dev, &reset_type))
-
-  /* collect some info for logging for now */
-  smu_resp = RREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_90);
-  mp1_intr = (RREG32_PCIE(MP1_Public |
-                          (smnMP1_FIRMWARE_FLAGS & 0xffffffff)) &
-              MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK) >>
-             MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED__SHIFT;
-  psp_bl_ready = !!(RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_35) & 0x80000000L);
-  vr_info(dev, "SMU response reg: %x, sol reg: %x, mp1 intr enabled? %s, bl ready? %s\n",
-          smu_resp, sol, mp1_intr ? "yes" : "no",
-          psp_bl_ready ? "yes" : "no");
-
-  /* okay, if we're in this state, we're probably reset */
-  if (sol == 0x0 && !mp1_intr && psp_bl_ready)
     goto free_adev;
 
-  /* this tells the drivers nvram is lost and everything needs to be reset */
+  if (reset_type == NAVI10_RESET_BACO)
-  vr_info(dev, "Clearing scratch regs 6 and 7\n");
+    ret = navi10_baco_reset(dev);
-  WREG32(adev->bios_scratch_reg_offset + 6, 0);
-  WREG32(adev->bios_scratch_reg_offset + 7, 0);
-
-  /* it only makes sense to reset mp1 if it's running
-   * XXX: is this even necessary? in early testing, I ran into
-   * situations where MP1 was alive but not responsive, but in
-   * later testing I have not been able to replicate this scenario.
-   */
-  if (smu_resp != 0x01 && mp1_intr)
-  {
-    vr_info(dev, "MP1 reset\n");
-    WREG32_PCIE(MP1_Public | (smnMP1_PUB_CTRL & 0xffffffff),
-                1 & MP1_SMN_PUB_CTRL__RESET_MASK);
-    WREG32_PCIE(MP1_Public | (smnMP1_PUB_CTRL & 0xffffffff),
-                1 & ~MP1_SMN_PUB_CTRL__RESET_MASK);
-
-    vr_info(dev, "wait for MP1\n");
-    for (timeout = 100000; timeout; --timeout)
-    {
-      tmp = RREG32_PCIE(MP1_Public |
-                        (smnMP1_FIRMWARE_FLAGS & 0xffffffff));
-      if ((tmp &
-           MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK) >>
-          MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED__SHIFT)
-        break;
-      udelay(1);
-    }
-
-    if (!timeout &&
-        !((tmp & MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED_MASK) >>
-          MP1_FIRMWARE_FLAGS__INTERRUPTS_ENABLED__SHIFT))
-    {
-      vr_warn(dev, "timed out waiting for MP1 reset\n");
-    }
-
-    smu_wait(adev);
-    smu_resp = RREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_90);
-    vr_info(dev, "SMU resp reg: %x\n", tmp);
-  }
-
-  /*
-   * again, this only makes sense if we have an SMU to talk to
-   * some of these may fail, that's okay. we're just turning off as many
-   * things as possible
-   */
-  if (mp1_intr)
-  {
-    smum_send_msg_to_smc(adev, PPSMC_MSG_DisallowGfxOff, NULL);
-    smum_send_msg_to_smc(adev, PPSMC_MSG_PrepareMp1ForReset, NULL);
-  }
-
-  vr_info(dev, "begin psp mode 1 reset\n");
-  amdgpu_atombios_scratch_regs_engine_hung(adev, true);
-
-  pci_save_state(dev->pdev);
-
-  /* check validity of PSP before reset */
-  offset = SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_64);
-  tmp = psp_wait_for(adev, offset, 0x80000000, 0x8000FFFF, false);
-  if (tmp)
-    vr_warn(dev, "timed out waiting for PSP to reach valid state, but continuing anyway\n");
-
-  /* reset command */
-  WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_64, GFX_CTRL_CMD_ID_MODE1_RST);
-  msleep(500);
-
-  /* wait for ACK */
-  offset = SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_33);
-  tmp = psp_wait_for(adev, offset, 0x80000000, 0x80000000, false);
-  if (tmp)
-  {
-    vr_warn(dev, "PSP did not acknowledger reset\n");
-    ret = -EINVAL;
-    goto out;
-  }
-
-  vr_info(dev, "mode1 reset succeeded\n");
-
-  pci_restore_state(dev->pdev);
-
-  for (timeout = 100000; timeout; --timeout)
-  {
-    tmp = RREG32_SOC15(NBIO, 0, mmRCC_DEV0_EPF0_RCC_CONFIG_MEMSIZE);
-
-    if (tmp != 0xffffffff)
-      break;
-    udelay(1);
-  }
-
-  /*
-   * this takes a long time :(
-   */
-  for (timeout = 100; timeout; --timeout)
-  {
-    /* see if PSP bootloader comes back */
-    if (RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_35) & 0x80000000L)
-      break;
-    msleep(100);
-  }
-
-  if (!timeout && !(RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_35) & 0x80000000L))
-  {
-    vr_warn(dev, "timed out waiting for PSP bootloader to respond after reset\n");
-    ret = -ETIME;
-  }
   else
-    vr_info(dev, "PSP mode1 reset successful\n");
+  {
+    ret = navi10_mode1_reset(dev);
+    if (ret)
+      goto mode1_out;
 
-out:
+    if ((ret = navi10_wait_for_psp_ready(dev)))
-  pci_restore_state(dev->pdev);
+      vr_warn(dev, "timed out waiting for PSP bootloader to respond after reset\n");
-  amdgpu_atombios_scratch_regs_engine_hung(adev, false);
+    else
+      vr_info(dev, "PSP mode1 reset successful\n");
+
+  mode1_out:
+    pci_restore_state(dev->pdev);
+    amdgpu_atombios_scratch_regs_engine_hung(adev, false);
+  }
+
+  if (!ret)
+  {
+    /* this tells the drivers nvram is lost and everything needs to be reset */
+    vr_info(dev, "Clearing scratch regs 6 and 7\n");
+    WREG32(adev->bios_scratch_reg_offset + 6, 0);
+    WREG32(adev->bios_scratch_reg_offset + 7, 0);
+  }
 
 free_adev:
   amd_fake_dev_fini(adev);
@@ -235,11 +265,10 @@ free_adev:
   return ret;
 }
 
-const struct vendor_reset_ops amd_navi10_ops =
+const struct vendor_reset_ops amd_navi10_ops = {
-{
+    .version = {2, 0},
-  .version = {1, 1},
+    .probe = amd_common_probe,
-  .probe = amd_common_probe,
+    .pre_reset = amd_common_pre_reset,
-  .pre_reset = amd_common_pre_reset,
+    .reset = amd_navi10_reset,
-  .reset = amd_navi10_reset,
+    .post_reset = amd_common_post_reset,
-  .post_reset = amd_common_post_reset,
 };

src/ioctl.c

@@ -21,7 +21,6 @@ Place, Suite 330, Boston, MA 02111-1307 USA
 #include "vendor-reset-ioctl.h"
 
 #include <linux/miscdevice.h>
-#include <linux/uaccess.h>
 
 #define VENDOR_RESET_IOCTL_DEVNAME "vendor_reset"