* [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
@ 2018-03-22 20:27 Ville Syrjala
2018-03-22 20:27 ` [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook Ville Syrjala
` (7 more replies)
0 siblings, 8 replies; 20+ messages in thread
From: Ville Syrjala @ 2018-03-22 20:27 UTC (permalink / raw)
To: dri-devel
Cc: Marek Vasut, David Lechner, intel-gfx, Noralf Trønnes,
Linus Walleij
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:
*
--
2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook
2018-03-22 20:27 [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Ville Syrjala
@ 2018-03-22 20:27 ` Ville Syrjala
2018-03-22 23:43 ` Noralf Trønnes
2018-03-23 15:35 ` [PATCH v2 " Ville Syrjala
2018-03-22 21:11 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Patchwork
` (6 subsequent siblings)
7 siblings, 2 replies; 20+ messages in thread
From: Ville Syrjala @ 2018-03-22 20:27 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Noralf Trønnes, David Lechner
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 *
--
2.16.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
2018-03-22 20:27 [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Ville Syrjala
2018-03-22 20:27 ` [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook Ville Syrjala
@ 2018-03-22 21:11 ` Patchwork
2018-03-22 21:26 ` ✓ Fi.CI.BAT: success " Patchwork
` (5 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-03-22 21:11 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
URL : https://patchwork.freedesktop.org/series/40514/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
e90adeef964f drm/simple-kms-helper: Plumb plane state to the enable hook
f3ea4037d879 drm/tinydrm: Make fb_dirty into a lower level hook
-:430: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#430: FILE: include/drm/tinydrm/tinydrm.h:30:
+ struct drm_file *file_priv, unsigned flags,
-:431: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#431: FILE: include/drm/tinydrm/tinydrm.h:31:
+ unsigned color, struct drm_clip_rect *clips,
-:432: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#432: FILE: include/drm/tinydrm/tinydrm.h:32:
+ unsigned num_clips);
total: 0 errors, 3 warnings, 0 checks, 344 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
2018-03-22 20:27 [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Ville Syrjala
2018-03-22 20:27 ` [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook Ville Syrjala
2018-03-22 21:11 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Patchwork
@ 2018-03-22 21:26 ` Patchwork
2018-03-23 0:26 ` ✓ Fi.CI.IGT: " Patchwork
` (4 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-03-22 21:26 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
URL : https://patchwork.freedesktop.org/series/40514/
State : success
== Summary ==
Series 40514v1 series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
https://patchwork.freedesktop.org/api/1.0/series/40514/revisions/1/mbox/
---- Known issues:
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-b-frame-sequence:
incomplete -> PASS (fi-cnl-y3) fdo#103191
Subgroup suspend-read-crc-pipe-c:
pass -> INCOMPLETE (fi-bxt-dsi) fdo#103927
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:433s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:445s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:379s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:536s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:298s
fi-bxt-dsi total:243 pass:216 dwarn:0 dfail:0 fail:0 skip:26
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:516s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:518s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:504s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:406s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:509s
fi-cnl-drrs total:285 pass:254 dwarn:3 dfail:0 fail:0 skip:28 time:519s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:587s
fi-elk-e7500 total:285 pass:225 dwarn:1 dfail:0 fail:0 skip:59 time:424s
fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:318s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:539s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:403s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:419s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:476s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:433s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:474s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:470s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:515s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:652s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:439s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:537s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:502s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:494s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:426s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:442s
fi-snb-2520m total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:399s
fi-cfl-s3 failed to connect after reboot
40fcdd23bec787a5913496f2b11c5d26bdff985a drm-tip: 2018y-03m-22d-15h-28m-32s UTC integration manifest
f3ea4037d879 drm/tinydrm: Make fb_dirty into a lower level hook
e90adeef964f drm/simple-kms-helper: Plumb plane state to the enable hook
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8459/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook
2018-03-22 20:27 ` [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook Ville Syrjala
@ 2018-03-22 23:43 ` Noralf Trønnes
2018-03-23 11:31 ` Ville Syrjälä
2018-03-23 15:35 ` [PATCH v2 " Ville Syrjala
1 sibling, 1 reply; 20+ messages in thread
From: Noralf Trønnes @ 2018-03-22 23:43 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx, David Lechner
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;
> } 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 *
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
2018-03-22 20:27 [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Ville Syrjala
` (2 preceding siblings ...)
2018-03-22 21:26 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-03-23 0:26 ` Patchwork
2018-03-23 17:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook (rev3) Patchwork
` (3 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-03-23 0:26 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
URL : https://patchwork.freedesktop.org/series/40514/
State : success
== Summary ==
---- Known issues:
Test kms_flip:
Subgroup 2x-wf_vblank-ts-check-interruptible:
pass -> FAIL (shard-hsw) fdo#100368
Subgroup dpms-vs-vblank-race:
fail -> PASS (shard-hsw) fdo#103060 +1
Subgroup flip-vs-expired-vblank:
pass -> FAIL (shard-hsw) fdo#102887
Test prime_vgem:
Subgroup basic-fence-flip:
pass -> FAIL (shard-apl) fdo#104008
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
shard-apl total:3484 pass:1819 dwarn:1 dfail:0 fail:8 skip:1655 time:13119s
shard-hsw total:3484 pass:1772 dwarn:1 dfail:0 fail:3 skip:1707 time:11856s
shard-snb total:3484 pass:1363 dwarn:1 dfail:0 fail:3 skip:2117 time:7288s
Blacklisted hosts:
shard-kbl total:3484 pass:1918 dwarn:27 dfail:0 fail:9 skip:1530 time:9995s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8459/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook
2018-03-22 23:43 ` Noralf Trønnes
@ 2018-03-23 11:31 ` Ville Syrjälä
2018-03-23 13:37 ` Noralf Trønnes
0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2018-03-23 11:31 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx, David Lechner, dri-devel
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;
> > } 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 *
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook
2018-03-23 11:31 ` Ville Syrjälä
@ 2018-03-23 13:37 ` Noralf Trønnes
2018-03-23 13:58 ` Ville Syrjälä
0 siblings, 1 reply; 20+ messages in thread
From: Noralf Trønnes @ 2018-03-23 13:37 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, David Lechner, dri-devel
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;
>>> } 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 *
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook
2018-03-23 13:37 ` Noralf Trønnes
@ 2018-03-23 13:58 ` Ville Syrjälä
2018-03-23 15:36 ` [Intel-gfx] " Ville Syrjälä
0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2018-03-23 13:58 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx, David Lechner, dri-devel
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;
> >>> } 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 *
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/2] drm/tinydrm: Make fb_dirty into a lower level hook
2018-03-22 20:27 ` [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook Ville Syrjala
2018-03-22 23:43 ` Noralf Trønnes
@ 2018-03-23 15:35 ` Ville Syrjala
2018-03-23 16:28 ` Noralf Trønnes
2018-03-24 17:22 ` David Lechner
1 sibling, 2 replies; 20+ messages in thread
From: Ville Syrjala @ 2018-03-23 15:35 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, David Lechner
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 *
--
2.16.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook
2018-03-23 13:58 ` Ville Syrjälä
@ 2018-03-23 15:36 ` Ville Syrjälä
0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2018-03-23 15:36 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx, David Lechner, dri-devel
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.
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] drm/tinydrm: Make fb_dirty into a lower level hook
2018-03-23 15:35 ` [PATCH v2 " Ville Syrjala
@ 2018-03-23 16:28 ` Noralf Trønnes
2018-03-28 16:23 ` Ville Syrjälä
2018-03-24 17:22 ` David Lechner
1 sibling, 1 reply; 20+ messages in thread
From: Noralf Trønnes @ 2018-03-23 16:28 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx, David Lechner
Den 23.03.2018 16.35, skrev Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> mipi_dbi_enable_flush() wants to call the fb->dirty() hook from the
> bowels of the .atomic_enable() hook. That prevents us from taking the
> plane mutex in fb->dirty() unless we also plumb down the acquire
> context.
>
> Instead it seems simpler to split the fb->dirty() into a tinydrm
> specific lower level hook that can be called from
> mipi_dbi_enable_flush() and from a generic higher level
> tinydrm_fb_dirty() helper. As we don't have a tinydrm specific
> vfuncs table we'll just stick it into tinydrm_device directly
> for now.
>
> v2: Deal with the fb->dirty() in tinydrm_display_pipe_update() as weel (Noralf)
>
> Cc: "Noralf Trønnes" <noralf@tronnes.org>
> Cc: David Lechner <david@lechnology.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
> ---
Tested mi0823qt which covers core and mipi-dbi:
Tested-by: Noralf Trønnes <noralf@tronnes.org>
> drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 30 ++++++++++++++++++++++++++
> drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 5 ++---
> drivers/gpu/drm/tinydrm/ili9225.c | 23 ++++++--------------
> drivers/gpu/drm/tinydrm/mi0283qt.c | 2 +-
> drivers/gpu/drm/tinydrm/mipi-dbi.c | 30 ++++++++++----------------
> drivers/gpu/drm/tinydrm/repaper.c | 28 ++++++++----------------
> drivers/gpu/drm/tinydrm/st7586.c | 23 ++++++--------------
> drivers/gpu/drm/tinydrm/st7735r.c | 2 +-
> include/drm/tinydrm/mipi-dbi.h | 4 +++-
> include/drm/tinydrm/tinydrm-helpers.h | 5 +++++
> include/drm/tinydrm/tinydrm.h | 4 ++++
> 11 files changed, 78 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> index d1c3ce9ab294..dcd390163a4a 100644
> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> @@ -78,6 +78,36 @@ bool tinydrm_merge_clips(struct drm_clip_rect *dst,
> }
> EXPORT_SYMBOL(tinydrm_merge_clips);
>
> +int tinydrm_fb_dirty(struct drm_framebuffer *fb,
> + struct drm_file *file_priv,
> + unsigned int flags, unsigned int color,
> + struct drm_clip_rect *clips,
> + unsigned int num_clips)
> +{
> + struct tinydrm_device *tdev = fb->dev->dev_private;
> + struct drm_plane *plane = &tdev->pipe.plane;
> + int ret = 0;
> +
> + drm_modeset_lock(&plane->mutex, NULL);
> +
> + /* fbdev can flush even when we're not interested */
> + if (plane->state->fb == fb) {
> + mutex_lock(&tdev->dirty_lock);
> + ret = tdev->fb_dirty(fb, file_priv, flags,
> + color, clips, num_clips);
> + mutex_unlock(&tdev->dirty_lock);
> + }
> +
> + drm_modeset_unlock(&plane->mutex);
> +
> + if (ret)
> + dev_err_once(fb->dev->dev,
> + "Failed to update display %d\n", ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(tinydrm_fb_dirty);
> +
> /**
> * tinydrm_memcpy - Copy clip buffer
> * @dst: Destination buffer
> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
> index 11ae950b0fc9..e68b528ae64d 100644
> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
> @@ -125,9 +125,8 @@ void tinydrm_display_pipe_update(struct drm_simple_display_pipe *pipe,
> struct drm_crtc *crtc = &tdev->pipe.crtc;
>
> if (fb && (fb != old_state->fb)) {
> - pipe->plane.fb = fb;
> - if (fb->funcs->dirty)
> - fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
> + if (tdev->fb_dirty)
> + tdev->fb_dirty(fb, NULL, 0, 0, NULL, 0);
> }
>
> if (crtc->state->event) {
> diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
> index 089d22798c8b..0874e877b111 100644
> --- a/drivers/gpu/drm/tinydrm/ili9225.c
> +++ b/drivers/gpu/drm/tinydrm/ili9225.c
> @@ -88,14 +88,8 @@ static int ili9225_fb_dirty(struct drm_framebuffer *fb,
> bool full;
> void *tr;
>
> - mutex_lock(&tdev->dirty_lock);
> -
> if (!mipi->enabled)
> - goto out_unlock;
> -
> - /* fbdev can flush even when we're not interested */
> - if (tdev->pipe.plane.fb != fb)
> - goto out_unlock;
> + return 0;
>
> full = tinydrm_merge_clips(&clip, clips, num_clips, flags,
> fb->width, fb->height);
> @@ -108,7 +102,7 @@ static int ili9225_fb_dirty(struct drm_framebuffer *fb,
> tr = mipi->tx_buf;
> ret = mipi_dbi_buf_copy(mipi->tx_buf, fb, &clip, swap);
> if (ret)
> - goto out_unlock;
> + return ret;
> } else {
> tr = cma_obj->vaddr;
> }
> @@ -159,20 +153,13 @@ static int ili9225_fb_dirty(struct drm_framebuffer *fb,
> ret = mipi_dbi_command_buf(mipi, ILI9225_WRITE_DATA_TO_GRAM, tr,
> (clip.x2 - clip.x1) * (clip.y2 - clip.y1) * 2);
>
> -out_unlock:
> - mutex_unlock(&tdev->dirty_lock);
> -
> - if (ret)
> - dev_err_once(fb->dev->dev, "Failed to update display %d\n",
> - ret);
> -
> return ret;
> }
>
> static const struct drm_framebuffer_funcs ili9225_fb_funcs = {
> .destroy = drm_gem_fb_destroy,
> .create_handle = drm_gem_fb_create_handle,
> - .dirty = ili9225_fb_dirty,
> + .dirty = tinydrm_fb_dirty,
> };
>
> static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
> @@ -269,7 +256,7 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
>
> ili9225_command(mipi, ILI9225_DISPLAY_CONTROL_1, 0x1017);
>
> - mipi_dbi_enable_flush(mipi);
> + mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
> }
>
> static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
> @@ -342,6 +329,8 @@ static int ili9225_init(struct device *dev, struct mipi_dbi *mipi,
> if (ret)
> return ret;
>
> + tdev->fb_dirty = ili9225_fb_dirty;
> +
> ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
> DRM_MODE_CONNECTOR_VIRTUAL,
> ili9225_formats,
> diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
> index 82ad9b61898e..4e6d2ee94e55 100644
> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c
> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
> @@ -127,7 +127,7 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
> msleep(100);
>
> out_enable:
> - mipi_dbi_enable_flush(mipi);
> + mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
> }
>
> static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
> diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> index 9e903812b573..4d1fb31a781f 100644
> --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
> +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
> @@ -219,14 +219,8 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
> bool full;
> void *tr;
>
> - mutex_lock(&tdev->dirty_lock);
> -
> if (!mipi->enabled)
> - goto out_unlock;
> -
> - /* fbdev can flush even when we're not interested */
> - if (tdev->pipe.plane.fb != fb)
> - goto out_unlock;
> + return 0;
>
> full = tinydrm_merge_clips(&clip, clips, num_clips, flags,
> fb->width, fb->height);
> @@ -239,7 +233,7 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
> tr = mipi->tx_buf;
> ret = mipi_dbi_buf_copy(mipi->tx_buf, fb, &clip, swap);
> if (ret)
> - goto out_unlock;
> + return ret;
> } else {
> tr = cma_obj->vaddr;
> }
> @@ -254,20 +248,13 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
> ret = mipi_dbi_command_buf(mipi, MIPI_DCS_WRITE_MEMORY_START, tr,
> (clip.x2 - clip.x1) * (clip.y2 - clip.y1) * 2);
>
> -out_unlock:
> - mutex_unlock(&tdev->dirty_lock);
> -
> - if (ret)
> - dev_err_once(fb->dev->dev, "Failed to update display %d\n",
> - ret);
> -
> return ret;
> }
>
> static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs = {
> .destroy = drm_gem_fb_destroy,
> .create_handle = drm_gem_fb_create_handle,
> - .dirty = mipi_dbi_fb_dirty,
> + .dirty = tinydrm_fb_dirty,
> };
>
> /**
> @@ -278,13 +265,16 @@ static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs = {
> * enables the backlight. Drivers can use this in their
> * &drm_simple_display_pipe_funcs->enable callback.
> */
> -void mipi_dbi_enable_flush(struct mipi_dbi *mipi)
> +void mipi_dbi_enable_flush(struct mipi_dbi *mipi,
> + struct drm_crtc_state *crtc_state,
> + struct drm_plane_state *plane_state)
> {
> - struct drm_framebuffer *fb = mipi->tinydrm.pipe.plane.fb;
> + struct tinydrm_device *tdev = &mipi->tinydrm;
> + struct drm_framebuffer *fb = plane_state->fb;
>
> mipi->enabled = true;
> if (fb)
> - fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
> + tdev->fb_dirty(fb, NULL, 0, 0, NULL, 0);
>
> backlight_enable(mipi->backlight);
> }
> @@ -381,6 +371,8 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
> if (ret)
> return ret;
>
> + tdev->fb_dirty = mipi_dbi_fb_dirty;
> +
> /* TODO: Maybe add DRM_MODE_CONNECTOR_SPI */
> ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
> DRM_MODE_CONNECTOR_VIRTUAL,
> diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
> index 33b4a71916e4..bb6f80a81899 100644
> --- a/drivers/gpu/drm/tinydrm/repaper.c
> +++ b/drivers/gpu/drm/tinydrm/repaper.c
> @@ -540,14 +540,8 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
> clip.y1 = 0;
> clip.y2 = fb->height;
>
> - mutex_lock(&tdev->dirty_lock);
> -
> if (!epd->enabled)
> - goto out_unlock;
> -
> - /* fbdev can flush even when we're not interested */
> - if (tdev->pipe.plane.fb != fb)
> - goto out_unlock;
> + return 0;
>
> repaper_get_temperature(epd);
>
> @@ -555,16 +549,14 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
> epd->factored_stage_time);
>
> buf = kmalloc(fb->width * fb->height, GFP_KERNEL);
> - if (!buf) {
> - ret = -ENOMEM;
> - goto out_unlock;
> - }
> + if (!buf)
> + return -ENOMEM;
>
> if (import_attach) {
> ret = dma_buf_begin_cpu_access(import_attach->dmabuf,
> DMA_FROM_DEVICE);
> if (ret)
> - goto out_unlock;
> + goto out_free;
> }
>
> tinydrm_xrgb8888_to_gray8(buf, cma_obj->vaddr, fb, &clip);
> @@ -573,7 +565,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
> ret = dma_buf_end_cpu_access(import_attach->dmabuf,
> DMA_FROM_DEVICE);
> if (ret)
> - goto out_unlock;
> + goto out_free;
> }
>
> repaper_gray8_to_mono_reversed(buf, fb->width, fb->height);
> @@ -625,11 +617,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
> }
> }
>
> -out_unlock:
> - mutex_unlock(&tdev->dirty_lock);
> -
> - if (ret)
> - DRM_DEV_ERROR(fb->dev->dev, "Failed to update display (%d)\n", ret);
> +out_free:
> kfree(buf);
>
> return ret;
> @@ -638,7 +626,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
> static const struct drm_framebuffer_funcs repaper_fb_funcs = {
> .destroy = drm_gem_fb_destroy,
> .create_handle = drm_gem_fb_create_handle,
> - .dirty = repaper_fb_dirty,
> + .dirty = tinydrm_fb_dirty,
> };
>
> static void power_off(struct repaper_epd *epd)
> @@ -1070,6 +1058,8 @@ static int repaper_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> + tdev->fb_dirty = repaper_fb_dirty;
> +
> ret = tinydrm_display_pipe_init(tdev, &repaper_pipe_funcs,
> DRM_MODE_CONNECTOR_VIRTUAL,
> repaper_formats,
> diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
> index bb08b293c8ce..22644b88199a 100644
> --- a/drivers/gpu/drm/tinydrm/st7586.c
> +++ b/drivers/gpu/drm/tinydrm/st7586.c
> @@ -120,14 +120,8 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb,
> int start, end;
> int ret = 0;
>
> - mutex_lock(&tdev->dirty_lock);
> -
> if (!mipi->enabled)
> - goto out_unlock;
> -
> - /* fbdev can flush even when we're not interested */
> - if (tdev->pipe.plane.fb != fb)
> - goto out_unlock;
> + return 0;
>
> tinydrm_merge_clips(&clip, clips, num_clips, flags, fb->width,
> fb->height);
> @@ -141,7 +135,7 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb,
>
> ret = st7586_buf_copy(mipi->tx_buf, fb, &clip);
> if (ret)
> - goto out_unlock;
> + return ret;
>
> /* Pixels are packed 3 per byte */
> start = clip.x1 / 3;
> @@ -158,20 +152,13 @@ static int st7586_fb_dirty(struct drm_framebuffer *fb,
> (u8 *)mipi->tx_buf,
> (end - start) * (clip.y2 - clip.y1));
>
> -out_unlock:
> - mutex_unlock(&tdev->dirty_lock);
> -
> - if (ret)
> - dev_err_once(fb->dev->dev, "Failed to update display %d\n",
> - ret);
> -
> return ret;
> }
>
> static const struct drm_framebuffer_funcs st7586_fb_funcs = {
> .destroy = drm_gem_fb_destroy,
> .create_handle = drm_gem_fb_create_handle,
> - .dirty = st7586_fb_dirty,
> + .dirty = tinydrm_fb_dirty,
> };
>
> static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
> @@ -238,7 +225,7 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
>
> mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
>
> - mipi_dbi_enable_flush(mipi);
> + mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
> }
>
> static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe)
> @@ -278,6 +265,8 @@ static int st7586_init(struct device *dev, struct mipi_dbi *mipi,
> if (ret)
> return ret;
>
> + tdev->fb_dirty = st7586_fb_dirty;
> +
> ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
> DRM_MODE_CONNECTOR_VIRTUAL,
> st7586_formats,
> diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
> index 19b28f8c78db..189a07894d36 100644
> --- a/drivers/gpu/drm/tinydrm/st7735r.c
> +++ b/drivers/gpu/drm/tinydrm/st7735r.c
> @@ -99,7 +99,7 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe,
>
> msleep(20);
>
> - mipi_dbi_enable_flush(mipi);
> + mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
> }
>
> static const struct drm_simple_display_pipe_funcs jd_t18003_t01_pipe_funcs = {
> diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
> index 44e824af2ef6..b8ba58861986 100644
> --- a/include/drm/tinydrm/mipi-dbi.h
> +++ b/include/drm/tinydrm/mipi-dbi.h
> @@ -67,7 +67,9 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
> const struct drm_simple_display_pipe_funcs *pipe_funcs,
> struct drm_driver *driver,
> const struct drm_display_mode *mode, unsigned int rotation);
> -void mipi_dbi_enable_flush(struct mipi_dbi *mipi);
> +void mipi_dbi_enable_flush(struct mipi_dbi *mipi,
> + struct drm_crtc_state *crtc_state,
> + struct drm_plane_state *plan_state);
> void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe);
> void mipi_dbi_hw_reset(struct mipi_dbi *mipi);
> bool mipi_dbi_display_is_on(struct mipi_dbi *mipi);
> diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
> index 0a4ddbc04c60..5b96f0b12c8c 100644
> --- a/include/drm/tinydrm/tinydrm-helpers.h
> +++ b/include/drm/tinydrm/tinydrm-helpers.h
> @@ -36,6 +36,11 @@ static inline bool tinydrm_machine_little_endian(void)
> bool tinydrm_merge_clips(struct drm_clip_rect *dst,
> struct drm_clip_rect *src, unsigned int num_clips,
> unsigned int flags, u32 max_width, u32 max_height);
> +int tinydrm_fb_dirty(struct drm_framebuffer *fb,
> + struct drm_file *file_priv,
> + unsigned int flags, unsigned int color,
> + struct drm_clip_rect *clips,
> + unsigned int num_clips);
> void tinydrm_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
> struct drm_clip_rect *clip);
> void tinydrm_swab16(u16 *dst, void *vaddr, struct drm_framebuffer *fb,
> diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
> index 07a9a11fe19d..ea2c71081190 100644
> --- a/include/drm/tinydrm/tinydrm.h
> +++ b/include/drm/tinydrm/tinydrm.h
> @@ -26,6 +26,10 @@ struct tinydrm_device {
> struct drm_simple_display_pipe pipe;
> struct mutex dirty_lock;
> const struct drm_framebuffer_funcs *fb_funcs;
> + int (*fb_dirty)(struct drm_framebuffer *framebuffer,
> + struct drm_file *file_priv, unsigned flags,
> + unsigned color, struct drm_clip_rect *clips,
> + unsigned num_clips);
> };
>
> static inline struct tinydrm_device *
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook (rev3)
2018-03-22 20:27 [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Ville Syrjala
` (3 preceding siblings ...)
2018-03-23 0:26 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-03-23 17:47 ` Patchwork
2018-03-23 18:01 ` ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-03-23 17:47 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook (rev3)
URL : https://patchwork.freedesktop.org/series/40514/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
e2190dc276fb drm/simple-kms-helper: Plumb plane state to the enable hook
061e9284de06 drm/tinydrm: Make fb_dirty into a lower level hook
-:21: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#21:
v2: Deal with the fb->dirty() in tinydrm_display_pipe_update() as weel (Noralf)
-:450: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#450: FILE: include/drm/tinydrm/tinydrm.h:30:
+ struct drm_file *file_priv, unsigned flags,
-:451: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#451: FILE: include/drm/tinydrm/tinydrm.h:31:
+ unsigned color, struct drm_clip_rect *clips,
-:452: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#452: FILE: include/drm/tinydrm/tinydrm.h:32:
+ unsigned num_clips);
total: 0 errors, 4 warnings, 0 checks, 355 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook (rev3)
2018-03-22 20:27 [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Ville Syrjala
` (4 preceding siblings ...)
2018-03-23 17:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook (rev3) Patchwork
@ 2018-03-23 18:01 ` Patchwork
2018-03-23 21:24 ` ✓ Fi.CI.IGT: " Patchwork
2018-03-24 17:26 ` [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook David Lechner
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-03-23 18:01 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook (rev3)
URL : https://patchwork.freedesktop.org/series/40514/
State : success
== Summary ==
Series 40514v3 series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
https://patchwork.freedesktop.org/api/1.0/series/40514/revisions/3/mbox/
---- Known issues:
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass -> INCOMPLETE (fi-bdw-5557u) fdo#104944
fdo#104944 https://bugs.freedesktop.org/show_bug.cgi?id=104944
fi-bdw-5557u total:242 pass:224 dwarn:0 dfail:0 fail:0 skip:17
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:442s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:378s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:540s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:299s
fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:515s
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:512s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:523s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:505s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:414s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:511s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:585s
fi-elk-e7500 total:285 pass:225 dwarn:1 dfail:0 fail:0 skip:59 time:426s
fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:314s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:535s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:404s
fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:417s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:472s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:429s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:478s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:466s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:519s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:654s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:444s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:540s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:504s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:505s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:427s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:444s
fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:576s
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:401s
Blacklisted hosts:
fi-cfl-s3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:568s
fi-cnl-psr total:224 pass:198 dwarn:0 dfail:0 fail:1 skip:24
fi-glk-j4005 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:486s
39c7a6abdfd77e8a15b6ee8f857e576d094b7afd drm-tip: 2018y-03m-23d-17h-05m-08s UTC integration manifest
061e9284de06 drm/tinydrm: Make fb_dirty into a lower level hook
e2190dc276fb drm/simple-kms-helper: Plumb plane state to the enable hook
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8478/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook (rev3)
2018-03-22 20:27 [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Ville Syrjala
` (5 preceding siblings ...)
2018-03-23 18:01 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-03-23 21:24 ` Patchwork
2018-03-24 17:26 ` [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook David Lechner
7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-03-23 21:24 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook (rev3)
URL : https://patchwork.freedesktop.org/series/40514/
State : success
== Summary ==
---- Possible new issues:
Test drv_suspend:
Subgroup fence-restore-tiled2untiled:
incomplete -> PASS (shard-snb)
Test kms_hdmi_inject:
Subgroup inject-audio:
fail -> PASS (shard-snb)
---- Known issues:
Test kms_flip:
Subgroup 2x-plain-flip-fb-recreate-interruptible:
pass -> FAIL (shard-hsw) fdo#100368 +2
Subgroup dpms-vs-vblank-race-interruptible:
fail -> PASS (shard-hsw) fdo#103060
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
shard-apl total:3484 pass:1820 dwarn:1 dfail:0 fail:7 skip:1655 time:13013s
shard-hsw total:3484 pass:1771 dwarn:1 dfail:0 fail:4 skip:1707 time:11668s
shard-snb total:3484 pass:1363 dwarn:1 dfail:0 fail:3 skip:2117 time:7032s
Blacklisted hosts:
shard-kbl total:3484 pass:1945 dwarn:1 dfail:0 fail:9 skip:1529 time:9905s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8478/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] drm/tinydrm: Make fb_dirty into a lower level hook
2018-03-23 15:35 ` [PATCH v2 " Ville Syrjala
2018-03-23 16:28 ` Noralf Trønnes
@ 2018-03-24 17:22 ` David Lechner
1 sibling, 0 replies; 20+ messages in thread
From: David Lechner @ 2018-03-24 17:22 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
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);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
2018-03-22 20:27 [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Ville Syrjala
` (6 preceding siblings ...)
2018-03-23 21:24 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-03-24 17:26 ` David Lechner
2018-03-27 10:07 ` Ville Syrjälä
7 siblings, 1 reply; 20+ messages in thread
From: David Lechner @ 2018-03-24 17:26 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: Marek Vasut, intel-gfx
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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
2018-03-24 17:26 ` [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook David Lechner
@ 2018-03-27 10:07 ` Ville Syrjälä
2018-03-27 16:40 ` David Lechner
0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2018-03-27 10:07 UTC (permalink / raw)
To: David Lechner
Cc: Marek Vasut, intel-gfx, dri-devel, Noralf Trønnes, Linus Walleij
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?
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook
2018-03-27 10:07 ` Ville Syrjälä
@ 2018-03-27 16:40 ` David Lechner
0 siblings, 0 replies; 20+ messages in thread
From: David Lechner @ 2018-03-27 16:40 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Marek Vasut, intel-gfx, dri-devel, Noralf Trønnes, Linus Walleij
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.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] drm/tinydrm: Make fb_dirty into a lower level hook
2018-03-23 16:28 ` Noralf Trønnes
@ 2018-03-28 16:23 ` Ville Syrjälä
0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2018-03-28 16:23 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: intel-gfx, David Lechner, dri-devel
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;
> > } 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 *
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-03-28 16:23 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-22 20:27 [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Ville Syrjala
2018-03-22 20:27 ` [PATCH 2/2] drm/tinydrm: Make fb_dirty into a lower level hook Ville Syrjala
2018-03-22 23:43 ` Noralf Trønnes
2018-03-23 11:31 ` Ville Syrjälä
2018-03-23 13:37 ` Noralf Trønnes
2018-03-23 13:58 ` Ville Syrjälä
2018-03-23 15:36 ` [Intel-gfx] " Ville Syrjälä
2018-03-23 15:35 ` [PATCH v2 " Ville Syrjala
2018-03-23 16:28 ` Noralf Trønnes
2018-03-28 16:23 ` Ville Syrjälä
2018-03-24 17:22 ` David Lechner
2018-03-22 21:11 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook Patchwork
2018-03-22 21:26 ` ✓ Fi.CI.BAT: success " Patchwork
2018-03-23 0:26 ` ✓ Fi.CI.IGT: " Patchwork
2018-03-23 17:47 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/simple-kms-helper: Plumb plane state to the enable hook (rev3) Patchwork
2018-03-23 18:01 ` ✓ Fi.CI.BAT: success " Patchwork
2018-03-23 21:24 ` ✓ Fi.CI.IGT: " Patchwork
2018-03-24 17:26 ` [PATCH 1/2] drm/simple-kms-helper: Plumb plane state to the enable hook David Lechner
2018-03-27 10:07 ` Ville Syrjälä
2018-03-27 16:40 ` David Lechner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.