Re: [PATCH v2 2/2] drm/tinydrm: Make fb_dirty into a lower level hook

Den 23.03.2018 16.35, skrev Ville Syrjala:

From: Ville Syrjälä ville.syrjala@linux.intel.com

mipi_dbi_enable_flush() wants to call the fb->dirty() hook from the bowels of the .atomic_enable() hook. That prevents us from taking the plane mutex in fb->dirty() unless we also plumb down the acquire context.

Instead it seems simpler to split the fb->dirty() into a tinydrm specific lower level hook that can be called from mipi_dbi_enable_flush() and from a generic higher level tinydrm_fb_dirty() helper. As we don't have a tinydrm specific vfuncs table we'll just stick it into tinydrm_device directly for now.

v2: Deal with the fb->dirty() in tinydrm_display_pipe_update() as weel (Noralf)

Cc: "Noralf Trønnes" noralf@tronnes.org Cc: David Lechner david@lechnology.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com Reviewed-by: Noralf Trønnes noralf@tronnes.org

---

Tested mi0823qt which covers core and mipi-dbi:

Tested-by: Noralf Trønnes noralf@tronnes.org

drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 30 ++++++++++++++++++++++++++ drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 5 ++--- drivers/gpu/drm/tinydrm/ili9225.c | 23 ++++++-------------- drivers/gpu/drm/tinydrm/mi0283qt.c | 2 +- drivers/gpu/drm/tinydrm/mipi-dbi.c | 30 ++++++++++---------------- drivers/gpu/drm/tinydrm/repaper.c | 28 ++++++++---------------- drivers/gpu/drm/tinydrm/st7586.c | 23 ++++++-------------- drivers/gpu/drm/tinydrm/st7735r.c | 2 +- include/drm/tinydrm/mipi-dbi.h | 4 +++- include/drm/tinydrm/tinydrm-helpers.h | 5 +++++ include/drm/tinydrm/tinydrm.h | 4 ++++ 11 files changed, 78 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c index d1c3ce9ab294..dcd390163a4a 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c @@ -78,6 +78,36 @@ bool tinydrm_merge_clips(struct drm_clip_rect *dst, } EXPORT_SYMBOL(tinydrm_merge_clips);

+int tinydrm_fb_dirty(struct drm_framebuffer *fb,

•      struct drm_file *file_priv,
  

•      unsigned int flags, unsigned int color,
  

•      struct drm_clip_rect *clips,
  

•      unsigned int num_clips)
  

+{

• struct tinydrm_device *tdev = fb->dev->dev_private;
• struct drm_plane *plane = &tdev->pipe.plane;
• int ret = 0;
• drm_modeset_lock(&plane->mutex, NULL);
• /* fbdev can flush even when we're not interested */
• if (plane->state->fb == fb) {

• mutex_lock(&tdev->dirty_lock);
  

• ret = tdev->fb_dirty(fb, file_priv, flags,
  

• 		     color, clips, num_clips);
  

• mutex_unlock(&tdev->dirty_lock);
  

• }
• drm_modeset_unlock(&plane->mutex);
• if (ret)

• dev_err_once(fb->dev->dev,
  

• 	     "Failed to update display %d\n", ret);
  

• return ret;

+} +EXPORT_SYMBOL(tinydrm_fb_dirty);

• /**
  • tinydrm_memcpy - Copy clip buffer
  • @dst: Destination buffer

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c index 11ae950b0fc9..e68b528ae64d 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c @@ -125,9 +125,8 @@ void tinydrm_display_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_crtc *crtc = &tdev->pipe.crtc;

if (fb && (fb != old_state->fb)) {

• pipe->plane.fb = fb;
  

• if (fb->funcs->dirty)
  

• 	fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
  

• if (tdev->fb_dirty)
  

• 	tdev->fb_dirty(fb, NULL, 0, 0, NULL, 0);
  

  }

  if (crtc->state->event) {

diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c index 089d22798c8b..0874e877b111 100644 --- a/drivers/gpu/drm/tinydrm/ili9225.c +++ b/drivers/gpu/drm/tinydrm/ili9225.c @@ -88,14 +88,8 @@ static int ili9225_fb_dirty(struct drm_framebuffer *fb, bool full; void *tr;

• mutex_lock(&tdev->dirty_lock);
• if (!mipi->enabled)

• goto out_unlock;
  

• /* fbdev can flush even when we're not interested */
• if (tdev->pipe.plane.fb != fb)

• goto out_unlock;
  

• return 0;
  
  

  full = tinydrm_merge_clips(&clip, clips, num_clips, flags, fb->width, fb->height);

@@ -108,7 +102,7 @@ static int ili9225_fb_dirty(struct drm_framebuffer *fb, tr = mipi->tx_buf; ret = mipi_dbi_buf_copy(mipi->tx_buf, fb, &clip, swap); if (ret)

• 	goto out_unlock;
  

• 	return ret;
  

  } else { tr = cma_obj->vaddr; }

@@ -159,20 +153,13 @@ static int ili9225_fb_dirty(struct drm_framebuffer *fb, ret = mipi_dbi_command_buf(mipi, ILI9225_WRITE_DATA_TO_GRAM, tr, (clip.x2 - clip.x1) * (clip.y2 - clip.y1) * 2);

-out_unlock:

• mutex_unlock(&tdev->dirty_lock);

• if (ret)

• dev_err_once(fb->dev->dev, "Failed to update display %d\n",
  

• 	     ret);
  

• return ret; }

  static const struct drm_framebuffer_funcs ili9225_fb_funcs = { .destroy = drm_gem_fb_destroy, .create_handle = drm_gem_fb_create_handle,

• .dirty = ili9225_fb_dirty,

• .dirty = tinydrm_fb_dirty, };

  static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,

@@ -269,7 +256,7 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,

ili9225_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x1017);

• mipi_dbi_enable_flush(mipi);

• mipi_dbi_enable_flush(mipi, crtc_state, plane_state); }

  static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)

@@ -342,6 +329,8 @@ static int ili9225_init(struct device *dev, struct mipi_dbi *mipi, if (ret) return ret;

• tdev->fb_dirty = ili9225_fb_dirty;
• ret = tinydrm_display_pipe_init(tdev, pipe_funcs, DRM_MODE_CONNECTOR_VIRTUAL, ili9225_formats,

diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index 82ad9b61898e..4e6d2ee94e55 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -127,7 +127,7 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe, msleep(100);

out_enable:

• mipi_dbi_enable_flush(mipi);

• mipi_dbi_enable_flush(mipi, crtc_state, plane_state); }

  static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {

diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index 9e903812b573..4d1fb31a781f 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -219,14 +219,8 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb, bool full; void *tr;

• mutex_lock(&tdev->dirty_lock);
• if (!mipi->enabled)

• goto out_unlock;
  

• /* fbdev can flush even when we're not interested */
• if (tdev->pipe.plane.fb != fb)

• goto out_unlock;
  

• return 0;
  
  

  full = tinydrm_merge_clips(&clip, clips, num_clips, flags, fb->width, fb->height);

@@ -239,7 +233,7 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb, tr = mipi->tx_buf; ret = mipi_dbi_buf_copy(mipi->tx_buf, fb, &clip, swap); if (ret)

• 	goto out_unlock;
  

• 	return ret;
  

  } else { tr = cma_obj->vaddr; }

@@ -254,20 +248,13 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb, ret = mipi_dbi_command_buf(mipi, MIPI_DCS_WRITE_MEMORY_START, tr, (clip.x2 - clip.x1) * (clip.y2 - clip.y1) * 2);

-out_unlock:

• mutex_unlock(&tdev->dirty_lock);

• if (ret)

• dev_err_once(fb->dev->dev, "Failed to update display %d\n",
  

• 	     ret);
  

• return ret; }

  static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs = { .destroy = drm_gem_fb_destroy, .create_handle = drm_gem_fb_create_handle,

• .dirty = mipi_dbi_fb_dirty,

• .dirty = tinydrm_fb_dirty, };

  /**

@@ -278,13 +265,16 @@ static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs = {

• enables the backlight. Drivers can use this in their
• &drm_simple_display_pipe_funcs->enable callback.

*/ -void mipi_dbi_enable_flush(struct mipi_dbi *mipi) +void mipi_dbi_enable_flush(struct mipi_dbi *mipi,

• 	   struct drm_crtc_state *crtc_state,
  

• 	   struct drm_plane_state *plane_state)
  

  {

• struct drm_framebuffer *fb = mipi->tinydrm.pipe.plane.fb;

• struct tinydrm_device *tdev = &mipi->tinydrm;

• struct drm_framebuffer *fb = plane_state->fb;

  mipi->enabled = true; if (fb)

• fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
  

• tdev->fb_dirty(fb, NULL, 0, 0, NULL, 0);
  
  

  backlight_enable(mipi->backlight); }

@@ -381,6 +371,8 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi, if (ret) return ret;

• tdev->fb_dirty = mipi_dbi_fb_dirty;
• /* TODO: Maybe add DRM_MODE_CONNECTOR_SPI */ ret = tinydrm_display_pipe_init(tdev, pipe_funcs, DRM_MODE_CONNECTOR_VIRTUAL,

diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c index 33b4a71916e4..bb6f80a81899 100644 --- a/drivers/gpu/drm/tinydrm/repaper.c +++ b/drivers/gpu/drm/tinydrm/repaper.c @@ -540,14 +540,8 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, clip.y1 = 0; clip.y2 = fb->height;

• mutex_lock(&tdev->dirty_lock);
• if (!epd->enabled)

• goto out_unlock;
  

• /* fbdev can flush even when we're not interested */
• if (tdev->pipe.plane.fb != fb)

• goto out_unlock;
  

• return 0;
  
  

  repaper_get_temperature(epd);

@@ -555,16 +549,14 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, epd->factored_stage_time);

buf = kmalloc(fb->width * fb->height, GFP_KERNEL);

• if (!buf) {

• ret = -ENOMEM;
  

• goto out_unlock;
  

• }

• if (!buf)

• return -ENOMEM;
  
  

  if (import_attach) { ret = dma_buf_begin_cpu_access(import_attach->dmabuf, DMA_FROM_DEVICE); if (ret)

• 	goto out_unlock;
  

• 	goto out_free;
  

  }

  tinydrm_xrgb8888_to_gray8(buf, cma_obj->vaddr, fb, &clip);

@@ -573,7 +565,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, ret = dma_buf_end_cpu_access(import_attach->dmabuf, DMA_FROM_DEVICE); if (ret)

• 	goto out_unlock;
  

• 	goto out_free;
  

  }

  repaper_gray8_to_mono_reversed(buf, fb->width, fb->height);

@@ -625,11 +617,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, } }

-out_unlock:

• mutex_unlock(&tdev->dirty_lock);
• if (ret)

• DRM_DEV_ERROR(fb->dev->dev, "Failed to update display (%d)\n", ret);
  

+out_free: kfree(buf);

return ret; @@ -638,7 +626,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, static const struct drm_framebuffer_funcs repaper_fb_funcs = { .destroy = drm_gem_fb_destroy, .create_handle = drm_gem_fb_create_handle,

• .dirty = repaper_fb_dirty,

• .dirty = tinydrm_fb_dirty, };

  static void power_off(struct repaper_epd *epd)

@@ -1070,6 +1058,8 @@ static int repaper_probe(struct spi_device *spi) if (ret) return ret;

• tdev->fb_dirty = repaper_fb_dirty;
• ret = tinydrm_display_pipe_init(tdev, &repaper_pipe_funcs, DRM_MODE_CONNECTOR_VIRTUAL, repaper_formats,

diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c index bb08b293c8ce..22644b88199a 100644 --- a/drivers/gpu/drm/tinydrm/st7586.c +++ b/drivers/gpu/drm/tinydrm/st7586.c @@ -120,14 +120,8 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb, int start, end; int ret = 0;

• mutex_lock(&tdev->dirty_lock);
• if (!mipi->enabled)

• goto out_unlock;
  

• /* fbdev can flush even when we're not interested */
• if (tdev->pipe.plane.fb != fb)

• goto out_unlock;
  

• return 0;
  
  

  tinydrm_merge_clips(&clip, clips, num_clips, flags, fb->width, fb->height);

@@ -141,7 +135,7 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb,

ret = st7586_buf_copy(mipi->tx_buf, fb, &clip); if (ret)

• goto out_unlock;
  

• return ret;
  
  

  /* Pixels are packed 3 per byte */ start = clip.x1 / 3;

@@ -158,20 +152,13 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb, (u8 *)mipi->tx_buf, (end - start) * (clip.y2 - clip.y1));

-out_unlock:

• mutex_unlock(&tdev->dirty_lock);

• if (ret)

• dev_err_once(fb->dev->dev, "Failed to update display %d\n",
  

• 	     ret);
  

• return ret; }

  static const struct drm_framebuffer_funcs st7586_fb_funcs = { .destroy = drm_gem_fb_destroy, .create_handle = drm_gem_fb_create_handle,

• .dirty = st7586_fb_dirty,

• .dirty = tinydrm_fb_dirty, };

  static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,

@@ -238,7 +225,7 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,

mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);

• mipi_dbi_enable_flush(mipi);

• mipi_dbi_enable_flush(mipi, crtc_state, plane_state); }

  static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe)

@@ -278,6 +265,8 @@ static int st7586_init(struct device *dev, struct mipi_dbi *mipi, if (ret) return ret;

• tdev->fb_dirty = st7586_fb_dirty;
• ret = tinydrm_display_pipe_init(tdev, pipe_funcs, DRM_MODE_CONNECTOR_VIRTUAL, st7586_formats,

diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c index 19b28f8c78db..189a07894d36 100644 --- a/drivers/gpu/drm/tinydrm/st7735r.c +++ b/drivers/gpu/drm/tinydrm/st7735r.c @@ -99,7 +99,7 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe,

msleep(20);

• mipi_dbi_enable_flush(mipi);

• mipi_dbi_enable_flush(mipi, crtc_state, plane_state); }

  static const struct drm_simple_display_pipe_funcs jd_t18003_t01_pipe_funcs = {

diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h index 44e824af2ef6..b8ba58861986 100644 --- a/include/drm/tinydrm/mipi-dbi.h +++ b/include/drm/tinydrm/mipi-dbi.h @@ -67,7 +67,9 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi, const struct drm_simple_display_pipe_funcs *pipe_funcs, struct drm_driver *driver, const struct drm_display_mode *mode, unsigned int rotation); -void mipi_dbi_enable_flush(struct mipi_dbi *mipi); +void mipi_dbi_enable_flush(struct mipi_dbi *mipi,

• 	   struct drm_crtc_state *crtc_state,
  

• 	   struct drm_plane_state *plan_state);
  

  void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe); void mipi_dbi_hw_reset(struct mipi_dbi *mipi); bool mipi_dbi_display_is_on(struct mipi_dbi *mipi);

diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h index 0a4ddbc04c60..5b96f0b12c8c 100644 --- a/include/drm/tinydrm/tinydrm-helpers.h +++ b/include/drm/tinydrm/tinydrm-helpers.h @@ -36,6 +36,11 @@ static inline bool tinydrm_machine_little_endian(void) bool tinydrm_merge_clips(struct drm_clip_rect *dst, struct drm_clip_rect *src, unsigned int num_clips, unsigned int flags, u32 max_width, u32 max_height); +int tinydrm_fb_dirty(struct drm_framebuffer *fb,

•      struct drm_file *file_priv,
  

•      unsigned int flags, unsigned int color,
  

•      struct drm_clip_rect *clips,
  

•      unsigned int num_clips);
  

  void tinydrm_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb, struct drm_clip_rect *clip); void tinydrm_swab16(u16 *dst, void *vaddr, struct drm_framebuffer *fb,

diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h index 07a9a11fe19d..ea2c71081190 100644 --- a/include/drm/tinydrm/tinydrm.h +++ b/include/drm/tinydrm/tinydrm.h @@ -26,6 +26,10 @@ struct tinydrm_device { struct drm_simple_display_pipe pipe; struct mutex dirty_lock; const struct drm_framebuffer_funcs *fb_funcs;

• int (*fb_dirty)(struct drm_framebuffer *framebuffer,

• 	struct drm_file *file_priv, unsigned flags,
  

• 	unsigned color, struct drm_clip_rect *clips,
  

• 	unsigned num_clips);
  

  };

  static inline struct tinydrm_device *