[AMD Official Use Only - General]
Good idea ! Thanks.
-----Original Message-----
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Wednesday, September 27, 2023 9:13 PM
To: Koenig, Christian <Christian.Koenig@xxxxxxx>
Cc: Zhang, Yifan <Yifan1.Zhang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Wentland, Harry <Harry.Wentland@xxxxxxx>
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@xxxxxxx> 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@xxxxxxx>
> > ---
> > .../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;
>
