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

[Koba Ko <koba.ko@canonical.com>]

On Wed, Aug 25, 2021 at 10:33 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Wed, Aug 25, 2021 at 10:22 AM Lazar, Lijo <lijo.lazar@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@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.
>
> 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, use Lazar's suggestion and don't need any information from i915.
I will send the next patch.

Thanks
Koba

>
> >
> > 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