Hi David,
This series fixes some issues in the Armada DRM driver: - A memory leak while cleaning up in the CRTC initialisation path - Avoid powering down YUV FIFOs when disabling the graphics plane - Several fixes for the overlay plane positioning
Minor updates for the comments from Sean Paul, and applies cleanly to your drm-fixes branch, but otherwise the same.
Thanks.
drivers/gpu/drm/armada/armada_crtc.c | 47 +++++++++++++++++++-------------- drivers/gpu/drm/armada/armada_crtc.h | 2 ++ drivers/gpu/drm/armada/armada_overlay.c | 38 +++++++++++++------------- 3 files changed, 48 insertions(+), 39 deletions(-)
Fix the leak of the CRTC structure in the failure paths of armada_drm_crtc_create().
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 2a4d163ac76f..79ce877bf45f 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -1225,17 +1225,13 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
ret = devm_request_irq(dev, irq, armada_drm_irq, 0, "armada_drm_crtc", dcrtc); - if (ret < 0) { - kfree(dcrtc); - return ret; - } + if (ret < 0) + goto err_crtc;
if (dcrtc->variant->init) { ret = dcrtc->variant->init(dcrtc, dev); - if (ret) { - kfree(dcrtc); - return ret; - } + if (ret) + goto err_crtc; }
/* Ensure AXI pipeline is enabled */ @@ -1246,13 +1242,15 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev, dcrtc->crtc.port = port;
primary = kzalloc(sizeof(*primary), GFP_KERNEL); - if (!primary) - return -ENOMEM; + if (!primary) { + ret = -ENOMEM; + goto err_crtc; + }
ret = armada_drm_plane_init(primary); if (ret) { kfree(primary); - return ret; + goto err_crtc; }
ret = drm_universal_plane_init(drm, &primary->base, 0, @@ -1263,7 +1261,7 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev, DRM_PLANE_TYPE_PRIMARY, NULL); if (ret) { kfree(primary); - return ret; + goto err_crtc; }
ret = drm_crtc_init_with_planes(drm, &dcrtc->crtc, &primary->base, NULL, @@ -1282,6 +1280,9 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev,
err_crtc_init: primary->base.funcs->destroy(&primary->base); +err_crtc: + kfree(dcrtc); + return ret; }
Avoid powering down the overlay SRAM banks when disabling the primary plane, thereby masking any overlay video. This feature is supposed to allow us to cut the bandwidth required while displaying full-frame overlay video.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 79ce877bf45f..77fc038cbfae 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -744,15 +744,14 @@ void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc, if (plane->fb) drm_framebuffer_put(plane->fb);
- /* Power down the Y/U/V FIFOs */ - sram_para1 = CFG_PDWN16x66 | CFG_PDWN32x66; - /* Power down most RAMs and FIFOs if this is the primary plane */ if (plane->type == DRM_PLANE_TYPE_PRIMARY) { - sram_para1 |= CFG_PDWN256x32 | CFG_PDWN256x24 | CFG_PDWN256x8 | - CFG_PDWN32x32 | CFG_PDWN64x66; + sram_para1 = CFG_PDWN256x32 | CFG_PDWN256x24 | CFG_PDWN256x8 | + CFG_PDWN32x32 | CFG_PDWN64x66; dma_ctrl0_mask = CFG_GRA_ENA; } else { + /* Power down the Y/U/V FIFOs */ + sram_para1 = CFG_PDWN16x66 | CFG_PDWN32x66; dma_ctrl0_mask = CFG_DMA_ENA; }
The UV swap code was not always programming things correctly when the source origin box has been offset. Fix this.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.h | 2 ++ drivers/gpu/drm/armada/armada_overlay.c | 38 ++++++++++++++++----------------- 2 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index bab11f483575..bfd3514fbe9b 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -42,6 +42,8 @@ struct armada_plane_work { };
struct armada_plane_state { + u16 src_x; + u16 src_y; u32 src_hw; u32 dst_hw; u32 dst_yx; diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index edc44910d79f..72f8094a75ae 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -99,6 +99,7 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, { struct armada_ovl_plane *dplane = drm_to_armada_ovl_plane(plane); struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); + const struct drm_format_info *format; struct drm_rect src = { .x1 = src_x, .y1 = src_y, @@ -117,7 +118,7 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, }; uint32_t val, ctrl0; unsigned idx = 0; - bool visible; + bool visible, fb_changed; int ret;
trace_armada_ovl_plane_update(plane, crtc, fb, @@ -138,6 +139,18 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (!visible) ctrl0 &= ~CFG_DMA_ENA;
+ /* + * Shifting a YUV packed format image by one pixel causes the U/V + * planes to swap. Compensate for it by also toggling the UV swap. + */ + format = fb->format; + if (format->num_planes == 1 && src.x1 >> 16 & (format->hsub - 1)) + ctrl0 ^= CFG_DMA_MOD(CFG_SWAPUV); + + fb_changed = plane->fb != fb || + dplane->base.state.src_x != src.x1 >> 16 || + dplane->base.state.src_y != src.y1 >> 16; + if (!dcrtc->plane) { dcrtc->plane = plane; armada_ovl_update_attr(&dplane->prop, dcrtc); @@ -145,7 +158,7 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
/* FIXME: overlay on an interlaced display */ /* Just updating the position/size? */ - if (plane->fb == fb && dplane->base.state.ctrl0 == ctrl0) { + if (!fb_changed && dplane->base.state.ctrl0 == ctrl0) { val = (drm_rect_height(&src) & 0xffff0000) | drm_rect_width(&src) >> 16; dplane->base.state.src_hw = val; @@ -169,9 +182,8 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (armada_drm_plane_work_wait(&dplane->base, HZ / 25) == 0) armada_drm_plane_work_cancel(dcrtc, &dplane->base);
- if (plane->fb != fb) { - u32 addrs[3], pixel_format; - int num_planes, hsub; + if (fb_changed) { + u32 addrs[3];
/* * Take a reference on the new framebuffer - we want to @@ -182,23 +194,11 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (plane->fb) armada_ovl_retire_fb(dplane, plane->fb);
- src_y = src.y1 >> 16; - src_x = src.x1 >> 16; + dplane->base.state.src_y = src_y = src.y1 >> 16; + dplane->base.state.src_x = src_x = src.x1 >> 16;
armada_drm_plane_calc_addrs(addrs, fb, src_x, src_y);
- pixel_format = fb->format->format; - hsub = drm_format_horz_chroma_subsampling(pixel_format); - num_planes = fb->format->num_planes; - - /* - * Annoyingly, shifting a YUYV-format image by one pixel - * causes the U/V planes to toggle. Toggle the UV swap. - * (Unfortunately, this causes momentary colour flickering.) - */ - if (src_x & (hsub - 1) && num_planes == 1) - ctrl0 ^= CFG_DMA_MOD(CFG_SWAPUV); - armada_reg_queue_set(dplane->vbl.regs, idx, addrs[0], LCD_SPU_DMA_START_ADDR_Y0); armada_reg_queue_set(dplane->vbl.regs, idx, addrs[1],
Lookup the drm_format_info structure once when computing all the framebuffer plane addresses by using drm_format_info(), rather than repetitive lookups via drm_format_plane_cpp().
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 77fc038cbfae..748de4691224 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -168,8 +168,9 @@ static void armada_drm_crtc_update(struct armada_crtc *dcrtc) void armada_drm_plane_calc_addrs(u32 *addrs, struct drm_framebuffer *fb, int x, int y) { + const struct drm_format_info *format = fb->format; + unsigned int num_planes = format->num_planes; u32 addr = drm_fb_obj(fb)->dev_addr; - int num_planes = fb->format->num_planes; int i;
if (num_planes > 3) @@ -177,7 +178,7 @@ void armada_drm_plane_calc_addrs(u32 *addrs, struct drm_framebuffer *fb,
for (i = 0; i < num_planes; i++) addrs[i] = addr + fb->offsets[i] + y * fb->pitches[i] + - x * fb->format->cpp[i]; + x * format->cpp[i]; for (; i < 3; i++) addrs[i] = 0; }
We weren't correctly calculating the YUV planar offsets for subsampled chroma planes correctly - fix up the coordinates for planes 1 and 2.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 748de4691224..17edd340f43e 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -176,7 +176,13 @@ void armada_drm_plane_calc_addrs(u32 *addrs, struct drm_framebuffer *fb, if (num_planes > 3) num_planes = 3;
- for (i = 0; i < num_planes; i++) + addrs[0] = addr + fb->offsets[0] + y * fb->pitches[0] + + x * format->cpp[0]; + + y /= format->vsub; + x /= format->hsub; + + for (i = 1; i < num_planes; i++) addrs[i] = addr + fb->offsets[i] + y * fb->pitches[i] + x * format->cpp[i]; for (; i < 3; i++)
Hi David,
This series builds upon the set of fixes previously submitted to move Armada DRM closer to atomic modeset. We're nowhere near yet, but this series helps to get us closer by unifying some of the differences between the primary and overlay planes.
New features added allows userspace to disable the primary plane if overlay is full screen and there's nothing obscuring the colorkey - this saves having to fetch an entire buffer containing nothing but colorkey when displaying full screen video.
Also we attach a property to the overlay plane to describe the colorimetry, which overrides the previous CRTC property doing the same thing. This is more in keeping with atomic modeset, and also means that the property becomes part of the metadata for the buffer it concerns.
drivers/gpu/drm/armada/armada_crtc.c | 420 ++++++++++++++++++++++++-------- drivers/gpu/drm/armada/armada_crtc.h | 26 +- drivers/gpu/drm/armada/armada_drm.h | 1 + drivers/gpu/drm/armada/armada_overlay.c | 305 +++++++++++------------ drivers/gpu/drm/armada/armada_trace.h | 24 +- 5 files changed, 507 insertions(+), 269 deletions(-)
armada_drm_plane_work_cancel()'s returned work structure is never used or referenced, so it's pointless returning it. It's also pointless because the caller doesn't have a clue what kind of work structure it is.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 8 +++----- drivers/gpu/drm/armada/armada_crtc.h | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index a0f4d2a2a481..7d2dfdfffb5e 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -254,15 +254,13 @@ int armada_drm_plane_work_wait(struct armada_plane *plane, long timeout) return wait_event_timeout(plane->frame_wait, !plane->work, timeout); }
-struct armada_plane_work *armada_drm_plane_work_cancel( - struct armada_crtc *dcrtc, struct armada_plane *plane) +void armada_drm_plane_work_cancel(struct armada_crtc *dcrtc, + struct armada_plane *dplane) { - struct armada_plane_work *work = xchg(&plane->work, NULL); + struct armada_plane_work *work = xchg(&dplane->work, NULL);
if (work) drm_crtc_vblank_put(&dcrtc->crtc); - - return work; }
static int armada_drm_crtc_queue_frame_work(struct armada_crtc *dcrtc, diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index bfd3514fbe9b..a054527eb962 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -62,8 +62,8 @@ int armada_drm_plane_init(struct armada_plane *plane); int armada_drm_plane_work_queue(struct armada_crtc *dcrtc, struct armada_plane *plane, struct armada_plane_work *work); int armada_drm_plane_work_wait(struct armada_plane *plane, long timeout); -struct armada_plane_work *armada_drm_plane_work_cancel( - struct armada_crtc *dcrtc, struct armada_plane *plane); +void armada_drm_plane_work_cancel(struct armada_crtc *dcrtc, + struct armada_plane *plane); void armada_drm_plane_calc_addrs(u32 *addrs, struct drm_framebuffer *fb, int x, int y);
Add and use a common frame work allocator, initialising the frame work to a sane state.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 7d2dfdfffb5e..8606f6e35986 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -293,6 +293,21 @@ static void armada_drm_crtc_complete_frame_work(struct armada_crtc *dcrtc, kfree(fwork); }
+static struct armada_frame_work *armada_drm_crtc_alloc_frame_work(void) +{ + struct armada_frame_work *work; + int i = 0; + + work = kzalloc(sizeof(*work), GFP_KERNEL); + if (!work) + return NULL; + + work->work.fn = armada_drm_crtc_complete_frame_work; + armada_reg_queue_end(work->regs, i); + + return work; +} + static void armada_drm_crtc_finish_fb(struct armada_crtc *dcrtc, struct drm_framebuffer *fb, bool force) { @@ -307,13 +322,9 @@ static void armada_drm_crtc_finish_fb(struct armada_crtc *dcrtc, return; }
- work = kmalloc(sizeof(*work), GFP_KERNEL); + work = armada_drm_crtc_alloc_frame_work(); if (work) { - int i = 0; - work->work.fn = armada_drm_crtc_complete_frame_work; - work->event = NULL; work->old_fb = fb; - armada_reg_queue_end(work->regs, i);
if (armada_drm_crtc_queue_frame_work(dcrtc, work) == 0) return; @@ -1033,11 +1044,10 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc, if (fb->format != crtc->primary->fb->format) return -EINVAL;
- work = kmalloc(sizeof(*work), GFP_KERNEL); + work = armada_drm_crtc_alloc_frame_work(); if (!work) return -ENOMEM;
- work->work.fn = armada_drm_crtc_complete_frame_work; work->event = event; work->old_fb = dcrtc->crtc.primary->fb;
Store the plane in the armada_plane_work structure rather than passing it around; it doesn't get used very much in the work structures, so passing it around is a needless expense.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 27 +++++++++++---------------- drivers/gpu/drm/armada/armada_crtc.h | 7 +++---- drivers/gpu/drm/armada/armada_overlay.c | 12 +++++++----- 3 files changed, 21 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 8606f6e35986..4d3db441466e 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -224,7 +224,7 @@ static void armada_drm_plane_work_run(struct armada_crtc *dcrtc,
/* Handle any pending frame work. */ if (work) { - work->fn(dcrtc, dplane, work); + work->fn(dcrtc, work); drm_crtc_vblank_put(&dcrtc->crtc); }
@@ -232,8 +232,9 @@ static void armada_drm_plane_work_run(struct armada_crtc *dcrtc, }
int armada_drm_plane_work_queue(struct armada_crtc *dcrtc, - struct armada_plane *plane, struct armada_plane_work *work) + struct armada_plane_work *work) { + struct armada_plane *plane = drm_to_armada_plane(work->plane); int ret;
ret = drm_crtc_vblank_get(&dcrtc->crtc); @@ -263,16 +264,8 @@ void armada_drm_plane_work_cancel(struct armada_crtc *dcrtc, drm_crtc_vblank_put(&dcrtc->crtc); }
-static int armada_drm_crtc_queue_frame_work(struct armada_crtc *dcrtc, - struct armada_frame_work *work) -{ - struct armada_plane *plane = drm_to_armada_plane(dcrtc->crtc.primary); - - return armada_drm_plane_work_queue(dcrtc, plane, &work->work); -} - static void armada_drm_crtc_complete_frame_work(struct armada_crtc *dcrtc, - struct armada_plane *plane, struct armada_plane_work *work) + struct armada_plane_work *work) { struct armada_frame_work *fwork = container_of(work, struct armada_frame_work, work); struct drm_device *dev = dcrtc->crtc.dev; @@ -293,7 +286,8 @@ static void armada_drm_crtc_complete_frame_work(struct armada_crtc *dcrtc, kfree(fwork); }
-static struct armada_frame_work *armada_drm_crtc_alloc_frame_work(void) +static struct armada_frame_work * +armada_drm_crtc_alloc_frame_work(struct drm_plane *plane) { struct armada_frame_work *work; int i = 0; @@ -302,6 +296,7 @@ static struct armada_frame_work *armada_drm_crtc_alloc_frame_work(void) if (!work) return NULL;
+ work->work.plane = plane; work->work.fn = armada_drm_crtc_complete_frame_work; armada_reg_queue_end(work->regs, i);
@@ -322,11 +317,11 @@ static void armada_drm_crtc_finish_fb(struct armada_crtc *dcrtc, return; }
- work = armada_drm_crtc_alloc_frame_work(); + work = armada_drm_crtc_alloc_frame_work(dcrtc->crtc.primary); if (work) { work->old_fb = fb;
- if (armada_drm_crtc_queue_frame_work(dcrtc, work) == 0) + if (armada_drm_plane_work_queue(dcrtc, work) == 0) return;
kfree(work); @@ -1044,7 +1039,7 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc, if (fb->format != crtc->primary->fb->format) return -EINVAL;
- work = armada_drm_crtc_alloc_frame_work(); + work = armada_drm_crtc_alloc_frame_work(dcrtc->crtc.primary); if (!work) return -ENOMEM;
@@ -1061,7 +1056,7 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc, */ drm_framebuffer_get(fb);
- ret = armada_drm_crtc_queue_frame_work(dcrtc, work); + ret = armada_drm_plane_work_queue(dcrtc, work); if (ret) { /* Undo our reference above */ drm_framebuffer_put(fb); diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index a054527eb962..821c0dd21e45 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -36,9 +36,8 @@ struct armada_plane; struct armada_variant;
struct armada_plane_work { - void (*fn)(struct armada_crtc *, - struct armada_plane *, - struct armada_plane_work *); + void (*fn)(struct armada_crtc *, struct armada_plane_work *); + struct drm_plane *plane; };
struct armada_plane_state { @@ -60,7 +59,7 @@ struct armada_plane {
int armada_drm_plane_init(struct armada_plane *plane); int armada_drm_plane_work_queue(struct armada_crtc *dcrtc, - struct armada_plane *plane, struct armada_plane_work *work); + struct armada_plane_work *work); int armada_drm_plane_work_wait(struct armada_plane *plane, long timeout); void armada_drm_plane_work_cancel(struct armada_crtc *dcrtc, struct armada_plane *plane); diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index aba947696178..1fa8ea8cb2de 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -80,11 +80,12 @@ static void armada_ovl_retire_fb(struct armada_ovl_plane *dplane,
/* === Plane support === */ static void armada_ovl_plane_work(struct armada_crtc *dcrtc, - struct armada_plane *plane, struct armada_plane_work *work) + struct armada_plane_work *work) { - struct armada_ovl_plane *dplane = container_of(plane, struct armada_ovl_plane, base); + struct armada_ovl_plane *dplane = container_of(work->plane, + struct armada_ovl_plane, base.base);
- trace_armada_ovl_plane_work(&dcrtc->crtc, &plane->base); + trace_armada_ovl_plane_work(&dcrtc->crtc, work->plane);
armada_drm_crtc_update_regs(dcrtc, dplane->vbl.regs); armada_ovl_retire_fb(dplane, NULL); @@ -252,8 +253,8 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, } if (idx) { armada_reg_queue_end(dplane->vbl.regs, idx); - armada_drm_plane_work_queue(dcrtc, &dplane->base, - &dplane->vbl.work); + /* Queue it for update on the next interrupt if we are enabled */ + armada_drm_plane_work_queue(dcrtc, &dplane->vbl.work); } return 0; } @@ -454,6 +455,7 @@ int armada_overlay_plane_create(struct drm_device *dev, unsigned long crtcs) return ret; }
+ dplane->vbl.work.plane = &dplane->base.base; dplane->vbl.work.fn = armada_ovl_plane_work;
ret = drm_universal_plane_init(dev, &dplane->base.base, crtcs,
Add a work cancel callback, so that work items can add functionality to clean themselves up when they are cancelled.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 23 ++++++++++++++++------- drivers/gpu/drm/armada/armada_crtc.h | 1 + 2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 4d3db441466e..f10ab0275ce7 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -216,6 +216,19 @@ static unsigned armada_drm_crtc_calc_fb(struct drm_framebuffer *fb, return i; }
+static void armada_drm_plane_work_call(struct armada_crtc *dcrtc, + struct armada_plane_work *work, + void (*fn)(struct armada_crtc *, struct armada_plane_work *)) +{ + struct armada_plane *dplane = drm_to_armada_plane(work->plane); + + if (fn) + fn(dcrtc, work); + drm_crtc_vblank_put(&dcrtc->crtc); + + wake_up(&dplane->frame_wait); +} + static void armada_drm_plane_work_run(struct armada_crtc *dcrtc, struct drm_plane *plane) { @@ -223,12 +236,8 @@ static void armada_drm_plane_work_run(struct armada_crtc *dcrtc, struct armada_plane_work *work = xchg(&dplane->work, NULL);
/* Handle any pending frame work. */ - if (work) { - work->fn(dcrtc, work); - drm_crtc_vblank_put(&dcrtc->crtc); - } - - wake_up(&dplane->frame_wait); + if (work) + armada_drm_plane_work_call(dcrtc, work, work->fn); }
int armada_drm_plane_work_queue(struct armada_crtc *dcrtc, @@ -261,7 +270,7 @@ void armada_drm_plane_work_cancel(struct armada_crtc *dcrtc, struct armada_plane_work *work = xchg(&dplane->work, NULL);
if (work) - drm_crtc_vblank_put(&dcrtc->crtc); + armada_drm_plane_work_call(dcrtc, work, work->cancel); }
static void armada_drm_crtc_complete_frame_work(struct armada_crtc *dcrtc, diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index 821c0dd21e45..c26814c2ce08 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -37,6 +37,7 @@ struct armada_variant;
struct armada_plane_work { void (*fn)(struct armada_crtc *, struct armada_plane_work *); + void (*cancel)(struct armada_crtc *, struct armada_plane_work *); struct drm_plane *plane; };
Wait for a second, and if we time out, cancel any pending work when disabling the primary plane. This ensures that any pending work is completed or cleaned up prior to the disable taking effect.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 28 ++++++++++++++++++++++------ drivers/gpu/drm/armada/armada_overlay.c | 2 -- 2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index f10ab0275ce7..328f030ffbdc 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -273,18 +273,15 @@ void armada_drm_plane_work_cancel(struct armada_crtc *dcrtc, armada_drm_plane_work_call(dcrtc, work, work->cancel); }
-static void armada_drm_crtc_complete_frame_work(struct armada_crtc *dcrtc, +static void armada_drm_crtc_finish_frame_work(struct armada_crtc *dcrtc, struct armada_plane_work *work) { struct armada_frame_work *fwork = container_of(work, struct armada_frame_work, work); - struct drm_device *dev = dcrtc->crtc.dev; unsigned long flags;
- spin_lock_irqsave(&dcrtc->irq_lock, flags); - armada_drm_crtc_update_regs(dcrtc, fwork->regs); - spin_unlock_irqrestore(&dcrtc->irq_lock, flags); - if (fwork->event) { + struct drm_device *dev = dcrtc->crtc.dev; + spin_lock_irqsave(&dev->event_lock, flags); drm_crtc_send_vblank_event(&dcrtc->crtc, fwork->event); spin_unlock_irqrestore(&dev->event_lock, flags); @@ -295,6 +292,19 @@ static void armada_drm_crtc_complete_frame_work(struct armada_crtc *dcrtc, kfree(fwork); }
+static void armada_drm_crtc_complete_frame_work(struct armada_crtc *dcrtc, + struct armada_plane_work *work) +{ + struct armada_frame_work *fwork = container_of(work, struct armada_frame_work, work); + unsigned long flags; + + spin_lock_irqsave(&dcrtc->irq_lock, flags); + armada_drm_crtc_update_regs(dcrtc, fwork->regs); + spin_unlock_irqrestore(&dcrtc->irq_lock, flags); + + armada_drm_crtc_finish_frame_work(dcrtc, work); +} + static struct armada_frame_work * armada_drm_crtc_alloc_frame_work(struct drm_plane *plane) { @@ -307,6 +317,7 @@ armada_drm_crtc_alloc_frame_work(struct drm_plane *plane)
work->work.plane = plane; work->work.fn = armada_drm_crtc_complete_frame_work; + work->work.cancel = armada_drm_crtc_finish_frame_work; armada_reg_queue_end(work->regs, i);
return work; @@ -752,6 +763,7 @@ static int armada_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc, struct drm_plane *plane) { + struct armada_plane *dplane = drm_to_armada_plane(plane); u32 sram_para1, dma_ctrl0_mask;
/* @@ -775,6 +787,10 @@ void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc, dma_ctrl0_mask = CFG_DMA_ENA; }
+ /* Wait for any preceding work to complete, but don't wedge */ + if (WARN_ON(!armada_drm_plane_work_wait(dplane, HZ))) + armada_drm_plane_work_cancel(dcrtc, dplane); + spin_lock_irq(&dcrtc->irq_lock); armada_updatel(0, dma_ctrl0_mask, dcrtc->base + LCD_SPU_DMA_CTRL0); spin_unlock_irq(&dcrtc->irq_lock); diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 1fa8ea8cb2de..cf8442583bfc 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -270,8 +270,6 @@ static int armada_ovl_plane_disable(struct drm_plane *plane, return 0;
dcrtc = drm_to_armada_crtc(dplane->base.base.crtc); - - armada_drm_plane_work_cancel(dcrtc, &dplane->base); armada_drm_crtc_plane_disable(dcrtc, plane);
dcrtc->plane = NULL;
Add our own hook to allow the primary plane to be disabled.
Signed-off-by: Russell King rmk+kernel@arm.linux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 99 ++++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 328f030ffbdc..7f7f21166488 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -341,7 +341,7 @@ static void armada_drm_crtc_finish_fb(struct armada_crtc *dcrtc, if (work) { work->old_fb = fb;
- if (armada_drm_plane_work_queue(dcrtc, work) == 0) + if (armada_drm_plane_work_queue(dcrtc, &work->work) == 0) return;
kfree(work); @@ -760,51 +760,13 @@ static int armada_drm_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, return 0; }
-void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc, - struct drm_plane *plane) -{ - struct armada_plane *dplane = drm_to_armada_plane(plane); - u32 sram_para1, dma_ctrl0_mask; - - /* - * Drop our reference on any framebuffer attached to this plane. - * We don't need to NULL this out as drm_plane_force_disable(), - * and __setplane_internal() will do so for an overlay plane, and - * __drm_helper_disable_unused_functions() will do so for the - * primary plane. - */ - if (plane->fb) - drm_framebuffer_put(plane->fb); - - /* Power down most RAMs and FIFOs if this is the primary plane */ - if (plane->type == DRM_PLANE_TYPE_PRIMARY) { - sram_para1 = CFG_PDWN256x32 | CFG_PDWN256x24 | CFG_PDWN256x8 | - CFG_PDWN32x32 | CFG_PDWN64x66; - dma_ctrl0_mask = CFG_GRA_ENA; - } else { - /* Power down the Y/U/V FIFOs */ - sram_para1 = CFG_PDWN16x66 | CFG_PDWN32x66; - dma_ctrl0_mask = CFG_DMA_ENA; - } - - /* Wait for any preceding work to complete, but don't wedge */ - if (WARN_ON(!armada_drm_plane_work_wait(dplane, HZ))) - armada_drm_plane_work_cancel(dcrtc, dplane); - - spin_lock_irq(&dcrtc->irq_lock); - armada_updatel(0, dma_ctrl0_mask, dcrtc->base + LCD_SPU_DMA_CTRL0); - spin_unlock_irq(&dcrtc->irq_lock); - - armada_updatel(sram_para1, 0, dcrtc->base + LCD_SPU_SRAM_PARA1); -} - /* The mode_config.mutex will be held for this call */ static void armada_drm_crtc_disable(struct drm_crtc *crtc) { - struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); - armada_drm_crtc_dpms(crtc, DRM_MODE_DPMS_OFF); - armada_drm_crtc_plane_disable(dcrtc, crtc->primary); + + /* Disable our primary plane when we disable the CRTC. */ + crtc->primary->funcs->disable_plane(crtc->primary, NULL); }
static const struct drm_crtc_helper_funcs armada_crtc_helper_funcs = { @@ -1081,7 +1043,7 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc, */ drm_framebuffer_get(fb);
- ret = armada_drm_plane_work_queue(dcrtc, work); + ret = armada_drm_plane_work_queue(dcrtc, &work->work); if (ret) { /* Undo our reference above */ drm_framebuffer_put(fb); @@ -1161,9 +1123,58 @@ static const struct drm_crtc_funcs armada_crtc_funcs = { .disable_vblank = armada_drm_crtc_disable_vblank, };
+void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc, + struct drm_plane *plane) +{ + struct armada_plane *dplane = drm_to_armada_plane(plane); + u32 sram_para1, dma_ctrl0_mask; + + /* + * Drop our reference on any framebuffer attached to this plane. + * We don't need to NULL this out as drm_plane_force_disable(), + * and __setplane_internal() will do so for an overlay plane, and + * __drm_helper_disable_unused_functions() will do so for the + * primary plane. + */ + if (plane->fb) + drm_framebuffer_put(plane->fb); + + /* Power down most RAMs and FIFOs if this is the primary plane */ + if (plane->type == DRM_PLANE_TYPE_PRIMARY) { + sram_para1 = CFG_PDWN256x32 | CFG_PDWN256x24 | CFG_PDWN256x8 | + CFG_PDWN32x32 | CFG_PDWN64x66; + dma_ctrl0_mask = CFG_GRA_ENA; + } else { + /* Power down the Y/U/V FIFOs */ + sram_para1 = CFG_PDWN16x66 | CFG_PDWN32x66; + dma_ctrl0_mask = CFG_DMA_ENA; + } + + /* Wait for any preceding work to complete, but don't wedge */ + if (WARN_ON(!armada_drm_plane_work_wait(dplane, HZ))) + armada_drm_plane_work_cancel(dcrtc, dplane); + + spin_lock_irq(&dcrtc->irq_lock); + armada_updatel(0, dma_ctrl0_mask, dcrtc->base + LCD_SPU_DMA_CTRL0); + spin_unlock_irq(&dcrtc->irq_lock); + + armada_updatel(sram_para1, 0, dcrtc->base + LCD_SPU_SRAM_PARA1); +} + +static int armada_drm_primary_disable(struct drm_plane *plane, + struct drm_modeset_acquire_ctx *ctx) +{ + if (plane->crtc) { + struct armada_crtc *dcrtc = drm_to_armada_crtc(plane->crtc); + + armada_drm_crtc_plane_disable(dcrtc, plane); + } + return 0; +} + static const struct drm_plane_funcs armada_primary_plane_funcs = { .update_plane = drm_primary_helper_update, - .disable_plane = drm_primary_helper_disable, + .disable_plane = armada_drm_primary_disable, .destroy = drm_primary_helper_destroy, };
Merge armada_drm_primary_disable() into armada_drm_crtc_plane_disable() and rename to armada_drm_plane_disable(). Use this to simplify armada_ovl_plane_disable().
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 21 +++++++++------------ drivers/gpu/drm/armada/armada_crtc.h | 4 ++-- drivers/gpu/drm/armada/armada_overlay.c | 9 +++------ 3 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 7f7f21166488..3287b72e48cc 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -1123,12 +1123,16 @@ static const struct drm_crtc_funcs armada_crtc_funcs = { .disable_vblank = armada_drm_crtc_disable_vblank, };
-void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc, - struct drm_plane *plane) +int armada_drm_plane_disable(struct drm_plane *plane, + struct drm_modeset_acquire_ctx *ctx) { struct armada_plane *dplane = drm_to_armada_plane(plane); + struct armada_crtc *dcrtc; u32 sram_para1, dma_ctrl0_mask;
+ if (!plane->crtc) + return 0; + /* * Drop our reference on any framebuffer attached to this plane. * We don't need to NULL this out as drm_plane_force_disable(), @@ -1150,6 +1154,8 @@ void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc, dma_ctrl0_mask = CFG_DMA_ENA; }
+ dcrtc = drm_to_armada_crtc(plane->crtc); + /* Wait for any preceding work to complete, but don't wedge */ if (WARN_ON(!armada_drm_plane_work_wait(dplane, HZ))) armada_drm_plane_work_cancel(dcrtc, dplane); @@ -1159,22 +1165,13 @@ void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc, spin_unlock_irq(&dcrtc->irq_lock);
armada_updatel(sram_para1, 0, dcrtc->base + LCD_SPU_SRAM_PARA1); -} - -static int armada_drm_primary_disable(struct drm_plane *plane, - struct drm_modeset_acquire_ctx *ctx) -{ - if (plane->crtc) { - struct armada_crtc *dcrtc = drm_to_armada_crtc(plane->crtc);
- armada_drm_crtc_plane_disable(dcrtc, plane); - } return 0; }
static const struct drm_plane_funcs armada_primary_plane_funcs = { .update_plane = drm_primary_helper_update, - .disable_plane = armada_drm_primary_disable, + .disable_plane = armada_drm_plane_disable, .destroy = drm_primary_helper_destroy, };
diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index c26814c2ce08..521ae5b6ad86 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -106,8 +106,8 @@ struct armada_crtc {
void armada_drm_crtc_update_regs(struct armada_crtc *, struct armada_regs *);
-void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc, - struct drm_plane *plane); +int armada_drm_plane_disable(struct drm_plane *plane, + struct drm_modeset_acquire_ctx *ctx);
extern struct platform_driver armada_lcd_platform_driver;
diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index cf8442583bfc..a53e7dd26b0b 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -264,15 +264,12 @@ static int armada_ovl_plane_disable(struct drm_plane *plane, { struct armada_ovl_plane *dplane = drm_to_armada_ovl_plane(plane); struct drm_framebuffer *fb; - struct armada_crtc *dcrtc;
- if (!dplane->base.base.crtc) - return 0; + armada_drm_plane_disable(plane, ctx);
- dcrtc = drm_to_armada_crtc(dplane->base.base.crtc); - armada_drm_crtc_plane_disable(dcrtc, plane); + if (dplane->base.base.crtc) + drm_to_armada_crtc(dplane->base.base.crtc)->plane = NULL;
- dcrtc->plane = NULL; dplane->base.state.ctrl0 = 0;
fb = xchg(&dplane->old_fb, NULL);
Clear the plane enable bit in the software state within armada_drm_plane_disable() when disabling either the primary or overlay planes.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 10 ++++++---- drivers/gpu/drm/armada/armada_overlay.c | 2 -- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 3287b72e48cc..bedcaed81ffa 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -1128,7 +1128,7 @@ int armada_drm_plane_disable(struct drm_plane *plane, { struct armada_plane *dplane = drm_to_armada_plane(plane); struct armada_crtc *dcrtc; - u32 sram_para1, dma_ctrl0_mask; + u32 sram_para1, enable_mask;
if (!plane->crtc) return 0; @@ -1147,13 +1147,15 @@ int armada_drm_plane_disable(struct drm_plane *plane, if (plane->type == DRM_PLANE_TYPE_PRIMARY) { sram_para1 = CFG_PDWN256x32 | CFG_PDWN256x24 | CFG_PDWN256x8 | CFG_PDWN32x32 | CFG_PDWN64x66; - dma_ctrl0_mask = CFG_GRA_ENA; + enable_mask = CFG_GRA_ENA; } else { /* Power down the Y/U/V FIFOs */ sram_para1 = CFG_PDWN16x66 | CFG_PDWN32x66; - dma_ctrl0_mask = CFG_DMA_ENA; + enable_mask = CFG_DMA_ENA; }
+ dplane->state.ctrl0 &= ~enable_mask; + dcrtc = drm_to_armada_crtc(plane->crtc);
/* Wait for any preceding work to complete, but don't wedge */ @@ -1161,7 +1163,7 @@ int armada_drm_plane_disable(struct drm_plane *plane, armada_drm_plane_work_cancel(dcrtc, dplane);
spin_lock_irq(&dcrtc->irq_lock); - armada_updatel(0, dma_ctrl0_mask, dcrtc->base + LCD_SPU_DMA_CTRL0); + armada_updatel(0, enable_mask, dcrtc->base + LCD_SPU_DMA_CTRL0); spin_unlock_irq(&dcrtc->irq_lock);
armada_updatel(sram_para1, 0, dcrtc->base + LCD_SPU_SRAM_PARA1); diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index a53e7dd26b0b..995463cd542d 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -270,8 +270,6 @@ static int armada_ovl_plane_disable(struct drm_plane *plane, if (dplane->base.base.crtc) drm_to_armada_crtc(dplane->base.base.crtc)->plane = NULL;
- dplane->base.state.ctrl0 = 0; - fb = xchg(&dplane->old_fb, NULL); if (fb) drm_framebuffer_put(fb);
Move the overlay plane work out from under the spinlock so that both the primary and overlay planes run their work in the same context. This is necessary so that we can use frame works with the overlay plane.
However, we must update the CRTC registers under the spinlock, so fix up the overlay code for that.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 2 +- drivers/gpu/drm/armada/armada_overlay.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index bedcaed81ffa..be3fd82ef516 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -471,11 +471,11 @@ static void armada_drm_crtc_irq(struct armada_crtc *dcrtc, u32 stat) if (stat & VSYNC_IRQ) drm_crtc_handle_vblank(&dcrtc->crtc);
- spin_lock(&dcrtc->irq_lock); ovl_plane = dcrtc->plane; if (ovl_plane) armada_drm_plane_work_run(dcrtc, ovl_plane);
+ spin_lock(&dcrtc->irq_lock); if (stat & GRA_FRAME_IRQ && dcrtc->interlaced) { int i = stat & GRA_FRAME_IRQ0 ? 0 : 1; uint32_t val; diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 995463cd542d..04746ade74e6 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -84,10 +84,14 @@ static void armada_ovl_plane_work(struct armada_crtc *dcrtc, { struct armada_ovl_plane *dplane = container_of(work->plane, struct armada_ovl_plane, base.base); + unsigned long flags;
trace_armada_ovl_plane_work(&dcrtc->crtc, work->plane);
+ spin_lock_irqsave(&dcrtc->irq_lock, flags); armada_drm_crtc_update_regs(dcrtc, dplane->vbl.regs); + spin_unlock_irqrestore(&dcrtc->irq_lock, flags); + armada_ovl_retire_fb(dplane, NULL); }
Both the primary and overlay planes retire framebuffers in a similar manner; this can be consolidated by moving the retirement up to the armada_plane_work layer.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 12 +++++++----- drivers/gpu/drm/armada/armada_crtc.h | 1 + drivers/gpu/drm/armada/armada_overlay.c | 30 +++++------------------------- 3 files changed, 13 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index be3fd82ef516..d1f4171966cc 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -24,7 +24,6 @@ struct armada_frame_work { struct armada_plane_work work; struct drm_pending_vblank_event *event; struct armada_regs regs[4]; - struct drm_framebuffer *old_fb; };
enum csc_mode { @@ -221,11 +220,16 @@ static void armada_drm_plane_work_call(struct armada_crtc *dcrtc, void (*fn)(struct armada_crtc *, struct armada_plane_work *)) { struct armada_plane *dplane = drm_to_armada_plane(work->plane); + struct drm_framebuffer *fb = work->old_fb;
if (fn) fn(dcrtc, work); drm_crtc_vblank_put(&dcrtc->crtc);
+ /* Finally, queue the process-half of the cleanup. */ + if (fb) + armada_drm_queue_unref_work(dcrtc->crtc.dev, fb); + wake_up(&dplane->frame_wait); }
@@ -287,8 +291,6 @@ static void armada_drm_crtc_finish_frame_work(struct armada_crtc *dcrtc, spin_unlock_irqrestore(&dev->event_lock, flags); }
- /* Finally, queue the process-half of the cleanup. */ - __armada_drm_queue_unref_work(dcrtc->crtc.dev, fwork->old_fb); kfree(fwork); }
@@ -339,7 +341,7 @@ static void armada_drm_crtc_finish_fb(struct armada_crtc *dcrtc,
work = armada_drm_crtc_alloc_frame_work(dcrtc->crtc.primary); if (work) { - work->old_fb = fb; + work->work.old_fb = fb;
if (armada_drm_plane_work_queue(dcrtc, &work->work) == 0) return; @@ -1031,7 +1033,7 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc, return -ENOMEM;
work->event = event; - work->old_fb = dcrtc->crtc.primary->fb; + work->work.old_fb = dcrtc->crtc.primary->fb;
i = armada_drm_crtc_calc_fb(fb, crtc->x, crtc->y, work->regs, dcrtc->interlaced); diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index 521ae5b6ad86..b40db72c61d8 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -39,6 +39,7 @@ struct armada_plane_work { void (*fn)(struct armada_crtc *, struct armada_plane_work *); void (*cancel)(struct armada_crtc *, struct armada_plane_work *); struct drm_plane *plane; + struct drm_framebuffer *old_fb; };
struct armada_plane_state { diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 04746ade74e6..01087c952916 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -32,7 +32,6 @@ struct armada_ovl_plane_properties {
struct armada_ovl_plane { struct armada_plane base; - struct drm_framebuffer *old_fb; struct { struct armada_plane_work work; struct armada_regs regs[13]; @@ -67,17 +66,6 @@ armada_ovl_update_attr(struct armada_ovl_plane_properties *prop, spin_unlock_irq(&dcrtc->irq_lock); }
-static void armada_ovl_retire_fb(struct armada_ovl_plane *dplane, - struct drm_framebuffer *fb) -{ - struct drm_framebuffer *old_fb; - - old_fb = xchg(&dplane->old_fb, fb); - - if (old_fb) - armada_drm_queue_unref_work(dplane->base.base.dev, old_fb); -} - /* === Plane support === */ static void armada_ovl_plane_work(struct armada_crtc *dcrtc, struct armada_plane_work *work) @@ -91,8 +79,6 @@ static void armada_ovl_plane_work(struct armada_crtc *dcrtc, spin_lock_irqsave(&dcrtc->irq_lock, flags); armada_drm_crtc_update_regs(dcrtc, dplane->vbl.regs); spin_unlock_irqrestore(&dcrtc->irq_lock, flags); - - armada_ovl_retire_fb(dplane, NULL); }
static int @@ -196,8 +182,7 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, */ drm_framebuffer_get(fb);
- if (plane->fb) - armada_ovl_retire_fb(dplane, plane->fb); + dplane->vbl.work.old_fb = plane->fb;
dplane->base.state.src_y = src_y = src.y1 >> 16; dplane->base.state.src_x = src_x = src.x1 >> 16; @@ -223,6 +208,8 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, val = fb->pitches[1] << 16 | fb->pitches[2]; armada_reg_queue_set(dplane->vbl.regs, idx, val, LCD_SPU_DMA_PITCH_UV); + } else { + dplane->vbl.work.old_fb = NULL; }
val = (drm_rect_height(&src) & 0xffff0000) | drm_rect_width(&src) >> 16; @@ -266,17 +253,10 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, static int armada_ovl_plane_disable(struct drm_plane *plane, struct drm_modeset_acquire_ctx *ctx) { - struct armada_ovl_plane *dplane = drm_to_armada_ovl_plane(plane); - struct drm_framebuffer *fb; - armada_drm_plane_disable(plane, ctx);
- if (dplane->base.base.crtc) - drm_to_armada_crtc(dplane->base.base.crtc)->plane = NULL; - - fb = xchg(&dplane->old_fb, NULL); - if (fb) - drm_framebuffer_put(fb); + if (plane->crtc) + drm_to_armada_crtc(plane->crtc)->plane = NULL;
return 0; }
Move the sending of events into the armada_plane_work structure, and combine the processing in armada_drm_plane_work_call().
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 27 +++++++++++++-------------- drivers/gpu/drm/armada/armada_crtc.h | 1 + 2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index d1f4171966cc..b043766c416c 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -22,7 +22,6 @@
struct armada_frame_work { struct armada_plane_work work; - struct drm_pending_vblank_event *event; struct armada_regs regs[4]; };
@@ -220,15 +219,24 @@ static void armada_drm_plane_work_call(struct armada_crtc *dcrtc, void (*fn)(struct armada_crtc *, struct armada_plane_work *)) { struct armada_plane *dplane = drm_to_armada_plane(work->plane); + struct drm_pending_vblank_event *event = work->event; struct drm_framebuffer *fb = work->old_fb;
if (fn) fn(dcrtc, work); drm_crtc_vblank_put(&dcrtc->crtc);
- /* Finally, queue the process-half of the cleanup. */ - if (fb) - armada_drm_queue_unref_work(dcrtc->crtc.dev, fb); + if (event || fb) { + struct drm_device *dev = dcrtc->crtc.dev; + unsigned long flags; + + spin_lock_irqsave(&dev->event_lock, flags); + if (event) + drm_crtc_send_vblank_event(&dcrtc->crtc, event); + if (fb) + __armada_drm_queue_unref_work(dev, fb); + spin_unlock_irqrestore(&dev->event_lock, flags); + }
wake_up(&dplane->frame_wait); } @@ -281,15 +289,6 @@ static void armada_drm_crtc_finish_frame_work(struct armada_crtc *dcrtc, struct armada_plane_work *work) { struct armada_frame_work *fwork = container_of(work, struct armada_frame_work, work); - unsigned long flags; - - if (fwork->event) { - struct drm_device *dev = dcrtc->crtc.dev; - - spin_lock_irqsave(&dev->event_lock, flags); - drm_crtc_send_vblank_event(&dcrtc->crtc, fwork->event); - spin_unlock_irqrestore(&dev->event_lock, flags); - }
kfree(fwork); } @@ -1032,7 +1031,7 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc, if (!work) return -ENOMEM;
- work->event = event; + work->work.event = event; work->work.old_fb = dcrtc->crtc.primary->fb;
i = armada_drm_crtc_calc_fb(fb, crtc->x, crtc->y, work->regs, diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index b40db72c61d8..4cdd2f0eabd9 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -40,6 +40,7 @@ struct armada_plane_work { void (*cancel)(struct armada_crtc *, struct armada_plane_work *); struct drm_plane *plane; struct drm_framebuffer *old_fb; + struct drm_pending_vblank_event *event; };
struct armada_plane_state {
Move the register update structure out of the overlay private structure into armada_plane_work, as this is common to both the primary and overlay planes.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 42 ++++++++++++------------------ drivers/gpu/drm/armada/armada_crtc.h | 1 + drivers/gpu/drm/armada/armada_overlay.c | 46 +++++++++++++++------------------ 3 files changed, 39 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index b043766c416c..02eefdc6f062 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -20,11 +20,6 @@ #include "armada_hw.h" #include "armada_trace.h"
-struct armada_frame_work { - struct armada_plane_work work; - struct armada_regs regs[4]; -}; - enum csc_mode { CSC_AUTO = 0, CSC_YUV_CCIR601 = 1, @@ -288,37 +283,34 @@ void armada_drm_plane_work_cancel(struct armada_crtc *dcrtc, static void armada_drm_crtc_finish_frame_work(struct armada_crtc *dcrtc, struct armada_plane_work *work) { - struct armada_frame_work *fwork = container_of(work, struct armada_frame_work, work); - - kfree(fwork); + kfree(work); }
static void armada_drm_crtc_complete_frame_work(struct armada_crtc *dcrtc, struct armada_plane_work *work) { - struct armada_frame_work *fwork = container_of(work, struct armada_frame_work, work); unsigned long flags;
spin_lock_irqsave(&dcrtc->irq_lock, flags); - armada_drm_crtc_update_regs(dcrtc, fwork->regs); + armada_drm_crtc_update_regs(dcrtc, work->regs); spin_unlock_irqrestore(&dcrtc->irq_lock, flags);
armada_drm_crtc_finish_frame_work(dcrtc, work); }
-static struct armada_frame_work * -armada_drm_crtc_alloc_frame_work(struct drm_plane *plane) +static struct armada_plane_work * +armada_drm_crtc_alloc_plane_work(struct drm_plane *plane) { - struct armada_frame_work *work; + struct armada_plane_work *work; int i = 0;
work = kzalloc(sizeof(*work), GFP_KERNEL); if (!work) return NULL;
- work->work.plane = plane; - work->work.fn = armada_drm_crtc_complete_frame_work; - work->work.cancel = armada_drm_crtc_finish_frame_work; + work->plane = plane; + work->fn = armada_drm_crtc_complete_frame_work; + work->cancel = armada_drm_crtc_finish_frame_work; armada_reg_queue_end(work->regs, i);
return work; @@ -327,7 +319,7 @@ armada_drm_crtc_alloc_frame_work(struct drm_plane *plane) static void armada_drm_crtc_finish_fb(struct armada_crtc *dcrtc, struct drm_framebuffer *fb, bool force) { - struct armada_frame_work *work; + struct armada_plane_work *work;
if (!fb) return; @@ -338,11 +330,11 @@ static void armada_drm_crtc_finish_fb(struct armada_crtc *dcrtc, return; }
- work = armada_drm_crtc_alloc_frame_work(dcrtc->crtc.primary); + work = armada_drm_crtc_alloc_plane_work(dcrtc->crtc.primary); if (work) { - work->work.old_fb = fb; + work->old_fb = fb;
- if (armada_drm_plane_work_queue(dcrtc, &work->work) == 0) + if (armada_drm_plane_work_queue(dcrtc, work) == 0) return;
kfree(work); @@ -1019,7 +1011,7 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc, struct drm_modeset_acquire_ctx *ctx) { struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); - struct armada_frame_work *work; + struct armada_plane_work *work; unsigned i; int ret;
@@ -1027,12 +1019,12 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc, if (fb->format != crtc->primary->fb->format) return -EINVAL;
- work = armada_drm_crtc_alloc_frame_work(dcrtc->crtc.primary); + work = armada_drm_crtc_alloc_plane_work(dcrtc->crtc.primary); if (!work) return -ENOMEM;
- work->work.event = event; - work->work.old_fb = dcrtc->crtc.primary->fb; + work->event = event; + work->old_fb = dcrtc->crtc.primary->fb;
i = armada_drm_crtc_calc_fb(fb, crtc->x, crtc->y, work->regs, dcrtc->interlaced); @@ -1044,7 +1036,7 @@ static int armada_drm_crtc_page_flip(struct drm_crtc *crtc, */ drm_framebuffer_get(fb);
- ret = armada_drm_plane_work_queue(dcrtc, &work->work); + ret = armada_drm_plane_work_queue(dcrtc, work); if (ret) { /* Undo our reference above */ drm_framebuffer_put(fb); diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index 4cdd2f0eabd9..12ef9688a45a 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -41,6 +41,7 @@ struct armada_plane_work { struct drm_plane *plane; struct drm_framebuffer *old_fb; struct drm_pending_vblank_event *event; + struct armada_regs regs[14]; };
struct armada_plane_state { diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 01087c952916..200223861bfb 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -32,10 +32,7 @@ struct armada_ovl_plane_properties {
struct armada_ovl_plane { struct armada_plane base; - struct { - struct armada_plane_work work; - struct armada_regs regs[13]; - } vbl; + struct armada_plane_work work; struct armada_ovl_plane_properties prop; }; #define drm_to_armada_ovl_plane(p) \ @@ -70,14 +67,12 @@ armada_ovl_update_attr(struct armada_ovl_plane_properties *prop, static void armada_ovl_plane_work(struct armada_crtc *dcrtc, struct armada_plane_work *work) { - struct armada_ovl_plane *dplane = container_of(work->plane, - struct armada_ovl_plane, base.base); unsigned long flags;
trace_armada_ovl_plane_work(&dcrtc->crtc, work->plane);
spin_lock_irqsave(&dcrtc->irq_lock, flags); - armada_drm_crtc_update_regs(dcrtc, dplane->vbl.regs); + armada_drm_crtc_update_regs(dcrtc, work->regs); spin_unlock_irqrestore(&dcrtc->irq_lock, flags); }
@@ -90,6 +85,7 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, { struct armada_ovl_plane *dplane = drm_to_armada_ovl_plane(plane); struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); + struct armada_plane_work *work = &dplane->work; const struct drm_format_info *format; struct drm_rect src = { .x1 = src_x, @@ -182,60 +178,60 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, */ drm_framebuffer_get(fb);
- dplane->vbl.work.old_fb = plane->fb; + work->old_fb = plane->fb;
dplane->base.state.src_y = src_y = src.y1 >> 16; dplane->base.state.src_x = src_x = src.x1 >> 16;
armada_drm_plane_calc_addrs(addrs, fb, src_x, src_y);
- armada_reg_queue_set(dplane->vbl.regs, idx, addrs[0], + armada_reg_queue_set(work->regs, idx, addrs[0], LCD_SPU_DMA_START_ADDR_Y0); - armada_reg_queue_set(dplane->vbl.regs, idx, addrs[1], + armada_reg_queue_set(work->regs, idx, addrs[1], LCD_SPU_DMA_START_ADDR_U0); - armada_reg_queue_set(dplane->vbl.regs, idx, addrs[2], + armada_reg_queue_set(work->regs, idx, addrs[2], LCD_SPU_DMA_START_ADDR_V0); - armada_reg_queue_set(dplane->vbl.regs, idx, addrs[0], + armada_reg_queue_set(work->regs, idx, addrs[0], LCD_SPU_DMA_START_ADDR_Y1); - armada_reg_queue_set(dplane->vbl.regs, idx, addrs[1], + armada_reg_queue_set(work->regs, idx, addrs[1], LCD_SPU_DMA_START_ADDR_U1); - armada_reg_queue_set(dplane->vbl.regs, idx, addrs[2], + armada_reg_queue_set(work->regs, idx, addrs[2], LCD_SPU_DMA_START_ADDR_V1);
val = fb->pitches[0] << 16 | fb->pitches[0]; - armada_reg_queue_set(dplane->vbl.regs, idx, val, + armada_reg_queue_set(work->regs, idx, val, LCD_SPU_DMA_PITCH_YC); val = fb->pitches[1] << 16 | fb->pitches[2]; - armada_reg_queue_set(dplane->vbl.regs, idx, val, + armada_reg_queue_set(work->regs, idx, val, LCD_SPU_DMA_PITCH_UV); } else { - dplane->vbl.work.old_fb = NULL; + work->old_fb = NULL; }
val = (drm_rect_height(&src) & 0xffff0000) | drm_rect_width(&src) >> 16; if (dplane->base.state.src_hw != val) { dplane->base.state.src_hw = val; - armada_reg_queue_set(dplane->vbl.regs, idx, val, + armada_reg_queue_set(work->regs, idx, val, LCD_SPU_DMA_HPXL_VLN); }
val = drm_rect_height(&dest) << 16 | drm_rect_width(&dest); if (dplane->base.state.dst_hw != val) { dplane->base.state.dst_hw = val; - armada_reg_queue_set(dplane->vbl.regs, idx, val, + armada_reg_queue_set(work->regs, idx, val, LCD_SPU_DZM_HPXL_VLN); }
val = dest.y1 << 16 | dest.x1; if (dplane->base.state.dst_yx != val) { dplane->base.state.dst_yx = val; - armada_reg_queue_set(dplane->vbl.regs, idx, val, + armada_reg_queue_set(work->regs, idx, val, LCD_SPU_DMA_OVSA_HPXL_VLN); }
if (dplane->base.state.ctrl0 != ctrl0) { dplane->base.state.ctrl0 = ctrl0; - armada_reg_queue_mod(dplane->vbl.regs, idx, ctrl0, + armada_reg_queue_mod(work->regs, idx, ctrl0, CFG_CBSH_ENA | CFG_DMAFORMAT | CFG_DMA_FTOGGLE | CFG_DMA_HSMOOTH | CFG_DMA_TSTMODE | CFG_DMA_MOD(CFG_SWAPRB | CFG_SWAPUV | CFG_SWAPYU | @@ -243,9 +239,9 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, LCD_SPU_DMA_CTRL0); } if (idx) { - armada_reg_queue_end(dplane->vbl.regs, idx); + armada_reg_queue_end(work->regs, idx); /* Queue it for update on the next interrupt if we are enabled */ - armada_drm_plane_work_queue(dcrtc, &dplane->vbl.work); + armada_drm_plane_work_queue(dcrtc, work); } return 0; } @@ -432,8 +428,8 @@ int armada_overlay_plane_create(struct drm_device *dev, unsigned long crtcs) return ret; }
- dplane->vbl.work.plane = &dplane->base.base; - dplane->vbl.work.fn = armada_ovl_plane_work; + dplane->work.plane = &dplane->base.base; + dplane->work.fn = armada_ovl_plane_work;
ret = drm_universal_plane_init(dev, &dplane->base.base, crtcs, &armada_ovl_plane_funcs,
Move writes of LCD_SPU_SRAM_PARA1 under the irq lock, so that we can add this to the frame updates at interrupt time when disabling a plane.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 10 ++++++---- drivers/gpu/drm/armada/armada_overlay.c | 5 +++-- 2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 02eefdc6f062..b0091142a79f 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -657,8 +657,6 @@ static int armada_drm_crtc_mode_set(struct drm_crtc *crtc, /* Now compute the divider for real */ dcrtc->variant->compute_clock(dcrtc, adj, &sclk);
- /* Ensure graphic fifo is enabled */ - armada_reg_queue_mod(regs, i, 0, CFG_PDWN64x66, LCD_SPU_SRAM_PARA1); armada_reg_queue_set(regs, i, sclk, LCD_CFG_SCLK_DIV);
if (interlaced ^ dcrtc->interlaced) { @@ -671,6 +669,9 @@ static int armada_drm_crtc_mode_set(struct drm_crtc *crtc,
spin_lock_irqsave(&dcrtc->irq_lock, flags);
+ /* Ensure graphic fifo is enabled */ + armada_reg_queue_mod(regs, i, 0, CFG_PDWN64x66, LCD_SPU_SRAM_PARA1); + /* Even interlaced/progressive frame */ dcrtc->v[1].spu_v_h_total = adj->crtc_vtotal << 16 | adj->crtc_htotal; @@ -869,9 +870,11 @@ static int armada_drm_crtc_cursor_update(struct armada_crtc *dcrtc, bool reload) return 0; }
+ spin_lock_irq(&dcrtc->irq_lock); para1 = readl_relaxed(dcrtc->base + LCD_SPU_SRAM_PARA1); armada_updatel(CFG_CSB_256x32, CFG_CSB_256x32 | CFG_PDWN256x32, dcrtc->base + LCD_SPU_SRAM_PARA1); + spin_unlock_irq(&dcrtc->irq_lock);
/* * Initialize the transparency if the SRAM was powered down. @@ -1157,9 +1160,8 @@ int armada_drm_plane_disable(struct drm_plane *plane,
spin_lock_irq(&dcrtc->irq_lock); armada_updatel(0, enable_mask, dcrtc->base + LCD_SPU_DMA_CTRL0); - spin_unlock_irq(&dcrtc->irq_lock); - armada_updatel(sram_para1, 0, dcrtc->base + LCD_SPU_SRAM_PARA1); + spin_unlock_irq(&dcrtc->irq_lock);
return 0; } diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 200223861bfb..e02d0d9d4c23 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -162,8 +162,9 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, return 0; } else if (~dplane->base.state.ctrl0 & ctrl0 & CFG_DMA_ENA) { /* Power up the Y/U/V FIFOs on ENA 0->1 transitions */ - armada_updatel(0, CFG_PDWN16x66 | CFG_PDWN32x66, - dcrtc->base + LCD_SPU_SRAM_PARA1); + armada_reg_queue_mod(work->regs, idx, + 0, CFG_PDWN16x66 | CFG_PDWN32x66, + LCD_SPU_SRAM_PARA1); }
if (armada_drm_plane_work_wait(&dplane->base, HZ / 25) == 0)
Only enable the HSMOOTH control bit if we are scaling horizontally, otherwise it makes no sense to enable the horizontal scaler.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 5 +++-- drivers/gpu/drm/armada/armada_overlay.c | 10 +++++----- 2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index b0091142a79f..401ad854d751 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -583,7 +583,8 @@ static void armada_drm_primary_set(struct drm_crtc *crtc, armada_reg_queue_mod(regs, i, ctrl0, CFG_GRAFORMAT | CFG_GRA_MOD(CFG_SWAPRB | CFG_SWAPUV | CFG_SWAPYU | CFG_YUV2RGB) | - CFG_PALETTE_ENA | CFG_GRA_FTOGGLE, + CFG_PALETTE_ENA | CFG_GRA_FTOGGLE | + CFG_GRA_HSMOOTH | CFG_GRA_ENA, LCD_SPU_DMA_CTRL0); armada_reg_queue_end(regs, i); armada_drm_crtc_update_regs(dcrtc, regs); @@ -605,7 +606,7 @@ static int armada_drm_crtc_mode_set(struct drm_crtc *crtc,
interlaced = !!(adj->flags & DRM_MODE_FLAG_INTERLACE);
- val = CFG_GRA_ENA | CFG_GRA_HSMOOTH; + val = CFG_GRA_ENA; val |= CFG_GRA_FMT(drm_fb_to_armada_fb(dcrtc->crtc.primary->fb)->fmt); val |= CFG_GRA_MOD(drm_fb_to_armada_fb(dcrtc->crtc.primary->fb)->mod);
diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index e02d0d9d4c23..19fce1a7159f 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -120,11 +120,11 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
ctrl0 = CFG_DMA_FMT(drm_fb_to_armada_fb(fb)->fmt) | CFG_DMA_MOD(drm_fb_to_armada_fb(fb)->mod) | - CFG_CBSH_ENA | CFG_DMA_HSMOOTH | CFG_DMA_ENA; - - /* Does the position/size result in nothing to display? */ - if (!visible) - ctrl0 &= ~CFG_DMA_ENA; + CFG_CBSH_ENA; + if (visible) + ctrl0 |= CFG_DMA_ENA; + if (drm_rect_width(&src) >> 16 != drm_rect_width(&dest)) + ctrl0 |= CFG_DMA_HSMOOTH;
/* * Shifting a YUV packed format image by one pixel causes the U/V
Use drm_plane_helper_check_state() to check the overlay plane state rather than drm_plane_helper_check_update(), as:
(1) using drm_plane_helper_check_state() provides a better migration path to atomic modeset (2) it avoids needless copies of drm rectangle structures, and so is more efficient.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_overlay.c | 61 +++++++++++++++++---------------- 1 file changed, 32 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 19fce1a7159f..010f3e438607 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -87,17 +87,19 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); struct armada_plane_work *work = &dplane->work; const struct drm_format_info *format; - struct drm_rect src = { - .x1 = src_x, - .y1 = src_y, - .x2 = src_x + src_w, - .y2 = src_y + src_h, - }; - struct drm_rect dest = { - .x1 = crtc_x, - .y1 = crtc_y, - .x2 = crtc_x + crtc_w, - .y2 = crtc_y + crtc_h, + struct drm_plane_state state = { + .plane = plane, + .crtc = crtc, + .fb = fb, + .src_x = src_x, + .src_y = src_y, + .src_w = src_w, + .src_h = src_h, + .crtc_x = crtc_x, + .crtc_y = crtc_y, + .crtc_w = crtc_w, + .crtc_h = crtc_h, + .rotation = DRM_MODE_ROTATE_0, }; const struct drm_rect clip = { .x2 = crtc->mode.hdisplay, @@ -105,25 +107,24 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, }; uint32_t val, ctrl0; unsigned idx = 0; - bool visible, fb_changed; + bool fb_changed; int ret;
trace_armada_ovl_plane_update(plane, crtc, fb, crtc_x, crtc_y, crtc_w, crtc_h, src_x, src_y, src_w, src_h);
- ret = drm_plane_helper_check_update(plane, crtc, fb, &src, &dest, &clip, - DRM_MODE_ROTATE_0, - 0, INT_MAX, true, false, &visible); + ret = drm_plane_helper_check_state(&state, &clip, 0, INT_MAX, true, + false); if (ret) return ret;
ctrl0 = CFG_DMA_FMT(drm_fb_to_armada_fb(fb)->fmt) | CFG_DMA_MOD(drm_fb_to_armada_fb(fb)->mod) | CFG_CBSH_ENA; - if (visible) + if (state.visible) ctrl0 |= CFG_DMA_ENA; - if (drm_rect_width(&src) >> 16 != drm_rect_width(&dest)) + if (drm_rect_width(&state.src) >> 16 != drm_rect_width(&state.dst)) ctrl0 |= CFG_DMA_HSMOOTH;
/* @@ -131,12 +132,12 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, * planes to swap. Compensate for it by also toggling the UV swap. */ format = fb->format; - if (format->num_planes == 1 && src.x1 >> 16 & (format->hsub - 1)) + if (format->num_planes == 1 && state.src.x1 >> 16 & (format->hsub - 1)) ctrl0 ^= CFG_DMA_MOD(CFG_SWAPUV);
fb_changed = plane->fb != fb || - dplane->base.state.src_x != src.x1 >> 16 || - dplane->base.state.src_y != src.y1 >> 16; + dplane->base.state.src_x != state.src.x1 >> 16 || + dplane->base.state.src_y != state.src.y1 >> 16;
if (!dcrtc->plane) { dcrtc->plane = plane; @@ -146,16 +147,17 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, /* FIXME: overlay on an interlaced display */ /* Just updating the position/size? */ if (!fb_changed && dplane->base.state.ctrl0 == ctrl0) { - val = (drm_rect_height(&src) & 0xffff0000) | - drm_rect_width(&src) >> 16; + val = (drm_rect_height(&state.src) & 0xffff0000) | + drm_rect_width(&state.src) >> 16; dplane->base.state.src_hw = val; writel_relaxed(val, dcrtc->base + LCD_SPU_DMA_HPXL_VLN);
- val = drm_rect_height(&dest) << 16 | drm_rect_width(&dest); + val = drm_rect_height(&state.dst) << 16 | + drm_rect_width(&state.dst); dplane->base.state.dst_hw = val; writel_relaxed(val, dcrtc->base + LCD_SPU_DZM_HPXL_VLN);
- val = dest.y1 << 16 | dest.x1; + val = state.dst.y1 << 16 | state.dst.x1; dplane->base.state.dst_yx = val; writel_relaxed(val, dcrtc->base + LCD_SPU_DMA_OVSA_HPXL_VLN);
@@ -181,8 +183,8 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
work->old_fb = plane->fb;
- dplane->base.state.src_y = src_y = src.y1 >> 16; - dplane->base.state.src_x = src_x = src.x1 >> 16; + dplane->base.state.src_y = src_y = state.src.y1 >> 16; + dplane->base.state.src_x = src_x = state.src.x1 >> 16;
armada_drm_plane_calc_addrs(addrs, fb, src_x, src_y);
@@ -209,21 +211,22 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, work->old_fb = NULL; }
- val = (drm_rect_height(&src) & 0xffff0000) | drm_rect_width(&src) >> 16; + val = (drm_rect_height(&state.src) & 0xffff0000) | + drm_rect_width(&state.src) >> 16; if (dplane->base.state.src_hw != val) { dplane->base.state.src_hw = val; armada_reg_queue_set(work->regs, idx, val, LCD_SPU_DMA_HPXL_VLN); }
- val = drm_rect_height(&dest) << 16 | drm_rect_width(&dest); + val = drm_rect_height(&state.dst) << 16 | drm_rect_width(&state.dst); if (dplane->base.state.dst_hw != val) { dplane->base.state.dst_hw = val; armada_reg_queue_set(work->regs, idx, val, LCD_SPU_DZM_HPXL_VLN); }
- val = dest.y1 << 16 | dest.x1; + val = state.dst.y1 << 16 | state.dst.x1; if (dplane->base.state.dst_yx != val) { dplane->base.state.dst_yx = val; armada_reg_queue_set(work->regs, idx, val,
Avoid printing an error message when armada_drm_plane_work_queue() is unable to get the vblank (eg, because we're doing a modeset.) Continue to report the failure to the caller, so the caller can handle this.
Move the error message into armada_ovl_plane_update().
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 4 +--- drivers/gpu/drm/armada/armada_overlay.c | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 401ad854d751..2f8e45976444 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -254,10 +254,8 @@ int armada_drm_plane_work_queue(struct armada_crtc *dcrtc, int ret;
ret = drm_crtc_vblank_get(&dcrtc->crtc); - if (ret) { - DRM_ERROR("failed to acquire vblank counter\n"); + if (ret) return ret; - }
ret = cmpxchg(&plane->work, NULL, work) ? -EBUSY : 0; if (ret) diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 010f3e438607..53edf42c5863 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -245,7 +245,9 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (idx) { armada_reg_queue_end(work->regs, idx); /* Queue it for update on the next interrupt if we are enabled */ - armada_drm_plane_work_queue(dcrtc, work); + ret = armada_drm_plane_work_queue(dcrtc, work); + if (ret) + DRM_ERROR("failed to queue plane work: %d\n", ret); } return 0; }
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 24 +++++++++++++----------- drivers/gpu/drm/armada/armada_crtc.h | 3 +++ drivers/gpu/drm/armada/armada_overlay.c | 11 +++++++---- 3 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 2f8e45976444..98fb955f6889 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -214,13 +214,15 @@ static void armada_drm_plane_work_call(struct armada_crtc *dcrtc, void (*fn)(struct armada_crtc *, struct armada_plane_work *)) { struct armada_plane *dplane = drm_to_armada_plane(work->plane); - struct drm_pending_vblank_event *event = work->event; - struct drm_framebuffer *fb = work->old_fb; + struct drm_pending_vblank_event *event; + struct drm_framebuffer *fb;
if (fn) fn(dcrtc, work); drm_crtc_vblank_put(&dcrtc->crtc);
+ event = work->event; + fb = work->old_fb; if (event || fb) { struct drm_device *dev = dcrtc->crtc.dev; unsigned long flags; @@ -233,6 +235,9 @@ static void armada_drm_plane_work_call(struct armada_crtc *dcrtc, spin_unlock_irqrestore(&dev->event_lock, flags); }
+ if (work->need_kfree) + kfree(work); + wake_up(&dplane->frame_wait); }
@@ -278,12 +283,6 @@ void armada_drm_plane_work_cancel(struct armada_crtc *dcrtc, armada_drm_plane_work_call(dcrtc, work, work->cancel); }
-static void armada_drm_crtc_finish_frame_work(struct armada_crtc *dcrtc, - struct armada_plane_work *work) -{ - kfree(work); -} - static void armada_drm_crtc_complete_frame_work(struct armada_crtc *dcrtc, struct armada_plane_work *work) { @@ -292,8 +291,6 @@ static void armada_drm_crtc_complete_frame_work(struct armada_crtc *dcrtc, spin_lock_irqsave(&dcrtc->irq_lock, flags); armada_drm_crtc_update_regs(dcrtc, work->regs); spin_unlock_irqrestore(&dcrtc->irq_lock, flags); - - armada_drm_crtc_finish_frame_work(dcrtc, work); }
static struct armada_plane_work * @@ -308,7 +305,7 @@ armada_drm_crtc_alloc_plane_work(struct drm_plane *plane)
work->plane = plane; work->fn = armada_drm_crtc_complete_frame_work; - work->cancel = armada_drm_crtc_finish_frame_work; + work->need_kfree = true; armada_reg_queue_end(work->regs, i);
return work; @@ -1173,6 +1170,11 @@ static const struct drm_plane_funcs armada_primary_plane_funcs = {
int armada_drm_plane_init(struct armada_plane *plane) { + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(plane->works); i++) + plane->works[i].plane = &plane->base; + init_waitqueue_head(&plane->frame_wait);
return 0; diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index 12ef9688a45a..0c7b519c09e8 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -38,6 +38,7 @@ struct armada_variant; struct armada_plane_work { void (*fn)(struct armada_crtc *, struct armada_plane_work *); void (*cancel)(struct armada_crtc *, struct armada_plane_work *); + bool need_kfree; struct drm_plane *plane; struct drm_framebuffer *old_fb; struct drm_pending_vblank_event *event; @@ -56,6 +57,8 @@ struct armada_plane_state { struct armada_plane { struct drm_plane base; wait_queue_head_t frame_wait; + bool next_work; + struct armada_plane_work works[2]; struct armada_plane_work *work; struct armada_plane_state state; }; diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 53edf42c5863..bad966ae6758 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -32,7 +32,6 @@ struct armada_ovl_plane_properties {
struct armada_ovl_plane { struct armada_plane base; - struct armada_plane_work work; struct armada_ovl_plane_properties prop; }; #define drm_to_armada_ovl_plane(p) \ @@ -85,7 +84,7 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, { struct armada_ovl_plane *dplane = drm_to_armada_ovl_plane(plane); struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); - struct armada_plane_work *work = &dplane->work; + struct armada_plane_work *work; const struct drm_format_info *format; struct drm_plane_state state = { .plane = plane, @@ -119,6 +118,8 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (ret) return ret;
+ work = &dplane->base.works[dplane->base.next_work]; + ctrl0 = CFG_DMA_FMT(drm_fb_to_armada_fb(fb)->fmt) | CFG_DMA_MOD(drm_fb_to_armada_fb(fb)->mod) | CFG_CBSH_ENA; @@ -248,6 +249,8 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, ret = armada_drm_plane_work_queue(dcrtc, work); if (ret) DRM_ERROR("failed to queue plane work: %d\n", ret); + + dplane->base.next_work = !dplane->base.next_work; } return 0; } @@ -434,8 +437,8 @@ int armada_overlay_plane_create(struct drm_device *dev, unsigned long crtcs) return ret; }
- dplane->work.plane = &dplane->base.base; - dplane->work.fn = armada_ovl_plane_work; + dplane->base.works[0].fn = armada_ovl_plane_work; + dplane->base.works[1].fn = armada_ovl_plane_work;
ret = drm_universal_plane_init(dev, &dplane->base.base, crtcs, &armada_ovl_plane_funcs,
Disable planes at the next blanking period rather than immediately. In order to achieve this, we need to delay the clearing of dcrtc->plane until after the next blanking period, so move that into a separate work function. To avoid races, we also need to move its assignment in the overlay code.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 59 ++++++++++++++++++++++++--------- drivers/gpu/drm/armada/armada_overlay.c | 23 ++++--------- 2 files changed, 50 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 98fb955f6889..c38a1409a14e 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -293,6 +293,19 @@ static void armada_drm_crtc_complete_frame_work(struct armada_crtc *dcrtc, spin_unlock_irqrestore(&dcrtc->irq_lock, flags); }
+static void armada_drm_crtc_complete_disable_work(struct armada_crtc *dcrtc, + struct armada_plane_work *work) +{ + unsigned long flags; + + if (dcrtc->plane == work->plane) + dcrtc->plane = NULL; + + spin_lock_irqsave(&dcrtc->irq_lock, flags); + armada_drm_crtc_update_regs(dcrtc, work->regs); + spin_unlock_irqrestore(&dcrtc->irq_lock, flags); +} + static struct armada_plane_work * armada_drm_crtc_alloc_plane_work(struct drm_plane *plane) { @@ -392,8 +405,11 @@ static void armada_drm_crtc_prepare(struct drm_crtc *crtc) * the new mode parameters. */ plane = dcrtc->plane; - if (plane) + if (plane) { drm_plane_force_disable(plane); + WARN_ON(!armada_drm_plane_work_wait(drm_to_armada_plane(plane), + HZ)); + } }
/* The mode_config.mutex will be held for this call */ @@ -1120,28 +1136,22 @@ int armada_drm_plane_disable(struct drm_plane *plane, { struct armada_plane *dplane = drm_to_armada_plane(plane); struct armada_crtc *dcrtc; + struct armada_plane_work *work; + unsigned int idx = 0; u32 sram_para1, enable_mask;
if (!plane->crtc) return 0;
/* - * Drop our reference on any framebuffer attached to this plane. - * We don't need to NULL this out as drm_plane_force_disable(), - * and __setplane_internal() will do so for an overlay plane, and - * __drm_helper_disable_unused_functions() will do so for the - * primary plane. + * Arrange to power down most RAMs and FIFOs if this is the primary + * plane, otherwise just the YUV FIFOs for the overlay plane. */ - if (plane->fb) - drm_framebuffer_put(plane->fb); - - /* Power down most RAMs and FIFOs if this is the primary plane */ if (plane->type == DRM_PLANE_TYPE_PRIMARY) { sram_para1 = CFG_PDWN256x32 | CFG_PDWN256x24 | CFG_PDWN256x8 | CFG_PDWN32x32 | CFG_PDWN64x66; enable_mask = CFG_GRA_ENA; } else { - /* Power down the Y/U/V FIFOs */ sram_para1 = CFG_PDWN16x66 | CFG_PDWN32x66; enable_mask = CFG_DMA_ENA; } @@ -1150,14 +1160,33 @@ int armada_drm_plane_disable(struct drm_plane *plane,
dcrtc = drm_to_armada_crtc(plane->crtc);
+ /* + * Try to disable the plane and drop our ref on the framebuffer + * at the next frame update. If we fail for any reason, disable + * the plane immediately. + */ + work = &dplane->works[dplane->next_work]; + work->fn = armada_drm_crtc_complete_disable_work; + work->cancel = armada_drm_crtc_complete_disable_work; + work->old_fb = plane->fb; + + armada_reg_queue_mod(work->regs, idx, + 0, enable_mask, LCD_SPU_DMA_CTRL0); + armada_reg_queue_mod(work->regs, idx, + sram_para1, 0, LCD_SPU_SRAM_PARA1); + armada_reg_queue_end(work->regs, idx); + /* Wait for any preceding work to complete, but don't wedge */ if (WARN_ON(!armada_drm_plane_work_wait(dplane, HZ))) armada_drm_plane_work_cancel(dcrtc, dplane);
- spin_lock_irq(&dcrtc->irq_lock); - armada_updatel(0, enable_mask, dcrtc->base + LCD_SPU_DMA_CTRL0); - armada_updatel(sram_para1, 0, dcrtc->base + LCD_SPU_SRAM_PARA1); - spin_unlock_irq(&dcrtc->irq_lock); + if (armada_drm_plane_work_queue(dcrtc, work)) { + work->fn(dcrtc, work); + if (work->old_fb) + drm_framebuffer_unreference(work->old_fb); + } + + dplane->next_work = !dplane->next_work;
return 0; } diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index bad966ae6758..0fe3f2db8ff5 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -140,11 +140,6 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, dplane->base.state.src_x != state.src.x1 >> 16 || dplane->base.state.src_y != state.src.y1 >> 16;
- if (!dcrtc->plane) { - dcrtc->plane = plane; - armada_ovl_update_attr(&dplane->prop, dcrtc); - } - /* FIXME: overlay on an interlaced display */ /* Just updating the position/size? */ if (!fb_changed && dplane->base.state.ctrl0 == ctrl0) { @@ -173,6 +168,11 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (armada_drm_plane_work_wait(&dplane->base, HZ / 25) == 0) armada_drm_plane_work_cancel(dcrtc, &dplane->base);
+ if (!dcrtc->plane) { + dcrtc->plane = plane; + armada_ovl_update_attr(&dplane->prop, dcrtc); + } + if (fb_changed) { u32 addrs[3];
@@ -255,17 +255,6 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, return 0; }
-static int armada_ovl_plane_disable(struct drm_plane *plane, - struct drm_modeset_acquire_ctx *ctx) -{ - armada_drm_plane_disable(plane, ctx); - - if (plane->crtc) - drm_to_armada_crtc(plane->crtc)->plane = NULL; - - return 0; -} - static void armada_ovl_plane_destroy(struct drm_plane *plane) { struct armada_ovl_plane *dplane = drm_to_armada_ovl_plane(plane); @@ -345,7 +334,7 @@ static int armada_ovl_plane_set_property(struct drm_plane *plane,
static const struct drm_plane_funcs armada_ovl_plane_funcs = { .update_plane = armada_ovl_plane_update, - .disable_plane = armada_ovl_plane_disable, + .disable_plane = armada_drm_plane_disable, .destroy = armada_ovl_plane_destroy, .set_property = armada_ovl_plane_set_property, };
Re-organise overlay register generation so that we do not have to wait for the previous update to complete while creating the new state. This allows the update to be fully prepared before queueing it for the next interrupt.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_overlay.c | 52 ++++++++++++++------------------- 1 file changed, 22 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 0fe3f2db8ff5..00da2c58701c 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -136,43 +136,18 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (format->num_planes == 1 && state.src.x1 >> 16 & (format->hsub - 1)) ctrl0 ^= CFG_DMA_MOD(CFG_SWAPUV);
- fb_changed = plane->fb != fb || - dplane->base.state.src_x != state.src.x1 >> 16 || - dplane->base.state.src_y != state.src.y1 >> 16; - - /* FIXME: overlay on an interlaced display */ - /* Just updating the position/size? */ - if (!fb_changed && dplane->base.state.ctrl0 == ctrl0) { - val = (drm_rect_height(&state.src) & 0xffff0000) | - drm_rect_width(&state.src) >> 16; - dplane->base.state.src_hw = val; - writel_relaxed(val, dcrtc->base + LCD_SPU_DMA_HPXL_VLN); - - val = drm_rect_height(&state.dst) << 16 | - drm_rect_width(&state.dst); - dplane->base.state.dst_hw = val; - writel_relaxed(val, dcrtc->base + LCD_SPU_DZM_HPXL_VLN); - - val = state.dst.y1 << 16 | state.dst.x1; - dplane->base.state.dst_yx = val; - writel_relaxed(val, dcrtc->base + LCD_SPU_DMA_OVSA_HPXL_VLN); - - return 0; - } else if (~dplane->base.state.ctrl0 & ctrl0 & CFG_DMA_ENA) { + if (~dplane->base.state.ctrl0 & ctrl0 & CFG_DMA_ENA) { /* Power up the Y/U/V FIFOs on ENA 0->1 transitions */ armada_reg_queue_mod(work->regs, idx, 0, CFG_PDWN16x66 | CFG_PDWN32x66, LCD_SPU_SRAM_PARA1); }
- if (armada_drm_plane_work_wait(&dplane->base, HZ / 25) == 0) - armada_drm_plane_work_cancel(dcrtc, &dplane->base); - - if (!dcrtc->plane) { - dcrtc->plane = plane; - armada_ovl_update_attr(&dplane->prop, dcrtc); - } + fb_changed = plane->fb != fb || + dplane->base.state.src_x != state.src.x1 >> 16 || + dplane->base.state.src_y != state.src.y1 >> 16;
+ /* FIXME: overlay on an interlaced display */ if (fb_changed) { u32 addrs[3];
@@ -243,6 +218,23 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, CFG_YUV2RGB) | CFG_DMA_ENA, LCD_SPU_DMA_CTRL0); } + + /* Just updating the position/size? */ + if (!fb_changed && dplane->base.state.ctrl0 == ctrl0) { + armada_reg_queue_end(work->regs, idx); + armada_ovl_plane_work(dcrtc, work); + return 0; + } + + /* Wait for pending work to complete */ + if (armada_drm_plane_work_wait(&dplane->base, HZ / 25) == 0) + armada_drm_plane_work_cancel(dcrtc, &dplane->base); + + if (!dcrtc->plane) { + dcrtc->plane = plane; + armada_ovl_update_attr(&dplane->prop, dcrtc); + } + if (idx) { armada_reg_queue_end(work->regs, idx); /* Queue it for update on the next interrupt if we are enabled */
Move the overlay plane register update generation to a separate function as this is independent of the legacy or atomic update.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.h | 2 + drivers/gpu/drm/armada/armada_overlay.c | 203 +++++++++++++++++--------------- 2 files changed, 112 insertions(+), 93 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index 0c7b519c09e8..445829b8877a 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -52,6 +52,8 @@ struct armada_plane_state { u32 dst_hw; u32 dst_yx; u32 ctrl0; + bool changed; + bool vsync_update; };
struct armada_plane { diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 00da2c58701c..e5fa346f572b 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -75,153 +75,172 @@ static void armada_ovl_plane_work(struct armada_crtc *dcrtc, spin_unlock_irqrestore(&dcrtc->irq_lock, flags); }
-static int -armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, - int crtc_x, int crtc_y, unsigned crtc_w, unsigned crtc_h, - uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h, - struct drm_modeset_acquire_ctx *ctx) +static void armada_ovl_plane_update_state(struct drm_plane_state *state, + struct armada_regs *regs) { - struct armada_ovl_plane *dplane = drm_to_armada_ovl_plane(plane); - struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); - struct armada_plane_work *work; + struct armada_ovl_plane *dplane = drm_to_armada_ovl_plane(state->plane); + struct armada_framebuffer *dfb = drm_fb_to_armada_fb(state->fb); const struct drm_format_info *format; - struct drm_plane_state state = { - .plane = plane, - .crtc = crtc, - .fb = fb, - .src_x = src_x, - .src_y = src_y, - .src_w = src_w, - .src_h = src_h, - .crtc_x = crtc_x, - .crtc_y = crtc_y, - .crtc_w = crtc_w, - .crtc_h = crtc_h, - .rotation = DRM_MODE_ROTATE_0, - }; - const struct drm_rect clip = { - .x2 = crtc->mode.hdisplay, - .y2 = crtc->mode.vdisplay, - }; - uint32_t val, ctrl0; - unsigned idx = 0; + unsigned int idx = 0; bool fb_changed; - int ret; + u32 val, ctrl0; + u16 src_x, src_y;
- trace_armada_ovl_plane_update(plane, crtc, fb, - crtc_x, crtc_y, crtc_w, crtc_h, - src_x, src_y, src_w, src_h); - - ret = drm_plane_helper_check_state(&state, &clip, 0, INT_MAX, true, - false); - if (ret) - return ret; - - work = &dplane->base.works[dplane->base.next_work]; - - ctrl0 = CFG_DMA_FMT(drm_fb_to_armada_fb(fb)->fmt) | - CFG_DMA_MOD(drm_fb_to_armada_fb(fb)->mod) | - CFG_CBSH_ENA; - if (state.visible) + ctrl0 = CFG_DMA_FMT(dfb->fmt) | CFG_DMA_MOD(dfb->mod) | CFG_CBSH_ENA; + if (state->visible) ctrl0 |= CFG_DMA_ENA; - if (drm_rect_width(&state.src) >> 16 != drm_rect_width(&state.dst)) + if (drm_rect_width(&state->src) >> 16 != drm_rect_width(&state->dst)) ctrl0 |= CFG_DMA_HSMOOTH;
/* * Shifting a YUV packed format image by one pixel causes the U/V * planes to swap. Compensate for it by also toggling the UV swap. */ - format = fb->format; - if (format->num_planes == 1 && state.src.x1 >> 16 & (format->hsub - 1)) + format = dfb->fb.format; + if (format->num_planes == 1 && state->src.x1 >> 16 & (format->hsub - 1)) ctrl0 ^= CFG_DMA_MOD(CFG_SWAPUV);
if (~dplane->base.state.ctrl0 & ctrl0 & CFG_DMA_ENA) { /* Power up the Y/U/V FIFOs on ENA 0->1 transitions */ - armada_reg_queue_mod(work->regs, idx, + armada_reg_queue_mod(regs, idx, 0, CFG_PDWN16x66 | CFG_PDWN32x66, LCD_SPU_SRAM_PARA1); }
- fb_changed = plane->fb != fb || - dplane->base.state.src_x != state.src.x1 >> 16 || - dplane->base.state.src_y != state.src.y1 >> 16; + fb_changed = dplane->base.base.fb != &dfb->fb || + dplane->base.state.src_x != state->src.x1 >> 16 || + dplane->base.state.src_y != state->src.y1 >> 16; + + dplane->base.state.vsync_update = fb_changed;
/* FIXME: overlay on an interlaced display */ if (fb_changed) { u32 addrs[3];
- /* - * Take a reference on the new framebuffer - we want to - * hold on to it while the hardware is displaying it. - */ - drm_framebuffer_get(fb); - - work->old_fb = plane->fb; + dplane->base.state.src_y = src_y = state->src.y1 >> 16; + dplane->base.state.src_x = src_x = state->src.x1 >> 16;
- dplane->base.state.src_y = src_y = state.src.y1 >> 16; - dplane->base.state.src_x = src_x = state.src.x1 >> 16; + armada_drm_plane_calc_addrs(addrs, &dfb->fb, src_x, src_y);
- armada_drm_plane_calc_addrs(addrs, fb, src_x, src_y); - - armada_reg_queue_set(work->regs, idx, addrs[0], + armada_reg_queue_set(regs, idx, addrs[0], LCD_SPU_DMA_START_ADDR_Y0); - armada_reg_queue_set(work->regs, idx, addrs[1], + armada_reg_queue_set(regs, idx, addrs[1], LCD_SPU_DMA_START_ADDR_U0); - armada_reg_queue_set(work->regs, idx, addrs[2], + armada_reg_queue_set(regs, idx, addrs[2], LCD_SPU_DMA_START_ADDR_V0); - armada_reg_queue_set(work->regs, idx, addrs[0], + armada_reg_queue_set(regs, idx, addrs[0], LCD_SPU_DMA_START_ADDR_Y1); - armada_reg_queue_set(work->regs, idx, addrs[1], + armada_reg_queue_set(regs, idx, addrs[1], LCD_SPU_DMA_START_ADDR_U1); - armada_reg_queue_set(work->regs, idx, addrs[2], + armada_reg_queue_set(regs, idx, addrs[2], LCD_SPU_DMA_START_ADDR_V1);
- val = fb->pitches[0] << 16 | fb->pitches[0]; - armada_reg_queue_set(work->regs, idx, val, + val = dfb->fb.pitches[0] << 16 | dfb->fb.pitches[0]; + armada_reg_queue_set(regs, idx, val, LCD_SPU_DMA_PITCH_YC); - val = fb->pitches[1] << 16 | fb->pitches[2]; - armada_reg_queue_set(work->regs, idx, val, + val = dfb->fb.pitches[1] << 16 | dfb->fb.pitches[2]; + armada_reg_queue_set(regs, idx, val, LCD_SPU_DMA_PITCH_UV); - } else { - work->old_fb = NULL; }
- val = (drm_rect_height(&state.src) & 0xffff0000) | - drm_rect_width(&state.src) >> 16; + val = (drm_rect_height(&state->src) & 0xffff0000) | + drm_rect_width(&state->src) >> 16; if (dplane->base.state.src_hw != val) { dplane->base.state.src_hw = val; - armada_reg_queue_set(work->regs, idx, val, + armada_reg_queue_set(regs, idx, val, LCD_SPU_DMA_HPXL_VLN); }
- val = drm_rect_height(&state.dst) << 16 | drm_rect_width(&state.dst); + val = drm_rect_height(&state->dst) << 16 | drm_rect_width(&state->dst); if (dplane->base.state.dst_hw != val) { dplane->base.state.dst_hw = val; - armada_reg_queue_set(work->regs, idx, val, + armada_reg_queue_set(regs, idx, val, LCD_SPU_DZM_HPXL_VLN); }
- val = state.dst.y1 << 16 | state.dst.x1; + val = state->dst.y1 << 16 | state->dst.x1; if (dplane->base.state.dst_yx != val) { dplane->base.state.dst_yx = val; - armada_reg_queue_set(work->regs, idx, val, + armada_reg_queue_set(regs, idx, val, LCD_SPU_DMA_OVSA_HPXL_VLN); }
if (dplane->base.state.ctrl0 != ctrl0) { dplane->base.state.ctrl0 = ctrl0; - armada_reg_queue_mod(work->regs, idx, ctrl0, + armada_reg_queue_mod(regs, idx, ctrl0, CFG_CBSH_ENA | CFG_DMAFORMAT | CFG_DMA_FTOGGLE | CFG_DMA_HSMOOTH | CFG_DMA_TSTMODE | CFG_DMA_MOD(CFG_SWAPRB | CFG_SWAPUV | CFG_SWAPYU | CFG_YUV2RGB) | CFG_DMA_ENA, LCD_SPU_DMA_CTRL0); + dplane->base.state.vsync_update = true; }
+ dplane->base.state.changed = idx != 0; + + armada_reg_queue_end(regs, idx); +} + +static int +armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, + struct drm_framebuffer *fb, + int crtc_x, int crtc_y, unsigned crtc_w, unsigned crtc_h, + uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h, + struct drm_modeset_acquire_ctx *ctx) +{ + struct armada_ovl_plane *dplane = drm_to_armada_ovl_plane(plane); + struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); + struct armada_plane_work *work; + struct drm_plane_state state = { + .plane = plane, + .crtc = crtc, + .fb = fb, + .src_x = src_x, + .src_y = src_y, + .src_w = src_w, + .src_h = src_h, + .crtc_x = crtc_x, + .crtc_y = crtc_y, + .crtc_w = crtc_w, + .crtc_h = crtc_h, + .rotation = DRM_MODE_ROTATE_0, + }; + const struct drm_rect clip = { + .x2 = crtc->mode.hdisplay, + .y2 = crtc->mode.vdisplay, + }; + int ret; + + trace_armada_ovl_plane_update(plane, crtc, fb, + crtc_x, crtc_y, crtc_w, crtc_h, + src_x, src_y, src_w, src_h); + + ret = drm_plane_helper_check_state(&state, &clip, 0, INT_MAX, true, + false); + if (ret) + return ret; + + work = &dplane->base.works[dplane->base.next_work]; + + if (plane->fb != fb) { + /* + * Take a reference on the new framebuffer - we want to + * hold on to it while the hardware is displaying it. + */ + drm_framebuffer_reference(fb); + + work->old_fb = plane->fb; + } else { + work->old_fb = NULL; + } + + armada_ovl_plane_update_state(&state, work->regs); + + if (!dplane->base.state.changed) + return 0; + /* Just updating the position/size? */ - if (!fb_changed && dplane->base.state.ctrl0 == ctrl0) { - armada_reg_queue_end(work->regs, idx); + if (!dplane->base.state.vsync_update) { armada_ovl_plane_work(dcrtc, work); return 0; } @@ -235,15 +254,13 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, armada_ovl_update_attr(&dplane->prop, dcrtc); }
- if (idx) { - armada_reg_queue_end(work->regs, idx); - /* Queue it for update on the next interrupt if we are enabled */ - ret = armada_drm_plane_work_queue(dcrtc, work); - if (ret) - DRM_ERROR("failed to queue plane work: %d\n", ret); + /* Queue it for update on the next interrupt if we are enabled */ + ret = armada_drm_plane_work_queue(dcrtc, work); + if (ret) + DRM_ERROR("failed to queue plane work: %d\n", ret); + + dplane->base.next_work = !dplane->base.next_work;
- dplane->base.next_work = !dplane->base.next_work; - } return 0; }
We must wait for the previous plane work to complete before moving the overlay window, as it could overwrite our positioning update.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_overlay.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index e5fa346f572b..853f889e84f5 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -239,16 +239,16 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (!dplane->base.state.changed) return 0;
+ /* Wait for pending work to complete */ + if (armada_drm_plane_work_wait(&dplane->base, HZ / 25) == 0) + armada_drm_plane_work_cancel(dcrtc, &dplane->base); + /* Just updating the position/size? */ if (!dplane->base.state.vsync_update) { armada_ovl_plane_work(dcrtc, work); return 0; }
- /* Wait for pending work to complete */ - if (armada_drm_plane_work_wait(&dplane->base, HZ / 25) == 0) - armada_drm_plane_work_cancel(dcrtc, &dplane->base); - if (!dcrtc->plane) { dcrtc->plane = plane; armada_ovl_update_attr(&dplane->prop, dcrtc);
Extract the register generation from armada_drm_primary_set(), so that it can be re-used.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index c38a1409a14e..9958f2d3d0e8 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -571,18 +571,14 @@ static uint32_t armada_drm_crtc_calculate_csc(struct armada_crtc *dcrtc) return val; }
-static void armada_drm_primary_set(struct drm_crtc *crtc, - struct drm_plane *plane, int x, int y) +static void armada_drm_gra_plane_regs(struct armada_regs *regs, + struct drm_framebuffer *fb, struct armada_plane_state *state, + int x, int y, bool interlaced) { - struct armada_plane_state *state = &drm_to_armada_plane(plane)->state; - struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); - struct armada_regs regs[8]; - bool interlaced = dcrtc->interlaced; - unsigned i; + unsigned int i; u32 ctrl0;
- i = armada_drm_crtc_calc_fb(plane->fb, x, y, regs, interlaced); - + i = armada_drm_crtc_calc_fb(fb, x, y, regs, interlaced); armada_reg_queue_set(regs, i, state->dst_yx, LCD_SPU_GRA_OVSA_HPXL_VLN); armada_reg_queue_set(regs, i, state->src_hw, LCD_SPU_GRA_HPXL_VLN); armada_reg_queue_set(regs, i, state->dst_hw, LCD_SPU_GZM_HPXL_VLN); @@ -598,6 +594,17 @@ static void armada_drm_primary_set(struct drm_crtc *crtc, CFG_GRA_HSMOOTH | CFG_GRA_ENA, LCD_SPU_DMA_CTRL0); armada_reg_queue_end(regs, i); +} + +static void armada_drm_primary_set(struct drm_crtc *crtc, + struct drm_plane *plane, int x, int y) +{ + struct armada_plane_state *state = &drm_to_armada_plane(plane)->state; + struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); + struct armada_regs regs[8]; + bool interlaced = dcrtc->interlaced; + + armada_drm_gra_plane_regs(regs, plane->fb, state, x, y, interlaced); armada_drm_crtc_update_regs(dcrtc, regs); }
Implement primary plane update without having to go through a modeset to achieve that; the hardware does not require such complexity. This means we treat the primary plane as any other, allowing the format, size, position and scaling to be updated via the normal plane ioctls.
This also allows us to seemlessly disable and re-enable the primary plane when (eg) displaying full-frame video without any graphic clipping the overlaid video - which saves wasting memory bandwidth needlessly verifying that the colorkey is indeed filling the entire primary plane.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 118 ++++++++++++++++++++++++++++++++++- 1 file changed, 117 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 9958f2d3d0e8..8b66377a4890 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -1138,6 +1138,122 @@ static const struct drm_crtc_funcs armada_crtc_funcs = { .disable_vblank = armada_drm_crtc_disable_vblank, };
+static void armada_drm_primary_update_state(struct drm_plane_state *state, + struct armada_regs *regs) +{ + struct armada_plane *dplane = drm_to_armada_plane(state->plane); + struct armada_crtc *dcrtc = drm_to_armada_crtc(state->crtc); + struct armada_framebuffer *dfb = drm_fb_to_armada_fb(state->fb); + bool was_disabled; + unsigned int idx = 0; + u32 val; + + val = CFG_GRA_FMT(dfb->fmt) | CFG_GRA_MOD(dfb->mod); + if (dfb->fmt > CFG_420) + val |= CFG_PALETTE_ENA; + if (state->visible) + val |= CFG_GRA_ENA; + if (drm_rect_width(&state->src) >> 16 != drm_rect_width(&state->dst)) + val |= CFG_GRA_HSMOOTH; + + was_disabled = !(dplane->state.ctrl0 & CFG_GRA_ENA); + if (was_disabled) + armada_reg_queue_mod(regs, idx, + 0, CFG_PDWN64x66, LCD_SPU_SRAM_PARA1); + + dplane->state.ctrl0 = val; + dplane->state.src_hw = (drm_rect_height(&state->src) & 0xffff0000) | + drm_rect_width(&state->src) >> 16; + dplane->state.dst_hw = drm_rect_height(&state->dst) << 16 | + drm_rect_width(&state->dst); + dplane->state.dst_yx = state->dst.y1 << 16 | state->dst.x1; + + armada_drm_gra_plane_regs(regs + idx, &dfb->fb, &dplane->state, + state->src.x1 >> 16, state->src.y1 >> 16, + dcrtc->interlaced); + + dplane->state.vsync_update = !was_disabled; + dplane->state.changed = true; +} + +static int armada_drm_primary_update(struct drm_plane *plane, + struct drm_crtc *crtc, struct drm_framebuffer *fb, + int crtc_x, int crtc_y, unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h, + struct drm_modeset_acquire_ctx *ctx) +{ + struct armada_plane *dplane = drm_to_armada_plane(plane); + struct armada_crtc *dcrtc = drm_to_armada_crtc(crtc); + struct armada_plane_work *work; + struct drm_plane_state state = { + .plane = plane, + .crtc = crtc, + .fb = fb, + .src_x = src_x, + .src_y = src_y, + .src_w = src_w, + .src_h = src_h, + .crtc_x = crtc_x, + .crtc_y = crtc_y, + .crtc_w = crtc_w, + .crtc_h = crtc_h, + .rotation = DRM_MODE_ROTATE_0, + }; + const struct drm_rect clip = { + .x2 = crtc->mode.hdisplay, + .y2 = crtc->mode.vdisplay, + }; + int ret; + + ret = drm_plane_helper_check_state(&state, &clip, 0, INT_MAX, true, + false); + if (ret) + return ret; + + work = &dplane->works[dplane->next_work]; + work->fn = armada_drm_crtc_complete_frame_work; + + if (plane->fb != fb) { + /* + * Take a reference on the new framebuffer - we want to + * hold on to it while the hardware is displaying it. + */ + drm_framebuffer_reference(fb); + + work->old_fb = plane->fb; + } else { + work->old_fb = NULL; + } + + armada_drm_primary_update_state(&state, work->regs); + + if (!dplane->state.changed) + return 0; + + /* Wait for pending work to complete */ + if (armada_drm_plane_work_wait(dplane, HZ / 10) == 0) + armada_drm_plane_work_cancel(dcrtc, dplane); + + if (!dplane->state.vsync_update) { + work->fn(dcrtc, work); + if (work->old_fb) + drm_framebuffer_unreference(work->old_fb); + return 0; + } + + /* Queue it for update on the next interrupt if we are enabled */ + ret = armada_drm_plane_work_queue(dcrtc, work); + if (ret) { + work->fn(dcrtc, work); + if (work->old_fb) + drm_framebuffer_unreference(work->old_fb); + } + + dplane->next_work = !dplane->next_work; + + return 0; +} + int armada_drm_plane_disable(struct drm_plane *plane, struct drm_modeset_acquire_ctx *ctx) { @@ -1199,7 +1315,7 @@ int armada_drm_plane_disable(struct drm_plane *plane, }
static const struct drm_plane_funcs armada_primary_plane_funcs = { - .update_plane = drm_primary_helper_update, + .update_plane = armada_drm_primary_update, .disable_plane = armada_drm_plane_disable, .destroy = drm_primary_helper_destroy, };
Add CRTC and source positions to the Armada overlay trace entry.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_trace.h | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_trace.h b/drivers/gpu/drm/armada/armada_trace.h index 8dbfea7a00fe..f03a56bda596 100644 --- a/drivers/gpu/drm/armada/armada_trace.h +++ b/drivers/gpu/drm/armada/armada_trace.h @@ -34,14 +34,34 @@ TRACE_EVENT(armada_ovl_plane_update, __field(struct drm_plane *, plane) __field(struct drm_crtc *, crtc) __field(struct drm_framebuffer *, fb) + __field(int, crtc_x) + __field(int, crtc_y) + __field(unsigned int, crtc_w) + __field(unsigned int, crtc_h) + __field(u32, src_x) + __field(u32, src_y) + __field(u32, src_w) + __field(u32, src_h) ), TP_fast_assign( __entry->plane = plane; __entry->crtc = crtc; __entry->fb = fb; + __entry->crtc_x = crtc_x; + __entry->crtc_y = crtc_y; + __entry->crtc_w = crtc_w; + __entry->crtc_h = crtc_h; + __entry->src_x = src_x; + __entry->src_y = src_y; + __entry->src_w = src_w; + __entry->src_h = src_h; ), - TP_printk("plane %p crtc %p fb %p", - __entry->plane, __entry->crtc, __entry->fb) + TP_printk("plane %p crtc %p fb %p crtc @ (%d,%d, %ux%u) src @ (%u,%u, %ux%u)", + __entry->plane, __entry->crtc, __entry->fb, + __entry->crtc_x, __entry->crtc_y, + __entry->crtc_w, __entry->crtc_h, + __entry->src_x >> 16, __entry->src_y >> 16, + __entry->src_w >> 16, __entry->src_h >> 16) );
TRACE_EVENT(armada_ovl_plane_work,
Add the defacto-standard "iturbt_709" property to the overlay plane to control the YUV to RGB colorspace conversion. This is mutually exclusive with the CSC_YUV CRTC property - the last property to be set determines the resulting colorspace conversion.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk --- drivers/gpu/drm/armada/armada_crtc.c | 26 +++++++++++++++++++++++--- drivers/gpu/drm/armada/armada_crtc.h | 2 ++ drivers/gpu/drm/armada/armada_drm.h | 1 + drivers/gpu/drm/armada/armada_overlay.c | 25 ++++++++++++++++++++++--- 4 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 8b66377a4890..1bf632bb8855 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -535,12 +535,30 @@ static irqreturn_t armada_drm_irq(int irq, void *arg) return IRQ_NONE; }
+void armada_drm_crtc_set_yuv_colorimetry(struct armada_crtc *dcrtc, u32 csc) +{ + armada_updatel(csc, CFG_CSC_YUV_CCIR709, + dcrtc->base + LCD_SPU_IOPAD_CONTROL); + + dcrtc->csc_yuv_ovl_mode = csc == CFG_CSC_YUV_CCIR709 ? + CSC_YUV_CCIR709 : CSC_YUV_CCIR601; +} + static uint32_t armada_drm_crtc_calculate_csc(struct armada_crtc *dcrtc) { struct drm_display_mode *adj = &dcrtc->crtc.mode; - uint32_t val = 0; + u32 val = 0; + u8 csc_yuv_mode; + + /* + * If the iturbt_709 overlay plane property is used, it overrides + * the CSC_YUV crtc property until the CSC_YUV property is set. + */ + csc_yuv_mode = dcrtc->csc_yuv_ovl_mode; + if (csc_yuv_mode == CSC_AUTO) + csc_yuv_mode = dcrtc->csc_yuv_mode;
- if (dcrtc->csc_yuv_mode == CSC_YUV_CCIR709) + if (csc_yuv_mode == CSC_YUV_CCIR709) val |= CFG_CSC_YUV_CCIR709; if (dcrtc->csc_rgb_mode == CSC_RGB_STUDIO) val |= CFG_CSC_RGB_STUDIO; @@ -555,7 +573,7 @@ static uint32_t armada_drm_crtc_calculate_csc(struct armada_crtc *dcrtc) if ((adj->hdisplay == 1280 && adj->vdisplay == 720 && !(adj->flags & DRM_MODE_FLAG_INTERLACE)) || (adj->hdisplay == 1920 && adj->vdisplay == 1080)) { - if (dcrtc->csc_yuv_mode == CSC_AUTO) + if (csc_yuv_mode == CSC_AUTO) val |= CFG_CSC_YUV_CCIR709; }
@@ -1094,6 +1112,7 @@ armada_drm_crtc_set_property(struct drm_crtc *crtc,
if (property == priv->csc_yuv_prop) { dcrtc->csc_yuv_mode = val; + dcrtc->csc_yuv_ovl_mode = CSC_AUTO; update_csc = true; } else if (property == priv->csc_rgb_prop) { dcrtc->csc_rgb_mode = val; @@ -1396,6 +1415,7 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev, dcrtc->num = drm->mode_config.num_crtc; dcrtc->clk = ERR_PTR(-EINVAL); dcrtc->csc_yuv_mode = CSC_AUTO; + dcrtc->csc_yuv_ovl_mode = CSC_AUTO; dcrtc->csc_rgb_mode = CSC_AUTO; dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0; dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24; diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index 445829b8877a..5d70b4af20b7 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -90,6 +90,7 @@ struct armada_crtc { bool interlaced; bool cursor_update; uint8_t csc_yuv_mode; + uint8_t csc_yuv_ovl_mode; uint8_t csc_rgb_mode;
struct drm_plane *plane; @@ -113,6 +114,7 @@ struct armada_crtc { #define drm_to_armada_crtc(c) container_of(c, struct armada_crtc, crtc)
void armada_drm_crtc_update_regs(struct armada_crtc *, struct armada_regs *); +void armada_drm_crtc_set_yuv_colorimetry(struct armada_crtc *dcrtc, u32 csc);
int armada_drm_plane_disable(struct drm_plane *plane, struct drm_modeset_acquire_ctx *ctx); diff --git a/drivers/gpu/drm/armada/armada_drm.h b/drivers/gpu/drm/armada/armada_drm.h index b064879ecdbd..45d5168d5748 100644 --- a/drivers/gpu/drm/armada/armada_drm.h +++ b/drivers/gpu/drm/armada/armada_drm.h @@ -60,6 +60,7 @@ struct armada_private { struct armada_crtc *dcrtc[2]; struct drm_mm linear; /* protected by linear_lock */ struct mutex linear_lock; + struct drm_property *iturbt709_prop; struct drm_property *csc_yuv_prop; struct drm_property *csc_rgb_prop; struct drm_property *colorkey_prop; diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 853f889e84f5..d7a9c79f0ada 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -28,6 +28,8 @@ struct armada_ovl_plane_properties { uint16_t contrast; uint16_t saturation; uint32_t colorkey_mode; + uint32_t csc; + bool csc_set; };
struct armada_ovl_plane { @@ -252,6 +254,10 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (!dcrtc->plane) { dcrtc->plane = plane; armada_ovl_update_attr(&dplane->prop, dcrtc); + if (dplane->prop.csc_set) { + armada_drm_crtc_set_yuv_colorimetry(dcrtc, dplane->prop.csc); + dplane->prop.csc_set = false; + } }
/* Queue it for update on the next interrupt if we are enabled */ @@ -332,11 +338,21 @@ static int armada_ovl_plane_set_property(struct drm_plane *plane, } else if (property == priv->saturation_prop) { dplane->prop.saturation = val; update_attr = true; + } else if (property == priv->iturbt709_prop) { + dplane->prop.csc = val ? CFG_CSC_YUV_CCIR709 : + CFG_CSC_YUV_CCIR601; + dplane->prop.csc_set = true; }
- if (update_attr && dplane->base.base.crtc) - armada_ovl_update_attr(&dplane->prop, - drm_to_armada_crtc(dplane->base.base.crtc)); + if (plane->crtc) { + struct armada_crtc *dcrtc = drm_to_armada_crtc(plane->crtc); + if (update_attr) + armada_ovl_update_attr(&dplane->prop, dcrtc); + if (dplane->prop.csc_set) { + armada_drm_crtc_set_yuv_colorimetry(dcrtc, dplane->prop.csc); + dplane->prop.csc_set = false; + } + }
return 0; } @@ -407,6 +423,8 @@ static int armada_overlay_create_properties(struct drm_device *dev) "contrast", 0, 0x7fff); priv->saturation_prop = drm_property_create_range(dev, 0, "saturation", 0, 0x7fff); + priv->iturbt709_prop = drm_property_create_bool(dev, 0, + "iturbt_709");
if (!priv->colorkey_prop) return -ENOMEM; @@ -475,6 +493,7 @@ int armada_overlay_plane_create(struct drm_device *dev, unsigned long crtcs) dplane->prop.contrast); drm_object_attach_property(mobj, priv->saturation_prop, dplane->prop.saturation); + drm_object_attach_property(mobj, priv->iturbt709_prop, false);
return 0; }
On Fri, Dec 08, 2017 at 12:31:08PM +0000, Russell King wrote:
Add the defacto-standard "iturbt_709" property to the overlay plane to control the YUV to RGB colorspace conversion. This is mutually exclusive with the CSC_YUV CRTC property - the last property to be set determines the resulting colorspace conversion.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk
Can you pls provide a link to the corresponding userspace code using this?
Thanks, Daniel
drivers/gpu/drm/armada/armada_crtc.c | 26 +++++++++++++++++++++++--- drivers/gpu/drm/armada/armada_crtc.h | 2 ++ drivers/gpu/drm/armada/armada_drm.h | 1 + drivers/gpu/drm/armada/armada_overlay.c | 25 ++++++++++++++++++++++--- 4 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 8b66377a4890..1bf632bb8855 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -535,12 +535,30 @@ static irqreturn_t armada_drm_irq(int irq, void *arg) return IRQ_NONE; }
+void armada_drm_crtc_set_yuv_colorimetry(struct armada_crtc *dcrtc, u32 csc) +{
- armada_updatel(csc, CFG_CSC_YUV_CCIR709,
dcrtc->base + LCD_SPU_IOPAD_CONTROL);- dcrtc->csc_yuv_ovl_mode = csc == CFG_CSC_YUV_CCIR709 ?
CSC_YUV_CCIR709 : CSC_YUV_CCIR601;+}
static uint32_t armada_drm_crtc_calculate_csc(struct armada_crtc *dcrtc) { struct drm_display_mode *adj = &dcrtc->crtc.mode;
- uint32_t val = 0;
- u32 val = 0;
- u8 csc_yuv_mode;
- /*
* If the iturbt_709 overlay plane property is used, it overrides* the CSC_YUV crtc property until the CSC_YUV property is set.*/- csc_yuv_mode = dcrtc->csc_yuv_ovl_mode;
- if (csc_yuv_mode == CSC_AUTO)
csc_yuv_mode = dcrtc->csc_yuv_mode;
- if (dcrtc->csc_yuv_mode == CSC_YUV_CCIR709)
- if (csc_yuv_mode == CSC_YUV_CCIR709) val |= CFG_CSC_YUV_CCIR709; if (dcrtc->csc_rgb_mode == CSC_RGB_STUDIO) val |= CFG_CSC_RGB_STUDIO;
@@ -555,7 +573,7 @@ static uint32_t armada_drm_crtc_calculate_csc(struct armada_crtc *dcrtc) if ((adj->hdisplay == 1280 && adj->vdisplay == 720 && !(adj->flags & DRM_MODE_FLAG_INTERLACE)) || (adj->hdisplay == 1920 && adj->vdisplay == 1080)) {
if (dcrtc->csc_yuv_mode == CSC_AUTO)
if (csc_yuv_mode == CSC_AUTO) val |= CFG_CSC_YUV_CCIR709;@@ -1094,6 +1112,7 @@ armada_drm_crtc_set_property(struct drm_crtc *crtc,
if (property == priv->csc_yuv_prop) { dcrtc->csc_yuv_mode = val;
dcrtc->csc_yuv_ovl_mode = CSC_AUTO;@@ -1396,6 +1415,7 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev, dcrtc->num = drm->mode_config.num_crtc; dcrtc->clk = ERR_PTR(-EINVAL); dcrtc->csc_yuv_mode = CSC_AUTO;
- dcrtc->csc_yuv_ovl_mode = CSC_AUTO; dcrtc->csc_rgb_mode = CSC_AUTO; dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0; dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index 445829b8877a..5d70b4af20b7 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -90,6 +90,7 @@ struct armada_crtc { bool interlaced; bool cursor_update; uint8_t csc_yuv_mode;
uint8_t csc_yuv_ovl_mode; uint8_t csc_rgb_mode;
struct drm_plane *plane;
@@ -113,6 +114,7 @@ struct armada_crtc { #define drm_to_armada_crtc(c) container_of(c, struct armada_crtc, crtc)
void armada_drm_crtc_update_regs(struct armada_crtc *, struct armada_regs *); +void armada_drm_crtc_set_yuv_colorimetry(struct armada_crtc *dcrtc, u32 csc);
int armada_drm_plane_disable(struct drm_plane *plane, struct drm_modeset_acquire_ctx *ctx); diff --git a/drivers/gpu/drm/armada/armada_drm.h b/drivers/gpu/drm/armada/armada_drm.h index b064879ecdbd..45d5168d5748 100644 --- a/drivers/gpu/drm/armada/armada_drm.h +++ b/drivers/gpu/drm/armada/armada_drm.h @@ -60,6 +60,7 @@ struct armada_private { struct armada_crtc *dcrtc[2]; struct drm_mm linear; /* protected by linear_lock */ struct mutex linear_lock;
- struct drm_property *iturbt709_prop; struct drm_property *csc_yuv_prop; struct drm_property *csc_rgb_prop; struct drm_property *colorkey_prop;
diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 853f889e84f5..d7a9c79f0ada 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -28,6 +28,8 @@ struct armada_ovl_plane_properties { uint16_t contrast; uint16_t saturation; uint32_t colorkey_mode;
- uint32_t csc;
- bool csc_set;
};
struct armada_ovl_plane { @@ -252,6 +254,10 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (!dcrtc->plane) { dcrtc->plane = plane; armada_ovl_update_attr(&dplane->prop, dcrtc);
if (dplane->prop.csc_set) {armada_drm_crtc_set_yuv_colorimetry(dcrtc, dplane->prop.csc);dplane->prop.csc_set = false;}}
/* Queue it for update on the next interrupt if we are enabled */
@@ -332,11 +338,21 @@ static int armada_ovl_plane_set_property(struct drm_plane *plane, } else if (property == priv->saturation_prop) { dplane->prop.saturation = val; update_attr = true;
- } else if (property == priv->iturbt709_prop) {
dplane->prop.csc = val ? CFG_CSC_YUV_CCIR709 :CFG_CSC_YUV_CCIR601;dplane->prop.csc_set = true;
- if (update_attr && dplane->base.base.crtc)
armada_ovl_update_attr(&dplane->prop,drm_to_armada_crtc(dplane->base.base.crtc));
if (plane->crtc) {
struct armada_crtc *dcrtc = drm_to_armada_crtc(plane->crtc);if (update_attr)armada_ovl_update_attr(&dplane->prop, dcrtc);if (dplane->prop.csc_set) {armada_drm_crtc_set_yuv_colorimetry(dcrtc, dplane->prop.csc);dplane->prop.csc_set = false;}}
return 0;
} @@ -407,6 +423,8 @@ static int armada_overlay_create_properties(struct drm_device *dev) "contrast", 0, 0x7fff); priv->saturation_prop = drm_property_create_range(dev, 0, "saturation", 0, 0x7fff);
priv->iturbt709_prop = drm_property_create_bool(dev, 0,
"iturbt_709");if (!priv->colorkey_prop) return -ENOMEM;
@@ -475,6 +493,7 @@ int armada_overlay_plane_create(struct drm_device *dev, unsigned long crtcs) dplane->prop.contrast); drm_object_attach_property(mobj, priv->saturation_prop, dplane->prop.saturation);
drm_object_attach_property(mobj, priv->iturbt709_prop, false);
return 0;
}
2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Dec 11, 2017 at 09:54:02PM +0100, Daniel Vetter wrote:
On Fri, Dec 08, 2017 at 12:31:08PM +0000, Russell King wrote:
Add the defacto-standard "iturbt_709" property to the overlay plane to control the YUV to RGB colorspace conversion. This is mutually exclusive with the CSC_YUV CRTC property - the last property to be set determines the resulting colorspace conversion.
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk
Can you pls provide a link to the corresponding userspace code using this?
http://git.armlinux.org.uk/cgit/xf86-video-armada.git/commit/?h=unstable-dev...
On Fri, Dec 08, 2017 at 12:31:08PM +0000, Russell King wrote:
Add the defacto-standard "iturbt_709" property to the overlay plane to control the YUV to RGB colorspace conversion. This is mutually exclusive with the CSC_YUV CRTC property - the last property to be set determines the resulting colorspace conversion.
See https://patchwork.freedesktop.org/patch/158920/ by Jyri.
For the correspoding i915 implementation I cooked up see: https://patchwork.freedesktop.org/series/25505/
Signed-off-by: Russell King rmk+kernel@armlinux.org.uk
drivers/gpu/drm/armada/armada_crtc.c | 26 +++++++++++++++++++++++--- drivers/gpu/drm/armada/armada_crtc.h | 2 ++ drivers/gpu/drm/armada/armada_drm.h | 1 + drivers/gpu/drm/armada/armada_overlay.c | 25 ++++++++++++++++++++++--- 4 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 8b66377a4890..1bf632bb8855 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -535,12 +535,30 @@ static irqreturn_t armada_drm_irq(int irq, void *arg) return IRQ_NONE; }
+void armada_drm_crtc_set_yuv_colorimetry(struct armada_crtc *dcrtc, u32 csc) +{
- armada_updatel(csc, CFG_CSC_YUV_CCIR709,
dcrtc->base + LCD_SPU_IOPAD_CONTROL);- dcrtc->csc_yuv_ovl_mode = csc == CFG_CSC_YUV_CCIR709 ?
CSC_YUV_CCIR709 : CSC_YUV_CCIR601;+}
static uint32_t armada_drm_crtc_calculate_csc(struct armada_crtc *dcrtc) { struct drm_display_mode *adj = &dcrtc->crtc.mode;
- uint32_t val = 0;
- u32 val = 0;
- u8 csc_yuv_mode;
- /*
* If the iturbt_709 overlay plane property is used, it overrides* the CSC_YUV crtc property until the CSC_YUV property is set.*/- csc_yuv_mode = dcrtc->csc_yuv_ovl_mode;
- if (csc_yuv_mode == CSC_AUTO)
csc_yuv_mode = dcrtc->csc_yuv_mode;
- if (dcrtc->csc_yuv_mode == CSC_YUV_CCIR709)
- if (csc_yuv_mode == CSC_YUV_CCIR709) val |= CFG_CSC_YUV_CCIR709; if (dcrtc->csc_rgb_mode == CSC_RGB_STUDIO) val |= CFG_CSC_RGB_STUDIO;
@@ -555,7 +573,7 @@ static uint32_t armada_drm_crtc_calculate_csc(struct armada_crtc *dcrtc) if ((adj->hdisplay == 1280 && adj->vdisplay == 720 && !(adj->flags & DRM_MODE_FLAG_INTERLACE)) || (adj->hdisplay == 1920 && adj->vdisplay == 1080)) {
if (dcrtc->csc_yuv_mode == CSC_AUTO)
if (csc_yuv_mode == CSC_AUTO) val |= CFG_CSC_YUV_CCIR709;@@ -1094,6 +1112,7 @@ armada_drm_crtc_set_property(struct drm_crtc *crtc,
if (property == priv->csc_yuv_prop) { dcrtc->csc_yuv_mode = val;
dcrtc->csc_yuv_ovl_mode = CSC_AUTO;@@ -1396,6 +1415,7 @@ static int armada_drm_crtc_create(struct drm_device *drm, struct device *dev, dcrtc->num = drm->mode_config.num_crtc; dcrtc->clk = ERR_PTR(-EINVAL); dcrtc->csc_yuv_mode = CSC_AUTO;
- dcrtc->csc_yuv_ovl_mode = CSC_AUTO; dcrtc->csc_rgb_mode = CSC_AUTO; dcrtc->cfg_dumb_ctrl = DUMB24_RGB888_0; dcrtc->spu_iopad_ctrl = CFG_VSCALE_LN_EN | CFG_IOPAD_DUMB24;
diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index 445829b8877a..5d70b4af20b7 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -90,6 +90,7 @@ struct armada_crtc { bool interlaced; bool cursor_update; uint8_t csc_yuv_mode;
uint8_t csc_yuv_ovl_mode; uint8_t csc_rgb_mode;
struct drm_plane *plane;
@@ -113,6 +114,7 @@ struct armada_crtc { #define drm_to_armada_crtc(c) container_of(c, struct armada_crtc, crtc)
void armada_drm_crtc_update_regs(struct armada_crtc *, struct armada_regs *); +void armada_drm_crtc_set_yuv_colorimetry(struct armada_crtc *dcrtc, u32 csc);
int armada_drm_plane_disable(struct drm_plane *plane, struct drm_modeset_acquire_ctx *ctx); diff --git a/drivers/gpu/drm/armada/armada_drm.h b/drivers/gpu/drm/armada/armada_drm.h index b064879ecdbd..45d5168d5748 100644 --- a/drivers/gpu/drm/armada/armada_drm.h +++ b/drivers/gpu/drm/armada/armada_drm.h @@ -60,6 +60,7 @@ struct armada_private { struct armada_crtc *dcrtc[2]; struct drm_mm linear; /* protected by linear_lock */ struct mutex linear_lock;
- struct drm_property *iturbt709_prop; struct drm_property *csc_yuv_prop; struct drm_property *csc_rgb_prop; struct drm_property *colorkey_prop;
diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c index 853f889e84f5..d7a9c79f0ada 100644 --- a/drivers/gpu/drm/armada/armada_overlay.c +++ b/drivers/gpu/drm/armada/armada_overlay.c @@ -28,6 +28,8 @@ struct armada_ovl_plane_properties { uint16_t contrast; uint16_t saturation; uint32_t colorkey_mode;
- uint32_t csc;
- bool csc_set;
};
struct armada_ovl_plane { @@ -252,6 +254,10 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (!dcrtc->plane) { dcrtc->plane = plane; armada_ovl_update_attr(&dplane->prop, dcrtc);
if (dplane->prop.csc_set) {armada_drm_crtc_set_yuv_colorimetry(dcrtc, dplane->prop.csc);dplane->prop.csc_set = false;}}
/* Queue it for update on the next interrupt if we are enabled */
@@ -332,11 +338,21 @@ static int armada_ovl_plane_set_property(struct drm_plane *plane, } else if (property == priv->saturation_prop) { dplane->prop.saturation = val; update_attr = true;
- } else if (property == priv->iturbt709_prop) {
dplane->prop.csc = val ? CFG_CSC_YUV_CCIR709 :CFG_CSC_YUV_CCIR601;dplane->prop.csc_set = true;
- if (update_attr && dplane->base.base.crtc)
armada_ovl_update_attr(&dplane->prop,drm_to_armada_crtc(dplane->base.base.crtc));
if (plane->crtc) {
struct armada_crtc *dcrtc = drm_to_armada_crtc(plane->crtc);if (update_attr)armada_ovl_update_attr(&dplane->prop, dcrtc);if (dplane->prop.csc_set) {armada_drm_crtc_set_yuv_colorimetry(dcrtc, dplane->prop.csc);dplane->prop.csc_set = false;}}
return 0;
} @@ -407,6 +423,8 @@ static int armada_overlay_create_properties(struct drm_device *dev) "contrast", 0, 0x7fff); priv->saturation_prop = drm_property_create_range(dev, 0, "saturation", 0, 0x7fff);
priv->iturbt709_prop = drm_property_create_bool(dev, 0,
"iturbt_709");if (!priv->colorkey_prop) return -ENOMEM;
@@ -475,6 +493,7 @@ int armada_overlay_plane_create(struct drm_device *dev, unsigned long crtcs) dplane->prop.contrast); drm_object_attach_property(mobj, priv->saturation_prop, dplane->prop.saturation);
drm_object_attach_property(mobj, priv->iturbt709_prop, false);
return 0;
}
2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Russell,
On 8 December 2017 at 12:31, Russell King rmk+kernel@armlinux.org.uk wrote:
Add the defacto-standard "iturbt_709" property to the overlay plane to control the YUV to RGB colorspace conversion. This is mutually exclusive with the CSC_YUV CRTC property - the last property to be set determines the resulting colorspace conversion.
I haven't seen this in other drivers - is it a 'defacto standard'? I prefer VIlle's choice of an explicit 601/709 enum, since that's more easily expandable. I do worry about the interaction with the CTM properties, but I also don't have a good answer as to what that would look like. I also worry that implementing the 'last-property-set' semantics might be difficult to implement in an atomic driver.
struct armada_ovl_plane { @@ -252,6 +254,10 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (!dcrtc->plane) { dcrtc->plane = plane; armada_ovl_update_attr(&dplane->prop, dcrtc);
if (dplane->prop.csc_set) {armada_drm_crtc_set_yuv_colorimetry(dcrtc, dplane->prop.csc);dplane->prop.csc_set = false;} }
Just trying to understand this a little better: setting this property on a plane results in a CRTC change. This is only implemented for the overlay ('video') plane, but the primary ('graphics') plane also supports YUV formats. Does this mean that the two planes have to have the same configuration wrt 601/709 if both are YUV? That could again get painful to express.
Cheers, Daniel
On Wed, Dec 13, 2017 at 03:41:49PM +0000, Daniel Stone wrote:
Hi Russell,
On 8 December 2017 at 12:31, Russell King rmk+kernel@armlinux.org.uk wrote:
Add the defacto-standard "iturbt_709" property to the overlay plane to control the YUV to RGB colorspace conversion. This is mutually exclusive with the CSC_YUV CRTC property - the last property to be set determines the resulting colorspace conversion.
I haven't seen this in other drivers - is it a 'defacto standard'?
As far as I know, nothing for this is standardised - when I implemented this method, I did survey the drivers, and decided it was best to use what existing implementations were already using. I wrote this patch back in February, so memory is now hazy, but iirc, it came from:
drivers/gpu/drm/nouveau/dispnv04/overlay.c
I prefer VIlle's choice of an explicit 601/709 enum, since that's more easily expandable. I do worry about the interaction with the CTM properties, but I also don't have a good answer as to what that would look like. I also worry that implementing the 'last-property-set' semantics might be difficult to implement in an atomic driver.
Nothing really made use of the CRTC property, so its unlikely that there'll be a conflict. I suspect I could delete the CRTC property when eventually switching to atomic modeset.
struct armada_ovl_plane { @@ -252,6 +254,10 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, if (!dcrtc->plane) { dcrtc->plane = plane; armada_ovl_update_attr(&dplane->prop, dcrtc);
if (dplane->prop.csc_set) {armada_drm_crtc_set_yuv_colorimetry(dcrtc, dplane->prop.csc);dplane->prop.csc_set = false;} }Just trying to understand this a little better: setting this property on a plane results in a CRTC change. This is only implemented for the overlay ('video') plane, but the primary ('graphics') plane also supports YUV formats. Does this mean that the two planes have to have the same configuration wrt 601/709 if both are YUV? That could again get painful to express.
The same is true of the saturation, brightness, contrast etc values. There is only one set of YUV to RGB conversion in the hardware block and its settings will be used for both.
However, practically I don't think YUV is ever used for the primary plane, so I could just drop those from the list of primary plane formats.
On Wed, Dec 13, 2017 at 10:41 AM, Daniel Stone daniel@fooishbar.org wrote:
Hi Russell,
On 8 December 2017 at 12:31, Russell King rmk+kernel@armlinux.org.uk wrote:
Add the defacto-standard "iturbt_709" property to the overlay plane to control the YUV to RGB colorspace conversion. This is mutually exclusive with the CSC_YUV CRTC property - the last property to be set determines the resulting colorspace conversion.
I haven't seen this in other drivers - is it a 'defacto standard'? I
xf86-video-nv supported it, and I added it to nouveau as well when I ported YUV plane support. Some video players use the Xv property when available.
https://cgit.freedesktop.org/xorg/driver/xf86-video-nv/tree/src/nv_video.c#n... https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
Cheers,
-ilia
On Wed, Dec 13, 2017 at 11:12:18AM -0500, Ilia Mirkin wrote:
On Wed, Dec 13, 2017 at 10:41 AM, Daniel Stone daniel@fooishbar.org wrote:
Hi Russell,
On 8 December 2017 at 12:31, Russell King rmk+kernel@armlinux.org.uk wrote:
Add the defacto-standard "iturbt_709" property to the overlay plane to control the YUV to RGB colorspace conversion. This is mutually exclusive with the CSC_YUV CRTC property - the last property to be set determines the resulting colorspace conversion.
I haven't seen this in other drivers - is it a 'defacto standard'? I
xf86-video-nv supported it, and I added it to nouveau as well when I ported YUV plane support. Some video players use the Xv property when available.
https://cgit.freedesktop.org/xorg/driver/xf86-video-nv/tree/src/nv_video.c#n... https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
{XvSettable | XvGettable, 0, 1, "XV_ITURBT_709"}
Who came up with that and when? XV_COLORSPACE was the one semi-standard I know of.
Cheers,
-ilia _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Dec 13, 2017 at 06:22:14PM +0200, Ville Syrjälä wrote:
On Wed, Dec 13, 2017 at 11:12:18AM -0500, Ilia Mirkin wrote:
On Wed, Dec 13, 2017 at 10:41 AM, Daniel Stone daniel@fooishbar.org wrote:
Hi Russell,
On 8 December 2017 at 12:31, Russell King rmk+kernel@armlinux.org.uk wrote:
Add the defacto-standard "iturbt_709" property to the overlay plane to control the YUV to RGB colorspace conversion. This is mutually exclusive with the CSC_YUV CRTC property - the last property to be set determines the resulting colorspace conversion.
I haven't seen this in other drivers - is it a 'defacto standard'? I
xf86-video-nv supported it, and I added it to nouveau as well when I ported YUV plane support. Some video players use the Xv property when available.
https://cgit.freedesktop.org/xorg/driver/xf86-video-nv/tree/src/nv_video.c#n... https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
{XvSettable | XvGettable, 0, 1, "XV_ITURBT_709"}
Who came up with that and when? XV_COLORSPACE was the one semi-standard I know of.
I've no idea, and I was hoping that someone else would know - my use of it comes from research into what will make userspace work, not what standards may say.
XV_ITURBT_709 is already in-use in distro standard userspace programs:
# grep XV_ITURBT_709 /usr/lib/vlc/plugins/ -r Binary file /usr/lib/vlc/plugins/video_output/libxcb_xv_plugin.so matches # grep XV_ITURBT_709 /usr/lib/gstreamer-1.0/ -r Binary file /usr/lib/gstreamer-1.0/libgstxvimagesink.so matches
but not XV_COLORSPACE:
# grep XV_COLORSPACE /usr/lib/vlc/plugins/ -r # grep XV_COLORSPACE /usr/lib/gstreamer-1.0/ -r
So while XV_COLORSPACE may be some kind of standard, it seems that userspace has decided otherwise to go with a different name for this control.
Anyway, I'll drop this patch and send a pull request for the remainder to David.
On Mon, Jan 01, 2018 at 12:17:35PM +0000, Russell King - ARM Linux wrote:
On Wed, Dec 13, 2017 at 06:22:14PM +0200, Ville Syrjälä wrote:
On Wed, Dec 13, 2017 at 11:12:18AM -0500, Ilia Mirkin wrote:
On Wed, Dec 13, 2017 at 10:41 AM, Daniel Stone daniel@fooishbar.org wrote:
Hi Russell,
On 8 December 2017 at 12:31, Russell King rmk+kernel@armlinux.org.uk wrote:
Add the defacto-standard "iturbt_709" property to the overlay plane to control the YUV to RGB colorspace conversion. This is mutually exclusive with the CSC_YUV CRTC property - the last property to be set determines the resulting colorspace conversion.
I haven't seen this in other drivers - is it a 'defacto standard'? I
xf86-video-nv supported it, and I added it to nouveau as well when I ported YUV plane support. Some video players use the Xv property when available.
https://cgit.freedesktop.org/xorg/driver/xf86-video-nv/tree/src/nv_video.c#n... https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
{XvSettable | XvGettable, 0, 1, "XV_ITURBT_709"}
Who came up with that and when? XV_COLORSPACE was the one semi-standard I know of.
I've no idea, and I was hoping that someone else would know - my use of it comes from research into what will make userspace work, not what standards may say.
XV_ITURBT_709 is already in-use in distro standard userspace programs:
# grep XV_ITURBT_709 /usr/lib/vlc/plugins/ -r Binary file /usr/lib/vlc/plugins/video_output/libxcb_xv_plugin.so matches # grep XV_ITURBT_709 /usr/lib/gstreamer-1.0/ -r Binary file /usr/lib/gstreamer-1.0/libgstxvimagesink.so matches
but not XV_COLORSPACE:
# grep XV_COLORSPACE /usr/lib/vlc/plugins/ -r # grep XV_COLORSPACE /usr/lib/gstreamer-1.0/ -r
So while XV_COLORSPACE may be some kind of standard, it seems that userspace has decided otherwise to go with a different name for this control.
Re-opening this discussion, since the above point was never replied to.
I can find no video players that make use of the "XV_COLORSPACE" property, but two that make use of the "XV_ITURBT_709" property.
It was added to gstreamer in 2014: https://github.com/GStreamer/gst-plugins-base/commit/d99e270fc83278c309ec7ca...
and is present in VLC since 2016: https://github.com/videolan/vlc/commit/8172a5470964550a1e5d6e2b7082650f932e6...
We seem to have the situation where some Xv backends implement "XV_COLORSPACE", others "XV_ITURBT_709" but players implement only "XV_ITURBT_709".
A DDX /could/ consider implementing both, but there is no way to notify Xv event listeners from a DDX's Xv backend that "the other" property has been changed - XvdiSendPortNotify() needs an XvPortPtr but the DDX has no access to that due to the xf86 layer on top hiding that, and there's nothing at xf86 level to allow that.
This doesn't seem to be a very productive situation, and certainly not useful for the user.
I think that the conclusion I'd come to given this is that 99.9% of people don't care about correct Xv colorimetry.
On Fri, Jul 20, 2018 at 12:39:30PM +0100, Russell King - ARM Linux wrote:
On Mon, Jan 01, 2018 at 12:17:35PM +0000, Russell King - ARM Linux wrote:
On Wed, Dec 13, 2017 at 06:22:14PM +0200, Ville Syrjälä wrote:
On Wed, Dec 13, 2017 at 11:12:18AM -0500, Ilia Mirkin wrote:
On Wed, Dec 13, 2017 at 10:41 AM, Daniel Stone daniel@fooishbar.org wrote:
Hi Russell,
On 8 December 2017 at 12:31, Russell King rmk+kernel@armlinux.org.uk wrote:
Add the defacto-standard "iturbt_709" property to the overlay plane to control the YUV to RGB colorspace conversion. This is mutually exclusive with the CSC_YUV CRTC property - the last property to be set determines the resulting colorspace conversion.
I haven't seen this in other drivers - is it a 'defacto standard'? I
xf86-video-nv supported it, and I added it to nouveau as well when I ported YUV plane support. Some video players use the Xv property when available.
https://cgit.freedesktop.org/xorg/driver/xf86-video-nv/tree/src/nv_video.c#n... https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
{XvSettable | XvGettable, 0, 1, "XV_ITURBT_709"}
Who came up with that and when? XV_COLORSPACE was the one semi-standard I know of.
I've no idea, and I was hoping that someone else would know - my use of it comes from research into what will make userspace work, not what standards may say.
XV_ITURBT_709 is already in-use in distro standard userspace programs:
# grep XV_ITURBT_709 /usr/lib/vlc/plugins/ -r Binary file /usr/lib/vlc/plugins/video_output/libxcb_xv_plugin.so matches # grep XV_ITURBT_709 /usr/lib/gstreamer-1.0/ -r Binary file /usr/lib/gstreamer-1.0/libgstxvimagesink.so matches
but not XV_COLORSPACE:
# grep XV_COLORSPACE /usr/lib/vlc/plugins/ -r # grep XV_COLORSPACE /usr/lib/gstreamer-1.0/ -r
So while XV_COLORSPACE may be some kind of standard, it seems that userspace has decided otherwise to go with a different name for this control.
Re-opening this discussion, since the above point was never replied to.
I can find no video players that make use of the "XV_COLORSPACE" property, but two that make use of the "XV_ITURBT_709" property.
mpv supports both, and xvattr is of course one way to change it but requires manual user intervention.
I have a patch locally for gst xvimagesink to use XV_COLORSPACE as well. I wrote it when I posted the proposed XV_COLOR_RANGE attribute (and I wrote patches for that one too, for gst and mpv). Didn't bother posting any of these until I got some feedback on XV_COLOR_RANGE. No feedback was received though so I suppose everyone is OK with it.
It was added to gstreamer in 2014: https://github.com/GStreamer/gst-plugins-base/commit/d99e270fc83278c309ec7ca...
and is present in VLC since 2016: https://github.com/videolan/vlc/commit/8172a5470964550a1e5d6e2b7082650f932e6...
We seem to have the situation where some Xv backends implement "XV_COLORSPACE", others "XV_ITURBT_709" but players implement only "XV_ITURBT_709".
A DDX /could/ consider implementing both, but there is no way to notify Xv event listeners from a DDX's Xv backend that "the other" property has been changed - XvdiSendPortNotify() needs an XvPortPtr but the DDX has no access to that due to the xf86 layer on top hiding that, and there's nothing at xf86 level to allow that.
This doesn't seem to be a very productive situation, and certainly not useful for the user.
I think that the conclusion I'd come to given this is that 99.9% of people don't care about correct Xv colorimetry.
Also Xv usage is probably on the decline seeing as it's not used by browsers which is probably how many people watch videos these days.
dri-devel@lists.freedesktop.org
-
Daniel Stone -
Daniel Vetter -
Ilia Mirkin -
Russell King -
Russell King -
Russell King - ARM Linux -
Ville Syrjälä