Message ID | 20230301153101.4282-1-tzimmermann@suse.de (mailing list archive) |
---|---|
Headers | show |
Series | drm/dma-helper: Add dedicated fbdev emulation | expand |
On Wed, Mar 1, 2023 at 4:31 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > Add fbdev emulation that is optimized for DMA helpers, as used by most > drivers. It operates directly on GEM DMA buffers in system memory. > Memory pages are mmap'ed directly to userspace. No implicit shadow > buffers need to be allocated; as can happen with the generic fbdev > emulation. Convert drivers that fulfil the requirements. > > Tested with fbcon and IGT on vc4. > > Future direction: providing a dedicated fbdev emulation for GEM DMA > helpers will allow us to remove this case from the generic fbdev code. > The latter can then be simplified. 1) I love your work. 2) Why isn't this DRM driver changed? drivers/gpu/drm/mcde/mcde_drv.c AFAICT it also uses GEM buffers in system memory. 3) This one: drivers/gpu/drm/pl111/pl111_drv.c is also very similar, but can sometimes use a dedicated RAM memory for allocations using CMA, does that make it not a candidate? They aren't much different in how they work from the TVE200. Yours, Linus Walleij
Hi Am 06.03.23 um 23:19 schrieb Linus Walleij: > On Wed, Mar 1, 2023 at 4:31 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > >> Add fbdev emulation that is optimized for DMA helpers, as used by most >> drivers. It operates directly on GEM DMA buffers in system memory. >> Memory pages are mmap'ed directly to userspace. No implicit shadow >> buffers need to be allocated; as can happen with the generic fbdev >> emulation. Convert drivers that fulfil the requirements. >> >> Tested with fbcon and IGT on vc4. >> >> Future direction: providing a dedicated fbdev emulation for GEM DMA >> helpers will allow us to remove this case from the generic fbdev code. >> The latter can then be simplified. > > 1) I love your work. Thank you. :) > > 2) Why isn't this DRM driver changed? > drivers/gpu/drm/mcde/mcde_drv.c > AFAICT it also uses GEM buffers in system memory. This driver requires damage handling https://elixir.bootlin.com/linux/v6.2/source/drivers/gpu/drm/mcde/mcde_drv.c#L97 for which we have to call the framebuffer's dirty callback https://elixir.bootlin.com/linux/v6.2/source/drivers/gpu/drm/drm_gem_framebuffer_helper.c#L285 after each write. Doing this with fbdev emulation requires tracking of mmap'ed pages via fbdev's deferred-I/O mechanisms. That makes the fbdev-emulation code more complex. AFAICT, the existing generic fbdev emulation already implements this case 'good enough.' > > 3) This one: > drivers/gpu/drm/pl111/pl111_drv.c > is also very similar, but can sometimes use a dedicated > RAM memory for allocations using CMA, does that make > it not a candidate? Thanks, I think I simply missed pl111. Best regards Thomas > > They aren't much different in how they work from the TVE200. > > Yours, > Linus Walleij
On Tue, Mar 7, 2023 at 9:55 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > Am 06.03.23 um 23:19 schrieb Linus Walleij: > > 2) Why isn't this DRM driver changed? > > drivers/gpu/drm/mcde/mcde_drv.c > > AFAICT it also uses GEM buffers in system memory. > > This driver requires damage handling > https://elixir.bootlin.com/linux/v6.2/source/drivers/gpu/drm/mcde/mcde_drv.c#L97 > > for which we have to call the framebuffer's dirty callback Oh that one is on me ... I no longer remember exactly why I used drm_gem_fb_create_with_dirty() but I think it was because I had the ambition that the driver would only send out updates to DSI command displays whenever something changed, so as to minimize traffic. It turns out this ambition with command mode isn't working in practice because all the MCDE does is to create a continuous stream of DSI commands and while it is possible to send single frame updates with it, it's not working in practice. So we are just setting up continuous updates. We turn of the VBLANK IRQs a bit, but I guess the DRM framework does that for us when nothing goes on. I tested to replace this with drm_gem_fb_create and it works just fine. I'll send out a patch so you can make this change also to the MCDE driver. Yours, Linus Walleij