[PATCH v2 0/5] Armada DRM fixes

[PATCH v2 0/5] Armada DRM fixes

[Russell King - ARM Linux]

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(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/5] drm/armada: fix leak of crtc structure

[Russell King]

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;
 }
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/5] drm/armada: fix SRAM powerdown

[Russell King]

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;
 	}
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/5] drm/armada: fix UV swap code

[Russell King]

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],
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/5] drm/armada: improve efficiency of armada_drm_plane_calc_addrs()

[Russell King]

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;
 }
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/5] drm/armada: fix YUV planar format framebuffer offsets

[Russell King]

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++)
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 00/25] Armada DRM development for 4.16

[Russell King - ARM Linux]

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(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 01/25] drm/armada: remove armada_drm_plane_work_cancel() return value

[Russell King]

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);
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/25] drm/armada: add a common frame work allocator

[Russell King]

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;
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/25] drm/armada: store plane in armada_plane_work

[Russell King]

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,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/25] drm/armada: add work cancel callback

[Russell King]

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;
 };
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/25] drm/armada: wait and cancel any pending frame work at disable

[Russell King]

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;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/25] drm/armada: allow the primary plane to be disabled

[Russell King]

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,
 };
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/25] drm/armada: clean up armada_drm_crtc_plane_disable()

[Russell King]

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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/25] drm/armada: clear plane enable bit when disabling

[Russell King]

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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/25] drm/armada: move overlay plane work out from under spinlock

[Russell King]

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);
 }
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/25] drm/armada: move fb retirement into armada_plane_work

[Russell King]

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;
 }
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/25] drm/armada: move event sending into armada_plane_work

[Russell King]

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 {
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 12/25] drm/armada: move regs into armada_plane_work

[Russell King]

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,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 13/25] drm/armada: move writes of LCD_SPU_SRAM_PARA1 under lock

[Russell King]

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)
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 14/25] drm/armada: only enable HSMOOTH if scaling horizontally

[Russell King]

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
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 15/25] drm/armada: use drm_plane_helper_check_state()

[Russell King]

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,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 16/25] drm/armada: allow armada_drm_plane_work_queue() to silently fail

[Russell King]

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;
 }
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 17/25] drm/armada: avoid work allocation

[Russell King]

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,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 18/25] drm/armada: disable planes at next blanking period

[Russell King]

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,
 };
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 19/25] drm/armada: re-organise overlay register update generation

[Russell King]

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 */
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 20/25] drm/armada: move overlay plane register update generation

[Russell King]

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;
 }
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 21/25] drm/armada: wait for previous work when moving overlay window

[Russell King]

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);
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 22/25] drm/armada: extract register generation from armada_drm_primary_set()

[Russell King]

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);
 }
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 23/25] drm/armada: implement primary plane update

[Russell King]

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,
 };
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 24/25] drm/armada: expand overlay trace entry

[Russell King]

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,
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Russell King]

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;
 }
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Daniel Vetter]

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;
>  		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;
>  }
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Russell King - ARM Linux]

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-devel&id=7282633efd1c702a770b046505d7330fed028de0

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Ville Syrjälä]

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;
>  		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;
>  }
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Daniel Stone]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Russell King - ARM Linux]

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.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Ilia Mirkin]

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#n128
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/dispnv04/overlay.c?h=v4.15-rc3#n316

Cheers,

  -ilia
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Ville Syrjälä]

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#n128
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/dispnv04/overlay.c?h=v4.15-rc3#n316

   {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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Russell King - ARM Linux]

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#n128
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/dispnv04/overlay.c?h=v4.15-rc3#n316
> 
>    {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.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Russell King - ARM Linux]

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#n128
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/dispnv04/overlay.c?h=v4.15-rc3#n316
> > 
> >    {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/d99e270fc83278c309ec7cad20d75181d90b8722

and is present in VLC since 2016:
https://github.com/videolan/vlc/commit/8172a5470964550a1e5d6e2b7082650f932e6ce6

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.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 25/25] drm/armada: add iturbt_709 plane property to control YUV colorspace

[Ville Syrjälä]

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#n128
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/dispnv04/overlay.c?h=v4.15-rc3#n316
> > > 
> > >    {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/d99e270fc83278c309ec7cad20d75181d90b8722
> 
> and is present in VLC since 2016:
> https://github.com/videolan/vlc/commit/8172a5470964550a1e5d6e2b7082650f932e6ce6
> 
> 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.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel