| Message ID | 20210106204630.1800284-1-jernej.skrabec@siol.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/sun4i: de2: Reimplement plane z position setting logic | expand |
Dne sreda, 06. januar 2021 ob 21:46:30 CET je Jernej Skrabec napisal(a): > From: Roman Stratiienko <roman.stratiienko@globallogic.com> > > To set blending channel order register software needs to know state and > position of each channel, which impossible at plane commit stage. > > Move this procedure to atomic_flush stage, where all necessary information > is available. > > Fixes: f88c5ee77496 ("drm/sun4i: Implement zpos for DE2") > Fixes: d8b3f454dab4 ("drm/sun4i: sun8i: Avoid clearing blending order at each atomic commit") > Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com> > [rebased, addressed comments] > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > --- Forgot to include changelog: This is update of: https://patchwork.kernel.org/project/dri-devel/patch/20191229162828.3326-1-roman.stratiienko@globallogic.com/ with addressed comments. Changes from v2: - renamed SUN8I_MIXER_MAX_LAYERS to SUN8I_MIXER_MAX_CHANNELS - removed unused variable in sun8i_vi_layer_enable() - renamed and reordered variables in sun8i_mixer_commit() - removed route allocation for disabled channels - write SUN8I_MIXER_BLEND_PIPE_CTL reg only in commit hook - added fixed tags Best regards, Jernej > drivers/gpu/drm/sun4i/sun8i_mixer.c | 57 +++++++++++++++++++++----- > drivers/gpu/drm/sun4i/sun8i_mixer.h | 5 +++ > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 42 +++---------------- > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 42 +++---------------- > 4 files changed, 64 insertions(+), 82 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/ sun8i_mixer.c > index 5b42cf25cc86..d2153b10b08d 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > @@ -250,6 +250,50 @@ int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format) > > static void sun8i_mixer_commit(struct sunxi_engine *engine) > { > + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine); > + int channel_by_zpos[SUN8I_MIXER_MAX_CHANNELS]; > + u32 base = sun8i_blender_base(mixer); > + u32 route = 0, pipe_ctl = 0; > + unsigned int channel_count; > + int i, j; > + > + channel_count = mixer->cfg->vi_num + mixer->cfg->ui_num; > + > + DRM_DEBUG_DRIVER("Update blender routing\n"); > + > + for (i = 0; i < SUN8I_MIXER_MAX_CHANNELS; i++) > + channel_by_zpos[i] = -1; > + > + for (i = 0; i < channel_count; i++) { > + int zpos = mixer->channel_zpos[i]; > + > + if (zpos >= 0 && zpos < channel_count) > + channel_by_zpos[zpos] = i; > + } > + > + j = 0; > + for (i = 0; i < channel_count; i++) { > + int ch = channel_by_zpos[i]; > + > + if (ch >= 0) { > + pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_EN(j); > + route |= ch << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j); > + j++; > + } > + } > + > + /* > + * Set fill color of bottom plane to black. Generally not needed > + * except when VI plane is at bottom (zpos = 0) and enabled. > + */ > + pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0); > + > + regmap_write(mixer->engine.regs, > + SUN8I_MIXER_BLEND_PIPE_CTL(base), pipe_ctl); > + > + regmap_write(mixer->engine.regs, > + SUN8I_MIXER_BLEND_ROUTE(base), route); > + > DRM_DEBUG_DRIVER("Committing changes\n"); > > regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF, > @@ -479,23 +523,16 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, > regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_BKCOLOR(base), > SUN8I_MIXER_BLEND_COLOR_BLACK); > > - /* > - * Set fill color of bottom plane to black. Generally not needed > - * except when VI plane is at bottom (zpos = 0) and enabled. > - */ > - regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base), > - SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0)); > regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_ATTR_FCOLOR(base, 0), > SUN8I_MIXER_BLEND_COLOR_BLACK); > > plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num; > - for (i = 0; i < plane_cnt; i++) > + for (i = 0; i < plane_cnt; i++) { > + mixer->channel_zpos[i] = -1; > regmap_write(mixer->engine.regs, > SUN8I_MIXER_BLEND_MODE(base, i), > SUN8I_MIXER_BLEND_MODE_DEF); > - > - regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base), > - SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0); > + } > > return 0; > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/ sun8i_mixer.h > index 7576b523fdbb..7b378d6e4dd9 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h > @@ -12,6 +12,8 @@ > > #include "sunxi_engine.h" > > +#define SUN8I_MIXER_MAX_CHANNELS 5 > + > #define SUN8I_MIXER_SIZE(w, h) (((h) - 1) << 16 | ((w) - 1)) > #define SUN8I_MIXER_COORD(x, y) ((y) << 16 | (x)) > > @@ -179,6 +181,9 @@ struct sun8i_mixer { > > struct clk *bus_clk; > struct clk *mod_clk; > + > + /* -1 means that layer is disabled */ > + int channel_zpos[SUN8I_MIXER_MAX_CHANNELS]; > }; > > static inline struct sun8i_mixer * > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/ sun8i_ui_layer.c > index 816ad4ce8996..9f82e7c33e90 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > @@ -24,12 +24,10 @@ > #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 enable, unsigned int zpos) > { > - u32 val, bld_base, ch_base; > + u32 val, ch_base; > > - bld_base = sun8i_blender_base(mixer); > ch_base = sun8i_channel_base(mixer, channel); > > DRM_DEBUG_DRIVER("%sabling channel %d overlay %d\n", > @@ -44,32 +42,7 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel, > 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); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_ROUTE(bld_base), > - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos), > - 0); > - } > - > - if (enable) { > - val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_PIPE_CTL(bld_base), > - val, val); > - > - val = channel << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(zpos); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_ROUTE(bld_base), > - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(zpos), > - val); > - } > + mixer->channel_zpos[channel] = enable ? zpos : -1; > } > > static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel, > @@ -267,11 +240,9 @@ static void sun8i_ui_layer_atomic_disable(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > 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); > + sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0); > } > > static void sun8i_ui_layer_atomic_update(struct drm_plane *plane, > @@ -279,12 +250,11 @@ static void sun8i_ui_layer_atomic_update(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); > + layer->overlay, false, 0); > return; > } > > @@ -295,7 +265,7 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane *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); > + true, zpos); > } > > static const struct drm_plane_helper_funcs sun8i_ui_layer_helper_funcs = { > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/ sun8i_vi_layer.c > index 76393fc976fe..c8c418fb906b 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > @@ -18,12 +18,10 @@ > #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 enable, unsigned int zpos) > { > - u32 val, bld_base, ch_base; > + u32 val, ch_base; > > - bld_base = sun8i_blender_base(mixer); > ch_base = sun8i_channel_base(mixer, channel); > > DRM_DEBUG_DRIVER("%sabling VI channel %d overlay %d\n", > @@ -38,32 +36,7 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel, > SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay), > SUN8I_MIXER_CHAN_VI_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); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_ROUTE(bld_base), > - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos), > - 0); > - } > - > - if (enable) { > - val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_PIPE_CTL(bld_base), > - val, val); > - > - val = channel << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(zpos); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_ROUTE(bld_base), > - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(zpos), > - val); > - } > + mixer->channel_zpos[channel] = enable ? zpos : -1; > } > > static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int channel, > @@ -370,11 +343,9 @@ static void sun8i_vi_layer_atomic_disable(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > 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); > + sun8i_vi_layer_enable(mixer, layer->channel, layer->overlay, false, 0); > } > > static void sun8i_vi_layer_atomic_update(struct drm_plane *plane, > @@ -382,12 +353,11 @@ static void sun8i_vi_layer_atomic_update(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); > + layer->overlay, false, 0); > return; > } > > @@ -398,7 +368,7 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane *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); > + true, zpos); > } > > static const struct drm_plane_helper_funcs sun8i_vi_layer_helper_funcs = { > -- > 2.30.0 > >
Hi, On Wed, Jan 06, 2021 at 09:46:30PM +0100, Jernej Skrabec wrote: > From: Roman Stratiienko <roman.stratiienko@globallogic.com> > > To set blending channel order register software needs to know state and > position of each channel, which impossible at plane commit stage. > > Move this procedure to atomic_flush stage, where all necessary information > is available. Expanding a bit on what the blending order register is supposed to be doing, why it's impossible at the plane commit, and why atomic_flush is a better option would be nice > Fixes: f88c5ee77496 ("drm/sun4i: Implement zpos for DE2") > Fixes: d8b3f454dab4 ("drm/sun4i: sun8i: Avoid clearing blending order at each atomic commit") > Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com> > [rebased, addressed comments] > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > --- > drivers/gpu/drm/sun4i/sun8i_mixer.c | 57 +++++++++++++++++++++----- > drivers/gpu/drm/sun4i/sun8i_mixer.h | 5 +++ > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 42 +++---------------- > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 42 +++---------------- > 4 files changed, 64 insertions(+), 82 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c > index 5b42cf25cc86..d2153b10b08d 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > @@ -250,6 +250,50 @@ int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format) > > static void sun8i_mixer_commit(struct sunxi_engine *engine) > { > + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine); > + int channel_by_zpos[SUN8I_MIXER_MAX_CHANNELS]; > + u32 base = sun8i_blender_base(mixer); > + u32 route = 0, pipe_ctl = 0; > + unsigned int channel_count; > + int i, j; > + > + channel_count = mixer->cfg->vi_num + mixer->cfg->ui_num; > + > + DRM_DEBUG_DRIVER("Update blender routing\n"); > + > + for (i = 0; i < SUN8I_MIXER_MAX_CHANNELS; i++) > + channel_by_zpos[i] = -1; > + > + for (i = 0; i < channel_count; i++) { > + int zpos = mixer->channel_zpos[i]; Why do we need the channel_zpos in the mixer structure, this looks related to the state itself, so we should store it into a custom state structure > + if (zpos >= 0 && zpos < channel_count) > + channel_by_zpos[zpos] = i; > + } > + > + j = 0; > + for (i = 0; i < channel_count; i++) { > + int ch = channel_by_zpos[i]; > + > + if (ch >= 0) { > + pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_EN(j); > + route |= ch << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j); > + j++; > + } > + } Similarly, having a comment somewhere to explain that algorithm would be nice. > + /* > + * Set fill color of bottom plane to black. Generally not needed > + * except when VI plane is at bottom (zpos = 0) and enabled. > + */ > + pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0); > + > + regmap_write(mixer->engine.regs, > + SUN8I_MIXER_BLEND_PIPE_CTL(base), pipe_ctl); > + > + regmap_write(mixer->engine.regs, > + SUN8I_MIXER_BLEND_ROUTE(base), route); > + > DRM_DEBUG_DRIVER("Committing changes\n"); > > regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF, > @@ -479,23 +523,16 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, > regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_BKCOLOR(base), > SUN8I_MIXER_BLEND_COLOR_BLACK); > > - /* > - * Set fill color of bottom plane to black. Generally not needed > - * except when VI plane is at bottom (zpos = 0) and enabled. > - */ > - regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base), > - SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0)); > regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_ATTR_FCOLOR(base, 0), > SUN8I_MIXER_BLEND_COLOR_BLACK); > > plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num; > - for (i = 0; i < plane_cnt; i++) > + for (i = 0; i < plane_cnt; i++) { > + mixer->channel_zpos[i] = -1; > regmap_write(mixer->engine.regs, > SUN8I_MIXER_BLEND_MODE(base, i), > SUN8I_MIXER_BLEND_MODE_DEF); > - > - regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base), > - SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0); > + } > > return 0; > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h > index 7576b523fdbb..7b378d6e4dd9 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h > @@ -12,6 +12,8 @@ > > #include "sunxi_engine.h" > > +#define SUN8I_MIXER_MAX_CHANNELS 5 > + > #define SUN8I_MIXER_SIZE(w, h) (((h) - 1) << 16 | ((w) - 1)) > #define SUN8I_MIXER_COORD(x, y) ((y) << 16 | (x)) > > @@ -179,6 +181,9 @@ struct sun8i_mixer { > > struct clk *bus_clk; > struct clk *mod_clk; > + > + /* -1 means that layer is disabled */ > + int channel_zpos[SUN8I_MIXER_MAX_CHANNELS]; > }; > > static inline struct sun8i_mixer * > diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > index 816ad4ce8996..9f82e7c33e90 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c > @@ -24,12 +24,10 @@ > #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 enable, unsigned int zpos) > { > - u32 val, bld_base, ch_base; > + u32 val, ch_base; > > - bld_base = sun8i_blender_base(mixer); > ch_base = sun8i_channel_base(mixer, channel); > > DRM_DEBUG_DRIVER("%sabling channel %d overlay %d\n", > @@ -44,32 +42,7 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel, > 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); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_ROUTE(bld_base), > - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos), > - 0); > - } > - > - if (enable) { > - val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_PIPE_CTL(bld_base), > - val, val); > - > - val = channel << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(zpos); > - > - regmap_update_bits(mixer->engine.regs, > - SUN8I_MIXER_BLEND_ROUTE(bld_base), > - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(zpos), > - val); > - } > + mixer->channel_zpos[channel] = enable ? zpos : -1; > } > > static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel, > @@ -267,11 +240,9 @@ static void sun8i_ui_layer_atomic_disable(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > 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); > + sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0); > } > > static void sun8i_ui_layer_atomic_update(struct drm_plane *plane, > @@ -279,12 +250,11 @@ static void sun8i_ui_layer_atomic_update(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); > + layer->overlay, false, 0); > return; > } > > @@ -295,7 +265,7 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane *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); > + true, zpos); > } The prototype change could be in a separate patch, it would make things easier to review. Maxime
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 5b42cf25cc86..d2153b10b08d 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -250,6 +250,50 @@ int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format) static void sun8i_mixer_commit(struct sunxi_engine *engine) { + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine); + int channel_by_zpos[SUN8I_MIXER_MAX_CHANNELS]; + u32 base = sun8i_blender_base(mixer); + u32 route = 0, pipe_ctl = 0; + unsigned int channel_count; + int i, j; + + channel_count = mixer->cfg->vi_num + mixer->cfg->ui_num; + + DRM_DEBUG_DRIVER("Update blender routing\n"); + + for (i = 0; i < SUN8I_MIXER_MAX_CHANNELS; i++) + channel_by_zpos[i] = -1; + + for (i = 0; i < channel_count; i++) { + int zpos = mixer->channel_zpos[i]; + + if (zpos >= 0 && zpos < channel_count) + channel_by_zpos[zpos] = i; + } + + j = 0; + for (i = 0; i < channel_count; i++) { + int ch = channel_by_zpos[i]; + + if (ch >= 0) { + pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_EN(j); + route |= ch << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j); + j++; + } + } + + /* + * Set fill color of bottom plane to black. Generally not needed + * except when VI plane is at bottom (zpos = 0) and enabled. + */ + pipe_ctl |= SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0); + + regmap_write(mixer->engine.regs, + SUN8I_MIXER_BLEND_PIPE_CTL(base), pipe_ctl); + + regmap_write(mixer->engine.regs, + SUN8I_MIXER_BLEND_ROUTE(base), route); + DRM_DEBUG_DRIVER("Committing changes\n"); regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF, @@ -479,23 +523,16 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_BKCOLOR(base), SUN8I_MIXER_BLEND_COLOR_BLACK); - /* - * Set fill color of bottom plane to black. Generally not needed - * except when VI plane is at bottom (zpos = 0) and enabled. - */ - regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base), - SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0)); regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_ATTR_FCOLOR(base, 0), SUN8I_MIXER_BLEND_COLOR_BLACK); plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num; - for (i = 0; i < plane_cnt; i++) + for (i = 0; i < plane_cnt; i++) { + mixer->channel_zpos[i] = -1; regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_MODE(base, i), SUN8I_MIXER_BLEND_MODE_DEF); - - regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base), - SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0); + } return 0; diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index 7576b523fdbb..7b378d6e4dd9 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -12,6 +12,8 @@ #include "sunxi_engine.h" +#define SUN8I_MIXER_MAX_CHANNELS 5 + #define SUN8I_MIXER_SIZE(w, h) (((h) - 1) << 16 | ((w) - 1)) #define SUN8I_MIXER_COORD(x, y) ((y) << 16 | (x)) @@ -179,6 +181,9 @@ struct sun8i_mixer { struct clk *bus_clk; struct clk *mod_clk; + + /* -1 means that layer is disabled */ + int channel_zpos[SUN8I_MIXER_MAX_CHANNELS]; }; static inline struct sun8i_mixer * diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 816ad4ce8996..9f82e7c33e90 100644 --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c @@ -24,12 +24,10 @@ #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 enable, unsigned int zpos) { - u32 val, bld_base, ch_base; + u32 val, ch_base; - bld_base = sun8i_blender_base(mixer); ch_base = sun8i_channel_base(mixer, channel); DRM_DEBUG_DRIVER("%sabling channel %d overlay %d\n", @@ -44,32 +42,7 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel, 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); - - regmap_update_bits(mixer->engine.regs, - SUN8I_MIXER_BLEND_ROUTE(bld_base), - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos), - 0); - } - - if (enable) { - val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos); - - regmap_update_bits(mixer->engine.regs, - SUN8I_MIXER_BLEND_PIPE_CTL(bld_base), - val, val); - - val = channel << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(zpos); - - regmap_update_bits(mixer->engine.regs, - SUN8I_MIXER_BLEND_ROUTE(bld_base), - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(zpos), - val); - } + mixer->channel_zpos[channel] = enable ? zpos : -1; } static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel, @@ -267,11 +240,9 @@ static void sun8i_ui_layer_atomic_disable(struct drm_plane *plane, struct drm_plane_state *old_state) { 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); + sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0); } static void sun8i_ui_layer_atomic_update(struct drm_plane *plane, @@ -279,12 +250,11 @@ static void sun8i_ui_layer_atomic_update(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); + layer->overlay, false, 0); return; } @@ -295,7 +265,7 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane *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); + true, zpos); } static const struct drm_plane_helper_funcs sun8i_ui_layer_helper_funcs = { diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index 76393fc976fe..c8c418fb906b 100644 --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c @@ -18,12 +18,10 @@ #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 enable, unsigned int zpos) { - u32 val, bld_base, ch_base; + u32 val, ch_base; - bld_base = sun8i_blender_base(mixer); ch_base = sun8i_channel_base(mixer, channel); DRM_DEBUG_DRIVER("%sabling VI channel %d overlay %d\n", @@ -38,32 +36,7 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel, SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay), SUN8I_MIXER_CHAN_VI_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); - - regmap_update_bits(mixer->engine.regs, - SUN8I_MIXER_BLEND_ROUTE(bld_base), - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos), - 0); - } - - if (enable) { - val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos); - - regmap_update_bits(mixer->engine.regs, - SUN8I_MIXER_BLEND_PIPE_CTL(bld_base), - val, val); - - val = channel << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(zpos); - - regmap_update_bits(mixer->engine.regs, - SUN8I_MIXER_BLEND_ROUTE(bld_base), - SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(zpos), - val); - } + mixer->channel_zpos[channel] = enable ? zpos : -1; } static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int channel, @@ -370,11 +343,9 @@ static void sun8i_vi_layer_atomic_disable(struct drm_plane *plane, struct drm_plane_state *old_state) { 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); + sun8i_vi_layer_enable(mixer, layer->channel, layer->overlay, false, 0); } static void sun8i_vi_layer_atomic_update(struct drm_plane *plane, @@ -382,12 +353,11 @@ static void sun8i_vi_layer_atomic_update(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); + layer->overlay, false, 0); return; } @@ -398,7 +368,7 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane *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); + true, zpos); } static const struct drm_plane_helper_funcs sun8i_vi_layer_helper_funcs = {