Some corrections and improvement for Tegra DRM

HW reset isn't actually broken on Tegra20, but there is a dependency on first display controller to be taken out of reset for the second to be enabled successfully.

Signed-off-by Dmitry Osipenko <[email protected]>

---

Change log:

v2: Got rid of global variable and now use driver_find_device() instead.

 drivers/gpu/drm/tegra/dc.c | 80 +++++++++++++++++++++++++++++-----------------
 drivers/gpu/drm/tegra/dc.h |  2 +-
 2 files changed, 51 insertions(+), 31 deletions(-)

Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with both modesetting and opentegra drivers. On older Tegra's each plane has a blending configuration which should be used to enable / disable alpha blending and right now the blending configs are hardcoded to disabled alpha blending. In order to support alpha formats properly, planes blending configuration must be adjusted, until then the alpha formats are equal to non-alpha.

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c    | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h    |  1 +
 drivers/gpu/drm/tegra/fb.c    | 13 -------------
 drivers/gpu/drm/tegra/hub.c   |  3 ++-
 drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/tegra/plane.h |  2 +-
 6 files changed, 39 insertions(+), 31 deletions(-)

Quoted 12 lines · Dmitry Osipenko

Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with both modesetting and opentegra drivers. On older Tegra's each plane has a blending configuration which should be used to enable / disable alpha blending and right now the blending configs are hardcoded to disabled alpha blending. In order to support alpha formats properly, planes blending configuration must be adjusted, until then the alpha formats are equal to non-alpha.

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Dmitry Osipenko <[email protected]>

Quoted 8 lines

---
 drivers/gpu/drm/tegra/dc.c    | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h    |  1 +
 drivers/gpu/drm/tegra/fb.c    | 13 -------------
 drivers/gpu/drm/tegra/hub.c   |  3 ++-
 drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/tegra/plane.h |  2 +-
 6 files changed, 39 insertions(+), 31 deletions(-)

This kept bugging me, so I spent some time looking at the blending programming. I came up with the attached patch which seems to work for all scenarios and is fairly similar to your patch. It has the added benefit that we can keep support for more formats.

Any comments?

Thierry
--- >8 ---
From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
From: Thierry Reding <[email protected]>
Date: Wed, 20 Dec 2017 09:39:14 +0100
Subject: [PATCH] drm/tegra: dc: Implement legacy blending

This implements alpha blending on legacy display controllers (Tegra20, Tegra30 and Tegra114). While it's theoretically possible to support the zpos property to enable userspace to specify the Z-order of each plane individually, this is not currently supported and the same fixed Z- order as previously defined is used.

Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since the opaque formats are now supported.

Reported-by Dmitry Osipenko <[email protected]>

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c    | 74 ++++++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/tegra/dc.h    | 13 ++++++++
 drivers/gpu/drm/tegra/fb.c    | 12 -------
 drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++
 drivers/gpu/drm/tegra/plane.h |  3 ++
 5 files changed, 116 insertions(+), 27 deletions(-)

drivers/gpu/drm/tegra/dc.c +4 -1

index bc65c314e00f..07c687d7f615 100644
@@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in)
 	return dfixed_frac(inf);
 }
 
-static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
+static void
+tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
+				  const struct tegra_dc_window *window)
 {
+	u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |

drivers/gpu/drm/tegra/dc.c +45 -14

+
 	/*
 	 * Disable blending and assume Window A is the bottom-most window,
 	 * Window C is the top-most window and Window B is in the middle.
 	 */
-	tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_NOKEY);
-	tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_1WIN);
+	tegra_plane_writel(plane, blendnokey, DC_WIN_BLEND_NOKEY);
+	tegra_plane_writel(plane, foreground, DC_WIN_BLEND_1WIN);
 
 	switch (plane->index) {
 	case 0:
-		tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_2WIN_X);
-		tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_2WIN_Y);
-		tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_3WIN_XY);
+		tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_X);
+		tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_Y);
+		tegra_plane_writel(plane, background, DC_WIN_BLEND_3WIN_XY);
 		break;
 
 	case 1:
-		tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_2WIN_X);
-		tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_2WIN_Y);
-		tegra_plane_writel(plane, 0x000000, DC_WIN_BLEND_3WIN_XY);
+		tegra_plane_writel(plane, foreground, DC_WIN_BLEND_2WIN_X);
+		tegra_plane_writel(plane, background, DC_WIN_BLEND_2WIN_Y);
+		tegra_plane_writel(plane, background, DC_WIN_BLEND_3WIN_XY);
 		break;
 
 	case 2:
-		tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_2WIN_X);
-		tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_2WIN_Y);
-		tegra_plane_writel(plane, 0xffff00, DC_WIN_BLEND_3WIN_XY);
+		tegra_plane_writel(plane, foreground, DC_WIN_BLEND_2WIN_X);
+		tegra_plane_writel(plane, foreground, DC_WIN_BLEND_2WIN_Y);
+		tegra_plane_writel(plane, foreground, DC_WIN_BLEND_3WIN_XY);
 		break;
 	}
 }

@@ -359,7 +373,7 @@ static void tegra_dc_setup_window(struct tegra_plane *plane,
 	if (dc->soc->supports_blending) {
 		tegra_plane_setup_blending(plane, window);
 	} else {
-		tegra_plane_setup_blending_legacy(plane);
+		tegra_plane_setup_blending_legacy(plane, window);
 	}
 }
 

@@ -370,6 +384,11 @@ static const u32 tegra20_primary_formats[] = {
 	DRM_FORMAT_RGBA5551,
 	DRM_FORMAT_ABGR8888,
 	DRM_FORMAT_ARGB8888,
+	/* non-native formats */
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGBX5551,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_XRGB8888,
 };
 
 static const u32 tegra114_primary_formats[] = {

@@ -426,18 +445,37 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
 	struct tegra_bo_tiling *tiling = &plane_state->tiling;
 	struct tegra_plane *tegra = to_tegra_plane(plane);
 	struct tegra_dc *dc = to_tegra_dc(state->crtc);
+	unsigned int format;
 	int err;
 
 	/* no need for further checks if the plane is being disabled */
 	if (!state->crtc)
 		return 0;
 
-	err = tegra_plane_format(state->fb->format->format,
-				 &plane_state->format,
+	err = tegra_plane_format(state->fb->format->format, &format,
 				 &plane_state->swap);
 	if (err < 0)
 		return err;
 
+	/*
+	 * Tegra20 and Tegra30 are special cases here because they support
+	 * only variants of specific formats with an alpha component, but not
+	 * the corresponding opaque formats. However, the opaque formats can
+	 * be emulated by disabling alpha blending for the plane.
+	 */
+	if (tegra_plane_format_is_opaque(format) &&
+	    !dc->soc->supports_blending) {
+		err = tegra_plane_format_get_alpha(format, &format);
+		if (err < 0)
+			return err;
+
+		plane_state->alpha = false;
+	} else {
+		plane_state->alpha = true;
+	}
+
+	plane_state->format = format;
+
 	err = tegra_fb_get_tiling(state->fb, tiling);
 	if (err < 0)
 		return err;

@@ -514,6 +552,7 @@ static void tegra_plane_atomic_update(struct drm_plane *plane,
 	window.zpos = plane->state->normalized_zpos;
 	window.tiling = state->tiling;
 	window.format = state->format;
+	window.alpha = state->alpha;
 	window.swap = state->swap;
 
 	for (i = 0; i < fb->format->num_planes; i++) {

@@ -768,6 +807,11 @@ static const u32 tegra20_overlay_formats[] = {
 	DRM_FORMAT_RGBA5551,
 	DRM_FORMAT_ABGR8888,
 	DRM_FORMAT_ARGB8888,
+	/* non-native formats */
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGBX5551,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_XRGB8888,
 	/* planar formats */
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_YUYV,

drivers/gpu/drm/tegra/dc.h +13 -0

index 0c887b53cb10..03b04a7e2814 100644
@@ -151,6 +151,7 @@ struct tegra_dc_window {
 
 	struct tegra_bo_tiling tiling;
 	u32 format;
+	bool alpha;
 	u32 swap;
 };
 

@@ -667,8 +668,20 @@ int tegra_dc_rgb_exit(struct tegra_dc *dc);
 #define  SCALER_COEFF_DATA(x) (((x) & 0x3ff) << 0)
 
 #define DC_WIN_BLEND_NOKEY			0x70f
+#define  BLEND_WEIGHT1(x) (((x) & 0xff) << 16)
+#define  BLEND_WEIGHT0(x) (((x) & 0xff) <<  8)
+
 #define DC_WIN_BLEND_1WIN			0x710
+#define  BLEND_CONTROL_FIX    (0 << 2)
+#define  BLEND_CONTROL_ALPHA  (1 << 2)
+#define  BLEND_COLOR_KEY_NONE (0 << 0)
+#define  BLEND_COLOR_KEY_0    (1 << 0)
+#define  BLEND_COLOR_KEY_1    (2 << 0)
+#define  BLEND_COLOR_KEY_BOTH (3 << 0)
+
 #define DC_WIN_BLEND_2WIN_X			0x711
+#define  BLEND_CONTROL_DEPENDENT (2 << 2)
+
 #define DC_WIN_BLEND_2WIN_Y			0x712
 #define DC_WIN_BLEND_3WIN_XY			0x713
 

drivers/gpu/drm/tegra/fb.c +0 -12

index 5f6289c4e56a..001cb77e2f59 100644
@@ -254,18 +254,6 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
 	cmd.pitches[0] = round_up(sizes->surface_width * bytes_per_pixel,
 				  tegra->pitch_align);
 
-	/*
-	 * Early generations of Tegra (Tegra20 and Tegra30) do not support any
-	 * of the X* or *X formats, only their A* or *A equivalents. Force the
-	 * legacy framebuffer format to include an alpha component so that the
-	 * framebuffer emulation can be supported on all generations.
-	 */
-	if (sizes->surface_bpp == 32 && sizes->surface_depth == 24)
-		sizes->surface_depth = 32;
-
-	if (sizes->surface_bpp == 16 && sizes->surface_depth == 15)
-		sizes->surface_depth = 16;
-
 	cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
 						     sizes->surface_depth);
 

drivers/gpu/drm/tegra/plane.c +41 -0

index 215051579b3a..dfe991e3d48d 100644
@@ -51,6 +51,7 @@ tegra_plane_atomic_duplicate_state(struct drm_plane *plane)
 	__drm_atomic_helper_plane_duplicate_state(plane, &copy->base);
 	copy->tiling = state->tiling;
 	copy->format = state->format;
+	copy->alpha = state->alpha;
 	copy->swap = state->swap;
 
 	return &copy->base;

@@ -256,6 +257,46 @@ bool tegra_plane_format_is_yuv(unsigned int format, bool *planar)
 	return false;
 }
 
+/*
+ * This is applicable to Tegra20 and Tegra30 only where the opaque formats can
+ * be emulated using the alpha formats and blending disabled.
+ */
+bool tegra_plane_format_is_opaque(unsigned int format)
+{
+	switch (format) {
+	case WIN_COLOR_DEPTH_B5G5R5X1:
+	case WIN_COLOR_DEPTH_X1B5G5R5:
+	case WIN_COLOR_DEPTH_R8G8B8X8:
+	case WIN_COLOR_DEPTH_B8G8R8X8:
+		return true;
+	}
+
+	return false;
+}
+
+int tegra_plane_format_get_alpha(unsigned int opaque, unsigned int *alpha)
+{
+	switch (opaque) {
+	case WIN_COLOR_DEPTH_B5G5R5X1:
+		*alpha = WIN_COLOR_DEPTH_B5G5R5A;
+		return 0;
+
+	case WIN_COLOR_DEPTH_X1B5G5R5:
+		*alpha = WIN_COLOR_DEPTH_AB5G5R5;
+		return 0;
+
+	case WIN_COLOR_DEPTH_R8G8B8X8:
+		*alpha = WIN_COLOR_DEPTH_R8G8B8A8;
+		return 0;
+
+	case WIN_COLOR_DEPTH_B8G8R8X8:
+		*alpha = WIN_COLOR_DEPTH_B8G8R8A8;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 void tegra_plane_get(struct tegra_plane *plane)
 {
 	mutex_lock(&plane->lock);

drivers/gpu/drm/tegra/plane.h +3 -0

index 429a086ffe88..06d8b0005fe7 100644
@@ -48,6 +48,7 @@ struct tegra_plane_state {
 
 	struct tegra_bo_tiling tiling;
 	u32 format;
+	bool alpha;
 	u32 swap;
 };
 

@@ -69,6 +70,8 @@ int tegra_plane_state_add(struct tegra_plane *plane,
 
 int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap);
 bool tegra_plane_format_is_yuv(unsigned int format, bool *planar);
+bool tegra_plane_format_is_opaque(unsigned int format);
+int tegra_plane_format_get_alpha(unsigned int opaque, unsigned int *alpha);
 
 void tegra_plane_get(struct tegra_plane *plane);
 void tegra_plane_put(struct tegra_plane *plane);

Quoted 36 lines · Thierry Reding

Quoted 12 lines · Dmitry Osipenko

Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with both modesetting and opentegra drivers. On older Tegra's each plane has a blending configuration which should be used to enable / disable alpha blending and right now the blending configs are hardcoded to disabled alpha blending. In order to support alpha formats properly, planes blending configuration must be adjusted, until then the alpha formats are equal to non-alpha.

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Dmitry Osipenko <[email protected]>

Quoted 8 lines

---
 drivers/gpu/drm/tegra/dc.c    | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h    |  1 +
 drivers/gpu/drm/tegra/fb.c    | 13 -------------
 drivers/gpu/drm/tegra/hub.c   |  3 ++-
 drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/tegra/plane.h |  2 +-
 6 files changed, 39 insertions(+), 31 deletions(-)

This kept bugging me, so I spent some time looking at the blending programming. I came up with the attached patch which seems to work for all scenarios and is fairly similar to your patch. It has the added benefit that we can keep support for more formats.

Any comments?

Thierry
--- >8 ---
From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
From: Thierry Reding <[email protected]>
Date: Wed, 20 Dec 2017 09:39:14 +0100
Subject: [PATCH] drm/tegra: dc: Implement legacy blending

Quoted 5 lines

This implements alpha blending on legacy display controllers (Tegra20, Tegra30 and Tegra114). While it's theoretically possible to support the zpos property to enable userspace to specify the Z-order of each plane individually, this is not currently supported and the same fixed Z- order as previously defined is used.

Perhaps one variant of implementing zpos could be by making overlays 'virtual', so each virtual overlay will be backed by the real HW plane and we could swap the HW planes of the virtual overlays, emulating zpos.

Quoted 34 lines

Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since the opaque formats are now supported.

Reported-by Dmitry Osipenko <[email protected]>

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c    | 74 ++++++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/tegra/dc.h    | 13 ++++++++
 drivers/gpu/drm/tegra/fb.c    | 12 -------
 drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++
 drivers/gpu/drm/tegra/plane.h |  3 ++
 5 files changed, 116 insertions(+), 27 deletions(-)

drivers/gpu/drm/tegra/dc.c +10 -1

index bc65c314e00f..07c687d7f615 100644
@@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in)
 	return dfixed_frac(inf);
 }
 
-static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
+static void
+tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
+				  const struct tegra_dc_window *window)
 {
+	u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
+			 BLEND_COLOR_KEY_NONE;
+	u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
+			 BLEND_COLOR_KEY_NONE;
+	u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
+
+	/* enable alpha blending if window->alpha */

drivers/gpu/drm/tegra/dc.c static inline u32 compute_initial_dda(unsigned int in) · line 185

Quoted 4 lines

drivers/gpu/drm/tegra/dc.c +4 -0

+	if (window->alpha) {
+		background |= BLEND_CONTROL_DEPENDENT;
+		foreground |= BLEND_CONTROL_ALPHA;
+	}

I think dependent weight means that window doesn't have alpha transparency. So we should set the dependent_weight mode for opaque formats and alpha_weight for formats with alpha channel.

If the above is correct, then I'm suggesting to not expose alpha formats, we should properly test all combinations of blending of all the windows. In one case you could apply my patch for now and then me/you/we could work on a proper legacy blending implementation based on your patch. In the other case I could take your patch into v4 (cursor patch would have to be rabased in that case) and we will correct blending sometime later. I don't mind either case, up to you to decide.

Is there any ready-made testsuite for the DRM planes blending? Or you have made a test by yourself? In the latter case, could you please share the code so that I could test it too without burden of writing testcases from scratch?

Quoted 81 lines · Dmitry Osipenko

Quoted 36 lines · Thierry Reding

Quoted 12 lines · Dmitry Osipenko

Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with both modesetting and opentegra drivers. On older Tegra's each plane has a blending configuration which should be used to enable / disable alpha blending and right now the blending configs are hardcoded to disabled alpha blending. In order to support alpha formats properly, planes blending configuration must be adjusted, until then the alpha formats are equal to non-alpha.

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Dmitry Osipenko <[email protected]>

Quoted 8 lines

---
 drivers/gpu/drm/tegra/dc.c    | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h    |  1 +
 drivers/gpu/drm/tegra/fb.c    | 13 -------------
 drivers/gpu/drm/tegra/hub.c   |  3 ++-
 drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/tegra/plane.h |  2 +-
 6 files changed, 39 insertions(+), 31 deletions(-)

This kept bugging me, so I spent some time looking at the blending programming. I came up with the attached patch which seems to work for all scenarios and is fairly similar to your patch. It has the added benefit that we can keep support for more formats.

Any comments?

Thierry
--- >8 ---
From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
From: Thierry Reding <[email protected]>
Date: Wed, 20 Dec 2017 09:39:14 +0100
Subject: [PATCH] drm/tegra: dc: Implement legacy blending

Quoted 5 lines

This implements alpha blending on legacy display controllers (Tegra20, Tegra30 and Tegra114). While it's theoretically possible to support the zpos property to enable userspace to specify the Z-order of each plane individually, this is not currently supported and the same fixed Z- order as previously defined is used.

Perhaps one variant of implementing zpos could be by making overlays 'virtual', so each virtual overlay will be backed by the real HW plane and we could swap the HW planes of the virtual overlays, emulating zpos.

Quoted 34 lines

Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since the opaque formats are now supported.

Reported-by Dmitry Osipenko <[email protected]>

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c    | 74 ++++++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/tegra/dc.h    | 13 ++++++++
 drivers/gpu/drm/tegra/fb.c    | 12 -------
 drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++
 drivers/gpu/drm/tegra/plane.h |  3 ++
 5 files changed, 116 insertions(+), 27 deletions(-)

drivers/gpu/drm/tegra/dc.c +10 -1

index bc65c314e00f..07c687d7f615 100644
@@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in)
 	return dfixed_frac(inf);
 }
 
-static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
+static void
+tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
+				  const struct tegra_dc_window *window)
 {
+	u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
+			 BLEND_COLOR_KEY_NONE;
+	u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
+			 BLEND_COLOR_KEY_NONE;
+	u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
+
+	/* enable alpha blending if window->alpha */

Quoted 8 lines

drivers/gpu/drm/tegra/dc.c static inline u32 compute_initial_dda(unsigned int in) · line 185

Quoted 4 lines

drivers/gpu/drm/tegra/dc.c +4 -0

+	if (window->alpha) {
+		background |= BLEND_CONTROL_DEPENDENT;
+		foreground |= BLEND_CONTROL_ALPHA;
+	}

I think dependent weight means that window doesn't have alpha transparency. So we should set the dependent_weight mode for opaque formats and alpha_weight for formats with alpha channel.

According to the TRM, dependent weight means that its weight will be 1 - the sum of the other overlapping windows. And it certainly seems to work that way in my tests (see below).

Quoted 7 lines

If the above is correct, then I'm suggesting to not expose alpha formats, we should properly test all combinations of blending of all the windows. In one case you could apply my patch for now and then me/you/we could work on a proper legacy blending implementation based on your patch. In the other case I could take your patch into v4 (cursor patch would have to be rabased in that case) and we will correct blending sometime later. I don't mind either case, up to you to decide.

The problem with not exposing alpha formats is that we either have to omit them everywhere or implement complicated workarounds that make the old hardware special. That's still true to some degree in this patch, but I think it's fairly cleanly split out.

Quoted 3 lines

Is there any ready-made testsuite for the DRM planes blending? Or you have made a test by yourself? In the latter case, could you please share the code so that I could test it too without burden of writing testcases from scratch?

I've implemented a test for this in my fork of libdrm:

https://cgit.freedesktop.org/~tagr/drm/

The kms-planes-blending test will fill three framebuffers with the primary colors at 0.5 alpha and presents them in three planes that overlay at the screen center.

I tested it on TrimSlice and the result looks correct.

Quoted 91 lines · Thierry Reding

Quoted 89 lines · Dmitry Osipenko

Quoted 36 lines · Thierry Reding

Quoted 12 lines · Dmitry Osipenko

Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with both modesetting and opentegra drivers. On older Tegra's each plane has a blending configuration which should be used to enable / disable alpha blending and right now the blending configs are hardcoded to disabled alpha blending. In order to support alpha formats properly, planes blending configuration must be adjusted, until then the alpha formats are equal to non-alpha.

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Dmitry Osipenko <[email protected]>

Quoted 8 lines

---
 drivers/gpu/drm/tegra/dc.c    | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h    |  1 +
 drivers/gpu/drm/tegra/fb.c    | 13 -------------
 drivers/gpu/drm/tegra/hub.c   |  3 ++-
 drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/tegra/plane.h |  2 +-
 6 files changed, 39 insertions(+), 31 deletions(-)

This kept bugging me, so I spent some time looking at the blending programming. I came up with the attached patch which seems to work for all scenarios and is fairly similar to your patch. It has the added benefit that we can keep support for more formats.

Any comments?

Thierry
--- >8 ---
From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
From: Thierry Reding <[email protected]>
Date: Wed, 20 Dec 2017 09:39:14 +0100
Subject: [PATCH] drm/tegra: dc: Implement legacy blending

Quoted 5 lines

This implements alpha blending on legacy display controllers (Tegra20, Tegra30 and Tegra114). While it's theoretically possible to support the zpos property to enable userspace to specify the Z-order of each plane individually, this is not currently supported and the same fixed Z- order as previously defined is used.

Perhaps one variant of implementing zpos could be by making overlays 'virtual', so each virtual overlay will be backed by the real HW plane and we could swap the HW planes of the virtual overlays, emulating zpos.

Quoted 28 lines

Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since the opaque formats are now supported.

Reported-by Dmitry Osipenko <[email protected]>

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c    | 74 ++++++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/tegra/dc.h    | 13 ++++++++
 drivers/gpu/drm/tegra/fb.c    | 12 -------
 drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++
 drivers/gpu/drm/tegra/plane.h |  3 ++
 5 files changed, 116 insertions(+), 27 deletions(-)

drivers/gpu/drm/tegra/dc.c +4 -1

index bc65c314e00f..07c687d7f615 100644
@@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in)
 	return dfixed_frac(inf);
 }
 
-static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
+static void
+tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
+				  const struct tegra_dc_window *window)
 {
+	u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |

drivers/gpu/drm/tegra/dc.c static inline u32 compute_initial_dda(unsigned int in) · line 185

Quoted 10 lines

drivers/gpu/drm/tegra/dc.c +10 -0

+			 BLEND_COLOR_KEY_NONE;
+	u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
+			 BLEND_COLOR_KEY_NONE;
+	u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
+
+	/* enable alpha blending if window->alpha */
+	if (window->alpha) {
+		background |= BLEND_CONTROL_DEPENDENT;
+		foreground |= BLEND_CONTROL_ALPHA;
+	}

I think dependent weight means that window doesn't have alpha transparency. So we should set the dependent_weight mode for opaque formats and alpha_weight for formats with alpha channel.

Quoted 3 lines

According to the TRM, dependent weight means that its weight will be 1 - the sum of the other overlapping windows. And it certainly seems to work that way in my tests (see below).

Yes, and you are hardcoding the blending modes regardless of the actual plane format. So even if underlying window has alpha format, it will be treated as it is opaque.

Quoted 92 lines · Dmitry Osipenko

Quoted 91 lines · Thierry Reding

Quoted 89 lines · Dmitry Osipenko

Quoted 36 lines · Thierry Reding

Quoted 12 lines · Dmitry Osipenko

Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with both modesetting and opentegra drivers. On older Tegra's each plane has a blending configuration which should be used to enable / disable alpha blending and right now the blending configs are hardcoded to disabled alpha blending. In order to support alpha formats properly, planes blending configuration must be adjusted, until then the alpha formats are equal to non-alpha.

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Dmitry Osipenko <[email protected]>

Quoted 8 lines

---
 drivers/gpu/drm/tegra/dc.c    | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h    |  1 +
 drivers/gpu/drm/tegra/fb.c    | 13 -------------
 drivers/gpu/drm/tegra/hub.c   |  3 ++-
 drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/tegra/plane.h |  2 +-
 6 files changed, 39 insertions(+), 31 deletions(-)

This kept bugging me, so I spent some time looking at the blending programming. I came up with the attached patch which seems to work for all scenarios and is fairly similar to your patch. It has the added benefit that we can keep support for more formats.

Any comments?

Thierry
--- >8 ---
From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
From: Thierry Reding <[email protected]>
Date: Wed, 20 Dec 2017 09:39:14 +0100
Subject: [PATCH] drm/tegra: dc: Implement legacy blending

Quoted 5 lines

This implements alpha blending on legacy display controllers (Tegra20, Tegra30 and Tegra114). While it's theoretically possible to support the zpos property to enable userspace to specify the Z-order of each plane individually, this is not currently supported and the same fixed Z- order as previously defined is used.

Perhaps one variant of implementing zpos could be by making overlays 'virtual', so each virtual overlay will be backed by the real HW plane and we could swap the HW planes of the virtual overlays, emulating zpos.

Quoted 28 lines

Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since the opaque formats are now supported.

Reported-by Dmitry Osipenko <[email protected]>

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c    | 74 ++++++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/tegra/dc.h    | 13 ++++++++
 drivers/gpu/drm/tegra/fb.c    | 12 -------
 drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++
 drivers/gpu/drm/tegra/plane.h |  3 ++
 5 files changed, 116 insertions(+), 27 deletions(-)

drivers/gpu/drm/tegra/dc.c +4 -1

index bc65c314e00f..07c687d7f615 100644
@@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in)
 	return dfixed_frac(inf);
 }
 
-static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
+static void
+tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
+				  const struct tegra_dc_window *window)
 {
+	u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |

drivers/gpu/drm/tegra/dc.c static inline u32 compute_initial_dda(unsigned int in) · line 185

Quoted 10 lines

drivers/gpu/drm/tegra/dc.c +10 -0

+			 BLEND_COLOR_KEY_NONE;
+	u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
+			 BLEND_COLOR_KEY_NONE;
+	u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
+
+	/* enable alpha blending if window->alpha */
+	if (window->alpha) {
+		background |= BLEND_CONTROL_DEPENDENT;
+		foreground |= BLEND_CONTROL_ALPHA;
+	}

I think dependent weight means that window doesn't have alpha transparency. So we should set the dependent_weight mode for opaque formats and alpha_weight for formats with alpha channel.

Quoted 7 lines

Quoted 3 lines

According to the TRM, dependent weight means that its weight will be 1 - the sum of the other overlapping windows. And it certainly seems to work that way in my tests (see below).

Yes, and you are hardcoding the blending modes regardless of the actual plane format. So even if underlying window has alpha format, it will be treated as it is opaque.

Ah yes, indeed. The patch currently assumes that if the current plane has an alpha component, then all the others will, too. That can probably be done fairly easily by looking at the current atomic state and inspecting the pixel format for each plane on the same CRTC. Let me take a stab at that tomorrow.

I'm wondering if we can deal with zpos, too, if we're already going through the trouble of looking at all planes involved. I think we can simply permute the WIN_BLEND register writes depending on the Z-order to implement proper zpos support.

Quoted 101 lines · Thierry Reding

Quoted 92 lines · Dmitry Osipenko

Quoted 91 lines · Thierry Reding

Quoted 89 lines · Dmitry Osipenko

Quoted 36 lines · Thierry Reding

Quoted 12 lines · Dmitry Osipenko

Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with both modesetting and opentegra drivers. On older Tegra's each plane has a blending configuration which should be used to enable / disable alpha blending and right now the blending configs are hardcoded to disabled alpha blending. In order to support alpha formats properly, planes blending configuration must be adjusted, until then the alpha formats are equal to non-alpha.

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Dmitry Osipenko <[email protected]>

Quoted 8 lines

---
 drivers/gpu/drm/tegra/dc.c    | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h    |  1 +
 drivers/gpu/drm/tegra/fb.c    | 13 -------------
 drivers/gpu/drm/tegra/hub.c   |  3 ++-
 drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/tegra/plane.h |  2 +-
 6 files changed, 39 insertions(+), 31 deletions(-)

This kept bugging me, so I spent some time looking at the blending programming. I came up with the attached patch which seems to work for all scenarios and is fairly similar to your patch. It has the added benefit that we can keep support for more formats.

Any comments?

Thierry
--- >8 ---
From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
From: Thierry Reding <[email protected]>
Date: Wed, 20 Dec 2017 09:39:14 +0100
Subject: [PATCH] drm/tegra: dc: Implement legacy blending

Quoted 5 lines

This implements alpha blending on legacy display controllers (Tegra20, Tegra30 and Tegra114). While it's theoretically possible to support the zpos property to enable userspace to specify the Z-order of each plane individually, this is not currently supported and the same fixed Z- order as previously defined is used.

Perhaps one variant of implementing zpos could be by making overlays 'virtual', so each virtual overlay will be backed by the real HW plane and we could swap the HW planes of the virtual overlays, emulating zpos.

Quoted 28 lines

Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since the opaque formats are now supported.

Reported-by Dmitry Osipenko <[email protected]>

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c    | 74 ++++++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/tegra/dc.h    | 13 ++++++++
 drivers/gpu/drm/tegra/fb.c    | 12 -------
 drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++
 drivers/gpu/drm/tegra/plane.h |  3 ++
 5 files changed, 116 insertions(+), 27 deletions(-)

drivers/gpu/drm/tegra/dc.c +4 -1

index bc65c314e00f..07c687d7f615 100644
@@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in)
 	return dfixed_frac(inf);
 }
 
-static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
+static void
+tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
+				  const struct tegra_dc_window *window)
 {
+	u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |

drivers/gpu/drm/tegra/dc.c static inline u32 compute_initial_dda(unsigned int in) · line 185

Quoted 10 lines

drivers/gpu/drm/tegra/dc.c +10 -0

+			 BLEND_COLOR_KEY_NONE;
+	u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
+			 BLEND_COLOR_KEY_NONE;
+	u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
+
+	/* enable alpha blending if window->alpha */
+	if (window->alpha) {
+		background |= BLEND_CONTROL_DEPENDENT;
+		foreground |= BLEND_CONTROL_ALPHA;
+	}

I think dependent weight means that window doesn't have alpha transparency. So we should set the dependent_weight mode for opaque formats and alpha_weight for formats with alpha channel.

Quoted 7 lines

Quoted 3 lines

According to the TRM, dependent weight means that its weight will be 1 - the sum of the other overlapping windows. And it certainly seems to work that way in my tests (see below).

Yes, and you are hardcoding the blending modes regardless of the actual plane format. So even if underlying window has alpha format, it will be treated as it is opaque.

Quoted 5 lines

Ah yes, indeed. The patch currently assumes that if the current plane has an alpha component, then all the others will, too. That can probably be done fairly easily by looking at the current atomic state and inspecting the pixel format for each plane on the same CRTC. Let me take a stab at that tomorrow.

Okay, please push patch to the public repo once it is ready and let me know. I'll rebase cursor patch on it.

Quoted 4 lines

I'm wondering if we can deal with zpos, too, if we're already going through the trouble of looking at all planes involved. I think we can simply permute the WIN_BLEND register writes depending on the Z-order to implement proper zpos support.

I think we will have to permute the whole window state, not only the WIN_BLEND register.

Also I'm not sure how to make topmost window opaque and underlying windows transparent, will have to check how blending works. Maybe it not possible at all..

Quoted 118 lines · Dmitry Osipenko

Quoted 101 lines · Thierry Reding

Quoted 92 lines · Dmitry Osipenko

Quoted 91 lines · Thierry Reding

Quoted 89 lines · Dmitry Osipenko

Quoted 36 lines · Thierry Reding

Quoted 12 lines · Dmitry Osipenko

Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with both modesetting and opentegra drivers. On older Tegra's each plane has a blending configuration which should be used to enable / disable alpha blending and right now the blending configs are hardcoded to disabled alpha blending. In order to support alpha formats properly, planes blending configuration must be adjusted, until then the alpha formats are equal to non-alpha.

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Dmitry Osipenko <[email protected]>

Quoted 8 lines

---
 drivers/gpu/drm/tegra/dc.c    | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h    |  1 +
 drivers/gpu/drm/tegra/fb.c    | 13 -------------
 drivers/gpu/drm/tegra/hub.c   |  3 ++-
 drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/tegra/plane.h |  2 +-
 6 files changed, 39 insertions(+), 31 deletions(-)

This kept bugging me, so I spent some time looking at the blending programming. I came up with the attached patch which seems to work for all scenarios and is fairly similar to your patch. It has the added benefit that we can keep support for more formats.

Any comments?

Thierry
--- >8 ---
From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
From: Thierry Reding <[email protected]>
Date: Wed, 20 Dec 2017 09:39:14 +0100
Subject: [PATCH] drm/tegra: dc: Implement legacy blending

Quoted 5 lines

This implements alpha blending on legacy display controllers (Tegra20, Tegra30 and Tegra114). While it's theoretically possible to support the zpos property to enable userspace to specify the Z-order of each plane individually, this is not currently supported and the same fixed Z- order as previously defined is used.

Perhaps one variant of implementing zpos could be by making overlays 'virtual', so each virtual overlay will be backed by the real HW plane and we could swap the HW planes of the virtual overlays, emulating zpos.

Quoted 28 lines

Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since the opaque formats are now supported.

Reported-by Dmitry Osipenko <[email protected]>

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c    | 74 ++++++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/tegra/dc.h    | 13 ++++++++
 drivers/gpu/drm/tegra/fb.c    | 12 -------
 drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++
 drivers/gpu/drm/tegra/plane.h |  3 ++
 5 files changed, 116 insertions(+), 27 deletions(-)

drivers/gpu/drm/tegra/dc.c +4 -1

index bc65c314e00f..07c687d7f615 100644
@@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in)
 	return dfixed_frac(inf);
 }
 
-static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
+static void
+tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
+				  const struct tegra_dc_window *window)
 {
+	u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |

drivers/gpu/drm/tegra/dc.c static inline u32 compute_initial_dda(unsigned int in) · line 185

Quoted 10 lines

drivers/gpu/drm/tegra/dc.c +10 -0

+			 BLEND_COLOR_KEY_NONE;
+	u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
+			 BLEND_COLOR_KEY_NONE;
+	u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
+
+	/* enable alpha blending if window->alpha */
+	if (window->alpha) {
+		background |= BLEND_CONTROL_DEPENDENT;
+		foreground |= BLEND_CONTROL_ALPHA;
+	}

I think dependent weight means that window doesn't have alpha transparency. So we should set the dependent_weight mode for opaque formats and alpha_weight for formats with alpha channel.

Quoted 7 lines

Quoted 3 lines

According to the TRM, dependent weight means that its weight will be 1 - the sum of the other overlapping windows. And it certainly seems to work that way in my tests (see below).

Yes, and you are hardcoding the blending modes regardless of the actual plane format. So even if underlying window has alpha format, it will be treated as it is opaque.

Quoted 5 lines

Ah yes, indeed. The patch currently assumes that if the current plane has an alpha component, then all the others will, too. That can probably be done fairly easily by looking at the current atomic state and inspecting the pixel format for each plane on the same CRTC. Let me take a stab at that tomorrow.

Okay, please push patch to the public repo once it is ready and let me know. I'll rebase cursor patch on it.

Quoted 4 lines

I'm wondering if we can deal with zpos, too, if we're already going through the trouble of looking at all planes involved. I think we can simply permute the WIN_BLEND register writes depending on the Z-order to implement proper zpos support.

I think we will have to permute the whole window state, not only the WIN_BLEND register.

Quoted 2 lines

Also I'm not sure how to make topmost window opaque and underlying windows transparent, will have to check how blending works. Maybe it not possible at all..

Although it is simple to implement using the fixed weights for 2WIN cases, but then we will have to enhance the legacy blending code a tad to handle this case properly. You'll have to do quite a lot for tomorrow ;) Alternatively you can still apply my patch that drops alpha formats on T20/30 and we will try to implement alpha + zpos for 4.17.

Quoted 119 lines · Dmitry Osipenko

Quoted 118 lines · Dmitry Osipenko

Quoted 101 lines · Thierry Reding

Quoted 92 lines · Dmitry Osipenko

Quoted 91 lines · Thierry Reding

Quoted 89 lines · Dmitry Osipenko

Quoted 36 lines · Thierry Reding

Quoted 12 lines · Dmitry Osipenko

Commit 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats") broke DRM's MODE_ADDFB IOCTL on Tegra20/30, because IOCTL uses XRGB format if requested FB depth is 24bpp. As a result, Xorg doesn't work anymore with both modesetting and opentegra drivers. On older Tegra's each plane has a blending configuration which should be used to enable / disable alpha blending and right now the blending configs are hardcoded to disabled alpha blending. In order to support alpha formats properly, planes blending configuration must be adjusted, until then the alpha formats are equal to non-alpha.

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Dmitry Osipenko <[email protected]>

Quoted 8 lines

---
 drivers/gpu/drm/tegra/dc.c    | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h    |  1 +
 drivers/gpu/drm/tegra/fb.c    | 13 -------------
 drivers/gpu/drm/tegra/hub.c   |  3 ++-
 drivers/gpu/drm/tegra/plane.c | 22 +++++++++++++++++-----
 drivers/gpu/drm/tegra/plane.h |  2 +-
 6 files changed, 39 insertions(+), 31 deletions(-)

This kept bugging me, so I spent some time looking at the blending programming. I came up with the attached patch which seems to work for all scenarios and is fairly similar to your patch. It has the added benefit that we can keep support for more formats.

Any comments?

Thierry
--- >8 ---
From 3d2b7d1a9b8239dc6940477d8783461ac60783bc Mon Sep 17 00:00:00 2001
From: Thierry Reding <[email protected]>
Date: Wed, 20 Dec 2017 09:39:14 +0100
Subject: [PATCH] drm/tegra: dc: Implement legacy blending

Quoted 5 lines

This implements alpha blending on legacy display controllers (Tegra20, Tegra30 and Tegra114). While it's theoretically possible to support the zpos property to enable userspace to specify the Z-order of each plane individually, this is not currently supported and the same fixed Z- order as previously defined is used.

Perhaps one variant of implementing zpos could be by making overlays 'virtual', so each virtual overlay will be backed by the real HW plane and we could swap the HW planes of the virtual overlays, emulating zpos.

Quoted 28 lines

Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since the opaque formats are now supported.

Reported-by Dmitry Osipenko <[email protected]>

Fixes 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")

Signed-off-by Thierry Reding <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c    | 74 ++++++++++++++++++++++++++++++++++---------
 drivers/gpu/drm/tegra/dc.h    | 13 ++++++++
 drivers/gpu/drm/tegra/fb.c    | 12 -------
 drivers/gpu/drm/tegra/plane.c | 41 ++++++++++++++++++++++++
 drivers/gpu/drm/tegra/plane.h |  3 ++
 5 files changed, 116 insertions(+), 27 deletions(-)

drivers/gpu/drm/tegra/dc.c +4 -1

index bc65c314e00f..07c687d7f615 100644
@@ -168,32 +168,46 @@ static inline u32 compute_initial_dda(unsigned int in)
 	return dfixed_frac(inf);
 }
 
-static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
+static void
+tegra_plane_setup_blending_legacy(struct tegra_plane *plane,
+				  const struct tegra_dc_window *window)
 {
+	u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |

drivers/gpu/drm/tegra/dc.c static inline u32 compute_initial_dda(unsigned int in) · line 185

Quoted 10 lines

drivers/gpu/drm/tegra/dc.c +10 -0

+			 BLEND_COLOR_KEY_NONE;
+	u32 background = BLEND_WEIGHT1(0) | BLEND_WEIGHT0(0) |
+			 BLEND_COLOR_KEY_NONE;
+	u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
+
+	/* enable alpha blending if window->alpha */
+	if (window->alpha) {
+		background |= BLEND_CONTROL_DEPENDENT;
+		foreground |= BLEND_CONTROL_ALPHA;
+	}

I think dependent weight means that window doesn't have alpha transparency. So we should set the dependent_weight mode for opaque formats and alpha_weight for formats with alpha channel.

Quoted 7 lines

Quoted 3 lines

According to the TRM, dependent weight means that its weight will be 1 - the sum of the other overlapping windows. And it certainly seems to work that way in my tests (see below).

Yes, and you are hardcoding the blending modes regardless of the actual plane format. So even if underlying window has alpha format, it will be treated as it is opaque.

Quoted 5 lines

Ah yes, indeed. The patch currently assumes that if the current plane has an alpha component, then all the others will, too. That can probably be done fairly easily by looking at the current atomic state and inspecting the pixel format for each plane on the same CRTC. Let me take a stab at that tomorrow.

Okay, please push patch to the public repo once it is ready and let me know. I'll rebase cursor patch on it.

Quoted 4 lines

I'm wondering if we can deal with zpos, too, if we're already going through the trouble of looking at all planes involved. I think we can simply permute the WIN_BLEND register writes depending on the Z-order to implement proper zpos support.

I think we will have to permute the whole window state, not only the WIN_BLEND register.

Quoted 8 lines

Quoted 2 lines

Also I'm not sure how to make topmost window opaque and underlying windows transparent, will have to check how blending works. Maybe it not possible at all..

Although it is simple to implement using the fixed weights for 2WIN cases, but then we will have to enhance the legacy blending code a tad to handle this case properly. You'll have to do quite a lot for tomorrow ;) Alternatively you can still apply my patch that drops alpha formats on T20/30 and we will try to implement alpha + zpos for 4.17.

I think I finally nailed this. Just sent out a v2 and updated my test program to use different formats for each plane. I tested all opaque & alpha combinations and it all looks correct now.

Older Tegra's do not support RGBA format for the cursor, but instead overlay plane could be used for it. Since there is no much use for the overlays on a regular desktop and HW-accelerated cursor is much nicer than the jerky SW cursor, let's trade one overlay plane for the cursor.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c | 75 +++++++++++++++++++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h |  2 ++
 2 files changed, 59 insertions(+), 18 deletions(-)

Quoted 10 lines · Dmitry Osipenko

Older Tegra's do not support RGBA format for the cursor, but instead overlay plane could be used for it. Since there is no much use for the overlays on a regular desktop and HW-accelerated cursor is much nicer than the jerky SW cursor, let's trade one overlay plane for the cursor.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/gpu/drm/tegra/dc.c | 75 +++++++++++++++++++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h |  2 ++
 2 files changed, 59 insertions(+), 18 deletions(-)

Given the dependency on the alpha formats patch and due to lack of time because of the holidays messing up the schedule I'd like to defer this to v4.17, unless we can settle all of it until Friday.

iommu_map_sg() doesn't return a error value, but a size of the requested IOMMU mapping or zero in case of error.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/gpu/drm/tegra/gem.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

host1x_syncpt_wait() takes timeout value in jiffies, but DRM passes it in milliseconds.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 drivers/gpu/drm/tegra/drm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)