From: Ville Syrjälä ville.syrjala@linux.intel.com
We'll need access to the plane state during .atomic_enable().
Performed with coccinelle: @r1@ identifier F =~ ".*enable$"; identifier P, CS; @@ F( struct drm_simple_display_pipe *P ,struct drm_crtc_state *CS + ,struct drm_plane_state *plane_state ) { ... }
@@ struct drm_simple_display_pipe *P; expression E; @@ { + struct drm_plane *plane; ... + plane = &P->plane; P->funcs->enable(P ,E + ,plane->state ); ... }
@@ identifier P, CS; @@ struct drm_simple_display_pipe_funcs { ... void (*enable)(struct drm_simple_display_pipe *P ,struct drm_crtc_state *CS + ,struct drm_plane_state *plane_state ); ... };
Cc: Marek Vasut marex@denx.de Cc: Eric Anholt eric@anholt.net Cc: David Lechner david@lechnology.com Cc: "Noralf Trønnes" noralf@tronnes.org Cc: Linus Walleij linus.walleij@linaro.org Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_simple_kms_helper.c | 4 +++- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 ++- drivers/gpu/drm/pl111/pl111_display.c | 3 ++- drivers/gpu/drm/tinydrm/ili9225.c | 3 ++- drivers/gpu/drm/tinydrm/mi0283qt.c | 3 ++- drivers/gpu/drm/tinydrm/repaper.c | 3 ++- drivers/gpu/drm/tinydrm/st7586.c | 3 ++- drivers/gpu/drm/tinydrm/st7735r.c | 3 ++- drivers/gpu/drm/tve200/tve200_display.c | 3 ++- include/drm/drm_simple_kms_helper.h | 3 ++- 10 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index 987a353c7f72..7a00455ca568 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -64,13 +64,15 @@ static int drm_simple_kms_crtc_check(struct drm_crtc *crtc, static void drm_simple_kms_crtc_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { + struct drm_plane *plane; struct drm_simple_display_pipe *pipe;
pipe = container_of(crtc, struct drm_simple_display_pipe, crtc); if (!pipe->funcs || !pipe->funcs->enable) return;
- pipe->funcs->enable(pipe, crtc->state); + plane = &pipe->plane; + pipe->funcs->enable(pipe, crtc->state, plane->state); }
static void drm_simple_kms_crtc_disable(struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 5cae8db9dcd4..b9c7507813db 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -99,7 +99,8 @@ static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = { };
static void mxsfb_pipe_enable(struct drm_simple_display_pipe *pipe, - struct drm_crtc_state *crtc_state) + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) { struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe);
diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c index 310646427907..1fee578e05b0 100644 --- a/drivers/gpu/drm/pl111/pl111_display.c +++ b/drivers/gpu/drm/pl111/pl111_display.c @@ -120,7 +120,8 @@ static int pl111_display_check(struct drm_simple_display_pipe *pipe, }
static void pl111_display_enable(struct drm_simple_display_pipe *pipe, - struct drm_crtc_state *cstate) + struct drm_crtc_state *cstate, + struct drm_plane_state *plane_state) { struct drm_crtc *crtc = &pipe->crtc; struct drm_plane *plane = &pipe->plane; diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c index a0759502b81a..089d22798c8b 100644 --- a/drivers/gpu/drm/tinydrm/ili9225.c +++ b/drivers/gpu/drm/tinydrm/ili9225.c @@ -176,7 +176,8 @@ static const struct drm_framebuffer_funcs ili9225_fb_funcs = { };
static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe, - struct drm_crtc_state *crtc_state) + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index d8ed6e6f8e05..82ad9b61898e 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -49,7 +49,8 @@ #define ILI9341_MADCTL_MY BIT(7)
static void mi0283qt_enable(struct drm_simple_display_pipe *pipe, - struct drm_crtc_state *crtc_state) + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c index 75740630c410..33b4a71916e4 100644 --- a/drivers/gpu/drm/tinydrm/repaper.c +++ b/drivers/gpu/drm/tinydrm/repaper.c @@ -659,7 +659,8 @@ static void power_off(struct repaper_epd *epd) }
static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe, - struct drm_crtc_state *crtc_state) + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct repaper_epd *epd = epd_from_tinydrm(tdev); diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c index a6396ef9cc4a..bb08b293c8ce 100644 --- a/drivers/gpu/drm/tinydrm/st7586.c +++ b/drivers/gpu/drm/tinydrm/st7586.c @@ -175,7 +175,8 @@ static const struct drm_framebuffer_funcs st7586_fb_funcs = { };
static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe, - struct drm_crtc_state *crtc_state) + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c index 67d197ecfc4b..19b28f8c78db 100644 --- a/drivers/gpu/drm/tinydrm/st7735r.c +++ b/drivers/gpu/drm/tinydrm/st7735r.c @@ -37,7 +37,8 @@ #define ST7735R_MV BIT(5)
static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe, - struct drm_crtc_state *crtc_state) + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) { struct tinydrm_device *tdev = pipe_to_tinydrm(pipe); struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev); diff --git a/drivers/gpu/drm/tve200/tve200_display.c b/drivers/gpu/drm/tve200/tve200_display.c index db397fcb345a..108f3b2b5d25 100644 --- a/drivers/gpu/drm/tve200/tve200_display.c +++ b/drivers/gpu/drm/tve200/tve200_display.c @@ -120,7 +120,8 @@ static int tve200_display_check(struct drm_simple_display_pipe *pipe, }
static void tve200_display_enable(struct drm_simple_display_pipe *pipe, - struct drm_crtc_state *cstate) + struct drm_crtc_state *cstate, + struct drm_plane_state *plane_state) { struct drm_crtc *crtc = &pipe->crtc; struct drm_plane *plane = &pipe->plane; diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h index 1b4e352143fd..b02793742317 100644 --- a/include/drm/drm_simple_kms_helper.h +++ b/include/drm/drm_simple_kms_helper.h @@ -64,7 +64,8 @@ struct drm_simple_display_pipe_funcs { * This hook is optional. */ void (*enable)(struct drm_simple_display_pipe *pipe, - struct drm_crtc_state *crtc_state); + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state); /** * @disable: *
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.
Cc: "Noralf Trønnes" noralf@tronnes.org Cc: David Lechner david@lechnology.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 30 ++++++++++++++++++++++++++ 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 ++++ 10 files changed, 76 insertions(+), 75 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/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 *
Den 22.03.2018 21.27, 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.
The long term goal is to try and get rid of tinydrm.ko by moving stuff elsewhere as it's kind of a middle layer. So I'd really like to avoid adding a callback like this. Hopefully we can work out a solution based on my suggestion in the 'drm: Eliminate plane->fb/crtc usage for atomic drivers' thread.
If we do need a hook, I prefer that we add it to drm_simple_display_pipe_funcs.
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
drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 30 ++++++++++++++++++++++++++ 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 ++++ 10 files changed, 76 insertions(+), 75 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/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;@@ -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;@@ -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);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);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 *
On Fri, Mar 23, 2018 at 12:43:58AM +0100, Noralf Trønnes wrote:
Den 22.03.2018 21.27, 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.
The long term goal is to try and get rid of tinydrm.ko by moving stuff elsewhere as it's kind of a middle layer. So I'd really like to avoid adding a callback like this. Hopefully we can work out a solution based on my suggestion in the 'drm: Eliminate plane->fb/crtc usage for atomic drivers' thread.
I don't want to start redesigning the entire architecture at this point. That would also cause a bigger risk of regression and then we'd potentially have to try and revert the entire plane->fb thing, which would not be fun if any significant changes have occurred in the meantime.
If we do need a hook, I prefer that we add it to drm_simple_display_pipe_funcs.
If you have plans to redesign tinydrm anyway it seems to me that a temporary hook in tinydrm is may be a bit less intrusive than inflicting it on the simple_kms_helper. But if you do prefer drm_simple_display_pipe_funcs I can move it there of course.
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
drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 30 ++++++++++++++++++++++++++ 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 ++++ 10 files changed, 76 insertions(+), 75 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/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;@@ -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;@@ -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);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);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 *
Den 23.03.2018 12.31, skrev Ville Syrjälä:
On Fri, Mar 23, 2018 at 12:43:58AM +0100, Noralf Trønnes wrote:
Den 22.03.2018 21.27, 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.
The long term goal is to try and get rid of tinydrm.ko by moving stuff elsewhere as it's kind of a middle layer. So I'd really like to avoid adding a callback like this. Hopefully we can work out a solution based on my suggestion in the 'drm: Eliminate plane->fb/crtc usage for atomic drivers' thread.
I don't want to start redesigning the entire architecture at this point. That would also cause a bigger risk of regression and then we'd potentially have to try and revert the entire plane->fb thing, which would not be fun if any significant changes have occurred in the meantime.
If we do need a hook, I prefer that we add it to drm_simple_display_pipe_funcs.
If you have plans to redesign tinydrm anyway it seems to me that a temporary hook in tinydrm is may be a bit less intrusive than inflicting it on the simple_kms_helper.
Yes you're right of course, what was I thinking.
You missed out on one call site:
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c index 11ae950b0fc9..7924eb59c2e1 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c @@ -124,11 +124,8 @@ void tinydrm_display_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_framebuffer *fb = pipe->plane.state->fb; 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 (fb && (fb != old_state->fb)) + tdev->fb_dirty(fb, NULL, 0, 0, NULL, 0);
if (crtc->state->event) { spin_lock_irq(&crtc->dev->event_lock);
With that fixed, series is:
Reviewed-by: Noralf Trønnes noralf@tronnes.org
To be honest I don't understand why it's necessary to wire through the plane state to the enable hook instead of just looking it up on the pipe object. You look it up on the pipe in tinydrm_fb_dirty(), and I look up the new plane state and crtc state on the pipe in the update hook.
Noralf.
But if you do prefer drm_simple_display_pipe_funcs I can move it there of course.
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
drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 30 ++++++++++++++++++++++++++ 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 ++++ 10 files changed, 76 insertions(+), 75 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/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;@@ -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;@@ -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);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);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 *
On Fri, Mar 23, 2018 at 02:37:23PM +0100, Noralf Trønnes wrote:
Den 23.03.2018 12.31, skrev Ville Syrjälä:
On Fri, Mar 23, 2018 at 12:43:58AM +0100, Noralf Trønnes wrote:
Den 22.03.2018 21.27, 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.
The long term goal is to try and get rid of tinydrm.ko by moving stuff elsewhere as it's kind of a middle layer. So I'd really like to avoid adding a callback like this. Hopefully we can work out a solution based on my suggestion in the 'drm: Eliminate plane->fb/crtc usage for atomic drivers' thread.
I don't want to start redesigning the entire architecture at this point. That would also cause a bigger risk of regression and then we'd potentially have to try and revert the entire plane->fb thing, which would not be fun if any significant changes have occurred in the meantime.
If we do need a hook, I prefer that we add it to drm_simple_display_pipe_funcs.
If you have plans to redesign tinydrm anyway it seems to me that a temporary hook in tinydrm is may be a bit less intrusive than inflicting it on the simple_kms_helper.
Yes you're right of course, what was I thinking.
You missed out on one call site:
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c index 11ae950b0fc9..7924eb59c2e1 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c @@ -124,11 +124,8 @@ void tinydrm_display_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_framebuffer *fb = pipe->plane.state->fb; 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 (fb && (fb != old_state->fb)) + tdev->fb_dirty(fb, NULL, 0, 0, NULL, 0);
if (crtc->state->event) { spin_lock_irq(&crtc->dev->event_lock);
With that fixed, series is:
Reviewed-by: Noralf Trønnes noralf@tronnes.org
Awesome. Thanks. And thanks for catching that extra dirty() call. I'll go and fix it up.
To be honest I don't understand why it's necessary to wire through the plane state to the enable hook instead of just looking it up on the pipe object. You look it up on the pipe in tinydrm_fb_dirty(), and I look up the new plane state and crtc state on the pipe in the update hook.
The use of plane->state etc. isn't really the best practice. What we really should be doing is plumbing down the drm_atomic_state instead, which would allow us to look up the correct new state explicitly. That said, I guess there is something in the helpers to prevent the next swap_state() from overtaking the previous commit, so supposedly using plane->state etc. does actually result in the correct thing currently.
In this case since we were already plumbing down the new crtc state I figured I'd just plumb down its friend as well.
Noralf.
But if you do prefer drm_simple_display_pipe_funcs I can move it there of course.
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
drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 30 ++++++++++++++++++++++++++ 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 ++++ 10 files changed, 76 insertions(+), 75 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/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;@@ -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;@@ -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);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);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 *
On Fri, Mar 23, 2018 at 03:58:06PM +0200, Ville Syrjälä wrote:
On Fri, Mar 23, 2018 at 02:37:23PM +0100, Noralf Trønnes wrote:
Den 23.03.2018 12.31, skrev Ville Syrjälä:
On Fri, Mar 23, 2018 at 12:43:58AM +0100, Noralf Trønnes wrote:
Den 22.03.2018 21.27, 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.
The long term goal is to try and get rid of tinydrm.ko by moving stuff elsewhere as it's kind of a middle layer. So I'd really like to avoid adding a callback like this. Hopefully we can work out a solution based on my suggestion in the 'drm: Eliminate plane->fb/crtc usage for atomic drivers' thread.
I don't want to start redesigning the entire architecture at this point. That would also cause a bigger risk of regression and then we'd potentially have to try and revert the entire plane->fb thing, which would not be fun if any significant changes have occurred in the meantime.
If we do need a hook, I prefer that we add it to drm_simple_display_pipe_funcs.
If you have plans to redesign tinydrm anyway it seems to me that a temporary hook in tinydrm is may be a bit less intrusive than inflicting it on the simple_kms_helper.
Yes you're right of course, what was I thinking.
You missed out on one call site:
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c index 11ae950b0fc9..7924eb59c2e1 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c @@ -124,11 +124,8 @@ void tinydrm_display_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_framebuffer *fb = pipe->plane.state->fb; 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 (fb && (fb != old_state->fb)) + tdev->fb_dirty(fb, NULL, 0, 0, NULL, 0);
if (crtc->state->event) { spin_lock_irq(&crtc->dev->event_lock);
With that fixed, series is:
Reviewed-by: Noralf Trønnes noralf@tronnes.org
Awesome. Thanks. And thanks for catching that extra dirty() call. I'll go and fix it up.
OK, I posted the fixed version.
Would you be interested in running some kind of smoke test on this before I push it? I'd hate to break things for you, and unfortunately (or maybe fortunately :) I don't have any hardware to test this.
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 --- 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 *
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;@@ -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;@@ -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);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);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 *
On Fri, Mar 23, 2018 at 05:28:03PM +0100, Noralf Trønnes wrote:
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
Thanks. Both patches now pushed to drm-misc-next.
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;@@ -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;@@ -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);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);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 *
On 03/23/2018 10:35 AM, Ville Syrjala wrote:
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
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
...
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);
s/plan_state/plane_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);
On 03/22/2018 03:27 PM, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
We'll need access to the plane state during .atomic_enable().
Some more details in the commit message would be useful. It is not clear to me why this change is being made.
On Sat, Mar 24, 2018 at 12:26:32PM -0500, David Lechner wrote:
On 03/22/2018 03:27 PM, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
We'll need access to the plane state during .atomic_enable().
Some more details in the commit message would be useful. It is not clear to me why this change is being made.
"tinydrm enable hook wants to play around with the new fb in .atomic_enable(), thus we'll need access to the plane state."
Better? Worse?
On 03/27/2018 05:07 AM, Ville Syrjälä wrote:
On Sat, Mar 24, 2018 at 12:26:32PM -0500, David Lechner wrote:
On 03/22/2018 03:27 PM, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
We'll need access to the plane state during .atomic_enable().
Some more details in the commit message would be useful. It is not clear to me why this change is being made.
"tinydrm enable hook wants to play around with the new fb in .atomic_enable(), thus we'll need access to the plane state."
Better? Worse?
Better.
dri-devel@lists.freedesktop.org
-
David Lechner -
Noralf Trønnes -
Ville Syrjala -
Ville Syrjälä