[PATCH] drm/amdgpu: attempt to enable gfxoff on more raven1 boards

[Alex Deucher]

On Thu, Jan 16, 2020 at 12:27 PM Luben Tuikov <luben.tuikov at amd.com> wrote:
>
> On 2020-01-15 12:31, Alex Deucher wrote:
> > Switch to a blacklist so we can disable specific boards
> > that are problematic.
> >
> > Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 42 ++++++++++++++++++++++++---
> >  1 file changed, 38 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > index e3d466bd5c4e..b48b07bcd0fb 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > @@ -1031,6 +1031,37 @@ static void gfx_v9_0_check_fw_write_wait(struct amdgpu_device *adev)
> >       }
> >  }
> >
> > +struct amdgpu_gfxoff_quirk {
> > +     u16 chip_vendor;
> > +     u16 chip_device;
> > +     u16 subsys_vendor;
> > +     u16 subsys_device;
> > +     u8 revision;
> > +};
> > +
> > +static const struct amdgpu_gfxoff_quirk amdgpu_gfxoff_quirk_list[] = {
> > +     /* https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D204689&data=02%7C01%7Cluben.tuikov%40amd.com%7C683669e5a2c74bcbbc9108d799e0cda4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637147063903364365&sdata=UL9SCKI7OchzK6a27AxkjrpeLNw%2BWH5DmpWGKutCI4A%3D&reserved=0 */
> > +     { 0x1002, 0x15dd, 0x1002, 0x15dd, 0xc8 },
> > +     { 0, 0, 0, 0, 0 },
> > +};
> > +
> > +static bool gfx_v9_0_raven_check_disable_gfxoff(struct pci_dev *pdev)
> > +{
> > +     const struct amdgpu_gfxoff_quirk *p = amdgpu_gfxoff_quirk_list;
> > +
> > +     while (p && p->chip_device != 0) {
>
> Maybe a "for" loop would make it compact?

Seems like the same difference either way.

>
> for ( ; p && p->chip_device != 0; p++) {
>         if (pdev->vendor == p->chip_vendor &&
>             pdev->device == p->chip_device &&
>             pdev->subsystem_vendor == p->subsys_vendor &&
>             pdev->subsystem_device == p->subsys_device &&
>             pdev->revision == p->revision) {
>                 return true;
>         }
> }
>
> I wonder if the structure "amdgpu_gfxoff_quirk" which stores
> device ID information can be named something more generic, (struct device_id?)
> and also used in "pdev"? (Reuse the struct.)
>
> Then we'd only compare structs:
>
> for ( ; p && p->chip_device != 0; p++) {
>         if (pdev->dev_id == *p)
>                 return true;
> }

pdev structure is huge.  All we need are the ids.

Alex

>
> Regards,
> Luben
>
> > +             if (pdev->vendor == p->chip_vendor &&
> > +                 pdev->device == p->chip_device &&
> > +                 pdev->subsystem_vendor == p->subsys_vendor &&
> > +                 pdev->subsystem_device == p->subsys_device &&
> > +                 pdev->revision == p->revision) {
> > +                     return true;
> > +             }
> > +             ++p;
> > +     }
> > +     return false;
> > +}
> > +
> >  static void gfx_v9_0_check_if_need_gfxoff(struct amdgpu_device *adev)
> >  {
> >       switch (adev->asic_type) {
> > @@ -1039,10 +1070,13 @@ static void gfx_v9_0_check_if_need_gfxoff(struct amdgpu_device *adev)
> >       case CHIP_VEGA20:
> >               break;
> >       case CHIP_RAVEN:
> > -             if (!(adev->rev_id >= 0x8 ||
> > -                   adev->pdev->device == 0x15d8) &&
> > -                 (adev->pm.fw_version < 0x41e2b || /* not raven1 fresh */
> > -                  !adev->gfx.rlc.is_rlc_v2_1)) /* without rlc save restore ucodes */
> > +             if (!(adev->rev_id >= 0x8 || adev->pdev->device == 0x15d8) &&
> > +                 ((adev->gfx.rlc_fw_version != 106 &&
> > +                   adev->gfx.rlc_fw_version < 531) ||
> > +                  (adev->gfx.rlc_fw_version == 53815) ||
> > +                  (adev->gfx.rlc_feature_version < 1) ||
> > +                  !adev->gfx.rlc.is_rlc_v2_1) &&
> > +                 !gfx_v9_0_raven_check_disable_gfxoff(adev->pdev))
> >                       adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
> >
> >               if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> >
>