build/patch/kernel/archive/sunxi-5.15/patches.megous/drm-sun8i-ui-vi-Fix-layer-zpos-change-atomic-modesetting.patch

From d0103b81264af511b074a0627a64c607fe0427c8 Mon Sep 17 00:00:00 2001
From: Ondrej Jirman <megous@megous.com>
Date: Sun, 8 Sep 2019 11:42:29 +0200
Subject: [PATCH 312/478] drm: sun8i-ui/vi: Fix layer zpos change/atomic
 modesetting

A problem was found where identical configuration of planes leads
to different register settings at the HW layer when using a X server
with modesetting driver and one plane marked as a cursor.

On first run of the X server, only the black screen and the layer
containing the cursor is visible. Switching to console and back
corrects the situation.

I have dumped registers, and found out this:

1) First Xorg run:

  0x01101000 : 00000201
  0x01101080 : 00000030

  BLD_FILL_COLOR_CTL: (aka SUN8I_MIXER_BLEND_PIPE_CTL)
    P1_EN

  BLD_CH_RTCTL: (aka SUN8I_MIXER_BLEND_ROUTE)
    P0_RTCTL channel0
    P1_RTCTL channel3

2) After switch to console and back to Xorg:

0x01101000 : 00000301
0x01101080 : 00000031

  BLD_FILL_COLOR_CTL:
    P1_EN and P0_EN

  BLD_CH_RTCTL:
    P0_RTCTL channel1
    P1_RTCTL channel3

What happens is that sun8i_ui_layer_enable() function may disable
blending pipes even if it is no longer assigned to its layer, because
of incorrect state/zpos tracking in the driver.

In particular, layer 1 is configured to zpos 0 and thus uses pipe 0.
When layer 3 is enabledb y X server, sun8i_ui_layer_enable() will get
called with old_zpos=0 zpos=1, which will lead to disabling of pipe 0.

In general this issue can happen to any layer during enable or zpos
changes on multiple layers at once.

To correct this we now pass previous enabled/disabled state of the
layer, and pass real previous zpos of the layer to
sun8i_ui_layer_enable() and rework the sun8i_ui_layer_enable() function
to take react to the state changes correctly. In order to not complicate
the atomic_disable callback with all of the above changes, we simply
remove it and implement all the chanes as part of atomic_update, which
also reduces the code duplication.

To make this all work, initial zpos positions of all layers need to be
restored to initial values on reset.

Signed-off-by: Ondrej Jirman <megous@megous.com>
---
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 120 +++++++++++++++----------
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 120 +++++++++++++++----------
 2 files changed, 148 insertions(+), 92 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 7845c2a53a7f..9c406f3896f6 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -24,10 +24,11 @@
 #include "sun8i_ui_scaler.h"
 
 static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel,
-				  int overlay, bool enable, unsigned int zpos,
-				  unsigned int old_zpos)
+				  int overlay, bool was_enabled, bool enable,
+				  unsigned int zpos, unsigned int old_zpos)
 {
 	u32 val, bld_base, ch_base;
+	unsigned int old_pipe_ch;
 
 	bld_base = sun8i_blender_base(mixer);
 	ch_base = sun8i_channel_base(mixer, channel);
@@ -35,28 +36,57 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel,
 	DRM_DEBUG_DRIVER("%sabling channel %d overlay %d\n",
 			 enable ? "En" : "Dis", channel, overlay);
 
-	if (enable)
-		val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN;
-	else
-		val = 0;
+	if (!was_enabled != !enable) {
+		val = enable ? SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN : 0;
 
-	regmap_update_bits(mixer->engine.regs,
-			   SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
-			   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val);
-
-	if (!enable || zpos != old_zpos) {
 		regmap_update_bits(mixer->engine.regs,
-				   SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
-				   SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
-				   0);
+				   SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
+				   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val);
+	}
 
-		regmap_update_bits(mixer->engine.regs,
+	/*
+	 * If this layer was enabled and is being disabled or if it is
+	 * enabled and just changing zpos, clear the old route, if it is
+	 * still configured to this layer in HW.
+	 */
+	if ((was_enabled && !enable) || (enable && zpos != old_zpos)) {
+		/* get channel the pipe for old_zpos is routed to from the HW */
+		regmap_read(mixer->engine.regs,
 				   SUN8I_MIXER_BLEND_ROUTE(bld_base),
-				   SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
-				   0);
+				   &old_pipe_ch);
+		old_pipe_ch &= SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos);
+		old_pipe_ch >>= SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(old_zpos);
+
+		/*
+		 * Check that pipe for old_zpos is still routed to our layer,
+		 * and clear/disable it if it is.
+		 */
+
+		if (old_pipe_ch == channel) {
+			DRM_DEBUG_DRIVER("chan=%d en=%d->%d zpos=%d->%d\n",
+			       channel, was_enabled, enable, old_zpos, zpos);
+
+			DRM_DEBUG_DRIVER("  disable pipe %d\n", old_zpos);
+
+			regmap_update_bits(mixer->engine.regs,
+					   SUN8I_MIXER_BLEND_ROUTE(bld_base),
+					   SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
+					   0);
+
+			regmap_update_bits(mixer->engine.regs,
+					   SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
+					   SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
+					   0);
+		}
 	}
 
-	if (enable) {
+	/*
+	 * If enabling this layer or changin zpos, set route to this layer.
+	 */
+	if ((enable && !was_enabled) || (enable && zpos != old_zpos)) {
+		DRM_DEBUG_DRIVER("chan=%d en=%d->%d zpos=%d->%d\n",
+		       channel, was_enabled, enable, old_zpos, zpos);
+
 		val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
 
 		regmap_update_bits(mixer->engine.regs,
@@ -69,6 +99,8 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel,
 				   SUN8I_MIXER_BLEND_ROUTE(bld_base),
 				   SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(zpos),
 				   val);
+
+		DRM_DEBUG_DRIVER("  enable pipe %d <- ch %d\n", zpos, channel);
 	}
 }
 
@@ -288,19 +320,6 @@ static int sun8i_ui_layer_atomic_check(struct drm_plane *plane,
 						   true, true);
 }
 
-static void sun8i_ui_layer_atomic_disable(struct drm_plane *plane,
-					  struct drm_atomic_state *state)
-{
-	struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state,
-									   plane);
-	struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
-	unsigned int old_zpos = old_state->normalized_zpos;
-	struct sun8i_mixer *mixer = layer->mixer;
-
-	sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0,
-			      old_zpos);
-}
-
 static void sun8i_ui_layer_atomic_update(struct drm_plane *plane,
 					 struct drm_atomic_state *state)
 {
@@ -312,28 +331,37 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane *plane,
 	unsigned int zpos = new_state->normalized_zpos;
 	unsigned int old_zpos = old_state->normalized_zpos;
 	struct sun8i_mixer *mixer = layer->mixer;
+	bool was_enabled = old_state->crtc && old_state->visible;
+	bool enable = new_state->crtc && new_state->visible;
 
-	if (!new_state->visible) {
-		sun8i_ui_layer_enable(mixer, layer->channel,
-				      layer->overlay, false, 0, old_zpos);
-		return;
+	if (enable) {
+		sun8i_ui_layer_update_coord(mixer, layer->channel,
+					    layer->overlay, plane, zpos);
+		sun8i_ui_layer_update_alpha(mixer, layer->channel,
+					    layer->overlay, plane);
+		sun8i_ui_layer_update_formats(mixer, layer->channel,
+					      layer->overlay, plane);
+		sun8i_ui_layer_update_buffer(mixer, layer->channel,
+					     layer->overlay, plane);
 	}
 
-	sun8i_ui_layer_update_coord(mixer, layer->channel,
-				    layer->overlay, plane, zpos);
-	sun8i_ui_layer_update_alpha(mixer, layer->channel,
-				    layer->overlay, plane);
-	sun8i_ui_layer_update_formats(mixer, layer->channel,
-				      layer->overlay, plane);
-	sun8i_ui_layer_update_buffer(mixer, layer->channel,
-				     layer->overlay, plane);
 	sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay,
-			      true, zpos, old_zpos);
+			      was_enabled, enable, zpos, old_zpos);
+}
+
+void sun8i_ui_layer_plane_reset(struct drm_plane *plane)
+{
+	struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
+
+	drm_atomic_helper_plane_reset(plane);
+	if (!plane->state)
+		return;
+
+	plane->state->zpos = layer->channel;
 }
 
 static const struct drm_plane_helper_funcs sun8i_ui_layer_helper_funcs = {
 	.atomic_check	= sun8i_ui_layer_atomic_check,
-	.atomic_disable	= sun8i_ui_layer_atomic_disable,
 	.atomic_update	= sun8i_ui_layer_atomic_update,
 };
 
@@ -342,7 +370,7 @@ static const struct drm_plane_funcs sun8i_ui_layer_funcs = {
 	.atomic_duplicate_state	= drm_atomic_helper_plane_duplicate_state,
 	.destroy		= drm_plane_cleanup,
 	.disable_plane		= drm_atomic_helper_disable_plane,
-	.reset			= drm_atomic_helper_plane_reset,
+	.reset			= sun8i_ui_layer_plane_reset,
 	.update_plane		= drm_atomic_helper_update_plane,
 };
 
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index bb7c43036dfa..3929b0e38a10 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -18,10 +18,11 @@
 #include "sun8i_vi_scaler.h"
 
 static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel,
-				  int overlay, bool enable, unsigned int zpos,
-				  unsigned int old_zpos)
+				  int overlay, bool was_enabled, bool enable,
+				  unsigned int zpos, unsigned int old_zpos)
 {
 	u32 val, bld_base, ch_base;
+	unsigned int old_pipe_ch;
 
 	bld_base = sun8i_blender_base(mixer);
 	ch_base = sun8i_channel_base(mixer, channel);
@@ -29,28 +30,57 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel,
 	DRM_DEBUG_DRIVER("%sabling VI channel %d overlay %d\n",
 			 enable ? "En" : "Dis", channel, overlay);
 
-	if (enable)
-		val = SUN8I_MIXER_CHAN_VI_LAYER_ATTR_EN;
-	else
-		val = 0;
-
-	regmap_update_bits(mixer->engine.regs,
-			   SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay),
-			   SUN8I_MIXER_CHAN_VI_LAYER_ATTR_EN, val);
+	if (!was_enabled != !enable) {
+		val = enable ? SUN8I_MIXER_CHAN_VI_LAYER_ATTR_EN : 0;
 
-	if (!enable || zpos != old_zpos) {
 		regmap_update_bits(mixer->engine.regs,
-				   SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
-				   SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
-				   0);
+				   SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay),
+				   SUN8I_MIXER_CHAN_VI_LAYER_ATTR_EN, val);
+	}
 
-		regmap_update_bits(mixer->engine.regs,
+	/*
+	 * If this layer was enabled and is being disabled or if it is
+	 * enabled and just changing zpos, clear the old route, if it is
+	 * still configured to this layer in HW.
+	 */
+	if ((was_enabled && !enable) || (enable && zpos != old_zpos)) {
+		/* get channel the pipe for old_zpos is routed to from the HW */
+		regmap_read(mixer->engine.regs,
 				   SUN8I_MIXER_BLEND_ROUTE(bld_base),
-				   SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
-				   0);
+				   &old_pipe_ch);
+		old_pipe_ch &= SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos);
+		old_pipe_ch >>= SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(old_zpos);
+
+		/*
+		 * Check that pipe for old_zpos is still routed to our layer,
+		 * and clear/disable it if it is.
+		 */
+
+		if (old_pipe_ch == channel) {
+			DRM_DEBUG_DRIVER("chan=%d en=%d->%d zpos=%d->%d\n",
+			       channel, was_enabled, enable, old_zpos, zpos);
+
+			DRM_DEBUG_DRIVER("  disable pipe %d\n", old_zpos);
+
+			regmap_update_bits(mixer->engine.regs,
+					   SUN8I_MIXER_BLEND_ROUTE(bld_base),
+					   SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
+					   0);
+
+			regmap_update_bits(mixer->engine.regs,
+					   SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
+					   SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
+					   0);
+		}
 	}
 
-	if (enable) {
+	/*
+	 * If enabling this layer or changin zpos, set route to this layer.
+	 */
+	if ((enable && !was_enabled) || (enable && zpos != old_zpos)) {
+		DRM_DEBUG_DRIVER("chan=%d en=%d->%d zpos=%d->%d\n",
+		       channel, was_enabled, enable, old_zpos, zpos);
+
 		val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
 
 		regmap_update_bits(mixer->engine.regs,
@@ -63,6 +93,8 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel,
 				   SUN8I_MIXER_BLEND_ROUTE(bld_base),
 				   SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(zpos),
 				   val);
+
+		DRM_DEBUG_DRIVER("  enable pipe %d <- ch %d\n", zpos, channel);
 	}
 }
 
@@ -392,19 +424,6 @@ static int sun8i_vi_layer_atomic_check(struct drm_plane *plane,
 						   true, true);
 }
 
-static void sun8i_vi_layer_atomic_disable(struct drm_plane *plane,
-					  struct drm_atomic_state *state)
-{
-	struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state,
-									   plane);
-	struct sun8i_vi_layer *layer = plane_to_sun8i_vi_layer(plane);
-	unsigned int old_zpos = old_state->normalized_zpos;
-	struct sun8i_mixer *mixer = layer->mixer;
-
-	sun8i_vi_layer_enable(mixer, layer->channel, layer->overlay, false, 0,
-			      old_zpos);
-}
-
 static void sun8i_vi_layer_atomic_update(struct drm_plane *plane,
 					 struct drm_atomic_state *state)
 {
@@ -416,28 +435,37 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane *plane,
 	unsigned int zpos = new_state->normalized_zpos;
 	unsigned int old_zpos = old_state->normalized_zpos;
 	struct sun8i_mixer *mixer = layer->mixer;
+	bool was_enabled = old_state->crtc && old_state->visible;
+	bool enable = new_state->crtc && new_state->visible;
 
-	if (!new_state->visible) {
-		sun8i_vi_layer_enable(mixer, layer->channel,
-				      layer->overlay, false, 0, old_zpos);
-		return;
+	if (enable) {
+		sun8i_vi_layer_update_coord(mixer, layer->channel,
+					    layer->overlay, plane, zpos);
+		sun8i_vi_layer_update_alpha(mixer, layer->channel,
+					    layer->overlay, plane);
+		sun8i_vi_layer_update_formats(mixer, layer->channel,
+					      layer->overlay, plane);
+		sun8i_vi_layer_update_buffer(mixer, layer->channel,
+					     layer->overlay, plane);
 	}
 
-	sun8i_vi_layer_update_coord(mixer, layer->channel,
-				    layer->overlay, plane, zpos);
-	sun8i_vi_layer_update_alpha(mixer, layer->channel,
-				    layer->overlay, plane);
-	sun8i_vi_layer_update_formats(mixer, layer->channel,
-				      layer->overlay, plane);
-	sun8i_vi_layer_update_buffer(mixer, layer->channel,
-				     layer->overlay, plane);
 	sun8i_vi_layer_enable(mixer, layer->channel, layer->overlay,
-			      true, zpos, old_zpos);
+			      was_enabled, enable, zpos, old_zpos);
+}
+
+void sun8i_vi_layer_plane_reset(struct drm_plane *plane)
+{
+	struct sun8i_vi_layer *layer = plane_to_sun8i_vi_layer(plane);
+
+	drm_atomic_helper_plane_reset(plane);
+	if (!plane->state)
+		return;
+
+	plane->state->zpos = layer->channel;
 }
 
 static const struct drm_plane_helper_funcs sun8i_vi_layer_helper_funcs = {
 	.atomic_check	= sun8i_vi_layer_atomic_check,
-	.atomic_disable	= sun8i_vi_layer_atomic_disable,
 	.atomic_update	= sun8i_vi_layer_atomic_update,
 };
 
@@ -446,7 +474,7 @@ static const struct drm_plane_funcs sun8i_vi_layer_funcs = {
 	.atomic_duplicate_state	= drm_atomic_helper_plane_duplicate_state,
 	.destroy		= drm_plane_cleanup,
 	.disable_plane		= drm_atomic_helper_disable_plane,
-	.reset			= drm_atomic_helper_plane_reset,
+	.reset			= sun8i_vi_layer_plane_reset,
 	.update_plane		= drm_atomic_helper_update_plane,
 };
 
-- 
2.35.3