[PATCH v2] drm/amdgpu: Do not change amdgpu framebuffer format during page flip

[Liu, Zhan]

> -----Original Message-----
> From: Daniel Vetter <daniel at ffwll.ch>
> Sent: 2021/January/07, Thursday 12:33 PM
> To: Koenig, Christian <Christian.Koenig at amd.com>
> Cc: Liu, Zhan <Zhan.Liu at amd.com>; amd-gfx at lists.freedesktop.org; Cornij,
> Nikola <Nikola.Cornij at amd.com>; Wang, Chao-kai (Stylon)
> <Stylon.Wang at amd.com>; Wang, Chao-kai (Stylon)
> <Stylon.Wang at amd.com>; dri-devel at lists.freedesktop.org; Kazlauskas,
> Nicholas <Nicholas.Kazlauskas at amd.com>; bas at basnieuwenhuizen.nl
> Subject: Re: [PATCH v2] drm/amdgpu: Do not change amdgpu framebuffer
> format during page flip
> 
> On Sun, Jan 03, 2021 at 04:43:37PM +0100, Christian König wrote:
> > Am 29.12.20 um 22:10 schrieb Zhan Liu:
> > > [Why]
> > > Driver cannot change amdgpu framebuffer (afb) format while doing
> > > page flip. Force system doing so will cause ioctl error, and result
> > > in breaking several functionalities including FreeSync.
> > >
> > > If afb format is forced to change during page flip, following
> > > message will appear in dmesg.log:
> > >
> > > "[drm:drm_mode_page_flip_ioctl [drm]] Page flip is not allowed to
> > > change frame buffer format."
> > >
> > > [How]
> > > Do not change afb format while doing page flip. It is okay to check
> > > whether afb format is valid here, however, forcing afb format change
> > > shouldn't happen here.
> >
> > I don't think this we can do this.
> >
> > It is perfectly valid for a page flip to switch between linear and
> > tiled formats on an I+A or A+A laptop.
> 
> It is, but that's not the bug here. struct drm_framebuffer.format is supposed
> to be invariant over the lifetime of a drm_fb object, you need to set it when
> the fb is created and never change it afterwards. So the patch here isn't yet
> the real deal.
> 
> Also this means the implicit tiling information cannot be changed after a fb is
> created for a given bo, but we've discussed this at length and that limitation
> should be all ok.
> -Daniel

Thank you Christian and Daniel for the input!

Bas recently submitted an alternative patch ([PATCH] drm: Check actual format for legacy pageflip.) 
which addresses the same issue, and his patch makes more sense to me, so I will abandon my patch in this case.

Thanks,
Zhan


> 
> >
> > Christian.
> >
> > >
> > > Signed-off-by: Zhan Liu <zhan.liu at amd.com>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > Thanks Nick and Bas. Here is my second patch for review.
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > > index a638709e9c92..8a12e27ff4ec 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > > @@ -832,7 +832,8 @@ static int convert_tiling_flags_to_modifier(struct
> amdgpu_framebuffer *afb)
> > >   			if (!format_info)
> > >   				return -EINVAL;
> > > -			afb->base.format = format_info;
> > > +			if (!afb->base.format)
> > > +				afb->base.format = format_info;
> > >   		}
> > >   	}
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > s.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-
> devel&data=04%7C01%7C
> >
> zhan.liu%40amd.com%7Cda23e6e33a7e46dfc4e308d8b33242c8%7C3dd896
> 1fe4884e
> >
> 608e11a82d994e183d%7C0%7C0%7C637456375746425509%7CUnknown%
> 7CTWFpbGZsb3
> >
> d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%7
> >
> C1000&sdata=5lCm4d6FHihfFHUf5mVym0O6lKmZHgR89%2F2Eqj2ojhg
> %3D&r
> > eserved=0
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ff
> wll.ch%2F&data=04%7C01%7Czhan.liu%40amd.com%7Cda23e6e33a7e
> 46dfc4e308d8b33242c8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7
> C0%7C637456375746425509%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&a
> mp;sdata=44x858klbIcVeRtP%2BuJST2K3xuCLisjbfhV9rEQrzpA%3D&rese
> rved=0