LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v1 0/3] Couple more DRM plane features on Tegra
@ 2018-05-04  0:08 Dmitry Osipenko
  2018-05-04  0:08 ` [PATCH v1 1/3] drm/tegra: dc: Enable plane scaling filters Dmitry Osipenko
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2018-05-04  0:08 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

Hi,

This series improves DRM plane support by supporting zPos on older Tegra's
and enabling plane scaling filters (up to Tegra210).

Dmitry Osipenko (3):
  drm/tegra: dc: Enable plane scaling filters
  drm/tegra: plane: Implement zPos plane property for older Tegra's
  drm/tegra: dc: Rename supports_blending to has_legacy_blending

 drivers/gpu/drm/tegra/dc.c    | 205 ++++++++++++++++++++++++++--------
 drivers/gpu/drm/tegra/dc.h    |   9 +-
 drivers/gpu/drm/tegra/plane.c | 193 +++++++++++++++++++++++---------
 drivers/gpu/drm/tegra/plane.h |  13 ++-
 4 files changed, 313 insertions(+), 107 deletions(-)

-- 
2.17.0

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

* [PATCH v1 1/3] drm/tegra: dc: Enable plane scaling filters
  2018-05-04  0:08 [PATCH v1 0/3] Couple more DRM plane features on Tegra Dmitry Osipenko
@ 2018-05-04  0:08 ` Dmitry Osipenko
  2018-05-04 11:10   ` Thierry Reding
  2018-05-04  0:08 ` [PATCH v1 2/3] drm/tegra: plane: Implement zPos plane property for older Tegra's Dmitry Osipenko
  2018-05-04  0:08 ` [PATCH v1 3/3] drm/tegra: dc: Rename supports_blending to has_legacy_blending Dmitry Osipenko
  2 siblings, 1 reply; 10+ messages in thread
From: Dmitry Osipenko @ 2018-05-04  0:08 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

Currently resized plane produces a "pixelated" image which doesn't look
nice, especially in a case of a video overlay. Enable scaling filters that
significantly improve image quality of a scaled overlay.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/gpu/drm/tegra/dc.c | 51 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tegra/dc.h |  7 ++++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 9f83a65b5ea9..2e81142281c3 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -361,6 +361,47 @@ static void tegra_dc_setup_window(struct tegra_plane *plane,
 	if (window->bottom_up)
 		value |= V_DIRECTION;
 
+	if (!(dc->soc->has_win_a_without_filter && plane->index == 0) &&
+	    !(window->src.w == window->dst.w)) {
+		/*
+		 * Enable horizontal 6-tap filter and set filtering
+		 * coefficients to the default values defined in TRM.
+		 */
+		tegra_plane_writel(plane, 0x00008000, DC_WIN_H_FILTER_P(0));
+		tegra_plane_writel(plane, 0x3e087ce1, DC_WIN_H_FILTER_P(1));
+		tegra_plane_writel(plane, 0x3b117ac1, DC_WIN_H_FILTER_P(2));
+		tegra_plane_writel(plane, 0x591b73aa, DC_WIN_H_FILTER_P(3));
+		tegra_plane_writel(plane, 0x57256d9a, DC_WIN_H_FILTER_P(4));
+		tegra_plane_writel(plane, 0x552f668b, DC_WIN_H_FILTER_P(5));
+		tegra_plane_writel(plane, 0x73385e8b, DC_WIN_H_FILTER_P(6));
+		tegra_plane_writel(plane, 0x72435583, DC_WIN_H_FILTER_P(7));
+		tegra_plane_writel(plane, 0x714c4c8b, DC_WIN_H_FILTER_P(8));
+		tegra_plane_writel(plane, 0x70554393, DC_WIN_H_FILTER_P(9));
+		tegra_plane_writel(plane, 0x715e389b, DC_WIN_H_FILTER_P(10));
+		tegra_plane_writel(plane, 0x71662faa, DC_WIN_H_FILTER_P(11));
+		tegra_plane_writel(plane, 0x536d25ba, DC_WIN_H_FILTER_P(12));
+		tegra_plane_writel(plane, 0x55731bca, DC_WIN_H_FILTER_P(13));
+		tegra_plane_writel(plane, 0x387a11d9, DC_WIN_H_FILTER_P(14));
+		tegra_plane_writel(plane, 0x3c7c08f1, DC_WIN_H_FILTER_P(15));
+
+		value |= H_FILTER;
+	}
+
+	if (!(dc->soc->has_win_a_without_filter && plane->index == 0) &&
+	    !(dc->soc->has_win_c_without_vert_filter && plane->index == 2) &&
+	    !(window->src.h == window->dst.h)) {
+		unsigned int i, k;
+
+		/*
+		 * Enable vertical 2-tap filter and set filtering
+		 * coefficients to the default values defined in TRM.
+		 */
+		for (i = 0, k = 128; i < 16; i++, k -= 8)
+			tegra_plane_writel(plane, k, DC_WIN_V_FILTER_P(i));
+
+		value |= V_FILTER;
+	}
+
 	tegra_plane_writel(plane, value, DC_WIN_WIN_OPTIONS);
 
 	if (dc->soc->supports_blending)
@@ -1978,6 +2019,8 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
 	.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
 	.overlay_formats = tegra20_overlay_formats,
 	.modifiers = tegra20_modifiers,
+	.has_win_a_without_filter = true,
+	.has_win_c_without_vert_filter = true,
 };
 
 static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
@@ -1995,6 +2038,8 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
 	.num_overlay_formats = ARRAY_SIZE(tegra20_overlay_formats),
 	.overlay_formats = tegra20_overlay_formats,
 	.modifiers = tegra20_modifiers,
+	.has_win_a_without_filter = false,
+	.has_win_c_without_vert_filter = false,
 };
 
 static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
@@ -2012,6 +2057,8 @@ static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
 	.num_overlay_formats = ARRAY_SIZE(tegra114_overlay_formats),
 	.overlay_formats = tegra114_overlay_formats,
 	.modifiers = tegra20_modifiers,
+	.has_win_a_without_filter = false,
+	.has_win_c_without_vert_filter = false,
 };
 
 static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
@@ -2029,6 +2076,8 @@ static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
 	.num_overlay_formats = ARRAY_SIZE(tegra124_overlay_formats),
 	.overlay_formats = tegra124_overlay_formats,
 	.modifiers = tegra124_modifiers,
+	.has_win_a_without_filter = false,
+	.has_win_c_without_vert_filter = false,
 };
 
 static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
@@ -2046,6 +2095,8 @@ static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
 	.num_overlay_formats = ARRAY_SIZE(tegra114_overlay_formats),
 	.overlay_formats = tegra114_overlay_formats,
 	.modifiers = tegra124_modifiers,
+	.has_win_a_without_filter = false,
+	.has_win_c_without_vert_filter = false,
 };
 
 static const struct tegra_windowgroup_soc tegra186_dc_wgrps[] = {
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index d2b50d32de4d..5c3d1d6faa3b 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -67,6 +67,8 @@ struct tegra_dc_soc_info {
 	const u32 *overlay_formats;
 	unsigned int num_overlay_formats;
 	const u64 *modifiers;
+	bool has_win_a_without_filter;
+	bool has_win_c_without_vert_filter;
 };
 
 struct tegra_dc {
@@ -553,6 +555,9 @@ int tegra_dc_rgb_exit(struct tegra_dc *dc);
 #define  THREAD_NUM(x) (((x) & 0x1f) << 1)
 #define  THREAD_GROUP_ENABLE (1 << 0)
 
+#define DC_WIN_H_FILTER_P(p)			(0x601 + p)
+#define DC_WIN_V_FILTER_P(p)			(0x619 + p)
+
 #define DC_WIN_CSC_YOF				0x611
 #define DC_WIN_CSC_KYRGB			0x612
 #define DC_WIN_CSC_KUR				0x613
@@ -566,6 +571,8 @@ int tegra_dc_rgb_exit(struct tegra_dc *dc);
 #define H_DIRECTION  (1 <<  0)
 #define V_DIRECTION  (1 <<  2)
 #define COLOR_EXPAND (1 <<  6)
+#define H_FILTER     (1 <<  8)
+#define V_FILTER     (1 << 10)
 #define CSC_ENABLE   (1 << 18)
 #define WIN_ENABLE   (1 << 30)
 
-- 
2.17.0

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

* [PATCH v1 2/3] drm/tegra: plane: Implement zPos plane property for older Tegra's
  2018-05-04  0:08 [PATCH v1 0/3] Couple more DRM plane features on Tegra Dmitry Osipenko
  2018-05-04  0:08 ` [PATCH v1 1/3] drm/tegra: dc: Enable plane scaling filters Dmitry Osipenko
@ 2018-05-04  0:08 ` Dmitry Osipenko
  2018-05-04 12:15   ` Thierry Reding
  2018-05-04  0:08 ` [PATCH v1 3/3] drm/tegra: dc: Rename supports_blending to has_legacy_blending Dmitry Osipenko
  2 siblings, 1 reply; 10+ messages in thread
From: Dmitry Osipenko @ 2018-05-04  0:08 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

Older Tegra's do not support planes z position handling in hardware,
but HW provides knobs for zPos implementation in software.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/gpu/drm/tegra/dc.c    | 134 ++++++++++++++++-------
 drivers/gpu/drm/tegra/plane.c | 193 ++++++++++++++++++++++++----------
 drivers/gpu/drm/tegra/plane.h |  13 ++-
 3 files changed, 244 insertions(+), 96 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 2e81142281c3..ba5481cd470d 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -162,29 +162,90 @@ static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
 	u32 foreground = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255) |
 			 BLEND_COLOR_KEY_NONE;
 	u32 blendnokey = BLEND_WEIGHT1(255) | BLEND_WEIGHT0(255);
+	u32 blending[2];
 	struct tegra_plane_state *state;
 	unsigned int i;
 
+	/* disable blending for non-overlapping case */
+	tegra_plane_writel(plane, blendnokey, DC_WIN_BLEND_NOKEY);
+	tegra_plane_writel(plane, foreground, DC_WIN_BLEND_1WIN);
+
 	state = to_tegra_plane_state(plane->base.state);
 
-	/* alpha contribution is 1 minus sum of overlapping windows */
-	for (i = 0; i < 3; i++) {
-		if (state->dependent[i])
-			background[i] |= BLEND_CONTROL_DEPENDENT;
-	}
+	if (state->opaque) {
+		/*
+		 * Since custom fix-weight blending isn't utilized and weight
+		 * of top window is set to max, we can enforce dependent
+		 * blending which in this case results in transparent bottom
+		 * window if top window is opaque and if top window enables
+		 * alpha blending, then bottom window is getting alpha value
+		 * of 1 minus the sum of alpha components of the overlapping
+		 * plane.
+		 */
+		background[0] |= BLEND_CONTROL_DEPENDENT;
+		background[1] |= BLEND_CONTROL_DEPENDENT;
 
-	/* enable alpha blending if pixel format has an alpha component */
-	if (!state->opaque)
+		/*
+		 * The region where three windows overlap is the intersection
+		 * of the two regions where two windows overlap. It contributes
+		 * to the area if all of the windows on top of it have an alpha
+		 * component.
+		 */
+		switch (state->base.normalized_zpos) {
+		case 0:
+			if (state->blending[0].alpha &&
+			    state->blending[1].alpha)
+				background[2] |= BLEND_CONTROL_DEPENDENT;
+			break;
+
+		case 1:
+			background[2] |= BLEND_CONTROL_DEPENDENT;
+			break;
+		}
+	} else {
+		/*
+		 * Enable alpha blending if pixel format has an alpha
+		 * component.
+		 */
 		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, blendnokey, DC_WIN_BLEND_NOKEY);
-	tegra_plane_writel(plane, foreground, DC_WIN_BLEND_1WIN);
+		/*
+		 * If any of the windows on top of this window is opaque, it
+		 * will completely conceal this window within that area. If
+		 * top window has an alpha component, it is blended over the
+		 * bottom window.
+		 */
+		for (i = 0; i < 2; i++) {
+			if (state->blending[i].alpha &&
+			    state->blending[i].top)
+				background[i] |= BLEND_CONTROL_DEPENDENT;
+		}
+
+		switch (state->base.normalized_zpos) {
+		case 0:
+			if (state->blending[0].alpha &&
+			    state->blending[1].alpha)
+				background[2] |= BLEND_CONTROL_DEPENDENT;
+			break;
+
+		case 1:
+			/*
+			 * When both middle and topmost windows have an alpha,
+			 * these windows a mixed together and then the result
+			 * is blended over the bottom window.
+			 */
+			if ((state->blending[0].alpha &&
+			     state->blending[0].top))
+				background[2] |= BLEND_CONTROL_ALPHA;
 
-	switch (plane->index) {
+			if (state->blending[1].alpha &&
+			    state->blending[1].top)
+				background[2] |= BLEND_CONTROL_ALPHA;
+			break;
+		}
+	}
+
+	switch (state->base.normalized_zpos) {
 	case 0:
 		tegra_plane_writel(plane, background[0], DC_WIN_BLEND_2WIN_X);
 		tegra_plane_writel(plane, background[1], DC_WIN_BLEND_2WIN_Y);
@@ -192,8 +253,21 @@ static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
 		break;
 
 	case 1:
-		tegra_plane_writel(plane, foreground, DC_WIN_BLEND_2WIN_X);
-		tegra_plane_writel(plane, background[1], DC_WIN_BLEND_2WIN_Y);
+		/*
+		 * If window B / C is topmost, then X / Y registers are
+		 * matching the order of blending[...] state indices,
+		 * otherwise a swap is required.
+		 */
+		if (!state->blending[0].top && state->blending[1].top) {
+			blending[0] = foreground;
+			blending[1] = background[1];
+		} else {
+			blending[0] = background[0];
+			blending[1] = foreground;
+		}
+
+		tegra_plane_writel(plane, blending[0], DC_WIN_BLEND_2WIN_X);
+		tegra_plane_writel(plane, blending[1], DC_WIN_BLEND_2WIN_Y);
 		tegra_plane_writel(plane, background[2], DC_WIN_BLEND_3WIN_XY);
 		break;
 
@@ -495,14 +569,14 @@ 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, &format,
+	err = tegra_plane_format(state->fb->format->format,
+				 &plane_state->format,
 				 &plane_state->swap);
 	if (err < 0)
 		return err;
@@ -514,21 +588,11 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
 	 * be emulated by disabling alpha blending for the plane.
 	 */
 	if (!dc->soc->supports_blending) {
-		if (!tegra_plane_format_has_alpha(format)) {
-			err = tegra_plane_format_get_alpha(format, &format);
-			if (err < 0)
-				return err;
-
-			plane_state->opaque = true;
-		} else {
-			plane_state->opaque = false;
-		}
-
-		tegra_plane_check_dependent(tegra, plane_state);
+		err = tegra_plane_setup_legacy_state(tegra, plane_state);
+		if (err < 0)
+			return err;
 	}
 
-	plane_state->format = format;
-
 	err = tegra_fb_get_tiling(state->fb, tiling);
 	if (err < 0)
 		return err;
@@ -680,9 +744,7 @@ static struct drm_plane *tegra_primary_plane_create(struct drm_device *drm,
 	}
 
 	drm_plane_helper_add(&plane->base, &tegra_plane_helper_funcs);
-
-	if (dc->soc->supports_blending)
-		drm_plane_create_zpos_property(&plane->base, 0, 0, 255);
+	drm_plane_create_zpos_property(&plane->base, plane->index, 0, 255);
 
 	return &plane->base;
 }
@@ -959,9 +1021,7 @@ static struct drm_plane *tegra_dc_overlay_plane_create(struct drm_device *drm,
 	}
 
 	drm_plane_helper_add(&plane->base, &tegra_plane_helper_funcs);
-
-	if (dc->soc->supports_blending)
-		drm_plane_create_zpos_property(&plane->base, 0, 0, 255);
+	drm_plane_create_zpos_property(&plane->base, plane->index, 0, 255);
 
 	return &plane->base;
 }
diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 176ef46c615c..0406c2ef432c 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -23,6 +23,7 @@ static void tegra_plane_destroy(struct drm_plane *plane)
 
 static void tegra_plane_reset(struct drm_plane *plane)
 {
+	struct tegra_plane *p = to_tegra_plane(plane);
 	struct tegra_plane_state *state;
 
 	if (plane->state)
@@ -35,6 +36,8 @@ static void tegra_plane_reset(struct drm_plane *plane)
 	if (state) {
 		plane->state = &state->base;
 		plane->state->plane = plane;
+		plane->state->zpos = p->index;
+		plane->state->normalized_zpos = p->index;
 	}
 }
 
@@ -55,8 +58,8 @@ tegra_plane_atomic_duplicate_state(struct drm_plane *plane)
 	copy->swap = state->swap;
 	copy->opaque = state->opaque;
 
-	for (i = 0; i < 3; i++)
-		copy->dependent[i] = state->dependent[i];
+	for (i = 0; i < 2; i++)
+		copy->blending[i] = state->blending[i];
 
 	return &copy->base;
 }
@@ -267,24 +270,8 @@ static bool __drm_format_has_alpha(u32 format)
 	return false;
 }
 
-/*
- * This is applicable to Tegra20 and Tegra30 only where the opaque formats can
- * be emulated using the alpha formats and alpha blending disabled.
- */
-bool tegra_plane_format_has_alpha(unsigned int format)
-{
-	switch (format) {
-	case WIN_COLOR_DEPTH_B5G5R5A1:
-	case WIN_COLOR_DEPTH_A1B5G5R5:
-	case WIN_COLOR_DEPTH_R8G8B8A8:
-	case WIN_COLOR_DEPTH_B8G8R8A8:
-		return true;
-	}
-
-	return false;
-}
-
-int tegra_plane_format_get_alpha(unsigned int opaque, unsigned int *alpha)
+static int tegra_plane_format_get_alpha(unsigned int opaque,
+					unsigned int *alpha)
 {
 	if (tegra_plane_format_is_yuv(opaque, NULL)) {
 		*alpha = opaque;
@@ -316,6 +303,67 @@ int tegra_plane_format_get_alpha(unsigned int opaque, unsigned int *alpha)
 	return -EINVAL;
 }
 
+/*
+ * This is applicable to Tegra20 and Tegra30 only where the opaque formats can
+ * be emulated using the alpha formats and alpha blending disabled.
+ */
+static int tegra_plane_setup_opacity(struct tegra_plane *tegra,
+				     struct tegra_plane_state *state)
+{
+	unsigned int format;
+	int err;
+
+	switch (state->format) {
+	case WIN_COLOR_DEPTH_B5G5R5A1:
+	case WIN_COLOR_DEPTH_A1B5G5R5:
+	case WIN_COLOR_DEPTH_R8G8B8A8:
+	case WIN_COLOR_DEPTH_B8G8R8A8:
+		state->opaque = false;
+		break;
+
+	default:
+		err = tegra_plane_format_get_alpha(state->format, &format);
+		if (err < 0)
+			return err;
+
+		state->format = format;
+		state->opaque = true;
+		break;
+	}
+
+	return 0;
+}
+
+static int tegra_plane_check_transparency(struct tegra_plane *tegra,
+					  struct tegra_plane_state *state)
+{
+	struct drm_plane_state *old, *plane_state;
+	struct drm_plane *plane;
+
+	old = drm_atomic_get_old_plane_state(state->base.state, &tegra->base);
+
+	/* check if zpos / transparency changed */
+	if (old->normalized_zpos == state->base.normalized_zpos &&
+	    to_tegra_plane_state(old)->opaque == state->opaque)
+		return 0;
+
+	/* include all sibling planes into this commit */
+	drm_for_each_plane(plane, tegra->base.dev) {
+		struct tegra_plane *p = to_tegra_plane(plane);
+
+		/* skip this plane and planes on different CRTCs */
+		if (p == tegra || p->dc != tegra->dc)
+			continue;
+
+		plane_state = drm_atomic_get_plane_state(state->base.state,
+							 plane);
+		if (IS_ERR(plane_state))
+			return PTR_ERR(plane_state);
+	}
+
+	return 1;
+}
+
 static unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
 						  struct tegra_plane *other)
 {
@@ -336,61 +384,98 @@ static unsigned int tegra_plane_get_overlap_index(struct tegra_plane *plane,
 	return index;
 }
 
-void tegra_plane_check_dependent(struct tegra_plane *tegra,
-				 struct tegra_plane_state *state)
+static void tegra_plane_update_transparency(struct tegra_plane *tegra,
+					    struct tegra_plane_state *state)
 {
-	struct drm_plane_state *old, *new;
+	struct drm_plane_state *new;
 	struct drm_plane *plane;
-	unsigned int zpos[2];
 	unsigned int i;
 
-	for (i = 0; i < 2; i++)
-		zpos[i] = 0;
-
-	for_each_oldnew_plane_in_state(state->base.state, plane, old, new, i) {
+	for_each_new_plane_in_state(state->base.state, plane, new, i) {
 		struct tegra_plane *p = to_tegra_plane(plane);
 		unsigned index;
 
 		/* skip this plane and planes on different CRTCs */
-		if (p == tegra || new->crtc != state->base.crtc)
+		if (p == tegra || p->dc != tegra->dc)
 			continue;
 
 		index = tegra_plane_get_overlap_index(tegra, p);
 
-		state->dependent[index] = false;
+		if (new->fb && __drm_format_has_alpha(new->fb->format->format))
+			state->blending[index].alpha = true;
+		else
+			state->blending[index].alpha = false;
+
+		if (new->normalized_zpos > state->base.normalized_zpos)
+			state->blending[index].top = true;
+		else
+			state->blending[index].top = false;
 
 		/*
-		 * If any of the other planes is on top of this plane and uses
-		 * a format with an alpha component, mark this plane as being
-		 * dependent, meaning it's alpha value will be 1 minus the sum
-		 * of alpha components of the overlapping planes.
+		 * Missing framebuffer means that plane is disabled, in this
+		 * case mark B / C window as top to be able to differentiate
+		 * windows indices order in regards to zPos for the middle
+		 * window X / Y registers programming.
 		 */
-		if (p->index > tegra->index) {
-			if (__drm_format_has_alpha(new->fb->format->format))
-				state->dependent[index] = true;
-
-			/* keep track of the Z position */
-			zpos[index] = p->index;
-		}
+		if (!new->fb)
+			state->blending[index].top = (index == 1);
 	}
+}
+
+static int tegra_plane_setup_transparency(struct tegra_plane *tegra,
+					  struct tegra_plane_state *state)
+{
+	struct tegra_plane_state *tegra_state;
+	struct drm_plane_state *new;
+	struct drm_plane *plane;
+	int err;
 
 	/*
-	 * The region where three windows overlap is the intersection of the
-	 * two regions where two windows overlap. It contributes to the area
-	 * if any of the windows on top of it have an alpha component.
+	 * If planes zpos / transparency changed, sibling planes blending
+	 * state may require adjustment and in this case they will be included
+	 * into this atom commit, otherwise blending state is unchanged.
 	 */
-	for (i = 0; i < 2; i++)
-		state->dependent[2] = state->dependent[2] ||
-				      state->dependent[i];
+	err = tegra_plane_check_transparency(tegra, state);
+	if (err <= 0)
+		return err;
 
 	/*
-	 * However, if any of the windows on top of this window is opaque, it
-	 * will completely conceal this window within that area, so avoid the
-	 * window from contributing to the area.
+	 * All planes are now in the atomic state, walk them up and update
+	 * transparency state for each plane.
 	 */
-	for (i = 0; i < 2; i++) {
-		if (zpos[i] > tegra->index)
-			state->dependent[2] = state->dependent[2] &&
-					      state->dependent[i];
+	drm_for_each_plane(plane, tegra->base.dev) {
+		struct tegra_plane *p = to_tegra_plane(plane);
+
+		/* skip planes on different CRTCs */
+		if (p->dc != tegra->dc)
+			continue;
+
+		new = drm_atomic_get_new_plane_state(state->base.state, plane);
+		tegra_state = to_tegra_plane_state(new);
+
+		/*
+		 * There is no need to update blending state for the disabled
+		 * plane.
+		 */
+		if (new->fb)
+			tegra_plane_update_transparency(p, tegra_state);
 	}
+
+	return 0;
+}
+
+int tegra_plane_setup_legacy_state(struct tegra_plane *tegra,
+				   struct tegra_plane_state *state)
+{
+	int err;
+
+	err = tegra_plane_setup_opacity(tegra, state);
+	if (err < 0)
+		return err;
+
+	err = tegra_plane_setup_transparency(tegra, state);
+	if (err < 0)
+		return err;
+
+	return 0;
 }
diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h
index 6938719e7e5d..7360ddfafee8 100644
--- a/drivers/gpu/drm/tegra/plane.h
+++ b/drivers/gpu/drm/tegra/plane.h
@@ -34,6 +34,11 @@ static inline struct tegra_plane *to_tegra_plane(struct drm_plane *plane)
 	return container_of(plane, struct tegra_plane, base);
 }
 
+struct tegra_plane_legacy_blending_state {
+	bool alpha;
+	bool top;
+};
+
 struct tegra_plane_state {
 	struct drm_plane_state base;
 
@@ -42,8 +47,8 @@ struct tegra_plane_state {
 	u32 swap;
 
 	/* used for legacy blending support only */
+	struct tegra_plane_legacy_blending_state blending[2];
 	bool opaque;
-	bool dependent[3];
 };
 
 static inline struct tegra_plane_state *
@@ -62,9 +67,7 @@ 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_has_alpha(unsigned int format);
-int tegra_plane_format_get_alpha(unsigned int opaque, unsigned int *alpha);
-void tegra_plane_check_dependent(struct tegra_plane *tegra,
-				 struct tegra_plane_state *state);
+int tegra_plane_setup_legacy_state(struct tegra_plane *tegra,
+				   struct tegra_plane_state *state);
 
 #endif /* TEGRA_PLANE_H */
-- 
2.17.0

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

* [PATCH v1 3/3] drm/tegra: dc: Rename supports_blending to has_legacy_blending
  2018-05-04  0:08 [PATCH v1 0/3] Couple more DRM plane features on Tegra Dmitry Osipenko
  2018-05-04  0:08 ` [PATCH v1 1/3] drm/tegra: dc: Enable plane scaling filters Dmitry Osipenko
  2018-05-04  0:08 ` [PATCH v1 2/3] drm/tegra: plane: Implement zPos plane property for older Tegra's Dmitry Osipenko
@ 2018-05-04  0:08 ` Dmitry Osipenko
  2018-05-04 12:16   ` Thierry Reding
  2 siblings, 1 reply; 10+ messages in thread
From: Dmitry Osipenko @ 2018-05-04  0:08 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

Older Tegra's support blending. Rename SoC info entry supports_blending
to has_legacy_blending to eliminate confusion.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/gpu/drm/tegra/dc.c | 20 ++++++++++----------
 drivers/gpu/drm/tegra/dc.h |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index ba5481cd470d..fe00b3250f6a 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -478,10 +478,10 @@ static void tegra_dc_setup_window(struct tegra_plane *plane,
 
 	tegra_plane_writel(plane, value, DC_WIN_WIN_OPTIONS);
 
-	if (dc->soc->supports_blending)
-		tegra_plane_setup_blending(plane, window);
-	else
+	if (dc->soc->has_legacy_blending)
 		tegra_plane_setup_blending_legacy(plane);
+	else
+		tegra_plane_setup_blending(plane, window);
 }
 
 static const u32 tegra20_primary_formats[] = {
@@ -587,7 +587,7 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
 	 * the corresponding opaque formats. However, the opaque formats can
 	 * be emulated by disabling alpha blending for the plane.
 	 */
-	if (!dc->soc->supports_blending) {
+	if (dc->soc->has_legacy_blending) {
 		err = tegra_plane_setup_legacy_state(tegra, plane_state);
 		if (err < 0)
 			return err;
@@ -2069,7 +2069,7 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
 	.supports_interlacing = false,
 	.supports_cursor = false,
 	.supports_block_linear = false,
-	.supports_blending = false,
+	.has_legacy_blending = true,
 	.pitch_align = 8,
 	.has_powergate = false,
 	.coupled_pm = true,
@@ -2088,7 +2088,7 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
 	.supports_interlacing = false,
 	.supports_cursor = false,
 	.supports_block_linear = false,
-	.supports_blending = false,
+	.has_legacy_blending = true,
 	.pitch_align = 8,
 	.has_powergate = false,
 	.coupled_pm = false,
@@ -2107,7 +2107,7 @@ static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
 	.supports_interlacing = false,
 	.supports_cursor = false,
 	.supports_block_linear = false,
-	.supports_blending = false,
+	.has_legacy_blending = true,
 	.pitch_align = 64,
 	.has_powergate = true,
 	.coupled_pm = false,
@@ -2126,7 +2126,7 @@ static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
 	.supports_interlacing = true,
 	.supports_cursor = true,
 	.supports_block_linear = true,
-	.supports_blending = true,
+	.has_legacy_blending = false,
 	.pitch_align = 64,
 	.has_powergate = true,
 	.coupled_pm = false,
@@ -2145,7 +2145,7 @@ static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
 	.supports_interlacing = true,
 	.supports_cursor = true,
 	.supports_block_linear = true,
-	.supports_blending = true,
+	.has_legacy_blending = false,
 	.pitch_align = 64,
 	.has_powergate = true,
 	.coupled_pm = false,
@@ -2198,7 +2198,7 @@ static const struct tegra_dc_soc_info tegra186_dc_soc_info = {
 	.supports_interlacing = true,
 	.supports_cursor = true,
 	.supports_block_linear = true,
-	.supports_blending = true,
+	.has_legacy_blending = false,
 	.pitch_align = 64,
 	.has_powergate = false,
 	.coupled_pm = false,
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 5c3d1d6faa3b..ca5cac6bf8ea 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -55,7 +55,7 @@ struct tegra_dc_soc_info {
 	bool supports_interlacing;
 	bool supports_cursor;
 	bool supports_block_linear;
-	bool supports_blending;
+	bool has_legacy_blending;
 	unsigned int pitch_align;
 	bool has_powergate;
 	bool coupled_pm;
-- 
2.17.0

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

* Re: [PATCH v1 1/3] drm/tegra: dc: Enable plane scaling filters
  2018-05-04  0:08 ` [PATCH v1 1/3] drm/tegra: dc: Enable plane scaling filters Dmitry Osipenko
@ 2018-05-04 11:10   ` Thierry Reding
  2018-05-04 11:55     ` Dmitry Osipenko
  0 siblings, 1 reply; 10+ messages in thread
From: Thierry Reding @ 2018-05-04 11:10 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

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

On Fri, May 04, 2018 at 03:08:42AM +0300, Dmitry Osipenko wrote:
> Currently resized plane produces a "pixelated" image which doesn't look
> nice, especially in a case of a video overlay. Enable scaling filters that
> significantly improve image quality of a scaled overlay.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/gpu/drm/tegra/dc.c | 51 ++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tegra/dc.h |  7 ++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 9f83a65b5ea9..2e81142281c3 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -361,6 +361,47 @@ static void tegra_dc_setup_window(struct tegra_plane *plane,
>  	if (window->bottom_up)
>  		value |= V_DIRECTION;
>  
> +	if (!(dc->soc->has_win_a_without_filter && plane->index == 0) &&
> +	    !(window->src.w == window->dst.w)) {

I don't know about you, but I get dizzy trying to parse the above. How
about we move this into helpers:

	static bool tegra_plane_has_horizontal_filter(struct tegra_plane *plane)
	{
		struct tegra_dc *dc = plane->dc;

		/* this function should never be called on inactive planes */
		if (WARN_ON(!dc))
			return false;

		if (plane->index == 0 && dc->soc->has_win_a_without_filter)
			return false;

		return true;
	}

Then the above becomes:

	bool scale_x = window->dst.w != window->src.w;

	if (scale_x && tegra_plane_has_horizontal_filter(plane)) {

> +		/*
> +		 * Enable horizontal 6-tap filter and set filtering
> +		 * coefficients to the default values defined in TRM.
> +		 */
> +		tegra_plane_writel(plane, 0x00008000, DC_WIN_H_FILTER_P(0));
> +		tegra_plane_writel(plane, 0x3e087ce1, DC_WIN_H_FILTER_P(1));
> +		tegra_plane_writel(plane, 0x3b117ac1, DC_WIN_H_FILTER_P(2));
> +		tegra_plane_writel(plane, 0x591b73aa, DC_WIN_H_FILTER_P(3));
> +		tegra_plane_writel(plane, 0x57256d9a, DC_WIN_H_FILTER_P(4));
> +		tegra_plane_writel(plane, 0x552f668b, DC_WIN_H_FILTER_P(5));
> +		tegra_plane_writel(plane, 0x73385e8b, DC_WIN_H_FILTER_P(6));
> +		tegra_plane_writel(plane, 0x72435583, DC_WIN_H_FILTER_P(7));
> +		tegra_plane_writel(plane, 0x714c4c8b, DC_WIN_H_FILTER_P(8));
> +		tegra_plane_writel(plane, 0x70554393, DC_WIN_H_FILTER_P(9));
> +		tegra_plane_writel(plane, 0x715e389b, DC_WIN_H_FILTER_P(10));
> +		tegra_plane_writel(plane, 0x71662faa, DC_WIN_H_FILTER_P(11));
> +		tegra_plane_writel(plane, 0x536d25ba, DC_WIN_H_FILTER_P(12));
> +		tegra_plane_writel(plane, 0x55731bca, DC_WIN_H_FILTER_P(13));
> +		tegra_plane_writel(plane, 0x387a11d9, DC_WIN_H_FILTER_P(14));
> +		tegra_plane_writel(plane, 0x3c7c08f1, DC_WIN_H_FILTER_P(15));
> +
> +		value |= H_FILTER;
> +	}
> +
> +	if (!(dc->soc->has_win_a_without_filter && plane->index == 0) &&
> +	    !(dc->soc->has_win_c_without_vert_filter && plane->index == 2) &&
> +	    !(window->src.h == window->dst.h)) {

	static bool tegra_plane_has_vertical_filter(struct tegra_plane *plane)
	{
		struct tegra_dc *dc = plane->dc;

		/* this function should never be called on inactive planes */
		if (WARN_ON(!dc))
			return false;

		if (plane->index == 0 && dc->soc->has_win_a_without_filter)
			return false;

		if (plane->index == 2 && dc->soc->has_win_c_without_vert_filter)
			return false;

		return true;
	}

And the above becomes:

	bool scale_y = window->dst.h != window->src.h;

	if (scale_y && tegra_plane_has_vertical_filter(plane)) {

Thierry

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

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

* Re: [PATCH v1 1/3] drm/tegra: dc: Enable plane scaling filters
  2018-05-04 11:10   ` Thierry Reding
@ 2018-05-04 11:55     ` Dmitry Osipenko
  2018-05-04 12:09       ` Thierry Reding
  0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Osipenko @ 2018-05-04 11:55 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

On 04.05.2018 14:10, Thierry Reding wrote:
> On Fri, May 04, 2018 at 03:08:42AM +0300, Dmitry Osipenko wrote:
>> Currently resized plane produces a "pixelated" image which doesn't look
>> nice, especially in a case of a video overlay. Enable scaling filters that
>> significantly improve image quality of a scaled overlay.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/gpu/drm/tegra/dc.c | 51 ++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/tegra/dc.h |  7 ++++++
>>  2 files changed, 58 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>> index 9f83a65b5ea9..2e81142281c3 100644
>> --- a/drivers/gpu/drm/tegra/dc.c
>> +++ b/drivers/gpu/drm/tegra/dc.c
>> @@ -361,6 +361,47 @@ static void tegra_dc_setup_window(struct tegra_plane *plane,
>>  	if (window->bottom_up)
>>  		value |= V_DIRECTION;
>>  
>> +	if (!(dc->soc->has_win_a_without_filter && plane->index == 0) &&
>> +	    !(window->src.w == window->dst.w)) {
> 
> I don't know about you, but I get dizzy trying to parse the above. How
> about we move this into helpers:

+1

> 	static bool tegra_plane_has_horizontal_filter(struct tegra_plane *plane)
> 	{
> 		struct tegra_dc *dc = plane->dc;
> 
> 		/* this function should never be called on inactive planes */
> 		if (WARN_ON(!dc))
> 			return false;
> 
> 		if (plane->index == 0 && dc->soc->has_win_a_without_filter)
> 			return false;
> 
> 		return true;
> 	}
> 
> Then the above becomes:
> 
> 	bool scale_x = window->dst.w != window->src.w;
> 
> 	if (scale_x && tegra_plane_has_horizontal_filter(plane)) {
> 
>> +		/*
>> +		 * Enable horizontal 6-tap filter and set filtering
>> +		 * coefficients to the default values defined in TRM.
>> +		 */
>> +		tegra_plane_writel(plane, 0x00008000, DC_WIN_H_FILTER_P(0));
>> +		tegra_plane_writel(plane, 0x3e087ce1, DC_WIN_H_FILTER_P(1));
>> +		tegra_plane_writel(plane, 0x3b117ac1, DC_WIN_H_FILTER_P(2));
>> +		tegra_plane_writel(plane, 0x591b73aa, DC_WIN_H_FILTER_P(3));
>> +		tegra_plane_writel(plane, 0x57256d9a, DC_WIN_H_FILTER_P(4));
>> +		tegra_plane_writel(plane, 0x552f668b, DC_WIN_H_FILTER_P(5));
>> +		tegra_plane_writel(plane, 0x73385e8b, DC_WIN_H_FILTER_P(6));
>> +		tegra_plane_writel(plane, 0x72435583, DC_WIN_H_FILTER_P(7));
>> +		tegra_plane_writel(plane, 0x714c4c8b, DC_WIN_H_FILTER_P(8));
>> +		tegra_plane_writel(plane, 0x70554393, DC_WIN_H_FILTER_P(9));
>> +		tegra_plane_writel(plane, 0x715e389b, DC_WIN_H_FILTER_P(10));
>> +		tegra_plane_writel(plane, 0x71662faa, DC_WIN_H_FILTER_P(11));
>> +		tegra_plane_writel(plane, 0x536d25ba, DC_WIN_H_FILTER_P(12));
>> +		tegra_plane_writel(plane, 0x55731bca, DC_WIN_H_FILTER_P(13));
>> +		tegra_plane_writel(plane, 0x387a11d9, DC_WIN_H_FILTER_P(14));
>> +		tegra_plane_writel(plane, 0x3c7c08f1, DC_WIN_H_FILTER_P(15));
>> +
>> +		value |= H_FILTER;
>> +	}
>> +
>> +	if (!(dc->soc->has_win_a_without_filter && plane->index == 0) &&
>> +	    !(dc->soc->has_win_c_without_vert_filter && plane->index == 2) &&
>> +	    !(window->src.h == window->dst.h)) {
> 
> 	static bool tegra_plane_has_vertical_filter(struct tegra_plane *plane)
> 	{
> 		struct tegra_dc *dc = plane->dc;
> 
> 		/* this function should never be called on inactive planes */
> 		if (WARN_ON(!dc))
> 			return false;
> 
> 		if (plane->index == 0 && dc->soc->has_win_a_without_filter)
> 			return false;
> 
> 		if (plane->index == 2 && dc->soc->has_win_c_without_vert_filter)
> 			return false;
> 
> 		return true;
> 	}
> 
> And the above becomes:
> 
> 	bool scale_y = window->dst.h != window->src.h;
> 
> 	if (scale_y && tegra_plane_has_vertical_filter(plane)) {

I'll adjust this patch, thanks.

Maybe it also worth to factor filtering out into a custom DRM property? For
example in a case of libvdpau-tegra we perform the exact same filtering on
blitting YUV to RGB surface (shown as overlay) by GR2D for displaying video
players interface on top of video, this results in a double filtering that has
no visual effect and probably wastes memory bandwidth a tad.

Though I'm not very keen anymore on trying to add custom plane properties after
being previously NAK'ed. We can wrap filtering into a property later if will be
desired, should be good to just have it enabled by default for now.

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

* Re: [PATCH v1 1/3] drm/tegra: dc: Enable plane scaling filters
  2018-05-04 11:55     ` Dmitry Osipenko
@ 2018-05-04 12:09       ` Thierry Reding
  0 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2018-05-04 12:09 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

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

On Fri, May 04, 2018 at 02:55:01PM +0300, Dmitry Osipenko wrote:
> On 04.05.2018 14:10, Thierry Reding wrote:
> > On Fri, May 04, 2018 at 03:08:42AM +0300, Dmitry Osipenko wrote:
> >> Currently resized plane produces a "pixelated" image which doesn't look
> >> nice, especially in a case of a video overlay. Enable scaling filters that
> >> significantly improve image quality of a scaled overlay.
> >>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---
> >>  drivers/gpu/drm/tegra/dc.c | 51 ++++++++++++++++++++++++++++++++++++++
> >>  drivers/gpu/drm/tegra/dc.h |  7 ++++++
> >>  2 files changed, 58 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> >> index 9f83a65b5ea9..2e81142281c3 100644
> >> --- a/drivers/gpu/drm/tegra/dc.c
> >> +++ b/drivers/gpu/drm/tegra/dc.c
> >> @@ -361,6 +361,47 @@ static void tegra_dc_setup_window(struct tegra_plane *plane,
> >>  	if (window->bottom_up)
> >>  		value |= V_DIRECTION;
> >>  
> >> +	if (!(dc->soc->has_win_a_without_filter && plane->index == 0) &&
> >> +	    !(window->src.w == window->dst.w)) {
> > 
> > I don't know about you, but I get dizzy trying to parse the above. How
> > about we move this into helpers:
> 
> +1
> 
> > 	static bool tegra_plane_has_horizontal_filter(struct tegra_plane *plane)
> > 	{
> > 		struct tegra_dc *dc = plane->dc;
> > 
> > 		/* this function should never be called on inactive planes */
> > 		if (WARN_ON(!dc))
> > 			return false;
> > 
> > 		if (plane->index == 0 && dc->soc->has_win_a_without_filter)
> > 			return false;
> > 
> > 		return true;
> > 	}
> > 
> > Then the above becomes:
> > 
> > 	bool scale_x = window->dst.w != window->src.w;
> > 
> > 	if (scale_x && tegra_plane_has_horizontal_filter(plane)) {
> > 
> >> +		/*
> >> +		 * Enable horizontal 6-tap filter and set filtering
> >> +		 * coefficients to the default values defined in TRM.
> >> +		 */
> >> +		tegra_plane_writel(plane, 0x00008000, DC_WIN_H_FILTER_P(0));
> >> +		tegra_plane_writel(plane, 0x3e087ce1, DC_WIN_H_FILTER_P(1));
> >> +		tegra_plane_writel(plane, 0x3b117ac1, DC_WIN_H_FILTER_P(2));
> >> +		tegra_plane_writel(plane, 0x591b73aa, DC_WIN_H_FILTER_P(3));
> >> +		tegra_plane_writel(plane, 0x57256d9a, DC_WIN_H_FILTER_P(4));
> >> +		tegra_plane_writel(plane, 0x552f668b, DC_WIN_H_FILTER_P(5));
> >> +		tegra_plane_writel(plane, 0x73385e8b, DC_WIN_H_FILTER_P(6));
> >> +		tegra_plane_writel(plane, 0x72435583, DC_WIN_H_FILTER_P(7));
> >> +		tegra_plane_writel(plane, 0x714c4c8b, DC_WIN_H_FILTER_P(8));
> >> +		tegra_plane_writel(plane, 0x70554393, DC_WIN_H_FILTER_P(9));
> >> +		tegra_plane_writel(plane, 0x715e389b, DC_WIN_H_FILTER_P(10));
> >> +		tegra_plane_writel(plane, 0x71662faa, DC_WIN_H_FILTER_P(11));
> >> +		tegra_plane_writel(plane, 0x536d25ba, DC_WIN_H_FILTER_P(12));
> >> +		tegra_plane_writel(plane, 0x55731bca, DC_WIN_H_FILTER_P(13));
> >> +		tegra_plane_writel(plane, 0x387a11d9, DC_WIN_H_FILTER_P(14));
> >> +		tegra_plane_writel(plane, 0x3c7c08f1, DC_WIN_H_FILTER_P(15));
> >> +
> >> +		value |= H_FILTER;
> >> +	}
> >> +
> >> +	if (!(dc->soc->has_win_a_without_filter && plane->index == 0) &&
> >> +	    !(dc->soc->has_win_c_without_vert_filter && plane->index == 2) &&
> >> +	    !(window->src.h == window->dst.h)) {
> > 
> > 	static bool tegra_plane_has_vertical_filter(struct tegra_plane *plane)
> > 	{
> > 		struct tegra_dc *dc = plane->dc;
> > 
> > 		/* this function should never be called on inactive planes */
> > 		if (WARN_ON(!dc))
> > 			return false;
> > 
> > 		if (plane->index == 0 && dc->soc->has_win_a_without_filter)
> > 			return false;
> > 
> > 		if (plane->index == 2 && dc->soc->has_win_c_without_vert_filter)
> > 			return false;
> > 
> > 		return true;
> > 	}
> > 
> > And the above becomes:
> > 
> > 	bool scale_y = window->dst.h != window->src.h;
> > 
> > 	if (scale_y && tegra_plane_has_vertical_filter(plane)) {
> 
> I'll adjust this patch, thanks.
> 
> Maybe it also worth to factor filtering out into a custom DRM property? For
> example in a case of libvdpau-tegra we perform the exact same filtering on
> blitting YUV to RGB surface (shown as overlay) by GR2D for displaying video
> players interface on top of video, this results in a double filtering that has
> no visual effect and probably wastes memory bandwidth a tad.
> 
> Though I'm not very keen anymore on trying to add custom plane properties after
> being previously NAK'ed. We can wrap filtering into a property later if will be
> desired, should be good to just have it enabled by default for now.

Agreed. Let's keep this enabled by default and then address the corner
cases separately.

Thierry

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

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

* Re: [PATCH v1 2/3] drm/tegra: plane: Implement zPos plane property for older Tegra's
  2018-05-04  0:08 ` [PATCH v1 2/3] drm/tegra: plane: Implement zPos plane property for older Tegra's Dmitry Osipenko
@ 2018-05-04 12:15   ` Thierry Reding
  2018-05-04 12:32     ` Dmitry Osipenko
  0 siblings, 1 reply; 10+ messages in thread
From: Thierry Reding @ 2018-05-04 12:15 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

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

On Fri, May 04, 2018 at 03:08:43AM +0300, Dmitry Osipenko wrote:
> Older Tegra's do not support planes z position handling in hardware,
> but HW provides knobs for zPos implementation in software.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/gpu/drm/tegra/dc.c    | 134 ++++++++++++++++-------
>  drivers/gpu/drm/tegra/plane.c | 193 ++++++++++++++++++++++++----------
>  drivers/gpu/drm/tegra/plane.h |  13 ++-
>  3 files changed, 244 insertions(+), 96 deletions(-)

This is obviously a lot to review, but it looks pretty good. On minor
comment below.

> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 2e81142281c3..ba5481cd470d 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -162,29 +162,90 @@ static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
[...]
> +		case 1:
> +			/*
> +			 * When both middle and topmost windows have an alpha,
> +			 * these windows a mixed together and then the result
> +			 * is blended over the bottom window.
> +			 */
> +			if ((state->blending[0].alpha &&
> +			     state->blending[0].top))

There seems to be one pair of parentheses too much here.

Thierry

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

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

* Re: [PATCH v1 3/3] drm/tegra: dc: Rename supports_blending to has_legacy_blending
  2018-05-04  0:08 ` [PATCH v1 3/3] drm/tegra: dc: Rename supports_blending to has_legacy_blending Dmitry Osipenko
@ 2018-05-04 12:16   ` Thierry Reding
  0 siblings, 0 replies; 10+ messages in thread
From: Thierry Reding @ 2018-05-04 12:16 UTC (permalink / raw)
  To: Dmitry Osipenko; +Cc: dri-devel, linux-tegra, linux-kernel

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

On Fri, May 04, 2018 at 03:08:44AM +0300, Dmitry Osipenko wrote:
> Older Tegra's support blending. Rename SoC info entry supports_blending
> to has_legacy_blending to eliminate confusion.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/gpu/drm/tegra/dc.c | 20 ++++++++++----------
>  drivers/gpu/drm/tegra/dc.h |  2 +-
>  2 files changed, 11 insertions(+), 11 deletions(-)

Looks good to me.

Thierry

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

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

* Re: [PATCH v1 2/3] drm/tegra: plane: Implement zPos plane property for older Tegra's
  2018-05-04 12:15   ` Thierry Reding
@ 2018-05-04 12:32     ` Dmitry Osipenko
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2018-05-04 12:32 UTC (permalink / raw)
  To: Thierry Reding; +Cc: dri-devel, linux-tegra, linux-kernel

On 04.05.2018 15:15, Thierry Reding wrote:
> On Fri, May 04, 2018 at 03:08:43AM +0300, Dmitry Osipenko wrote:
>> Older Tegra's do not support planes z position handling in hardware,
>> but HW provides knobs for zPos implementation in software.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/gpu/drm/tegra/dc.c    | 134 ++++++++++++++++-------
>>  drivers/gpu/drm/tegra/plane.c | 193 ++++++++++++++++++++++++----------
>>  drivers/gpu/drm/tegra/plane.h |  13 ++-
>>  3 files changed, 244 insertions(+), 96 deletions(-)
> 
> This is obviously a lot to review, but it looks pretty good. On minor
> comment below.
> 
>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>> index 2e81142281c3..ba5481cd470d 100644
>> --- a/drivers/gpu/drm/tegra/dc.c
>> +++ b/drivers/gpu/drm/tegra/dc.c
>> @@ -162,29 +162,90 @@ static void tegra_plane_setup_blending_legacy(struct tegra_plane *plane)
> [...]
>> +		case 1:
>> +			/*
>> +			 * When both middle and topmost windows have an alpha,
>> +			 * these windows a mixed together and then the result
>> +			 * is blended over the bottom window.
>> +			 */
>> +			if ((state->blending[0].alpha &&
>> +			     state->blending[0].top))
> 
> There seems to be one pair of parentheses too much here.

Good catch ;)

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

end of thread, other threads:[~2018-05-04 12:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04  0:08 [PATCH v1 0/3] Couple more DRM plane features on Tegra Dmitry Osipenko
2018-05-04  0:08 ` [PATCH v1 1/3] drm/tegra: dc: Enable plane scaling filters Dmitry Osipenko
2018-05-04 11:10   ` Thierry Reding
2018-05-04 11:55     ` Dmitry Osipenko
2018-05-04 12:09       ` Thierry Reding
2018-05-04  0:08 ` [PATCH v1 2/3] drm/tegra: plane: Implement zPos plane property for older Tegra's Dmitry Osipenko
2018-05-04 12:15   ` Thierry Reding
2018-05-04 12:32     ` Dmitry Osipenko
2018-05-04  0:08 ` [PATCH v1 3/3] drm/tegra: dc: Rename supports_blending to has_legacy_blending Dmitry Osipenko
2018-05-04 12:16   ` Thierry Reding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).