[PATCH V2 1/1] drm/amdgpu: Disable PCIE_DPM on Intel RKL Platform

[Lazar, Lijo]

On 8/26/2021 3:24 PM, Koba Ko wrote:
> On Thu, Aug 26, 2021 at 5:34 PM Koba Ko <koba.ko at canonical.com> wrote:
>>
>> On Thu, Aug 26, 2021 at 5:07 PM Lazar, Lijo <lijo.lazar at amd.com> wrote:
>>>
>>>
>>>
>>> On 8/26/2021 7:05 AM, Koba Ko wrote:
>>>> AMD polaris GPUs have an issue about audio noise on RKL platform,
>>>> they provide a commit to fix but for SMU7-based GPU still
>>>> need another module parameter,
>>>>
>>>> modprobe amdgpu ppfeaturemask=0xfff7bffb
>>>>
>>>> to avoid the module parameter, switch PCI_DPM by determining
>>>> intel platform in amd drm driver is a better way.
>>>>
>>>> Fixes: 1a31474cdb48 ("drm/amd/pm: workaround for audio noise issue")
>>>> Ref: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Famd-gfx%2F2021-August%2F067413.html&data=04%7C01%7Clijo.lazar%40amd.com%7C86f18ece04774ed787e408d9687784a3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637655684803425194%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=6dRblodTvF5XDlRDFtYwVDv6Go2eAd9R9q%2B8hfy6lsY%3D&reserved=0
>>>> Signed-off-by: Koba Ko <koba.ko at canonical.com>
>>>> ---
>>>>    .../gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c   | 15 ++++++++++++++-
>>>>    1 file changed, 14 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>>>> index 0541bfc81c1b..6ce2a2046457 100644
>>>> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>>>> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>>>> @@ -27,6 +27,7 @@
>>>>    #include <linux/pci.h>
>>>>    #include <linux/slab.h>
>>>>    #include <asm/div64.h> > +#include <asm/intel-family.h>
>>>
>>> Maybe, include conditionally for X86_64.
>>>
>>>>    #include <drm/amdgpu_drm.h>
>>>>    #include "ppatomctrl.h"
>>>>    #include "atombios.h"
>>>> @@ -1733,6 +1734,17 @@ static int smu7_disable_dpm_tasks(struct pp_hwmgr *hwmgr)
>>>>        return result;
>>>>    }
>>>>
>>>> +static bool intel_core_rkl_chk(void)
>>>> +{
>>>> +#ifdef CONFIG_X86_64
>>>
>>> Better to use IS_ENABLED() here.
>>>
>>> Apart from that, looks fine to me.
>>>
>>> Reviewed-by: Lijo Lazar <lijo.lazar at amd.com>
>>
>> Thanks for the comments.
>> I will send v3.
> 
> Should I nack v2 after sending v3?
> Thanks

v3 supersedes v2.

My comments are not major that I want to see the patch again after fixing :)

You may fix it before submitting or send a v3 so that others take a look 
as well.

Thanks,
Lijo

>>>
>>> Thanks,
>>> Lijo
>>>
>>>> +     struct cpuinfo_x86 *c = &cpu_data(0);
>>>> +
>>>> +     return (c->x86 == 6 && c->x86_model == INTEL_FAM6_ROCKETLAKE);
>>>> +#else
>>>> +     return false;
>>>> +#endif
>>>> +}
>>>> +
>>>>    static void smu7_init_dpm_defaults(struct pp_hwmgr *hwmgr)
>>>>    {
>>>>        struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
>>>> @@ -1758,7 +1770,8 @@ static void smu7_init_dpm_defaults(struct pp_hwmgr *hwmgr)
>>>>
>>>>        data->mclk_dpm_key_disabled = hwmgr->feature_mask & PP_MCLK_DPM_MASK ? false : true;
>>>>        data->sclk_dpm_key_disabled = hwmgr->feature_mask & PP_SCLK_DPM_MASK ? false : true;
>>>> -     data->pcie_dpm_key_disabled = hwmgr->feature_mask & PP_PCIE_DPM_MASK ? false : true;
>>>> +     data->pcie_dpm_key_disabled =
>>>> +             intel_core_rkl_chk() || !(hwmgr->feature_mask & PP_PCIE_DPM_MASK);
>>>>        /* need to set voltage control types before EVV patching */
>>>>        data->voltage_control = SMU7_VOLTAGE_CONTROL_NONE;
>>>>        data->vddci_control = SMU7_VOLTAGE_CONTROL_NONE;
>>>>