It turns out that none of the VI channel registers were meant to be
read. Mostly it works fine but sometimes it returns incorrect values.

Rework VI layer code to write all registers in one go to avoid reads.

This rework will also allow proper code separation.

Signed-off-by: Jernej Skrabec <[email protected]>
---
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 71 ++++++++++----------------
 1 file changed, 27 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index 1f4fa63ef153..dcc4429368d6 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -18,34 +18,35 @@
 #include "sun8i_vi_layer.h"
 #include "sun8i_vi_scaler.h"
 
-static void sun8i_vi_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
-                                       int overlay, struct drm_plane *plane)
+static void sun8i_vi_layer_update_attributes(struct sun8i_mixer *mixer,
+                                            int channel, int overlay,
+                                            struct drm_plane *plane)
 {
-       u32 mask, val, ch_base;
+       struct drm_plane_state *state = plane->state;
+       const struct drm_format_info *fmt;
+       u32 val, ch_base, hw_fmt;
 
        ch_base = sun8i_channel_base(mixer, channel);
+       fmt = state->fb->format;
+       sun8i_mixer_drm_format_to_hw(fmt->format, &hw_fmt);
 
+       val = hw_fmt << SUN8I_MIXER_CHAN_VI_LAYER_ATTR_FBFMT_OFFSET;
+       if (!fmt->is_yuv)
+               val |= SUN8I_MIXER_CHAN_VI_LAYER_ATTR_RGB_MODE;
+       val |= SUN8I_MIXER_CHAN_VI_LAYER_ATTR_EN;
        if (mixer->cfg->de_type >= SUN8I_MIXER_DE3) {
-               mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
-                      SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK;
-               val = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA
-                       (plane->state->alpha >> 8);
-
-               val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+               val |= SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA(state->alpha >> 8);
+               val |= (state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
                        SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL :
                        SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED;
-
-               regmap_update_bits(mixer->engine.regs,
-                                  SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
-                                                                 overlay),
-                                  mask, val);
        } else if (mixer->cfg->vi_num == 1) {
-               regmap_update_bits(mixer->engine.regs,
-                                  SUN8I_MIXER_FCC_GLOBAL_ALPHA_REG,
-                                  SUN8I_MIXER_FCC_GLOBAL_ALPHA_MASK,
-                                  SUN8I_MIXER_FCC_GLOBAL_ALPHA
-                                       (plane->state->alpha >> 8));
+               regmap_write(mixer->engine.regs,
+                            SUN8I_MIXER_FCC_GLOBAL_ALPHA_REG,
+                            SUN8I_MIXER_FCC_GLOBAL_ALPHA(state->alpha >> 8));
        }
+
+       regmap_write(mixer->engine.regs,
+                    SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay), val);
 }
 
 static void sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int channel,
@@ -194,23 +195,14 @@ static u32 sun8i_vi_layer_get_csc_mode(const struct 
drm_format_info *format)
        }
 }
 
-static void sun8i_vi_layer_update_formats(struct sun8i_mixer *mixer, int 
channel,
-                                         int overlay, struct drm_plane *plane)
+static void sun8i_vi_layer_update_colors(struct sun8i_mixer *mixer, int 
channel,
+                                        int overlay, struct drm_plane *plane)
 {
        struct drm_plane_state *state = plane->state;
-       u32 val, ch_base, csc_mode, hw_fmt;
        const struct drm_format_info *fmt;
-
-       ch_base = sun8i_channel_base(mixer, channel);
+       u32 csc_mode;
 
        fmt = state->fb->format;
-       sun8i_mixer_drm_format_to_hw(fmt->format, &hw_fmt);
-
-       val = hw_fmt << SUN8I_MIXER_CHAN_VI_LAYER_ATTR_FBFMT_OFFSET;
-       regmap_update_bits(mixer->engine.regs,
-                          SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay),
-                          SUN8I_MIXER_CHAN_VI_LAYER_ATTR_FBFMT_MASK, val);
-
        csc_mode = sun8i_vi_layer_get_csc_mode(fmt);
        if (csc_mode != SUN8I_CSC_MODE_OFF) {
                sun8i_csc_set_ccsc_coefficients(mixer, channel, csc_mode,
@@ -220,15 +212,6 @@ static void sun8i_vi_layer_update_formats(struct 
sun8i_mixer *mixer, int channel
        } else {
                sun8i_csc_enable_ccsc(mixer, channel, false);
        }
-
-       if (!fmt->is_yuv)
-               val = SUN8I_MIXER_CHAN_VI_LAYER_ATTR_RGB_MODE;
-       else
-               val = 0;
-
-       regmap_update_bits(mixer->engine.regs,
-                          SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay),
-                          SUN8I_MIXER_CHAN_VI_LAYER_ATTR_RGB_MODE, val);
 }
 
 static void sun8i_vi_layer_update_buffer(struct sun8i_mixer *mixer, int 
channel,
@@ -340,12 +323,12 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane 
*plane,
        if (!new_state->crtc || !new_state->visible)
                return;
 
+       sun8i_vi_layer_update_attributes(mixer, layer->channel,
+                                        layer->overlay, plane);
        sun8i_vi_layer_update_coord(mixer, layer->channel,
                                    layer->overlay, plane);
-       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_colors(mixer, layer->channel,
+                                    layer->overlay, plane);
        sun8i_vi_layer_update_buffer(mixer, layer->channel,
                                     layer->overlay, plane);
 }
-- 
2.51.0

Reply via email to