[PATCH 08/30] drm/sun4i: ui layer: Write attributes in one go

[Chen-Yu Tsai]

On Mon, Oct 13, 2025 at 3:23 AM Jernej Skrabec <jernej.skrabec at gmail.com> wrote:
>
> It turns out that none of the UI channel registers were meant to be
> read. Mostly it works fine but sometimes it returns incorrect values.
>
> Rework UI 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 <jernej.skrabec at gmail.com>

Reviewed-by: Chen-Yu Tsai <wens at kernel.org>

> ---
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 50 +++++++++-----------------
>  1 file changed, 16 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> index 12c83c54f9bc..8634d2ee613a 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -25,25 +25,27 @@
>  #include "sun8i_ui_scaler.h"
>  #include "sun8i_vi_scaler.h"
>
> -static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
> -                                       int overlay, struct drm_plane *plane)
> +static void sun8i_ui_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);
>
> -       mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
> -               SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
> -
> -       val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >> 8);
> -
> -       val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
> +       val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(state->alpha >> 8);
> +       val |= (state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?

Changing "plane->state" to "state" made the diff somewhat harder to read.

>                 SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
>                 SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
> +       val |= hw_fmt << SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_OFFSET;
> +       val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN;
>
> -       regmap_update_bits(mixer->engine.regs,
> -                          SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
> -                          mask, val);
> +       regmap_write(mixer->engine.regs,
> +                    SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay), val);
>  }
>
>  static void sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
> @@ -111,24 +113,6 @@ static void sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
>         }
>  }
>
> -static void sun8i_ui_layer_update_formats(struct sun8i_mixer *mixer, int channel,
> -                                         int overlay, struct drm_plane *plane)
> -{
> -       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_UI_LAYER_ATTR_FBFMT_OFFSET;
> -       regmap_update_bits(mixer->engine.regs,
> -                          SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
> -                          SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_MASK, val);
> -}
> -
>  static void sun8i_ui_layer_update_buffer(struct sun8i_mixer *mixer, int channel,
>                                          int overlay, struct drm_plane *plane)
>  {
> @@ -220,12 +204,10 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane *plane,
>         if (!new_state->crtc || !new_state->visible)
>                 return;
>
> +       sun8i_ui_layer_update_attributes(mixer, layer->channel,
> +                                        layer->overlay, plane);
>         sun8i_ui_layer_update_coord(mixer, layer->channel,
>                                     layer->overlay, plane);
> -       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);
>  }
> --
> 2.51.0
>
>