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