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

Wed Aug 25 14:32:48 UTC 2021

On Wed, Aug 25, 2021 at 10:22 AM Lazar, Lijo <lijo.lazar at amd.com> wrote:
>
>
>
> On 8/25/2021 4:46 PM, Koba Ko wrote:
> > On Wed, Aug 25, 2021 at 6:24 PM Jani Nikula <jani.nikula at linux.intel.com> wrote:
> >>
> >> On Wed, 25 Aug 2021, Koba Ko <koba.ko at canonical.com> wrote:
> >>> On Wed, Aug 25, 2021 at 5:22 PM Jani Nikula <jani.nikula at linux.intel.com> wrote:
> >>>>
> >>>> On Wed, 25 Aug 2021, Koba Ko <koba.ko at 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.

I think that makes sense.  For some background the issue that was
observed with RKL was that the PCIE gen switching has a very high
latency which can lead to audio problems during playback if PCIE DPM
is enabled.

Alex

>
> 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&data=04%7C01%7Clijo.lazar%40amd.com%7C888ab428f2bb4f32e4d408d967c4ae08%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637654916721463596%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Bgf14CmIx%2FTOD54LN6dccZL0U5gT9lv9yTw7MfKc2sQ%3D&reserved=0
> >>>>> Signed-off-by: Koba Ko <koba.ko at 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