<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> >Still has risk for s3, we would better not put PG disabling (gfxoff) behind of CG >disabling, even here is SMC clockgating. It is to avoid any MMU using when >gfx is in "off" state. I think logically we need to power on/off GFX ip through SMU, so first ungate SMU block first. Currently, it is not a big deal, because SMU don't support CG/PG on all legacy asics. Best Regards Rex <div style="color: rgb(0, 0, 0);"> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Huang Rui <ray.huang@amd.com> Sent: Wednesday, June 6, 2018 10:33 AM To: Zhu, Rex Cc: amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 2/2] drm/amdgpu: Make gfx_off control by GFX ip <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">On Tue, Jun 05, 2018 at 07:51:17PM +0800, Rex Zhu wrote: > v3: 1. Delete the dead gfx off code in powerplay late_init. > 2. Revert v2. > 3. call smu to power on gfx at the begin of ip_suspend/device_fini > 4. only power off gfx ip in the end of gfx power gate function > v2: Delete the dead gfx off code in ip_suspend. > > gfx off should be controlled by GFX IP. > Powerplay only export interface to gfx ip. > This logic is same as uvd/vce cg/pg. > > Signed-off-by: Rex Zhu <Rex.Zhu@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 ++++++------ > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 4 ++++ > drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 8 -------- > drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 4 ++-- > 4 files changed, 12 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index df6ef9c..ee87fea 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -1830,6 +1830,8 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev) > adev->ip_blocks[i].version->funcs->name, r); > return r; > } > + if (adev->powerplay.pp_funcs->set_powergating_by_smu) > + amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false); > r = adev->ip_blocks[i].version->funcs->hw_fini((void *)adev); > /* XXX handle errors */ > if (r) { > @@ -1940,12 +1942,6 @@ int amdgpu_device_ip_suspend(struct amdgpu_device *adev) > if (amdgpu_sriov_vf(adev)) > amdgpu_virt_request_full_gpu(adev, false); > > - /* ungate SMC block powergating */ > - if (adev->powerplay.pp_feature & PP_GFXOFF_MASK) > - amdgpu_device_ip_set_powergating_state(adev, > - AMD_IP_BLOCK_TYPE_SMC, > - AMD_CG_STATE_UNGATE); > - > /* ungate SMC block first */ > r = amdgpu_device_ip_set_clockgating_state(adev, AMD_IP_BLOCK_TYPE_SMC, > AMD_CG_STATE_UNGATE); > @@ -1953,6 +1949,10 @@ int amdgpu_device_ip_suspend(struct amdgpu_device *adev) > DRM_ERROR("set_clockgating_state(ungate) SMC failed %d\n", r); > } > > + /* call smu to disable gfx off feature first when suspend */ > + if (adev->powerplay.pp_funcs->set_powergating_by_smu) > + amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, false); > + Still has risk for s3, we would better not put PG disabling (gfxoff) behind of CG disabling, even here is SMC clockgating. It is to avoid any MMU using when gfx is in "off" state. Thanks, Ray > for (i = adev->num_ip_blocks - 1; i >= 0; i--) { > if (!adev->ip_blocks[i].status.valid) > continue; > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > index c382969..43c8023 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > @@ -3712,6 +3712,10 @@ static int gfx_v9_0_set_powergating_state(void *handle, > > /* update mgcg state */ > gfx_v9_0_update_gfx_mg_power_gating(adev, enable); > + > + /* set gfx off through smu */ > + if (enable && adev->powerplay.pp_funcs->set_powergating_by_smu) > + amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GFX, true); > break; > default: > break; > diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > index fe3ed8c..8bb3a9a 100644 > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > @@ -180,7 +180,6 @@ static int pp_late_init(void *handle) > { > struct amdgpu_device *adev = handle; > struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle; > - int ret; > > if (hwmgr && hwmgr->pm_en) { > mutex_lock(&hwmgr->smu_lock); > @@ -191,13 +190,6 @@ static int pp_late_init(void *handle) > if (adev->pm.smu_prv_buffer_size != 0) > pp_reserve_vram_for_smu(adev); > > - if (hwmgr->hwmgr_func->gfx_off_control && > - (hwmgr->feature_mask & PP_GFXOFF_MASK)) { > - ret = hwmgr->hwmgr_func->gfx_off_control(hwmgr, true); > - if (ret) > - pr_err("gfx off enabling failed!\n"); > - } > - > return 0; > } > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c > index b9e6dfb..5abae28 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c > @@ -314,7 +314,7 @@ static int smu10_disable_gfx_off(struct pp_hwmgr *hwmgr) > > static int smu10_disable_dpm_tasks(struct pp_hwmgr *hwmgr) > { > - return smu10_disable_gfx_off(hwmgr); > + return 0; > } > > static int smu10_enable_gfx_off(struct pp_hwmgr *hwmgr) > @@ -329,7 +329,7 @@ static int smu10_enable_gfx_off(struct pp_hwmgr *hwmgr) > > static int smu10_enable_dpm_tasks(struct pp_hwmgr *hwmgr) > { > - return smu10_enable_gfx_off(hwmgr); > + return 0; > } > > static int smu10_gfx_off_control(struct pp_hwmgr *hwmgr, bool enable) > -- > 1.9.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" id="LPlnk850295" class="OWAAutoLink" previewremoved="true"> https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> <div id="LPBorder_GT_15282551139680.24578071702098847" style="margin-bottom: 20px; overflow: auto; width: 100%; text-indent: 0px;"> <table id="LPContainer_15282551139660.08193335755167419" style="width: 90%; background-color: rgb(255, 255, 255); position: relative; overflow: auto; padding-top: 20px; padding-bottom: 20px; margin-top: 20px; border-top: 1px dotted rgb(200, 200, 200); border-bottom: 1px dotted rgb(200, 200, 200);" role="presentation" cellspacing="0"> <tbody> <tr style="border-spacing: 0px;" valign="top"> <td id="TextCell_15282551139660.6451629222547607" style="vertical-align: top; position: relative; padding: 0px; display: table-cell;" colspan="2"> <div id="LPRemovePreviewContainer_15282551139660.23860263831547934"></div> <div id="LPTitle_15282551139660.24812846614496475" style="top: 0px; color: rgb(0, 120, 215); font-weight: 400; font-size: 21px; font-family: "wf_segoe-ui_light", "Segoe UI Light", "Segoe WP Light", "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; line-height: 21px;"> <a id="LPUrlAnchor_15282551139660.0021238385388475667" style="text-decoration: none;" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" target="_blank">amd-gfx Info Page - freedesktop.org</a></div> <div id="LPMetadata_15282551139660.31688669868279207" style="margin: 10px 0px 16px; color: rgb(102, 102, 102); font-weight: 400; font-family: "wf_segoe-ui_normal", "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; font-size: 14px; line-height: 14px;"> lists.freedesktop.org</div> <div id="LPDescription_15282551139680.39901652585399017" style="display: block; color: rgb(102, 102, 102); font-weight: 400; font-family: "wf_segoe-ui_normal", "Segoe UI", "Segoe WP", Tahoma, Arial, sans-serif; font-size: 14px; line-height: 20px; max-height: 100px; overflow: hidden;"> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org lists is subject to our Code of Conduct.</div> </td> </tr> </tbody> </table> </div> </div> </div> </div> </div> </body> </html>