[AMD Official Use Only]
> -----Original Message----- > From: Lazar, Lijo <[email protected]> > Sent: Friday, November 12, 2021 4:46 PM > To: Quan, Evan <[email protected]>; [email protected] > Cc: Deucher, Alexander <[email protected]>; Borislav Petkov > <[email protected]> > Subject: Re: [PATCH] drm/amd/pm: avoid duplicate powergate/ungate > setting > > > > On 11/11/2021 1:27 PM, Evan Quan wrote: > > Just bail out if the target IP block is already in the desired > > powergate/ungate state. This can avoid some duplicate settings which > > sometimes may cause unexpected issues. > > > > Link: https://lore.kernel.org/all/[email protected]/ > > > > Change-Id: I66346c69f121df0f5ee20182451313ae4fda2d04 > > Signed-off-by: Evan Quan <[email protected]> > > Tested-by: Borislav Petkov <[email protected]> > > -- > > v1->v2: > > - typo fix and add link for the issue referred in commit > > message(Paul/Boris) > > - update the data type to uint32_t(Paul) > > - better Macro naming(Lijo) > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++ > > drivers/gpu/drm/amd/include/amd_shared.h | 3 ++- > > drivers/gpu/drm/amd/pm/amdgpu_dpm.c | 12 +++++++++++- > > drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 8 ++++++++ > > 4 files changed, 24 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > index 0bd90ec9e43e..fca592394fa1 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > @@ -3508,6 +3508,9 @@ int amdgpu_device_init(struct amdgpu_device > *adev, > > adev->rmmio_size = pci_resource_len(adev->pdev, 2); > > } > > > > + for (i = 0; i < AMD_IP_BLOCK_TYPE_NUM; i++) > > + atomic_set(&adev->pm.pwr_state[i], > POWER_STATE_UNKNOWN); > > + > > adev->rmmio = ioremap(adev->rmmio_base, adev->rmmio_size); > > if (adev->rmmio == NULL) { > > return -ENOMEM; > > diff --git a/drivers/gpu/drm/amd/include/amd_shared.h > > b/drivers/gpu/drm/amd/include/amd_shared.h > > index f1a46d16f7ea..4b9e68a79f06 100644 > > --- a/drivers/gpu/drm/amd/include/amd_shared.h > > +++ b/drivers/gpu/drm/amd/include/amd_shared.h > > @@ -98,7 +98,8 @@ enum amd_ip_block_type { > > AMD_IP_BLOCK_TYPE_ACP, > > AMD_IP_BLOCK_TYPE_VCN, > > AMD_IP_BLOCK_TYPE_MES, > > - AMD_IP_BLOCK_TYPE_JPEG > > + AMD_IP_BLOCK_TYPE_JPEG, > > + AMD_IP_BLOCK_TYPE_NUM, > > }; > > > > enum amd_clockgating_state { > > diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c > > b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c > > index 03581d5b1836..40fda199a86f 100644 > > --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c > > +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c > > @@ -927,6 +927,13 @@ int > amdgpu_dpm_set_powergating_by_smu(struct amdgpu_device *adev, > uint32_t block > > { > > int ret = 0; > > const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs; > > + enum ip_power_state pwr_state = gate ? POWER_STATE_OFF : > > +POWER_STATE_ON; > > + > > + if (atomic_read(&adev->pm.pwr_state[block_type]) == pwr_state) { > > + dev_dbg(adev->dev, "IP block%d already in the target %s > state!", > > + block_type, gate ? "gate" : "ungate"); > > + return 0; > > + } > > > > switch (block_type) { > > case AMD_IP_BLOCK_TYPE_UVD: > > @@ -976,9 +983,12 @@ int > amdgpu_dpm_set_powergating_by_smu(struct amdgpu_device *adev, > uint32_t block > > } > > break; > > default: > > - break; > > + return -EINVAL; > > IP blocks which were not handled by PMFW were default pass before. Will > this be a problem or return 0 here to keep the legacy behavior? [Quan, Evan] Although I do not think there will be a problem with such change(considering there is no such IP block). I will update the patch to stick to its original logic. Thanks, Evan > > Thanks, > Lijo > > > } > > > > + if (!ret) > > + atomic_set(&adev->pm.pwr_state[block_type], pwr_state); > > + > > return ret; > > } > > > > diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h > > b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h > > index 98f1b3d8c1d5..16e3f72d31b9 100644 > > --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h > > +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h > > @@ -417,6 +417,12 @@ struct amdgpu_dpm { > > enum amd_dpm_forced_level forced_level; > > }; > > > > +enum ip_power_state { > > + POWER_STATE_UNKNOWN, > > + POWER_STATE_ON, > > + POWER_STATE_OFF, > > +}; > > + > > struct amdgpu_pm { > > struct mutex mutex; > > u32 current_sclk; > > @@ -452,6 +458,8 @@ struct amdgpu_pm { > > struct i2c_adapter smu_i2c; > > struct mutex smu_i2c_mutex; > > struct list_head pm_attr_list; > > + > > + atomic_t pwr_state[AMD_IP_BLOCK_TYPE_NUM]; > > }; > > > > #define R600_SSTU_DFLT 0 > >
