Re: [PATCH 2/3] drm/amdgpu: Add USBC PD FW load to PSP 11

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 3/2/20 4:30 PM, Alex Deucher wrote:
On Mon, Mar 2, 2020 at 2:24 PM Andrey Grodzovsky
<andrey.grodzovsky@xxxxxxx> wrote:
Add the programming sequence.

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
---
  drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 76 ++++++++++++++++++++++++++++++++++
  1 file changed, 76 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index 8ab3bf3..621b99c 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -65,6 +65,9 @@ MODULE_FIRMWARE("amdgpu/arcturus_ta.bin");
  /* memory training timeout define */
  #define MEM_TRAIN_SEND_MSG_TIMEOUT_US  3000000

+/* USBC PD FW version retrieval command */
+#define C2PMSG_CMD_GFX_USB_PD_FW_VER 0x2000000
+
This probably belongs in psp_gfx_if.h.

  static int psp_v11_0_init_microcode(struct psp_context *psp)
  {
         struct amdgpu_device *adev = psp->adev;
@@ -1109,6 +1112,77 @@ static void psp_v11_0_ring_set_wptr(struct psp_context *psp, uint32_t value)
                 WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_67, value);
  }

+static int psp_v11_0_load_usbc_pd_fw(struct psp_context *psp, dma_addr_t dma_addr)
+{
+       struct amdgpu_device *adev = psp->adev;
+       uint32_t reg_status;
+       int ret;
+
+       /* Write lower 32-bit address of the PD Controller FW */
+       WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36, lower_32_bits(dma_addr));
+       ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_35),
+                            0x80000000, 0x80000000, false);
+       if (ret)
+               return ret;
+
+        // Fireup interrupt so PSP can pick up the lower address
C style comments please.

+       WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_35, 0x800000);
+       ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_35),
+                            0x80000000, 0x80000000, false);
+       if (ret)
+               return ret;
+
+       reg_status = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_35);
+
+       if ((reg_status & 0xFFFF) != 0) {
+               DRM_ERROR("Lower address load failed - MP0_SMN_C2PMSG_35.Bits [15:0] = %02x...\n",
+                               reg_status & 0xFFFF);
+               return -EIO;
+       }
+
+       /* Write upper 32-bit address of the PD Controller FW */
+       WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36, upper_32_bits(dma_addr));
+
+
+       ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_35),
+                            0x80000000, 0x80000000, false);
+       if (ret)
+               return ret;
+
+        // Fireup interrupt so PSP can pick up the upper address
C style comments.

+       WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_35, 0x4000000);
+
+       /* FW load takes long time */
+       while(psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_35),
+                                            0x80000000, 0x80000000, false))
+               msleep(1000);
+
Are we actually waiting for the full loading here or just the ack of
the command?

+       reg_status = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_35);
+
+       if ((reg_status & 0xFFFF) != 0) {
+               DRM_ERROR("Upper address load failed - MP0_SMN_C2PMSG_35.Bits [15:0] = %02x...\n",
+                               reg_status & 0xFFFF);
+               return -EIO;
+       }
+
+       return 0;
+}
+
+static int psp_v11_0_read_usbc_pd_fw(struct psp_context *psp, uint32_t *fw_ver)
+{
+       struct amdgpu_device *adev = psp->adev;
+
+       WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_35, C2PMSG_CMD_GFX_USB_PD_FW_VER);
+
+       while(psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_35),
+                                    0x80000000, 0x80000000, false))
+               msleep(1);
+
+       *fw_ver = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36);
+
+       return 0;
+}
I think we need locking for mmMP0_SMN_C2PMSG_35/mmMP0_SMN_C2PMSG_36
since they are the mailbox registers for communicating with the PSP.
Maybe just a generic psp lock since I don't think we need fine grained
locking for psp.

Alex


I wonder why we don't lock in any other place we access them (e.g. psp_v11_0_memory_training_send_msg or psp_v11_0_bootloader_load_sos) ?

Andrey


+
  static const struct psp_funcs psp_v11_0_funcs = {
         .init_microcode = psp_v11_0_init_microcode,
         .bootloader_load_kdb = psp_v11_0_bootloader_load_kdb,
@@ -1133,6 +1207,8 @@ static const struct psp_funcs psp_v11_0_funcs = {
         .mem_training = psp_v11_0_memory_training,
         .ring_get_wptr = psp_v11_0_ring_get_wptr,
         .ring_set_wptr = psp_v11_0_ring_set_wptr,
+       .load_usbc_pd_fw = psp_v11_0_load_usbc_pd_fw,
+       .read_usbc_pd_fw = psp_v11_0_read_usbc_pd_fw
  };

  void psp_v11_0_set_psp_funcs(struct psp_context *psp)
--
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Candrey.grodzovsky%40amd.com%7Cf3c9cee87052401e07f508d7bef10174%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637187814630848721&amp;sdata=Mo9%2BR4Nrh0rPxZ7xJrBzHl71M6rWL7KcrgEWOne6yQE%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux