On Mon, Oct 13, 2025 at 3:23 AM Jernej Skrabec <[email protected]> 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 <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]> > --- > 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 > >
