Re: [PATCH 1/2] drm amdgpu: Skip tmr load for SRIOV
Reviewed-by: Luben Tuikov On 2020-07-29 04:23, Liu ChengZhe wrote: > 1. For Navi12, CHIP_SIENNA_CICHLID, skip tmr load operation; > 2. Check pointer before release firmware. > > v2: use CHIP_SIENNA_CICHLID instead > v3: remove local "bool ret"; fix grammer issue > v4: use my name instead of "root" > v5: fix grammer issue and indent issue > > Signed-off-by: Liu ChengZhe > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 35 - > 1 file changed, 29 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index a053b7af0680..c68369731b20 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -193,12 +193,18 @@ static int psp_sw_fini(void *handle) > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > psp_memory_training_fini(&adev->psp); > - release_firmware(adev->psp.sos_fw); > - adev->psp.sos_fw = NULL; > - release_firmware(adev->psp.asd_fw); > - adev->psp.asd_fw = NULL; > - release_firmware(adev->psp.ta_fw); > - adev->psp.ta_fw = NULL; > + if (adev->psp.sos_fw) { > + release_firmware(adev->psp.sos_fw); > + adev->psp.sos_fw = NULL; > + } > + if (adev->psp.asd_fw) { > + release_firmware(adev->psp.asd_fw); > + adev->psp.asd_fw = NULL; > + } > + if (adev->psp.ta_fw) { > + release_firmware(adev->psp.ta_fw); > + adev->psp.ta_fw = NULL; > + } > > if (adev->asic_type == CHIP_NAVI10) > psp_sysfs_fini(adev); > @@ -409,11 +415,28 @@ static int psp_clear_vf_fw(struct psp_context *psp) > return ret; > } > > +static bool psp_skip_tmr(struct psp_context *psp) > +{ > + switch (psp->adev->asic_type) { > + case CHIP_NAVI12: > + case CHIP_SIENNA_CICHLID: > + return true; > + default: > + return false; > + } > +} > + > static int psp_tmr_load(struct psp_context *psp) > { > int ret; > struct psp_gfx_cmd_resp *cmd; > > + /* For Navi12 and CHIP_SIENNA_CICHLID SRIOV, do not set up TMR. > + * Already set up by host driver. > + */ > + if (amdgpu_sriov_vf(psp->adev) && psp_skip_tmr(psp)) > + return 0; > + > cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL); > if (!cmd) > return -ENOMEM; > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/2] drm amdgpu: Skip tmr load for SRIOV
On 2020-07-28 1:36 a.m., Liu ChengZhe wrote: > 1. For Navi12, CHIP_SIENNA_CICHLID, skip tmr load operation; > 2. Check pointer before release firmware. > > v2: use CHIP_SIENNA_CICHLID instead > v3: remove local "bool ret"; fix grammer issue > v4: use my name instead of "root" > Don't indent any lines. > Signed-off-by: Liu ChengZhe > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 35 - > 1 file changed, 29 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index a053b7af0680..7f18286a0cc2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -193,12 +193,18 @@ static int psp_sw_fini(void *handle) > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > psp_memory_training_fini(&adev->psp); > - release_firmware(adev->psp.sos_fw); > - adev->psp.sos_fw = NULL; > - release_firmware(adev->psp.asd_fw); > - adev->psp.asd_fw = NULL; > - release_firmware(adev->psp.ta_fw); > - adev->psp.ta_fw = NULL; > + if (adev->psp.sos_fw) { > + release_firmware(adev->psp.sos_fw); > + adev->psp.sos_fw = NULL; > + } > + if (adev->psp.asd_fw) { > + release_firmware(adev->psp.asd_fw); > + adev->psp.asd_fw = NULL; > + } > + if (adev->psp.ta_fw) { > + release_firmware(adev->psp.ta_fw); > + adev->psp.ta_fw = NULL; > + } > > if (adev->asic_type == CHIP_NAVI10) > psp_sysfs_fini(adev); > @@ -409,11 +415,28 @@ static int psp_clear_vf_fw(struct psp_context *psp) > return ret; > } > > +static bool psp_skip_tmr(struct psp_context *psp) > +{ > + switch (psp->adev->asic_type) { > + case CHIP_NAVI12: > + case CHIP_SIENNA_CICHLID: > + return true; > + default: > + return false; > + } > +} Yeah, that's very nice now. > + > static int psp_tmr_load(struct psp_context *psp) > { > int ret; > struct psp_gfx_cmd_resp *cmd; > Fix this: > + /* (F)for Navi12 and CHIP_SIENNA_CICHLID SRIOV, do not set up TMR(.) > + * (A)already set( )up by host driver(.) Thanks, Luben > + */ > + if (amdgpu_sriov_vf(psp->adev) && psp_skip_tmr(psp)) > + return 0; > + > cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL); > if (!cmd) > return -ENOMEM; > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/2] drm amdgpu: Skip tmr load for SRIOV
Thanks for this patch. On 2020-07-28 1:12 a.m., Liu ChengZhe wrote: > From: root You should fix your Git setup to show proper user name, not "root". I've prepared a Confluence page which shows a way to do it, and a few other things along the way: http://confluence.amd.com/display/~ltuikov/Git+Setup > > 1. For Navi12, CHIP_SIENNA_CICHLID, skip tmr load operation; > 2. Check pointer before release firmware. > > v2: use CHIP_SIENNA_CICHLID instead > v3: remove local "bool ret"; fix grammer issue > Signed-off-by: root You're missing an empty line between your commit message and the Signed-off-by: line. Also please do not indent your commit message. "git log" already indents it and it would look too indented to the right. Below for more: > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 35 - > 1 file changed, 29 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index a053b7af0680..7f18286a0cc2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -193,12 +193,18 @@ static int psp_sw_fini(void *handle) > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > psp_memory_training_fini(&adev->psp); > - release_firmware(adev->psp.sos_fw); > - adev->psp.sos_fw = NULL; > - release_firmware(adev->psp.asd_fw); > - adev->psp.asd_fw = NULL; > - release_firmware(adev->psp.ta_fw); > - adev->psp.ta_fw = NULL; > + if (adev->psp.sos_fw) { > + release_firmware(adev->psp.sos_fw); > + adev->psp.sos_fw = NULL; > + } > + if (adev->psp.asd_fw) { > + release_firmware(adev->psp.asd_fw); > + adev->psp.asd_fw = NULL; > + } > + if (adev->psp.ta_fw) { > + release_firmware(adev->psp.ta_fw); > + adev->psp.ta_fw = NULL; > + } > > if (adev->asic_type == CHIP_NAVI10) > psp_sysfs_fini(adev); > @@ -409,11 +415,28 @@ static int psp_clear_vf_fw(struct psp_context *psp) > return ret; > } > > +static bool psp_skip_tmr(struct psp_context *psp) > +{ > + switch (psp->adev->asic_type) { > + case CHIP_NAVI12: > + case CHIP_SIENNA_CICHLID: > + return true; > + default: > + return false; > + } > +} > + > static int psp_tmr_load(struct psp_context *psp) > { > int ret; > struct psp_gfx_cmd_resp *cmd; > > + /* for Navi12 and CHIP_SIENNA_CICHLID SRIOV, do not set up TMR > + * (already setup by host driver) Thanks for fixing noun "setup" to verb "set up". But there is another "already setup by" should be "already set up by the host driver". Thanks and regards, Luben > + */ > + if (amdgpu_sriov_vf(psp->adev) && psp_skip_tmr(psp)) > + return 0; > + > cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL); > if (!cmd) > return -ENOMEM; > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/2] drm amdgpu: Skip tmr load for SRIOV
On 2020-07-27 6:57 a.m., Liu ChengZhe wrote: > From: root > > 1. For Navi12, CHIP_SIENNA_CICHLID, skip tmr load operation; > 2. Check pointer before release firmware. > > Signed-off-by: root > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 40 + > 1 file changed, 34 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index a053b7af0680..a9481e112cb3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -193,12 +193,18 @@ static int psp_sw_fini(void *handle) > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > psp_memory_training_fini(&adev->psp); > - release_firmware(adev->psp.sos_fw); > - adev->psp.sos_fw = NULL; > - release_firmware(adev->psp.asd_fw); > - adev->psp.asd_fw = NULL; > - release_firmware(adev->psp.ta_fw); > - adev->psp.ta_fw = NULL; > + if (adev->psp.sos_fw) { > + release_firmware(adev->psp.sos_fw); > + adev->psp.sos_fw = NULL; > + } > + if (adev->psp.asd_fw) { > + release_firmware(adev->psp.asd_fw); > + adev->psp.asd_fw = NULL; > + } > + if (adev->psp.ta_fw) { > + release_firmware(adev->psp.ta_fw); > + adev->psp.ta_fw = NULL; > + } > > if (adev->asic_type == CHIP_NAVI10) > psp_sysfs_fini(adev); > @@ -409,11 +415,33 @@ static int psp_clear_vf_fw(struct psp_context *psp) > return ret; > } > > +static bool psp_skip_tmr(struct psp_context *psp) > +{ > + bool ret = false; > + > + switch (psp->adev->asic_type) { > + case CHIP_NAVI12: > + case CHIP_SIENNA_CICHLID: > + ret = true; > + break; > + default: > + return false; > + } > + > + return ret; > +} There's no need for the local "bool ret". Remove it. See in the "default:" case you already do "return false;". Do "return true;" in the NAVI12 and SIENNA CICHLID case. Regards, Luben > + > static int psp_tmr_load(struct psp_context *psp) > { > int ret; > struct psp_gfx_cmd_resp *cmd; > > + /* for Navi12 and CHIP_SIENNA_CICHLID SRIOV, do not setup TMR > + * (already setup by host driver) > + */ > + if (amdgpu_sriov_vf(psp->adev) && psp_skip_tmr(psp)) > + return 0; > + > cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL); > if (!cmd) > return -ENOMEM; > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/2] drm amdgpu: Skip tmr load for SRIOV
[AMD Official Use Only - Internal Distribution Only] Please fix your git setup to use your name rather than "root" as the author. Alex From: Liu ChengZhe Sent: Monday, July 27, 2020 6:57 AM To: amd-gfx@lists.freedesktop.org Cc: Tuikov, Luben ; Koenig, Christian ; Deucher, Alexander ; Xiao, Jack ; Zhang, Hawking ; Xu, Feifei ; Wang, Kevin(Yang) ; Yuan, Xiaojie ; Liu, Cheng Zhe Subject: [PATCH 1/2] drm amdgpu: Skip tmr load for SRIOV From: root 1. For Navi12, CHIP_SIENNA_CICHLID, skip tmr load operation; 2. Check pointer before release firmware. Signed-off-by: root --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 40 + 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index a053b7af0680..a9481e112cb3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -193,12 +193,18 @@ static int psp_sw_fini(void *handle) struct amdgpu_device *adev = (struct amdgpu_device *)handle; psp_memory_training_fini(&adev->psp); - release_firmware(adev->psp.sos_fw); - adev->psp.sos_fw = NULL; - release_firmware(adev->psp.asd_fw); - adev->psp.asd_fw = NULL; - release_firmware(adev->psp.ta_fw); - adev->psp.ta_fw = NULL; + if (adev->psp.sos_fw) { + release_firmware(adev->psp.sos_fw); + adev->psp.sos_fw = NULL; + } + if (adev->psp.asd_fw) { + release_firmware(adev->psp.asd_fw); + adev->psp.asd_fw = NULL; + } + if (adev->psp.ta_fw) { + release_firmware(adev->psp.ta_fw); + adev->psp.ta_fw = NULL; + } if (adev->asic_type == CHIP_NAVI10) psp_sysfs_fini(adev); @@ -409,11 +415,33 @@ static int psp_clear_vf_fw(struct psp_context *psp) return ret; } +static bool psp_skip_tmr(struct psp_context *psp) +{ + bool ret = false; + + switch (psp->adev->asic_type) { + case CHIP_NAVI12: + case CHIP_SIENNA_CICHLID: + ret = true; + break; + default: + return false; + } + + return ret; +} + static int psp_tmr_load(struct psp_context *psp) { int ret; struct psp_gfx_cmd_resp *cmd; + /* for Navi12 and CHIP_SIENNA_CICHLID SRIOV, do not setup TMR +* (already setup by host driver) +*/ + if (amdgpu_sriov_vf(psp->adev) && psp_skip_tmr(psp)) + return 0; + cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL); if (!cmd) return -ENOMEM; -- 2.25.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx