-----Original Message----- From: Daniel Vetter daniel@ffwll.ch Sent: 2021/January/07, Thursday 12:33 PM To: Koenig, Christian Christian.Koenig@amd.com Cc: Liu, Zhan Zhan.Liu@amd.com; amd-gfx@lists.freedesktop.org; Cornij, Nikola Nikola.Cornij@amd.com; Wang, Chao-kai (Stylon) Stylon.Wang@amd.com; Wang, Chao-kai (Stylon) Stylon.Wang@amd.com; dri-devel@lists.freedesktop.org; Kazlauskas, Nicholas Nicholas.Kazlauskas@amd.com; bas@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@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@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