From: Roman Stratiienko <[email protected]>
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 <[email protected]>
[rebased, addressed comments]
Signed-off-by: Jernej Skrabec <[email protected]>
---
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
Dne sreda, 06. januar 2021 ob 21:46:30 CET je Jernej Skrabec napisal(a):
> From: Roman Stratiienko <[email protected]>
>
> 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 <[email protected]>
> [rebased, addressed comments]
> Signed-off-by: Jernej Skrabec <[email protected]>
> ---
Forgot to include changelog:
This is update of:
https://patchwork.kernel.org/project/dri-devel/patch/[email protected]/
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 <[email protected]>
>
> 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 <[email protected]>
> [rebased, addressed comments]
> Signed-off-by: Jernej Skrabec <[email protected]>
> ---
> 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