Re: [Intel-gfx] [PATCH 2/2] drm/amdgpu: Disable PCIE_DPM on Intel RKL Platform

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 8/25/2021 4:46 PM, Koba Ko wrote:
> On Wed, Aug 25, 2021 at 6:24 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>>
>> On Wed, 25 Aug 2021, Koba Ko <koba.ko@canonical.com> wrote:
>>> On Wed, Aug 25, 2021 at 5:22 PM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>>>>
>>>> On Wed, 25 Aug 2021, Koba Ko <koba.ko@canonical.com> 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,
>>>>>
>>>>> For avoiding the module parameter, switch PCI_DPM by determining
>>>>> intel platform in amd drm driver.
>>>>
>>>> I'll just note that you could have a Tiger Lake PCH combined with a
>>>> number of platforms other than Rocket Lake, including not just the
>>>> obvious Tiger Lake but also Sky Lake, Kaby Lake, Coffee Lake, and Comet
>>>> Lake.
>>>>
>>>> Again, I don't know what the root cause or fix should be, the workaround
>>>> presented here impacts a much larger number of platforms than where
>>>> you're claiming the issue is.
>>>
>>> Hi Jani, thanks for your feedback.
>>> Is there any way to identify the RKL PCH?
>>> I trace the intel_pch.c and can't find the only pch id for RKL.
>>>
>>> INTEL_PCH_TGP_DEVICE_ID_TYPE is used by both TGL and RKL.
>>>
>>> so it seems that using IS_ROCKETLAKE() is the only way.
>>
>> I don't think there is a Rocket Lake PCH. But is the problem related to
>> the PCH or not?
> 
> I thought its' not because the issue wouldn't be observed on the TGL platform.
> I only tried RKL platform and it use
> INTEL_PCH_TGP_DEVICE_ID_TYPE/INTEL_PCH_TGP2_DEVICE_ID_TYPE,
> As per AMD guys, they said the issue is only triggered in RKL platform.
> 
>>
>> The GPU PCI IDs are in i915_pciids.h. See INTEL_RKL_IDS() for
>> RKL. There's a lot of indirection, but that's what IS_ROCKETLAKE() boils
>> down to. But again, I'm not sure if that's what you want or not.
> Thanks for suggestions,
> 
> Just want a way to check if it's a RKL platform,
> After tracing the kernel, can check by CPU VENDOR(lacks type), check
> igpu(but there're cpus without igpu)
> and check pch type(it seems one pch has multiple combinations with CPU).
> for check igpu, as per my current understanding,  only found RKL CPU with igpu.
> Is there a RKL CPU without integrated gpu?
> 

Just for RKL - you could do fetch the x86 info and check

#ifdef CONFIG_X86_64
         struct cpuinfo_x86 *c = &cpu_data(0);
	// Family/Model check, find the model
	(c->x86 == 6 && c->x86_model == INTEL_FAM6_ROCKETLAKE)
#endif

I think we don't use anything like this so far. So Alex should give a 
nod as well.

Thanks,
Lijo

>>
>> BR,
>> Jani.
>>
>>
>>>
>>> Thanks
>>>>
>>>> BR,
>>>> Jani.
>>>>
>>>>
>>>>>
>>>>> 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&amp;data=04%7C01%7Clijo.lazar%40amd.com%7C888ab428f2bb4f32e4d408d967c4ae08%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637654916721463596%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Bgf14CmIx%2FTOD54LN6dccZL0U5gT9lv9yTw7MfKc2sQ%3D&amp;reserved=0
>>>>> Signed-off-by: Koba Ko <koba.ko@canonical.com>
>>>>> ---
>>>>>   .../drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c   | 21 ++++++++++++++++++-
>>>>>   1 file changed, 20 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..346110dd0f51 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
>>>>> @@ -1733,6 +1733,25 @@ static int smu7_disable_dpm_tasks(struct pp_hwmgr *hwmgr)
>>>>>        return result;
>>>>>   }
>>>>>
>>>>> +#include <drm/intel_pch.h>
>>>>> +
>>>>> +static bool intel_tgp_chk(void)
>>>>> +{
>>>>> +     struct pci_dev *pch = NULL;
>>>>> +     unsigned short id;
>>>>> +
>>>>> +     while ((pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, pch))) {
>>>>> +             if (pch->vendor != PCI_VENDOR_ID_INTEL)
>>>>> +                     continue;
>>>>> +
>>>>> +             id = pch->device & INTEL_PCH_DEVICE_ID_MASK;
>>>>> +             if (id == INTEL_PCH_TGP_DEVICE_ID_TYPE || INTEL_PCH_TGP2_DEVICE_ID_TYPE)
>>>>
>>>> PS. This is always true. ;)
>>>
>>> got, thanks
>>>
>>>>
>>>>> +                     return true;
>>>>> +     }
>>>>> +
>>>>> +     return false;
>>>>> +}
>>>>> +
>>>>>   static void smu7_init_dpm_defaults(struct pp_hwmgr *hwmgr)
>>>>>   {
>>>>>        struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
>>>>> @@ -1758,7 +1777,7 @@ 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_tgp_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;
>>>>
>>>> --
>>>> Jani Nikula, Intel Open Source Graphics Center
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center