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

["Noralf Trønnes" <noralf@tronnes.org>]

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@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.

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.

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.


Noralf.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel