Re: [PATCH v3 3/7] drm/fb-helper: Add fb_deferred_io support

2016-04-28 Thread Daniel Vetter 
On Wed, Apr 27, 2016 at 09:24:52PM +0200, Noralf Trønnes wrote:
> 
> Den 27.04.2016 21:20, skrev Daniel Vetter:
> >On Wed, Apr 27, 2016 at 08:16:32PM +0200, 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 
> >>---
> >>
> >>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 | 109 
> >> +++-
> >>  include/drm/drm_fb_helper.h |  15 ++
> >>  3 files changed, 124 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..5112b5d 100644
> >>--- a/drivers/gpu/drm/drm_fb_helper.c
> >>+++ b/drivers/gpu/drm/drm_fb_helper.c
> >>@@ -48,6 +48,8 @@ MODULE_PARM_DESC(fbdev_emulation,
> >>
> >>  static LIST_HEAD(kernel_fb_helper_list);
> >>
> >>+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper);
> >Instead of this forward decl just inline the few setup commands into
> >drm_fb_helper_prepare.
> 
> That would mean that I have to forward declare drm_fb_helper_dirty_work()
> instead. Is that better?
> Or should I relocate the functions?

Yeah, just move them all I think. This means that usually the setup
function for a component is at the very bottom, and also that you have the
inverted reading order with first reading leaf/helper functions, then the
bigger ones that assemble things together. Personally I find that
backwards, but otoh consistency is more important. And avoid forward decls
for static functions is standard style in the kernel.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v3 3/7] drm/fb-helper: Add fb_deferred_io support

2016-04-28 Thread Daniel Vetter 
On Wed, Apr 27, 2016 at 09:24:52PM +0200, Noralf Trønnes wrote:
> 
> Den 27.04.2016 21:20, skrev Daniel Vetter:
> >On Wed, Apr 27, 2016 at 08:16:32PM +0200, 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 
> >>---
> >>
> >>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 | 109 
> >> +++-
> >>  include/drm/drm_fb_helper.h |  15 ++
> >>  3 files changed, 124 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..5112b5d 100644
> >>--- a/drivers/gpu/drm/drm_fb_helper.c
> >>+++ b/drivers/gpu/drm/drm_fb_helper.c
> >>@@ -48,6 +48,8 @@ MODULE_PARM_DESC(fbdev_emulation,
> >>
> >>  static LIST_HEAD(kernel_fb_helper_list);
> >>
> >>+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper);
> >Instead of this forward decl just inline the few setup commands into
> >drm_fb_helper_prepare.
> 
> That would mean that I have to forward declare drm_fb_helper_dirty_work()
> instead. Is that better?
> Or should I relocate the functions?

Yeah, just move them all I think. This means that usually the setup
function for a component is at the very bottom, and also that you have the
inverted reading order with first reading leaf/helper functions, then the
bigger ones that assemble things together. Personally I find that
backwards, but otoh consistency is more important. And avoid forward decls
for static functions is standard style in the kernel.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v3 3/7] drm/fb-helper: Add fb_deferred_io support

2016-04-27 Thread Noralf Trønnes 


Den 27.04.2016 21:20, skrev Daniel Vetter:

On Wed, Apr 27, 2016 at 08:16:32PM +0200, 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 
---

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 | 109 +++-
  include/drm/drm_fb_helper.h |  15 ++
  3 files changed, 124 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..5112b5d 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -48,6 +48,8 @@ MODULE_PARM_DESC(fbdev_emulation,

  static LIST_HEAD(kernel_fb_helper_list);

+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper);

Instead of this forward decl just inline the few setup commands into
drm_fb_helper_prepare.


That would mean that I have to forward declare drm_fb_helper_dirty_work()
instead. Is that better?
Or should I relocate the functions?


+
  /**
   * DOC: fbdev helpers
   *
@@ -84,6 +86,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 _framebuffer_funcs ->dirty is
+ * set, the drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit}
+ * functions will accumulate changes and schedule _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.
   */

  /**
@@ -650,6 +661,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(>kernel_fb_list);
+   drm_fb_helper_dirty_init(helper);
helper->funcs = funcs;
helper->dev = dev;
  }
@@ -834,6 +846,82 @@ 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_work(struct work_struct *work)
+{
+   struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper,
+   dirty_work);
+   struct drm_clip_rect *clip = >dirty_clip;
+   struct drm_clip_rect clip_copy;
+   unsigned long flags;
+
+   spin_lock_irqsave(>dirty_lock, flags);
+   clip_copy = *clip;
+   clip->x1 = clip->y1 = ~0;
+   clip->x2 = clip->y2 = 0;
+   spin_unlock_irqrestore(>dirty_lock, flags);
+
+   helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, _copy, 1);
+}
+
+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper)
+{
+   spin_lock_init(>dirty_lock);
+   INIT_WORK(>dirty_work, drm_fb_helper_dirty_work);
+   helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
+}
+
+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 = >dirty_clip;
+   unsigned long flags;
+
+   if (!helper->fb->funcs->dirty)
+   return;
+
+   spin_lock_irqsave(>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(>dirty_lock, flags);
+
+   schedule_work(>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 _deferred_io ->deferred_io
+ * callback function for flushing the fbdev mmap

Re: [PATCH v3 3/7] drm/fb-helper: Add fb_deferred_io support

2016-04-27 Thread Noralf Trønnes 


Den 27.04.2016 21:20, skrev Daniel Vetter:

On Wed, Apr 27, 2016 at 08:16:32PM +0200, 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 
---

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 | 109 +++-
  include/drm/drm_fb_helper.h |  15 ++
  3 files changed, 124 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..5112b5d 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -48,6 +48,8 @@ MODULE_PARM_DESC(fbdev_emulation,

  static LIST_HEAD(kernel_fb_helper_list);

+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper);

Instead of this forward decl just inline the few setup commands into
drm_fb_helper_prepare.


That would mean that I have to forward declare drm_fb_helper_dirty_work()
instead. Is that better?
Or should I relocate the functions?


+
  /**
   * DOC: fbdev helpers
   *
@@ -84,6 +86,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 _framebuffer_funcs ->dirty is
+ * set, the drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit}
+ * functions will accumulate changes and schedule _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.
   */

  /**
@@ -650,6 +661,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(>kernel_fb_list);
+   drm_fb_helper_dirty_init(helper);
helper->funcs = funcs;
helper->dev = dev;
  }
@@ -834,6 +846,82 @@ 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_work(struct work_struct *work)
+{
+   struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper,
+   dirty_work);
+   struct drm_clip_rect *clip = >dirty_clip;
+   struct drm_clip_rect clip_copy;
+   unsigned long flags;
+
+   spin_lock_irqsave(>dirty_lock, flags);
+   clip_copy = *clip;
+   clip->x1 = clip->y1 = ~0;
+   clip->x2 = clip->y2 = 0;
+   spin_unlock_irqrestore(>dirty_lock, flags);
+
+   helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, _copy, 1);
+}
+
+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper)
+{
+   spin_lock_init(>dirty_lock);
+   INIT_WORK(>dirty_work, drm_fb_helper_dirty_work);
+   helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
+}
+
+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 = >dirty_clip;
+   unsigned long flags;
+
+   if (!helper->fb->funcs->dirty)
+   return;
+
+   spin_lock_irqsave(>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(>dirty_lock, flags);
+
+   schedule_work(>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 _deferred_io ->deferred_io
+ * callback function for flushing the fbdev mmap writes.
+ */
+void

Re: [PATCH v3 3/7] drm/fb-helper: Add fb_deferred_io support

2016-04-27 Thread Daniel Vetter 
On Wed, Apr 27, 2016 at 08:16:32PM +0200, 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 
> ---
> 
> 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 | 109 
> +++-
>  include/drm/drm_fb_helper.h |  15 ++
>  3 files changed, 124 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..5112b5d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -48,6 +48,8 @@ MODULE_PARM_DESC(fbdev_emulation,
> 
>  static LIST_HEAD(kernel_fb_helper_list);
> 
> +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper);

Instead of this forward decl just inline the few setup commands into
drm_fb_helper_prepare.

> +
>  /**
>   * DOC: fbdev helpers
>   *
> @@ -84,6 +86,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 _framebuffer_funcs ->dirty is
> + * set, the drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit}
> + * functions will accumulate changes and schedule _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.
>   */
> 
>  /**
> @@ -650,6 +661,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(>kernel_fb_list);
> + drm_fb_helper_dirty_init(helper);
>   helper->funcs = funcs;
>   helper->dev = dev;
>  }
> @@ -834,6 +846,82 @@ 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_work(struct work_struct *work)
> +{
> + struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper,
> + dirty_work);
> + struct drm_clip_rect *clip = >dirty_clip;
> + struct drm_clip_rect clip_copy;
> + unsigned long flags;
> +
> + spin_lock_irqsave(>dirty_lock, flags);
> + clip_copy = *clip;
> + clip->x1 = clip->y1 = ~0;
> + clip->x2 = clip->y2 = 0;
> + spin_unlock_irqrestore(>dirty_lock, flags);
> +
> + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, _copy, 1);
> +}
> +
> +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper)
> +{
> + spin_lock_init(>dirty_lock);
> + INIT_WORK(>dirty_work, drm_fb_helper_dirty_work);
> + helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
> +}
> +
> +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 = >dirty_clip;
> + unsigned long flags;
> +
> + if (!helper->fb->funcs->dirty)
> + return;
> +
> + spin_lock_irqsave(>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(>dirty_lock, flags);
> +
> + schedule_work(>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 _deferred_io ->deferred_io
> + * callback function for flushing the fbdev mmap writes.
> + */

Re: [PATCH v3 3/7] drm/fb-helper: Add fb_deferred_io support

2016-04-27 Thread Daniel Vetter 
On Wed, Apr 27, 2016 at 08:16:32PM +0200, 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 
> ---
> 
> 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 | 109 
> +++-
>  include/drm/drm_fb_helper.h |  15 ++
>  3 files changed, 124 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..5112b5d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -48,6 +48,8 @@ MODULE_PARM_DESC(fbdev_emulation,
> 
>  static LIST_HEAD(kernel_fb_helper_list);
> 
> +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper);

Instead of this forward decl just inline the few setup commands into
drm_fb_helper_prepare.

> +
>  /**
>   * DOC: fbdev helpers
>   *
> @@ -84,6 +86,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 _framebuffer_funcs ->dirty is
> + * set, the drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit}
> + * functions will accumulate changes and schedule _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.
>   */
> 
>  /**
> @@ -650,6 +661,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(>kernel_fb_list);
> + drm_fb_helper_dirty_init(helper);
>   helper->funcs = funcs;
>   helper->dev = dev;
>  }
> @@ -834,6 +846,82 @@ 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_work(struct work_struct *work)
> +{
> + struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper,
> + dirty_work);
> + struct drm_clip_rect *clip = >dirty_clip;
> + struct drm_clip_rect clip_copy;
> + unsigned long flags;
> +
> + spin_lock_irqsave(>dirty_lock, flags);
> + clip_copy = *clip;
> + clip->x1 = clip->y1 = ~0;
> + clip->x2 = clip->y2 = 0;
> + spin_unlock_irqrestore(>dirty_lock, flags);
> +
> + helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, _copy, 1);
> +}
> +
> +static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper)
> +{
> + spin_lock_init(>dirty_lock);
> + INIT_WORK(>dirty_work, drm_fb_helper_dirty_work);
> + helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
> +}
> +
> +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 = >dirty_clip;
> + unsigned long flags;
> +
> + if (!helper->fb->funcs->dirty)
> + return;
> +
> + spin_lock_irqsave(>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(>dirty_lock, flags);
> +
> + schedule_work(>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 _deferred_io ->deferred_io
> + * callback function for flushing the fbdev mmap writes.
> + */
> +void

[PATCH v3 3/7] drm/fb-helper: Add fb_deferred_io support

2016-04-27 Thread Noralf Trønnes 
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 
---

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 | 109 +++-
 include/drm/drm_fb_helper.h |  15 ++
 3 files changed, 124 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..5112b5d 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -48,6 +48,8 @@ MODULE_PARM_DESC(fbdev_emulation,

 static LIST_HEAD(kernel_fb_helper_list);

+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper);
+
 /**
  * DOC: fbdev helpers
  *
@@ -84,6 +86,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 _framebuffer_funcs ->dirty is
+ * set, the drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit}
+ * functions will accumulate changes and schedule _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.
  */

 /**
@@ -650,6 +661,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(>kernel_fb_list);
+   drm_fb_helper_dirty_init(helper);
helper->funcs = funcs;
helper->dev = dev;
 }
@@ -834,6 +846,82 @@ 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_work(struct work_struct *work)
+{
+   struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper,
+   dirty_work);
+   struct drm_clip_rect *clip = >dirty_clip;
+   struct drm_clip_rect clip_copy;
+   unsigned long flags;
+
+   spin_lock_irqsave(>dirty_lock, flags);
+   clip_copy = *clip;
+   clip->x1 = clip->y1 = ~0;
+   clip->x2 = clip->y2 = 0;
+   spin_unlock_irqrestore(>dirty_lock, flags);
+
+   helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, _copy, 1);
+}
+
+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper)
+{
+   spin_lock_init(>dirty_lock);
+   INIT_WORK(>dirty_work, drm_fb_helper_dirty_work);
+   helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
+}
+
+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 = >dirty_clip;
+   unsigned long flags;
+
+   if (!helper->fb->funcs->dirty)
+   return;
+
+   spin_lock_irqsave(>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(>dirty_lock, flags);
+
+   schedule_work(>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 _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 <<

[PATCH v3 3/7] drm/fb-helper: Add fb_deferred_io support

2016-04-27 Thread Noralf Trønnes 
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 
---

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 | 109 +++-
 include/drm/drm_fb_helper.h |  15 ++
 3 files changed, 124 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..5112b5d 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -48,6 +48,8 @@ MODULE_PARM_DESC(fbdev_emulation,

 static LIST_HEAD(kernel_fb_helper_list);

+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper);
+
 /**
  * DOC: fbdev helpers
  *
@@ -84,6 +86,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 _framebuffer_funcs ->dirty is
+ * set, the drm_fb_helper_{cfb,sys}_{write,fillrect,copyarea,imageblit}
+ * functions will accumulate changes and schedule _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.
  */

 /**
@@ -650,6 +661,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(>kernel_fb_list);
+   drm_fb_helper_dirty_init(helper);
helper->funcs = funcs;
helper->dev = dev;
 }
@@ -834,6 +846,82 @@ 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_work(struct work_struct *work)
+{
+   struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper,
+   dirty_work);
+   struct drm_clip_rect *clip = >dirty_clip;
+   struct drm_clip_rect clip_copy;
+   unsigned long flags;
+
+   spin_lock_irqsave(>dirty_lock, flags);
+   clip_copy = *clip;
+   clip->x1 = clip->y1 = ~0;
+   clip->x2 = clip->y2 = 0;
+   spin_unlock_irqrestore(>dirty_lock, flags);
+
+   helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, _copy, 1);
+}
+
+static void drm_fb_helper_dirty_init(struct drm_fb_helper *helper)
+{
+   spin_lock_init(>dirty_lock);
+   INIT_WORK(>dirty_work, drm_fb_helper_dirty_work);
+   helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0;
+}
+
+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 = >dirty_clip;
+   unsigned long flags;
+
+   if (!helper->fb->funcs->dirty)
+   return;
+
+   spin_lock_irqsave(>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(>dirty_lock, flags);
+
+   schedule_work(>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 _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;
+