[PATCH v3 0/5] Some corrections and improvement for Tegra DRM

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v3 0/5] Some corrections and improvement for Tegra DRM
@ 2017-12-20 15:46 ` Dmitry Osipenko
  0 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 15:46 UTC (permalink / raw)
  To: Thierry Reding
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

I've aggregated all Tegra DRM patches that I've sent before into a single
series.

What's changed:

	- Alpha formats been dropped in addition to restore of opaque formats
	  on T20/30.

	- Reworked the HW cursor patch a tad, since alpha formats have been
	  dropped from the overlay plane.

	- Fixed warning that was reported by kbuild bot for ARM64 build.

Dmitry Osipenko (5):
  drm/tegra: dc: Link DC1 to DC0 on Tegra20
  drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  drm/tegra: Trade overlay plane for cursor on older Tegra's
  drm/tegra: gem: Correct iommu_map_sg() error checking
  drm/tegra: Correct timeout in tegra_syncpt_wait

 drivers/gpu/drm/tegra/dc.c    | 184 ++++++++++++++++++++++++++++--------------
 drivers/gpu/drm/tegra/dc.h    |   5 +-
 drivers/gpu/drm/tegra/drm.c   |   3 +-
 drivers/gpu/drm/tegra/fb.c    |  13 ---
 drivers/gpu/drm/tegra/gem.c   |  15 ++--
 drivers/gpu/drm/tegra/hub.c   |   3 +-
 drivers/gpu/drm/tegra/plane.c |  22 +++--
 drivers/gpu/drm/tegra/plane.h |   2 +-
 8 files changed, 158 insertions(+), 89 deletions(-)

-- 
2.15.1

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH v3 0/5] Some corrections and improvement for Tegra DRM
@ 2017-12-20 15:46 ` Dmitry Osipenko
  0 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 15:46 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

I've aggregated all Tegra DRM patches that I've sent before into a single
series.

What's changed:

	- Alpha formats been dropped in addition to restore of opaque formats
	  on T20/30.

	- Reworked the HW cursor patch a tad, since alpha formats have been
	  dropped from the overlay plane.

	- Fixed warning that was reported by kbuild bot for ARM64 build.

Dmitry Osipenko (5):
  drm/tegra: dc: Link DC1 to DC0 on Tegra20
  drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  drm/tegra: Trade overlay plane for cursor on older Tegra's
  drm/tegra: gem: Correct iommu_map_sg() error checking
  drm/tegra: Correct timeout in tegra_syncpt_wait

 drivers/gpu/drm/tegra/dc.c    | 184 ++++++++++++++++++++++++++++--------------
 drivers/gpu/drm/tegra/dc.h    |   5 +-
 drivers/gpu/drm/tegra/drm.c   |   3 +-
 drivers/gpu/drm/tegra/fb.c    |  13 ---
 drivers/gpu/drm/tegra/gem.c   |  15 ++--
 drivers/gpu/drm/tegra/hub.c   |   3 +-
 drivers/gpu/drm/tegra/plane.c |  22 +++--
 drivers/gpu/drm/tegra/plane.h |   2 +-
 8 files changed, 158 insertions(+), 89 deletions(-)

-- 
2.15.1

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH v3 1/5] drm/tegra: dc: Link DC1 to DC0 on Tegra20
  2017-12-20 15:46 ` Dmitry Osipenko
  (?)
@ 2017-12-20 15:46 ` Dmitry Osipenko
  2017-12-20 20:17     ` Thierry Reding
  -1 siblings, 1 reply; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 15:46 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

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 <digetx@gmail.com>
---

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

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index e8a0cad5899c..5299185cea2f 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1848,7 +1848,7 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
 	.supports_block_linear = false,
 	.pitch_align = 8,
 	.has_powergate = false,
-	.broken_reset = true,
+	.coupled_pm = true,
 	.has_nvdisplay = false,
 	.num_primary_formats = ARRAY_SIZE(tegra20_primary_formats),
 	.primary_formats = tegra20_primary_formats,
@@ -1863,7 +1863,7 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
 	.supports_block_linear = false,
 	.pitch_align = 8,
 	.has_powergate = false,
-	.broken_reset = false,
+	.coupled_pm = false,
 	.has_nvdisplay = false,
 	.num_primary_formats = ARRAY_SIZE(tegra20_primary_formats),
 	.primary_formats = tegra20_primary_formats,
@@ -1878,7 +1878,7 @@ static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
 	.supports_block_linear = false,
 	.pitch_align = 64,
 	.has_powergate = true,
-	.broken_reset = false,
+	.coupled_pm = false,
 	.has_nvdisplay = false,
 	.num_primary_formats = ARRAY_SIZE(tegra114_primary_formats),
 	.primary_formats = tegra114_primary_formats,
@@ -1893,7 +1893,7 @@ static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
 	.supports_block_linear = true,
 	.pitch_align = 64,
 	.has_powergate = true,
-	.broken_reset = false,
+	.coupled_pm = false,
 	.has_nvdisplay = false,
 	.num_primary_formats = ARRAY_SIZE(tegra124_primary_formats),
 	.primary_formats = tegra114_primary_formats,
@@ -1908,7 +1908,7 @@ static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
 	.supports_block_linear = true,
 	.pitch_align = 64,
 	.has_powergate = true,
-	.broken_reset = false,
+	.coupled_pm = false,
 	.has_nvdisplay = false,
 	.num_primary_formats = ARRAY_SIZE(tegra114_primary_formats),
 	.primary_formats = tegra114_primary_formats,
@@ -1957,7 +1957,7 @@ static const struct tegra_dc_soc_info tegra186_dc_soc_info = {
 	.supports_block_linear = true,
 	.pitch_align = 64,
 	.has_powergate = false,
-	.broken_reset = false,
+	.coupled_pm = false,
 	.has_nvdisplay = true,
 	.wgrps = tegra186_dc_wgrps,
 	.num_wgrps = ARRAY_SIZE(tegra186_dc_wgrps),
@@ -2025,6 +2025,11 @@ static int tegra_dc_parse_dt(struct tegra_dc *dc)
 	return 0;
 }
 
+static int tegra_dc_match(struct device *dev, void *data)
+{
+	return of_device_is_compatible(dev->of_node, "nvidia,tegra20-dc");
+}
+
 static int tegra_dc_probe(struct platform_device *pdev)
 {
 	struct resource *regs;
@@ -2045,6 +2050,28 @@ static int tegra_dc_probe(struct platform_device *pdev)
 	if (err < 0)
 		return err;
 
+	/*
+	 * On Tegra20 DC1 requires DC0 to be taken out of reset in order to
+	 * be enabled, otherwise CPU hangs on writing to CMD_DISPLAY_COMMAND /
+	 * POWER_CONTROL registers during CRTC enabling.
+	 */
+	if (dc->pipe == 1 && dc->soc->coupled_pm) {
+		struct device_link *link;
+		struct device *dc0_dev;
+
+		dc0_dev = driver_find_device(pdev->dev.driver, NULL, NULL,
+					     tegra_dc_match);
+		if (!dc0_dev)
+			return -EPROBE_DEFER;
+
+		link = device_link_add(&pdev->dev, dc0_dev,
+				       DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE);
+		if (!link) {
+			dev_err(&pdev->dev, "failed to link to DC0\n");
+			return -EINVAL;
+		}
+	}
+
 	dc->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(dc->clk)) {
 		dev_err(&pdev->dev, "failed to get clock\n");
@@ -2058,21 +2085,19 @@ static int tegra_dc_probe(struct platform_device *pdev)
 	}
 
 	/* assert reset and disable clock */
-	if (!dc->soc->broken_reset) {
-		err = clk_prepare_enable(dc->clk);
-		if (err < 0)
-			return err;
+	err = clk_prepare_enable(dc->clk);
+	if (err < 0)
+		return err;
 
-		usleep_range(2000, 4000);
+	usleep_range(2000, 4000);
 
-		err = reset_control_assert(dc->rst);
-		if (err < 0)
-			return err;
+	err = reset_control_assert(dc->rst);
+	if (err < 0)
+		return err;
 
-		usleep_range(2000, 4000);
+	usleep_range(2000, 4000);
 
-		clk_disable_unprepare(dc->clk);
-	}
+	clk_disable_unprepare(dc->clk);
 
 	if (dc->soc->has_powergate) {
 		if (dc->pipe == 0)
@@ -2146,12 +2171,10 @@ static int tegra_dc_suspend(struct device *dev)
 	struct tegra_dc *dc = dev_get_drvdata(dev);
 	int err;
 
-	if (!dc->soc->broken_reset) {
-		err = reset_control_assert(dc->rst);
-		if (err < 0) {
-			dev_err(dev, "failed to assert reset: %d\n", err);
-			return err;
-		}
+	err = reset_control_assert(dc->rst);
+	if (err < 0) {
+		dev_err(dev, "failed to assert reset: %d\n", err);
+		return err;
 	}
 
 	if (dc->soc->has_powergate)
@@ -2181,13 +2204,10 @@ static int tegra_dc_resume(struct device *dev)
 			return err;
 		}
 
-		if (!dc->soc->broken_reset) {
-			err = reset_control_deassert(dc->rst);
-			if (err < 0) {
-				dev_err(dev,
-					"failed to deassert reset: %d\n", err);
-				return err;
-			}
+		err = reset_control_deassert(dc->rst);
+		if (err < 0) {
+			dev_err(dev, "failed to deassert reset: %d\n", err);
+			return err;
 		}
 	}
 
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 8d68997e6263..8098f49c0d96 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -57,7 +57,7 @@ struct tegra_dc_soc_info {
 	bool supports_block_linear;
 	unsigned int pitch_align;
 	bool has_powergate;
-	bool broken_reset;
+	bool coupled_pm;
 	bool has_nvdisplay;
 	const struct tegra_windowgroup_soc *wgrps;
 	unsigned int num_wgrps;
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  2017-12-20 15:46 ` Dmitry Osipenko
@ 2017-12-20 15:46     ` Dmitry Osipenko
  -1 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 15:46 UTC (permalink / raw)
  To: Thierry Reding
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 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(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 5299185cea2f..460510366bb8 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -299,12 +299,12 @@ static void tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
 }
 
 static const u32 tegra20_primary_formats[] = {
-	DRM_FORMAT_ARGB4444,
-	DRM_FORMAT_ARGB1555,
 	DRM_FORMAT_RGB565,
-	DRM_FORMAT_RGBA5551,
-	DRM_FORMAT_ABGR8888,
-	DRM_FORMAT_ARGB8888,
+	/* non-native formats */
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGBX5551,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
 };
 
 static const u32 tegra114_primary_formats[] = {
@@ -369,7 +369,8 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
 
 	err = tegra_plane_format(state->fb->format->format,
 				 &plane_state->format,
-				 &plane_state->swap);
+				 &plane_state->swap,
+				 dc->soc->supports_opaque_formats);
 	if (err < 0)
 		return err;
 
@@ -692,12 +693,12 @@ static struct drm_plane *tegra_dc_cursor_plane_create(struct drm_device *drm,
 }
 
 static const u32 tegra20_overlay_formats[] = {
-	DRM_FORMAT_ARGB4444,
-	DRM_FORMAT_ARGB1555,
 	DRM_FORMAT_RGB565,
-	DRM_FORMAT_RGBA5551,
-	DRM_FORMAT_ABGR8888,
-	DRM_FORMAT_ARGB8888,
+	/* non-native formats */
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGBX5551,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
 	/* planar formats */
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_YUYV,
@@ -1854,6 +1855,7 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
 	.primary_formats = tegra20_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
 	.overlay_formats = tegra20_overlay_formats,
+	.supports_opaque_formats = false,
 };
 
 static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
@@ -1869,6 +1871,7 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
 	.primary_formats = tegra20_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
 	.overlay_formats = tegra20_overlay_formats,
+	.supports_opaque_formats = false,
 };
 
 static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
@@ -1884,6 +1887,7 @@ static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
 	.primary_formats = tegra114_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra114_overlay_formats),
 	.overlay_formats = tegra114_overlay_formats,
+	.supports_opaque_formats = true,
 };
 
 static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
@@ -1899,6 +1903,7 @@ static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
 	.primary_formats = tegra114_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra124_overlay_formats),
 	.overlay_formats = tegra114_overlay_formats,
+	.supports_opaque_formats = true,
 };
 
 static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
@@ -1914,6 +1919,7 @@ static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
 	.primary_formats = tegra114_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra114_overlay_formats),
 	.overlay_formats = tegra114_overlay_formats,
+	.supports_opaque_formats = true,
 };
 
 static const struct tegra_windowgroup_soc tegra186_dc_wgrps[] = {
@@ -1961,6 +1967,7 @@ static const struct tegra_dc_soc_info tegra186_dc_soc_info = {
 	.has_nvdisplay = true,
 	.wgrps = tegra186_dc_wgrps,
 	.num_wgrps = ARRAY_SIZE(tegra186_dc_wgrps),
+	.supports_opaque_formats = true,
 };
 
 static const struct of_device_id tegra_dc_of_match[] = {
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 8098f49c0d96..3a66a1127ee7 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -65,6 +65,7 @@ struct tegra_dc_soc_info {
 	unsigned int num_primary_formats;
 	const u32 *overlay_formats;
 	unsigned int num_overlay_formats;
+	bool supports_opaque_formats;
 };
 
 struct tegra_dc {
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index 5f6289c4e56a..bd8c9da1ed0d 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -253,19 +253,6 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
 	cmd.height = sizes->surface_height;
 	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);
 
diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index f4911feda9ff..03298a1e87cc 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -332,7 +332,8 @@ static int tegra_shared_plane_atomic_check(struct drm_plane *plane,
 
 	err = tegra_plane_format(state->fb->format->format,
 				 &plane_state->format,
-				 &plane_state->swap);
+				 &plane_state->swap,
+				 true);
 	if (err < 0)
 		return err;
 
diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 326700cd0d80..345ec37c7481 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -103,7 +103,7 @@ int tegra_plane_state_add(struct tegra_plane *plane,
 	return 0;
 }
 
-int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap)
+int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap, bool opaque_fmt)
 {
 	/* assume no swapping of fetched data */
 	if (swap)
@@ -147,11 +147,17 @@ int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap)
 		break;
 
 	case DRM_FORMAT_XRGB1555:
-		*format = WIN_COLOR_DEPTH_B5G5R5X1;
+		if (opaque_fmt)
+			*format = WIN_COLOR_DEPTH_B5G5R5X1;
+		else
+			*format = WIN_COLOR_DEPTH_B5G5R5A;
 		break;
 
 	case DRM_FORMAT_RGBX5551:
-		*format = WIN_COLOR_DEPTH_X1B5G5R5;
+		if (opaque_fmt)
+			*format = WIN_COLOR_DEPTH_X1B5G5R5;
+		else
+			*format = WIN_COLOR_DEPTH_AB5G5R5;
 		break;
 
 	case DRM_FORMAT_XBGR1555:
@@ -175,11 +181,17 @@ int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap)
 		break;
 
 	case DRM_FORMAT_XRGB8888:
-		*format = WIN_COLOR_DEPTH_B8G8R8X8;
+		if (opaque_fmt)
+			*format = WIN_COLOR_DEPTH_B8G8R8X8;
+		else
+			*format = WIN_COLOR_DEPTH_B8G8R8A8;
 		break;
 
 	case DRM_FORMAT_XBGR8888:
-		*format = WIN_COLOR_DEPTH_R8G8B8X8;
+		if (opaque_fmt)
+			*format = WIN_COLOR_DEPTH_R8G8B8X8;
+		else
+			*format = WIN_COLOR_DEPTH_R8G8B8A8;
 		break;
 
 	case DRM_FORMAT_UYVY:
diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h
index fc7566f630fa..27145d64ad90 100644
--- a/drivers/gpu/drm/tegra/plane.h
+++ b/drivers/gpu/drm/tegra/plane.h
@@ -55,7 +55,7 @@ extern const struct drm_plane_funcs tegra_plane_funcs;
 int tegra_plane_state_add(struct tegra_plane *plane,
 			  struct drm_plane_state *state);
 
-int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap);
+int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap, bool opaque_fmt);
 bool tegra_plane_format_is_yuv(unsigned int format, bool *planar);
 
 #endif /* TEGRA_PLANE_H */
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
@ 2017-12-20 15:46     ` Dmitry Osipenko
  0 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 15:46 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

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 <digetx@gmail.com>
---
 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(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 5299185cea2f..460510366bb8 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -299,12 +299,12 @@ static void tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
 }
 
 static const u32 tegra20_primary_formats[] = {
-	DRM_FORMAT_ARGB4444,
-	DRM_FORMAT_ARGB1555,
 	DRM_FORMAT_RGB565,
-	DRM_FORMAT_RGBA5551,
-	DRM_FORMAT_ABGR8888,
-	DRM_FORMAT_ARGB8888,
+	/* non-native formats */
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGBX5551,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
 };
 
 static const u32 tegra114_primary_formats[] = {
@@ -369,7 +369,8 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
 
 	err = tegra_plane_format(state->fb->format->format,
 				 &plane_state->format,
-				 &plane_state->swap);
+				 &plane_state->swap,
+				 dc->soc->supports_opaque_formats);
 	if (err < 0)
 		return err;
 
@@ -692,12 +693,12 @@ static struct drm_plane *tegra_dc_cursor_plane_create(struct drm_device *drm,
 }
 
 static const u32 tegra20_overlay_formats[] = {
-	DRM_FORMAT_ARGB4444,
-	DRM_FORMAT_ARGB1555,
 	DRM_FORMAT_RGB565,
-	DRM_FORMAT_RGBA5551,
-	DRM_FORMAT_ABGR8888,
-	DRM_FORMAT_ARGB8888,
+	/* non-native formats */
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGBX5551,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
 	/* planar formats */
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_YUYV,
@@ -1854,6 +1855,7 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
 	.primary_formats = tegra20_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
 	.overlay_formats = tegra20_overlay_formats,
+	.supports_opaque_formats = false,
 };
 
 static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
@@ -1869,6 +1871,7 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
 	.primary_formats = tegra20_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
 	.overlay_formats = tegra20_overlay_formats,
+	.supports_opaque_formats = false,
 };
 
 static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
@@ -1884,6 +1887,7 @@ static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
 	.primary_formats = tegra114_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra114_overlay_formats),
 	.overlay_formats = tegra114_overlay_formats,
+	.supports_opaque_formats = true,
 };
 
 static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
@@ -1899,6 +1903,7 @@ static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
 	.primary_formats = tegra114_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra124_overlay_formats),
 	.overlay_formats = tegra114_overlay_formats,
+	.supports_opaque_formats = true,
 };
 
 static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
@@ -1914,6 +1919,7 @@ static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
 	.primary_formats = tegra114_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra114_overlay_formats),
 	.overlay_formats = tegra114_overlay_formats,
+	.supports_opaque_formats = true,
 };
 
 static const struct tegra_windowgroup_soc tegra186_dc_wgrps[] = {
@@ -1961,6 +1967,7 @@ static const struct tegra_dc_soc_info tegra186_dc_soc_info = {
 	.has_nvdisplay = true,
 	.wgrps = tegra186_dc_wgrps,
 	.num_wgrps = ARRAY_SIZE(tegra186_dc_wgrps),
+	.supports_opaque_formats = true,
 };
 
 static const struct of_device_id tegra_dc_of_match[] = {
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 8098f49c0d96..3a66a1127ee7 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -65,6 +65,7 @@ struct tegra_dc_soc_info {
 	unsigned int num_primary_formats;
 	const u32 *overlay_formats;
 	unsigned int num_overlay_formats;
+	bool supports_opaque_formats;
 };
 
 struct tegra_dc {
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index 5f6289c4e56a..bd8c9da1ed0d 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -253,19 +253,6 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
 	cmd.height = sizes->surface_height;
 	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);
 
diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index f4911feda9ff..03298a1e87cc 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -332,7 +332,8 @@ static int tegra_shared_plane_atomic_check(struct drm_plane *plane,
 
 	err = tegra_plane_format(state->fb->format->format,
 				 &plane_state->format,
-				 &plane_state->swap);
+				 &plane_state->swap,
+				 true);
 	if (err < 0)
 		return err;
 
diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 326700cd0d80..345ec37c7481 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -103,7 +103,7 @@ int tegra_plane_state_add(struct tegra_plane *plane,
 	return 0;
 }
 
-int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap)
+int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap, bool opaque_fmt)
 {
 	/* assume no swapping of fetched data */
 	if (swap)
@@ -147,11 +147,17 @@ int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap)
 		break;
 
 	case DRM_FORMAT_XRGB1555:
-		*format = WIN_COLOR_DEPTH_B5G5R5X1;
+		if (opaque_fmt)
+			*format = WIN_COLOR_DEPTH_B5G5R5X1;
+		else
+			*format = WIN_COLOR_DEPTH_B5G5R5A;
 		break;
 
 	case DRM_FORMAT_RGBX5551:
-		*format = WIN_COLOR_DEPTH_X1B5G5R5;
+		if (opaque_fmt)
+			*format = WIN_COLOR_DEPTH_X1B5G5R5;
+		else
+			*format = WIN_COLOR_DEPTH_AB5G5R5;
 		break;
 
 	case DRM_FORMAT_XBGR1555:
@@ -175,11 +181,17 @@ int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap)
 		break;
 
 	case DRM_FORMAT_XRGB8888:
-		*format = WIN_COLOR_DEPTH_B8G8R8X8;
+		if (opaque_fmt)
+			*format = WIN_COLOR_DEPTH_B8G8R8X8;
+		else
+			*format = WIN_COLOR_DEPTH_B8G8R8A8;
 		break;
 
 	case DRM_FORMAT_XBGR8888:
-		*format = WIN_COLOR_DEPTH_R8G8B8X8;
+		if (opaque_fmt)
+			*format = WIN_COLOR_DEPTH_R8G8B8X8;
+		else
+			*format = WIN_COLOR_DEPTH_R8G8B8A8;
 		break;
 
 	case DRM_FORMAT_UYVY:
diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h
index fc7566f630fa..27145d64ad90 100644
--- a/drivers/gpu/drm/tegra/plane.h
+++ b/drivers/gpu/drm/tegra/plane.h
@@ -55,7 +55,7 @@ extern const struct drm_plane_funcs tegra_plane_funcs;
 int tegra_plane_state_add(struct tegra_plane *plane,
 			  struct drm_plane_state *state);
 
-int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap);
+int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap, bool opaque_fmt);
 bool tegra_plane_format_is_yuv(unsigned int format, bool *planar);
 
 #endif /* TEGRA_PLANE_H */
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 3/5] drm/tegra: Trade overlay plane for cursor on older Tegra's
  2017-12-20 15:46 ` Dmitry Osipenko
@ 2017-12-20 15:46     ` Dmitry Osipenko
  -1 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 15:46 UTC (permalink / raw)
  To: Thierry Reding
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/tegra/dc.c | 75 +++++++++++++++++++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h |  2 ++
 2 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 460510366bb8..eaff8757bbe0 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -125,9 +125,10 @@ static inline u32 compute_initial_dda(unsigned int in)
 	return dfixed_frac(inf);
 }
 
-static void tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
+static void tegra_dc_setup_window(struct tegra_dc *dc, struct drm_plane *plane,
 				  const struct tegra_dc_window *window)
 {
+	struct tegra_plane *p = to_tegra_plane(plane);
 	unsigned h_offset, v_offset, h_size, v_size, h_dda, v_dda, bpp;
 	unsigned long value, flags;
 	bool yuv, planar;
@@ -144,7 +145,7 @@ static void tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
 
 	spin_lock_irqsave(&dc->lock, flags);
 
-	value = WINDOW_A_SELECT << index;
+	value = WINDOW_A_SELECT << p->index;
 	tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
 
 	tegra_dc_writel(dc, window->format, DC_WIN_COLOR_DEPTH);
@@ -275,23 +276,29 @@ static void tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
 	tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_NOKEY);
 	tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_1WIN);
 
-	switch (index) {
+	switch (p->index) {
 	case 0:
 		tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_2WIN_X);
-		tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_2WIN_Y);
+		tegra_dc_writel(dc, 0x000008, DC_WIN_BLEND_2WIN_Y);
 		tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_3WIN_XY);
 		break;
 
 	case 1:
 		tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_X);
-		tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_2WIN_Y);
-		tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_3WIN_XY);
+		tegra_dc_writel(dc, 0x000008, DC_WIN_BLEND_2WIN_Y);
+		tegra_dc_writel(dc, 0x000008, DC_WIN_BLEND_3WIN_XY);
 		break;
 
 	case 2:
-		tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_X);
-		tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_Y);
-		tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_3WIN_XY);
+		if (plane->type == DRM_PLANE_TYPE_CURSOR) {
+			tegra_dc_writel(dc, 0xffff04, DC_WIN_BLEND_2WIN_X);
+			tegra_dc_writel(dc, 0xffff04, DC_WIN_BLEND_2WIN_Y);
+			tegra_dc_writel(dc, 0xffff04, DC_WIN_BLEND_3WIN_XY);
+		} else {
+			tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_X);
+			tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_Y);
+			tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_3WIN_XY);
+		}
 		break;
 	}
 
@@ -433,7 +440,6 @@ static void tegra_plane_atomic_update(struct drm_plane *plane,
 	struct tegra_plane_state *state = to_tegra_plane_state(plane->state);
 	struct tegra_dc *dc = to_tegra_dc(plane->state->crtc);
 	struct drm_framebuffer *fb = plane->state->fb;
-	struct tegra_plane *p = to_tegra_plane(plane);
 	struct tegra_dc_window window;
 	unsigned int i;
 
@@ -475,7 +481,7 @@ static void tegra_plane_atomic_update(struct drm_plane *plane,
 			window.stride[i] = fb->pitches[i];
 	}
 
-	tegra_dc_setup_window(dc, p->index, &window);
+	tegra_dc_setup_window(dc, plane, &window);
 }
 
 static const struct drm_plane_helper_funcs tegra_plane_helper_funcs = {
@@ -706,6 +712,14 @@ static const u32 tegra20_overlay_formats[] = {
 	DRM_FORMAT_YUV422,
 };
 
+static const u32 tegra20_overlay_cursor_formats[] = {
+	DRM_FORMAT_ARGB4444,
+	DRM_FORMAT_ARGB1555,
+	DRM_FORMAT_RGBA5551,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_ARGB8888,
+};
+
 static const u32 tegra114_overlay_formats[] = {
 	DRM_FORMAT_ARGB4444,
 	DRM_FORMAT_ARGB1555,
@@ -765,9 +779,11 @@ static const u32 tegra124_overlay_formats[] = {
 
 static struct drm_plane *tegra_dc_overlay_plane_create(struct drm_device *drm,
 						       struct tegra_dc *dc,
-						       unsigned int index)
+						       unsigned int index,
+						       bool cursor)
 {
 	struct tegra_plane *plane;
+	enum drm_plane_type type;
 	unsigned int num_formats;
 	const u32 *formats;
 	int err;
@@ -781,13 +797,19 @@ static struct drm_plane *tegra_dc_overlay_plane_create(struct drm_device *drm,
 	plane->index = index;
 	plane->depth = 0;
 
-	num_formats = dc->soc->num_overlay_formats;
-	formats = dc->soc->overlay_formats;
+	if (cursor) {
+		num_formats = dc->soc->num_overlay_cursor_formats;
+		formats = dc->soc->overlay_cursor_formats;
+		type = DRM_PLANE_TYPE_CURSOR;
+	} else {
+		num_formats = dc->soc->num_overlay_formats;
+		formats = dc->soc->overlay_formats;
+		type = DRM_PLANE_TYPE_OVERLAY;
+	}
 
 	err = drm_universal_plane_init(drm, &plane->base, 1 << dc->pipe,
 				       &tegra_plane_funcs, formats,
-				       num_formats, NULL,
-				       DRM_PLANE_TYPE_OVERLAY, NULL);
+				       num_formats, NULL, type, NULL);
 	if (err < 0) {
 		kfree(plane);
 		return ERR_PTR(err);
@@ -836,14 +858,18 @@ static struct drm_plane *tegra_dc_add_planes(struct drm_device *drm,
 					     struct tegra_dc *dc)
 {
 	struct drm_plane *plane, *primary;
+	unsigned int planes_num = 2;
 	unsigned int i;
 
 	primary = tegra_primary_plane_create(drm, dc);
 	if (IS_ERR(primary))
 		return primary;
 
-	for (i = 0; i < 2; i++) {
-		plane = tegra_dc_overlay_plane_create(drm, dc, 1 + i);
+	if (!dc->soc->supports_cursor)
+		planes_num--;
+
+	for (i = 0; i < planes_num; i++) {
+		plane = tegra_dc_overlay_plane_create(drm, dc, 1 + i, false);
 		if (IS_ERR(plane)) {
 			/* XXX tegra_plane_destroy() */
 			drm_plane_cleanup(primary);
@@ -1766,6 +1792,13 @@ static int tegra_dc_init(struct host1x_client *client)
 			err = PTR_ERR(cursor);
 			goto cleanup;
 		}
+	} else {
+		/* trade overlay for RGBA cursor plane on older Tegra's */
+		cursor = tegra_dc_overlay_plane_create(drm, dc, 2, true);
+		if (IS_ERR(cursor)) {
+			err = PTR_ERR(cursor);
+			goto cleanup;
+		}
 	}
 
 	err = drm_crtc_init_with_planes(drm, &dc->base, primary, cursor,
@@ -1855,6 +1888,9 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
 	.primary_formats = tegra20_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
 	.overlay_formats = tegra20_overlay_formats,
+	.num_overlay_cursor_formats =
+				ARRAY_SIZE(tegra20_overlay_cursor_formats),
+	.overlay_cursor_formats = tegra20_overlay_cursor_formats,
 	.supports_opaque_formats = false,
 };
 
@@ -1871,6 +1907,9 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
 	.primary_formats = tegra20_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
 	.overlay_formats = tegra20_overlay_formats,
+	.num_overlay_cursor_formats =
+				ARRAY_SIZE(tegra20_overlay_cursor_formats),
+	.overlay_cursor_formats = tegra20_overlay_cursor_formats,
 	.supports_opaque_formats = false,
 };
 
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 3a66a1127ee7..e7cdf1d0729d 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -65,6 +65,8 @@ struct tegra_dc_soc_info {
 	unsigned int num_primary_formats;
 	const u32 *overlay_formats;
 	unsigned int num_overlay_formats;
+	const u32 *overlay_cursor_formats;
+	unsigned int num_overlay_cursor_formats;
 	bool supports_opaque_formats;
 };
 
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 3/5] drm/tegra: Trade overlay plane for cursor on older Tegra's
@ 2017-12-20 15:46     ` Dmitry Osipenko
  0 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 15:46 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

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 <digetx@gmail.com>
---
 drivers/gpu/drm/tegra/dc.c | 75 +++++++++++++++++++++++++++++++++++-----------
 drivers/gpu/drm/tegra/dc.h |  2 ++
 2 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 460510366bb8..eaff8757bbe0 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -125,9 +125,10 @@ static inline u32 compute_initial_dda(unsigned int in)
 	return dfixed_frac(inf);
 }
 
-static void tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
+static void tegra_dc_setup_window(struct tegra_dc *dc, struct drm_plane *plane,
 				  const struct tegra_dc_window *window)
 {
+	struct tegra_plane *p = to_tegra_plane(plane);
 	unsigned h_offset, v_offset, h_size, v_size, h_dda, v_dda, bpp;
 	unsigned long value, flags;
 	bool yuv, planar;
@@ -144,7 +145,7 @@ static void tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
 
 	spin_lock_irqsave(&dc->lock, flags);
 
-	value = WINDOW_A_SELECT << index;
+	value = WINDOW_A_SELECT << p->index;
 	tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
 
 	tegra_dc_writel(dc, window->format, DC_WIN_COLOR_DEPTH);
@@ -275,23 +276,29 @@ static void tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
 	tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_NOKEY);
 	tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_1WIN);
 
-	switch (index) {
+	switch (p->index) {
 	case 0:
 		tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_2WIN_X);
-		tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_2WIN_Y);
+		tegra_dc_writel(dc, 0x000008, DC_WIN_BLEND_2WIN_Y);
 		tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_3WIN_XY);
 		break;
 
 	case 1:
 		tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_X);
-		tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_2WIN_Y);
-		tegra_dc_writel(dc, 0x000000, DC_WIN_BLEND_3WIN_XY);
+		tegra_dc_writel(dc, 0x000008, DC_WIN_BLEND_2WIN_Y);
+		tegra_dc_writel(dc, 0x000008, DC_WIN_BLEND_3WIN_XY);
 		break;
 
 	case 2:
-		tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_X);
-		tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_Y);
-		tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_3WIN_XY);
+		if (plane->type == DRM_PLANE_TYPE_CURSOR) {
+			tegra_dc_writel(dc, 0xffff04, DC_WIN_BLEND_2WIN_X);
+			tegra_dc_writel(dc, 0xffff04, DC_WIN_BLEND_2WIN_Y);
+			tegra_dc_writel(dc, 0xffff04, DC_WIN_BLEND_3WIN_XY);
+		} else {
+			tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_X);
+			tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_2WIN_Y);
+			tegra_dc_writel(dc, 0xffff00, DC_WIN_BLEND_3WIN_XY);
+		}
 		break;
 	}
 
@@ -433,7 +440,6 @@ static void tegra_plane_atomic_update(struct drm_plane *plane,
 	struct tegra_plane_state *state = to_tegra_plane_state(plane->state);
 	struct tegra_dc *dc = to_tegra_dc(plane->state->crtc);
 	struct drm_framebuffer *fb = plane->state->fb;
-	struct tegra_plane *p = to_tegra_plane(plane);
 	struct tegra_dc_window window;
 	unsigned int i;
 
@@ -475,7 +481,7 @@ static void tegra_plane_atomic_update(struct drm_plane *plane,
 			window.stride[i] = fb->pitches[i];
 	}
 
-	tegra_dc_setup_window(dc, p->index, &window);
+	tegra_dc_setup_window(dc, plane, &window);
 }
 
 static const struct drm_plane_helper_funcs tegra_plane_helper_funcs = {
@@ -706,6 +712,14 @@ static const u32 tegra20_overlay_formats[] = {
 	DRM_FORMAT_YUV422,
 };
 
+static const u32 tegra20_overlay_cursor_formats[] = {
+	DRM_FORMAT_ARGB4444,
+	DRM_FORMAT_ARGB1555,
+	DRM_FORMAT_RGBA5551,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_ARGB8888,
+};
+
 static const u32 tegra114_overlay_formats[] = {
 	DRM_FORMAT_ARGB4444,
 	DRM_FORMAT_ARGB1555,
@@ -765,9 +779,11 @@ static const u32 tegra124_overlay_formats[] = {
 
 static struct drm_plane *tegra_dc_overlay_plane_create(struct drm_device *drm,
 						       struct tegra_dc *dc,
-						       unsigned int index)
+						       unsigned int index,
+						       bool cursor)
 {
 	struct tegra_plane *plane;
+	enum drm_plane_type type;
 	unsigned int num_formats;
 	const u32 *formats;
 	int err;
@@ -781,13 +797,19 @@ static struct drm_plane *tegra_dc_overlay_plane_create(struct drm_device *drm,
 	plane->index = index;
 	plane->depth = 0;
 
-	num_formats = dc->soc->num_overlay_formats;
-	formats = dc->soc->overlay_formats;
+	if (cursor) {
+		num_formats = dc->soc->num_overlay_cursor_formats;
+		formats = dc->soc->overlay_cursor_formats;
+		type = DRM_PLANE_TYPE_CURSOR;
+	} else {
+		num_formats = dc->soc->num_overlay_formats;
+		formats = dc->soc->overlay_formats;
+		type = DRM_PLANE_TYPE_OVERLAY;
+	}
 
 	err = drm_universal_plane_init(drm, &plane->base, 1 << dc->pipe,
 				       &tegra_plane_funcs, formats,
-				       num_formats, NULL,
-				       DRM_PLANE_TYPE_OVERLAY, NULL);
+				       num_formats, NULL, type, NULL);
 	if (err < 0) {
 		kfree(plane);
 		return ERR_PTR(err);
@@ -836,14 +858,18 @@ static struct drm_plane *tegra_dc_add_planes(struct drm_device *drm,
 					     struct tegra_dc *dc)
 {
 	struct drm_plane *plane, *primary;
+	unsigned int planes_num = 2;
 	unsigned int i;
 
 	primary = tegra_primary_plane_create(drm, dc);
 	if (IS_ERR(primary))
 		return primary;
 
-	for (i = 0; i < 2; i++) {
-		plane = tegra_dc_overlay_plane_create(drm, dc, 1 + i);
+	if (!dc->soc->supports_cursor)
+		planes_num--;
+
+	for (i = 0; i < planes_num; i++) {
+		plane = tegra_dc_overlay_plane_create(drm, dc, 1 + i, false);
 		if (IS_ERR(plane)) {
 			/* XXX tegra_plane_destroy() */
 			drm_plane_cleanup(primary);
@@ -1766,6 +1792,13 @@ static int tegra_dc_init(struct host1x_client *client)
 			err = PTR_ERR(cursor);
 			goto cleanup;
 		}
+	} else {
+		/* trade overlay for RGBA cursor plane on older Tegra's */
+		cursor = tegra_dc_overlay_plane_create(drm, dc, 2, true);
+		if (IS_ERR(cursor)) {
+			err = PTR_ERR(cursor);
+			goto cleanup;
+		}
 	}
 
 	err = drm_crtc_init_with_planes(drm, &dc->base, primary, cursor,
@@ -1855,6 +1888,9 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
 	.primary_formats = tegra20_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
 	.overlay_formats = tegra20_overlay_formats,
+	.num_overlay_cursor_formats =
+				ARRAY_SIZE(tegra20_overlay_cursor_formats),
+	.overlay_cursor_formats = tegra20_overlay_cursor_formats,
 	.supports_opaque_formats = false,
 };
 
@@ -1871,6 +1907,9 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
 	.primary_formats = tegra20_primary_formats,
 	.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
 	.overlay_formats = tegra20_overlay_formats,
+	.num_overlay_cursor_formats =
+				ARRAY_SIZE(tegra20_overlay_cursor_formats),
+	.overlay_cursor_formats = tegra20_overlay_cursor_formats,
 	.supports_opaque_formats = false,
 };
 
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 3a66a1127ee7..e7cdf1d0729d 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -65,6 +65,8 @@ struct tegra_dc_soc_info {
 	unsigned int num_primary_formats;
 	const u32 *overlay_formats;
 	unsigned int num_overlay_formats;
+	const u32 *overlay_cursor_formats;
+	unsigned int num_overlay_cursor_formats;
 	bool supports_opaque_formats;
 };
 
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 4/5] drm/tegra: gem: Correct iommu_map_sg() error checking
  2017-12-20 15:46 ` Dmitry Osipenko
                   ` (2 preceding siblings ...)
  (?)
@ 2017-12-20 15:46 ` Dmitry Osipenko
       [not found]   ` <1daf8bfe7c4ef37a4d5d6d5f87eae36425a3df14.1513783739.git.digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  -1 siblings, 1 reply; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 15:46 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

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 <digetx@gmail.com>
---
 drivers/gpu/drm/tegra/gem.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index ab1e53d434e8..49b9bf28f872 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -114,7 +114,7 @@ static const struct host1x_bo_ops tegra_bo_ops = {
 static int tegra_bo_iommu_map(struct tegra_drm *tegra, struct tegra_bo *bo)
 {
 	int prot = IOMMU_READ | IOMMU_WRITE;
-	ssize_t err;
+	int err;
 
 	if (bo->mm)
 		return -EBUSY;
@@ -128,22 +128,21 @@ static int tegra_bo_iommu_map(struct tegra_drm *tegra, struct tegra_bo *bo)
 	err = drm_mm_insert_node_generic(&tegra->mm,
 					 bo->mm, bo->gem.size, PAGE_SIZE, 0, 0);
 	if (err < 0) {
-		dev_err(tegra->drm->dev, "out of I/O virtual memory: %zd\n",
+		dev_err(tegra->drm->dev, "out of I/O virtual memory: %d\n",
 			err);
 		goto unlock;
 	}
 
 	bo->paddr = bo->mm->start;
 
-	err = iommu_map_sg(tegra->domain, bo->paddr, bo->sgt->sgl,
-			   bo->sgt->nents, prot);
-	if (err < 0) {
-		dev_err(tegra->drm->dev, "failed to map buffer: %zd\n", err);
+	bo->size = iommu_map_sg(tegra->domain, bo->paddr, bo->sgt->sgl,
+				bo->sgt->nents, prot);
+	if (!bo->size) {
+		dev_err(tegra->drm->dev, "failed to map buffer\n");
+		err = -ENOMEM;
 		goto remove;
 	}
 
-	bo->size = err;
-
 	mutex_unlock(&tegra->mm_lock);
 
 	return 0;
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH v3 5/5] drm/tegra: Correct timeout in tegra_syncpt_wait
  2017-12-20 15:46 ` Dmitry Osipenko
                   ` (3 preceding siblings ...)
  (?)
@ 2017-12-20 15:46 ` Dmitry Osipenko
  2017-12-20 20:17     ` Thierry Reding
  -1 siblings, 1 reply; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 15:46 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

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

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/gpu/drm/tegra/drm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index bb98336fa8d7..57396388341b 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -629,7 +629,8 @@ static int tegra_syncpt_wait(struct drm_device *drm, void *data,
 	if (!sp)
 		return -EINVAL;
 
-	return host1x_syncpt_wait(sp, args->thresh, args->timeout,
+	return host1x_syncpt_wait(sp, args->thresh,
+				  msecs_to_jiffies(args->timeout),
 				  &args->value);
 }
 
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  2017-12-20 15:46     ` Dmitry Osipenko
@ 2017-12-20 18:01         ` Thierry Reding
  -1 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 18:01 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 12197 bytes --]

On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
> 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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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 <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 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(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index bc65c314e00f..07c687d7f615 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -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 */
+	if (window->alpha) {
+		background |= BLEND_CONTROL_DEPENDENT;
+		foreground |= BLEND_CONTROL_ALPHA;
+	}
+
 	/*
 	 * 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,
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 0c887b53cb10..03b04a7e2814 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -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
 
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index 5f6289c4e56a..001cb77e2f59 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -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);
 
diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 215051579b3a..dfe991e3d48d 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -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);
diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h
index 429a086ffe88..06d8b0005fe7 100644
--- a/drivers/gpu/drm/tegra/plane.h
+++ b/drivers/gpu/drm/tegra/plane.h
@@ -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);
-- 
2.15.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
@ 2017-12-20 18:01         ` Thierry Reding
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 18:01 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 12079 bytes --]

On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
> 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 <digetx@gmail.com>
> ---
>  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 <treding@nvidia.com>
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 <digetx@gmail.com>
Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 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(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index bc65c314e00f..07c687d7f615 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -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 */
+	if (window->alpha) {
+		background |= BLEND_CONTROL_DEPENDENT;
+		foreground |= BLEND_CONTROL_ALPHA;
+	}
+
 	/*
 	 * 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,
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 0c887b53cb10..03b04a7e2814 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -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
 
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index 5f6289c4e56a..001cb77e2f59 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -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);
 
diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 215051579b3a..dfe991e3d48d 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -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);
diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h
index 429a086ffe88..06d8b0005fe7 100644
--- a/drivers/gpu/drm/tegra/plane.h
+++ b/drivers/gpu/drm/tegra/plane.h
@@ -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);
-- 
2.15.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  2017-12-20 18:01         ` Thierry Reding
@ 2017-12-20 20:01           ` Dmitry Osipenko
  -1 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 20:01 UTC (permalink / raw)
  To: Thierry Reding
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 20.12.2017 21:01, Thierry Reding wrote:
> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>> 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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  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 <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 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.

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.

> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
> the opaque formats are now supported.
> 
> Reported-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  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(-)
> 
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index bc65c314e00f..07c687d7f615 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -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 */
> +	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?

> +
>  	/*
>  	 * 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,
> diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
> index 0c887b53cb10..03b04a7e2814 100644
> --- a/drivers/gpu/drm/tegra/dc.h
> +++ b/drivers/gpu/drm/tegra/dc.h
> @@ -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
>  
> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
> index 5f6289c4e56a..001cb77e2f59 100644
> --- a/drivers/gpu/drm/tegra/fb.c
> +++ b/drivers/gpu/drm/tegra/fb.c
> @@ -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);
>  
> diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
> index 215051579b3a..dfe991e3d48d 100644
> --- a/drivers/gpu/drm/tegra/plane.c
> +++ b/drivers/gpu/drm/tegra/plane.c
> @@ -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);
> diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h
> index 429a086ffe88..06d8b0005fe7 100644
> --- a/drivers/gpu/drm/tegra/plane.h
> +++ b/drivers/gpu/drm/tegra/plane.h
> @@ -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);
> 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
@ 2017-12-20 20:01           ` Dmitry Osipenko
  0 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 20:01 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

On 20.12.2017 21:01, Thierry Reding wrote:
> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>> 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 <digetx@gmail.com>
>> ---
>>  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 <treding@nvidia.com>
> 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.

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.

> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
> the opaque formats are now supported.
> 
> Reported-by: Dmitry Osipenko <digetx@gmail.com>
> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  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(-)
> 
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index bc65c314e00f..07c687d7f615 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -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 */
> +	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?

> +
>  	/*
>  	 * 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,
> diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
> index 0c887b53cb10..03b04a7e2814 100644
> --- a/drivers/gpu/drm/tegra/dc.h
> +++ b/drivers/gpu/drm/tegra/dc.h
> @@ -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
>  
> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
> index 5f6289c4e56a..001cb77e2f59 100644
> --- a/drivers/gpu/drm/tegra/fb.c
> +++ b/drivers/gpu/drm/tegra/fb.c
> @@ -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);
>  
> diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
> index 215051579b3a..dfe991e3d48d 100644
> --- a/drivers/gpu/drm/tegra/plane.c
> +++ b/drivers/gpu/drm/tegra/plane.c
> @@ -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);
> diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h
> index 429a086ffe88..06d8b0005fe7 100644
> --- a/drivers/gpu/drm/tegra/plane.h
> +++ b/drivers/gpu/drm/tegra/plane.h
> @@ -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);
> 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  2017-12-20 20:01           ` Dmitry Osipenko
@ 2017-12-20 20:16               ` Thierry Reding
  -1 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 20:16 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 6019 bytes --]

On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
> On 20.12.2017 21:01, Thierry Reding wrote:
> > On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
> >> 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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> ---
> >>  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 <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > 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.
> 
> 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.
> 
> > Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
> > the opaque formats are now supported.
> > 
> > Reported-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> >  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(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> > index bc65c314e00f..07c687d7f615 100644
> > --- a/drivers/gpu/drm/tegra/dc.c
> > +++ b/drivers/gpu/drm/tegra/dc.c
> > @@ -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 */
> > +	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).

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

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

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
@ 2017-12-20 20:16               ` Thierry Reding
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 20:16 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 5901 bytes --]

On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
> On 20.12.2017 21:01, Thierry Reding wrote:
> > On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
> >> 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 <digetx@gmail.com>
> >> ---
> >>  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 <treding@nvidia.com>
> > 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.
> 
> 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.
> 
> > Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
> > the opaque formats are now supported.
> > 
> > Reported-by: Dmitry Osipenko <digetx@gmail.com>
> > Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  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(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> > index bc65c314e00f..07c687d7f615 100644
> > --- a/drivers/gpu/drm/tegra/dc.c
> > +++ b/drivers/gpu/drm/tegra/dc.c
> > @@ -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 */
> > +	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).

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

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

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 1/5] drm/tegra: dc: Link DC1 to DC0 on Tegra20
  2017-12-20 15:46 ` [PATCH v3 1/5] drm/tegra: dc: Link DC1 to DC0 on Tegra20 Dmitry Osipenko
@ 2017-12-20 20:17     ` Thierry Reding
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 20:17 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: linux-tegra, linux-kernel, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 615 bytes --]

On Wed, Dec 20, 2017 at 06:46:10PM +0300, Dmitry Osipenko wrote:
> 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 <digetx@gmail.com>
> ---
> 
> 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(-)

Applied, thanks.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 1/5] drm/tegra: dc: Link DC1 to DC0 on Tegra20
@ 2017-12-20 20:17     ` Thierry Reding
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 20:17 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 615 bytes --]

On Wed, Dec 20, 2017 at 06:46:10PM +0300, Dmitry Osipenko wrote:
> 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 <digetx@gmail.com>
> ---
> 
> 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(-)

Applied, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 4/5] drm/tegra: gem: Correct iommu_map_sg() error checking
  2017-12-20 15:46 ` [PATCH v3 4/5] drm/tegra: gem: Correct iommu_map_sg() error checking Dmitry Osipenko
@ 2017-12-20 20:17       ` Thierry Reding
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 20:17 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 415 bytes --]

On Wed, Dec 20, 2017 at 06:46:13PM +0300, Dmitry Osipenko wrote:
> 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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/gpu/drm/tegra/gem.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)

Applied, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 4/5] drm/tegra: gem: Correct iommu_map_sg() error checking
@ 2017-12-20 20:17       ` Thierry Reding
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 20:17 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 385 bytes --]

On Wed, Dec 20, 2017 at 06:46:13PM +0300, Dmitry Osipenko wrote:
> 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 <digetx@gmail.com>
> ---
>  drivers/gpu/drm/tegra/gem.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)

Applied, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 5/5] drm/tegra: Correct timeout in tegra_syncpt_wait
  2017-12-20 15:46 ` [PATCH v3 5/5] drm/tegra: Correct timeout in tegra_syncpt_wait Dmitry Osipenko
@ 2017-12-20 20:17     ` Thierry Reding
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 20:17 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: linux-tegra, linux-kernel, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 346 bytes --]

On Wed, Dec 20, 2017 at 06:46:14PM +0300, Dmitry Osipenko wrote:
> host1x_syncpt_wait() takes timeout value in jiffies, but DRM passes it in
> milliseconds.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/gpu/drm/tegra/drm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied, thanks.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 5/5] drm/tegra: Correct timeout in tegra_syncpt_wait
@ 2017-12-20 20:17     ` Thierry Reding
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 20:17 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

On Wed, Dec 20, 2017 at 06:46:14PM +0300, Dmitry Osipenko wrote:
> host1x_syncpt_wait() takes timeout value in jiffies, but DRM passes it in
> milliseconds.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/gpu/drm/tegra/drm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 3/5] drm/tegra: Trade overlay plane for cursor on older Tegra's
  2017-12-20 15:46     ` Dmitry Osipenko
@ 2017-12-20 20:19         ` Thierry Reding
  -1 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 20:19 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

On Wed, Dec 20, 2017 at 06:46:12PM +0300, Dmitry Osipenko wrote:
> 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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 3/5] drm/tegra: Trade overlay plane for cursor on older Tegra's
@ 2017-12-20 20:19         ` Thierry Reding
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 20:19 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]

On Wed, Dec 20, 2017 at 06:46:12PM +0300, Dmitry Osipenko wrote:
> 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 <digetx@gmail.com>
> ---
>  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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 3/5] drm/tegra: Trade overlay plane for cursor on older Tegra's
  2017-12-20 20:19         ` Thierry Reding
@ 2017-12-20 20:28           ` Dmitry Osipenko
  -1 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 20:28 UTC (permalink / raw)
  To: Thierry Reding
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 20.12.2017 23:19, Thierry Reding wrote:
> On Wed, Dec 20, 2017 at 06:46:12PM +0300, Dmitry Osipenko wrote:
>> 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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  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.

I'll rebase and re-test this patch on top of your "Implement legacy blending"
patch ASAP. I'm fine with the deferring if it won't workout well.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 3/5] drm/tegra: Trade overlay plane for cursor on older Tegra's
@ 2017-12-20 20:28           ` Dmitry Osipenko
  0 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 20:28 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

On 20.12.2017 23:19, Thierry Reding wrote:
> On Wed, Dec 20, 2017 at 06:46:12PM +0300, Dmitry Osipenko wrote:
>> 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 <digetx@gmail.com>
>> ---
>>  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.

I'll rebase and re-test this patch on top of your "Implement legacy blending"
patch ASAP. I'm fine with the deferring if it won't workout well.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  2017-12-20 20:16               ` Thierry Reding
@ 2017-12-20 21:05                 ` Dmitry Osipenko
  -1 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 21:05 UTC (permalink / raw)
  To: Thierry Reding
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 20.12.2017 23:16, Thierry Reding wrote:
> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
>> On 20.12.2017 21:01, Thierry Reding wrote:
>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>>>> 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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>>  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 <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> 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.
>>
>> 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.
>>
>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
>>> the opaque formats are now supported.
>>>
>>> Reported-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>>> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  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(-)
>>>
>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>> index bc65c314e00f..07c687d7f615 100644
>>> --- a/drivers/gpu/drm/tegra/dc.c
>>> +++ b/drivers/gpu/drm/tegra/dc.c
>>> @@ -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 */
>>> +	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).

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.

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

But maybe I'm missing something. Thanks for sharing the test, I'll try it out.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
@ 2017-12-20 21:05                 ` Dmitry Osipenko
  0 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 21:05 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

On 20.12.2017 23:16, Thierry Reding wrote:
> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
>> On 20.12.2017 21:01, Thierry Reding wrote:
>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>>>> 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 <digetx@gmail.com>
>>>> ---
>>>>  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 <treding@nvidia.com>
>>> 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.
>>
>> 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.
>>
>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
>>> the opaque formats are now supported.
>>>
>>> Reported-by: Dmitry Osipenko <digetx@gmail.com>
>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>  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(-)
>>>
>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>> index bc65c314e00f..07c687d7f615 100644
>>> --- a/drivers/gpu/drm/tegra/dc.c
>>> +++ b/drivers/gpu/drm/tegra/dc.c
>>> @@ -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 */
>>> +	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).

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.

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

But maybe I'm missing something. Thanks for sharing the test, I'll try it out.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  2017-12-20 21:05                 ` Dmitry Osipenko
@ 2017-12-20 22:02                   ` Thierry Reding
  -1 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 22:02 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: linux-tegra, linux-kernel, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 5620 bytes --]

On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
> On 20.12.2017 23:16, Thierry Reding wrote:
> > On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
> >> On 20.12.2017 21:01, Thierry Reding wrote:
> >>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
> >>>> 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 <digetx@gmail.com>
> >>>> ---
> >>>>  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 <treding@nvidia.com>
> >>> 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.
> >>
> >> 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.
> >>
> >>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
> >>> the opaque formats are now supported.
> >>>
> >>> Reported-by: Dmitry Osipenko <digetx@gmail.com>
> >>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> >>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >>> ---
> >>>  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(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> >>> index bc65c314e00f..07c687d7f615 100644
> >>> --- a/drivers/gpu/drm/tegra/dc.c
> >>> +++ b/drivers/gpu/drm/tegra/dc.c
> >>> @@ -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 */
> >>> +	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).
> 
> 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.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
@ 2017-12-20 22:02                   ` Thierry Reding
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-20 22:02 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 5620 bytes --]

On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
> On 20.12.2017 23:16, Thierry Reding wrote:
> > On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
> >> On 20.12.2017 21:01, Thierry Reding wrote:
> >>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
> >>>> 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 <digetx@gmail.com>
> >>>> ---
> >>>>  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 <treding@nvidia.com>
> >>> 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.
> >>
> >> 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.
> >>
> >>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
> >>> the opaque formats are now supported.
> >>>
> >>> Reported-by: Dmitry Osipenko <digetx@gmail.com>
> >>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> >>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >>> ---
> >>>  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(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> >>> index bc65c314e00f..07c687d7f615 100644
> >>> --- a/drivers/gpu/drm/tegra/dc.c
> >>> +++ b/drivers/gpu/drm/tegra/dc.c
> >>> @@ -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 */
> >>> +	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).
> 
> 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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  2017-12-20 22:02                   ` Thierry Reding
@ 2017-12-20 22:23                     ` Dmitry Osipenko
  -1 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 22:23 UTC (permalink / raw)
  To: Thierry Reding
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 21.12.2017 01:02, Thierry Reding wrote:
> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
>> On 20.12.2017 23:16, Thierry Reding wrote:
>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
>>>> On 20.12.2017 21:01, Thierry Reding wrote:
>>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>>>>>> 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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>> ---
>>>>>>  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 <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>> 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.
>>>>
>>>> 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.
>>>>
>>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
>>>>> the opaque formats are now supported.
>>>>>
>>>>> Reported-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>>>>> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>> ---
>>>>>  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(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>>>> index bc65c314e00f..07c687d7f615 100644
>>>>> --- a/drivers/gpu/drm/tegra/dc.c
>>>>> +++ b/drivers/gpu/drm/tegra/dc.c
>>>>> @@ -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 */
>>>>> +	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).
>>
>> 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.

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

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
@ 2017-12-20 22:23                     ` Dmitry Osipenko
  0 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 22:23 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

On 21.12.2017 01:02, Thierry Reding wrote:
> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
>> On 20.12.2017 23:16, Thierry Reding wrote:
>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
>>>> On 20.12.2017 21:01, Thierry Reding wrote:
>>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>>>>>> 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 <digetx@gmail.com>
>>>>>> ---
>>>>>>  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 <treding@nvidia.com>
>>>>> 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.
>>>>
>>>> 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.
>>>>
>>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
>>>>> the opaque formats are now supported.
>>>>>
>>>>> Reported-by: Dmitry Osipenko <digetx@gmail.com>
>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>>> ---
>>>>>  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(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>>>> index bc65c314e00f..07c687d7f615 100644
>>>>> --- a/drivers/gpu/drm/tegra/dc.c
>>>>> +++ b/drivers/gpu/drm/tegra/dc.c
>>>>> @@ -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 */
>>>>> +	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).
>>
>> 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.

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

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  2017-12-20 22:23                     ` Dmitry Osipenko
@ 2017-12-20 22:38                         ` Dmitry Osipenko
  -1 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 22:38 UTC (permalink / raw)
  To: Thierry Reding
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 21.12.2017 01:23, Dmitry Osipenko wrote:
> On 21.12.2017 01:02, Thierry Reding wrote:
>> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
>>> On 20.12.2017 23:16, Thierry Reding wrote:
>>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
>>>>> On 20.12.2017 21:01, Thierry Reding wrote:
>>>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>>>>>>> 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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>>> ---
>>>>>>>  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 <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>>> 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.
>>>>>
>>>>> 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.
>>>>>
>>>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
>>>>>> the opaque formats are now supported.
>>>>>>
>>>>>> Reported-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>>>>>> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>>> ---
>>>>>>  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(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>>>>> index bc65c314e00f..07c687d7f615 100644
>>>>>> --- a/drivers/gpu/drm/tegra/dc.c
>>>>>> +++ b/drivers/gpu/drm/tegra/dc.c
>>>>>> @@ -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 */
>>>>>> +	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).
>>>
>>> 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.
> 
> Okay, please push patch to the public repo once it is ready and let me know.
> I'll rebase cursor patch on it.
> 
>> 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..

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.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
@ 2017-12-20 22:38                         ` Dmitry Osipenko
  0 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-20 22:38 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

On 21.12.2017 01:23, Dmitry Osipenko wrote:
> On 21.12.2017 01:02, Thierry Reding wrote:
>> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
>>> On 20.12.2017 23:16, Thierry Reding wrote:
>>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
>>>>> On 20.12.2017 21:01, Thierry Reding wrote:
>>>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>>>>>>> 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 <digetx@gmail.com>
>>>>>>> ---
>>>>>>>  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 <treding@nvidia.com>
>>>>>> 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.
>>>>>
>>>>> 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.
>>>>>
>>>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
>>>>>> the opaque formats are now supported.
>>>>>>
>>>>>> Reported-by: Dmitry Osipenko <digetx@gmail.com>
>>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>>>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>>>> ---
>>>>>>  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(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>>>>> index bc65c314e00f..07c687d7f615 100644
>>>>>> --- a/drivers/gpu/drm/tegra/dc.c
>>>>>> +++ b/drivers/gpu/drm/tegra/dc.c
>>>>>> @@ -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 */
>>>>>> +	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).
>>>
>>> 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.
> 
> Okay, please push patch to the public repo once it is ready and let me know.
> I'll rebase cursor patch on it.
> 
>> 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..

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.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  2017-12-20 22:38                         ` Dmitry Osipenko
@ 2017-12-21 14:10                             ` Thierry Reding
  -1 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-21 14:10 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 7214 bytes --]

On Thu, Dec 21, 2017 at 01:38:31AM +0300, Dmitry Osipenko wrote:
> On 21.12.2017 01:23, Dmitry Osipenko wrote:
> > On 21.12.2017 01:02, Thierry Reding wrote:
> >> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
> >>> On 20.12.2017 23:16, Thierry Reding wrote:
> >>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
> >>>>> On 20.12.2017 21:01, Thierry Reding wrote:
> >>>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
> >>>>>>> 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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>>>>>> ---
> >>>>>>>  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 <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> >>>>>> 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.
> >>>>>
> >>>>> 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.
> >>>>>
> >>>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
> >>>>>> the opaque formats are now supported.
> >>>>>>
> >>>>>> Reported-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> >>>>>> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> >>>>>> ---
> >>>>>>  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(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> >>>>>> index bc65c314e00f..07c687d7f615 100644
> >>>>>> --- a/drivers/gpu/drm/tegra/dc.c
> >>>>>> +++ b/drivers/gpu/drm/tegra/dc.c
> >>>>>> @@ -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 */
> >>>>>> +	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).
> >>>
> >>> 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.
> > 
> > Okay, please push patch to the public repo once it is ready and let me know.
> > I'll rebase cursor patch on it.
> > 
> >> 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..
> 
> 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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
@ 2017-12-21 14:10                             ` Thierry Reding
  0 siblings, 0 replies; 37+ messages in thread
From: Thierry Reding @ 2017-12-21 14:10 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 7096 bytes --]

On Thu, Dec 21, 2017 at 01:38:31AM +0300, Dmitry Osipenko wrote:
> On 21.12.2017 01:23, Dmitry Osipenko wrote:
> > On 21.12.2017 01:02, Thierry Reding wrote:
> >> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
> >>> On 20.12.2017 23:16, Thierry Reding wrote:
> >>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
> >>>>> On 20.12.2017 21:01, Thierry Reding wrote:
> >>>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
> >>>>>>> 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 <digetx@gmail.com>
> >>>>>>> ---
> >>>>>>>  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 <treding@nvidia.com>
> >>>>>> 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.
> >>>>>
> >>>>> 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.
> >>>>>
> >>>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
> >>>>>> the opaque formats are now supported.
> >>>>>>
> >>>>>> Reported-by: Dmitry Osipenko <digetx@gmail.com>
> >>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
> >>>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >>>>>> ---
> >>>>>>  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(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> >>>>>> index bc65c314e00f..07c687d7f615 100644
> >>>>>> --- a/drivers/gpu/drm/tegra/dc.c
> >>>>>> +++ b/drivers/gpu/drm/tegra/dc.c
> >>>>>> @@ -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 */
> >>>>>> +	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).
> >>>
> >>> 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.
> > 
> > Okay, please push patch to the public repo once it is ready and let me know.
> > I'll rebase cursor patch on it.
> > 
> >> 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..
> 
> 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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
  2017-12-21 14:10                             ` Thierry Reding
@ 2017-12-21 14:38                               ` Dmitry Osipenko
  -1 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-21 14:38 UTC (permalink / raw)
  To: Thierry Reding
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 21.12.2017 17:10, Thierry Reding wrote:
> On Thu, Dec 21, 2017 at 01:38:31AM +0300, Dmitry Osipenko wrote:
>> On 21.12.2017 01:23, Dmitry Osipenko wrote:
>>> On 21.12.2017 01:02, Thierry Reding wrote:
>>>> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
>>>>> On 20.12.2017 23:16, Thierry Reding wrote:
>>>>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
>>>>>>> On 20.12.2017 21:01, Thierry Reding wrote:
>>>>>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>>>>>>>>> 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 <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>>>>> ---
>>>>>>>>>  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 <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>>>>> 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.
>>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
>>>>>>>> the opaque formats are now supported.
>>>>>>>>
>>>>>>>> Reported-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>>>>>>>> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>>>>> ---
>>>>>>>>  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(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>>>>>>> index bc65c314e00f..07c687d7f615 100644
>>>>>>>> --- a/drivers/gpu/drm/tegra/dc.c
>>>>>>>> +++ b/drivers/gpu/drm/tegra/dc.c
>>>>>>>> @@ -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 */
>>>>>>>> +	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).
>>>>>
>>>>> 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.
>>>
>>> Okay, please push patch to the public repo once it is ready and let me know.
>>> I'll rebase cursor patch on it.
>>>
>>>> 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..
>>
>> 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.

Awesome, I'll test it soon.

Please don't forget to correct the improper changes that you made to my "Link
DC1 to DC0 on Tegra20" patch. See my replies to the mail from kbuild test bot.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30
@ 2017-12-21 14:38                               ` Dmitry Osipenko
  0 siblings, 0 replies; 37+ messages in thread
From: Dmitry Osipenko @ 2017-12-21 14:38 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

On 21.12.2017 17:10, Thierry Reding wrote:
> On Thu, Dec 21, 2017 at 01:38:31AM +0300, Dmitry Osipenko wrote:
>> On 21.12.2017 01:23, Dmitry Osipenko wrote:
>>> On 21.12.2017 01:02, Thierry Reding wrote:
>>>> On Thu, Dec 21, 2017 at 12:05:40AM +0300, Dmitry Osipenko wrote:
>>>>> On 20.12.2017 23:16, Thierry Reding wrote:
>>>>>> On Wed, Dec 20, 2017 at 11:01:49PM +0300, Dmitry Osipenko wrote:
>>>>>>> On 20.12.2017 21:01, Thierry Reding wrote:
>>>>>>>> On Wed, Dec 20, 2017 at 06:46:11PM +0300, Dmitry Osipenko wrote:
>>>>>>>>> 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 <digetx@gmail.com>
>>>>>>>>> ---
>>>>>>>>>  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 <treding@nvidia.com>
>>>>>>>> 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.
>>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>>>> Reverts commit 71835caa00e8 ("drm/tegra: fb: Force alpha formats") since
>>>>>>>> the opaque formats are now supported.
>>>>>>>>
>>>>>>>> Reported-by: Dmitry Osipenko <digetx@gmail.com>
>>>>>>>> Fixes: 7772fdaef939 ("drm/tegra: Support ARGB and ABGR formats")
>>>>>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>>>>>> ---
>>>>>>>>  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(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>>>>>>>> index bc65c314e00f..07c687d7f615 100644
>>>>>>>> --- a/drivers/gpu/drm/tegra/dc.c
>>>>>>>> +++ b/drivers/gpu/drm/tegra/dc.c
>>>>>>>> @@ -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 */
>>>>>>>> +	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).
>>>>>
>>>>> 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.
>>>
>>> Okay, please push patch to the public repo once it is ready and let me know.
>>> I'll rebase cursor patch on it.
>>>
>>>> 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..
>>
>> 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.

Awesome, I'll test it soon.

Please don't forget to correct the improper changes that you made to my "Link
DC1 to DC0 on Tegra20" patch. See my replies to the mail from kbuild test bot.

^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2017-12-21 14:38 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-20 15:46 [PATCH v3 0/5] Some corrections and improvement for Tegra DRM Dmitry Osipenko
2017-12-20 15:46 ` Dmitry Osipenko
2017-12-20 15:46 ` [PATCH v3 1/5] drm/tegra: dc: Link DC1 to DC0 on Tegra20 Dmitry Osipenko
2017-12-20 20:17   ` Thierry Reding
2017-12-20 20:17     ` Thierry Reding
     [not found] ` <cover.1513783738.git.digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-12-20 15:46   ` [PATCH v3 2/5] drm/tegra: Restore opaque and drop alpha formats on Tegra20/30 Dmitry Osipenko
2017-12-20 15:46     ` Dmitry Osipenko
     [not found]     ` <0476e28d70b47ec599fb65d1cfa457276629de27.1513783738.git.digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-12-20 18:01       ` Thierry Reding
2017-12-20 18:01         ` Thierry Reding
2017-12-20 20:01         ` Dmitry Osipenko
2017-12-20 20:01           ` Dmitry Osipenko
     [not found]           ` <3924358d-ae3b-78a4-1227-164435add8d1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-12-20 20:16             ` Thierry Reding
2017-12-20 20:16               ` Thierry Reding
2017-12-20 21:05               ` Dmitry Osipenko
2017-12-20 21:05                 ` Dmitry Osipenko
2017-12-20 22:02                 ` Thierry Reding
2017-12-20 22:02                   ` Thierry Reding
2017-12-20 22:23                   ` Dmitry Osipenko
2017-12-20 22:23                     ` Dmitry Osipenko
     [not found]                     ` <2c52becd-f20e-599b-c421-bf4b18dda6bf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-12-20 22:38                       ` Dmitry Osipenko
2017-12-20 22:38                         ` Dmitry Osipenko
     [not found]                         ` <a6644fc8-9c22-4b6b-1fec-82a7636bda65-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-12-21 14:10                           ` Thierry Reding
2017-12-21 14:10                             ` Thierry Reding
2017-12-21 14:38                             ` Dmitry Osipenko
2017-12-21 14:38                               ` Dmitry Osipenko
2017-12-20 15:46   ` [PATCH v3 3/5] drm/tegra: Trade overlay plane for cursor on older Tegra's Dmitry Osipenko
2017-12-20 15:46     ` Dmitry Osipenko
     [not found]     ` <fd0034ee935cc17915f4189ee65865c974f34d98.1513783739.git.digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-12-20 20:19       ` Thierry Reding
2017-12-20 20:19         ` Thierry Reding
2017-12-20 20:28         ` Dmitry Osipenko
2017-12-20 20:28           ` Dmitry Osipenko
2017-12-20 15:46 ` [PATCH v3 4/5] drm/tegra: gem: Correct iommu_map_sg() error checking Dmitry Osipenko
     [not found]   ` <1daf8bfe7c4ef37a4d5d6d5f87eae36425a3df14.1513783739.git.digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-12-20 20:17     ` Thierry Reding
2017-12-20 20:17       ` Thierry Reding
2017-12-20 15:46 ` [PATCH v3 5/5] drm/tegra: Correct timeout in tegra_syncpt_wait Dmitry Osipenko
2017-12-20 20:17   ` Thierry Reding
2017-12-20 20:17     ` Thierry Reding

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.