From: Ondrej Jirman <meg...@megous.com>

There are various issues that this re-work of sun8i_[uv]i_layer_enable
function fixes:

- Make sure that we re-initialize zpos on reset
- Minimize register updates by doing them only when state changes
- Fix issue where DE pipe might get disabled even if it is no longer
  used by the layer that's currently calling sun8i_ui_layer_enable
- .atomic_disable callback is not really needed because .atomic_update
  can do the disable too, so drop the duplicate code

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

diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index dd2a1c851939..b88e8ac5ad1c 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);
        }
 }
 
@@ -261,45 +293,43 @@ 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_plane_state *old_state)
+static void sun8i_ui_layer_atomic_update(struct drm_plane *plane,
+                                        struct drm_plane_state *old_state)
 {
        struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
+       unsigned int zpos = plane->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 = plane->state->crtc && plane->state->visible;
 
-       sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0,
-                             old_zpos);
+       if (enable) {
+               sun8i_ui_layer_update_coord(mixer, layer->channel,
+                                           layer->overlay, plane, zpos);
+               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,
+                             was_enabled, enable, zpos, old_zpos);
 }
 
-static void sun8i_ui_layer_atomic_update(struct drm_plane *plane,
-                                        struct drm_plane_state *old_state)
+void sun8i_ui_layer_plane_reset(struct drm_plane *plane)
 {
        struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
-       unsigned int zpos = plane->state->normalized_zpos;
-       unsigned int old_zpos = old_state->normalized_zpos;
-       struct sun8i_mixer *mixer = layer->mixer;
 
-       if (!plane->state->visible) {
-               sun8i_ui_layer_enable(mixer, layer->channel,
-                                     layer->overlay, false, 0, old_zpos);
+       drm_atomic_helper_plane_reset(plane);
+       if (!plane->state)
                return;
-       }
 
-       sun8i_ui_layer_update_coord(mixer, layer->channel,
-                                   layer->overlay, plane, zpos);
-       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);
+       plane->state->zpos = layer->channel;
 }
 
 static struct drm_plane_helper_funcs sun8i_ui_layer_helper_funcs = {
        .prepare_fb     = drm_gem_fb_prepare_fb,
        .atomic_check   = sun8i_ui_layer_atomic_check,
-       .atomic_disable = sun8i_ui_layer_atomic_disable,
        .atomic_update  = sun8i_ui_layer_atomic_update,
 };
 
@@ -308,7 +338,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 bd0e6a52d1d8..675ebcdac00b 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);
        }
 }
 
@@ -345,45 +377,43 @@ 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_plane_state *old_state)
+static void sun8i_vi_layer_atomic_update(struct drm_plane *plane,
+                                        struct drm_plane_state *old_state)
 {
        struct sun8i_vi_layer *layer = plane_to_sun8i_vi_layer(plane);
+       unsigned int zpos = plane->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 = plane->state->crtc && plane->state->visible;
 
-       sun8i_vi_layer_enable(mixer, layer->channel, layer->overlay, false, 0,
-                             old_zpos);
+       if (enable) {
+               sun8i_vi_layer_update_coord(mixer, layer->channel,
+                                           layer->overlay, plane, zpos);
+               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,
+                             was_enabled, enable, zpos, old_zpos);
 }
 
-static void sun8i_vi_layer_atomic_update(struct drm_plane *plane,
-                                        struct drm_plane_state *old_state)
+void sun8i_vi_layer_plane_reset(struct drm_plane *plane)
 {
        struct sun8i_vi_layer *layer = plane_to_sun8i_vi_layer(plane);
-       unsigned int zpos = plane->state->normalized_zpos;
-       unsigned int old_zpos = old_state->normalized_zpos;
-       struct sun8i_mixer *mixer = layer->mixer;
 
-       if (!plane->state->visible) {
-               sun8i_vi_layer_enable(mixer, layer->channel,
-                                     layer->overlay, false, 0, old_zpos);
+       drm_atomic_helper_plane_reset(plane);
+       if (!plane->state)
                return;
-       }
 
-       sun8i_vi_layer_update_coord(mixer, layer->channel,
-                                   layer->overlay, plane, zpos);
-       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);
+       plane->state->zpos = layer->channel;
 }
 
 static struct drm_plane_helper_funcs sun8i_vi_layer_helper_funcs = {
        .prepare_fb     = drm_gem_fb_prepare_fb,
        .atomic_check   = sun8i_vi_layer_atomic_check,
-       .atomic_disable = sun8i_vi_layer_atomic_disable,
        .atomic_update  = sun8i_vi_layer_atomic_update,
 };
 
@@ -392,7 +422,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.23.0

Reply via email to