[RFC v2 1/8] drm/fb-helper: Add fb_deferred_io support

[Daniel Vetter]

On Mon, Apr 18, 2016 at 05:15:03PM +0200, Noralf Trønnes wrote:
> 
> Den 13.04.2016 13:09, skrev Daniel Vetter:
> >On Fri, Apr 08, 2016 at 07:05:03PM +0200, Noralf Trønnes wrote:
> >>This adds deferred io support if CONFIG_FB_DEFERRED_IO is enabled.
> >>Accumulated fbdev framebuffer changes are signaled using the callback
> >>(struct drm_framebuffer_funcs *)->dirty()
> >>
> >>The drm_fb_helper_sys_*() functions will accumulate changes and
> >>schedule fb_info.deferred_work _if_ fb_info.fbdefio is set.
> >>This worker is used by the deferred io mmap code to signal that it
> >>has been collecting page faults. The page faults and/or other changes
> >>are then merged into a drm_clip_rect and passed to the framebuffer
> >>dirty() function.
> >>
> >>The driver is responsible for setting up the fb_info.fbdefio structure
> >>and calling fb_deferred_io_init() using the provided callback:
> >>(struct fb_info *)->fbdefio->deferred_io = drm_fb_helper_deferred_io;
> >>
> >>Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
> >For this one it'd be awesome to throw patches for qxl/udl on top to remove
> >their own hand-rolled implementations. Just to maximize the testing
> >coverage of this new code. Should be doable to set up a qxl virtual
> >machine quickly, but even without that we should be able to pull it in
> >(since it's mostly just about removing code from these two drivers).
> 
> There are three fb_deferred_io users in drivers/gpu/drm: qxl, udl and
> vmwgfx.
> 
> drivers/gpu/drm/vmwgfx
> It doesn't use drm_fb_helper (uses the cfb_{fillrect,copyarea,imageblit}
> functions directly). This made me think that I should probably add
> fb_deferred_io support to drm_fb_helper_cfb_*() as well.

Yup, that one is special and can be ignored.

> drivers/gpu/drm/udl
> First of all it has had deferred io disabled by default since 2013
> (commit 677d23b). Secondly it handles damage directly in it's
> fb_{fillrect,copyarea,imageblit} functions, but defers to the next call if
> it is in atomic context. My patch always defers those function to a worker.
> The driver uses the drm_fb_helper_sys_* functions, so my patch would mess
> it up if someone would enable deferred io (module_param: fb_defio).
> So this driver isn't a good candidate for easy conversion also because it
> has different code paths for fbdev mmap damage and the other damages
> (although the code is similar). But it needs a patch to use the sys_*()
> functions directly.

Since it's disabled by default I'd just do a conversion. It should result
largely in deleting code and just using the fb helpers. What's special
with the mmap handling, and do we care?

> drivers/gpu/drm/qxl
> This one uses a worker as a buffer between the mmap damage tracking and
> fb_*() functions, and flushing of the changes. I'll give it a go.
> 
> Studying these in detail and looking at the git log was useful as it showed
> me that the (struct fb_ops *)->fb_*() functions can be called in interrupt
> context. This means that I need the irq version of spin_lock(). It also
> validates my choice to defer these calls using the mmap defer damage worker
> ((struct fb_info).deferred_work), because it ensures that the dirty() call
> will always run in process context.

Yeah, mostly I'd like to get qxl&udl converted so that all the lessons
learned in those implementations aren't lost. Great work digging out those
details ;-)

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch