[PATCH] drm/amd/display: enable S/G display for for recent APUs by default

[Zhang, Yifan]

[AMD Official Use Only - General]

Good idea ! Thanks.

-----Original Message-----
From: Alex Deucher <alexdeucher at gmail.com>
Sent: Wednesday, September 27, 2023 9:13 PM
To: Koenig, Christian <Christian.Koenig at amd.com>
Cc: Zhang, Yifan <Yifan1.Zhang at amd.com>; amd-gfx at lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher at amd.com>; Wentland, Harry <Harry.Wentland at amd.com>
Subject: Re: [PATCH] drm/amd/display: enable S/G display for for recent APUs by default

On Wed, Sep 27, 2023 at 8:26 AM Christian König <christian.koenig at amd.com> wrote:
>
> Am 27.09.23 um 07:41 schrieb Yifan Zhang:
> > With S/G display becomes stable, enable S/G display for recent APUs
> > by default rather than white list.
> >
> > Signed-off-by: Yifan Zhang <yifan1.zhang at amd.com>
> > ---
> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 43 +++++--------------
> >   1 file changed, 10 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 7e6a693d6369..241fd8defdee 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -1626,41 +1626,18 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
> >               break;
> >       }
> >
> > -     switch (adev->asic_type) {
> > -     case CHIP_CARRIZO:
> > -     case CHIP_STONEY:
> > +     if ((adev->asic_type == CHIP_CARRIZO ||
> > +             adev->asic_type == CHIP_STONEY ||
> > +             ((adev->flags & AMD_IS_APU) &&
> > +             amdgpu_ip_version(adev, DCE_HWIP, 0) >= IP_VERSION(1, 0, 0) &&
> > +             !(adev->apu_flags & AMD_APU_IS_RAVEN))) &&
> > +             amdgpu_sg_display != 0) {
>
> Looks like a good idea to me, but please double check your coding
> style settings. The second line of an "if (" should be indented so
> that it starts after the "(" of the previous line.
>
> Additional to that this check actually looks quite messy to me and
> should probably be converted to always using the IP version instead of
> the asic_type.

Or just explicitly disable it on the handful or pre-CZ chips that don't actually support it.
E.g.,

switch (asic_type) {
case CHIP_KAVERI:
case CHIP_KABINI:
case CHIP_MULLINS:
   vm = false;
   break;
case CHIP_RAVEN:
   if (apu_flags & AMD_APU_IS_RAVEN)
       vm = false;
   else
       vm = amdgpu_sg_display != 0;
   break;
default:
   vm = (amdgpu_sg_display != 0) && (flags && AMD_IS_APU);
   break;

}

>
> Regards,
> Christian.
>
> >               init_data.flags.gpu_vm_support = true;
> > -             break;
> > -     default:
> > -             switch (amdgpu_ip_version(adev, DCE_HWIP, 0)) {
> > -             case IP_VERSION(1, 0, 0):
> > -             case IP_VERSION(1, 0, 1):
> > -                     /* enable S/G on PCO and RV2 */
> > -                     if ((adev->apu_flags & AMD_APU_IS_RAVEN2) ||
> > -                         (adev->apu_flags & AMD_APU_IS_PICASSO))
> > -                             init_data.flags.gpu_vm_support = true;
> > -                     break;
> > -             case IP_VERSION(2, 1, 0):
> > -             case IP_VERSION(3, 0, 1):
> > -             case IP_VERSION(3, 1, 2):
> > -             case IP_VERSION(3, 1, 3):
> > -             case IP_VERSION(3, 1, 4):
> > -             case IP_VERSION(3, 1, 5):
> > -             case IP_VERSION(3, 1, 6):
> > -             case IP_VERSION(3, 5, 0):
> > -                     init_data.flags.gpu_vm_support = true;
> > -                     break;
> > -             default:
> > -                     break;
> > -             }
> > -             break;
> > -     }
> > -     if (init_data.flags.gpu_vm_support &&
> > -         (amdgpu_sg_display == 0))
> > -             init_data.flags.gpu_vm_support = false;
> > -
> > -     if (init_data.flags.gpu_vm_support)
> >               adev->mode_info.gpu_vm_support = true;
> > +     } else {
> > +             init_data.flags.gpu_vm_support = false;
> > +             adev->mode_info.gpu_vm_support = false;
> > +     }
> >
> >       if (amdgpu_dc_feature_mask & DC_FBC_MASK)
> >               init_data.flags.fbc_support = true;
>