This patchset adds fbdev deferred io support to drm_fb_helper and drm_fb_cma_helper.
It channels fbdev mmap and fb_{write,fillrect,copyarea,imageblit} damage through the (struct drm_framebuffer_funcs)->dirty callback on the fb_helper framebuffer which will always run in process context.
I have also added patches that converts qxl and udl to use this deferred io support. I have only compile tested it, no functional testing. I know that qxl is purely a software thing so I could actually test it, but I have never used qemu so I'm not keen on spending a lot of time on that.
This was originally part of the tinydrm patchset.
Changes since v3: - drm/fb-helper: Add fb_deferred_io support - Don't use forward decl, move drm_fb_helper_dirty_work() - Use DIV_ROUND_UP in drm_fb_helper_deferred_io()
Changes since v2: - drm/rect: Add some drm_clip_rect utility functions - This patch is dropped - drm/fb-helper: Add fb_deferred_io support - FB_DEFERRED_IO is now always selected by DRM_KMS_FB_HELPER, ifdef removed - The drm_clip_rect utility functions are dropped, so open code it - docs: use & to denote structs - drm/qxl: Use drm_fb_helper deferred_io support - The drm_clip_rect_{width/height} functions are dropped, so open code it
Changes since v1: - drm/fb-helper: Add fb_deferred_io support - Use a dedicated worker to run the framebuffer flushing like qxl does - Add parameter descriptions to drm_fb_helper_deferred_io - fbdev: fb_defio: Export fb_deferred_io_mmap - Expand commit message - drm/qxl: Use drm_fb_helper deferred_io support - Add FIXME about special dirty() callback for fbdev - Remove note in commit message about deferred worker, drm_fb_helper is similar to qxl now. - drm/udl: Use drm_fb_helper deferred_io support - No need to enable deferred_io by default since drm_fb_helper uses a dedicated worker for flushing
Changes since RFC: - Fix drm_clip_rect use to be exclusive on x2/y2 - Put drm_clip_rect functions in drm_rect.{h,c} - Take into account that (struct fb_ops *)->fb_{write,...}() can be called from atomic context (spin_lock_irqsave) - Export fb_deferred_io_mmap() - Add some more documentation - Add qxl and udl patches
Noralf Trønnes (7): drm/udl: Change drm_fb_helper_sys_*() calls to sys_*() drm/qxl: Change drm_fb_helper_sys_*() calls to sys_*() drm/fb-helper: Add fb_deferred_io support fbdev: fb_defio: Export fb_deferred_io_mmap drm/fb-cma-helper: Add fb_deferred_io support drm/qxl: Use drm_fb_helper deferred_io support drm/udl: Use drm_fb_helper deferred_io support
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_fb_cma_helper.c | 178 ++++++++++++++++++++++++++-- drivers/gpu/drm/drm_fb_helper.c | 103 ++++++++++++++++- drivers/gpu/drm/qxl/qxl_display.c | 9 +- drivers/gpu/drm/qxl/qxl_drv.h | 7 +- drivers/gpu/drm/qxl/qxl_fb.c | 223 +++++++++--------------------------- drivers/gpu/drm/qxl/qxl_kms.c | 4 - drivers/gpu/drm/udl/udl_drv.h | 2 - drivers/gpu/drm/udl/udl_fb.c | 140 +--------------------- drivers/video/fbdev/core/fb_defio.c | 3 +- include/drm/drm_fb_cma_helper.h | 14 +++ include/drm/drm_fb_helper.h | 15 +++ include/linux/fb.h | 1 + 13 files changed, 372 insertions(+), 328 deletions(-)
-- 2.2.2
Now that drm_fb_helper gets deferred io support, the drm_fb_helper_sys_{fillrect,copyarea,imageblit} functions will schedule a worker that will call the (struct drm_framebuffer *)->funcs->dirty() function. This will break this driver so use the sys_{fillrect,copyarea,imageblit} functions directly.
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/udl/udl_fb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index fd1eb9d..a52de2f 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -287,7 +287,7 @@ static void udl_fb_fillrect(struct fb_info *info, const struct fb_fillrect *rect { struct udl_fbdev *ufbdev = info->par;
- drm_fb_helper_sys_fillrect(info, rect); + sys_fillrect(info, rect);
udl_handle_damage(&ufbdev->ufb, rect->dx, rect->dy, rect->width, rect->height); @@ -297,7 +297,7 @@ static void udl_fb_copyarea(struct fb_info *info, const struct fb_copyarea *regi { struct udl_fbdev *ufbdev = info->par;
- drm_fb_helper_sys_copyarea(info, region); + sys_copyarea(info, region);
udl_handle_damage(&ufbdev->ufb, region->dx, region->dy, region->width, region->height); @@ -307,7 +307,7 @@ static void udl_fb_imageblit(struct fb_info *info, const struct fb_image *image) { struct udl_fbdev *ufbdev = info->par;
- drm_fb_helper_sys_imageblit(info, image); + sys_imageblit(info, image);
udl_handle_damage(&ufbdev->ufb, image->dx, image->dy, image->width, image->height);
Now that drm_fb_helper gets deferred io support, the drm_fb_helper_sys_{fillrect,copyarea,imageblit} functions will schedule a worker that will call the (struct drm_framebuffer *)->funcs->dirty() function. This will break this driver so use the sys_{fillrect,copyarea,imageblit} functions directly.
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/qxl/qxl_fb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c index bb7ce07..3f7c543 100644 --- a/drivers/gpu/drm/qxl/qxl_fb.c +++ b/drivers/gpu/drm/qxl/qxl_fb.c @@ -199,7 +199,7 @@ static void qxl_fb_fillrect(struct fb_info *info, { struct qxl_fbdev *qfbdev = info->par;
- drm_fb_helper_sys_fillrect(info, rect); + sys_fillrect(info, rect); qxl_dirty_update(qfbdev, rect->dx, rect->dy, rect->width, rect->height); } @@ -209,7 +209,7 @@ static void qxl_fb_copyarea(struct fb_info *info, { struct qxl_fbdev *qfbdev = info->par;
- drm_fb_helper_sys_copyarea(info, area); + sys_copyarea(info, area); qxl_dirty_update(qfbdev, area->dx, area->dy, area->width, area->height); } @@ -219,7 +219,7 @@ static void qxl_fb_imageblit(struct fb_info *info, { struct qxl_fbdev *qfbdev = info->par;
- drm_fb_helper_sys_imageblit(info, image); + sys_imageblit(info, image); qxl_dirty_update(qfbdev, image->dx, image->dy, image->width, image->height); }
This adds deferred io support to drm_fb_helper. 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 Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch ---
Changes since v3: - Don't use forward decl, move drm_fb_helper_dirty_work() - Use DIV_ROUND_UP in drm_fb_helper_deferred_io()
Changes since v2: - FB_DEFERRED_IO is now always selected by DRM_KMS_FB_HELPER, ifdef removed - The drm_clip_rect utility functions are dropped, so open code it - docs: use & to denote structs
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/Kconfig | 1 + drivers/gpu/drm/drm_fb_helper.c | 103 +++++++++++++++++++++++++++++++++++++++- include/drm/drm_fb_helper.h | 15 ++++++ 3 files changed, 118 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 9e4f2f1..8e6f34b 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -52,6 +52,7 @@ config DRM_KMS_FB_HELPER select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT + select FB_DEFERRED_IO help FBDEV helpers for KMS drivers.
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 855108e..62f849f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -84,6 +84,15 @@ 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 &drm_framebuffer_funcs ->dirty is + * set, the drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit} + * functions will accumulate changes and schedule &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. */
/** @@ -637,6 +646,23 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) kfree(helper->crtc_info); }
+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 = &helper->dirty_clip; + struct drm_clip_rect clip_copy; + unsigned long flags; + + spin_lock_irqsave(&helper->dirty_lock, flags); + clip_copy = *clip; + clip->x1 = clip->y1 = ~0; + clip->x2 = clip->y2 = 0; + spin_unlock_irqrestore(&helper->dirty_lock, flags); + + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, &clip_copy, 1); +} + /** * drm_fb_helper_prepare - setup a drm_fb_helper structure * @dev: DRM device @@ -650,6 +676,9 @@ 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); + spin_lock_init(&helper->dirty_lock); + INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); + helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; helper->funcs = funcs; helper->dev = dev; } @@ -834,6 +863,59 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper) } EXPORT_SYMBOL(drm_fb_helper_unlink_fbi);
+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 = &helper->dirty_clip; + unsigned long flags; + + if (!helper->fb->funcs->dirty) + return; + + spin_lock_irqsave(&helper->dirty_lock, flags); + clip->x1 = min_t(u32, clip->x1, x); + clip->y1 = min_t(u32, clip->y1, y); + clip->x2 = max_t(u32, clip->x2, x + width); + clip->y2 = max_t(u32, clip->y2, y + height); + 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 &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, DIV_ROUND_UP(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); + /** * drm_fb_helper_sys_read - wrapper around fb_sys_read * @info: fb_info struct pointer @@ -862,7 +944,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 +966,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 +982,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 +998,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 +1014,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 +1030,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 +1046,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..5b4aa35 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,9 @@ struct drm_fb_helper { const struct drm_fb_helper_funcs *funcs; struct fb_info *fbdev; u32 pseudo_palette[17]; + struct drm_clip_rect dirty_clip; + spinlock_t dirty_lock; + struct work_struct dirty_work;
/** * @kernel_fb_list: @@ -245,6 +252,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 +378,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
Hi,
On 28/04/16 18:18, Noralf Trønnes wrote:
This adds deferred io support to drm_fb_helper. 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 Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Thanks for the series! Unfortunately I haven't been able to follow the discussions properly, so I hope my questions haven't been covered earlier.
Changes since v3:
- Don't use forward decl, move drm_fb_helper_dirty_work()
- Use DIV_ROUND_UP in drm_fb_helper_deferred_io()
Changes since v2:
- FB_DEFERRED_IO is now always selected by DRM_KMS_FB_HELPER, ifdef removed
- The drm_clip_rect utility functions are dropped, so open code it
- docs: use & to denote structs
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/Kconfig | 1 + drivers/gpu/drm/drm_fb_helper.c | 103 +++++++++++++++++++++++++++++++++++++++- include/drm/drm_fb_helper.h | 15 ++++++ 3 files changed, 118 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 9e4f2f1..8e6f34b 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -52,6 +52,7 @@ config DRM_KMS_FB_HELPER select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT
- select FB_DEFERRED_IO help FBDEV helpers for KMS drivers.
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 855108e..62f849f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -84,6 +84,15 @@ 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 &drm_framebuffer_funcs ->dirty is
- set, the drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit}
- functions will accumulate changes and schedule &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
Who's calling {write,fillrect,copyarea,imageblit} in an atomic context? That sounds like a very bad idea to me...
If this is only for accumulating changes, I think it may be better to leave that to the driver as it may have better idea of how to accumulate.
But, of course, this is a helper, so if all the drivers use this kind of accumulation, it makes sense =).
Tomi
Den 29.04.2016 14:50, skrev Tomi Valkeinen:
Hi,
On 28/04/16 18:18, Noralf Trønnes wrote:
This adds deferred io support to drm_fb_helper. 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 Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Thanks for the series! Unfortunately I haven't been able to follow the discussions properly, so I hope my questions haven't been covered earlier.
Changes since v3:
- Don't use forward decl, move drm_fb_helper_dirty_work()
- Use DIV_ROUND_UP in drm_fb_helper_deferred_io()
Changes since v2:
- FB_DEFERRED_IO is now always selected by DRM_KMS_FB_HELPER, ifdef removed
- The drm_clip_rect utility functions are dropped, so open code it
- docs: use & to denote structs
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/Kconfig | 1 + drivers/gpu/drm/drm_fb_helper.c | 103 +++++++++++++++++++++++++++++++++++++++- include/drm/drm_fb_helper.h | 15 ++++++ 3 files changed, 118 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 9e4f2f1..8e6f34b 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -52,6 +52,7 @@ config DRM_KMS_FB_HELPER select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT
- select FB_DEFERRED_IO help FBDEV helpers for KMS drivers.
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 855108e..62f849f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -84,6 +84,15 @@ 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 &drm_framebuffer_funcs ->dirty is
- set, the drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit}
- functions will accumulate changes and schedule &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
Who's calling {write,fillrect,copyarea,imageblit} in an atomic context? That sounds like a very bad idea to me...
I haven't verified it myself, but took it as fact based on commit: bcb39af4486be07e896fc374a2336bad3104ae0a
drm/udl: make usage as a console safer Okay you don't really want to use udl devices as your console, but if you are unlucky enough to do so, you run into a lot of schedule while atomic due to printk being called from all sorts of funky places. So check if we are in an atomic context, and queue the damage for later, the next printk should cause it to appear. This isn't ideal, but it is simple, and seems to work okay in my testing here.
(dirty area idea came from xenfb)
fixes a bunch of sleeping while atomic issues running fbcon on udl devices.
Cc: stable@vger.kernel.org Signed-off-by: Dave Airlie airlied@redhat.com
If this is only for accumulating changes, I think it may be better to leave that to the driver as it may have better idea of how to accumulate.
But, of course, this is a helper, so if all the drivers use this kind of accumulation, it makes sense =).
qxl already accumulates damage this way and upcoming tinydrm also. udl has handled this damage inline except when in_atomic() which results in damage being deferred to the next fb_*() call.
It is possible for drivers to have their own fb_*() handling and still use drm_fb_helper_deferred_io().
For those who likes details, this is fbcon unblanking which results in a full display update on a 320x240 display, shell with blinking cursor on the last line of the console:
[ 5505.164150] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.164186] [drm:drm_atomic_state_init] Allocated atomic state b859e400 [...more drm atomic msgs...] [ 5505.164713] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.164746] [drm:drm_atomic_state_init] Allocated atomic state b859e440 [...more drm atomic msgs...] [ 5505.165086] drm_fb_helper_dirty: x=0,width=320,y=0,height=16 [ 5505.165153] drm_fb_helper_dirty: x=0,width=320,y=16,height=16 [ 5505.165220] drm_fb_helper_dirty: x=0,width=320,y=32,height=16 [ 5505.165287] drm_fb_helper_dirty: x=0,width=320,y=48,height=16 [ 5505.165354] drm_fb_helper_dirty: x=0,width=320,y=64,height=16 [ 5505.165420] drm_fb_helper_dirty: x=0,width=320,y=80,height=16 [ 5505.165487] drm_fb_helper_dirty: x=0,width=320,y=96,height=16 [ 5505.165553] drm_fb_helper_dirty: x=0,width=320,y=112,height=16 [ 5505.165619] drm_fb_helper_dirty: x=0,width=320,y=128,height=16 [ 5505.165686] drm_fb_helper_dirty: x=0,width=320,y=144,height=16 [ 5505.165752] drm_fb_helper_dirty: x=0,width=320,y=160,height=16 [ 5505.165818] drm_fb_helper_dirty: x=0,width=320,y=176,height=16 [ 5505.165884] drm_fb_helper_dirty: x=0,width=320,y=192,height=16 [ 5505.165949] drm_fb_helper_dirty: x=0,width=320,y=208,height=16 [ 5505.165978] drm_fb_helper_dirty: x=0,width=112,y=224,height=16 [ 5505.165988] drm_fb_helper_dirty: x=112,width=8,y=224,height=16 [ 5505.166002] drm_fb_helper_dirty: x=120,width=24,y=224,height=16 [ 5505.166041] drm_fb_helper_dirty: x=144,width=176,y=224,height=16 [ 5505.166058] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.166079] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.166424] drm_fb_helper_dirty_work: x1=0,x2=320,y1=0,y2=240 [ 5505.166452] adafruit-tft spi0.0: mipi_dbi_dirtyfb: vmem=bac40000, x1=0, x2=320, y1=0, y2=240
Cursor blinking:
[ 5505.363478] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.363509] drm_fb_helper_dirty_work: x1=152,x2=160,y1=224,y2=240 [ 5505.363539] adafruit-tft spi0.0: mipi_dbi_dirtyfb: vmem=bac40000, x1=152, x2=160, y1=224, y2=240
[ 5505.563488] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.563514] drm_fb_helper_dirty_work: x1=152,x2=160,y1=224,y2=240 [ 5505.563542] adafruit-tft spi0.0: mipi_dbi_dirtyfb: vmem=bac40000, x1=152, x2=160, y1=224, y2=240
Noralf.
On Fri, Apr 29, 2016 at 04:47:08PM +0200, Noralf Trønnes wrote:
Den 29.04.2016 14:50, skrev Tomi Valkeinen:
Hi,
On 28/04/16 18:18, Noralf Trønnes wrote:
This adds deferred io support to drm_fb_helper. 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 Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Thanks for the series! Unfortunately I haven't been able to follow the discussions properly, so I hope my questions haven't been covered earlier.
Changes since v3:
- Don't use forward decl, move drm_fb_helper_dirty_work()
- Use DIV_ROUND_UP in drm_fb_helper_deferred_io()
Changes since v2:
- FB_DEFERRED_IO is now always selected by DRM_KMS_FB_HELPER, ifdef removed
- The drm_clip_rect utility functions are dropped, so open code it
- docs: use & to denote structs
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/Kconfig | 1 + drivers/gpu/drm/drm_fb_helper.c | 103 +++++++++++++++++++++++++++++++++++++++- include/drm/drm_fb_helper.h | 15 ++++++ 3 files changed, 118 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 9e4f2f1..8e6f34b 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -52,6 +52,7 @@ config DRM_KMS_FB_HELPER select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT
- select FB_DEFERRED_IO help FBDEV helpers for KMS drivers.
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 855108e..62f849f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -84,6 +84,15 @@ 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 &drm_framebuffer_funcs ->dirty is
- set, the drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit}
- functions will accumulate changes and schedule &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
Who's calling {write,fillrect,copyarea,imageblit} in an atomic context? That sounds like a very bad idea to me...
I haven't verified it myself, but took it as fact based on commit: bcb39af4486be07e896fc374a2336bad3104ae0a
I've added a citation to this commit as the reason why we need to handle atomic, to make sure Tomi's question is answered for posterity. -Daniel
drm/udl: make usage as a console safer Okay you don't really want to use udl devices as your console, but if you are unlucky enough to do so, you run into a lot of schedule while atomic due to printk being called from all sorts of funky places. So check if we are in an atomic context, and queue the damage for later, the next printk should cause it to appear. This isn't ideal, but it is simple, and seems to work okay in my testing here.
(dirty area idea came from xenfb)
fixes a bunch of sleeping while atomic issues running fbcon on udl devices.
Cc: stable@vger.kernel.org Signed-off-by: Dave Airlie airlied@redhat.com
If this is only for accumulating changes, I think it may be better to leave that to the driver as it may have better idea of how to accumulate.
But, of course, this is a helper, so if all the drivers use this kind of accumulation, it makes sense =).
qxl already accumulates damage this way and upcoming tinydrm also. udl has handled this damage inline except when in_atomic() which results in damage being deferred to the next fb_*() call.
It is possible for drivers to have their own fb_*() handling and still use drm_fb_helper_deferred_io().
For those who likes details, this is fbcon unblanking which results in a full display update on a 320x240 display, shell with blinking cursor on the last line of the console:
[ 5505.164150] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.164186] [drm:drm_atomic_state_init] Allocated atomic state b859e400 [...more drm atomic msgs...] [ 5505.164713] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.164746] [drm:drm_atomic_state_init] Allocated atomic state b859e440 [...more drm atomic msgs...] [ 5505.165086] drm_fb_helper_dirty: x=0,width=320,y=0,height=16 [ 5505.165153] drm_fb_helper_dirty: x=0,width=320,y=16,height=16 [ 5505.165220] drm_fb_helper_dirty: x=0,width=320,y=32,height=16 [ 5505.165287] drm_fb_helper_dirty: x=0,width=320,y=48,height=16 [ 5505.165354] drm_fb_helper_dirty: x=0,width=320,y=64,height=16 [ 5505.165420] drm_fb_helper_dirty: x=0,width=320,y=80,height=16 [ 5505.165487] drm_fb_helper_dirty: x=0,width=320,y=96,height=16 [ 5505.165553] drm_fb_helper_dirty: x=0,width=320,y=112,height=16 [ 5505.165619] drm_fb_helper_dirty: x=0,width=320,y=128,height=16 [ 5505.165686] drm_fb_helper_dirty: x=0,width=320,y=144,height=16 [ 5505.165752] drm_fb_helper_dirty: x=0,width=320,y=160,height=16 [ 5505.165818] drm_fb_helper_dirty: x=0,width=320,y=176,height=16 [ 5505.165884] drm_fb_helper_dirty: x=0,width=320,y=192,height=16 [ 5505.165949] drm_fb_helper_dirty: x=0,width=320,y=208,height=16 [ 5505.165978] drm_fb_helper_dirty: x=0,width=112,y=224,height=16 [ 5505.165988] drm_fb_helper_dirty: x=112,width=8,y=224,height=16 [ 5505.166002] drm_fb_helper_dirty: x=120,width=24,y=224,height=16 [ 5505.166041] drm_fb_helper_dirty: x=144,width=176,y=224,height=16 [ 5505.166058] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.166079] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.166424] drm_fb_helper_dirty_work: x1=0,x2=320,y1=0,y2=240 [ 5505.166452] adafruit-tft spi0.0: mipi_dbi_dirtyfb: vmem=bac40000, x1=0, x2=320, y1=0, y2=240
Cursor blinking:
[ 5505.363478] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.363509] drm_fb_helper_dirty_work: x1=152,x2=160,y1=224,y2=240 [ 5505.363539] adafruit-tft spi0.0: mipi_dbi_dirtyfb: vmem=bac40000, x1=152, x2=160, y1=224, y2=240
[ 5505.563488] drm_fb_helper_dirty: x=152,width=8,y=224,height=16 [ 5505.563514] drm_fb_helper_dirty_work: x1=152,x2=160,y1=224,y2=240 [ 5505.563542] adafruit-tft spi0.0: mipi_dbi_dirtyfb: vmem=bac40000, x1=152, x2=160, y1=224, y2=240
Noralf.
On Fri, Apr 29, 2016 at 03:50:40PM +0300, Tomi Valkeinen wrote:
Hi,
On 28/04/16 18:18, Noralf Trønnes wrote:
This adds deferred io support to drm_fb_helper. 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 Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Thanks for the series! Unfortunately I haven't been able to follow the discussions properly, so I hope my questions haven't been covered earlier.
Changes since v3:
- Don't use forward decl, move drm_fb_helper_dirty_work()
- Use DIV_ROUND_UP in drm_fb_helper_deferred_io()
Changes since v2:
- FB_DEFERRED_IO is now always selected by DRM_KMS_FB_HELPER, ifdef removed
- The drm_clip_rect utility functions are dropped, so open code it
- docs: use & to denote structs
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/Kconfig | 1 + drivers/gpu/drm/drm_fb_helper.c | 103 +++++++++++++++++++++++++++++++++++++++- include/drm/drm_fb_helper.h | 15 ++++++ 3 files changed, 118 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 9e4f2f1..8e6f34b 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -52,6 +52,7 @@ config DRM_KMS_FB_HELPER select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT
- select FB_DEFERRED_IO help FBDEV helpers for KMS drivers.
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 855108e..62f849f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -84,6 +84,15 @@ 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 &drm_framebuffer_funcs ->dirty is
- set, the drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit}
- functions will accumulate changes and schedule &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
Who's calling {write,fillrect,copyarea,imageblit} in an atomic context? That sounds like a very bad idea to me...
If this is only for accumulating changes, I think it may be better to leave that to the driver as it may have better idea of how to accumulate.
But, of course, this is a helper, so if all the drivers use this kind of accumulation, it makes sense =).
Apparently panic context and cursor timer and stuff like that. I think this started with udl, and just to make sure (it's fbdev after all, no one wants to read the code itself) we've put those checks onto all entry points that draw stuff. It could be overkill, but this is what udl/qxl already do, so better to keep imo for now.
I guess if it's really not needed we could later on change that, but not sure that's worth the effort. And we can't get rid of it entirely. -Daniel
On 29/04/16 17:55, Daniel Vetter wrote:
Who's calling {write,fillrect,copyarea,imageblit} in an atomic context? That sounds like a very bad idea to me...
If this is only for accumulating changes, I think it may be better to leave that to the driver as it may have better idea of how to accumulate.
But, of course, this is a helper, so if all the drivers use this kind of accumulation, it makes sense =).
Apparently panic context and cursor timer and stuff like that. I think this started with udl, and just to make sure (it's fbdev after all, no one wants to read the code itself) we've put those checks onto all entry points that draw stuff. It could be overkill, but this is what udl/qxl already do, so better to keep imo for now.
I guess if it's really not needed we could later on change that, but not sure that's worth the effort. And we can't get rid of it entirely.
Yep... Sounds fine to me.
Someone should implement (minimal) fbdev userspace API support to DRM without using the current fbdev, and implement fbcon on top of that. I don't like how DRM and fbdev gets mixed...
I offer a beer to anyone who does that =).
Tomi
On Fri, Apr 29, 2016 at 06:00:18PM +0300, Tomi Valkeinen wrote:
On 29/04/16 17:55, Daniel Vetter wrote:
Who's calling {write,fillrect,copyarea,imageblit} in an atomic context? That sounds like a very bad idea to me...
If this is only for accumulating changes, I think it may be better to leave that to the driver as it may have better idea of how to accumulate.
But, of course, this is a helper, so if all the drivers use this kind of accumulation, it makes sense =).
Apparently panic context and cursor timer and stuff like that. I think this started with udl, and just to make sure (it's fbdev after all, no one wants to read the code itself) we've put those checks onto all entry points that draw stuff. It could be overkill, but this is what udl/qxl already do, so better to keep imo for now.
I guess if it's really not needed we could later on change that, but not sure that's worth the effort. And we can't get rid of it entirely.
Yep... Sounds fine to me.
Someone should implement (minimal) fbdev userspace API support to DRM without using the current fbdev, and implement fbcon on top of that. I don't like how DRM and fbdev gets mixed...
I offer a beer to anyone who does that =).
The big plan that has been floated years back already (before David Herrmann disappeared into the rh systemd gang) was to have a pure userspace kmscon for console (including vt-switching in userspace like X does), plus a super-minimal panic console on top of kms in the kernel. That looked like a really solid design, unfornutately it didn't go anywhere. But basic patches are still around, and it really just boils down to that some distro would need to integrate kmscon properly, and polish of the kernel side too for integration. -Daniel
Adding David, forgot that before hitting send. -Daniel
On Fri, Apr 29, 2016 at 5:36 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Fri, Apr 29, 2016 at 06:00:18PM +0300, Tomi Valkeinen wrote:
On 29/04/16 17:55, Daniel Vetter wrote:
Who's calling {write,fillrect,copyarea,imageblit} in an atomic context? That sounds like a very bad idea to me...
If this is only for accumulating changes, I think it may be better to leave that to the driver as it may have better idea of how to accumulate.
But, of course, this is a helper, so if all the drivers use this kind of accumulation, it makes sense =).
Apparently panic context and cursor timer and stuff like that. I think this started with udl, and just to make sure (it's fbdev after all, no one wants to read the code itself) we've put those checks onto all entry points that draw stuff. It could be overkill, but this is what udl/qxl already do, so better to keep imo for now.
I guess if it's really not needed we could later on change that, but not sure that's worth the effort. And we can't get rid of it entirely.
Yep... Sounds fine to me.
Someone should implement (minimal) fbdev userspace API support to DRM without using the current fbdev, and implement fbcon on top of that. I don't like how DRM and fbdev gets mixed...
I offer a beer to anyone who does that =).
The big plan that has been floated years back already (before David Herrmann disappeared into the rh systemd gang) was to have a pure userspace kmscon for console (including vt-switching in userspace like X does), plus a super-minimal panic console on top of kms in the kernel. That looked like a really solid design, unfornutately it didn't go anywhere. But basic patches are still around, and it really just boils down to that some distro would need to integrate kmscon properly, and polish of the kernel side too for integration.
-Daniel
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Export fb_deferred_io_mmap so drivers can change vma->vm_page_prot. When the framebuffer memory is allocated using dma_alloc_writecombine() instead of vmalloc(), I get cache syncing problems on ARM. This solves it:
static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma) { fb_deferred_io_mmap(info, vma); vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
return 0; }
Could this have been done in the core? Drivers that don't set (struct fb_ops *)->fb_mmap, gets a call to fb_pgprotect() at the end of the default fb_mmap implementation (drivers/video/fbdev/core/fbmem.c). This is an architecture specific function that on many platforms uses pgprot_writecombine(), but not on all. And looking at some of the fb_mmap implementations, some of them sets vm_page_prot to nocache for instance, so I think the safest bet is to do this in the driver and not in the fbdev core. And we can't call fb_pgprotect() from fb_deferred_io_mmap() either because we don't have access to the file pointer that powerpc needs.
Signed-off-by: Noralf Trønnes noralf@tronnes.org ---
Changes since v1: - Expand commit message
drivers/video/fbdev/core/fb_defio.c | 3 ++- include/linux/fb.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c index 57721c7..74b5bca 100644 --- a/drivers/video/fbdev/core/fb_defio.c +++ b/drivers/video/fbdev/core/fb_defio.c @@ -164,7 +164,7 @@ static const struct address_space_operations fb_deferred_io_aops = { .set_page_dirty = fb_deferred_io_set_page_dirty, };
-static int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma) +int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma) { vma->vm_ops = &fb_deferred_io_vm_ops; vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP; @@ -173,6 +173,7 @@ static int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma) vma->vm_private_data = info; return 0; } +EXPORT_SYMBOL(fb_deferred_io_mmap);
/* workqueue callback */ static void fb_deferred_io_work(struct work_struct *work) diff --git a/include/linux/fb.h b/include/linux/fb.h index dfe8835..a964d07 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -673,6 +673,7 @@ static inline void __fb_pad_aligned_buffer(u8 *dst, u32 d_pitch, }
/* drivers/video/fb_defio.c */ +int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma); extern void fb_deferred_io_init(struct fb_info *info); extern void fb_deferred_io_open(struct fb_info *info, struct inode *inode, -- 2.2.2
On 28/04/16 18:18, Noralf Trønnes wrote:
Export fb_deferred_io_mmap so drivers can change vma->vm_page_prot. When the framebuffer memory is allocated using dma_alloc_writecombine() instead of vmalloc(), I get cache syncing problems on ARM. This solves it:
static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma) { fb_deferred_io_mmap(info, vma); vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
return 0; }
Could this have been done in the core? Drivers that don't set (struct fb_ops *)->fb_mmap, gets a call to fb_pgprotect() at the end of the default fb_mmap implementation (drivers/video/fbdev/core/fbmem.c). This is an architecture specific function that on many platforms uses pgprot_writecombine(), but not on all. And looking at some of the fb_mmap implementations, some of them sets vm_page_prot to nocache for instance, so I think the safest bet is to do this in the driver and not in the fbdev core. And we can't call fb_pgprotect() from fb_deferred_io_mmap() either because we don't have access to the file pointer that powerpc needs.
Signed-off-by: Noralf Trønnes noralf@tronnes.org
Acked-by: Tomi Valkeinen tomi.valkeinen@ti.com
Tomi
This adds fbdev deferred io support if CONFIG_FB_DEFERRED_IO is enabled. The driver has to provide a (struct drm_framebuffer_funcs *)->dirty() callback to get notification of fbdev framebuffer changes. If the dirty() hook is set, then fb_deferred_io is set up automatically by the helper.
Two functions have been added so that the driver can provide a dirty() function: - drm_fbdev_cma_init_with_funcs() This makes it possible for the driver to provided a custom (struct drm_fb_helper_funcs *)->fb_probe() function. - drm_fbdev_cma_create_with_funcs() This is used by the .fb_probe hook to set a driver provided (struct drm_framebuffer_funcs *)->dirty() function.
Cc: laurent.pinchart@ideasonboard.com Signed-off-by: Noralf Trønnes noralf@tronnes.org ---
Changes since v2: - FB_DEFERRED_IO is now always selected by DRM_KMS_FB_HELPER, ifdef removed
drivers/gpu/drm/drm_fb_cma_helper.c | 178 +++++++++++++++++++++++++++++++++--- include/drm/drm_fb_cma_helper.h | 14 +++ 2 files changed, 180 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index bb88e3d..086f600 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -25,6 +25,8 @@ #include <drm/drm_fb_cma_helper.h> #include <linux/module.h>
+#define DEFAULT_FBDEFIO_DELAY_MS 50 + struct drm_fb_cma { struct drm_framebuffer fb; struct drm_gem_cma_object *obj[4]; @@ -35,6 +37,61 @@ struct drm_fbdev_cma { struct drm_fb_cma *fb; };
+/** + * DOC: framebuffer cma helper functions + * + * Provides helper functions for creating a cma (contiguous memory allocator) + * backed framebuffer. + * + * drm_fb_cma_create() is used in the + * (struct drm_mode_config_funcs *)->fb_create callback function to create the + * cma backed framebuffer. + * + * An fbdev framebuffer backed by cma is also available by calling + * drm_fbdev_cma_init(). drm_fbdev_cma_fini() tears it down. + * If CONFIG_FB_DEFERRED_IO is enabled and the callback + * (struct drm_framebuffer_funcs)->dirty is set, fb_deferred_io + * will be set up automatically. dirty() is called by + * drm_fb_helper_deferred_io() in process context (struct delayed_work). + * + * Example fbdev deferred io code: + * + * static int driver_fbdev_fb_dirty(struct drm_framebuffer *fb, + * struct drm_file *file_priv, + * unsigned flags, unsigned color, + * struct drm_clip_rect *clips, + * unsigned num_clips) + * { + * struct drm_gem_cma_object *cma = drm_fb_cma_get_gem_obj(fb, 0); + * ... push changes ... + * return 0; + * } + * + * static struct drm_framebuffer_funcs driver_fbdev_fb_funcs = { + * .destroy = drm_fb_cma_destroy, + * .create_handle = drm_fb_cma_create_handle, + * .dirty = driver_fbdev_fb_dirty, + * }; + * + * static int driver_fbdev_create(struct drm_fb_helper *helper, + * struct drm_fb_helper_surface_size *sizes) + * { + * return drm_fbdev_cma_create_with_funcs(helper, sizes, + * &driver_fbdev_fb_funcs); + * } + * + * static const struct drm_fb_helper_funcs driver_fb_helper_funcs = { + * .fb_probe = driver_fbdev_create, + * }; + * + * Initialize: + * fbdev = drm_fbdev_cma_init_with_funcs(dev, 16, + * dev->mode_config.num_crtc, + * dev->mode_config.num_connector, + * &driver_fb_helper_funcs); + * + */ + static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper) { return container_of(helper, struct drm_fbdev_cma, fb_helper); @@ -45,7 +102,7 @@ static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb) return container_of(fb, struct drm_fb_cma, fb); }
-static void drm_fb_cma_destroy(struct drm_framebuffer *fb) +void drm_fb_cma_destroy(struct drm_framebuffer *fb) { struct drm_fb_cma *fb_cma = to_fb_cma(fb); int i; @@ -58,8 +115,9 @@ static void drm_fb_cma_destroy(struct drm_framebuffer *fb) drm_framebuffer_cleanup(fb); kfree(fb_cma); } +EXPORT_SYMBOL(drm_fb_cma_destroy);
-static int drm_fb_cma_create_handle(struct drm_framebuffer *fb, +int drm_fb_cma_create_handle(struct drm_framebuffer *fb, struct drm_file *file_priv, unsigned int *handle) { struct drm_fb_cma *fb_cma = to_fb_cma(fb); @@ -67,6 +125,7 @@ static int drm_fb_cma_create_handle(struct drm_framebuffer *fb, return drm_gem_handle_create(file_priv, &fb_cma->obj[0]->base, handle); } +EXPORT_SYMBOL(drm_fb_cma_create_handle);
static struct drm_framebuffer_funcs drm_fb_cma_funcs = { .destroy = drm_fb_cma_destroy, @@ -76,7 +135,7 @@ static struct drm_framebuffer_funcs drm_fb_cma_funcs = { static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_cma_object **obj, - unsigned int num_planes) + unsigned int num_planes, struct drm_framebuffer_funcs *funcs) { struct drm_fb_cma *fb_cma; int ret; @@ -91,7 +150,7 @@ static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev, for (i = 0; i < num_planes; i++) fb_cma->obj[i] = obj[i];
- ret = drm_framebuffer_init(dev, &fb_cma->fb, &drm_fb_cma_funcs); + ret = drm_framebuffer_init(dev, &fb_cma->fb, funcs); if (ret) { dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", ret); kfree(fb_cma); @@ -145,7 +204,7 @@ struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, objs[i] = to_drm_gem_cma_obj(obj); }
- fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i); + fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, &drm_fb_cma_funcs); if (IS_ERR(fb_cma)) { ret = PTR_ERR(fb_cma); goto err_gem_object_unreference; @@ -233,8 +292,67 @@ static struct fb_ops drm_fbdev_cma_ops = { .fb_setcmap = drm_fb_helper_setcmap, };
-static int drm_fbdev_cma_create(struct drm_fb_helper *helper, - struct drm_fb_helper_surface_size *sizes) +static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info, + struct vm_area_struct *vma) +{ + fb_deferred_io_mmap(info, vma); + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); + + return 0; +} + +static int drm_fbdev_cma_defio_init(struct fb_info *fbi, + struct drm_gem_cma_object *cma_obj) +{ + struct fb_deferred_io *fbdefio; + struct fb_ops *fbops; + + /* + * Per device structures are needed because: + * fbops: fb_deferred_io_cleanup() clears fbops.fb_mmap + * fbdefio: individual delays + */ + fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL); + fbops = kzalloc(sizeof(*fbops), GFP_KERNEL); + if (!fbdefio || !fbops) { + kfree(fbdefio); + return -ENOMEM; + } + + /* can't be offset from vaddr since dirty() uses cma_obj */ + fbi->screen_buffer = cma_obj->vaddr; + /* fb_deferred_io_fault() needs a physical address */ + fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer)); + + *fbops = *fbi->fbops; + fbi->fbops = fbops; + + fbdefio->delay = msecs_to_jiffies(DEFAULT_FBDEFIO_DELAY_MS); + fbdefio->deferred_io = drm_fb_helper_deferred_io; + fbi->fbdefio = fbdefio; + fb_deferred_io_init(fbi); + fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap; + + return 0; +} + +static void drm_fbdev_cma_defio_fini(struct fb_info *fbi) +{ + if (!fbi->fbdefio) + return; + + fb_deferred_io_cleanup(fbi); + kfree(fbi->fbdefio); + kfree(fbi->fbops); +} + +/* + * For use in a (struct drm_fb_helper_funcs *)->fb_probe callback function that + * needs custom struct drm_framebuffer_funcs, like dirty() for deferred_io use. + */ +int drm_fbdev_cma_create_with_funcs(struct drm_fb_helper *helper, + struct drm_fb_helper_surface_size *sizes, + struct drm_framebuffer_funcs *funcs) { struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper); struct drm_mode_fb_cmd2 mode_cmd = { 0 }; @@ -270,7 +388,7 @@ static int drm_fbdev_cma_create(struct drm_fb_helper *helper, goto err_gem_free_object; }
- fbdev_cma->fb = drm_fb_cma_alloc(dev, &mode_cmd, &obj, 1); + fbdev_cma->fb = drm_fb_cma_alloc(dev, &mode_cmd, &obj, 1, funcs); if (IS_ERR(fbdev_cma->fb)) { dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n"); ret = PTR_ERR(fbdev_cma->fb); @@ -296,31 +414,48 @@ static int drm_fbdev_cma_create(struct drm_fb_helper *helper, fbi->screen_size = size; fbi->fix.smem_len = size;
+ if (funcs->dirty) { + ret = drm_fbdev_cma_defio_init(fbi, obj); + if (ret) + goto err_cma_destroy; + } + return 0;
+err_cma_destroy: + drm_framebuffer_unregister_private(&fbdev_cma->fb->fb); + drm_fb_cma_destroy(&fbdev_cma->fb->fb); err_fb_info_destroy: drm_fb_helper_release_fbi(helper); err_gem_free_object: dev->driver->gem_free_object(&obj->base); return ret; } +EXPORT_SYMBOL(drm_fbdev_cma_create_with_funcs); + +static int drm_fbdev_cma_create(struct drm_fb_helper *helper, + struct drm_fb_helper_surface_size *sizes) +{ + return drm_fbdev_cma_create_with_funcs(helper, sizes, &drm_fb_cma_funcs); +}
static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = { .fb_probe = drm_fbdev_cma_create, };
/** - * drm_fbdev_cma_init() - Allocate and initializes a drm_fbdev_cma struct + * drm_fbdev_cma_init_with_funcs() - Allocate and initializes a drm_fbdev_cma struct * @dev: DRM device * @preferred_bpp: Preferred bits per pixel for the device * @num_crtc: Number of CRTCs * @max_conn_count: Maximum number of connectors + * @funcs: fb helper functions, in particular fb_probe() * * Returns a newly allocated drm_fbdev_cma struct or a ERR_PTR. */ -struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev, +struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, unsigned int preferred_bpp, unsigned int num_crtc, - unsigned int max_conn_count) + unsigned int max_conn_count, const struct drm_fb_helper_funcs *funcs) { struct drm_fbdev_cma *fbdev_cma; struct drm_fb_helper *helper; @@ -334,7 +469,7 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
helper = &fbdev_cma->fb_helper;
- drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs); + drm_fb_helper_prepare(dev, helper, funcs);
ret = drm_fb_helper_init(dev, helper, num_crtc, max_conn_count); if (ret < 0) { @@ -364,6 +499,24 @@ err_free:
return ERR_PTR(ret); } +EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs); + +/** + * drm_fbdev_cma_init() - Allocate and initializes a drm_fbdev_cma struct + * @dev: DRM device + * @preferred_bpp: Preferred bits per pixel for the device + * @num_crtc: Number of CRTCs + * @max_conn_count: Maximum number of connectors + * + * Returns a newly allocated drm_fbdev_cma struct or a ERR_PTR. + */ +struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev, + unsigned int preferred_bpp, unsigned int num_crtc, + unsigned int max_conn_count) +{ + return drm_fbdev_cma_init_with_funcs(dev, preferred_bpp, num_crtc, + max_conn_count, &drm_fb_cma_helper_funcs); +} EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
/** @@ -373,6 +526,7 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init); void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma) { drm_fb_helper_unregister_fbi(&fbdev_cma->fb_helper); + drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev); drm_fb_helper_release_fbi(&fbdev_cma->fb_helper);
if (fbdev_cma->fb) { diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h index be62bd3..6554b6f 100644 --- a/include/drm/drm_fb_cma_helper.h +++ b/include/drm/drm_fb_cma_helper.h @@ -4,11 +4,18 @@ struct drm_fbdev_cma; struct drm_gem_cma_object;
+struct drm_fb_helper_surface_size; +struct drm_framebuffer_funcs; +struct drm_fb_helper_funcs; struct drm_framebuffer; +struct drm_fb_helper; struct drm_device; struct drm_file; struct drm_mode_fb_cmd2;
+struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, + unsigned int preferred_bpp, unsigned int num_crtc, + unsigned int max_conn_count, const struct drm_fb_helper_funcs *funcs); struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev, unsigned int preferred_bpp, unsigned int num_crtc, unsigned int max_conn_count); @@ -16,6 +23,13 @@ void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma);
void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma); void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma); +int drm_fbdev_cma_create_with_funcs(struct drm_fb_helper *helper, + struct drm_fb_helper_surface_size *sizes, + struct drm_framebuffer_funcs *funcs); + +void drm_fb_cma_destroy(struct drm_framebuffer *fb); +int drm_fb_cma_create_handle(struct drm_framebuffer *fb, + struct drm_file *file_priv, unsigned int *handle);
struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev, struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd); -- 2.2.2
Use the fbdev deferred io support in drm_fb_helper which mirrors the one qxl has had. This patch has only been compile tested.
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch ---
Changes since v2: - The drm_clip_rect_{width/height} functions are dropped, so open code it
Changes since v1: - Add FIXME about special dirty() callback for fbdev - Remove note in commit message about deferred worker, drm_fb_helper is similar to qxl now.
drivers/gpu/drm/qxl/qxl_display.c | 9 +- drivers/gpu/drm/qxl/qxl_drv.h | 7 +- drivers/gpu/drm/qxl/qxl_fb.c | 223 ++++++++++---------------------------- drivers/gpu/drm/qxl/qxl_kms.c | 4 - 4 files changed, 65 insertions(+), 178 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 030409a..9a03524 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -465,7 +465,7 @@ static const struct drm_crtc_funcs qxl_crtc_funcs = { .page_flip = qxl_crtc_page_flip, };
-static void qxl_user_framebuffer_destroy(struct drm_framebuffer *fb) +void qxl_user_framebuffer_destroy(struct drm_framebuffer *fb) { struct qxl_framebuffer *qxl_fb = to_qxl_framebuffer(fb);
@@ -527,12 +527,13 @@ int qxl_framebuffer_init(struct drm_device *dev, struct qxl_framebuffer *qfb, const struct drm_mode_fb_cmd2 *mode_cmd, - struct drm_gem_object *obj) + struct drm_gem_object *obj, + const struct drm_framebuffer_funcs *funcs) { int ret;
qfb->obj = obj; - ret = drm_framebuffer_init(dev, &qfb->base, &qxl_fb_funcs); + ret = drm_framebuffer_init(dev, &qfb->base, funcs); if (ret) { qfb->obj = NULL; return ret; @@ -999,7 +1000,7 @@ qxl_user_framebuffer_create(struct drm_device *dev, if (qxl_fb == NULL) return NULL;
- ret = qxl_framebuffer_init(dev, qxl_fb, mode_cmd, obj); + ret = qxl_framebuffer_init(dev, qxl_fb, mode_cmd, obj, &qxl_fb_funcs); if (ret) { kfree(qxl_fb); drm_gem_object_unreference_unlocked(obj); diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index 3f3897e..3ad6604 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -324,8 +324,6 @@ struct qxl_device { struct workqueue_struct *gc_queue; struct work_struct gc_work;
- struct work_struct fb_work; - struct drm_property *hotplug_mode_update_property; int monitors_config_width; int monitors_config_height; @@ -389,11 +387,13 @@ int qxl_get_handle_for_primary_fb(struct qxl_device *qdev, void qxl_fbdev_set_suspend(struct qxl_device *qdev, int state);
/* qxl_display.c */ +void qxl_user_framebuffer_destroy(struct drm_framebuffer *fb); int qxl_framebuffer_init(struct drm_device *dev, struct qxl_framebuffer *rfb, const struct drm_mode_fb_cmd2 *mode_cmd, - struct drm_gem_object *obj); + struct drm_gem_object *obj, + const struct drm_framebuffer_funcs *funcs); void qxl_display_read_client_monitors_config(struct qxl_device *qdev); void qxl_send_monitors_config(struct qxl_device *qdev); int qxl_create_monitors_object(struct qxl_device *qdev); @@ -553,7 +553,6 @@ int qxl_irq_init(struct qxl_device *qdev); irqreturn_t qxl_irq_handler(int irq, void *arg);
/* qxl_fb.c */ -int qxl_fb_init(struct qxl_device *qdev); bool qxl_fbdev_qobj_is_fb(struct qxl_device *qdev, struct qxl_bo *qobj);
int qxl_debugfs_add_files(struct qxl_device *qdev, diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c index 3f7c543..739a08c 100644 --- a/drivers/gpu/drm/qxl/qxl_fb.c +++ b/drivers/gpu/drm/qxl/qxl_fb.c @@ -46,15 +46,6 @@ struct qxl_fbdev { struct list_head delayed_ops; void *shadow; int size; - - /* dirty memory logging */ - struct { - spinlock_t lock; - unsigned x1; - unsigned y1; - unsigned x2; - unsigned y2; - } dirty; };
static void qxl_fb_image_init(struct qxl_fb_image *qxl_fb_image, @@ -82,169 +73,18 @@ static void qxl_fb_image_init(struct qxl_fb_image *qxl_fb_image, } }
-static void qxl_fb_dirty_flush(struct fb_info *info) -{ - struct qxl_fbdev *qfbdev = info->par; - struct qxl_device *qdev = qfbdev->qdev; - struct qxl_fb_image qxl_fb_image; - struct fb_image *image = &qxl_fb_image.fb_image; - unsigned long flags; - u32 x1, x2, y1, y2; - - /* TODO: hard coding 32 bpp */ - int stride = qfbdev->qfb.base.pitches[0]; - - spin_lock_irqsave(&qfbdev->dirty.lock, flags); - - x1 = qfbdev->dirty.x1; - x2 = qfbdev->dirty.x2; - y1 = qfbdev->dirty.y1; - y2 = qfbdev->dirty.y2; - qfbdev->dirty.x1 = 0; - qfbdev->dirty.x2 = 0; - qfbdev->dirty.y1 = 0; - qfbdev->dirty.y2 = 0; - - spin_unlock_irqrestore(&qfbdev->dirty.lock, flags); - - /* - * we are using a shadow draw buffer, at qdev->surface0_shadow - */ - qxl_io_log(qdev, "dirty x[%d, %d], y[%d, %d]", x1, x2, y1, y2); - image->dx = x1; - image->dy = y1; - image->width = x2 - x1 + 1; - image->height = y2 - y1 + 1; - image->fg_color = 0xffffffff; /* unused, just to avoid uninitialized - warnings */ - image->bg_color = 0; - image->depth = 32; /* TODO: take from somewhere? */ - image->cmap.start = 0; - image->cmap.len = 0; - image->cmap.red = NULL; - image->cmap.green = NULL; - image->cmap.blue = NULL; - image->cmap.transp = NULL; - image->data = qfbdev->shadow + (x1 * 4) + (stride * y1); - - qxl_fb_image_init(&qxl_fb_image, qdev, info, NULL); - qxl_draw_opaque_fb(&qxl_fb_image, stride); -} - -static void qxl_dirty_update(struct qxl_fbdev *qfbdev, - int x, int y, int width, int height) -{ - struct qxl_device *qdev = qfbdev->qdev; - unsigned long flags; - int x2, y2; - - x2 = x + width - 1; - y2 = y + height - 1; - - spin_lock_irqsave(&qfbdev->dirty.lock, flags); - - if ((qfbdev->dirty.y2 - qfbdev->dirty.y1) && - (qfbdev->dirty.x2 - qfbdev->dirty.x1)) { - if (qfbdev->dirty.y1 < y) - y = qfbdev->dirty.y1; - if (qfbdev->dirty.y2 > y2) - y2 = qfbdev->dirty.y2; - if (qfbdev->dirty.x1 < x) - x = qfbdev->dirty.x1; - if (qfbdev->dirty.x2 > x2) - x2 = qfbdev->dirty.x2; - } - - qfbdev->dirty.x1 = x; - qfbdev->dirty.x2 = x2; - qfbdev->dirty.y1 = y; - qfbdev->dirty.y2 = y2; - - spin_unlock_irqrestore(&qfbdev->dirty.lock, flags); - - schedule_work(&qdev->fb_work); -} - -static void qxl_deferred_io(struct fb_info *info, - struct list_head *pagelist) -{ - struct qxl_fbdev *qfbdev = info->par; - unsigned long start, end, min, max; - struct page *page; - int 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 = (max / info->fix.line_length) + 1; - qxl_dirty_update(qfbdev, 0, y1, info->var.xres, y2 - y1); - } -}; - static struct fb_deferred_io qxl_defio = { .delay = QXL_DIRTY_DELAY, - .deferred_io = qxl_deferred_io, + .deferred_io = drm_fb_helper_deferred_io, };
-static void qxl_fb_fillrect(struct fb_info *info, - const struct fb_fillrect *rect) -{ - struct qxl_fbdev *qfbdev = info->par; - - sys_fillrect(info, rect); - qxl_dirty_update(qfbdev, rect->dx, rect->dy, rect->width, - rect->height); -} - -static void qxl_fb_copyarea(struct fb_info *info, - const struct fb_copyarea *area) -{ - struct qxl_fbdev *qfbdev = info->par; - - sys_copyarea(info, area); - qxl_dirty_update(qfbdev, area->dx, area->dy, area->width, - area->height); -} - -static void qxl_fb_imageblit(struct fb_info *info, - const struct fb_image *image) -{ - struct qxl_fbdev *qfbdev = info->par; - - sys_imageblit(info, image); - qxl_dirty_update(qfbdev, image->dx, image->dy, image->width, - image->height); -} - -static void qxl_fb_work(struct work_struct *work) -{ - struct qxl_device *qdev = container_of(work, struct qxl_device, fb_work); - struct qxl_fbdev *qfbdev = qdev->mode_info.qfbdev; - - qxl_fb_dirty_flush(qfbdev->helper.fbdev); -} - -int qxl_fb_init(struct qxl_device *qdev) -{ - INIT_WORK(&qdev->fb_work, qxl_fb_work); - return 0; -} - static struct fb_ops qxlfb_ops = { .owner = THIS_MODULE, .fb_check_var = drm_fb_helper_check_var, .fb_set_par = drm_fb_helper_set_par, /* TODO: copy vmwgfx */ - .fb_fillrect = qxl_fb_fillrect, - .fb_copyarea = qxl_fb_copyarea, - .fb_imageblit = qxl_fb_imageblit, + .fb_fillrect = drm_fb_helper_sys_fillrect, + .fb_copyarea = drm_fb_helper_sys_copyarea, + .fb_imageblit = drm_fb_helper_sys_imageblit, .fb_pan_display = drm_fb_helper_pan_display, .fb_blank = drm_fb_helper_blank, .fb_setcmap = drm_fb_helper_setcmap, @@ -338,6 +178,57 @@ out_unref: return ret; }
+/* + * FIXME + * It should not be necessary to have a special dirty() callback for fbdev. + */ +static int qxlfb_framebuffer_dirty(struct drm_framebuffer *fb, + struct drm_file *file_priv, + unsigned flags, unsigned color, + struct drm_clip_rect *clips, + unsigned num_clips) +{ + struct qxl_device *qdev = fb->dev->dev_private; + struct fb_info *info = qdev->fbdev_info; + struct qxl_fbdev *qfbdev = info->par; + struct qxl_fb_image qxl_fb_image; + struct fb_image *image = &qxl_fb_image.fb_image; + + /* TODO: hard coding 32 bpp */ + int stride = qfbdev->qfb.base.pitches[0]; + + /* + * we are using a shadow draw buffer, at qdev->surface0_shadow + */ + qxl_io_log(qdev, "dirty x[%d, %d], y[%d, %d]", clips->x1, clips->x2, + clips->y1, clips->y2); + image->dx = clips->x1; + image->dy = clips->y1; + image->width = clips->x2 - clips->x1; + image->height = clips->y2 - clips->y1; + image->fg_color = 0xffffffff; /* unused, just to avoid uninitialized + warnings */ + image->bg_color = 0; + image->depth = 32; /* TODO: take from somewhere? */ + image->cmap.start = 0; + image->cmap.len = 0; + image->cmap.red = NULL; + image->cmap.green = NULL; + image->cmap.blue = NULL; + image->cmap.transp = NULL; + image->data = qfbdev->shadow + (clips->x1 * 4) + (stride * clips->y1); + + qxl_fb_image_init(&qxl_fb_image, qdev, info, NULL); + qxl_draw_opaque_fb(&qxl_fb_image, stride); + + return 0; +} + +static const struct drm_framebuffer_funcs qxlfb_fb_funcs = { + .destroy = qxl_user_framebuffer_destroy, + .dirty = qxlfb_framebuffer_dirty, +}; + static int qxlfb_create(struct qxl_fbdev *qfbdev, struct drm_fb_helper_surface_size *sizes) { @@ -383,7 +274,8 @@ static int qxlfb_create(struct qxl_fbdev *qfbdev,
info->par = qfbdev;
- qxl_framebuffer_init(qdev->ddev, &qfbdev->qfb, &mode_cmd, gobj); + qxl_framebuffer_init(qdev->ddev, &qfbdev->qfb, &mode_cmd, gobj, + &qxlfb_fb_funcs);
fb = &qfbdev->qfb.base;
@@ -504,7 +396,6 @@ int qxl_fbdev_init(struct qxl_device *qdev) qfbdev->qdev = qdev; qdev->mode_info.qfbdev = qfbdev; spin_lock_init(&qfbdev->delayed_ops_lock); - spin_lock_init(&qfbdev->dirty.lock); INIT_LIST_HEAD(&qfbdev->delayed_ops);
drm_fb_helper_prepare(qdev->ddev, &qfbdev->helper, diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index b2977a1..2319800 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -261,10 +261,6 @@ static int qxl_device_init(struct qxl_device *qdev, qdev->gc_queue = create_singlethread_workqueue("qxl_gc"); INIT_WORK(&qdev->gc_work, qxl_gc_work);
- r = qxl_fb_init(qdev); - if (r) - return r; - return 0; }
-- 2.2.2
Use the fbdev deferred io support in drm_fb_helper. The (struct fb_ops *)->fb_{fillrect,copyarea,imageblit} functions will now schedule a worker instead of being flushed directly like it was previously (recorded when in atomic).
This patch has only been compile tested.
Signed-off-by: Noralf Trønnes noralf@tronnes.org Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch ---
Changes since v1: - No need to enable deferred_io by default since drm_fb_helper uses a dedicated worker for flushing
drivers/gpu/drm/udl/udl_drv.h | 2 - drivers/gpu/drm/udl/udl_fb.c | 140 ++---------------------------------------- 2 files changed, 6 insertions(+), 136 deletions(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h index 4a064ef..0b03d34 100644 --- a/drivers/gpu/drm/udl/udl_drv.h +++ b/drivers/gpu/drm/udl/udl_drv.h @@ -81,8 +81,6 @@ struct udl_framebuffer { struct drm_framebuffer base; struct udl_gem_object *obj; bool active_16; /* active on the 16-bit channel */ - int x1, y1, x2, y2; /* dirty rect */ - spinlock_t dirty_lock; };
#define to_udl_fb(x) container_of(x, struct udl_framebuffer, base) diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index a52de2f..4a9b432 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -77,68 +77,6 @@ static uint16_t rgb16(uint32_t col) } #endif
-/* - * NOTE: fb_defio.c is holding info->fbdefio.mutex - * Touching ANY framebuffer memory that triggers a page fault - * in fb_defio will cause a deadlock, when it also tries to - * grab the same mutex. - */ -static void udlfb_dpy_deferred_io(struct fb_info *info, - struct list_head *pagelist) -{ - struct page *cur; - struct fb_deferred_io *fbdefio = info->fbdefio; - struct udl_fbdev *ufbdev = info->par; - struct drm_device *dev = ufbdev->ufb.base.dev; - struct udl_device *udl = dev->dev_private; - struct urb *urb; - char *cmd; - cycles_t start_cycles, end_cycles; - int bytes_sent = 0; - int bytes_identical = 0; - int bytes_rendered = 0; - - if (!fb_defio) - return; - - start_cycles = get_cycles(); - - urb = udl_get_urb(dev); - if (!urb) - return; - - cmd = urb->transfer_buffer; - - /* walk the written page list and render each to device */ - list_for_each_entry(cur, &fbdefio->pagelist, lru) { - - if (udl_render_hline(dev, (ufbdev->ufb.base.bits_per_pixel / 8), - &urb, (char *) info->fix.smem_start, - &cmd, cur->index << PAGE_SHIFT, - cur->index << PAGE_SHIFT, - PAGE_SIZE, &bytes_identical, &bytes_sent)) - goto error; - bytes_rendered += PAGE_SIZE; - } - - if (cmd > (char *) urb->transfer_buffer) { - /* Send partial buffer remaining before exiting */ - int len = cmd - (char *) urb->transfer_buffer; - udl_submit_urb(dev, urb, len); - bytes_sent += len; - } else - udl_urb_completion(urb); - -error: - atomic_add(bytes_sent, &udl->bytes_sent); - atomic_add(bytes_identical, &udl->bytes_identical); - atomic_add(bytes_rendered, &udl->bytes_rendered); - end_cycles = get_cycles(); - atomic_add(((unsigned int) ((end_cycles - start_cycles) - >> 10)), /* Kcycles */ - &udl->cpu_kcycles_used); -} - int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, int width, int height) { @@ -152,9 +90,6 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, struct urb *urb; int aligned_x; int bpp = (fb->base.bits_per_pixel / 8); - int x2, y2; - bool store_for_later = false; - unsigned long flags;
if (!fb->active_16) return 0; @@ -180,38 +115,6 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, (y + height > fb->base.height)) return -EINVAL;
- /* if we are in atomic just store the info - can't test inside spin lock */ - if (in_atomic()) - store_for_later = true; - - x2 = x + width - 1; - y2 = y + height - 1; - - spin_lock_irqsave(&fb->dirty_lock, flags); - - if (fb->y1 < y) - y = fb->y1; - if (fb->y2 > y2) - y2 = fb->y2; - if (fb->x1 < x) - x = fb->x1; - if (fb->x2 > x2) - x2 = fb->x2; - - if (store_for_later) { - fb->x1 = x; - fb->x2 = x2; - fb->y1 = y; - fb->y2 = y2; - spin_unlock_irqrestore(&fb->dirty_lock, flags); - return 0; - } - - fb->x1 = fb->y1 = INT_MAX; - fb->x2 = fb->y2 = 0; - - spin_unlock_irqrestore(&fb->dirty_lock, flags); start_cycles = get_cycles();
urb = udl_get_urb(dev); @@ -219,14 +122,14 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, return 0; cmd = urb->transfer_buffer;
- for (i = y; i <= y2 ; i++) { + for (i = y; i < height ; i++) { const int line_offset = fb->base.pitches[0] * i; const int byte_offset = line_offset + (x * bpp); const int dev_byte_offset = (fb->base.width * bpp * i) + (x * bpp); if (udl_render_hline(dev, bpp, &urb, (char *) fb->obj->vmapping, &cmd, byte_offset, dev_byte_offset, - (x2 - x + 1) * bpp, + width * bpp, &bytes_identical, &bytes_sent)) goto error; } @@ -283,36 +186,6 @@ static int udl_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) return 0; }
-static void udl_fb_fillrect(struct fb_info *info, const struct fb_fillrect *rect) -{ - struct udl_fbdev *ufbdev = info->par; - - sys_fillrect(info, rect); - - udl_handle_damage(&ufbdev->ufb, rect->dx, rect->dy, rect->width, - rect->height); -} - -static void udl_fb_copyarea(struct fb_info *info, const struct fb_copyarea *region) -{ - struct udl_fbdev *ufbdev = info->par; - - sys_copyarea(info, region); - - udl_handle_damage(&ufbdev->ufb, region->dx, region->dy, region->width, - region->height); -} - -static void udl_fb_imageblit(struct fb_info *info, const struct fb_image *image) -{ - struct udl_fbdev *ufbdev = info->par; - - sys_imageblit(info, image); - - udl_handle_damage(&ufbdev->ufb, image->dx, image->dy, image->width, - image->height); -} - /* * It's common for several clients to have framebuffer open simultaneously. * e.g. both fbcon and X. Makes things interesting. @@ -339,7 +212,7 @@ static int udl_fb_open(struct fb_info *info, int user)
if (fbdefio) { fbdefio->delay = DL_DEFIO_WRITE_DELAY; - fbdefio->deferred_io = udlfb_dpy_deferred_io; + fbdefio->deferred_io = drm_fb_helper_deferred_io; }
info->fbdefio = fbdefio; @@ -379,9 +252,9 @@ static struct fb_ops udlfb_ops = { .owner = THIS_MODULE, .fb_check_var = drm_fb_helper_check_var, .fb_set_par = drm_fb_helper_set_par, - .fb_fillrect = udl_fb_fillrect, - .fb_copyarea = udl_fb_copyarea, - .fb_imageblit = udl_fb_imageblit, + .fb_fillrect = drm_fb_helper_sys_fillrect, + .fb_copyarea = drm_fb_helper_sys_copyarea, + .fb_imageblit = drm_fb_helper_sys_imageblit, .fb_pan_display = drm_fb_helper_pan_display, .fb_blank = drm_fb_helper_blank, .fb_setcmap = drm_fb_helper_setcmap, @@ -458,7 +331,6 @@ udl_framebuffer_init(struct drm_device *dev, { int ret;
- spin_lock_init(&ufb->dirty_lock); ufb->obj = obj; drm_helper_mode_fill_fb_struct(&ufb->base, mode_cmd); ret = drm_framebuffer_init(dev, &ufb->base, &udlfb_funcs); -- 2.2.2
On Thu, Apr 28, 2016 at 05:18:30PM +0200, Noralf Trønnes wrote:
This patchset adds fbdev deferred io support to drm_fb_helper and drm_fb_cma_helper.
It channels fbdev mmap and fb_{write,fillrect,copyarea,imageblit} damage through the (struct drm_framebuffer_funcs)->dirty callback on the fb_helper framebuffer which will always run in process context.
I have also added patches that converts qxl and udl to use this deferred io support. I have only compile tested it, no functional testing. I know that qxl is purely a software thing so I could actually test it, but I have never used qemu so I'm not keen on spending a lot of time on that.
This was originally part of the tinydrm patchset.
Changes since v3:
- drm/fb-helper: Add fb_deferred_io support
- Don't use forward decl, move drm_fb_helper_dirty_work()
- Use DIV_ROUND_UP in drm_fb_helper_deferred_io()
Changes since v2:
- drm/rect: Add some drm_clip_rect utility functions
- This patch is dropped
- drm/fb-helper: Add fb_deferred_io support
- FB_DEFERRED_IO is now always selected by DRM_KMS_FB_HELPER, ifdef removed
- The drm_clip_rect utility functions are dropped, so open code it
- docs: use & to denote structs
- drm/qxl: Use drm_fb_helper deferred_io support
- The drm_clip_rect_{width/height} functions are dropped, so open code it
Changes since v1:
- drm/fb-helper: Add fb_deferred_io support
- Use a dedicated worker to run the framebuffer flushing like qxl does
- Add parameter descriptions to drm_fb_helper_deferred_io
- fbdev: fb_defio: Export fb_deferred_io_mmap
- Expand commit message
- drm/qxl: Use drm_fb_helper deferred_io support
- Add FIXME about special dirty() callback for fbdev
- Remove note in commit message about deferred worker, drm_fb_helper is similar to qxl now.
- drm/udl: Use drm_fb_helper deferred_io support
- No need to enable deferred_io by default since drm_fb_helper uses a dedicated worker for flushing
Changes since RFC:
- Fix drm_clip_rect use to be exclusive on x2/y2
- Put drm_clip_rect functions in drm_rect.{h,c}
- Take into account that (struct fb_ops *)->fb_{write,...}() can be called from atomic context (spin_lock_irqsave)
- Export fb_deferred_io_mmap()
- Add some more documentation
- Add qxl and udl patches
Noralf Trønnes (7): drm/udl: Change drm_fb_helper_sys_*() calls to sys_*() drm/qxl: Change drm_fb_helper_sys_*() calls to sys_*() drm/fb-helper: Add fb_deferred_io support fbdev: fb_defio: Export fb_deferred_io_mmap drm/fb-cma-helper: Add fb_deferred_io support drm/qxl: Use drm_fb_helper deferred_io support drm/udl: Use drm_fb_helper deferred_io support
Scrolled through them all once more, and didn't spot anything else. Great work! I plan to pull it all into drm-misc next week for 4.7 still. Please ping me in case I forget. -Daniel
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_fb_cma_helper.c | 178 ++++++++++++++++++++++++++-- drivers/gpu/drm/drm_fb_helper.c | 103 ++++++++++++++++- drivers/gpu/drm/qxl/qxl_display.c | 9 +- drivers/gpu/drm/qxl/qxl_drv.h | 7 +- drivers/gpu/drm/qxl/qxl_fb.c | 223 +++++++++--------------------------- drivers/gpu/drm/qxl/qxl_kms.c | 4 - drivers/gpu/drm/udl/udl_drv.h | 2 - drivers/gpu/drm/udl/udl_fb.c | 140 +--------------------- drivers/video/fbdev/core/fb_defio.c | 3 +- include/drm/drm_fb_cma_helper.h | 14 +++ include/drm/drm_fb_helper.h | 15 +++ include/linux/fb.h | 1 + 13 files changed, 372 insertions(+), 328 deletions(-)
-- 2.2.2
On Fri, Apr 29, 2016 at 04:57:05PM +0200, Daniel Vetter wrote:
On Thu, Apr 28, 2016 at 05:18:30PM +0200, Noralf Trønnes wrote:
This patchset adds fbdev deferred io support to drm_fb_helper and drm_fb_cma_helper.
It channels fbdev mmap and fb_{write,fillrect,copyarea,imageblit} damage through the (struct drm_framebuffer_funcs)->dirty callback on the fb_helper framebuffer which will always run in process context.
I have also added patches that converts qxl and udl to use this deferred io support. I have only compile tested it, no functional testing. I know that qxl is purely a software thing so I could actually test it, but I have never used qemu so I'm not keen on spending a lot of time on that.
This was originally part of the tinydrm patchset.
Changes since v3:
- drm/fb-helper: Add fb_deferred_io support
- Don't use forward decl, move drm_fb_helper_dirty_work()
- Use DIV_ROUND_UP in drm_fb_helper_deferred_io()
Changes since v2:
- drm/rect: Add some drm_clip_rect utility functions
- This patch is dropped
- drm/fb-helper: Add fb_deferred_io support
- FB_DEFERRED_IO is now always selected by DRM_KMS_FB_HELPER, ifdef removed
- The drm_clip_rect utility functions are dropped, so open code it
- docs: use & to denote structs
- drm/qxl: Use drm_fb_helper deferred_io support
- The drm_clip_rect_{width/height} functions are dropped, so open code it
Changes since v1:
- drm/fb-helper: Add fb_deferred_io support
- Use a dedicated worker to run the framebuffer flushing like qxl does
- Add parameter descriptions to drm_fb_helper_deferred_io
- fbdev: fb_defio: Export fb_deferred_io_mmap
- Expand commit message
- drm/qxl: Use drm_fb_helper deferred_io support
- Add FIXME about special dirty() callback for fbdev
- Remove note in commit message about deferred worker, drm_fb_helper is similar to qxl now.
- drm/udl: Use drm_fb_helper deferred_io support
- No need to enable deferred_io by default since drm_fb_helper uses a dedicated worker for flushing
Changes since RFC:
- Fix drm_clip_rect use to be exclusive on x2/y2
- Put drm_clip_rect functions in drm_rect.{h,c}
- Take into account that (struct fb_ops *)->fb_{write,...}() can be called from atomic context (spin_lock_irqsave)
- Export fb_deferred_io_mmap()
- Add some more documentation
- Add qxl and udl patches
Noralf Trønnes (7): drm/udl: Change drm_fb_helper_sys_*() calls to sys_*() drm/qxl: Change drm_fb_helper_sys_*() calls to sys_*() drm/fb-helper: Add fb_deferred_io support fbdev: fb_defio: Export fb_deferred_io_mmap drm/fb-cma-helper: Add fb_deferred_io support drm/qxl: Use drm_fb_helper deferred_io support drm/udl: Use drm_fb_helper deferred_io support
Scrolled through them all once more, and didn't spot anything else. Great work! I plan to pull it all into drm-misc next week for 4.7 still. Please ping me in case I forget.
And done, thanks a lot for your work!
Cheers, Daniel
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Noralf Trønnes -
Tomi Valkeinen