RE: [PATCH] drm/amdgpu: Add a chunk ID for spm trace

[He, Jacob]

[AMD Official Use Only - Internal Distribution Only]

Vulkan UMD makes sure that the SPM_VMID will be updated if the application 
enables the SPM. So there is no this kind of problem for Vulkan application. 
It’s kind of “atomic” operation to enable SPM and update SPM_VMID. Vulkan 
application even is not aware of SPM_VMID.
If there is any other UMD applications, such as OCL, and OCL driver doesn’t 
update the SPM_VMID before enable SPM, there is VMFault anyway even Vulkan 
driver restore the SPM_VMID to 0.

Thanks
Jacob

From: Koenig, Christian
Sent: Tuesday, February 25, 2020 2:51 AM
To: He, Jacob; Zhou, 
David(ChunMing); Deucher, 
Alexander; Christian 
König; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Add a chunk ID for spm trace

Leave the old spm_vmid there is not a problem because other new spm enabled job 
will update it before it’s running and other spm disabled job will not access 
spm_vmid register.
Do you have a high level description of how SPM works? For example would it be 
possible to cause a problem in another application if we leave the SPM VMID 
here and the VMID is reused?

Keep in mind that we need to guarantee process isolation, e.g. we can't allow 
anything bad to happen to another application if somebody is stupid enough to 
turn on SPM tracing without setting it up properly.

Regards,
Christian.

Am 24.02.20 um 09:06 schrieb He, Jacob:
Ok, I’m glad that there is no uapi change needed.
I checked the git log, and reserve_vmid was added for shader debugger, not gpa. 
I’m fine to re-use it since the spm will not enabled for shader debug in 
general. I’ll try to change my patch.
BTW, “ring->funcs->setup_spm(ring, NULL)” to clear the vmid is not gonna work 
since the job with spm enabled execution is not done yet. Leave the old 
spm_vmid there is not a problem because other new spm enabled job will update 
it before it’s running and other spm disabled job will not access spm_vmid 
register.

Thanks
Jacob

From: Zhou, David(ChunMing)
Sent: Saturday, February 22, 2020 12:45 AM
To: Koenig, Christian; Deucher, 
Alexander; Christian 
König; He, 
Jacob; 
amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amdgpu: Add a chunk ID for spm trace


[AMD Official Use Only - Internal Distribution Only]

That’s fine to me.

-David

From: Koenig, Christian 

Sent: Friday, February 21, 2020 11:33 PM
To: Deucher, Alexander 
; Christian König 
; 
Zhou, David(ChunMing) ; He, 
Jacob ; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Add a chunk ID for spm trace

I would just do this as part of the vm_flush() callback on the ring.

E.g. check if the VMID you want to flush is reserved and if yes enable SPM.

Maybe pass along a flag or something in the job to make things easier.

Christian.

Am 21.02.20 um 16:31 schrieb Deucher, Alexander:

[AMD Public Use]

We already have the RESERVE_VMID ioctl interface, can't we just use that 
internally in the kernel to update the rlc register via the ring when we 
schedule the relevant IB?  E.g., add a new ring callback to set SPM state and 
then set it to the reserved vmid before we schedule the ib, and then reset it 
to 0 after the IB in amdgpu_ib_schedule().

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 4b2342d11520..e0db9362c6ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -185,6 +185,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
num_ibs,
if (ring->funcs->insert_start)
ring->funcs->insert_start(ring);

+   if (ring->funcs->setup_spm)
+   ring->funcs->setup_spm(ring, job);
+
if (job) {
r = amdgpu_vm_flush(ring, job, need_pipe_sync);
if (r) {
@@ -273,6 +276,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
num_ibs,
return r;
}

+   if (ring->funcs->setup_spm)
+   ring->funcs->setup_spm(ring, NULL);
+
if (ring->funcs->insert_end)
ring->funcs->insert_end(ring);



Alex

From: amd-gfx 

 on behalf of Christian König 

Sent: Friday, February 21, 2020 5:28 AM
To: Zhou, David(ChunMing) ; 
He, Jacob ; Koenig, Christian 

RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence V2

[Zhang, Hawking]

[AMD Official Use Only - Internal Distribution Only]

Yes, we are not 100% confident on the sequence of SETUP_VMR command in guest 
driver, although it indeed happens after PMFW loading in GPUV driver.

I think now the concern was removed now. The current sequence should work in 
guest driver as well, although they have further discussion on whether removing 
SETUP_VMR command or not...

Please commit your patch. And thanks for the effort to get this landed and 
validated on various ASICs.

Regards,
Hawking
-Original Message-
From: Quan, Evan  
Sent: Tuesday, February 25, 2020 11:39
To: Zhang, Hawking ; amd-gfx@lists.freedesktop.org; Min, 
Frank 
Cc: Feng, Kenneth 
Subject: RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence V2

Hi Hawking,

Do you mean SRIOV may need this change also? 
Currently this covers bare-metal case only(SRIOV still follows old firmwares 
loading sequence).

Regards,
Evan
-Original Message-
From: Zhang, Hawking  
Sent: Monday, February 24, 2020 7:34 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org; Min, Frank 

Cc: Feng, Kenneth 
Subject: RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence V2

[AMD Official Use Only - Internal Distribution Only]

The patch looks good for bare-metal case now. Frank is still on the way to 
clean up our concern on SETUP_VMR command (i.e. SETUP_TMR for bare-metal case). 
After that settled, please push the patch with my RB. Thanks.

Regards,
Hawking
-Original Message-
From: Quan, Evan  
Sent: Monday, February 24, 2020 18:36
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Feng, Kenneth 
; Quan, Evan 
Subject: [PATCH] drm/amdgpu: update psp firmwares loading sequence V2

For those ASICs with DF Cstate management centralized to PMFW, TMR setup should 
be performed between pmfw loading and other non-psp firmwares loading.

V2: skip possible SMU firmware reloading

Change-Id: I8986ddb4d9ffe63ed0823d1dce8d9d52812a1240
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 65 ++---  
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 +
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 51839ab02b84..d33f74100094 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -38,6 +38,39 @@
 
 static void psp_set_funcs(struct amdgpu_device *adev);
 
+/*
+ * Due to DF Cstate management centralized to PMFW, the firmware
+ * loading sequence will be updated as below:
+ *   - Load KDB
+ *   - Load SYS_DRV
+ *   - Load tOS
+ *   - Load PMFW
+ *   - Setup TMR
+ *   - Load other non-psp fw
+ *   - Load ASD
+ *   - Load XGMI/RAS/HDCP/DTM TA if any
+ *
+ * This new sequence is required for
+ *   - Arcturus
+ *   - Navi12 and onwards
+ */
+static void psp_check_pmfw_centralized_cstate_management(struct 
+psp_context *psp) {
+   struct amdgpu_device *adev = psp->adev;
+
+   psp->pmfw_centralized_cstate_management = false;
+
+   if (amdgpu_sriov_vf(adev))
+   return;
+
+   if (adev->flags & AMD_IS_APU)
+   return;
+
+   if ((adev->asic_type == CHIP_ARCTURUS) ||
+   (adev->asic_type >= CHIP_NAVI12))
+   psp->pmfw_centralized_cstate_management = true; }
+
 static int psp_early_init(void *handle)  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle; @@ -75,6 
+108,8 @@ static int psp_early_init(void *handle)
 
psp->adev = adev;
 
+   psp_check_pmfw_centralized_cstate_management(psp);
+
return 0;
 }
 
@@ -1116,10 +1151,17 @@ static int psp_hw_start(struct psp_context *psp)
return ret;
}
 
-   ret = psp_tmr_load(psp);
-   if (ret) {
-   DRM_ERROR("PSP load tmr failed!\n");
-   return ret;
+   /*
+* For those ASICs with DF Cstate management centralized
+* to PMFW, TMR setup should be performed after PMFW
+* loaded and before other non-psp firmware loaded.
+*/
+   if (!psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
}
 
return 0;
@@ -1316,7 +1358,8 @@ static int psp_np_fw_load(struct psp_context *psp)
struct amdgpu_firmware_info *ucode;
struct amdgpu_device* adev = psp->adev;
 
-   if (psp->autoload_supported) {
+   if (psp->autoload_supported ||
+   psp->pmfw_centralized_cstate_management) {
ucode = >firmware.ucode[AMDGPU_UCODE_ID_SMC];
if (!ucode->fw || amdgpu_sriov_vf(adev))
goto out;
@@ -1326,6 +1369,14 @@ static int psp_np_fw_load(struct psp_context *psp)
return ret;
}
 
+   if (psp->pmfw_centralized_cstate_management) {
+   ret = 

RE: [PATCH] drm/amdgpu: log TA versions on init

[Zhang, Hawking]

[AMD Official Use Only - Internal Distribution Only]

Specific for your case, you just need add dtm and hdcp ta version in 
amdgpu_debugfs_firmware_info, Similar as xgmi and ras ta. The following cmd 
will give you all the firmware version information.

sudo cat /sys/kernel/debug/dri/x/amdgpu_firmware_info

Regards,
Hawking

-Original Message-
From: Zhang, Hawking 
Sent: Tuesday, February 25, 2020 14:27
To: Liu, Zhan ; Lakha, Bhawanpreet 
; amd-gfx@lists.freedesktop.org; Deucher, Alexander 

Cc: Lakha, Bhawanpreet 
Subject: RE: [PATCH] drm/amdgpu: log TA versions on init

[AMD Official Use Only - Internal Distribution Only]

Hold on please. 

I don't think this is the best approach as we already had existing debugfs 
interface for that purpose. We shall centralize all the firmware information 
query under amdgpu_debugfs_firmware_info, and user should go through debugfs to 
query firmware information.

Regards,
Hawking
-Original Message-
From: Liu, Zhan  
Sent: Tuesday, February 25, 2020 04:55
To: Lakha, Bhawanpreet ; 
amd-gfx@lists.freedesktop.org; Deucher, Alexander ; 
Zhang, Hawking 
Cc: Lakha, Bhawanpreet 
Subject: RE: [PATCH] drm/amdgpu: log TA versions on init


> -Original Message-
> From: amd-gfx  On Behalf Of 
> Bhawanpreet Lakha
> Sent: 2020/February/24, Monday 2:45 PM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander 
> ; Zhang, Hawking 
> Cc: Lakha, Bhawanpreet 
> Subject: [PATCH] drm/amdgpu: log TA versions on init
> 
> It is helpful to know what version the TA's are for debugging
> 
> Signed-off-by: Bhawanpreet Lakha 

Reviewed-by: Zhan Liu 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index a16c8101e250..09d1433677a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -358,6 +358,7 @@ static int psp_asd_load(struct psp_context *psp)
>   if (!ret) {
>   psp->asd_context.asd_initialized = true;
>   psp->asd_context.session_id = cmd->resp.session_id;
> + DRM_INFO("ASD: Initialized (version: 0x%X)", psp-
> >asd_fw_version);
>   }
> 
>   kfree(cmd);
> @@ -518,6 +519,7 @@ static int psp_xgmi_load(struct psp_context *psp)
>   if (!ret) {
>   psp->xgmi_context.initialized = 1;
>   psp->xgmi_context.session_id = cmd->resp.session_id;
> + DRM_INFO("XGMI: Initialized (version: 0x%X)", psp-
> >ta_xgmi_ucode_version);
>   }
> 
>   kfree(cmd);
> @@ -658,6 +660,7 @@ static int psp_ras_load(struct psp_context *psp)
>   if (!ret) {
>   psp->ras.ras_initialized = true;
>   psp->ras.session_id = cmd->resp.session_id;
> + DRM_INFO("RAS: Initialized (version: 0x%X)", psp-
> >ta_ras_ucode_version);
>   }
> 
>   kfree(cmd);
> @@ -832,6 +835,7 @@ static int psp_hdcp_load(struct psp_context *psp)
>   if (!ret) {
>   psp->hdcp_context.hdcp_initialized = true;
>   psp->hdcp_context.session_id = cmd->resp.session_id;
> + DRM_INFO("HDCP: Initialized (version: 0x%X)", psp-
> >ta_hdcp_ucode_version);
>   }
> 
>   kfree(cmd);
> @@ -977,6 +981,7 @@ static int psp_dtm_load(struct psp_context *psp)
>   if (!ret) {
>   psp->dtm_context.dtm_initialized = true;
>   psp->dtm_context.session_id = cmd->resp.session_id;
> + DRM_INFO("DTM: Initialized (version: 0x%X)", psp-
> >ta_dtm_ucode_version);
>   }
> 
>   kfree(cmd);
> --
> 2.17.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: log TA versions on init

[Zhang, Hawking]

[AMD Official Use Only - Internal Distribution Only]

Hold on please. 

I don't think this is the best approach as we already had existing debugfs 
interface for that purpose. We shall centralize all the firmware information 
query under amdgpu_debugfs_firmware_info, and user should go through debugfs to 
query firmware information.

Regards,
Hawking
-Original Message-
From: Liu, Zhan  
Sent: Tuesday, February 25, 2020 04:55
To: Lakha, Bhawanpreet ; 
amd-gfx@lists.freedesktop.org; Deucher, Alexander ; 
Zhang, Hawking 
Cc: Lakha, Bhawanpreet 
Subject: RE: [PATCH] drm/amdgpu: log TA versions on init


> -Original Message-
> From: amd-gfx  On Behalf Of 
> Bhawanpreet Lakha
> Sent: 2020/February/24, Monday 2:45 PM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander 
> ; Zhang, Hawking 
> Cc: Lakha, Bhawanpreet 
> Subject: [PATCH] drm/amdgpu: log TA versions on init
> 
> It is helpful to know what version the TA's are for debugging
> 
> Signed-off-by: Bhawanpreet Lakha 

Reviewed-by: Zhan Liu 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index a16c8101e250..09d1433677a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -358,6 +358,7 @@ static int psp_asd_load(struct psp_context *psp)
>   if (!ret) {
>   psp->asd_context.asd_initialized = true;
>   psp->asd_context.session_id = cmd->resp.session_id;
> + DRM_INFO("ASD: Initialized (version: 0x%X)", psp-
> >asd_fw_version);
>   }
> 
>   kfree(cmd);
> @@ -518,6 +519,7 @@ static int psp_xgmi_load(struct psp_context *psp)
>   if (!ret) {
>   psp->xgmi_context.initialized = 1;
>   psp->xgmi_context.session_id = cmd->resp.session_id;
> + DRM_INFO("XGMI: Initialized (version: 0x%X)", psp-
> >ta_xgmi_ucode_version);
>   }
> 
>   kfree(cmd);
> @@ -658,6 +660,7 @@ static int psp_ras_load(struct psp_context *psp)
>   if (!ret) {
>   psp->ras.ras_initialized = true;
>   psp->ras.session_id = cmd->resp.session_id;
> + DRM_INFO("RAS: Initialized (version: 0x%X)", psp-
> >ta_ras_ucode_version);
>   }
> 
>   kfree(cmd);
> @@ -832,6 +835,7 @@ static int psp_hdcp_load(struct psp_context *psp)
>   if (!ret) {
>   psp->hdcp_context.hdcp_initialized = true;
>   psp->hdcp_context.session_id = cmd->resp.session_id;
> + DRM_INFO("HDCP: Initialized (version: 0x%X)", psp-
> >ta_hdcp_ucode_version);
>   }
> 
>   kfree(cmd);
> @@ -977,6 +981,7 @@ static int psp_dtm_load(struct psp_context *psp)
>   if (!ret) {
>   psp->dtm_context.dtm_initialized = true;
>   psp->dtm_context.session_id = cmd->resp.session_id;
> + DRM_INFO("DTM: Initialized (version: 0x%X)", psp-
> >ta_dtm_ucode_version);
>   }
> 
>   kfree(cmd);
> --
> 2.17.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence V2

[Quan, Evan]

Hi Hawking,

Do you mean SRIOV may need this change also? 
Currently this covers bare-metal case only(SRIOV still follows old firmwares 
loading sequence).

Regards,
Evan
-Original Message-
From: Zhang, Hawking  
Sent: Monday, February 24, 2020 7:34 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org; Min, Frank 

Cc: Feng, Kenneth 
Subject: RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence V2

[AMD Official Use Only - Internal Distribution Only]

The patch looks good for bare-metal case now. Frank is still on the way to 
clean up our concern on SETUP_VMR command (i.e. SETUP_TMR for bare-metal case). 
After that settled, please push the patch with my RB. Thanks.

Regards,
Hawking
-Original Message-
From: Quan, Evan  
Sent: Monday, February 24, 2020 18:36
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Feng, Kenneth 
; Quan, Evan 
Subject: [PATCH] drm/amdgpu: update psp firmwares loading sequence V2

For those ASICs with DF Cstate management centralized to PMFW, TMR setup should 
be performed between pmfw loading and other non-psp firmwares loading.

V2: skip possible SMU firmware reloading

Change-Id: I8986ddb4d9ffe63ed0823d1dce8d9d52812a1240
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 65 ++---  
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 +
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 51839ab02b84..d33f74100094 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -38,6 +38,39 @@
 
 static void psp_set_funcs(struct amdgpu_device *adev);
 
+/*
+ * Due to DF Cstate management centralized to PMFW, the firmware
+ * loading sequence will be updated as below:
+ *   - Load KDB
+ *   - Load SYS_DRV
+ *   - Load tOS
+ *   - Load PMFW
+ *   - Setup TMR
+ *   - Load other non-psp fw
+ *   - Load ASD
+ *   - Load XGMI/RAS/HDCP/DTM TA if any
+ *
+ * This new sequence is required for
+ *   - Arcturus
+ *   - Navi12 and onwards
+ */
+static void psp_check_pmfw_centralized_cstate_management(struct 
+psp_context *psp) {
+   struct amdgpu_device *adev = psp->adev;
+
+   psp->pmfw_centralized_cstate_management = false;
+
+   if (amdgpu_sriov_vf(adev))
+   return;
+
+   if (adev->flags & AMD_IS_APU)
+   return;
+
+   if ((adev->asic_type == CHIP_ARCTURUS) ||
+   (adev->asic_type >= CHIP_NAVI12))
+   psp->pmfw_centralized_cstate_management = true; }
+
 static int psp_early_init(void *handle)  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle; @@ -75,6 
+108,8 @@ static int psp_early_init(void *handle)
 
psp->adev = adev;
 
+   psp_check_pmfw_centralized_cstate_management(psp);
+
return 0;
 }
 
@@ -1116,10 +1151,17 @@ static int psp_hw_start(struct psp_context *psp)
return ret;
}
 
-   ret = psp_tmr_load(psp);
-   if (ret) {
-   DRM_ERROR("PSP load tmr failed!\n");
-   return ret;
+   /*
+* For those ASICs with DF Cstate management centralized
+* to PMFW, TMR setup should be performed after PMFW
+* loaded and before other non-psp firmware loaded.
+*/
+   if (!psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
}
 
return 0;
@@ -1316,7 +1358,8 @@ static int psp_np_fw_load(struct psp_context *psp)
struct amdgpu_firmware_info *ucode;
struct amdgpu_device* adev = psp->adev;
 
-   if (psp->autoload_supported) {
+   if (psp->autoload_supported ||
+   psp->pmfw_centralized_cstate_management) {
ucode = >firmware.ucode[AMDGPU_UCODE_ID_SMC];
if (!ucode->fw || amdgpu_sriov_vf(adev))
goto out;
@@ -1326,6 +1369,14 @@ static int psp_np_fw_load(struct psp_context *psp)
return ret;
}
 
+   if (psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
+   }
+
 out:
for (i = 0; i < adev->firmware.max_ucodes; i++) {
ucode = >firmware.ucode[i];
@@ -1333,7 +1384,9 @@ static int psp_np_fw_load(struct psp_context *psp)
continue;
 
if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
-   (psp_smu_reload_quirk(psp) || psp->autoload_supported))
+   (psp_smu_reload_quirk(psp) ||
+psp->autoload_supported ||
+psp->pmfw_centralized_cstate_management))
continue;
 
if 

[PATCH 5/6] drm/amdkfd: Delete excessive printings

[Yong Zhao]

Those printings are duplicated or useless.

Change-Id: I88fbe8f5748bbd0a20bcf1f6ca67b9dde99733fe
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  | 2 --
 drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 4 +---
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index a3c44d88314b..958275db3f55 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -297,8 +297,6 @@ static int create_queue_nocpsch(struct device_queue_manager 
*dqm,
struct mqd_manager *mqd_mgr;
int retval;
 
-   print_queue(q);
-
dqm_lock(dqm);
 
if (dqm->total_queue_count >= max_num_of_queues_per_device) {
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index c604a2ede3f5..3bfa5c8d9654 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -257,7 +257,6 @@ int pqm_create_queue(struct process_queue_manager *pqm,
pqn->q = q;
pqn->kq = NULL;
retval = dev->dqm->ops.create_queue(dev->dqm, q, >qpd);
-   pr_debug("DQM returned %d for create_queue\n", retval);
print_queue(q);
break;
 
@@ -278,7 +277,6 @@ int pqm_create_queue(struct process_queue_manager *pqm,
pqn->q = q;
pqn->kq = NULL;
retval = dev->dqm->ops.create_queue(dev->dqm, q, >qpd);
-   pr_debug("DQM returned %d for create_queue\n", retval);
print_queue(q);
break;
case KFD_QUEUE_TYPE_DIQ:
@@ -299,7 +297,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
}
 
if (retval != 0) {
-   pr_err("Pasid 0x%x DQM create queue %d failed. ret %d\n",
+   pr_err("Pasid 0x%x DQM create queue type %d failed. ret %d\n",
pqm->process->pasid, type, retval);
goto err_create_queue;
}
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/6] drm/amdkfd: Rename queue_count to active_queue_count

[Yong Zhao]

The name is easier to understand the code.

Change-Id: I9064dab1d022e02780023131f940fff578a06b72
Signed-off-by: Yong Zhao 
---
 .../drm/amd/amdkfd/kfd_device_queue_manager.c | 38 +--
 .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 +-
 .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   |  4 +-
 .../amd/amdkfd/kfd_process_queue_manager.c|  2 +-
 4 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 80d22bf702e8..7ef9b89f5c70 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -359,7 +359,7 @@ static int create_queue_nocpsch(struct device_queue_manager 
*dqm,
list_add(>list, >queues_list);
qpd->queue_count++;
if (q->properties.is_active)
-   dqm->queue_count++;
+   dqm->active_queue_count++;
 
if (q->properties.type == KFD_QUEUE_TYPE_SDMA)
dqm->sdma_queue_count++;
@@ -494,7 +494,7 @@ static int destroy_queue_nocpsch_locked(struct 
device_queue_manager *dqm,
}
qpd->queue_count--;
if (q->properties.is_active)
-   dqm->queue_count--;
+   dqm->active_queue_count--;
 
return retval;
 }
@@ -563,13 +563,13 @@ static int update_queue(struct device_queue_manager *dqm, 
struct queue *q)
/*
 * check active state vs. the previous state and modify
 * counter accordingly. map_queues_cpsch uses the
-* dqm->queue_count to determine whether a new runlist must be
+* dqm->active_queue_count to determine whether a new runlist must be
 * uploaded.
 */
if (q->properties.is_active && !prev_active)
-   dqm->queue_count++;
+   dqm->active_queue_count++;
else if (!q->properties.is_active && prev_active)
-   dqm->queue_count--;
+   dqm->active_queue_count--;
 
if (dqm->sched_policy != KFD_SCHED_POLICY_NO_HWS)
retval = map_queues_cpsch(dqm);
@@ -618,7 +618,7 @@ static int evict_process_queues_nocpsch(struct 
device_queue_manager *dqm,
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
q->properties.is_active = false;
-   dqm->queue_count--;
+   dqm->active_queue_count--;
 
if (WARN_ONCE(!dqm->sched_running, "Evict when stopped\n"))
continue;
@@ -662,7 +662,7 @@ static int evict_process_queues_cpsch(struct 
device_queue_manager *dqm,
continue;
 
q->properties.is_active = false;
-   dqm->queue_count--;
+   dqm->active_queue_count--;
}
retval = execute_queues_cpsch(dqm,
qpd->is_debug ?
@@ -731,7 +731,7 @@ static int restore_process_queues_nocpsch(struct 
device_queue_manager *dqm,
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
q->properties.is_active = true;
-   dqm->queue_count++;
+   dqm->active_queue_count++;
 
if (WARN_ONCE(!dqm->sched_running, "Restore when stopped\n"))
continue;
@@ -786,7 +786,7 @@ static int restore_process_queues_cpsch(struct 
device_queue_manager *dqm,
continue;
 
q->properties.is_active = true;
-   dqm->queue_count++;
+   dqm->active_queue_count++;
}
retval = execute_queues_cpsch(dqm,
KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
@@ -899,7 +899,7 @@ static int initialize_nocpsch(struct device_queue_manager 
*dqm)
 
mutex_init(>lock_hidden);
INIT_LIST_HEAD(>queues);
-   dqm->queue_count = dqm->next_pipe_to_allocate = 0;
+   dqm->active_queue_count = dqm->next_pipe_to_allocate = 0;
dqm->sdma_queue_count = 0;
dqm->xgmi_sdma_queue_count = 0;
 
@@ -924,7 +924,7 @@ static void uninitialize(struct device_queue_manager *dqm)
 {
int i;
 
-   WARN_ON(dqm->queue_count > 0 || dqm->processes_count > 0);
+   WARN_ON(dqm->active_queue_count > 0 || dqm->processes_count > 0);
 
kfree(dqm->allocated_queues);
for (i = 0 ; i < KFD_MQD_TYPE_MAX ; i++)
@@ -1064,7 +1064,7 @@ static int initialize_cpsch(struct device_queue_manager 
*dqm)
 
mutex_init(>lock_hidden);
INIT_LIST_HEAD(>queues);
-   dqm->queue_count = dqm->processes_count = 0;
+   dqm->active_queue_count = dqm->processes_count = 0;
dqm->sdma_queue_count = 0;
dqm->xgmi_sdma_queue_count = 0;
dqm->active_runlist = false;
@@ -1158,7 +1158,7 @@ static int create_kernel_queue_cpsch(struct 
device_queue_manager *dqm,

[PATCH 6/6] drm/amdkfd: Delete unnecessary unmap queue package submissions

[Yong Zhao]

The previous SDMA queue counting was wrong. In addition, after confirming
with MEC firmware team, we understands that only one unmap queue package,
instead of one unmap queue package for CP and each SDMA engine, is needed,
which results in much simpler driver code.

Change-Id: I84fd2f7e63d6b7f664580b425a78d3e995ce9abc
Signed-off-by: Yong Zhao 
---
 .../drm/amd/amdkfd/kfd_device_queue_manager.c | 79 ++-
 .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 -
 .../amd/amdkfd/kfd_process_queue_manager.c| 16 ++--
 3 files changed, 29 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 958275db3f55..692abfd2088a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -109,6 +109,11 @@ static unsigned int get_num_xgmi_sdma_engines(struct 
device_queue_manager *dqm)
return dqm->dev->device_info->num_xgmi_sdma_engines;
 }
 
+static unsigned int get_num_all_sdma_engines(struct device_queue_manager *dqm)
+{
+   return get_num_sdma_engines(dqm) + get_num_xgmi_sdma_engines(dqm);
+}
+
 unsigned int get_num_sdma_queues(struct device_queue_manager *dqm)
 {
return dqm->dev->device_info->num_sdma_engines
@@ -375,11 +380,6 @@ static int create_queue_nocpsch(struct 
device_queue_manager *dqm,
if (q->properties.is_active)
increment_queue_count(dqm, q->properties.type);
 
-   if (q->properties.type == KFD_QUEUE_TYPE_SDMA)
-   dqm->sdma_queue_count++;
-   else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
-   dqm->xgmi_sdma_queue_count++;
-
/*
 * Unconditionally increment this counter, regardless of the queue's
 * type or whether the queue is active.
@@ -460,15 +460,13 @@ static int destroy_queue_nocpsch_locked(struct 
device_queue_manager *dqm,
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
 
-   if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE) {
+   if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE)
deallocate_hqd(dqm, q);
-   } else if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
-   dqm->sdma_queue_count--;
+   else if (q->properties.type == KFD_QUEUE_TYPE_SDMA)
deallocate_sdma_queue(dqm, q);
-   } else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
-   dqm->xgmi_sdma_queue_count--;
+   else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
deallocate_sdma_queue(dqm, q);
-   } else {
+   else {
pr_debug("q->properties.type %d is invalid\n",
q->properties.type);
return -EINVAL;
@@ -915,8 +913,6 @@ static int initialize_nocpsch(struct device_queue_manager 
*dqm)
INIT_LIST_HEAD(>queues);
dqm->active_queue_count = dqm->next_pipe_to_allocate = 0;
dqm->active_cp_queue_count = 0;
-   dqm->sdma_queue_count = 0;
-   dqm->xgmi_sdma_queue_count = 0;
 
for (pipe = 0; pipe < get_pipes_per_mec(dqm); pipe++) {
int pipe_offset = pipe * get_queues_per_pipe(dqm);
@@ -981,8 +977,11 @@ static int allocate_sdma_queue(struct device_queue_manager 
*dqm,
int bit;
 
if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
-   if (dqm->sdma_bitmap == 0)
+   if (dqm->sdma_bitmap == 0) {
+   pr_err("No more SDMA queue to allocate\n");
return -ENOMEM;
+   }
+
bit = __ffs64(dqm->sdma_bitmap);
dqm->sdma_bitmap &= ~(1ULL << bit);
q->sdma_id = bit;
@@ -991,8 +990,10 @@ static int allocate_sdma_queue(struct device_queue_manager 
*dqm,
q->properties.sdma_queue_id = q->sdma_id /
get_num_sdma_engines(dqm);
} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
-   if (dqm->xgmi_sdma_bitmap == 0)
+   if (dqm->xgmi_sdma_bitmap == 0) {
+   pr_err("No more XGMI SDMA queue to allocate\n");
return -ENOMEM;
+   }
bit = __ffs64(dqm->xgmi_sdma_bitmap);
dqm->xgmi_sdma_bitmap &= ~(1ULL << bit);
q->sdma_id = bit;
@@ -1081,8 +1082,7 @@ static int initialize_cpsch(struct device_queue_manager 
*dqm)
INIT_LIST_HEAD(>queues);
dqm->active_queue_count = dqm->processes_count = 0;
dqm->active_cp_queue_count = 0;
-   dqm->sdma_queue_count = 0;
-   dqm->xgmi_sdma_queue_count = 0;
+
dqm->active_runlist = false;
dqm->sdma_bitmap = ~0ULL >> (64 - get_num_sdma_queues(dqm));
dqm->xgmi_sdma_bitmap = ~0ULL >> (64 - get_num_xgmi_sdma_queues(dqm));
@@ -1254,11 +1254,6 @@ static int create_queue_cpsch(struct 

[PATCH 2/6] drm/amdkfd: Avoid ambiguity by indicating it's cp queue

[Yong Zhao]

The queues represented in queue_bitmap are only CP queues.

Change-Id: I7e6a75de39718d7c4da608166b85b9377d06d1b3
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c   |  4 ++--
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c| 12 ++--
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.h|  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c  |  2 +-
 .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c   |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c|  2 +-
 drivers/gpu/drm/amd/include/kgd_kfd_interface.h  |  2 +-
 7 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index 8609287620ea..ebe4b8f88e79 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -126,7 +126,7 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
/* this is going to have a few of the MSBs set that we need to
 * clear
 */
-   bitmap_complement(gpu_resources.queue_bitmap,
+   bitmap_complement(gpu_resources.cp_queue_bitmap,
  adev->gfx.mec.queue_bitmap,
  KGD_MAX_QUEUES);
 
@@ -137,7 +137,7 @@ void amdgpu_amdkfd_device_init(struct amdgpu_device *adev)
* adev->gfx.mec.num_pipe_per_mec
* adev->gfx.mec.num_queue_per_pipe;
for (i = last_valid_bit; i < KGD_MAX_QUEUES; ++i)
-   clear_bit(i, gpu_resources.queue_bitmap);
+   clear_bit(i, gpu_resources.cp_queue_bitmap);
 
amdgpu_doorbell_get_kfd_info(adev,
_resources.doorbell_physical_address,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 7ef9b89f5c70..973581c2b401 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -78,14 +78,14 @@ static bool is_pipe_enabled(struct device_queue_manager 
*dqm, int mec, int pipe)
/* queue is available for KFD usage if bit is 1 */
for (i = 0; i <  dqm->dev->shared_resources.num_queue_per_pipe; ++i)
if (test_bit(pipe_offset + i,
- dqm->dev->shared_resources.queue_bitmap))
+ dqm->dev->shared_resources.cp_queue_bitmap))
return true;
return false;
 }
 
-unsigned int get_queues_num(struct device_queue_manager *dqm)
+unsigned int get_cp_queues_num(struct device_queue_manager *dqm)
 {
-   return bitmap_weight(dqm->dev->shared_resources.queue_bitmap,
+   return bitmap_weight(dqm->dev->shared_resources.cp_queue_bitmap,
KGD_MAX_QUEUES);
 }
 
@@ -908,7 +908,7 @@ static int initialize_nocpsch(struct device_queue_manager 
*dqm)
 
for (queue = 0; queue < get_queues_per_pipe(dqm); queue++)
if (test_bit(pipe_offset + queue,
-dqm->dev->shared_resources.queue_bitmap))
+
dqm->dev->shared_resources.cp_queue_bitmap))
dqm->allocated_queues[pipe] |= 1 << queue;
}
 
@@ -1029,7 +1029,7 @@ static int set_sched_resources(struct 
device_queue_manager *dqm)
mec = (i / dqm->dev->shared_resources.num_queue_per_pipe)
/ dqm->dev->shared_resources.num_pipe_per_mec;
 
-   if (!test_bit(i, dqm->dev->shared_resources.queue_bitmap))
+   if (!test_bit(i, dqm->dev->shared_resources.cp_queue_bitmap))
continue;
 
/* only acquire queues from the first MEC */
@@ -1979,7 +1979,7 @@ int dqm_debugfs_hqds(struct seq_file *m, void *data)
 
for (queue = 0; queue < get_queues_per_pipe(dqm); queue++) {
if (!test_bit(pipe_offset + queue,
- dqm->dev->shared_resources.queue_bitmap))
+ 
dqm->dev->shared_resources.cp_queue_bitmap))
continue;
 
r = dqm->dev->kfd2kgd->hqd_dump(
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
index ee3400e92c30..3f0fb0d28c01 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
@@ -219,7 +219,7 @@ void device_queue_manager_init_v10_navi10(
struct device_queue_manager_asic_ops *asic_ops);
 void program_sh_mem_settings(struct device_queue_manager *dqm,
struct qcm_process_device *qpd);
-unsigned int get_queues_num(struct 

[PATCH 3/6] drm/amdkfd: Count active CP queues directly

[Yong Zhao]

The previous code of calculating active CP queues is problematic if
some SDMA queues are inactive. Fix that by counting CP queues directly.

Change-Id: I5ffaa75a95cbebc984558199ba2f3db6909c52a9
Signed-off-by: Yong Zhao 
---
 .../drm/amd/amdkfd/kfd_device_queue_manager.c | 47 +--
 .../drm/amd/amdkfd/kfd_device_queue_manager.h |  1 +
 .../gpu/drm/amd/amdkfd/kfd_packet_manager.c   |  3 +-
 3 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 973581c2b401..a3c44d88314b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -132,6 +132,22 @@ void program_sh_mem_settings(struct device_queue_manager 
*dqm,
qpd->sh_mem_bases);
 }
 
+void increment_queue_count(struct device_queue_manager *dqm,
+   enum kfd_queue_type type)
+{
+   dqm->active_queue_count++;
+   if (type == KFD_QUEUE_TYPE_COMPUTE || type == KFD_QUEUE_TYPE_DIQ)
+   dqm->active_cp_queue_count++;
+}
+
+void decrement_queue_count(struct device_queue_manager *dqm,
+   enum kfd_queue_type type)
+{
+   dqm->active_queue_count--;
+   if (type == KFD_QUEUE_TYPE_COMPUTE || type == KFD_QUEUE_TYPE_DIQ)
+   dqm->active_cp_queue_count--;
+}
+
 static int allocate_doorbell(struct qcm_process_device *qpd, struct queue *q)
 {
struct kfd_dev *dev = qpd->dqm->dev;
@@ -359,7 +375,7 @@ static int create_queue_nocpsch(struct device_queue_manager 
*dqm,
list_add(>list, >queues_list);
qpd->queue_count++;
if (q->properties.is_active)
-   dqm->active_queue_count++;
+   increment_queue_count(dqm, q->properties.type);
 
if (q->properties.type == KFD_QUEUE_TYPE_SDMA)
dqm->sdma_queue_count++;
@@ -494,7 +510,7 @@ static int destroy_queue_nocpsch_locked(struct 
device_queue_manager *dqm,
}
qpd->queue_count--;
if (q->properties.is_active)
-   dqm->active_queue_count--;
+   decrement_queue_count(dqm, q->properties.type);
 
return retval;
 }
@@ -567,9 +583,9 @@ static int update_queue(struct device_queue_manager *dqm, 
struct queue *q)
 * uploaded.
 */
if (q->properties.is_active && !prev_active)
-   dqm->active_queue_count++;
+   increment_queue_count(dqm, q->properties.type);
else if (!q->properties.is_active && prev_active)
-   dqm->active_queue_count--;
+   decrement_queue_count(dqm, q->properties.type);
 
if (dqm->sched_policy != KFD_SCHED_POLICY_NO_HWS)
retval = map_queues_cpsch(dqm);
@@ -618,7 +634,7 @@ static int evict_process_queues_nocpsch(struct 
device_queue_manager *dqm,
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
q->properties.is_active = false;
-   dqm->active_queue_count--;
+   decrement_queue_count(dqm, q->properties.type);
 
if (WARN_ONCE(!dqm->sched_running, "Evict when stopped\n"))
continue;
@@ -662,7 +678,7 @@ static int evict_process_queues_cpsch(struct 
device_queue_manager *dqm,
continue;
 
q->properties.is_active = false;
-   dqm->active_queue_count--;
+   decrement_queue_count(dqm, q->properties.type);
}
retval = execute_queues_cpsch(dqm,
qpd->is_debug ?
@@ -731,7 +747,7 @@ static int restore_process_queues_nocpsch(struct 
device_queue_manager *dqm,
mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
q->properties.type)];
q->properties.is_active = true;
-   dqm->active_queue_count++;
+   increment_queue_count(dqm, q->properties.type);
 
if (WARN_ONCE(!dqm->sched_running, "Restore when stopped\n"))
continue;
@@ -786,7 +802,7 @@ static int restore_process_queues_cpsch(struct 
device_queue_manager *dqm,
continue;
 
q->properties.is_active = true;
-   dqm->active_queue_count++;
+   increment_queue_count(dqm, q->properties.type);
}
retval = execute_queues_cpsch(dqm,
KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
@@ -900,6 +916,7 @@ static int initialize_nocpsch(struct device_queue_manager 
*dqm)
mutex_init(>lock_hidden);
INIT_LIST_HEAD(>queues);
dqm->active_queue_count = dqm->next_pipe_to_allocate = 0;
+   dqm->active_cp_queue_count = 0;
dqm->sdma_queue_count = 0;
dqm->xgmi_sdma_queue_count = 0;
 
@@ -1065,6 +1082,7 @@ static int 

[PATCH 4/6] drm/amdkfd: Fix a memory leak in queue creation error handling

[Yong Zhao]

When the queue creation failed, some resources were not freed. Fix it.

Change-Id: Ia24b6ad31528dceddfd4d1c58bb1d22c35d3eabf
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index b62ee2e3344a..c604a2ede3f5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -329,6 +329,9 @@ int pqm_create_queue(struct process_queue_manager *pqm,
return retval;
 
 err_create_queue:
+   uninit_queue(q);
+   if (kq)
+   kernel_queue_uninit(kq, false);
kfree(pqn);
 err_allocate_pqn:
/* check if queues list is empty unregister process from device */
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: add HDCP caps debugfs

[Harry Wentland]

On 2020-02-24 4:51 p.m., Bhawanpreet Lakha wrote:
> 
> On 2020-02-24 4:15 p.m., Harry Wentland wrote:
>>
>> On 2020-02-24 2:55 p.m., Bhawanpreet Lakha wrote:
>>> Add debugfs to get HDCP capability. This is also useful for
>>> kms_content_protection igt test.
>>>
>>> Use:
>>> cat /sys/kernel/debug/dri/0/DP-1/hdcp_sink_capability
>>> cat /sys/kernel/debug/dri/0/HDMI-A-1/hdcp_sink_capability
>>>
>>> Signed-off-by: Bhawanpreet Lakha 
>>> ---
>>>   .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 61 +++
>>>   1 file changed, 61 insertions(+)
>>>
>>> diff --git
>>> a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>>> index ead5c05eec92..52982c8c871f 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
>>> @@ -815,6 +815,44 @@ static int vrr_range_show(struct seq_file *m,
>>> void *data)
>>>   return 0;
>>>   }
>>>   +#ifdef CONFIG_DRM_AMD_DC_HDCP
>>> +/*
>>> + * Returns the HDCP capability of the Display (1.4 for now).
>>> + *
>>> + * NOTE* Not all HDMI displays report their HDCP caps even when they
>>> are capable.
>>> + * Since its rare for a display to not be HDCP 1.4 capable, we set
>>> HDMI as always capable.
>>> + *
>>> + * Example usage: cat /sys/kernel/debug/dri/0/DP-1/hdcp_sink_capability
>>> + *    or cat /sys/kernel/debug/dri/0/HDMI-A-1/hdcp_sink_capability
>>> + */
>>> +static int hdcp_sink_capability_show(struct seq_file *m, void *data)
>>> +{
>>> +    struct drm_connector *connector = m->private;
>>> +    struct amdgpu_dm_connector *aconnector =
>>> to_amdgpu_dm_connector(connector);
>>> +    bool hdcp_cap, hdcp2_cap;
>>> +
>>> +    if (connector->status != connector_status_connected)
>>> +    return -ENODEV;
>>> +
>>> +    seq_printf(m, "%s:%d HDCP version: ", connector->name,
>>> connector->base.id);
>>> +
>>> +    hdcp_cap = dc_link_is_hdcp14(aconnector->dc_link);
>>> +    hdcp2_cap = dc_link_is_hdcp22(aconnector->dc_link);
>>> +
>>> +
>>> +    if (hdcp_cap)
>>> +    seq_printf(m, "%s ", "HDCP1.4");
>>> +    if (hdcp2_cap)
>>> +    seq_printf(m, "%s ", "HDCP2.2");
>>> +
>>> +    if (!hdcp_cap && !hdcp2_cap)
>>> +    seq_printf(m, "%s ", "None");
>>> +
>>> +    seq_puts(m, "\n");
>>> +
>>> +    return 0;
>>> +}
>>> +#endif
>>>   /* function description
>>>    *
>>>    * generic SDP message access for testing
>>> @@ -940,6 +978,9 @@ static ssize_t dp_dpcd_data_read(struct file *f,
>>> char __user *buf,
>>>   DEFINE_SHOW_ATTRIBUTE(dmub_tracebuffer);
>>>   DEFINE_SHOW_ATTRIBUTE(output_bpc);
>>>   DEFINE_SHOW_ATTRIBUTE(vrr_range);
>>> +#ifdef CONFIG_DRM_AMD_DC_HDCP
>>> +DEFINE_SHOW_ATTRIBUTE(hdcp_sink_capability);
>>> +#endif
>>>     static const struct file_operations dp_link_settings_debugfs_fops
>>> = {
>>>   .owner = THIS_MODULE,
>>> @@ -995,12 +1036,23 @@ static const struct {
>>>   {"test_pattern", _phy_test_pattern_fops},
>>>   {"output_bpc", _bpc_fops},
>>>   {"vrr_range", _range_fops},
>>> +#ifdef CONFIG_DRM_AMD_DC_HDCP
>>> +    {"hdcp_sink_capability", _sink_capability_fops},
>>> +#endif
>>>   {"sdp_message", _message_fops},
>>>   {"aux_dpcd_address", _dpcd_address_debugfs_fops},
>>>   {"aux_dpcd_size", _dpcd_size_debugfs_fops},
>>>   {"aux_dpcd_data", _dpcd_data_debugfs_fops}
>>>   };
>>>   +#ifdef CONFIG_DRM_AMD_DC_HDCP
>>> +static const struct {
>>> +    char *name;
>>> +    const struct file_operations *fops;
>>> +} hdmi_debugfs_entries[] = {
>>> +    {"hdcp_sink_capability", _sink_capability_fops}
>>> +};
>>> +#endif
>>>   /*
>>>    * Force YUV420 output if available from the given mode
>>>    */
>>> @@ -1066,6 +1118,15 @@ void connector_debugfs_init(struct
>>> amdgpu_dm_connector *connector)
>>>   debugfs_create_file_unsafe("force_yuv420_output", 0644, dir,
>>> connector,
>>>  _yuv420_output_fops);
>>>   +#ifdef CONFIG_DRM_AMD_DC_HDCP
>>> +    if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA) {
>> Your patch description mentions DP and HDMI but here you're only
>> creating it for HDMI. Should we create it for all HDCP-capable signal
>> types, i.e. DP and HDMI?
>>
>> Harry
> 
> There is a loop above that is called for dp_debugfs_entries[]. I have
> added the HDCP debugfs to that struct so no need to recreate the loop.
> 

I see it now. Thanks.

Patch is
Reviewed-by: Harry Wentland 

Harry

> Bhawan
> 
>>> +    for (i = 0; i < ARRAY_SIZE(hdmi_debugfs_entries); i++) {
>>> +    debugfs_create_file(hdmi_debugfs_entries[i].name,
>>> +    0644, dir, connector,
>>> +    hdmi_debugfs_entries[i].fops);
>>> +    }
>>> +    }
>>> +#endif
>>>   }
>>>     /*
>>>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: add HDCP caps debugfs

[Bhawanpreet Lakha]

On 2020-02-24 4:15 p.m., Harry Wentland wrote:


On 2020-02-24 2:55 p.m., Bhawanpreet Lakha wrote:

Add debugfs to get HDCP capability. This is also useful for
kms_content_protection igt test.

Use:
cat /sys/kernel/debug/dri/0/DP-1/hdcp_sink_capability
cat /sys/kernel/debug/dri/0/HDMI-A-1/hdcp_sink_capability

Signed-off-by: Bhawanpreet Lakha 
---
  .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 61 +++
  1 file changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index ead5c05eec92..52982c8c871f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -815,6 +815,44 @@ static int vrr_range_show(struct seq_file *m, void *data)
return 0;
  }
  
+#ifdef CONFIG_DRM_AMD_DC_HDCP

+/*
+ * Returns the HDCP capability of the Display (1.4 for now).
+ *
+ * NOTE* Not all HDMI displays report their HDCP caps even when they are 
capable.
+ * Since its rare for a display to not be HDCP 1.4 capable, we set HDMI as 
always capable.
+ *
+ * Example usage: cat /sys/kernel/debug/dri/0/DP-1/hdcp_sink_capability
+ * or cat /sys/kernel/debug/dri/0/HDMI-A-1/hdcp_sink_capability
+ */
+static int hdcp_sink_capability_show(struct seq_file *m, void *data)
+{
+   struct drm_connector *connector = m->private;
+   struct amdgpu_dm_connector *aconnector = 
to_amdgpu_dm_connector(connector);
+   bool hdcp_cap, hdcp2_cap;
+
+   if (connector->status != connector_status_connected)
+   return -ENODEV;
+
+   seq_printf(m, "%s:%d HDCP version: ", connector->name, 
connector->base.id);
+
+   hdcp_cap = dc_link_is_hdcp14(aconnector->dc_link);
+   hdcp2_cap = dc_link_is_hdcp22(aconnector->dc_link);
+
+
+   if (hdcp_cap)
+   seq_printf(m, "%s ", "HDCP1.4");
+   if (hdcp2_cap)
+   seq_printf(m, "%s ", "HDCP2.2");
+
+   if (!hdcp_cap && !hdcp2_cap)
+   seq_printf(m, "%s ", "None");
+
+   seq_puts(m, "\n");
+
+   return 0;
+}
+#endif
  /* function description
   *
   * generic SDP message access for testing
@@ -940,6 +978,9 @@ static ssize_t dp_dpcd_data_read(struct file *f, char 
__user *buf,
  DEFINE_SHOW_ATTRIBUTE(dmub_tracebuffer);
  DEFINE_SHOW_ATTRIBUTE(output_bpc);
  DEFINE_SHOW_ATTRIBUTE(vrr_range);
+#ifdef CONFIG_DRM_AMD_DC_HDCP
+DEFINE_SHOW_ATTRIBUTE(hdcp_sink_capability);
+#endif
  
  static const struct file_operations dp_link_settings_debugfs_fops = {

.owner = THIS_MODULE,
@@ -995,12 +1036,23 @@ static const struct {
{"test_pattern", _phy_test_pattern_fops},
{"output_bpc", _bpc_fops},
{"vrr_range", _range_fops},
+#ifdef CONFIG_DRM_AMD_DC_HDCP
+   {"hdcp_sink_capability", _sink_capability_fops},
+#endif
{"sdp_message", _message_fops},
{"aux_dpcd_address", _dpcd_address_debugfs_fops},
{"aux_dpcd_size", _dpcd_size_debugfs_fops},
{"aux_dpcd_data", _dpcd_data_debugfs_fops}
  };
  
+#ifdef CONFIG_DRM_AMD_DC_HDCP

+static const struct {
+   char *name;
+   const struct file_operations *fops;
+} hdmi_debugfs_entries[] = {
+   {"hdcp_sink_capability", _sink_capability_fops}
+};
+#endif
  /*
   * Force YUV420 output if available from the given mode
   */
@@ -1066,6 +1118,15 @@ void connector_debugfs_init(struct amdgpu_dm_connector 
*connector)
debugfs_create_file_unsafe("force_yuv420_output", 0644, dir, connector,
   _yuv420_output_fops);
  
+#ifdef CONFIG_DRM_AMD_DC_HDCP

+   if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA) {

Your patch description mentions DP and HDMI but here you're only
creating it for HDMI. Should we create it for all HDCP-capable signal
types, i.e. DP and HDMI?

Harry


There is a loop above that is called for dp_debugfs_entries[]. I have 
added the HDCP debugfs to that struct so no need to recreate the loop.


Bhawan


+   for (i = 0; i < ARRAY_SIZE(hdmi_debugfs_entries); i++) {
+   debugfs_create_file(hdmi_debugfs_entries[i].name,
+   0644, dir, connector,
+   hdmi_debugfs_entries[i].fops);
+   }
+   }
+#endif
  }
  
  /*



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: add HDCP caps debugfs

[Harry Wentland]

On 2020-02-24 2:55 p.m., Bhawanpreet Lakha wrote:
> Add debugfs to get HDCP capability. This is also useful for
> kms_content_protection igt test.
> 
> Use:
>   cat /sys/kernel/debug/dri/0/DP-1/hdcp_sink_capability
>   cat /sys/kernel/debug/dri/0/HDMI-A-1/hdcp_sink_capability
> 
> Signed-off-by: Bhawanpreet Lakha 
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 61 +++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> index ead5c05eec92..52982c8c871f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> @@ -815,6 +815,44 @@ static int vrr_range_show(struct seq_file *m, void *data)
>   return 0;
>  }
>  
> +#ifdef CONFIG_DRM_AMD_DC_HDCP
> +/*
> + * Returns the HDCP capability of the Display (1.4 for now).
> + *
> + * NOTE* Not all HDMI displays report their HDCP caps even when they are 
> capable.
> + * Since its rare for a display to not be HDCP 1.4 capable, we set HDMI as 
> always capable.
> + *
> + * Example usage: cat /sys/kernel/debug/dri/0/DP-1/hdcp_sink_capability
> + *   or cat /sys/kernel/debug/dri/0/HDMI-A-1/hdcp_sink_capability
> + */
> +static int hdcp_sink_capability_show(struct seq_file *m, void *data)
> +{
> + struct drm_connector *connector = m->private;
> + struct amdgpu_dm_connector *aconnector = 
> to_amdgpu_dm_connector(connector);
> + bool hdcp_cap, hdcp2_cap;
> +
> + if (connector->status != connector_status_connected)
> + return -ENODEV;
> +
> + seq_printf(m, "%s:%d HDCP version: ", connector->name, 
> connector->base.id);
> +
> + hdcp_cap = dc_link_is_hdcp14(aconnector->dc_link);
> + hdcp2_cap = dc_link_is_hdcp22(aconnector->dc_link);
> +
> +
> + if (hdcp_cap)
> + seq_printf(m, "%s ", "HDCP1.4");
> + if (hdcp2_cap)
> + seq_printf(m, "%s ", "HDCP2.2");
> +
> + if (!hdcp_cap && !hdcp2_cap)
> + seq_printf(m, "%s ", "None");
> +
> + seq_puts(m, "\n");
> +
> + return 0;
> +}
> +#endif
>  /* function description
>   *
>   * generic SDP message access for testing
> @@ -940,6 +978,9 @@ static ssize_t dp_dpcd_data_read(struct file *f, char 
> __user *buf,
>  DEFINE_SHOW_ATTRIBUTE(dmub_tracebuffer);
>  DEFINE_SHOW_ATTRIBUTE(output_bpc);
>  DEFINE_SHOW_ATTRIBUTE(vrr_range);
> +#ifdef CONFIG_DRM_AMD_DC_HDCP
> +DEFINE_SHOW_ATTRIBUTE(hdcp_sink_capability);
> +#endif
>  
>  static const struct file_operations dp_link_settings_debugfs_fops = {
>   .owner = THIS_MODULE,
> @@ -995,12 +1036,23 @@ static const struct {
>   {"test_pattern", _phy_test_pattern_fops},
>   {"output_bpc", _bpc_fops},
>   {"vrr_range", _range_fops},
> +#ifdef CONFIG_DRM_AMD_DC_HDCP
> + {"hdcp_sink_capability", _sink_capability_fops},
> +#endif
>   {"sdp_message", _message_fops},
>   {"aux_dpcd_address", _dpcd_address_debugfs_fops},
>   {"aux_dpcd_size", _dpcd_size_debugfs_fops},
>   {"aux_dpcd_data", _dpcd_data_debugfs_fops}
>  };
>  
> +#ifdef CONFIG_DRM_AMD_DC_HDCP
> +static const struct {
> + char *name;
> + const struct file_operations *fops;
> +} hdmi_debugfs_entries[] = {
> + {"hdcp_sink_capability", _sink_capability_fops}
> +};
> +#endif
>  /*
>   * Force YUV420 output if available from the given mode
>   */
> @@ -1066,6 +1118,15 @@ void connector_debugfs_init(struct amdgpu_dm_connector 
> *connector)
>   debugfs_create_file_unsafe("force_yuv420_output", 0644, dir, connector,
>  _yuv420_output_fops);
>  
> +#ifdef CONFIG_DRM_AMD_DC_HDCP
> + if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA) {

Your patch description mentions DP and HDMI but here you're only
creating it for HDMI. Should we create it for all HDCP-capable signal
types, i.e. DP and HDMI?

Harry

> + for (i = 0; i < ARRAY_SIZE(hdmi_debugfs_entries); i++) {
> + debugfs_create_file(hdmi_debugfs_entries[i].name,
> + 0644, dir, connector,
> + hdmi_debugfs_entries[i].fops);
> + }
> + }
> +#endif
>  }
>  
>  /*
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 13/15] drm/amdgpu/display: split dp connector registration (v3)

[Harry Wentland]

On 2020-02-07 4:17 p.m., Alex Deucher wrote:
> Split into init and register functions to avoid a segfault
> in some configs when the load/unload callbacks are removed.
> 

Looks like MST is completely broken with this change with a NULL pointer
dereference in drm_dp_aux_register.

> v2:
> - add back accidently dropped has_aux setting
> - set dev in late_register
> 
> v3:
> - fix dp cec ordering
> 
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c   | 16 
>  drivers/gpu/drm/amd/amdgpu/atombios_dp.c | 10 ++
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c  |  7 ++-
>  3 files changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index ec1501e3a63a..f355d9a752d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -1461,6 +1461,20 @@ static enum drm_mode_status 
> amdgpu_connector_dp_mode_valid(struct drm_connector
>   return MODE_OK;
>  }
>  
> +static int
> +amdgpu_connector_late_register(struct drm_connector *connector)
> +{
> + struct amdgpu_connector *amdgpu_connector = 
> to_amdgpu_connector(connector);
> + int r = 0;
> +
> + if (amdgpu_connector->ddc_bus->has_aux) {
> + amdgpu_connector->ddc_bus->aux.dev = 
> amdgpu_connector->base.kdev;
> + r = drm_dp_aux_register(_connector->ddc_bus->aux);
> + }
> +
> + return r;
> +}
> +
>  static const struct drm_connector_helper_funcs 
> amdgpu_connector_dp_helper_funcs = {
>   .get_modes = amdgpu_connector_dp_get_modes,
>   .mode_valid = amdgpu_connector_dp_mode_valid,
> @@ -1475,6 +1489,7 @@ static const struct drm_connector_funcs 
> amdgpu_connector_dp_funcs = {
>   .early_unregister = amdgpu_connector_unregister,
>   .destroy = amdgpu_connector_destroy,
>   .force = amdgpu_connector_dvi_force,
> + .late_register = amdgpu_connector_late_register,
>  };
>  
>  static const struct drm_connector_funcs amdgpu_connector_edp_funcs = {
> @@ -1485,6 +1500,7 @@ static const struct drm_connector_funcs 
> amdgpu_connector_edp_funcs = {
>   .early_unregister = amdgpu_connector_unregister,
>   .destroy = amdgpu_connector_destroy,
>   .force = amdgpu_connector_dvi_force,
> + .late_register = amdgpu_connector_late_register,
>  };
>  
>  void
> diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c 
> b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> index ea702a64f807..9b74cfdba7b8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atombios_dp.c
> @@ -186,16 +186,10 @@ amdgpu_atombios_dp_aux_transfer(struct drm_dp_aux *aux, 
> struct drm_dp_aux_msg *m
>  
>  void amdgpu_atombios_dp_aux_init(struct amdgpu_connector *amdgpu_connector)
>  {
> - int ret;
> -
>   amdgpu_connector->ddc_bus->rec.hpd = amdgpu_connector->hpd.hpd;
> - amdgpu_connector->ddc_bus->aux.dev = amdgpu_connector->base.kdev;
>   amdgpu_connector->ddc_bus->aux.transfer = 
> amdgpu_atombios_dp_aux_transfer;
> - ret = drm_dp_aux_register(_connector->ddc_bus->aux);
> - if (!ret)
> - amdgpu_connector->ddc_bus->has_aux = true;
> -
> - WARN(ret, "drm_dp_aux_register_i2c_bus() failed with error %d\n", ret);
> + drm_dp_aux_init(_connector->ddc_bus->aux);
> + amdgpu_connector->ddc_bus->has_aux = true;
>  }
>  
>  /* general DP utility functions */
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index 3959c942c88b..d5b9e72f2649 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -155,6 +155,11 @@ amdgpu_dm_mst_connector_late_register(struct 
> drm_connector *connector)
>   struct amdgpu_dm_connector *amdgpu_dm_connector =
>   to_amdgpu_dm_connector(connector);
>   struct drm_dp_mst_port *port = amdgpu_dm_connector->port;
> + int r;
> +
> + r = drm_dp_aux_register(_dm_connector->dm_dp_aux.aux);

This calls drm_dp_aux_register_devnode which is also called later in
drm_dp_mst_connector_late_register. Wonder if that's a problem.

Harry

> + if (r)
> + return r;
>  
>  #if defined(CONFIG_DEBUG_FS)
>   connector_debugfs_init(amdgpu_dm_connector);
> @@ -484,7 +489,7 @@ void amdgpu_dm_initialize_dp_connector(struct 
> amdgpu_display_manager *dm,
>   aconnector->dm_dp_aux.aux.transfer = dm_dp_aux_transfer;
>   aconnector->dm_dp_aux.ddc_service = aconnector->dc_link->ddc;
>  
> - drm_dp_aux_register(>dm_dp_aux.aux);
> + drm_dp_aux_init(>dm_dp_aux.aux);
>   drm_dp_cec_register_connector(>dm_dp_aux.aux,
> >base);
>  
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org

RE: [PATCH] drm/amdgpu: log TA versions on init

[Liu, Zhan]

> -Original Message-
> From: amd-gfx  On Behalf Of
> Bhawanpreet Lakha
> Sent: 2020/February/24, Monday 2:45 PM
> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> ; Zhang, Hawking
> 
> Cc: Lakha, Bhawanpreet 
> Subject: [PATCH] drm/amdgpu: log TA versions on init
> 
> It is helpful to know what version the TA's are for debugging
> 
> Signed-off-by: Bhawanpreet Lakha 

Reviewed-by: Zhan Liu 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index a16c8101e250..09d1433677a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -358,6 +358,7 @@ static int psp_asd_load(struct psp_context *psp)
>   if (!ret) {
>   psp->asd_context.asd_initialized = true;
>   psp->asd_context.session_id = cmd->resp.session_id;
> + DRM_INFO("ASD: Initialized (version: 0x%X)", psp-
> >asd_fw_version);
>   }
> 
>   kfree(cmd);
> @@ -518,6 +519,7 @@ static int psp_xgmi_load(struct psp_context *psp)
>   if (!ret) {
>   psp->xgmi_context.initialized = 1;
>   psp->xgmi_context.session_id = cmd->resp.session_id;
> + DRM_INFO("XGMI: Initialized (version: 0x%X)", psp-
> >ta_xgmi_ucode_version);
>   }
> 
>   kfree(cmd);
> @@ -658,6 +660,7 @@ static int psp_ras_load(struct psp_context *psp)
>   if (!ret) {
>   psp->ras.ras_initialized = true;
>   psp->ras.session_id = cmd->resp.session_id;
> + DRM_INFO("RAS: Initialized (version: 0x%X)", psp-
> >ta_ras_ucode_version);
>   }
> 
>   kfree(cmd);
> @@ -832,6 +835,7 @@ static int psp_hdcp_load(struct psp_context *psp)
>   if (!ret) {
>   psp->hdcp_context.hdcp_initialized = true;
>   psp->hdcp_context.session_id = cmd->resp.session_id;
> + DRM_INFO("HDCP: Initialized (version: 0x%X)", psp-
> >ta_hdcp_ucode_version);
>   }
> 
>   kfree(cmd);
> @@ -977,6 +981,7 @@ static int psp_dtm_load(struct psp_context *psp)
>   if (!ret) {
>   psp->dtm_context.dtm_initialized = true;
>   psp->dtm_context.session_id = cmd->resp.session_id;
> + DRM_INFO("DTM: Initialized (version: 0x%X)", psp-
> >ta_dtm_ucode_version);
>   }
> 
>   kfree(cmd);
> --
> 2.17.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Daniel Vetter]

On Mon, Feb 24, 2020 at 9:46 PM Daniel Vetter  wrote:
>
> On Mon, Feb 24, 2020 at 5:31 PM Emil Velikov  wrote:
> >
> > On Sat, 22 Feb 2020 at 17:54, Daniel Vetter  wrote:
> > >
> > > It's the last user, and more importantly, it's the last non-legacy
> > > user of anything in drm_pci.c.
> > >
> > > The only tricky bit is the agp initialization. But a close look shows
> > > that radeon does not use the drm_agp midlayer (the main use of that is
> > > drm_bufs for legacy drivers), and instead could use the agp subsystem
> > > directly (like nouveau does already). Hence we can just pull this in
> > > too.
> > >
> > > A further step would be to entirely drop the use of drm_device->agp,
> > > but feels like too much churn just for this patch.
> > >
> > > Signed-off-by: Daniel Vetter 
> > > Cc: Alex Deucher 
> > > Cc: "Christian König" 
> > > Cc: "David (ChunMing) Zhou" 
> > > Cc: amd-gfx@lists.freedesktop.org
> > > ---
> > >  drivers/gpu/drm/radeon/radeon_drv.c | 43 +++--
> > >  drivers/gpu/drm/radeon/radeon_kms.c |  6 
> > >  2 files changed, 47 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
> > > b/drivers/gpu/drm/radeon/radeon_drv.c
> > > index 49ce2e7d5f9e..59f8186a2415 100644
> > > --- a/drivers/gpu/drm/radeon/radeon_drv.c
> > > +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> > > @@ -37,6 +37,7 @@
> > >  #include 
> > >  #include 
> > >
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> > > const struct pci_device_id *ent)
> > >  {
> > > unsigned long flags = 0;
> > > +   struct drm_device *dev;
> > > int ret;
> > >
> > > if (!ent)
> > > @@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> > > if (ret)
> > > return ret;
> > >
> > > -   return drm_get_pci_dev(pdev, ent, _driver);
> > > +   dev = drm_dev_alloc(_driver, >dev);
> > > +   if (IS_ERR(dev))
> > > +   return PTR_ERR(dev);
> > > +
> > > +   ret = pci_enable_device(pdev);
> > > +   if (ret)
> > > +   goto err_free;
> > > +
> > > +   dev->pdev = pdev;
> > > +#ifdef __alpha__
> > > +   dev->hose = pdev->sysdata;
> > > +#endif
> > > +
> > > +   pci_set_drvdata(pdev, dev);
> > > +
> > > +   if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> > > +   dev->agp = drm_agp_init(dev);
> > > +   if (dev->agp) {
> > > +   dev->agp->agp_mtrr = arch_phys_wc_add(
> > > +   dev->agp->agp_info.aper_base,
> > > +   dev->agp->agp_info.aper_size *
> > > +   1024 * 1024);
> > > +   }
> > > +
> > IMHO a better solution is kill off the drm_agpsupport.c dependency all 
> > together.
> > As-is it's still used, making the legacy vs not line really moot.
> >
> > Especially, since the AGP ioctl (in the legacy code) can manipulate
> > the underlying state.
> >
> > Off the top of my head, in radeon_agp_init():
> >  - at the top agp_backend_acquire() + agp_copy_info()
> >  - followed up by existing mode magic
> >  - opencode the enable - agp_enable() + acquired = true;
> >  - mtrr = arch_phys_wc_add() and the rest
> >
> > In radeon_agp_fini()
> >  - if !acquired { agp_backend_release(); acquired = false }
> >
> >
> > Something like ^^ should result in a net diffstat of around zero.
> > All thanks to the interesting layer that drm_agp is ;-)
> >
> > With this in place we can make move drm_device::agp and
> > DRM_IOCTL_AGP_INFO behind CONFIG_DRM_LEGACY.
>
> Yeah could be done, but I was more out to get the drm_pci.c stuff, not
> the agp stuff. But I did realize that nouveau also just directly
> accesses the agp bridge stuff, so we could indeed nuke the midlayer
> here. The legacy agp stuff ioctl stuff should be behind DRIVER_LEGACY
> checks already, so no way to cause harm that way (I hope at least, if
> there's a gap we'd need to close it).

Haha, I should read code before hitting send.

It's totally wide open root hole. I guess no one ever bothered to run
a fuzzer on a radeon.ko or amdgpu.ko (before the patch to stop using
drm_get_pci_dev at least) hw. And I never cared since we've killed the
fake agp stuff that i915.ko used a long time ago.

So yeah I think at least sprinkling DRIVER_LEGACY checks over all
this, and reviewing old radeon userspace, would be really good. Once
we have that we can then do the second step and retire the agp
midlayer. But first we need to add the uapi checks/changes.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Daniel Vetter]

On Mon, Feb 24, 2020 at 5:31 PM Emil Velikov  wrote:
>
> On Sat, 22 Feb 2020 at 17:54, Daniel Vetter  wrote:
> >
> > It's the last user, and more importantly, it's the last non-legacy
> > user of anything in drm_pci.c.
> >
> > The only tricky bit is the agp initialization. But a close look shows
> > that radeon does not use the drm_agp midlayer (the main use of that is
> > drm_bufs for legacy drivers), and instead could use the agp subsystem
> > directly (like nouveau does already). Hence we can just pull this in
> > too.
> >
> > A further step would be to entirely drop the use of drm_device->agp,
> > but feels like too much churn just for this patch.
> >
> > Signed-off-by: Daniel Vetter 
> > Cc: Alex Deucher 
> > Cc: "Christian König" 
> > Cc: "David (ChunMing) Zhou" 
> > Cc: amd-gfx@lists.freedesktop.org
> > ---
> >  drivers/gpu/drm/radeon/radeon_drv.c | 43 +++--
> >  drivers/gpu/drm/radeon/radeon_kms.c |  6 
> >  2 files changed, 47 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
> > b/drivers/gpu/drm/radeon/radeon_drv.c
> > index 49ce2e7d5f9e..59f8186a2415 100644
> > --- a/drivers/gpu/drm/radeon/radeon_drv.c
> > +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> > @@ -37,6 +37,7 @@
> >  #include 
> >  #include 
> >
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> > const struct pci_device_id *ent)
> >  {
> > unsigned long flags = 0;
> > +   struct drm_device *dev;
> > int ret;
> >
> > if (!ent)
> > @@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> > if (ret)
> > return ret;
> >
> > -   return drm_get_pci_dev(pdev, ent, _driver);
> > +   dev = drm_dev_alloc(_driver, >dev);
> > +   if (IS_ERR(dev))
> > +   return PTR_ERR(dev);
> > +
> > +   ret = pci_enable_device(pdev);
> > +   if (ret)
> > +   goto err_free;
> > +
> > +   dev->pdev = pdev;
> > +#ifdef __alpha__
> > +   dev->hose = pdev->sysdata;
> > +#endif
> > +
> > +   pci_set_drvdata(pdev, dev);
> > +
> > +   if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> > +   dev->agp = drm_agp_init(dev);
> > +   if (dev->agp) {
> > +   dev->agp->agp_mtrr = arch_phys_wc_add(
> > +   dev->agp->agp_info.aper_base,
> > +   dev->agp->agp_info.aper_size *
> > +   1024 * 1024);
> > +   }
> > +
> IMHO a better solution is kill off the drm_agpsupport.c dependency all 
> together.
> As-is it's still used, making the legacy vs not line really moot.
>
> Especially, since the AGP ioctl (in the legacy code) can manipulate
> the underlying state.
>
> Off the top of my head, in radeon_agp_init():
>  - at the top agp_backend_acquire() + agp_copy_info()
>  - followed up by existing mode magic
>  - opencode the enable - agp_enable() + acquired = true;
>  - mtrr = arch_phys_wc_add() and the rest
>
> In radeon_agp_fini()
>  - if !acquired { agp_backend_release(); acquired = false }
>
>
> Something like ^^ should result in a net diffstat of around zero.
> All thanks to the interesting layer that drm_agp is ;-)
>
> With this in place we can make move drm_device::agp and
> DRM_IOCTL_AGP_INFO behind CONFIG_DRM_LEGACY.

Yeah could be done, but I was more out to get the drm_pci.c stuff, not
the agp stuff. But I did realize that nouveau also just directly
accesses the agp bridge stuff, so we could indeed nuke the midlayer
here. The legacy agp stuff ioctl stuff should be behind DRIVER_LEGACY
checks already, so no way to cause harm that way (I hope at least, if
there's a gap we'd need to close it).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amd/display: add HDCP caps debugfs

[Bhawanpreet Lakha]

Add debugfs to get HDCP capability. This is also useful for
kms_content_protection igt test.

Use:
cat /sys/kernel/debug/dri/0/DP-1/hdcp_sink_capability
cat /sys/kernel/debug/dri/0/HDMI-A-1/hdcp_sink_capability

Signed-off-by: Bhawanpreet Lakha 
---
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 61 +++
 1 file changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index ead5c05eec92..52982c8c871f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -815,6 +815,44 @@ static int vrr_range_show(struct seq_file *m, void *data)
return 0;
 }
 
+#ifdef CONFIG_DRM_AMD_DC_HDCP
+/*
+ * Returns the HDCP capability of the Display (1.4 for now).
+ *
+ * NOTE* Not all HDMI displays report their HDCP caps even when they are 
capable.
+ * Since its rare for a display to not be HDCP 1.4 capable, we set HDMI as 
always capable.
+ *
+ * Example usage: cat /sys/kernel/debug/dri/0/DP-1/hdcp_sink_capability
+ * or cat /sys/kernel/debug/dri/0/HDMI-A-1/hdcp_sink_capability
+ */
+static int hdcp_sink_capability_show(struct seq_file *m, void *data)
+{
+   struct drm_connector *connector = m->private;
+   struct amdgpu_dm_connector *aconnector = 
to_amdgpu_dm_connector(connector);
+   bool hdcp_cap, hdcp2_cap;
+
+   if (connector->status != connector_status_connected)
+   return -ENODEV;
+
+   seq_printf(m, "%s:%d HDCP version: ", connector->name, 
connector->base.id);
+
+   hdcp_cap = dc_link_is_hdcp14(aconnector->dc_link);
+   hdcp2_cap = dc_link_is_hdcp22(aconnector->dc_link);
+
+
+   if (hdcp_cap)
+   seq_printf(m, "%s ", "HDCP1.4");
+   if (hdcp2_cap)
+   seq_printf(m, "%s ", "HDCP2.2");
+
+   if (!hdcp_cap && !hdcp2_cap)
+   seq_printf(m, "%s ", "None");
+
+   seq_puts(m, "\n");
+
+   return 0;
+}
+#endif
 /* function description
  *
  * generic SDP message access for testing
@@ -940,6 +978,9 @@ static ssize_t dp_dpcd_data_read(struct file *f, char 
__user *buf,
 DEFINE_SHOW_ATTRIBUTE(dmub_tracebuffer);
 DEFINE_SHOW_ATTRIBUTE(output_bpc);
 DEFINE_SHOW_ATTRIBUTE(vrr_range);
+#ifdef CONFIG_DRM_AMD_DC_HDCP
+DEFINE_SHOW_ATTRIBUTE(hdcp_sink_capability);
+#endif
 
 static const struct file_operations dp_link_settings_debugfs_fops = {
.owner = THIS_MODULE,
@@ -995,12 +1036,23 @@ static const struct {
{"test_pattern", _phy_test_pattern_fops},
{"output_bpc", _bpc_fops},
{"vrr_range", _range_fops},
+#ifdef CONFIG_DRM_AMD_DC_HDCP
+   {"hdcp_sink_capability", _sink_capability_fops},
+#endif
{"sdp_message", _message_fops},
{"aux_dpcd_address", _dpcd_address_debugfs_fops},
{"aux_dpcd_size", _dpcd_size_debugfs_fops},
{"aux_dpcd_data", _dpcd_data_debugfs_fops}
 };
 
+#ifdef CONFIG_DRM_AMD_DC_HDCP
+static const struct {
+   char *name;
+   const struct file_operations *fops;
+} hdmi_debugfs_entries[] = {
+   {"hdcp_sink_capability", _sink_capability_fops}
+};
+#endif
 /*
  * Force YUV420 output if available from the given mode
  */
@@ -1066,6 +1118,15 @@ void connector_debugfs_init(struct amdgpu_dm_connector 
*connector)
debugfs_create_file_unsafe("force_yuv420_output", 0644, dir, connector,
   _yuv420_output_fops);
 
+#ifdef CONFIG_DRM_AMD_DC_HDCP
+   if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA) {
+   for (i = 0; i < ARRAY_SIZE(hdmi_debugfs_entries); i++) {
+   debugfs_create_file(hdmi_debugfs_entries[i].name,
+   0644, dir, connector,
+   hdmi_debugfs_entries[i].fops);
+   }
+   }
+#endif
 }
 
 /*
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: log TA versions on init

[Bhawanpreet Lakha]

It is helpful to know what version the TA's are for debugging

Signed-off-by: Bhawanpreet Lakha 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index a16c8101e250..09d1433677a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -358,6 +358,7 @@ static int psp_asd_load(struct psp_context *psp)
if (!ret) {
psp->asd_context.asd_initialized = true;
psp->asd_context.session_id = cmd->resp.session_id;
+   DRM_INFO("ASD: Initialized (version: 0x%X)", 
psp->asd_fw_version);
}
 
kfree(cmd);
@@ -518,6 +519,7 @@ static int psp_xgmi_load(struct psp_context *psp)
if (!ret) {
psp->xgmi_context.initialized = 1;
psp->xgmi_context.session_id = cmd->resp.session_id;
+   DRM_INFO("XGMI: Initialized (version: 0x%X)", 
psp->ta_xgmi_ucode_version);
}
 
kfree(cmd);
@@ -658,6 +660,7 @@ static int psp_ras_load(struct psp_context *psp)
if (!ret) {
psp->ras.ras_initialized = true;
psp->ras.session_id = cmd->resp.session_id;
+   DRM_INFO("RAS: Initialized (version: 0x%X)", 
psp->ta_ras_ucode_version);
}
 
kfree(cmd);
@@ -832,6 +835,7 @@ static int psp_hdcp_load(struct psp_context *psp)
if (!ret) {
psp->hdcp_context.hdcp_initialized = true;
psp->hdcp_context.session_id = cmd->resp.session_id;
+   DRM_INFO("HDCP: Initialized (version: 0x%X)", 
psp->ta_hdcp_ucode_version);
}
 
kfree(cmd);
@@ -977,6 +981,7 @@ static int psp_dtm_load(struct psp_context *psp)
if (!ret) {
psp->dtm_context.dtm_initialized = true;
psp->dtm_context.session_id = cmd->resp.session_id;
+   DRM_INFO("DTM: Initialized (version: 0x%X)", 
psp->ta_dtm_ucode_version);
}
 
kfree(cmd);
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add a chunk ID for spm trace

[Christian König]

Leave the old spm_vmid there is not a problem because other new spm 
enabled job will update it before it’s running and other spm disabled 
job will not access spm_vmid register.
Do you have a high level description of how SPM works? For example would 
it be possible to cause a problem in another application if we leave the 
SPM VMID here and the VMID is reused?


Keep in mind that we need to guarantee process isolation, e.g. we can't 
allow anything bad to happen to another application if somebody is 
stupid enough to turn on SPM tracing without setting it up properly.


Regards,
Christian.

Am 24.02.20 um 09:06 schrieb He, Jacob:


Ok, I’m glad that there is no uapi change needed.

I checked the git log, and reserve_vmid was added for shader debugger, 
not gpa. I’m fine to re-use it since the spm will not enabled for 
shader debug in general. I’ll try to change my patch.


BTW, “ring->funcs->setup_spm(ring, NULL)” to clear the vmid is not 
gonna work since the job with spm enabled execution is not done yet. 
Leave the old spm_vmid there is not a problem because other new spm 
enabled job will update it before it’s running and other spm disabled 
job will not access spm_vmid register.


Thanks

Jacob

*From: *Zhou, David(ChunMing) 
*Sent: *Saturday, February 22, 2020 12:45 AM
*To: *Koenig, Christian ; Deucher, 
Alexander ; Christian König 
; He, Jacob 
; amd-gfx@lists.freedesktop.org 


*Subject: *RE: [PATCH] drm/amdgpu: Add a chunk ID for spm trace

[AMD Official Use Only - Internal Distribution Only]

That’s fine to me.

-David

*From:* Koenig, Christian 
*Sent:* Friday, February 21, 2020 11:33 PM
*To:* Deucher, Alexander ; Christian König 
; Zhou, David(ChunMing) 
; He, Jacob ; 
amd-gfx@lists.freedesktop.org

*Subject:* Re: [PATCH] drm/amdgpu: Add a chunk ID for spm trace

I would just do this as part of the vm_flush() callback on the ring.

E.g. check if the VMID you want to flush is reserved and if yes enable 
SPM.


Maybe pass along a flag or something in the job to make things easier.

Christian.

Am 21.02.20 um 16:31 schrieb Deucher, Alexander:

[AMD Public Use]

We already have the RESERVE_VMID ioctl interface, can't we just
use that internally in the kernel to update the rlc register via
the ring when we schedule the relevant IB?  E.g., add a new ring
callback to set SPM state and then set it to the reserved vmid
before we schedule the ib, and then reset it to 0 after the IB in
amdgpu_ib_schedule().

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c

index 4b2342d11520..e0db9362c6ee 100644

--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c

+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c

@@ -185,6 +185,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring
*ring, unsigned num_ibs,

        if (ring->funcs->insert_start)

ring->funcs->insert_start(ring);

+       if (ring->funcs->setup_spm)

+ ring->funcs->setup_spm(ring, job);

+

        if (job) {

r = amdgpu_vm_flush(ring, job, need_pipe_sync);

if (r) {

@@ -273,6 +276,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring
*ring, unsigned num_ibs,

return r;

        }

+       if (ring->funcs->setup_spm)

+ ring->funcs->setup_spm(ring, NULL);

+

        if (ring->funcs->insert_end)

ring->funcs->insert_end(ring);

Alex

*From:*amd-gfx 
 on behalf of
Christian König 

*Sent:* Friday, February 21, 2020 5:28 AM
*To:* Zhou, David(ChunMing) 
; He, Jacob 
; Koenig, Christian
 ;
amd-gfx@lists.freedesktop.org

 
*Subject:* Re: [PATCH] drm/amdgpu: Add a chunk ID for spm trace

That would probably be a no-go, but we could enhance the kernel
driver to update the RLC_SPM_VMID register with the reserved VMID.

Handling that in userspace is most likely not working anyway,
since the RLC registers are usually not accessible by userspace.

Regards,
Christian.

Am 20.02.20 um 16:15 schrieb Zhou, David(ChunMing):

[AMD Official Use Only - Internal Distribution Only]

You can enhance amdgpu_vm_ioctl In amdgpu_vm.c to return vmid
to userspace.

-David

*From:* He, Jacob  
*Sent:* Thursday, February 20, 2020 10:46 PM
*To:* Zhou, David(ChunMing) 
; Koenig, Christian
 ;
amd-gfx@lists.freedesktop.org

Re: [RFC PATCH 1/1] drm/amdgpu: wait for sched to become ready on job submit

[Christian König]

I think that the IB tests already set the ready flag to false when 
something goes wrong, but for the ring tests your probably want to 
double check and maybe generalize that.


Christian.

Am 24.02.20 um 19:15 schrieb Das, Nirmoy:


[AMD Official Use Only - Internal Distribution Only]


Thanks Christian. I will try to send a updated patch soon.

Get Outlook for Android 


*From:* Koenig, Christian 
*Sent:* Monday, February 24, 2020, 18:06
*To:* Nirmoy Das
*Cc:* amd-gfx@lists.freedesktop.org; Deucher, Alexander; Liu, Monk; 
Li, Dennis; Das, Nirmoy
*Subject:* Re: [RFC PATCH 1/1] drm/amdgpu: wait for sched to become 
ready on job submit


Hi Nirmoy,

Am 24.02.2020 17:48 schrieb Nirmoy Das :

On reset, amdgpu can set a drm sched's ready status to false
temporarily. drm job
init will fail if all of the drm scheds are not ready for a HW IP.
This patch tries to make
kernel's internal drm job submit handle, amdgpu_job_submit() a bit
more fault tolerant.


I don't think that this approach makes sense. Since it is a front end 
property we should rather stop setting the scheduler ready status to 
false during reset.


Instead we should only set it to false when the ring/IB test fails and 
we can't bring the ring back to life again.


Christian.


Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 35 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  5 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  6 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c |  2 +-
 7 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index d42be880a236..0745df80112f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -139,7 +139,38 @@ void amdgpu_job_free(struct amdgpu_job *job)
 kfree(job);
 }

-int amdgpu_job_submit(struct amdgpu_job *job, struct
drm_sched_entity *entity,
+static int amdgpu_job_try_init(struct amdgpu_device *adev,
+  struct drm_sched_job *base,
+  struct drm_sched_entity *entity,
+  void *owner)
+{
+   int r, i;
+
+   r = drm_sched_job_init(base, entity, owner);
+   if (r == -ENOENT) {
+   /* retry till we come out of reset phase */
+   while (!mutex_trylock(>lock_reset))
+   msleep(10);
+   /* retry for a second for the sched to get ready*/
+   for (i = 0; i < 100; i++) {
+   msleep(10);
+   r = drm_sched_job_init(base, entity, owner);
+   if (r == -ENOENT)
+   continue;
+   }
+
+ mutex_unlock(>lock_reset);
+   /* If after all these we failed to initialize a job
+    * it means the IP is unrecoverable */
+   if (r == -ENOENT)
+   return -ENODEV;
+   }
+
+   return r;
+}
+
+int amdgpu_job_submit(struct amdgpu_device *adev,struct
amdgpu_job *job,
+ struct drm_sched_entity *entity,
   void *owner, struct dma_fence **f)
 {
 enum drm_sched_priority priority;
@@ -149,7 +180,7 @@ int amdgpu_job_submit(struct amdgpu_job *job,
struct drm_sched_entity *entity,
 if (!f)
 return -EINVAL;

-   r = drm_sched_job_init(>base, entity, owner);
+   r = amdgpu_job_try_init(adev, >base, entity, owner);
 if (r)
 return r;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 2e2110dddb76..fed87e96cacc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -70,8 +70,9 @@ int amdgpu_job_alloc_with_ib(struct
amdgpu_device *adev, unsigned size,

 void amdgpu_job_free_resources(struct amdgpu_job *job);
 void amdgpu_job_free(struct amdgpu_job *job);
-int amdgpu_job_submit(struct amdgpu_job *job, struct
drm_sched_entity *entity,
- void *owner, struct dma_fence **f);
+int amdgpu_job_submit(struct amdgpu_device *adev, struct
amdgpu_job *job,
+ struct drm_sched_entity *entity, void *owner,
+ struct dma_fence **f);
 int amdgpu_job_submit_direct(struct amdgpu_job *job, struct
amdgpu_ring *ring,
   

Re: [RFC PATCH 1/1] drm/amdgpu: wait for sched to become ready on job submit

[Das, Nirmoy]

[AMD Official Use Only - Internal Distribution Only]

Thanks Christian. I will try to send a updated patch soon.

Get Outlook for Android


From: Koenig, Christian 
Sent: Monday, February 24, 2020, 18:06
To: Nirmoy Das
Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander; Liu, Monk; Li, Dennis; 
Das, Nirmoy
Subject: Re: [RFC PATCH 1/1] drm/amdgpu: wait for sched to become ready on job 
submit

Hi Nirmoy,

Am 24.02.2020 17:48 schrieb Nirmoy Das :
On reset, amdgpu can set a drm sched's ready status to false temporarily. drm 
job
init will fail if all of the drm scheds are not ready for a HW IP. This patch 
tries to make
kernel's internal drm job submit handle, amdgpu_job_submit() a bit more fault 
tolerant.

I don't think that this approach makes sense. Since it is a front end property 
we should rather stop setting the scheduler ready status to false during reset.

Instead we should only set it to false when the ring/IB test fails and we can't 
bring the ring back to life again.

Christian.


Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 35 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  5 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  6 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  |  2 +-
 7 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index d42be880a236..0745df80112f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -139,7 +139,38 @@ void amdgpu_job_free(struct amdgpu_job *job)
 kfree(job);
 }

-int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
+static int amdgpu_job_try_init(struct amdgpu_device *adev,
+  struct drm_sched_job *base,
+  struct drm_sched_entity *entity,
+  void *owner)
+{
+   int r, i;
+
+   r = drm_sched_job_init(base, entity, owner);
+   if (r == -ENOENT) {
+   /* retry till we come out of reset phase */
+   while (!mutex_trylock(>lock_reset))
+   msleep(10);
+   /* retry for a second for the sched to get ready*/
+   for (i = 0; i < 100; i++) {
+   msleep(10);
+   r = drm_sched_job_init(base, entity, owner);
+   if (r == -ENOENT)
+   continue;
+   }
+
+   mutex_unlock(>lock_reset);
+   /* If after all these we failed to initialize a job
+* it means the IP is unrecoverable */
+   if (r == -ENOENT)
+   return -ENODEV;
+   }
+
+   return r;
+}
+
+int amdgpu_job_submit(struct amdgpu_device *adev,struct amdgpu_job *job,
+ struct drm_sched_entity *entity,
   void *owner, struct dma_fence **f)
 {
 enum drm_sched_priority priority;
@@ -149,7 +180,7 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct 
drm_sched_entity *entity,
 if (!f)
 return -EINVAL;

-   r = drm_sched_job_init(>base, entity, owner);
+   r = amdgpu_job_try_init(adev, >base, entity, owner);
 if (r)
 return r;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 2e2110dddb76..fed87e96cacc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -70,8 +70,9 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, 
unsigned size,

 void amdgpu_job_free_resources(struct amdgpu_job *job);
 void amdgpu_job_free(struct amdgpu_job *job);
-int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
- void *owner, struct dma_fence **f);
+int amdgpu_job_submit(struct amdgpu_device *adev, struct amdgpu_job *job,
+ struct drm_sched_entity *entity, void *owner,
+ struct dma_fence **f);
 int amdgpu_job_submit_direct(struct amdgpu_job *job, struct amdgpu_ring *ring,
  struct dma_fence **fence);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 660867cf2597..adfde07eb75f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2066,7 +2066,7 @@ static int amdgpu_map_buffer(struct ttm_buffer_object *bo,
 if (r)
 goto error_free;

-   r = amdgpu_job_submit(job, >mman.entity,
+   r = amdgpu_job_submit(adev, job, >mman.entity,
   AMDGPU_FENCE_OWNER_UNDEFINED, );
 if (r)
   

Re: [RFC PATCH 1/1] drm/amdgpu: wait for sched to become ready on job submit

[Koenig, Christian]

Hi Nirmoy,

Am 24.02.2020 17:48 schrieb Nirmoy Das :
On reset, amdgpu can set a drm sched's ready status to false temporarily. drm 
job
init will fail if all of the drm scheds are not ready for a HW IP. This patch 
tries to make
kernel's internal drm job submit handle, amdgpu_job_submit() a bit more fault 
tolerant.

I don't think that this approach makes sense. Since it is a front end property 
we should rather stop setting the scheduler ready status to false during reset.

Instead we should only set it to false when the ring/IB test fails and we can't 
bring the ring back to life again.

Christian.


Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 35 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  5 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  6 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  |  2 +-
 7 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index d42be880a236..0745df80112f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -139,7 +139,38 @@ void amdgpu_job_free(struct amdgpu_job *job)
 kfree(job);
 }

-int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
+static int amdgpu_job_try_init(struct amdgpu_device *adev,
+  struct drm_sched_job *base,
+  struct drm_sched_entity *entity,
+  void *owner)
+{
+   int r, i;
+
+   r = drm_sched_job_init(base, entity, owner);
+   if (r == -ENOENT) {
+   /* retry till we come out of reset phase */
+   while (!mutex_trylock(>lock_reset))
+   msleep(10);
+   /* retry for a second for the sched to get ready*/
+   for (i = 0; i < 100; i++) {
+   msleep(10);
+   r = drm_sched_job_init(base, entity, owner);
+   if (r == -ENOENT)
+   continue;
+   }
+
+   mutex_unlock(>lock_reset);
+   /* If after all these we failed to initialize a job
+* it means the IP is unrecoverable */
+   if (r == -ENOENT)
+   return -ENODEV;
+   }
+
+   return r;
+}
+
+int amdgpu_job_submit(struct amdgpu_device *adev,struct amdgpu_job *job,
+ struct drm_sched_entity *entity,
   void *owner, struct dma_fence **f)
 {
 enum drm_sched_priority priority;
@@ -149,7 +180,7 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct 
drm_sched_entity *entity,
 if (!f)
 return -EINVAL;

-   r = drm_sched_job_init(>base, entity, owner);
+   r = amdgpu_job_try_init(adev, >base, entity, owner);
 if (r)
 return r;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 2e2110dddb76..fed87e96cacc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -70,8 +70,9 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, 
unsigned size,

 void amdgpu_job_free_resources(struct amdgpu_job *job);
 void amdgpu_job_free(struct amdgpu_job *job);
-int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
- void *owner, struct dma_fence **f);
+int amdgpu_job_submit(struct amdgpu_device *adev, struct amdgpu_job *job,
+ struct drm_sched_entity *entity, void *owner,
+ struct dma_fence **f);
 int amdgpu_job_submit_direct(struct amdgpu_job *job, struct amdgpu_ring *ring,
  struct dma_fence **fence);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 660867cf2597..adfde07eb75f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2066,7 +2066,7 @@ static int amdgpu_map_buffer(struct ttm_buffer_object *bo,
 if (r)
 goto error_free;

-   r = amdgpu_job_submit(job, >mman.entity,
+   r = amdgpu_job_submit(adev, job, >mman.entity,
   AMDGPU_FENCE_OWNER_UNDEFINED, );
 if (r)
 goto error_free;
@@ -2137,7 +2137,7 @@ int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t 
src_offset,
 if (direct_submit)
 r = amdgpu_job_submit_direct(job, ring, fence);
 else
-   r = amdgpu_job_submit(job, >mman.entity,
+   r = amdgpu_job_submit(adev, job, >mman.entity,
   AMDGPU_FENCE_OWNER_UNDEFINED, fence);
 if (r)
 

[RFC PATCH 1/1] drm/amdgpu: wait for sched to become ready on job submit

[Nirmoy Das]

On reset, amdgpu can set a drm sched's ready status to false temporarily. drm 
job
init will fail if all of the drm scheds are not ready for a HW IP. This patch 
tries to make
kernel's internal drm job submit handle, amdgpu_job_submit() a bit more fault 
tolerant.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 35 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  5 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  6 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  |  2 +-
 7 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index d42be880a236..0745df80112f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -139,7 +139,38 @@ void amdgpu_job_free(struct amdgpu_job *job)
kfree(job);
 }
 
-int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
+static int amdgpu_job_try_init(struct amdgpu_device *adev,
+  struct drm_sched_job *base,
+  struct drm_sched_entity *entity,
+  void *owner)
+{
+   int r, i;
+
+   r = drm_sched_job_init(base, entity, owner);
+   if (r == -ENOENT) {
+   /* retry till we come out of reset phase */
+   while (!mutex_trylock(>lock_reset))
+   msleep(10);
+   /* retry for a second for the sched to get ready*/
+   for (i = 0; i < 100; i++) {
+   msleep(10);
+   r = drm_sched_job_init(base, entity, owner);
+   if (r == -ENOENT)
+   continue;
+   }
+
+   mutex_unlock(>lock_reset);
+   /* If after all these we failed to initialize a job
+* it means the IP is unrecoverable */
+   if (r == -ENOENT)
+   return -ENODEV;
+   }
+
+   return r;
+}
+
+int amdgpu_job_submit(struct amdgpu_device *adev,struct amdgpu_job *job,
+ struct drm_sched_entity *entity,
  void *owner, struct dma_fence **f)
 {
enum drm_sched_priority priority;
@@ -149,7 +180,7 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct 
drm_sched_entity *entity,
if (!f)
return -EINVAL;
 
-   r = drm_sched_job_init(>base, entity, owner);
+   r = amdgpu_job_try_init(adev, >base, entity, owner);
if (r)
return r;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 2e2110dddb76..fed87e96cacc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -70,8 +70,9 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, 
unsigned size,
 
 void amdgpu_job_free_resources(struct amdgpu_job *job);
 void amdgpu_job_free(struct amdgpu_job *job);
-int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
- void *owner, struct dma_fence **f);
+int amdgpu_job_submit(struct amdgpu_device *adev, struct amdgpu_job *job,
+ struct drm_sched_entity *entity, void *owner,
+ struct dma_fence **f);
 int amdgpu_job_submit_direct(struct amdgpu_job *job, struct amdgpu_ring *ring,
 struct dma_fence **fence);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 660867cf2597..adfde07eb75f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2066,7 +2066,7 @@ static int amdgpu_map_buffer(struct ttm_buffer_object *bo,
if (r)
goto error_free;
 
-   r = amdgpu_job_submit(job, >mman.entity,
+   r = amdgpu_job_submit(adev, job, >mman.entity,
  AMDGPU_FENCE_OWNER_UNDEFINED, );
if (r)
goto error_free;
@@ -2137,7 +2137,7 @@ int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t 
src_offset,
if (direct_submit)
r = amdgpu_job_submit_direct(job, ring, fence);
else
-   r = amdgpu_job_submit(job, >mman.entity,
+   r = amdgpu_job_submit(adev, job, >mman.entity,
  AMDGPU_FENCE_OWNER_UNDEFINED, fence);
if (r)
goto error_free;
@@ -2231,7 +2231,7 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 
amdgpu_ring_pad_ib(ring, >ibs[0]);
WARN_ON(job->ibs[0].length_dw > num_dw);
-   r = amdgpu_job_submit(job, >mman.entity,
+   r = amdgpu_job_submit(adev, job, >mman.entity,
  AMDGPU_FENCE_OWNER_UNDEFINED, fence);
   

[RFC PATCH 1/1] drm/amdgpu: wait for sched to become ready on job submit

[Nirmoy Das]

On reset, amdgpu can set a drm sched's ready status to false temporarily. drm 
job
init will fail if all of the drm scheds are not ready for a HW IP. This patch 
tries to make
kernel's internal drm job submit handle, amdgpu_job_submit() a bit more fault 
tolerant.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 35 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  5 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  6 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  |  2 +-
 7 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index d42be880a236..0745df80112f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -139,7 +139,38 @@ void amdgpu_job_free(struct amdgpu_job *job)
kfree(job);
 }
 
-int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
+static int amdgpu_job_try_init(struct amdgpu_device *adev,
+  struct drm_sched_job *base,
+  struct drm_sched_entity *entity,
+  void *owner)
+{
+   int r, i;
+
+   r = drm_sched_job_init(base, entity, owner);
+   if (r == -ENOENT) {
+   /* retry till we come out of reset phase */
+   while (!mutex_trylock(>lock_reset))
+   msleep(10);
+   /* retry for a second for the sched to get ready*/
+   for (i = 0; i < 100; i++) {
+   msleep(10);
+   r = drm_sched_job_init(base, entity, owner);
+   if (r == -ENOENT)
+   continue;
+   }
+
+   mutex_unlock(>lock_reset);
+   /* If after all these we failed to initialize a job
+* it means the IP is unrecoverable */
+   if (r == -ENOENT)
+   return -ENODEV;
+   }
+
+   return r;
+}
+
+int amdgpu_job_submit(struct amdgpu_device *adev,struct amdgpu_job *job,
+ struct drm_sched_entity *entity,
  void *owner, struct dma_fence **f)
 {
enum drm_sched_priority priority;
@@ -149,7 +180,7 @@ int amdgpu_job_submit(struct amdgpu_job *job, struct 
drm_sched_entity *entity,
if (!f)
return -EINVAL;
 
-   r = drm_sched_job_init(>base, entity, owner);
+   r = amdgpu_job_try_init(adev, >base, entity, owner);
if (r)
return r;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 2e2110dddb76..fed87e96cacc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -70,8 +70,9 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, 
unsigned size,
 
 void amdgpu_job_free_resources(struct amdgpu_job *job);
 void amdgpu_job_free(struct amdgpu_job *job);
-int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
- void *owner, struct dma_fence **f);
+int amdgpu_job_submit(struct amdgpu_device *adev, struct amdgpu_job *job,
+ struct drm_sched_entity *entity, void *owner,
+ struct dma_fence **f);
 int amdgpu_job_submit_direct(struct amdgpu_job *job, struct amdgpu_ring *ring,
 struct dma_fence **fence);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 660867cf2597..adfde07eb75f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2066,7 +2066,7 @@ static int amdgpu_map_buffer(struct ttm_buffer_object *bo,
if (r)
goto error_free;
 
-   r = amdgpu_job_submit(job, >mman.entity,
+   r = amdgpu_job_submit(adev, job, >mman.entity,
  AMDGPU_FENCE_OWNER_UNDEFINED, );
if (r)
goto error_free;
@@ -2137,7 +2137,7 @@ int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t 
src_offset,
if (direct_submit)
r = amdgpu_job_submit_direct(job, ring, fence);
else
-   r = amdgpu_job_submit(job, >mman.entity,
+   r = amdgpu_job_submit(adev, job, >mman.entity,
  AMDGPU_FENCE_OWNER_UNDEFINED, fence);
if (r)
goto error_free;
@@ -2231,7 +2231,7 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 
amdgpu_ring_pad_ib(ring, >ibs[0]);
WARN_ON(job->ibs[0].length_dw > num_dw);
-   r = amdgpu_job_submit(job, >mman.entity,
+   r = amdgpu_job_submit(adev, job, >mman.entity,
  AMDGPU_FENCE_OWNER_UNDEFINED, fence);
   

Re: [PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Emil Velikov]

On Sat, 22 Feb 2020 at 17:54, Daniel Vetter  wrote:
>
> It's the last user, and more importantly, it's the last non-legacy
> user of anything in drm_pci.c.
>
> The only tricky bit is the agp initialization. But a close look shows
> that radeon does not use the drm_agp midlayer (the main use of that is
> drm_bufs for legacy drivers), and instead could use the agp subsystem
> directly (like nouveau does already). Hence we can just pull this in
> too.
>
> A further step would be to entirely drop the use of drm_device->agp,
> but feels like too much churn just for this patch.
>
> Signed-off-by: Daniel Vetter 
> Cc: Alex Deucher 
> Cc: "Christian König" 
> Cc: "David (ChunMing) Zhou" 
> Cc: amd-gfx@lists.freedesktop.org
> ---
>  drivers/gpu/drm/radeon/radeon_drv.c | 43 +++--
>  drivers/gpu/drm/radeon/radeon_kms.c |  6 
>  2 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 49ce2e7d5f9e..59f8186a2415 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -37,6 +37,7 @@
>  #include 
>  #include 
>
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> const struct pci_device_id *ent)
>  {
> unsigned long flags = 0;
> +   struct drm_device *dev;
> int ret;
>
> if (!ent)
> @@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> if (ret)
> return ret;
>
> -   return drm_get_pci_dev(pdev, ent, _driver);
> +   dev = drm_dev_alloc(_driver, >dev);
> +   if (IS_ERR(dev))
> +   return PTR_ERR(dev);
> +
> +   ret = pci_enable_device(pdev);
> +   if (ret)
> +   goto err_free;
> +
> +   dev->pdev = pdev;
> +#ifdef __alpha__
> +   dev->hose = pdev->sysdata;
> +#endif
> +
> +   pci_set_drvdata(pdev, dev);
> +
> +   if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> +   dev->agp = drm_agp_init(dev);
> +   if (dev->agp) {
> +   dev->agp->agp_mtrr = arch_phys_wc_add(
> +   dev->agp->agp_info.aper_base,
> +   dev->agp->agp_info.aper_size *
> +   1024 * 1024);
> +   }
> +
IMHO a better solution is kill off the drm_agpsupport.c dependency all together.
As-is it's still used, making the legacy vs not line really moot.

Especially, since the AGP ioctl (in the legacy code) can manipulate
the underlying state.

Off the top of my head, in radeon_agp_init():
 - at the top agp_backend_acquire() + agp_copy_info()
 - followed up by existing mode magic
 - opencode the enable - agp_enable() + acquired = true;
 - mtrr = arch_phys_wc_add() and the rest

In radeon_agp_fini()
 - if !acquired { agp_backend_release(); acquired = false }


Something like ^^ should result in a net diffstat of around zero.
All thanks to the interesting layer that drm_agp is ;-)

With this in place we can make move drm_device::agp and
DRM_IOCTL_AGP_INFO behind CONFIG_DRM_LEGACY.

-Emil
P.S. Watch out for radeon_ttm.c warnings/errors
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/amdgpu: Add gfxoff debugfs entry

[Tom St Denis]

On 2020-02-21 1:59 p.m., Alex Deucher wrote:

On Fri, Feb 21, 2020 at 1:45 PM Tom St Denis  wrote:

Write a 32-bit value of zero to disable GFXOFF and write a 32-bit
value of non-zero to enable GFXOFF.

Signed-off-by: Tom St Denis 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 56 +
  1 file changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 7379910790c9..3bb74056b9d2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -842,6 +842,55 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, 
char __user *buf,
 return result;
  }

+/**
+ * amdgpu_debugfs_regs_gfxoff_write - Enable/disable GFXOFF
+ *
+ * @f: open file handle
+ * @buf: User buffer to write data from
+ * @size: Number of bytes to write
+ * @pos:  Offset to seek to
+ *
+ * Write a 32-bit zero to disable or a 32-bit non-zero to enable
+ */
+static ssize_t amdgpu_debugfs_gfxoff_write(struct file *f, const char __user 
*buf,
+size_t size, loff_t *pos)
+{
+   struct amdgpu_device *adev = file_inode(f)->i_private;
+   ssize_t result = 0;
+   int r;
+
+   if (size & 0x3 || *pos & 0x3)
+   return -EINVAL;
+
+   r = pm_runtime_get_sync(adev->ddev->dev);

Not really directly related to this patch, but If you are using umr
for debugging, we should probably disable runtime pm, otherwise the
entire GPU may be powered down between accesses.  There is already an
interface to do that via the core kernel power subsystem in sysfs.
E.g.,
/sys/class/drm/card0/device/power/control
/sys/class/drm/card0/device/power/runtime_status
Something else to look at for umr.


We ran into something related to this for UVD/VCE access back in the 
day.  When powered down the MMIO registers are mirrored and accessible 
but while in transition they are not.  So we added a PG flag to the 
offset in the debugfs entry to flag when we need to take the pm mutex or 
not.







We don't store the state for when we dynamically turn it off like this
so if we get a GPU reset or a power management event (runtime pm or
S3), GFXOFF will be re-enabled at that point.  This is just for
debugging though so:
Acked-by: Alex Deucher 


Good to note.  Can I get a R-b from someone though so I can push this out?


Tom

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] add DST_SEL=8 field name for WRITE_DATA packet

[Tom St Denis]

Thanks, both pushed out to the master branch.

Cheers,

Tom

On 2020-02-24 5:59 a.m., Xiaojie Yuan wrote:

otherwise we'll out-of-bound when accessing op_37_dst_sel[8]

Signed-off-by: Xiaojie Yuan 
---
  src/lib/ring_decode.c| 2 +-
  src/lib/umr_pm4_decode_opcodes.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
index c5582f5..f26cf0d 100644
--- a/src/lib/ring_decode.c
+++ b/src/lib/ring_decode.c
@@ -465,7 +465,7 @@ static void print_decode_pm4_pkt3(struct umr_asic *asic, 
struct umr_ring_decoder
  {
static const char *op_3c_functions[] = { "true", "<", "<=", "==", "!=", ">=", 
">", "reserved" };
static const char *op_37_engines[] = { "ME", "PFP", "CE", "DE" };
-   static const char *op_37_dst_sel[] = { "mem-mapped reg", "memory sync", "TC/L2", "GDS", "reserved", 
"memory async", "reserved", "reserved" };
+   static const char *op_37_dst_sel[] = { "mem-mapped reg", "memory sync", "TC/L2", "GDS", "reserved", 
"memory async", "reserved", "reserved", "preemption meta memory" };
static const char *op_40_mem_sel[] = { "mem-mapped reg", "memory" "tc_l2", "gds", "perfcounters", "immediate data", 
"atomic return data", "gds_atomic_return_data_0", "gds_atomic_return_data1", "gpu_clock_count", "system_clock_count" };
static const char *op_84_cntr_sel[] = { "invalid", "ce", "cs", "ce and 
cs" };
static const char *op_7a_index_str[] = { "default", "prim_type", "index_type", "num_instance", 
"multi_vgt_param", "reserved", "reserved", "reserved" };
diff --git a/src/lib/umr_pm4_decode_opcodes.c b/src/lib/umr_pm4_decode_opcodes.c
index a823ecf..c4ad5ce 100644
--- a/src/lib/umr_pm4_decode_opcodes.c
+++ b/src/lib/umr_pm4_decode_opcodes.c
@@ -351,7 +351,7 @@ static void decode_pkt3(struct umr_asic *asic, struct 
umr_pm4_stream_decode_ui *
  {
static char *op_3c_functions[] = { "true", "<", "<=", "==", "!=", ">=", ">", 
"reserved" };
static char *op_37_engines[] = { "ME", "PFP", "CE", "DE" };
-   static char *op_37_dst_sel[] = { "mem-mapped reg", "memory sync", "TC/L2", "GDS", "reserved", 
"memory async", "reserved", "reserved" };
+   static char *op_37_dst_sel[] = { "mem-mapped reg", "memory sync", "TC/L2", "GDS", "reserved", "memory 
async", "reserved", "reserved", "preemption meta memory" };
static char *op_40_mem_sel[] = { "mem-mapped reg", "memory" "tc_l2", "gds", "perfcounters", "immediate data", 
"atomic return data", "gds_atomic_return_data_0", "gds_atomic_return_data1", "gpu_clock_count", "system_clock_count" };
static char *op_84_cntr_sel[] = { "invalid", "ce", "cs", "ce and cs" };
static char *op_7a_index_str[] = { "default", "prim_type", "index_type", "num_instance", 
"multi_vgt_param", "reserved", "reserved", "reserved" };

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence V2

[Zhang, Hawking]

[AMD Official Use Only - Internal Distribution Only]

The patch looks good for bare-metal case now. Frank is still on the way to 
clean up our concern on SETUP_VMR command (i.e. SETUP_TMR for bare-metal case). 
After that settled, please push the patch with my RB. Thanks.

Regards,
Hawking
-Original Message-
From: Quan, Evan  
Sent: Monday, February 24, 2020 18:36
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Feng, Kenneth 
; Quan, Evan 
Subject: [PATCH] drm/amdgpu: update psp firmwares loading sequence V2

For those ASICs with DF Cstate management centralized to PMFW, TMR setup should 
be performed between pmfw loading and other non-psp firmwares loading.

V2: skip possible SMU firmware reloading

Change-Id: I8986ddb4d9ffe63ed0823d1dce8d9d52812a1240
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 65 ++---  
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 +
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 51839ab02b84..d33f74100094 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -38,6 +38,39 @@
 
 static void psp_set_funcs(struct amdgpu_device *adev);
 
+/*
+ * Due to DF Cstate management centralized to PMFW, the firmware
+ * loading sequence will be updated as below:
+ *   - Load KDB
+ *   - Load SYS_DRV
+ *   - Load tOS
+ *   - Load PMFW
+ *   - Setup TMR
+ *   - Load other non-psp fw
+ *   - Load ASD
+ *   - Load XGMI/RAS/HDCP/DTM TA if any
+ *
+ * This new sequence is required for
+ *   - Arcturus
+ *   - Navi12 and onwards
+ */
+static void psp_check_pmfw_centralized_cstate_management(struct 
+psp_context *psp) {
+   struct amdgpu_device *adev = psp->adev;
+
+   psp->pmfw_centralized_cstate_management = false;
+
+   if (amdgpu_sriov_vf(adev))
+   return;
+
+   if (adev->flags & AMD_IS_APU)
+   return;
+
+   if ((adev->asic_type == CHIP_ARCTURUS) ||
+   (adev->asic_type >= CHIP_NAVI12))
+   psp->pmfw_centralized_cstate_management = true; }
+
 static int psp_early_init(void *handle)  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle; @@ -75,6 
+108,8 @@ static int psp_early_init(void *handle)
 
psp->adev = adev;
 
+   psp_check_pmfw_centralized_cstate_management(psp);
+
return 0;
 }
 
@@ -1116,10 +1151,17 @@ static int psp_hw_start(struct psp_context *psp)
return ret;
}
 
-   ret = psp_tmr_load(psp);
-   if (ret) {
-   DRM_ERROR("PSP load tmr failed!\n");
-   return ret;
+   /*
+* For those ASICs with DF Cstate management centralized
+* to PMFW, TMR setup should be performed after PMFW
+* loaded and before other non-psp firmware loaded.
+*/
+   if (!psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
}
 
return 0;
@@ -1316,7 +1358,8 @@ static int psp_np_fw_load(struct psp_context *psp)
struct amdgpu_firmware_info *ucode;
struct amdgpu_device* adev = psp->adev;
 
-   if (psp->autoload_supported) {
+   if (psp->autoload_supported ||
+   psp->pmfw_centralized_cstate_management) {
ucode = >firmware.ucode[AMDGPU_UCODE_ID_SMC];
if (!ucode->fw || amdgpu_sriov_vf(adev))
goto out;
@@ -1326,6 +1369,14 @@ static int psp_np_fw_load(struct psp_context *psp)
return ret;
}
 
+   if (psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
+   }
+
 out:
for (i = 0; i < adev->firmware.max_ucodes; i++) {
ucode = >firmware.ucode[i];
@@ -1333,7 +1384,9 @@ static int psp_np_fw_load(struct psp_context *psp)
continue;
 
if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
-   (psp_smu_reload_quirk(psp) || psp->autoload_supported))
+   (psp_smu_reload_quirk(psp) ||
+psp->autoload_supported ||
+psp->pmfw_centralized_cstate_management))
continue;
 
if (amdgpu_sriov_vf(adev) &&
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index c77e1abb538a..37fa184f27f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -264,6 +264,8 @@ struct psp_context
atomic_tfence_value;
/* flag to mark whether gfx fw autoload is supported or not */
bool   

[PATCH 2/2] add DST_SEL=8 field name for WRITE_DATA packet

[Xiaojie Yuan]

otherwise we'll out-of-bound when accessing op_37_dst_sel[8]

Signed-off-by: Xiaojie Yuan 
---
 src/lib/ring_decode.c| 2 +-
 src/lib/umr_pm4_decode_opcodes.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
index c5582f5..f26cf0d 100644
--- a/src/lib/ring_decode.c
+++ b/src/lib/ring_decode.c
@@ -465,7 +465,7 @@ static void print_decode_pm4_pkt3(struct umr_asic *asic, 
struct umr_ring_decoder
 {
static const char *op_3c_functions[] = { "true", "<", "<=", "==", "!=", 
">=", ">", "reserved" };
static const char *op_37_engines[] = { "ME", "PFP", "CE", "DE" };
-   static const char *op_37_dst_sel[] = { "mem-mapped reg", "memory sync", 
"TC/L2", "GDS", "reserved", "memory async", "reserved", "reserved" };
+   static const char *op_37_dst_sel[] = { "mem-mapped reg", "memory sync", 
"TC/L2", "GDS", "reserved", "memory async", "reserved", "reserved", "preemption 
meta memory" };
static const char *op_40_mem_sel[] = { "mem-mapped reg", "memory" 
"tc_l2", "gds", "perfcounters", "immediate data", "atomic return data", 
"gds_atomic_return_data_0", "gds_atomic_return_data1", "gpu_clock_count", 
"system_clock_count" };
static const char *op_84_cntr_sel[] = { "invalid", "ce", "cs", "ce and 
cs" };
static const char *op_7a_index_str[] = { "default", "prim_type", 
"index_type", "num_instance", "multi_vgt_param", "reserved", "reserved", 
"reserved" };
diff --git a/src/lib/umr_pm4_decode_opcodes.c b/src/lib/umr_pm4_decode_opcodes.c
index a823ecf..c4ad5ce 100644
--- a/src/lib/umr_pm4_decode_opcodes.c
+++ b/src/lib/umr_pm4_decode_opcodes.c
@@ -351,7 +351,7 @@ static void decode_pkt3(struct umr_asic *asic, struct 
umr_pm4_stream_decode_ui *
 {
static char *op_3c_functions[] = { "true", "<", "<=", "==", "!=", ">=", 
">", "reserved" };
static char *op_37_engines[] = { "ME", "PFP", "CE", "DE" };
-   static char *op_37_dst_sel[] = { "mem-mapped reg", "memory sync", 
"TC/L2", "GDS", "reserved", "memory async", "reserved", "reserved" };
+   static char *op_37_dst_sel[] = { "mem-mapped reg", "memory sync", 
"TC/L2", "GDS", "reserved", "memory async", "reserved", "reserved", "preemption 
meta memory" };
static char *op_40_mem_sel[] = { "mem-mapped reg", "memory" "tc_l2", 
"gds", "perfcounters", "immediate data", "atomic return data", 
"gds_atomic_return_data_0", "gds_atomic_return_data1", "gpu_clock_count", 
"system_clock_count" };
static char *op_84_cntr_sel[] = { "invalid", "ce", "cs", "ce and cs" };
static char *op_7a_index_str[] = { "default", "prim_type", 
"index_type", "num_instance", "multi_vgt_param", "reserved", "reserved", 
"reserved" };
-- 
2.20.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/2] print the missing 0x prefix in WRITE_DATA packet

[Xiaojie Yuan]

Signed-off-by: Xiaojie Yuan 
---
 src/lib/ring_decode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lib/ring_decode.c b/src/lib/ring_decode.c
index fa44f27..c5582f5 100644
--- a/src/lib/ring_decode.c
+++ b/src/lib/ring_decode.c
@@ -664,7 +664,7 @@ static void print_decode_pm4_pkt3(struct umr_asic *asic, 
struct umr_ring_decoder
if 
(!decoder->pm4.next_write_mem.addr_lo)

decoder->pm4.next_write_mem.addr_hi++;
} else {
-   printf("DATA: %s%08lx%s", 
YELLOW, (unsigned long)ib, RST);
+   printf("DATA: %s0x%08lx%s", 
YELLOW, (unsigned long)ib, RST);
}
}
break;
-- 
2.20.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: update psp firmwares loading sequence V2

[Evan Quan]

For those ASICs with DF Cstate management centralized to PMFW,
TMR setup should be performed between pmfw loading and other
non-psp firmwares loading.

V2: skip possible SMU firmware reloading

Change-Id: I8986ddb4d9ffe63ed0823d1dce8d9d52812a1240
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 65 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 +
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 51839ab02b84..d33f74100094 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -38,6 +38,39 @@
 
 static void psp_set_funcs(struct amdgpu_device *adev);
 
+/*
+ * Due to DF Cstate management centralized to PMFW, the firmware
+ * loading sequence will be updated as below:
+ *   - Load KDB
+ *   - Load SYS_DRV
+ *   - Load tOS
+ *   - Load PMFW
+ *   - Setup TMR
+ *   - Load other non-psp fw
+ *   - Load ASD
+ *   - Load XGMI/RAS/HDCP/DTM TA if any
+ *
+ * This new sequence is required for
+ *   - Arcturus
+ *   - Navi12 and onwards
+ */
+static void psp_check_pmfw_centralized_cstate_management(struct psp_context 
*psp)
+{
+   struct amdgpu_device *adev = psp->adev;
+
+   psp->pmfw_centralized_cstate_management = false;
+
+   if (amdgpu_sriov_vf(adev))
+   return;
+
+   if (adev->flags & AMD_IS_APU)
+   return;
+
+   if ((adev->asic_type == CHIP_ARCTURUS) ||
+   (adev->asic_type >= CHIP_NAVI12))
+   psp->pmfw_centralized_cstate_management = true;
+}
+
 static int psp_early_init(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -75,6 +108,8 @@ static int psp_early_init(void *handle)
 
psp->adev = adev;
 
+   psp_check_pmfw_centralized_cstate_management(psp);
+
return 0;
 }
 
@@ -1116,10 +1151,17 @@ static int psp_hw_start(struct psp_context *psp)
return ret;
}
 
-   ret = psp_tmr_load(psp);
-   if (ret) {
-   DRM_ERROR("PSP load tmr failed!\n");
-   return ret;
+   /*
+* For those ASICs with DF Cstate management centralized
+* to PMFW, TMR setup should be performed after PMFW
+* loaded and before other non-psp firmware loaded.
+*/
+   if (!psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
}
 
return 0;
@@ -1316,7 +1358,8 @@ static int psp_np_fw_load(struct psp_context *psp)
struct amdgpu_firmware_info *ucode;
struct amdgpu_device* adev = psp->adev;
 
-   if (psp->autoload_supported) {
+   if (psp->autoload_supported ||
+   psp->pmfw_centralized_cstate_management) {
ucode = >firmware.ucode[AMDGPU_UCODE_ID_SMC];
if (!ucode->fw || amdgpu_sriov_vf(adev))
goto out;
@@ -1326,6 +1369,14 @@ static int psp_np_fw_load(struct psp_context *psp)
return ret;
}
 
+   if (psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
+   }
+
 out:
for (i = 0; i < adev->firmware.max_ucodes; i++) {
ucode = >firmware.ucode[i];
@@ -1333,7 +1384,9 @@ static int psp_np_fw_load(struct psp_context *psp)
continue;
 
if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
-   (psp_smu_reload_quirk(psp) || psp->autoload_supported))
+   (psp_smu_reload_quirk(psp) ||
+psp->autoload_supported ||
+psp->pmfw_centralized_cstate_management))
continue;
 
if (amdgpu_sriov_vf(adev) &&
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index c77e1abb538a..37fa184f27f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -264,6 +264,8 @@ struct psp_context
atomic_tfence_value;
/* flag to mark whether gfx fw autoload is supported or not */
boolautoload_supported;
+   /* flag to mark whether df cstate management centralized to PMFW */
+   boolpmfw_centralized_cstate_management;
 
/* xgmi ta firmware and buffer */
const struct firmware   *ta_fw;
-- 
2.25.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu/display: clean up some indenting

[Dan Carpenter]

These lines were accidentally indented 4 spaces more than they should
be.

Signed-off-by: Dan Carpenter 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 4cb3eb7c6745..408405d9f30c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2138,10 +2138,10 @@ static void handle_hpd_rx_irq(void *param)
}
}
 #ifdef CONFIG_DRM_AMD_DC_HDCP
-   if (hpd_irq_data.bytes.device_service_irq.bits.CP_IRQ) {
-   if (adev->dm.hdcp_workqueue)
-   hdcp_handle_cpirq(adev->dm.hdcp_workqueue,  
aconnector->base.index);
-   }
+   if (hpd_irq_data.bytes.device_service_irq.bits.CP_IRQ) {
+   if (adev->dm.hdcp_workqueue)
+   hdcp_handle_cpirq(adev->dm.hdcp_workqueue,  
aconnector->base.index);
+   }
 #endif
if ((dc_link->cur_link_settings.lane_count != LANE_COUNT_UNKNOWN) ||
(dc_link->type == dc_connection_mst_branch))
-- 
2.11.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence

[Quan, Evan]

Thanks. Will update that in V2.
Only Arcturus was affected since Navi1x was already protected by 
autoload_supported.

As I know, psp will skip smu fw loading if SMU is already alive. So, there 
should be no reloading.
But there should be a skip in driver.

Regards,
Evan
-Original Message-
From: Zhang, Hawking  
Sent: Monday, February 24, 2020 6:19 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Feng, Kenneth 
Subject: RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence

[AMD Official Use Only - Internal Distribution Only]

Sorry just for Navi12. Arcturus should be fine as it doesn't support autoload.

Regards,
Hawking

-Original Message-
From: Zhang, Hawking 
Sent: Monday, February 24, 2020 18:18
To: Quan, Evan ; 'amd-gfx@lists.freedesktop.org' 

Cc: Feng, Kenneth 
Subject: RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence

[AMD Official Use Only - Internal Distribution Only]

That's saying I suspect the PMFW was loaded twice with the patch for Arcturus 
and Navi12.

Regards,
Hawking

-Original Message-
From: Zhang, Hawking 
Sent: Monday, February 24, 2020 18:16
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Feng, Kenneth 
Subject: RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence

[AMD Official Use Only - Internal Distribution Only]

Can you double check the following logic after your setup the TMR in 
psp_np_fw_load? For Arcturus and Navi12, it should be skipped as well.

if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
(psp_smu_reload_quirk(psp) || psp->autoload_supported))
continue;

Regards,
Hawking
-Original Message-
From: Quan, Evan  
Sent: Monday, February 24, 2020 17:24
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Feng, Kenneth 
; Quan, Evan 
Subject: [PATCH] drm/amdgpu: update psp firmwares loading sequence

For those ASICs with DF Cstate management centralized to PMFW, TMR setup should 
be performed between pmfw loading and other non-psp firmwares loading.

Change-Id: I8986ddb4d9ffe63ed0823d1dce8d9d52812a1240
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 61 +++--  
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 +
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 51839ab02b84..ef800ad68e1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -38,6 +38,39 @@
 
 static void psp_set_funcs(struct amdgpu_device *adev);
 
+/*
+ * Due to DF Cstate management centralized to PMFW, the firmware
+ * loading sequence will be updated as below:
+ *   - Load KDB
+ *   - Load SYS_DRV
+ *   - Load tOS
+ *   - Load PMFW
+ *   - Setup TMR
+ *   - Load other non-psp fw
+ *   - Load ASD
+ *   - Load XGMI/RAS/HDCP/DTM TA if any
+ *
+ * This new sequence is required for
+ *   - Arcturus
+ *   - Navi12 and onwards
+ */
+static void psp_check_pmfw_centralized_cstate_management(struct 
+psp_context *psp) {
+   struct amdgpu_device *adev = psp->adev;
+
+   psp->pmfw_centralized_cstate_management = false;
+
+   if (amdgpu_sriov_vf(adev))
+   return;
+
+   if (adev->flags & AMD_IS_APU)
+   return;
+
+   if ((adev->asic_type == CHIP_ARCTURUS) ||
+   (adev->asic_type >= CHIP_NAVI12))
+   psp->pmfw_centralized_cstate_management = true; }
+
 static int psp_early_init(void *handle)  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle; @@ -75,6 
+108,8 @@ static int psp_early_init(void *handle)
 
psp->adev = adev;
 
+   psp_check_pmfw_centralized_cstate_management(psp);
+
return 0;
 }
 
@@ -1116,10 +1151,17 @@ static int psp_hw_start(struct psp_context *psp)
return ret;
}
 
-   ret = psp_tmr_load(psp);
-   if (ret) {
-   DRM_ERROR("PSP load tmr failed!\n");
-   return ret;
+   /*
+* For those ASICs with DF Cstate management centralized
+* to PMFW, TMR setup should be performed after PMFW
+* loaded and before other non-psp firmware loaded.
+*/
+   if (!psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
}
 
return 0;
@@ -1316,7 +1358,8 @@ static int psp_np_fw_load(struct psp_context *psp)
struct amdgpu_firmware_info *ucode;
struct amdgpu_device* adev = psp->adev;
 
-   if (psp->autoload_supported) {
+   if (psp->autoload_supported ||
+   psp->pmfw_centralized_cstate_management) {
ucode = >firmware.ucode[AMDGPU_UCODE_ID_SMC];
if (!ucode->fw || amdgpu_sriov_vf(adev))
goto out;
@@ -1326,6 +1369,14 @@ static int psp_np_fw_load(struct psp_context *psp)
 

Recall: [PATCH] drm/amdgpu: update psp firmwares loading sequence

[Zhang, Hawking]

Zhang, Hawking would like to recall the message, "[PATCH] drm/amdgpu: update 
psp firmwares loading sequence".
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence

[Zhang, Hawking]

[AMD Official Use Only - Internal Distribution Only]

Sorry just for Navi12. Arcturus should be fine as it doesn't support autoload.

Regards,
Hawking

-Original Message-
From: Zhang, Hawking 
Sent: Monday, February 24, 2020 18:18
To: Quan, Evan ; 'amd-gfx@lists.freedesktop.org' 

Cc: Feng, Kenneth 
Subject: RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence

[AMD Official Use Only - Internal Distribution Only]

That's saying I suspect the PMFW was loaded twice with the patch for Arcturus 
and Navi12.

Regards,
Hawking

-Original Message-
From: Zhang, Hawking 
Sent: Monday, February 24, 2020 18:16
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Feng, Kenneth 
Subject: RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence

[AMD Official Use Only - Internal Distribution Only]

Can you double check the following logic after your setup the TMR in 
psp_np_fw_load? For Arcturus and Navi12, it should be skipped as well.

if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
(psp_smu_reload_quirk(psp) || psp->autoload_supported))
continue;

Regards,
Hawking
-Original Message-
From: Quan, Evan  
Sent: Monday, February 24, 2020 17:24
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Feng, Kenneth 
; Quan, Evan 
Subject: [PATCH] drm/amdgpu: update psp firmwares loading sequence

For those ASICs with DF Cstate management centralized to PMFW, TMR setup should 
be performed between pmfw loading and other non-psp firmwares loading.

Change-Id: I8986ddb4d9ffe63ed0823d1dce8d9d52812a1240
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 61 +++--  
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 +
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 51839ab02b84..ef800ad68e1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -38,6 +38,39 @@
 
 static void psp_set_funcs(struct amdgpu_device *adev);
 
+/*
+ * Due to DF Cstate management centralized to PMFW, the firmware
+ * loading sequence will be updated as below:
+ *   - Load KDB
+ *   - Load SYS_DRV
+ *   - Load tOS
+ *   - Load PMFW
+ *   - Setup TMR
+ *   - Load other non-psp fw
+ *   - Load ASD
+ *   - Load XGMI/RAS/HDCP/DTM TA if any
+ *
+ * This new sequence is required for
+ *   - Arcturus
+ *   - Navi12 and onwards
+ */
+static void psp_check_pmfw_centralized_cstate_management(struct 
+psp_context *psp) {
+   struct amdgpu_device *adev = psp->adev;
+
+   psp->pmfw_centralized_cstate_management = false;
+
+   if (amdgpu_sriov_vf(adev))
+   return;
+
+   if (adev->flags & AMD_IS_APU)
+   return;
+
+   if ((adev->asic_type == CHIP_ARCTURUS) ||
+   (adev->asic_type >= CHIP_NAVI12))
+   psp->pmfw_centralized_cstate_management = true; }
+
 static int psp_early_init(void *handle)  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle; @@ -75,6 
+108,8 @@ static int psp_early_init(void *handle)
 
psp->adev = adev;
 
+   psp_check_pmfw_centralized_cstate_management(psp);
+
return 0;
 }
 
@@ -1116,10 +1151,17 @@ static int psp_hw_start(struct psp_context *psp)
return ret;
}
 
-   ret = psp_tmr_load(psp);
-   if (ret) {
-   DRM_ERROR("PSP load tmr failed!\n");
-   return ret;
+   /*
+* For those ASICs with DF Cstate management centralized
+* to PMFW, TMR setup should be performed after PMFW
+* loaded and before other non-psp firmware loaded.
+*/
+   if (!psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
}
 
return 0;
@@ -1316,7 +1358,8 @@ static int psp_np_fw_load(struct psp_context *psp)
struct amdgpu_firmware_info *ucode;
struct amdgpu_device* adev = psp->adev;
 
-   if (psp->autoload_supported) {
+   if (psp->autoload_supported ||
+   psp->pmfw_centralized_cstate_management) {
ucode = >firmware.ucode[AMDGPU_UCODE_ID_SMC];
if (!ucode->fw || amdgpu_sriov_vf(adev))
goto out;
@@ -1326,6 +1369,14 @@ static int psp_np_fw_load(struct psp_context *psp)
return ret;
}
 
+   if (psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
+   }
+
 out:
for (i = 0; i < adev->firmware.max_ucodes; i++) {
ucode = >firmware.ucode[i];
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 

RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence

[Zhang, Hawking]

[AMD Official Use Only - Internal Distribution Only]

That's saying I suspect the PMFW was loaded twice with the patch for Arcturus 
and Navi12.

Regards,
Hawking

-Original Message-
From: Zhang, Hawking 
Sent: Monday, February 24, 2020 18:16
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Feng, Kenneth 
Subject: RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence

[AMD Official Use Only - Internal Distribution Only]

Can you double check the following logic after your setup the TMR in 
psp_np_fw_load? For Arcturus and Navi12, it should be skipped as well.

if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
(psp_smu_reload_quirk(psp) || psp->autoload_supported))
continue;

Regards,
Hawking
-Original Message-
From: Quan, Evan  
Sent: Monday, February 24, 2020 17:24
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Feng, Kenneth 
; Quan, Evan 
Subject: [PATCH] drm/amdgpu: update psp firmwares loading sequence

For those ASICs with DF Cstate management centralized to PMFW, TMR setup should 
be performed between pmfw loading and other non-psp firmwares loading.

Change-Id: I8986ddb4d9ffe63ed0823d1dce8d9d52812a1240
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 61 +++--  
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 +
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 51839ab02b84..ef800ad68e1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -38,6 +38,39 @@
 
 static void psp_set_funcs(struct amdgpu_device *adev);
 
+/*
+ * Due to DF Cstate management centralized to PMFW, the firmware
+ * loading sequence will be updated as below:
+ *   - Load KDB
+ *   - Load SYS_DRV
+ *   - Load tOS
+ *   - Load PMFW
+ *   - Setup TMR
+ *   - Load other non-psp fw
+ *   - Load ASD
+ *   - Load XGMI/RAS/HDCP/DTM TA if any
+ *
+ * This new sequence is required for
+ *   - Arcturus
+ *   - Navi12 and onwards
+ */
+static void psp_check_pmfw_centralized_cstate_management(struct 
+psp_context *psp) {
+   struct amdgpu_device *adev = psp->adev;
+
+   psp->pmfw_centralized_cstate_management = false;
+
+   if (amdgpu_sriov_vf(adev))
+   return;
+
+   if (adev->flags & AMD_IS_APU)
+   return;
+
+   if ((adev->asic_type == CHIP_ARCTURUS) ||
+   (adev->asic_type >= CHIP_NAVI12))
+   psp->pmfw_centralized_cstate_management = true; }
+
 static int psp_early_init(void *handle)  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle; @@ -75,6 
+108,8 @@ static int psp_early_init(void *handle)
 
psp->adev = adev;
 
+   psp_check_pmfw_centralized_cstate_management(psp);
+
return 0;
 }
 
@@ -1116,10 +1151,17 @@ static int psp_hw_start(struct psp_context *psp)
return ret;
}
 
-   ret = psp_tmr_load(psp);
-   if (ret) {
-   DRM_ERROR("PSP load tmr failed!\n");
-   return ret;
+   /*
+* For those ASICs with DF Cstate management centralized
+* to PMFW, TMR setup should be performed after PMFW
+* loaded and before other non-psp firmware loaded.
+*/
+   if (!psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
}
 
return 0;
@@ -1316,7 +1358,8 @@ static int psp_np_fw_load(struct psp_context *psp)
struct amdgpu_firmware_info *ucode;
struct amdgpu_device* adev = psp->adev;
 
-   if (psp->autoload_supported) {
+   if (psp->autoload_supported ||
+   psp->pmfw_centralized_cstate_management) {
ucode = >firmware.ucode[AMDGPU_UCODE_ID_SMC];
if (!ucode->fw || amdgpu_sriov_vf(adev))
goto out;
@@ -1326,6 +1369,14 @@ static int psp_np_fw_load(struct psp_context *psp)
return ret;
}
 
+   if (psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
+   }
+
 out:
for (i = 0; i < adev->firmware.max_ucodes; i++) {
ucode = >firmware.ucode[i];
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index c77e1abb538a..37fa184f27f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -264,6 +264,8 @@ struct psp_context
atomic_tfence_value;
/* flag to mark whether gfx fw autoload is supported or not */
boolautoload_supported;
+   /* flag to mark whether df 

RE: [PATCH 3/3] drm/amdgpu: toggle DF-Cstate to protect DF reg access

[Quan, Evan]

Patch1 is reviewed-by: Evan Quan 
Patch2 & 3 are acked-by: Evan Quan 

-Original Message-
From: Hawking Zhang  
Sent: Monday, February 24, 2020 6:06 PM
To: amd-gfx@lists.freedesktop.org; Clements, John ; 
Deucher, Alexander ; Feng, Kenneth 
; Quan, Evan 
Cc: Zhang, Hawking 
Subject: [PATCH 3/3] drm/amdgpu: toggle DF-Cstate to protect DF reg access

driver needs to take DF out Cstate before any DF register
access. otherwise, the DF register may not be accessible.

Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 25 +++-
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 8edd1db0d1ce..856dd22465d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -535,13 +535,28 @@ uint64_t amdgpu_xgmi_get_relative_phy_addr(struct 
amdgpu_device *adev,
   uint64_t addr)
 {
uint32_t df_inst_id;
+   uint64_t dram_base_addr = 0;
+   const struct amdgpu_df_funcs *df_funcs = adev->df.funcs;
+
+   if ((!df_funcs) ||
+   (!df_funcs->get_df_inst_id) ||
+   (!df_funcs->get_dram_base_addr)) {
+   dev_warn(adev->dev,
+"XGMI: relative phy_addr algorithm is not 
supported\n");
+   return addr;
+   }
 
-   if ((!adev->df.funcs) ||
-   (!adev->df.funcs->get_df_inst_id) ||
-   (!adev->df.funcs->get_dram_base_addr))
+   if (amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_DISALLOW)) {
+   dev_warn(adev->dev,
+"failed to disable DF-Cstate, DF register may not be 
accessible\n");
return addr;
+   }
+
+   df_inst_id = df_funcs->get_df_inst_id(adev);
+   dram_base_addr = df_funcs->get_dram_base_addr(adev, df_inst_id);
 
-   df_inst_id = adev->df.funcs->get_df_inst_id(adev);
+   if (amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_ALLOW))
+   dev_warn(adev->dev, "failed to enable DF-Cstate\n");
 
-   return addr + adev->df.funcs->get_dram_base_addr(adev, df_inst_id);
+   return addr + dram_base_addr;
 }
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: update psp firmwares loading sequence

[Zhang, Hawking]

[AMD Official Use Only - Internal Distribution Only]

Can you double check the following logic after your setup the TMR in 
psp_np_fw_load? For Arcturus and Navi12, it should be skipped as well.

if (ucode->ucode_id == AMDGPU_UCODE_ID_SMC &&
(psp_smu_reload_quirk(psp) || psp->autoload_supported))
continue;

Regards,
Hawking
-Original Message-
From: Quan, Evan  
Sent: Monday, February 24, 2020 17:24
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Feng, Kenneth 
; Quan, Evan 
Subject: [PATCH] drm/amdgpu: update psp firmwares loading sequence

For those ASICs with DF Cstate management centralized to PMFW, TMR setup should 
be performed between pmfw loading and other non-psp firmwares loading.

Change-Id: I8986ddb4d9ffe63ed0823d1dce8d9d52812a1240
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 61 +++--  
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 +
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 51839ab02b84..ef800ad68e1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -38,6 +38,39 @@
 
 static void psp_set_funcs(struct amdgpu_device *adev);
 
+/*
+ * Due to DF Cstate management centralized to PMFW, the firmware
+ * loading sequence will be updated as below:
+ *   - Load KDB
+ *   - Load SYS_DRV
+ *   - Load tOS
+ *   - Load PMFW
+ *   - Setup TMR
+ *   - Load other non-psp fw
+ *   - Load ASD
+ *   - Load XGMI/RAS/HDCP/DTM TA if any
+ *
+ * This new sequence is required for
+ *   - Arcturus
+ *   - Navi12 and onwards
+ */
+static void psp_check_pmfw_centralized_cstate_management(struct 
+psp_context *psp) {
+   struct amdgpu_device *adev = psp->adev;
+
+   psp->pmfw_centralized_cstate_management = false;
+
+   if (amdgpu_sriov_vf(adev))
+   return;
+
+   if (adev->flags & AMD_IS_APU)
+   return;
+
+   if ((adev->asic_type == CHIP_ARCTURUS) ||
+   (adev->asic_type >= CHIP_NAVI12))
+   psp->pmfw_centralized_cstate_management = true; }
+
 static int psp_early_init(void *handle)  {
struct amdgpu_device *adev = (struct amdgpu_device *)handle; @@ -75,6 
+108,8 @@ static int psp_early_init(void *handle)
 
psp->adev = adev;
 
+   psp_check_pmfw_centralized_cstate_management(psp);
+
return 0;
 }
 
@@ -1116,10 +1151,17 @@ static int psp_hw_start(struct psp_context *psp)
return ret;
}
 
-   ret = psp_tmr_load(psp);
-   if (ret) {
-   DRM_ERROR("PSP load tmr failed!\n");
-   return ret;
+   /*
+* For those ASICs with DF Cstate management centralized
+* to PMFW, TMR setup should be performed after PMFW
+* loaded and before other non-psp firmware loaded.
+*/
+   if (!psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
}
 
return 0;
@@ -1316,7 +1358,8 @@ static int psp_np_fw_load(struct psp_context *psp)
struct amdgpu_firmware_info *ucode;
struct amdgpu_device* adev = psp->adev;
 
-   if (psp->autoload_supported) {
+   if (psp->autoload_supported ||
+   psp->pmfw_centralized_cstate_management) {
ucode = >firmware.ucode[AMDGPU_UCODE_ID_SMC];
if (!ucode->fw || amdgpu_sriov_vf(adev))
goto out;
@@ -1326,6 +1369,14 @@ static int psp_np_fw_load(struct psp_context *psp)
return ret;
}
 
+   if (psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
+   }
+
 out:
for (i = 0; i < adev->firmware.max_ucodes; i++) {
ucode = >firmware.ucode[i];
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index c77e1abb538a..37fa184f27f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -264,6 +264,8 @@ struct psp_context
atomic_tfence_value;
/* flag to mark whether gfx fw autoload is supported or not */
boolautoload_supported;
+   /* flag to mark whether df cstate management centralized to PMFW */
+   boolpmfw_centralized_cstate_management;
 
/* xgmi ta firmware and buffer */
const struct firmware   *ta_fw;
--
2.25.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/3] drm/amdgpu: move get_xgmi_relative_phy_addr to amdgpu_xgmi.c

[Hawking Zhang]

centralize all the xgmi related function to amdgpu_xgmi.c

Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c  | 20 
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 15 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h |  2 ++
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 87f57878dbd5..c1d82f545c8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -31,6 +31,7 @@
 #include "amdgpu.h"
 #include "amdgpu_ras.h"
 #include "amdgpu_atomfirmware.h"
+#include "amdgpu_xgmi.h"
 #include "ivsrcid/nbio/irqsrcs_nbif_7_4.h"
 
 const char *ras_error_string[] = {
@@ -742,20 +743,6 @@ int amdgpu_ras_error_query(struct amdgpu_device *adev,
return 0;
 }
 
-uint64_t get_xgmi_relative_phy_addr(struct amdgpu_device *adev, uint64_t addr)
-{
-   uint32_t df_inst_id;
-
-   if ((!adev->df.funcs) ||
-   (!adev->df.funcs->get_df_inst_id) ||
-   (!adev->df.funcs->get_dram_base_addr))
-   return addr;
-
-   df_inst_id = adev->df.funcs->get_df_inst_id(adev);
-
-   return addr + adev->df.funcs->get_dram_base_addr(adev, df_inst_id);
-}
-
 /* wrapper of psp_ras_trigger_error */
 int amdgpu_ras_error_inject(struct amdgpu_device *adev,
struct ras_inject_if *info)
@@ -775,8 +762,9 @@ int amdgpu_ras_error_inject(struct amdgpu_device *adev,
 
/* Calculate XGMI relative offset */
if (adev->gmc.xgmi.num_physical_nodes > 1) {
-   block_info.address = get_xgmi_relative_phy_addr(adev,
-   
block_info.address);
+   block_info.address =
+   amdgpu_xgmi_get_relative_phy_addr(adev,
+ block_info.address);
}
 
switch (info->head.block) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index fbc41926de08..8edd1db0d1ce 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -530,3 +530,18 @@ void amdgpu_xgmi_ras_fini(struct amdgpu_device *adev)
kfree(ras_if);
}
 }
+
+uint64_t amdgpu_xgmi_get_relative_phy_addr(struct amdgpu_device *adev,
+  uint64_t addr)
+{
+   uint32_t df_inst_id;
+
+   if ((!adev->df.funcs) ||
+   (!adev->df.funcs->get_df_inst_id) ||
+   (!adev->df.funcs->get_dram_base_addr))
+   return addr;
+
+   df_inst_id = adev->df.funcs->get_df_inst_id(adev);
+
+   return addr + adev->df.funcs->get_dram_base_addr(adev, df_inst_id);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
index c62a4acf4c14..2aa61adee459 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
@@ -46,6 +46,8 @@ int amdgpu_xgmi_get_hops_count(struct amdgpu_device *adev,
struct amdgpu_device *peer_adev);
 int amdgpu_xgmi_ras_late_init(struct amdgpu_device *adev);
 void amdgpu_xgmi_ras_fini(struct amdgpu_device *adev);
+uint64_t amdgpu_xgmi_get_relative_phy_addr(struct amdgpu_device *adev,
+  uint64_t addr);
 
 static inline bool amdgpu_xgmi_same_hive(struct amdgpu_device *adev,
struct amdgpu_device *bo_adev)
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/3] drm/amdgpu: toggle DF-Cstate to protect DF reg access

[Hawking Zhang]

driver needs to take DF out Cstate before any DF register
access. otherwise, the DF register may not be accessible.

Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 25 +++-
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 8edd1db0d1ce..856dd22465d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -535,13 +535,28 @@ uint64_t amdgpu_xgmi_get_relative_phy_addr(struct 
amdgpu_device *adev,
   uint64_t addr)
 {
uint32_t df_inst_id;
+   uint64_t dram_base_addr = 0;
+   const struct amdgpu_df_funcs *df_funcs = adev->df.funcs;
+
+   if ((!df_funcs) ||
+   (!df_funcs->get_df_inst_id) ||
+   (!df_funcs->get_dram_base_addr)) {
+   dev_warn(adev->dev,
+"XGMI: relative phy_addr algorithm is not 
supported\n");
+   return addr;
+   }
 
-   if ((!adev->df.funcs) ||
-   (!adev->df.funcs->get_df_inst_id) ||
-   (!adev->df.funcs->get_dram_base_addr))
+   if (amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_DISALLOW)) {
+   dev_warn(adev->dev,
+"failed to disable DF-Cstate, DF register may not be 
accessible\n");
return addr;
+   }
+
+   df_inst_id = df_funcs->get_df_inst_id(adev);
+   dram_base_addr = df_funcs->get_dram_base_addr(adev, df_inst_id);
 
-   df_inst_id = adev->df.funcs->get_df_inst_id(adev);
+   if (amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_ALLOW))
+   dev_warn(adev->dev, "failed to enable DF-Cstate\n");
 
-   return addr + adev->df.funcs->get_dram_base_addr(adev, df_inst_id);
+   return addr + dram_base_addr;
 }
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/3] drm/amdgpu: add dpm helper function for DF Cstate control

[Hawking Zhang]

The helper function hides software smu and legacy powerplay
implementation for DF Cstate control.

Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c | 17 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c
index a2e8c3dfb4f1..ba1bb95a3cf9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c
@@ -1171,3 +1171,20 @@ int amdgpu_dpm_set_xgmi_pstate(struct amdgpu_device 
*adev,
 
return ret;
 }
+
+int amdgpu_dpm_set_df_cstate(struct amdgpu_device *adev,
+uint32_t cstate)
+{
+   int ret = 0;
+   const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
+   void *pp_handle = adev->powerplay.pp_handle;
+   struct smu_context *smu = >smu;
+
+   if (is_support_sw_smu(adev))
+   ret = smu_set_df_cstate(smu, cstate);
+   else if (pp_funcs &&
+pp_funcs->set_df_cstate)
+   ret = pp_funcs->set_df_cstate(pp_handle, cstate);
+
+   return ret;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
index 902ca6c00cca..168579492a55 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
@@ -533,4 +533,7 @@ int amdgpu_dpm_baco_exit(struct amdgpu_device *adev);
 
 int amdgpu_dpm_baco_enter(struct amdgpu_device *adev);
 
+int amdgpu_dpm_set_df_cstate(struct amdgpu_device *adev,
+uint32_t cstate);
+
 #endif
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: update psp firmwares loading sequence

[Evan Quan]

For those ASICs with DF Cstate management centralized to PMFW,
TMR setup should be performed between pmfw loading and other
non-psp firmwares loading.

Change-Id: I8986ddb4d9ffe63ed0823d1dce8d9d52812a1240
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 61 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 +
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 51839ab02b84..ef800ad68e1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -38,6 +38,39 @@
 
 static void psp_set_funcs(struct amdgpu_device *adev);
 
+/*
+ * Due to DF Cstate management centralized to PMFW, the firmware
+ * loading sequence will be updated as below:
+ *   - Load KDB
+ *   - Load SYS_DRV
+ *   - Load tOS
+ *   - Load PMFW
+ *   - Setup TMR
+ *   - Load other non-psp fw
+ *   - Load ASD
+ *   - Load XGMI/RAS/HDCP/DTM TA if any
+ *
+ * This new sequence is required for
+ *   - Arcturus
+ *   - Navi12 and onwards
+ */
+static void psp_check_pmfw_centralized_cstate_management(struct psp_context 
*psp)
+{
+   struct amdgpu_device *adev = psp->adev;
+
+   psp->pmfw_centralized_cstate_management = false;
+
+   if (amdgpu_sriov_vf(adev))
+   return;
+
+   if (adev->flags & AMD_IS_APU)
+   return;
+
+   if ((adev->asic_type == CHIP_ARCTURUS) ||
+   (adev->asic_type >= CHIP_NAVI12))
+   psp->pmfw_centralized_cstate_management = true;
+}
+
 static int psp_early_init(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -75,6 +108,8 @@ static int psp_early_init(void *handle)
 
psp->adev = adev;
 
+   psp_check_pmfw_centralized_cstate_management(psp);
+
return 0;
 }
 
@@ -1116,10 +1151,17 @@ static int psp_hw_start(struct psp_context *psp)
return ret;
}
 
-   ret = psp_tmr_load(psp);
-   if (ret) {
-   DRM_ERROR("PSP load tmr failed!\n");
-   return ret;
+   /*
+* For those ASICs with DF Cstate management centralized
+* to PMFW, TMR setup should be performed after PMFW
+* loaded and before other non-psp firmware loaded.
+*/
+   if (!psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
}
 
return 0;
@@ -1316,7 +1358,8 @@ static int psp_np_fw_load(struct psp_context *psp)
struct amdgpu_firmware_info *ucode;
struct amdgpu_device* adev = psp->adev;
 
-   if (psp->autoload_supported) {
+   if (psp->autoload_supported ||
+   psp->pmfw_centralized_cstate_management) {
ucode = >firmware.ucode[AMDGPU_UCODE_ID_SMC];
if (!ucode->fw || amdgpu_sriov_vf(adev))
goto out;
@@ -1326,6 +1369,14 @@ static int psp_np_fw_load(struct psp_context *psp)
return ret;
}
 
+   if (psp->pmfw_centralized_cstate_management) {
+   ret = psp_tmr_load(psp);
+   if (ret) {
+   DRM_ERROR("PSP load tmr failed!\n");
+   return ret;
+   }
+   }
+
 out:
for (i = 0; i < adev->firmware.max_ucodes; i++) {
ucode = >firmware.ucode[i];
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index c77e1abb538a..37fa184f27f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -264,6 +264,8 @@ struct psp_context
atomic_tfence_value;
/* flag to mark whether gfx fw autoload is supported or not */
boolautoload_supported;
+   /* flag to mark whether df cstate management centralized to PMFW */
+   boolpmfw_centralized_cstate_management;
 
/* xgmi ta firmware and buffer */
const struct firmware   *ta_fw;
-- 
2.25.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: Add a chunk ID for spm trace

[He, Jacob]

Ok, I’m glad that there is no uapi change needed.
I checked the git log, and reserve_vmid was added for shader debugger, not gpa. 
I’m fine to re-use it since the spm will not enabled for shader debug in 
general. I’ll try to change my patch.
BTW, “ring->funcs->setup_spm(ring, NULL)” to clear the vmid is not gonna work 
since the job with spm enabled execution is not done yet. Leave the old 
spm_vmid there is not a problem because other new spm enabled job will update 
it before it’s running and other spm disabled job will not access spm_vmid 
register.

Thanks
Jacob

From: Zhou, David(ChunMing)
Sent: Saturday, February 22, 2020 12:45 AM
To: Koenig, Christian; Deucher, 
Alexander; Christian 
König; He, 
Jacob; 
amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amdgpu: Add a chunk ID for spm trace


[AMD Official Use Only - Internal Distribution Only]

That’s fine to me.

-David

From: Koenig, Christian 
Sent: Friday, February 21, 2020 11:33 PM
To: Deucher, Alexander ; Christian König 
; Zhou, David(ChunMing) 
; He, Jacob ; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Add a chunk ID for spm trace

I would just do this as part of the vm_flush() callback on the ring.

E.g. check if the VMID you want to flush is reserved and if yes enable SPM.

Maybe pass along a flag or something in the job to make things easier.

Christian.

Am 21.02.20 um 16:31 schrieb Deucher, Alexander:

[AMD Public Use]

We already have the RESERVE_VMID ioctl interface, can't we just use that 
internally in the kernel to update the rlc register via the ring when we 
schedule the relevant IB?  E.g., add a new ring callback to set SPM state and 
then set it to the reserved vmid before we schedule the ib, and then reset it 
to 0 after the IB in amdgpu_ib_schedule().

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 4b2342d11520..e0db9362c6ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -185,6 +185,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
num_ibs,
if (ring->funcs->insert_start)
ring->funcs->insert_start(ring);

+   if (ring->funcs->setup_spm)
+   ring->funcs->setup_spm(ring, job);
+
if (job) {
r = amdgpu_vm_flush(ring, job, need_pipe_sync);
if (r) {
@@ -273,6 +276,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
num_ibs,
return r;
}

+   if (ring->funcs->setup_spm)
+   ring->funcs->setup_spm(ring, NULL);
+
if (ring->funcs->insert_end)
ring->funcs->insert_end(ring);



Alex

From: amd-gfx 

 on behalf of Christian König 

Sent: Friday, February 21, 2020 5:28 AM
To: Zhou, David(ChunMing) ; 
He, Jacob ; Koenig, Christian 
; 
amd-gfx@lists.freedesktop.org 

Subject: Re: [PATCH] drm/amdgpu: Add a chunk ID for spm trace

That would probably be a no-go, but we could enhance the kernel driver to 
update the RLC_SPM_VMID register with the reserved VMID.

Handling that in userspace is most likely not working anyway, since the RLC 
registers are usually not accessible by userspace.

Regards,
Christian.

Am 20.02.20 um 16:15 schrieb Zhou, David(ChunMing):

[AMD Official Use Only - Internal Distribution Only]



You can enhance amdgpu_vm_ioctl In amdgpu_vm.c to return vmid to userspace.



-David





From: He, Jacob 
Sent: Thursday, February 20, 2020 10:46 PM
To: Zhou, David(ChunMing) ; 
Koenig, Christian ; 
amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amdgpu: Add a chunk ID for spm trace



amdgpu_vm_reserve_vmid doesn’t return the reserved vmid back to user space. 
There is no chance for user mode driver to update RLC_SPM_VMID.



Thanks

Jacob



From: He, Jacob
Sent: Thursday, February 20, 2020 6:20 PM
To: Zhou, David(ChunMing); Koenig, 
Christian; 
amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amdgpu: Add a chunk ID for spm trace



Looks like amdgpu_vm_reserve_vmid could work, let me have a try to update the 
RLC_SPM_VMID with pm4 packets in UMD.



Thanks

Jacob



From: Zhou, David(ChunMing)
Sent: Thursday, February 20, 2020 10:13 AM
To: Koenig, Christian; He, 

Re: [PATCH 8/8] drm/ttm: do not keep GPU dependent addresses

[Gerd Hoffmann]

  Hi,

> 2 unfortunately I can't say the same for bochs but it works with this patch
> series so I think bochs is fine as well.

bochs needs the offset only to scanout framebuffers, which in turn
requires framebuffers being pinned to vram.  So all green here.

cheers,
  Gerd

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset

[Gerd Hoffmann]

On Mon, Feb 17, 2020 at 04:04:25PM +0100, Nirmoy Das wrote:
> Calculate GPU offset within vram-helper without depending on
> bo->offset
> 
> Signed-off-by: Nirmoy Das 

Acked-by: Gerd Hoffmann 

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 7/8] drm/bochs: use drm_gem_vram_offset to get bo offset

[Gerd Hoffmann]

On Mon, Feb 17, 2020 at 04:04:26PM +0100, Nirmoy Das wrote:
> Switch over to GEM VRAM's implementation to retrieve bo->offset
> 
> Signed-off-by: Nirmoy Das 

Acked-by: Gerd Hoffmann 

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx