| Message ID | 20220517113327.26919-1-tzimmermann@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm: Ignore non-existing color planes in helpers | expand |
Am 17.05.22 um 13:33 schrieb Thomas Zimmermann: > Some DRM helpers assume that all potential color planes of a framebuffer > are available; even if the color format didn't specify them. Non-existing > planes are silently ignored. This behavior is inconsistent with other > helpers and apparently leads to subtle bugs with uninitialized GEM buffer > mappings. [1] > > Change all affected code to look at the framebuffer format's number of > color planes and only process these planes. The update has been discussed > in [2] before. > > Tested with GEM SHMEM helpers on simpledrm and with GEM VRAM helpers on > ast. I'm not deep enough into all the details for a full review, but feel free to add an Acked-by: Christian König <christian.koenig@amd.com> to the series. Thanks, Christian. > > v2: > * refactor GEM VRAM code before fixing it (Javier) > * print more error information in #1 (Javier) > * commit-message fixes (Javier) > > [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20210730183511.20080-1-tzimmermann%40suse.de%2FT%2F%23md0172b10bb588d8f20f4f456e304f08d2a4505f7&data=05%7C01%7Cchristian.koenig%40amd.com%7Cc512a92c206f4af0691108da37f915a1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637883840259373791%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6fXRY%2BWlo1I47tcuRHiamAZ9JGM%2FHaYTfyGnxNrqUts%3D&reserved=0 > [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F877dc0d9-c6c6-022c-20d8-14b33e863934%40suse.de%2F&data=05%7C01%7Cchristian.koenig%40amd.com%7Cc512a92c206f4af0691108da37f915a1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637883840259373791%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6iFYw5EtQNaSiPrHIAawBw87lj6M66o7j0ADY0ipifs%3D&reserved=0 > > Thomas Zimmermann (5): > drm/gem: Share code between drm_gem_fb_{begin,end}_cpu_access() > drm/gem: Ignore color planes that are unused by framebuffer format > drm/gem-vram: Share code between GEM VRAM's _{prepare,cleanup}_fb() > drm/gem-vram: Ignore planes that are unused by framebuffer format > drm/gem: Warn on trying to use a non-existing framebuffer plane > > drivers/gpu/drm/drm_gem_atomic_helper.c | 6 +- > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 104 +++++++++---------- > drivers/gpu/drm/drm_gem_vram_helper.c | 54 ++++++---- > include/drm/drm_gem_framebuffer_helper.h | 10 +- > 4 files changed, 92 insertions(+), 82 deletions(-) >
Hi Am 17.05.22 um 13:56 schrieb Christian König: > Am 17.05.22 um 13:33 schrieb Thomas Zimmermann: >> Some DRM helpers assume that all potential color planes of a framebuffer >> are available; even if the color format didn't specify them. Non-existing >> planes are silently ignored. This behavior is inconsistent with other >> helpers and apparently leads to subtle bugs with uninitialized GEM buffer >> mappings. [1] >> >> Change all affected code to look at the framebuffer format's number of >> color planes and only process these planes. The update has been discussed >> in [2] before. >> >> Tested with GEM SHMEM helpers on simpledrm and with GEM VRAM helpers on >> ast. > > I'm not deep enough into all the details for a full review, but feel > free to add an Acked-by: Christian König <christian.koenig@amd.com> to > the series. Thanks a lot. I cc'ed you because we recently talked about the inconsistent use of num_planes throughout the DRM helpers. Best regards Thomas > > Thanks, > Christian. > >> >> v2: >> * refactor GEM VRAM code before fixing it (Javier) >> * print more error information in #1 (Javier) >> * commit-message fixes (Javier) >> >> [1] >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20210730183511.20080-1-tzimmermann%40suse.de%2FT%2F%23md0172b10bb588d8f20f4f456e304f08d2a4505f7&data=05%7C01%7Cchristian.koenig%40amd.com%7Cc512a92c206f4af0691108da37f915a1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637883840259373791%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6fXRY%2BWlo1I47tcuRHiamAZ9JGM%2FHaYTfyGnxNrqUts%3D&reserved=0 >> >> [2] >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F877dc0d9-c6c6-022c-20d8-14b33e863934%40suse.de%2F&data=05%7C01%7Cchristian.koenig%40amd.com%7Cc512a92c206f4af0691108da37f915a1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637883840259373791%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6iFYw5EtQNaSiPrHIAawBw87lj6M66o7j0ADY0ipifs%3D&reserved=0 >> >> >> Thomas Zimmermann (5): >> drm/gem: Share code between drm_gem_fb_{begin,end}_cpu_access() >> drm/gem: Ignore color planes that are unused by framebuffer format >> drm/gem-vram: Share code between GEM VRAM's _{prepare,cleanup}_fb() >> drm/gem-vram: Ignore planes that are unused by framebuffer format >> drm/gem: Warn on trying to use a non-existing framebuffer plane >> >> drivers/gpu/drm/drm_gem_atomic_helper.c | 6 +- >> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 104 +++++++++---------- >> drivers/gpu/drm/drm_gem_vram_helper.c | 54 ++++++---- >> include/drm/drm_gem_framebuffer_helper.h | 10 +- >> 4 files changed, 92 insertions(+), 82 deletions(-) >> >
Some DRM helpers assume that all potential color planes of a framebuffer are available; even if the color format didn't specify them. Non-existing planes are silently ignored. This behavior is inconsistent with other helpers and apparently leads to subtle bugs with uninitialized GEM buffer mappings. [1] Change all affected code to look at the framebuffer format's number of color planes and only process these planes. The update has been discussed in [2] before. Tested with GEM SHMEM helpers on simpledrm and with GEM VRAM helpers on ast. v2: * refactor GEM VRAM code before fixing it (Javier) * print more error information in #1 (Javier) * commit-message fixes (Javier) [1] https://lore.kernel.org/dri-devel/20210730183511.20080-1-tzimmermann@suse.de/T/#md0172b10bb588d8f20f4f456e304f08d2a4505f7 [2] https://lore.kernel.org/dri-devel/877dc0d9-c6c6-022c-20d8-14b33e863934@suse.de/ Thomas Zimmermann (5): drm/gem: Share code between drm_gem_fb_{begin,end}_cpu_access() drm/gem: Ignore color planes that are unused by framebuffer format drm/gem-vram: Share code between GEM VRAM's _{prepare,cleanup}_fb() drm/gem-vram: Ignore planes that are unused by framebuffer format drm/gem: Warn on trying to use a non-existing framebuffer plane drivers/gpu/drm/drm_gem_atomic_helper.c | 6 +- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 104 +++++++++---------- drivers/gpu/drm/drm_gem_vram_helper.c | 54 ++++++---- include/drm/drm_gem_framebuffer_helper.h | 10 +- 4 files changed, 92 insertions(+), 82 deletions(-)