| Message ID | 20191228202818.69908-4-roman.stratiienko@globallogic.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,1/4] drm/sun4i: Wait for previous mixing process finish before committing next | expand |
Hi! Dne sobota, 28. december 2019 ob 21:28:17 CET je roman.stratiienko@globallogic.com 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. > > Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com> > --- > drivers/gpu/drm/sun4i/sun8i_mixer.c | 47 +++++++++++++++++++++++++- > drivers/gpu/drm/sun4i/sun8i_mixer.h | 3 ++ > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 42 ++++------------------- > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 39 +++------------------ > 4 files changed, 60 insertions(+), 71 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index bb9a665fd053..da84fccf7784 > 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > @@ -307,8 +307,47 @@ static void sun8i_atomic_begin(struct sunxi_engine > *engine, > > static void sun8i_mixer_commit(struct sunxi_engine *engine) > { > - DRM_DEBUG_DRIVER("Committing changes\n"); > + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine); > + u32 base = sun8i_blender_base(mixer); > + int i, j; > + int channel_by_zpos[4] = {-1, -1, -1, -1}; > + u32 route = 0, pipe_ctl = 0; > + > + DRM_DEBUG_DRIVER("Update blender routing\n"); Use drm_dbg(). > + for (i = 0; i < 4; i++) { > + int zpos = mixer->channel_zpos[i]; channel_zpos can hold 5 elements which is also theoretical maximum for current HW design. Why do you check only 4 elements? It would be great to introduce a macro like SUN8I_MIXER_MAX_LAYERS so everyone would understand where this number comes from. > + > + if (zpos >= 0 && zpos < 4) > + channel_by_zpos[zpos] = i; > + } > + > + j = 0; > + for (i = 0; i < 4; 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++; > + } > + } > + > + for (i = 0; i < 4 && j < 4; i++) { > + int zpos = mixer->channel_zpos[i]; > > + if (zpos < 0) { > + route |= i << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j); > + j++; > + } > + } > + > + regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base), > + SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, pipe_ctl); > + > + regmap_write(mixer->engine.regs, > + SUN8I_MIXER_BLEND_ROUTE(base), route); > + > + DRM_DEBUG_DRIVER("Committing changes\n"); Use drm_dbg(). > regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF, > SUN8I_MIXER_GLOBAL_DBUFF_ENABLE); > } > @@ -422,6 +461,12 @@ static int sun8i_mixer_bind(struct device *dev, struct > device *master, mixer->engine.ops = &sun8i_engine_ops; > mixer->engine.node = dev->of_node; > > + mixer->channel_zpos[0] = -1; > + mixer->channel_zpos[1] = -1; > + mixer->channel_zpos[2] = -1; > + mixer->channel_zpos[3] = -1; > + mixer->channel_zpos[4] = -1; > + for loop would be better, especially using proposed macro. Best regards, Jernej > /* > * While this function can fail, we shouldn't do anything > * if this happens. Some early DE2 DT entries don't provide > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index 915479cc3077..9c2ff87923d8 > 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h > @@ -178,6 +178,9 @@ struct sun8i_mixer { > > struct clk *bus_clk; > struct clk *mod_clk; > + > + /* -1 means that layer is disabled */ > + int channel_zpos[5]; > }; > > 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 893076716070..23c2f4b68c89 > 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, @@ -235,11 +208,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, > @@ -247,12 +218,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; > } > > @@ -263,7 +233,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 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 42d445d23773..97cbc98bf781 > 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > @@ -17,8 +17,7 @@ > #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; > > @@ -37,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, @@ -350,11 +324,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, > @@ -362,12 +334,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; > } > > @@ -378,7 +349,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 struct drm_plane_helper_funcs sun8i_vi_layer_helper_funcs = {
Hello Jernej, Thank you for review. On Sun, Dec 29, 2019 at 11:40 AM Jernej Škrabec <jernej.skrabec@siol.net> wrote: > > Hi! > > Dne sobota, 28. december 2019 ob 21:28:17 CET je > roman.stratiienko@globallogic.com 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. > > > > Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com> > > --- > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 47 +++++++++++++++++++++++++- > > drivers/gpu/drm/sun4i/sun8i_mixer.h | 3 ++ > > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 42 ++++------------------- > > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 39 +++------------------ > > 4 files changed, 60 insertions(+), 71 deletions(-) > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c > > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index bb9a665fd053..da84fccf7784 > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > > @@ -307,8 +307,47 @@ static void sun8i_atomic_begin(struct sunxi_engine > > *engine, > > > > static void sun8i_mixer_commit(struct sunxi_engine *engine) > > { > > - DRM_DEBUG_DRIVER("Committing changes\n"); > > + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine); > > + u32 base = sun8i_blender_base(mixer); > > + int i, j; > > + int channel_by_zpos[4] = {-1, -1, -1, -1}; > > + u32 route = 0, pipe_ctl = 0; > > + > > + DRM_DEBUG_DRIVER("Update blender routing\n"); > > Use drm_dbg(). > > > + for (i = 0; i < 4; i++) { > > + int zpos = mixer->channel_zpos[i]; > > channel_zpos can hold 5 elements which is also theoretical maximum for current > HW design. Why do you check only 4 elements? > I'll use plane_cnt as it done in mixer_bind > It would be great to introduce a macro like SUN8I_MIXER_MAX_LAYERS so everyone > would understand where this number comes from. Will do. > > > + > > + if (zpos >= 0 && zpos < 4) > > + channel_by_zpos[zpos] = i; > > + } > > + > > + j = 0; > > + for (i = 0; i < 4; 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++; > > + } > > + } > > + > > + for (i = 0; i < 4 && j < 4; i++) { > > + int zpos = mixer->channel_zpos[i]; > > > > + if (zpos < 0) { > > + route |= i << > SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j); > > + j++; > > + } > > + } > > + > > + regmap_update_bits(mixer->engine.regs, > SUN8I_MIXER_BLEND_PIPE_CTL(base), > > + SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, > pipe_ctl); > > + > > + regmap_write(mixer->engine.regs, > > + SUN8I_MIXER_BLEND_ROUTE(base), route); > > + > > + DRM_DEBUG_DRIVER("Committing changes\n"); > > Use drm_dbg(). According to https://github.com/torvalds/linux/commit/99a954874e7b9f0c8058476575593b3beb5731a5#diff-b0cd2d683c6afbab7bd54173cfd3d3ecR289 , DRM_DEBUG_DRIVER uses drm_dbg. Also, using drm_dbg with category macro would require larger indent, making harder to fit in 80 chars limit. Are there any plans to deprecate DRM_DEBUG_DRIVER macro? > > > regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF, > > SUN8I_MIXER_GLOBAL_DBUFF_ENABLE); > > } > > @@ -422,6 +461,12 @@ static int sun8i_mixer_bind(struct device *dev, struct > > device *master, mixer->engine.ops = &sun8i_engine_ops; > > mixer->engine.node = dev->of_node; > > > > + mixer->channel_zpos[0] = -1; > > + mixer->channel_zpos[1] = -1; > > + mixer->channel_zpos[2] = -1; > > + mixer->channel_zpos[3] = -1; > > + mixer->channel_zpos[4] = -1; > > + > > for loop would be better, especially using proposed macro. I'll put it into already existent for-loop below. > > Best regards, > Jernej > > > /* > > * While this function can fail, we shouldn't do anything > > * if this happens. Some early DE2 DT entries don't provide > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h > > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index 915479cc3077..9c2ff87923d8 > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h > > @@ -178,6 +178,9 @@ struct sun8i_mixer { > > > > struct clk *bus_clk; > > struct clk *mod_clk; > > + > > + /* -1 means that layer is disabled */ > > + int channel_zpos[5]; > > }; > > > > 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 893076716070..23c2f4b68c89 > > 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, @@ -235,11 +208,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, > > @@ -247,12 +218,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; > > } > > > > @@ -263,7 +233,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 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 42d445d23773..97cbc98bf781 > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > > @@ -17,8 +17,7 @@ > > #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; > > > > @@ -37,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, @@ -350,11 +324,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, > > @@ -362,12 +334,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; > > } > > > > @@ -378,7 +349,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 struct drm_plane_helper_funcs sun8i_vi_layer_helper_funcs = { > > > >
Dne nedelja, 29. december 2019 ob 13:08:19 CET je Roman Stratiienko napisal(a): > Hello Jernej, > > Thank you for review. > > On Sun, Dec 29, 2019 at 11:40 AM Jernej Škrabec <jernej.skrabec@siol.net> wrote: > > Hi! > > > > Dne sobota, 28. december 2019 ob 21:28:17 CET je > > > > roman.stratiienko@globallogic.com 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. > > > > > > Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com> > > > --- > > > > > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 47 +++++++++++++++++++++++++- > > > drivers/gpu/drm/sun4i/sun8i_mixer.h | 3 ++ > > > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 42 ++++------------------- > > > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 39 +++------------------ > > > 4 files changed, 60 insertions(+), 71 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c > > > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index bb9a665fd053..da84fccf7784 > > > 100644 > > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > > > @@ -307,8 +307,47 @@ static void sun8i_atomic_begin(struct sunxi_engine > > > *engine, > > > > > > static void sun8i_mixer_commit(struct sunxi_engine *engine) > > > { > > > > > > - DRM_DEBUG_DRIVER("Committing changes\n"); > > > + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine); > > > + u32 base = sun8i_blender_base(mixer); > > > + int i, j; > > > + int channel_by_zpos[4] = {-1, -1, -1, -1}; > > > + u32 route = 0, pipe_ctl = 0; > > > + > > > + DRM_DEBUG_DRIVER("Update blender routing\n"); > > > > Use drm_dbg(). > > > > > + for (i = 0; i < 4; i++) { > > > + int zpos = mixer->channel_zpos[i]; > > > > channel_zpos can hold 5 elements which is also theoretical maximum for > > current HW design. Why do you check only 4 elements? > > I'll use plane_cnt as it done in mixer_bind > > > It would be great to introduce a macro like SUN8I_MIXER_MAX_LAYERS so > > everyone would understand where this number comes from. > > Will do. > > > > + > > > + if (zpos >= 0 && zpos < 4) > > > + channel_by_zpos[zpos] = i; > > > + } > > > + > > > + j = 0; > > > + for (i = 0; i < 4; 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++; > > > + } > > > + } > > > + > > > + for (i = 0; i < 4 && j < 4; i++) { > > > + int zpos = mixer->channel_zpos[i]; > > > > > > + if (zpos < 0) { > > > + route |= i << > > > > SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j); > > > > > + j++; > > > + } > > > + } > > > + > > > + regmap_update_bits(mixer->engine.regs, > > > > SUN8I_MIXER_BLEND_PIPE_CTL(base), > > > > > + SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, > > > > pipe_ctl); > > > > > + > > > + regmap_write(mixer->engine.regs, > > > + SUN8I_MIXER_BLEND_ROUTE(base), route); > > > + > > > + DRM_DEBUG_DRIVER("Committing changes\n"); > > > > Use drm_dbg(). > > According to > https://github.com/torvalds/linux/commit/99a954874e7b9f0c8058476575593b3beb > 5731a5#diff-b0cd2d683c6afbab7bd54173cfd3d3ecR289 , > DRM_DEBUG_DRIVER uses drm_dbg. > Also, using drm_dbg with category macro would require larger indent, > making harder to fit in 80 chars limit. From what I can see, category is already defined by macro name. Check here: https://cgit.freedesktop.org/drm/drm-misc/tree/include/drm/drm_print.h#n465 So it should be actually shorter. > Are there any plans to deprecate DRM_DEBUG_DRIVER macro? Yes, at least I understand following commit message in such manner: https://cgit.freedesktop.org/drm/drm-misc/commit/? id=93ccfa9a4eca482216c5caf88be77e5ffa0d744a Best regards, Jernej > > > > regmap_write(engine->regs, SUN8I_MIXER_GLOBAL_DBUFF, > > > > > > SUN8I_MIXER_GLOBAL_DBUFF_ENABLE); > > > > > > } > > > > > > @@ -422,6 +461,12 @@ static int sun8i_mixer_bind(struct device *dev, > > > struct > > > device *master, mixer->engine.ops = &sun8i_engine_ops; > > > > > > mixer->engine.node = dev->of_node; > > > > > > + mixer->channel_zpos[0] = -1; > > > + mixer->channel_zpos[1] = -1; > > > + mixer->channel_zpos[2] = -1; > > > + mixer->channel_zpos[3] = -1; > > > + mixer->channel_zpos[4] = -1; > > > + > > > > for loop would be better, especially using proposed macro. > > I'll put it into already existent for-loop below. > > > Best regards, > > Jernej > > > > > /* > > > > > > * While this function can fail, we shouldn't do anything > > > * if this happens. Some early DE2 DT entries don't provide > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h > > > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index 915479cc3077..9c2ff87923d8 > > > 100644 > > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h > > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h > > > @@ -178,6 +178,9 @@ struct sun8i_mixer { > > > > > > struct clk *bus_clk; > > > struct clk *mod_clk; > > > > > > + > > > + /* -1 means that layer is disabled */ > > > + int channel_zpos[5]; > > > > > > }; > > > > > > 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 > > > 893076716070..23c2f4b68c89 > > > 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, @@ -235,11 +208,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, > > > > > > @@ -247,12 +218,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; > > > > > > } > > > > > > @@ -263,7 +233,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 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 > > > 42d445d23773..97cbc98bf781 > > > 100644 > > > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > > > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > > > @@ -17,8 +17,7 @@ > > > > > > #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; > > > > > > @@ -37,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, @@ -350,11 +324,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, > > > > > > @@ -362,12 +334,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; > > > > > > } > > > > > > @@ -378,7 +349,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 struct drm_plane_helper_funcs sun8i_vi_layer_helper_funcs = {
On Sun, Dec 29, 2019 at 02:02:33PM +0100, Jernej Škrabec wrote: > Dne nedelja, 29. december 2019 ob 13:47:38 CET je Roman Stratiienko > napisal(a): > > On Sun, Dec 29, 2019 at 2:18 PM Jernej Škrabec <jernej.skrabec@siol.net> > wrote: > > > Dne nedelja, 29. december 2019 ob 13:08:19 CET je Roman Stratiienko > > > > > > napisal(a): > > > > Hello Jernej, > > > > > > > > Thank you for review. > > > > > > > > On Sun, Dec 29, 2019 at 11:40 AM Jernej Škrabec > > > > <jernej.skrabec@siol.net> > > > > > > wrote: > > > > > Hi! > > > > > > > > > > Dne sobota, 28. december 2019 ob 21:28:17 CET je > > > > > > > > > > roman.stratiienko@globallogic.com 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. > > > > > > > > > > > > Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com> > > > > > > --- > > > > > > > > > > > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 47 > > > > > > +++++++++++++++++++++++++- > > > > > > drivers/gpu/drm/sun4i/sun8i_mixer.h | 3 ++ > > > > > > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 42 ++++------------------- > > > > > > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 39 +++------------------ > > > > > > 4 files changed, 60 insertions(+), 71 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c > > > > > > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index > > > > > > bb9a665fd053..da84fccf7784 > > > > > > 100644 > > > > > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > > > > > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > > > > > > @@ -307,8 +307,47 @@ static void sun8i_atomic_begin(struct > > > > > > sunxi_engine > > > > > > *engine, > > > > > > > > > > > > static void sun8i_mixer_commit(struct sunxi_engine *engine) > > > > > > { > > > > > > > > > > > > - DRM_DEBUG_DRIVER("Committing changes\n"); > > > > > > + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine); > > > > > > + u32 base = sun8i_blender_base(mixer); > > > > > > + int i, j; > > > > > > + int channel_by_zpos[4] = {-1, -1, -1, -1}; > > > > > > + u32 route = 0, pipe_ctl = 0; > > > > > > + > > > > > > + DRM_DEBUG_DRIVER("Update blender routing\n"); > > > > > > > > > > Use drm_dbg(). > > > > > > > > > > > + for (i = 0; i < 4; i++) { > > > > > > + int zpos = mixer->channel_zpos[i]; > > > > > > > > > > channel_zpos can hold 5 elements which is also theoretical maximum for > > > > > current HW design. Why do you check only 4 elements? > > > > > > > > I'll use plane_cnt as it done in mixer_bind > > > > > > > > > It would be great to introduce a macro like SUN8I_MIXER_MAX_LAYERS so > > > > > everyone would understand where this number comes from. > > > > > > > > Will do. > > > > > > > > > > + > > > > > > + if (zpos >= 0 && zpos < 4) > > > > > > + channel_by_zpos[zpos] = i; > > > > > > + } > > > > > > + > > > > > > + j = 0; > > > > > > + for (i = 0; i < 4; 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++; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + for (i = 0; i < 4 && j < 4; i++) { > > > > > > + int zpos = mixer->channel_zpos[i]; > > > > > > > > > > > > + if (zpos < 0) { > > > > > > + route |= i << > > > > > > > > > > SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j); > > > > > > > > > > > + j++; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + regmap_update_bits(mixer->engine.regs, > > > > > > > > > > SUN8I_MIXER_BLEND_PIPE_CTL(base), > > > > > > > > > > > + SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, > > > > > > > > > > pipe_ctl); > > > > > > > > > > > + > > > > > > + regmap_write(mixer->engine.regs, > > > > > > + SUN8I_MIXER_BLEND_ROUTE(base), route); > > > > > > + > > > > > > + DRM_DEBUG_DRIVER("Committing changes\n"); > > > > > > > > > > Use drm_dbg(). > > > > > > > > According to > > > > https://github.com/torvalds/linux/commit/99a954874e7b9f0c8058476575593b3 > > > > beb > > > > 5731a5#diff-b0cd2d683c6afbab7bd54173cfd3d3ecR289 , > > > > DRM_DEBUG_DRIVER uses drm_dbg. > > > > Also, using drm_dbg with category macro would require larger indent, > > > > making harder to fit in 80 chars limit. > > > > > > From what I can see, category is already defined by macro name. Check > > > here: > > > https://cgit.freedesktop.org/drm/drm-misc/tree/include/drm/drm_print.h#n46 > > > 5 > > > > > > So it should be actually shorter. > > > > Ah, it something very recent. > > drm_dbg also require drm_device struct > > Do you know the best way to extract it from `struct engine`? > > I don't think there is a way. I guess we can solve this later. Maxime, any > thoughts? There's no way at the moment, but it would make sense to add a pointer to it. Maximey
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index bb9a665fd053..da84fccf7784 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -307,8 +307,47 @@ static void sun8i_atomic_begin(struct sunxi_engine *engine, static void sun8i_mixer_commit(struct sunxi_engine *engine) { - DRM_DEBUG_DRIVER("Committing changes\n"); + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine); + u32 base = sun8i_blender_base(mixer); + int i, j; + int channel_by_zpos[4] = {-1, -1, -1, -1}; + u32 route = 0, pipe_ctl = 0; + + DRM_DEBUG_DRIVER("Update blender routing\n"); + for (i = 0; i < 4; i++) { + int zpos = mixer->channel_zpos[i]; + + if (zpos >= 0 && zpos < 4) + channel_by_zpos[zpos] = i; + } + + j = 0; + for (i = 0; i < 4; 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++; + } + } + + for (i = 0; i < 4 && j < 4; i++) { + int zpos = mixer->channel_zpos[i]; + if (zpos < 0) { + route |= i << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j); + j++; + } + } + + regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base), + SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 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, SUN8I_MIXER_GLOBAL_DBUFF_ENABLE); } @@ -422,6 +461,12 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, mixer->engine.ops = &sun8i_engine_ops; mixer->engine.node = dev->of_node; + mixer->channel_zpos[0] = -1; + mixer->channel_zpos[1] = -1; + mixer->channel_zpos[2] = -1; + mixer->channel_zpos[3] = -1; + mixer->channel_zpos[4] = -1; + /* * While this function can fail, we shouldn't do anything * if this happens. Some early DE2 DT entries don't provide diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index 915479cc3077..9c2ff87923d8 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -178,6 +178,9 @@ struct sun8i_mixer { struct clk *bus_clk; struct clk *mod_clk; + + /* -1 means that layer is disabled */ + int channel_zpos[5]; }; 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 893076716070..23c2f4b68c89 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, @@ -235,11 +208,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, @@ -247,12 +218,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; } @@ -263,7 +233,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 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 42d445d23773..97cbc98bf781 100644 --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c @@ -17,8 +17,7 @@ #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; @@ -37,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, @@ -350,11 +324,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, @@ -362,12 +334,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; } @@ -378,7 +349,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 struct drm_plane_helper_funcs sun8i_vi_layer_helper_funcs = {