| Message ID | 1461530942-22485-5-git-send-email-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Apr 24, 2016 at 10:48:58PM +0200, Noralf Trønnes wrote: > This adds deferred io support if CONFIG_FB_DEFERRED_IO is enabled. > The fbdev framebuffer changes are flushed using the callback > (struct drm_framebuffer *)->funcs->dirty() by a dedicated worker > ensuring that it always runs in process context. > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > > Changes since v1: > - Use a dedicated worker to run the framebuffer flushing like qxl does > - Add parameter descriptions to drm_fb_helper_deferred_io > > drivers/gpu/drm/drm_fb_helper.c | 127 +++++++++++++++++++++++++++++++++++++++- > include/drm/drm_fb_helper.h | 17 ++++++ > 2 files changed, 143 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 855108e..46ee6f8 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -40,6 +40,7 @@ > #include <drm/drm_crtc_helper.h> > #include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.h> > +#include <drm/drm_rect.h> > > static bool drm_fbdev_emulation = true; > module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600); > @@ -48,6 +49,10 @@ MODULE_PARM_DESC(fbdev_emulation, > > static LIST_HEAD(kernel_fb_helper_list); > > +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper); > +static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, > + u32 width, u32 height); > + > /** > * DOC: fbdev helpers > * > @@ -84,6 +89,16 @@ static LIST_HEAD(kernel_fb_helper_list); > * and set up an initial configuration using the detected hardware, drivers > * should call drm_fb_helper_single_add_all_connectors() followed by > * drm_fb_helper_initial_config(). > + * > + * If CONFIG_FB_DEFERRED_IO is enabled and > + * (struct drm_framebuffer *)->funcs->dirty is set, the > + * drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit} functions > + * will accumulate changes and schedule (struct fb_helper).dirty_work to run > + * right away. This worker then calls the dirty() function ensuring that it > + * will always run in process context since the fb_*() function could be > + * running in atomic context. If drm_fb_helper_deferred_io() is used as the > + * deferred_io callback it will also schedule dirty_work with the damage > + * collected from the mmap page writes. One thing to consider (and personally I don't care either way) is whether we shouldn't just select CONFIG_FB_DEFERRED_IO if the fbdev helpers are enabled. Pushing that out to drivers is imo a bit fragile. But like I said I'm ok with either way. > */ > > /** > @@ -401,11 +416,14 @@ backoff: > static int restore_fbdev_mode(struct drm_fb_helper *fb_helper) > { > struct drm_device *dev = fb_helper->dev; > + struct fb_info *info = fb_helper->fbdev; > struct drm_plane *plane; > int i; > > drm_warn_on_modeset_not_all_locked(dev); > > + drm_fb_helper_dirty(info, 0, 0, info->var.xres, info->var.yres); Why is this needed? If you do a modeset (or pageflip or whatever) drivers are supposed to re-upload the entire screen. We've talked about adding a dirty rectangle to atomic to allow userspace to optimize this, but there should _never_ be a need to do a dirtyfb call around a modeset. Probably just a driver bug in your panel drm drivers? With the above line removed: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + > if (fb_helper->atomic) > return restore_fbdev_mode_atomic(fb_helper); > > @@ -650,6 +668,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, > const struct drm_fb_helper_funcs *funcs) > { > INIT_LIST_HEAD(&helper->kernel_fb_list); > + drm_fb_helper_dirty_init(helper); > helper->funcs = funcs; > helper->dev = dev; > } > @@ -834,6 +853,93 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper) > } > EXPORT_SYMBOL(drm_fb_helper_unlink_fbi); > > +#ifdef CONFIG_FB_DEFERRED_IO > +static void drm_fb_helper_dirty_work(struct work_struct *work) > +{ > + struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper, > + dirty_work); > + struct drm_clip_rect clip; > + unsigned long flags; > + > + spin_lock_irqsave(&helper->dirty_lock, flags); > + clip = helper->dirty_clip; > + drm_clip_rect_reset(&helper->dirty_clip); > + spin_unlock_irqrestore(&helper->dirty_lock, flags); > + > + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip, 1); > +} > + > +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper) > +{ > + spin_lock_init(&helper->dirty_lock); > + INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); > +} > + > +static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, > + u32 width, u32 height) > +{ > + struct drm_fb_helper *helper = info->par; > + struct drm_clip_rect clip; > + unsigned long flags; > + > + if (!helper->fb->funcs->dirty) > + return; > + > + clip.x1 = x; > + clip.x2 = x + width; > + clip.y1 = y; > + clip.y2 = y + height; > + > + spin_lock_irqsave(&helper->dirty_lock, flags); > + drm_clip_rect_merge(&helper->dirty_clip, &clip, 1, 0, 0, 0); > + spin_unlock_irqrestore(&helper->dirty_lock, flags); > + > + schedule_work(&helper->dirty_work); > +} > + > +/** > + * drm_fb_helper_deferred_io() - fbdev deferred_io callback function > + * @info: fb_info struct pointer > + * @pagelist: list of dirty mmap framebuffer pages > + * > + * This function is used as the (struct fb_deferred_io *)->deferred_io > + * callback function for flushing the fbdev mmap writes. > + */ > +void drm_fb_helper_deferred_io(struct fb_info *info, > + struct list_head *pagelist) > +{ > + unsigned long start, end, min, max; > + struct page *page; > + u32 y1, y2; > + > + min = ULONG_MAX; > + max = 0; > + list_for_each_entry(page, pagelist, lru) { > + start = page->index << PAGE_SHIFT; > + end = start + PAGE_SIZE - 1; > + min = min(min, start); > + max = max(max, end); > + } > + > + if (min < max) { > + y1 = min / info->fix.line_length; > + y2 = min_t(u32, max / info->fix.line_length, info->var.yres); > + drm_fb_helper_dirty(info, 0, y1, info->var.xres, y2 - y1); > + } > +} > +EXPORT_SYMBOL(drm_fb_helper_deferred_io); > + > +#else > +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper) > +{ > +} > + > +static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, > + u32 width, u32 height) > +{ > +} > +#endif /* CONFIG_FB_DEFERRED_IO */ > + > /** > * drm_fb_helper_sys_read - wrapper around fb_sys_read > * @info: fb_info struct pointer > @@ -862,7 +968,14 @@ EXPORT_SYMBOL(drm_fb_helper_sys_read); > ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, > size_t count, loff_t *ppos) > { > - return fb_sys_write(info, buf, count, ppos); > + ssize_t ret; > + > + ret = fb_sys_write(info, buf, count, ppos); > + if (ret > 0) > + drm_fb_helper_dirty(info, 0, 0, info->var.xres, > + info->var.yres); > + > + return ret; > } > EXPORT_SYMBOL(drm_fb_helper_sys_write); > > @@ -877,6 +990,8 @@ void drm_fb_helper_sys_fillrect(struct fb_info *info, > const struct fb_fillrect *rect) > { > sys_fillrect(info, rect); > + drm_fb_helper_dirty(info, rect->dx, rect->dy, > + rect->width, rect->height); > } > EXPORT_SYMBOL(drm_fb_helper_sys_fillrect); > > @@ -891,6 +1006,8 @@ void drm_fb_helper_sys_copyarea(struct fb_info *info, > const struct fb_copyarea *area) > { > sys_copyarea(info, area); > + drm_fb_helper_dirty(info, area->dx, area->dy, > + area->width, area->height); > } > EXPORT_SYMBOL(drm_fb_helper_sys_copyarea); > > @@ -905,6 +1022,8 @@ void drm_fb_helper_sys_imageblit(struct fb_info *info, > const struct fb_image *image) > { > sys_imageblit(info, image); > + drm_fb_helper_dirty(info, image->dx, image->dy, > + image->width, image->height); > } > EXPORT_SYMBOL(drm_fb_helper_sys_imageblit); > > @@ -919,6 +1038,8 @@ void drm_fb_helper_cfb_fillrect(struct fb_info *info, > const struct fb_fillrect *rect) > { > cfb_fillrect(info, rect); > + drm_fb_helper_dirty(info, rect->dx, rect->dy, > + rect->width, rect->height); > } > EXPORT_SYMBOL(drm_fb_helper_cfb_fillrect); > > @@ -933,6 +1054,8 @@ void drm_fb_helper_cfb_copyarea(struct fb_info *info, > const struct fb_copyarea *area) > { > cfb_copyarea(info, area); > + drm_fb_helper_dirty(info, area->dx, area->dy, > + area->width, area->height); > } > EXPORT_SYMBOL(drm_fb_helper_cfb_copyarea); > > @@ -947,6 +1070,8 @@ void drm_fb_helper_cfb_imageblit(struct fb_info *info, > const struct fb_image *image) > { > cfb_imageblit(info, image); > + drm_fb_helper_dirty(info, image->dx, image->dy, > + image->width, image->height); > } > EXPORT_SYMBOL(drm_fb_helper_cfb_imageblit); > > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h > index 062723b..dde825c 100644 > --- a/include/drm/drm_fb_helper.h > +++ b/include/drm/drm_fb_helper.h > @@ -172,6 +172,10 @@ struct drm_fb_helper_connector { > * @funcs: driver callbacks for fb helper > * @fbdev: emulated fbdev device info struct > * @pseudo_palette: fake palette of 16 colors > + * @dirty_clip: clip rectangle used with deferred_io to accumulate damage to > + * the screen buffer > + * @dirty_lock: spinlock protecting @dirty_clip > + * @dirty_work: worker used to flush the framebuffer > * > * This is the main structure used by the fbdev helpers. Drivers supporting > * fbdev emulation should embedded this into their overall driver structure. > @@ -189,6 +193,11 @@ struct drm_fb_helper { > const struct drm_fb_helper_funcs *funcs; > struct fb_info *fbdev; > u32 pseudo_palette[17]; > +#ifdef CONFIG_FB_DEFERRED_IO > + struct drm_clip_rect dirty_clip; > + spinlock_t dirty_lock; > + struct work_struct dirty_work; > +#endif > > /** > * @kernel_fb_list: > @@ -245,6 +254,9 @@ void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch, > > void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper); > > +void drm_fb_helper_deferred_io(struct fb_info *info, > + struct list_head *pagelist); > + > ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, > size_t count, loff_t *ppos); > ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, > @@ -368,6 +380,11 @@ static inline void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper) > { > } > > +static inline void drm_fb_helper_deferred_io(struct fb_info *info, > + struct list_head *pagelist) > +{ > +} > + > static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info, > char __user *buf, size_t count, > loff_t *ppos) > -- > 2.2.2 >
Den 25.04.2016 11:09, skrev Daniel Vetter: > On Sun, Apr 24, 2016 at 10:48:58PM +0200, Noralf Trønnes wrote: >> This adds deferred io support if CONFIG_FB_DEFERRED_IO is enabled. >> The fbdev framebuffer changes are flushed using the callback >> (struct drm_framebuffer *)->funcs->dirty() by a dedicated worker >> ensuring that it always runs in process context. >> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> --- >> >> Changes since v1: >> - Use a dedicated worker to run the framebuffer flushing like qxl does >> - Add parameter descriptions to drm_fb_helper_deferred_io >> >> drivers/gpu/drm/drm_fb_helper.c | 127 +++++++++++++++++++++++++++++++++++++++- >> include/drm/drm_fb_helper.h | 17 ++++++ >> 2 files changed, 143 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >> index 855108e..46ee6f8 100644 >> --- a/drivers/gpu/drm/drm_fb_helper.c >> +++ b/drivers/gpu/drm/drm_fb_helper.c >> @@ -40,6 +40,7 @@ >> #include <drm/drm_crtc_helper.h> >> #include <drm/drm_atomic.h> >> #include <drm/drm_atomic_helper.h> >> +#include <drm/drm_rect.h> >> >> static bool drm_fbdev_emulation = true; >> module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600); >> @@ -48,6 +49,10 @@ MODULE_PARM_DESC(fbdev_emulation, >> >> static LIST_HEAD(kernel_fb_helper_list); >> >> +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper); >> +static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, >> + u32 width, u32 height); >> + >> /** >> * DOC: fbdev helpers >> * >> @@ -84,6 +89,16 @@ static LIST_HEAD(kernel_fb_helper_list); >> * and set up an initial configuration using the detected hardware, drivers >> * should call drm_fb_helper_single_add_all_connectors() followed by >> * drm_fb_helper_initial_config(). >> + * >> + * If CONFIG_FB_DEFERRED_IO is enabled and >> + * (struct drm_framebuffer *)->funcs->dirty is set, the >> + * drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit} functions >> + * will accumulate changes and schedule (struct fb_helper).dirty_work to run >> + * right away. This worker then calls the dirty() function ensuring that it >> + * will always run in process context since the fb_*() function could be >> + * running in atomic context. If drm_fb_helper_deferred_io() is used as the >> + * deferred_io callback it will also schedule dirty_work with the damage >> + * collected from the mmap page writes. > One thing to consider (and personally I don't care either way) is whether > we shouldn't just select CONFIG_FB_DEFERRED_IO if the fbdev helpers are > enabled. Pushing that out to drivers is imo a bit fragile. > > But like I said I'm ok with either way. My concern was adding code and data that only a few drivers would actually use. But of course there's the tradeoff with complexity. I use this to enable it: select FB_DEFERRED_IO if DRM_KMS_FB_HELPER I guess the maintainer has to make this choice between size and complexity :-) I can enable it by default if you want, drm is both huge and complex so I don't know what's best. As a sidenote, I have also put all the fbdev code in a file of it's own to make it simple with regards to the DRM_FBDEV_EMULATION user option: tinydrm-$(CONFIG_DRM_KMS_FB_HELPER) += tinydrm-fbdev.o >> */ >> >> /** >> @@ -401,11 +416,14 @@ backoff: >> static int restore_fbdev_mode(struct drm_fb_helper *fb_helper) >> { >> struct drm_device *dev = fb_helper->dev; >> + struct fb_info *info = fb_helper->fbdev; >> struct drm_plane *plane; >> int i; >> >> drm_warn_on_modeset_not_all_locked(dev); >> >> + drm_fb_helper_dirty(info, 0, 0, info->var.xres, info->var.yres); > Why is this needed? If you do a modeset (or pageflip or whatever) drivers > are supposed to re-upload the entire screen. We've talked about adding a > dirty rectangle to atomic to allow userspace to optimize this, but there > should _never_ be a need to do a dirtyfb call around a modeset. Probably > just a driver bug in your panel drm drivers? Ok, in tinydrm I now set a flag in &drm_simple_display_pipe_funcs ->plane_update to indicate that the next dirty() should do the whole framebuffer which seems to work fine. Should I actually perform the update as well? If so I would need to add a worker in tinydrm to do that. Noralf. > With the above line removed: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >> + >> if (fb_helper->atomic) >> return restore_fbdev_mode_atomic(fb_helper); >> >> @@ -650,6 +668,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, >> const struct drm_fb_helper_funcs *funcs) >> { >> INIT_LIST_HEAD(&helper->kernel_fb_list); >> + drm_fb_helper_dirty_init(helper); >> helper->funcs = funcs; >> helper->dev = dev; >> } >> @@ -834,6 +853,93 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper) >> } >> EXPORT_SYMBOL(drm_fb_helper_unlink_fbi); >> >> +#ifdef CONFIG_FB_DEFERRED_IO >> +static void drm_fb_helper_dirty_work(struct work_struct *work) >> +{ >> + struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper, >> + dirty_work); >> + struct drm_clip_rect clip; >> + unsigned long flags; >> + >> + spin_lock_irqsave(&helper->dirty_lock, flags); >> + clip = helper->dirty_clip; >> + drm_clip_rect_reset(&helper->dirty_clip); >> + spin_unlock_irqrestore(&helper->dirty_lock, flags); >> + >> + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip, 1); >> +} >> + >> +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper) >> +{ >> + spin_lock_init(&helper->dirty_lock); >> + INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); >> +} >> + >> +static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, >> + u32 width, u32 height) >> +{ >> + struct drm_fb_helper *helper = info->par; >> + struct drm_clip_rect clip; >> + unsigned long flags; >> + >> + if (!helper->fb->funcs->dirty) >> + return; >> + >> + clip.x1 = x; >> + clip.x2 = x + width; >> + clip.y1 = y; >> + clip.y2 = y + height; >> + >> + spin_lock_irqsave(&helper->dirty_lock, flags); >> + drm_clip_rect_merge(&helper->dirty_clip, &clip, 1, 0, 0, 0); >> + spin_unlock_irqrestore(&helper->dirty_lock, flags); >> + >> + schedule_work(&helper->dirty_work); >> +} >> + >> +/** >> + * drm_fb_helper_deferred_io() - fbdev deferred_io callback function >> + * @info: fb_info struct pointer >> + * @pagelist: list of dirty mmap framebuffer pages >> + * >> + * This function is used as the (struct fb_deferred_io *)->deferred_io >> + * callback function for flushing the fbdev mmap writes. >> + */ >> +void drm_fb_helper_deferred_io(struct fb_info *info, >> + struct list_head *pagelist) >> +{ >> + unsigned long start, end, min, max; >> + struct page *page; >> + u32 y1, y2; >> + >> + min = ULONG_MAX; >> + max = 0; >> + list_for_each_entry(page, pagelist, lru) { >> + start = page->index << PAGE_SHIFT; >> + end = start + PAGE_SIZE - 1; >> + min = min(min, start); >> + max = max(max, end); >> + } >> + >> + if (min < max) { >> + y1 = min / info->fix.line_length; >> + y2 = min_t(u32, max / info->fix.line_length, info->var.yres); >> + drm_fb_helper_dirty(info, 0, y1, info->var.xres, y2 - y1); >> + } >> +} >> +EXPORT_SYMBOL(drm_fb_helper_deferred_io); >> + >> +#else >> +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper) >> +{ >> +} >> + >> +static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, >> + u32 width, u32 height) >> +{ >> +} >> +#endif /* CONFIG_FB_DEFERRED_IO */ >> + >> /** >> * drm_fb_helper_sys_read - wrapper around fb_sys_read >> * @info: fb_info struct pointer >> @@ -862,7 +968,14 @@ EXPORT_SYMBOL(drm_fb_helper_sys_read); >> ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, >> size_t count, loff_t *ppos) >> { >> - return fb_sys_write(info, buf, count, ppos); >> + ssize_t ret; >> + >> + ret = fb_sys_write(info, buf, count, ppos); >> + if (ret > 0) >> + drm_fb_helper_dirty(info, 0, 0, info->var.xres, >> + info->var.yres); >> + >> + return ret; >> } >> EXPORT_SYMBOL(drm_fb_helper_sys_write); >> >> @@ -877,6 +990,8 @@ void drm_fb_helper_sys_fillrect(struct fb_info *info, >> const struct fb_fillrect *rect) >> { >> sys_fillrect(info, rect); >> + drm_fb_helper_dirty(info, rect->dx, rect->dy, >> + rect->width, rect->height); >> } >> EXPORT_SYMBOL(drm_fb_helper_sys_fillrect); >> >> @@ -891,6 +1006,8 @@ void drm_fb_helper_sys_copyarea(struct fb_info *info, >> const struct fb_copyarea *area) >> { >> sys_copyarea(info, area); >> + drm_fb_helper_dirty(info, area->dx, area->dy, >> + area->width, area->height); >> } >> EXPORT_SYMBOL(drm_fb_helper_sys_copyarea); >> >> @@ -905,6 +1022,8 @@ void drm_fb_helper_sys_imageblit(struct fb_info *info, >> const struct fb_image *image) >> { >> sys_imageblit(info, image); >> + drm_fb_helper_dirty(info, image->dx, image->dy, >> + image->width, image->height); >> } >> EXPORT_SYMBOL(drm_fb_helper_sys_imageblit); >> >> @@ -919,6 +1038,8 @@ void drm_fb_helper_cfb_fillrect(struct fb_info *info, >> const struct fb_fillrect *rect) >> { >> cfb_fillrect(info, rect); >> + drm_fb_helper_dirty(info, rect->dx, rect->dy, >> + rect->width, rect->height); >> } >> EXPORT_SYMBOL(drm_fb_helper_cfb_fillrect); >> >> @@ -933,6 +1054,8 @@ void drm_fb_helper_cfb_copyarea(struct fb_info *info, >> const struct fb_copyarea *area) >> { >> cfb_copyarea(info, area); >> + drm_fb_helper_dirty(info, area->dx, area->dy, >> + area->width, area->height); >> } >> EXPORT_SYMBOL(drm_fb_helper_cfb_copyarea); >> >> @@ -947,6 +1070,8 @@ void drm_fb_helper_cfb_imageblit(struct fb_info *info, >> const struct fb_image *image) >> { >> cfb_imageblit(info, image); >> + drm_fb_helper_dirty(info, image->dx, image->dy, >> + image->width, image->height); >> } >> EXPORT_SYMBOL(drm_fb_helper_cfb_imageblit); >> >> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h >> index 062723b..dde825c 100644 >> --- a/include/drm/drm_fb_helper.h >> +++ b/include/drm/drm_fb_helper.h >> @@ -172,6 +172,10 @@ struct drm_fb_helper_connector { >> * @funcs: driver callbacks for fb helper >> * @fbdev: emulated fbdev device info struct >> * @pseudo_palette: fake palette of 16 colors >> + * @dirty_clip: clip rectangle used with deferred_io to accumulate damage to >> + * the screen buffer >> + * @dirty_lock: spinlock protecting @dirty_clip >> + * @dirty_work: worker used to flush the framebuffer >> * >> * This is the main structure used by the fbdev helpers. Drivers supporting >> * fbdev emulation should embedded this into their overall driver structure. >> @@ -189,6 +193,11 @@ struct drm_fb_helper { >> const struct drm_fb_helper_funcs *funcs; >> struct fb_info *fbdev; >> u32 pseudo_palette[17]; >> +#ifdef CONFIG_FB_DEFERRED_IO >> + struct drm_clip_rect dirty_clip; >> + spinlock_t dirty_lock; >> + struct work_struct dirty_work; >> +#endif >> >> /** >> * @kernel_fb_list: >> @@ -245,6 +254,9 @@ void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch, >> >> void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper); >> >> +void drm_fb_helper_deferred_io(struct fb_info *info, >> + struct list_head *pagelist); >> + >> ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, >> size_t count, loff_t *ppos); >> ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, >> @@ -368,6 +380,11 @@ static inline void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper) >> { >> } >> >> +static inline void drm_fb_helper_deferred_io(struct fb_info *info, >> + struct list_head *pagelist) >> +{ >> +} >> + >> static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info, >> char __user *buf, size_t count, >> loff_t *ppos) >> -- >> 2.2.2 >>
On Tue, Apr 26, 2016 at 06:24:54PM +0200, Noralf Trønnes wrote: > > Den 25.04.2016 11:09, skrev Daniel Vetter: > >On Sun, Apr 24, 2016 at 10:48:58PM +0200, Noralf Trønnes wrote: > >>This adds deferred io support if CONFIG_FB_DEFERRED_IO is enabled. > >>The fbdev framebuffer changes are flushed using the callback > >>(struct drm_framebuffer *)->funcs->dirty() by a dedicated worker > >>ensuring that it always runs in process context. > >> > >>Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > >>--- > >> > >>Changes since v1: > >>- Use a dedicated worker to run the framebuffer flushing like qxl does > >>- Add parameter descriptions to drm_fb_helper_deferred_io > >> > >> drivers/gpu/drm/drm_fb_helper.c | 127 +++++++++++++++++++++++++++++++++++++++- > >> include/drm/drm_fb_helper.h | 17 ++++++ > >> 2 files changed, 143 insertions(+), 1 deletion(-) > >> > >>diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > >>index 855108e..46ee6f8 100644 > >>--- a/drivers/gpu/drm/drm_fb_helper.c > >>+++ b/drivers/gpu/drm/drm_fb_helper.c > >>@@ -40,6 +40,7 @@ > >> #include <drm/drm_crtc_helper.h> > >> #include <drm/drm_atomic.h> > >> #include <drm/drm_atomic_helper.h> > >>+#include <drm/drm_rect.h> > >> > >> static bool drm_fbdev_emulation = true; > >> module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600); > >>@@ -48,6 +49,10 @@ MODULE_PARM_DESC(fbdev_emulation, > >> > >> static LIST_HEAD(kernel_fb_helper_list); > >> > >>+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper); > >>+static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, > >>+ u32 width, u32 height); > >>+ > >> /** > >> * DOC: fbdev helpers > >> * > >>@@ -84,6 +89,16 @@ static LIST_HEAD(kernel_fb_helper_list); > >> * and set up an initial configuration using the detected hardware, drivers > >> * should call drm_fb_helper_single_add_all_connectors() followed by > >> * drm_fb_helper_initial_config(). > >>+ * > >>+ * If CONFIG_FB_DEFERRED_IO is enabled and > >>+ * (struct drm_framebuffer *)->funcs->dirty is set, the > >>+ * drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit} functions > >>+ * will accumulate changes and schedule (struct fb_helper).dirty_work to run > >>+ * right away. This worker then calls the dirty() function ensuring that it > >>+ * will always run in process context since the fb_*() function could be > >>+ * running in atomic context. If drm_fb_helper_deferred_io() is used as the > >>+ * deferred_io callback it will also schedule dirty_work with the damage > >>+ * collected from the mmap page writes. > >One thing to consider (and personally I don't care either way) is whether > >we shouldn't just select CONFIG_FB_DEFERRED_IO if the fbdev helpers are > >enabled. Pushing that out to drivers is imo a bit fragile. > > > >But like I said I'm ok with either way. > > My concern was adding code and data that only a few drivers would > actually use. But of course there's the tradeoff with complexity. > I use this to enable it: > select FB_DEFERRED_IO if DRM_KMS_FB_HELPER > > I guess the maintainer has to make this choice between size and complexity > :-) > I can enable it by default if you want, drm is both huge and complex so I > don't know what's best. > > As a sidenote, I have also put all the fbdev code in a file of it's own to > make it simple with regards to the DRM_FBDEV_EMULATION user option: > tinydrm-$(CONFIG_DRM_KMS_FB_HELPER) += tinydrm-fbdev.o Ok, if you ask maintainers then please nuke the #ifdef from .c files. If you select CONFIG_DRM_KMS_FB_HELPER, then you get hdmi, edid, dp aux, dp mst and whatever else helpers, even if you don't need them. Adding 3 functions for defio when you select fbdev helpers and maybe don't need them is totally harmless. And removing the #ifdef will look so much better ;-) > > >> */ > >> > >> /** > >>@@ -401,11 +416,14 @@ backoff: > >> static int restore_fbdev_mode(struct drm_fb_helper *fb_helper) > >> { > >> struct drm_device *dev = fb_helper->dev; > >>+ struct fb_info *info = fb_helper->fbdev; > >> struct drm_plane *plane; > >> int i; > >> > >> drm_warn_on_modeset_not_all_locked(dev); > >> > >>+ drm_fb_helper_dirty(info, 0, 0, info->var.xres, info->var.yres); > >Why is this needed? If you do a modeset (or pageflip or whatever) drivers > >are supposed to re-upload the entire screen. We've talked about adding a > >dirty rectangle to atomic to allow userspace to optimize this, but there > >should _never_ be a need to do a dirtyfb call around a modeset. Probably > >just a driver bug in your panel drm drivers? > > Ok, in tinydrm I now set a flag in &drm_simple_display_pipe_funcs > ->plane_update to indicate that the next dirty() should do the whole > framebuffer which seems to work fine. > Should I actually perform the update as well? > If so I would need to add a worker in tinydrm to do that. Yes, plane update should always do a full update. Not sure how you get away with delaying that to ->dirty, maybe modesetting isn't double-buffering when you don't have a GL that could do glamour. ->dirty is _only_ for frontbuffer rendering, not for page-flipping to an entirely new buffer. In short if someone calls ->dirty on a buffer which is currently not being displayed than a) they're silly b) drivers should treat it as a no-op. Maybe we need a helper to do that ... -Daniel
Den 26.04.2016 19:19, skrev Daniel Vetter: > On Tue, Apr 26, 2016 at 06:24:54PM +0200, Noralf Trønnes wrote: >> Den 25.04.2016 11:09, skrev Daniel Vetter: >>> On Sun, Apr 24, 2016 at 10:48:58PM +0200, Noralf Trønnes wrote: >>>> This adds deferred io support if CONFIG_FB_DEFERRED_IO is enabled. >>>> The fbdev framebuffer changes are flushed using the callback >>>> (struct drm_framebuffer *)->funcs->dirty() by a dedicated worker >>>> ensuring that it always runs in process context. >>>> >>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >>>> --- >>>> >>>> Changes since v1: >>>> - Use a dedicated worker to run the framebuffer flushing like qxl does >>>> - Add parameter descriptions to drm_fb_helper_deferred_io >>>> >>>> drivers/gpu/drm/drm_fb_helper.c | 127 +++++++++++++++++++++++++++++++++++++++- >>>> include/drm/drm_fb_helper.h | 17 ++++++ >>>> 2 files changed, 143 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >>>> index 855108e..46ee6f8 100644 >>>> --- a/drivers/gpu/drm/drm_fb_helper.c >>>> +++ b/drivers/gpu/drm/drm_fb_helper.c >>>> @@ -40,6 +40,7 @@ >>>> #include <drm/drm_crtc_helper.h> >>>> #include <drm/drm_atomic.h> >>>> #include <drm/drm_atomic_helper.h> >>>> +#include <drm/drm_rect.h> >>>> >>>> static bool drm_fbdev_emulation = true; >>>> module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600); >>>> @@ -48,6 +49,10 @@ MODULE_PARM_DESC(fbdev_emulation, >>>> >>>> static LIST_HEAD(kernel_fb_helper_list); >>>> >>>> +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper); >>>> +static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, >>>> + u32 width, u32 height); >>>> + >>>> /** >>>> * DOC: fbdev helpers >>>> * >>>> @@ -84,6 +89,16 @@ static LIST_HEAD(kernel_fb_helper_list); >>>> * and set up an initial configuration using the detected hardware, drivers >>>> * should call drm_fb_helper_single_add_all_connectors() followed by >>>> * drm_fb_helper_initial_config(). >>>> + * >>>> + * If CONFIG_FB_DEFERRED_IO is enabled and >>>> + * (struct drm_framebuffer *)->funcs->dirty is set, the >>>> + * drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit} functions >>>> + * will accumulate changes and schedule (struct fb_helper).dirty_work to run >>>> + * right away. This worker then calls the dirty() function ensuring that it >>>> + * will always run in process context since the fb_*() function could be >>>> + * running in atomic context. If drm_fb_helper_deferred_io() is used as the >>>> + * deferred_io callback it will also schedule dirty_work with the damage >>>> + * collected from the mmap page writes. >>> One thing to consider (and personally I don't care either way) is whether >>> we shouldn't just select CONFIG_FB_DEFERRED_IO if the fbdev helpers are >>> enabled. Pushing that out to drivers is imo a bit fragile. >>> >>> But like I said I'm ok with either way. >> My concern was adding code and data that only a few drivers would >> actually use. But of course there's the tradeoff with complexity. >> I use this to enable it: >> select FB_DEFERRED_IO if DRM_KMS_FB_HELPER >> >> I guess the maintainer has to make this choice between size and complexity >> :-) >> I can enable it by default if you want, drm is both huge and complex so I >> don't know what's best. >> >> As a sidenote, I have also put all the fbdev code in a file of it's own to >> make it simple with regards to the DRM_FBDEV_EMULATION user option: >> tinydrm-$(CONFIG_DRM_KMS_FB_HELPER) += tinydrm-fbdev.o > Ok, if you ask maintainers then please nuke the #ifdef from .c files. If > you select CONFIG_DRM_KMS_FB_HELPER, then you get hdmi, edid, dp aux, dp > mst and whatever else helpers, even if you don't need them. Adding 3 > functions for defio when you select fbdev helpers and maybe don't need > them is totally harmless. And removing the #ifdef will look so much better > ;-) Will do :-) Kernel development is just my hobby so I'm not well versed in all of this. >>>> */ >>>> >>>> /** >>>> @@ -401,11 +416,14 @@ backoff: >>>> static int restore_fbdev_mode(struct drm_fb_helper *fb_helper) >>>> { >>>> struct drm_device *dev = fb_helper->dev; >>>> + struct fb_info *info = fb_helper->fbdev; >>>> struct drm_plane *plane; >>>> int i; >>>> >>>> drm_warn_on_modeset_not_all_locked(dev); >>>> >>>> + drm_fb_helper_dirty(info, 0, 0, info->var.xres, info->var.yres); >>> Why is this needed? If you do a modeset (or pageflip or whatever) drivers >>> are supposed to re-upload the entire screen. We've talked about adding a >>> dirty rectangle to atomic to allow userspace to optimize this, but there >>> should _never_ be a need to do a dirtyfb call around a modeset. Probably >>> just a driver bug in your panel drm drivers? >> Ok, in tinydrm I now set a flag in &drm_simple_display_pipe_funcs >> ->plane_update to indicate that the next dirty() should do the whole >> framebuffer which seems to work fine. >> Should I actually perform the update as well? >> If so I would need to add a worker in tinydrm to do that. > Yes, plane update should always do a full update. Not sure how you get > away with delaying that to ->dirty, maybe modesetting isn't > double-buffering when you don't have a GL that could do glamour. > > ->dirty is _only_ for frontbuffer rendering, not for page-flipping to an > entirely new buffer. In short if someone calls ->dirty on a buffer which > is currently not being displayed than a) they're silly b) drivers should > treat it as a no-op. Maybe we need a helper to do that ... > -Daniel drm_fb_helper will call dirty() as long as there's fbdev activity, so the driver needs to take that into account. For instance fbcon with a blinking cursor will trigger calls even if a buffer has been set up on the drm side. tinydrm checks the fb against the fb set on the plane and if it differs it's a no-op. Noralf.
On Wed, Apr 27, 2016 at 11:45:31AM +0200, Noralf Trønnes wrote: > > Den 26.04.2016 19:19, skrev Daniel Vetter: > >On Tue, Apr 26, 2016 at 06:24:54PM +0200, Noralf Trønnes wrote: > >>Den 25.04.2016 11:09, skrev Daniel Vetter: > >>>On Sun, Apr 24, 2016 at 10:48:58PM +0200, Noralf Trønnes wrote: > >>>>This adds deferred io support if CONFIG_FB_DEFERRED_IO is enabled. > >>>>The fbdev framebuffer changes are flushed using the callback > >>>>(struct drm_framebuffer *)->funcs->dirty() by a dedicated worker > >>>>ensuring that it always runs in process context. > >>>> > >>>>Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > >>>>--- > >>>> > >>>>Changes since v1: > >>>>- Use a dedicated worker to run the framebuffer flushing like qxl does > >>>>- Add parameter descriptions to drm_fb_helper_deferred_io > >>>> > >>>> drivers/gpu/drm/drm_fb_helper.c | 127 +++++++++++++++++++++++++++++++++++++++- > >>>> include/drm/drm_fb_helper.h | 17 ++++++ > >>>> 2 files changed, 143 insertions(+), 1 deletion(-) > >>>> > >>>>diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > >>>>index 855108e..46ee6f8 100644 > >>>>--- a/drivers/gpu/drm/drm_fb_helper.c > >>>>+++ b/drivers/gpu/drm/drm_fb_helper.c > >>>>@@ -40,6 +40,7 @@ > >>>> #include <drm/drm_crtc_helper.h> > >>>> #include <drm/drm_atomic.h> > >>>> #include <drm/drm_atomic_helper.h> > >>>>+#include <drm/drm_rect.h> > >>>> > >>>> static bool drm_fbdev_emulation = true; > >>>> module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600); > >>>>@@ -48,6 +49,10 @@ MODULE_PARM_DESC(fbdev_emulation, > >>>> > >>>> static LIST_HEAD(kernel_fb_helper_list); > >>>> > >>>>+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper); > >>>>+static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, > >>>>+ u32 width, u32 height); > >>>>+ > >>>> /** > >>>> * DOC: fbdev helpers > >>>> * > >>>>@@ -84,6 +89,16 @@ static LIST_HEAD(kernel_fb_helper_list); > >>>> * and set up an initial configuration using the detected hardware, drivers > >>>> * should call drm_fb_helper_single_add_all_connectors() followed by > >>>> * drm_fb_helper_initial_config(). > >>>>+ * > >>>>+ * If CONFIG_FB_DEFERRED_IO is enabled and > >>>>+ * (struct drm_framebuffer *)->funcs->dirty is set, the > >>>>+ * drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit} functions > >>>>+ * will accumulate changes and schedule (struct fb_helper).dirty_work to run > >>>>+ * right away. This worker then calls the dirty() function ensuring that it > >>>>+ * will always run in process context since the fb_*() function could be > >>>>+ * running in atomic context. If drm_fb_helper_deferred_io() is used as the > >>>>+ * deferred_io callback it will also schedule dirty_work with the damage > >>>>+ * collected from the mmap page writes. > >>>One thing to consider (and personally I don't care either way) is whether > >>>we shouldn't just select CONFIG_FB_DEFERRED_IO if the fbdev helpers are > >>>enabled. Pushing that out to drivers is imo a bit fragile. > >>> > >>>But like I said I'm ok with either way. > >>My concern was adding code and data that only a few drivers would > >>actually use. But of course there's the tradeoff with complexity. > >>I use this to enable it: > >> select FB_DEFERRED_IO if DRM_KMS_FB_HELPER > >> > >>I guess the maintainer has to make this choice between size and complexity > >>:-) > >>I can enable it by default if you want, drm is both huge and complex so I > >>don't know what's best. > >> > >>As a sidenote, I have also put all the fbdev code in a file of it's own to > >>make it simple with regards to the DRM_FBDEV_EMULATION user option: > >>tinydrm-$(CONFIG_DRM_KMS_FB_HELPER) += tinydrm-fbdev.o > >Ok, if you ask maintainers then please nuke the #ifdef from .c files. If > >you select CONFIG_DRM_KMS_FB_HELPER, then you get hdmi, edid, dp aux, dp > >mst and whatever else helpers, even if you don't need them. Adding 3 > >functions for defio when you select fbdev helpers and maybe don't need > >them is totally harmless. And removing the #ifdef will look so much better > >;-) > > Will do :-) > Kernel development is just my hobby so I'm not well versed in all of this. You're doing great tbh! > >>>> */ > >>>> > >>>> /** > >>>>@@ -401,11 +416,14 @@ backoff: > >>>> static int restore_fbdev_mode(struct drm_fb_helper *fb_helper) > >>>> { > >>>> struct drm_device *dev = fb_helper->dev; > >>>>+ struct fb_info *info = fb_helper->fbdev; > >>>> struct drm_plane *plane; > >>>> int i; > >>>> > >>>> drm_warn_on_modeset_not_all_locked(dev); > >>>> > >>>>+ drm_fb_helper_dirty(info, 0, 0, info->var.xres, info->var.yres); > >>>Why is this needed? If you do a modeset (or pageflip or whatever) drivers > >>>are supposed to re-upload the entire screen. We've talked about adding a > >>>dirty rectangle to atomic to allow userspace to optimize this, but there > >>>should _never_ be a need to do a dirtyfb call around a modeset. Probably > >>>just a driver bug in your panel drm drivers? > >>Ok, in tinydrm I now set a flag in &drm_simple_display_pipe_funcs > >>->plane_update to indicate that the next dirty() should do the whole > >>framebuffer which seems to work fine. > >>Should I actually perform the update as well? > >>If so I would need to add a worker in tinydrm to do that. > >Yes, plane update should always do a full update. Not sure how you get > >away with delaying that to ->dirty, maybe modesetting isn't > >double-buffering when you don't have a GL that could do glamour. > > > >->dirty is _only_ for frontbuffer rendering, not for page-flipping to an > >entirely new buffer. In short if someone calls ->dirty on a buffer which > >is currently not being displayed than a) they're silly b) drivers should > >treat it as a no-op. Maybe we need a helper to do that ... > >-Daniel > > drm_fb_helper will call dirty() as long as there's fbdev activity, so the > driver needs to take that into account. For instance fbcon with a blinking > cursor will trigger calls even if a buffer has been set up on the drm side. > tinydrm checks the fb against the fb set on the plane and if it differs > it's a no-op. Was really just an idea to make drivers a bit simpler, since pretty much all of them we need to do this check. But with a grand total of just 3 (4 with tinydrm) implementing a non-trivial dirty callback that's not really worth it I think. -Daniel
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 855108e..46ee6f8 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -40,6 +40,7 @@ #include <drm/drm_crtc_helper.h> #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_rect.h> static bool drm_fbdev_emulation = true; module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600); @@ -48,6 +49,10 @@ MODULE_PARM_DESC(fbdev_emulation, static LIST_HEAD(kernel_fb_helper_list); +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper); +static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, + u32 width, u32 height); + /** * DOC: fbdev helpers * @@ -84,6 +89,16 @@ static LIST_HEAD(kernel_fb_helper_list); * and set up an initial configuration using the detected hardware, drivers * should call drm_fb_helper_single_add_all_connectors() followed by * drm_fb_helper_initial_config(). + * + * If CONFIG_FB_DEFERRED_IO is enabled and + * (struct drm_framebuffer *)->funcs->dirty is set, the + * drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit} functions + * will accumulate changes and schedule (struct fb_helper).dirty_work to run + * right away. This worker then calls the dirty() function ensuring that it + * will always run in process context since the fb_*() function could be + * running in atomic context. If drm_fb_helper_deferred_io() is used as the + * deferred_io callback it will also schedule dirty_work with the damage + * collected from the mmap page writes. */ /** @@ -401,11 +416,14 @@ backoff: static int restore_fbdev_mode(struct drm_fb_helper *fb_helper) { struct drm_device *dev = fb_helper->dev; + struct fb_info *info = fb_helper->fbdev; struct drm_plane *plane; int i; drm_warn_on_modeset_not_all_locked(dev); + drm_fb_helper_dirty(info, 0, 0, info->var.xres, info->var.yres); + if (fb_helper->atomic) return restore_fbdev_mode_atomic(fb_helper); @@ -650,6 +668,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, const struct drm_fb_helper_funcs *funcs) { INIT_LIST_HEAD(&helper->kernel_fb_list); + drm_fb_helper_dirty_init(helper); helper->funcs = funcs; helper->dev = dev; } @@ -834,6 +853,93 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper) } EXPORT_SYMBOL(drm_fb_helper_unlink_fbi); +#ifdef CONFIG_FB_DEFERRED_IO +static void drm_fb_helper_dirty_work(struct work_struct *work) +{ + struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper, + dirty_work); + struct drm_clip_rect clip; + unsigned long flags; + + spin_lock_irqsave(&helper->dirty_lock, flags); + clip = helper->dirty_clip; + drm_clip_rect_reset(&helper->dirty_clip); + spin_unlock_irqrestore(&helper->dirty_lock, flags); + + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip, 1); +} + +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper) +{ + spin_lock_init(&helper->dirty_lock); + INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); +} + +static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, + u32 width, u32 height) +{ + struct drm_fb_helper *helper = info->par; + struct drm_clip_rect clip; + unsigned long flags; + + if (!helper->fb->funcs->dirty) + return; + + clip.x1 = x; + clip.x2 = x + width; + clip.y1 = y; + clip.y2 = y + height; + + spin_lock_irqsave(&helper->dirty_lock, flags); + drm_clip_rect_merge(&helper->dirty_clip, &clip, 1, 0, 0, 0); + spin_unlock_irqrestore(&helper->dirty_lock, flags); + + schedule_work(&helper->dirty_work); +} + +/** + * drm_fb_helper_deferred_io() - fbdev deferred_io callback function + * @info: fb_info struct pointer + * @pagelist: list of dirty mmap framebuffer pages + * + * This function is used as the (struct fb_deferred_io *)->deferred_io + * callback function for flushing the fbdev mmap writes. + */ +void drm_fb_helper_deferred_io(struct fb_info *info, + struct list_head *pagelist) +{ + unsigned long start, end, min, max; + struct page *page; + u32 y1, y2; + + min = ULONG_MAX; + max = 0; + list_for_each_entry(page, pagelist, lru) { + start = page->index << PAGE_SHIFT; + end = start + PAGE_SIZE - 1; + min = min(min, start); + max = max(max, end); + } + + if (min < max) { + y1 = min / info->fix.line_length; + y2 = min_t(u32, max / info->fix.line_length, info->var.yres); + drm_fb_helper_dirty(info, 0, y1, info->var.xres, y2 - y1); + } +} +EXPORT_SYMBOL(drm_fb_helper_deferred_io); + +#else +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper) +{ +} + +static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y, + u32 width, u32 height) +{ +} +#endif /* CONFIG_FB_DEFERRED_IO */ + /** * drm_fb_helper_sys_read - wrapper around fb_sys_read * @info: fb_info struct pointer @@ -862,7 +968,14 @@ EXPORT_SYMBOL(drm_fb_helper_sys_read); ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, size_t count, loff_t *ppos) { - return fb_sys_write(info, buf, count, ppos); + ssize_t ret; + + ret = fb_sys_write(info, buf, count, ppos); + if (ret > 0) + drm_fb_helper_dirty(info, 0, 0, info->var.xres, + info->var.yres); + + return ret; } EXPORT_SYMBOL(drm_fb_helper_sys_write); @@ -877,6 +990,8 @@ void drm_fb_helper_sys_fillrect(struct fb_info *info, const struct fb_fillrect *rect) { sys_fillrect(info, rect); + drm_fb_helper_dirty(info, rect->dx, rect->dy, + rect->width, rect->height); } EXPORT_SYMBOL(drm_fb_helper_sys_fillrect); @@ -891,6 +1006,8 @@ void drm_fb_helper_sys_copyarea(struct fb_info *info, const struct fb_copyarea *area) { sys_copyarea(info, area); + drm_fb_helper_dirty(info, area->dx, area->dy, + area->width, area->height); } EXPORT_SYMBOL(drm_fb_helper_sys_copyarea); @@ -905,6 +1022,8 @@ void drm_fb_helper_sys_imageblit(struct fb_info *info, const struct fb_image *image) { sys_imageblit(info, image); + drm_fb_helper_dirty(info, image->dx, image->dy, + image->width, image->height); } EXPORT_SYMBOL(drm_fb_helper_sys_imageblit); @@ -919,6 +1038,8 @@ void drm_fb_helper_cfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect) { cfb_fillrect(info, rect); + drm_fb_helper_dirty(info, rect->dx, rect->dy, + rect->width, rect->height); } EXPORT_SYMBOL(drm_fb_helper_cfb_fillrect); @@ -933,6 +1054,8 @@ void drm_fb_helper_cfb_copyarea(struct fb_info *info, const struct fb_copyarea *area) { cfb_copyarea(info, area); + drm_fb_helper_dirty(info, area->dx, area->dy, + area->width, area->height); } EXPORT_SYMBOL(drm_fb_helper_cfb_copyarea); @@ -947,6 +1070,8 @@ void drm_fb_helper_cfb_imageblit(struct fb_info *info, const struct fb_image *image) { cfb_imageblit(info, image); + drm_fb_helper_dirty(info, image->dx, image->dy, + image->width, image->height); } EXPORT_SYMBOL(drm_fb_helper_cfb_imageblit); diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 062723b..dde825c 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -172,6 +172,10 @@ struct drm_fb_helper_connector { * @funcs: driver callbacks for fb helper * @fbdev: emulated fbdev device info struct * @pseudo_palette: fake palette of 16 colors + * @dirty_clip: clip rectangle used with deferred_io to accumulate damage to + * the screen buffer + * @dirty_lock: spinlock protecting @dirty_clip + * @dirty_work: worker used to flush the framebuffer * * This is the main structure used by the fbdev helpers. Drivers supporting * fbdev emulation should embedded this into their overall driver structure. @@ -189,6 +193,11 @@ struct drm_fb_helper { const struct drm_fb_helper_funcs *funcs; struct fb_info *fbdev; u32 pseudo_palette[17]; +#ifdef CONFIG_FB_DEFERRED_IO + struct drm_clip_rect dirty_clip; + spinlock_t dirty_lock; + struct work_struct dirty_work; +#endif /** * @kernel_fb_list: @@ -245,6 +254,9 @@ void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch, void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper); +void drm_fb_helper_deferred_io(struct fb_info *info, + struct list_head *pagelist); + ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, size_t count, loff_t *ppos); ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, @@ -368,6 +380,11 @@ static inline void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper) { } +static inline void drm_fb_helper_deferred_io(struct fb_info *info, + struct list_head *pagelist) +{ +} + static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, size_t count, loff_t *ppos)
This adds deferred io support if CONFIG_FB_DEFERRED_IO is enabled. The fbdev framebuffer changes are flushed using the callback (struct drm_framebuffer *)->funcs->dirty() by a dedicated worker ensuring that it always runs in process context. Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- Changes since v1: - Use a dedicated worker to run the framebuffer flushing like qxl does - Add parameter descriptions to drm_fb_helper_deferred_io drivers/gpu/drm/drm_fb_helper.c | 127 +++++++++++++++++++++++++++++++++++++++- include/drm/drm_fb_helper.h | 17 ++++++ 2 files changed, 143 insertions(+), 1 deletion(-) -- 2.2.2