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") Signed-off-by: Roman Stratiienko roman.stratiienko@globallogic.com --- v2: - Use SUN8I_MIXER_MAX_LAYERS macro - Use plane_cnt instead of hard-coded number - Put initialization of channel_zpos into for loop - Add 'Fixes' line to the commit message - Minor clean-ups - Comments added --- drivers/gpu/drm/sun4i/sun8i_mixer.c | 52 +++++++++++++++++++++++++- 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 | 39 +++---------------- 4 files changed, 66 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..d306ad5dc093 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -259,8 +259,54 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 format)
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 = 0; + int channel_by_zpos[SUN8I_MIXER_MAX_LAYERS]; + u32 route = 0, pipe_ctl = 0; + int plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num; + + DRM_DEBUG_DRIVER("Update blender routing\n"); + /* Assume that values in mixer->channel_zpos[] are unique except -1 */ + + for (i = 0; i < plane_cnt; i++) + channel_by_zpos[i] = -1;
+ /* Sort by zpos */ + for (i = 0; i < plane_cnt; i++) { + int zpos = mixer->channel_zpos[i]; + + if (zpos >= 0 && zpos < plane_cnt) + channel_by_zpos[zpos] = i; + } + + /* Route enabled blending channels first */ + for (i = 0; i < plane_cnt; 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 remaining routing fields to match disabled channel indices */ + for (i = 0; i < SUN8I_MIXER_MAX_LAYERS && j < SUN8I_MIXER_MAX_LAYERS; + i++) { + if (mixer->channel_zpos[i] < 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); } @@ -485,10 +531,12 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, 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); diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index c6cc94057faf..b193d9d1db66 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -13,6 +13,8 @@ #include "sun8i_csc.h" #include "sunxi_engine.h"
+#define SUN8I_MIXER_MAX_LAYERS 5 + #define SUN8I_MIXER_SIZE(w, h) (((h) - 1) << 16 | ((w) - 1)) #define SUN8I_MIXER_COORD(x, y) ((y) << 16 | (x))
@@ -176,6 +178,9 @@ struct sun8i_mixer {
struct clk *bus_clk; struct clk *mod_clk; + + /* -1 means that layer is disabled */ + int channel_zpos[SUN8I_MIXER_MAX_LAYERS]; };
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 c87fd842918e..ee7c13d8710f 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, @@ -265,11 +238,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, @@ -277,12 +248,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; }
@@ -293,7 +263,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 = {
From: Roman Stratiienko roman.stratiienko@globallogic.com
Create callback to update engine's registers on mode change.
Signed-off-by: Roman Stratiienko roman.stratiienko@globallogic.com --- v2: - Split commit in 2 parts. - Add description to mode_set callback - Dropped 1 line from sun4i_crtc_mode_set_nofb() - Add struct drm_display_mode declaration (fix build warning) --- drivers/gpu/drm/sun4i/sun4i_crtc.c | 3 +++ drivers/gpu/drm/sun4i/sunxi_engine.h | 12 ++++++++++++ 2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c index 3a153648b369..f9c627d601c3 100644 --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c @@ -141,6 +141,9 @@ static void sun4i_crtc_mode_set_nofb(struct drm_crtc *crtc) struct sun4i_crtc *scrtc = drm_crtc_to_sun4i_crtc(crtc);
sun4i_tcon_mode_set(scrtc->tcon, encoder, mode); + + if (scrtc->engine->ops->mode_set) + scrtc->engine->ops->mode_set(scrtc->engine, mode); }
static const struct drm_crtc_helper_funcs sun4i_crtc_helper_funcs = { diff --git a/drivers/gpu/drm/sun4i/sunxi_engine.h b/drivers/gpu/drm/sun4i/sunxi_engine.h index 548710a936d5..44102783ee3c 100644 --- a/drivers/gpu/drm/sun4i/sunxi_engine.h +++ b/drivers/gpu/drm/sun4i/sunxi_engine.h @@ -9,6 +9,7 @@ struct drm_plane; struct drm_device; struct drm_crtc_state; +struct drm_display_mode;
struct sunxi_engine;
@@ -108,6 +109,17 @@ struct sunxi_engine_ops { * This function is optional. */ void (*vblank_quirk)(struct sunxi_engine *engine); + + /** + * @mode_set: + * + * This callback is used to update engine registers that + * responsible for display frame size other mode attributes. + * + * This function is optional. + */ + void (*mode_set)(struct sunxi_engine *engine, + struct drm_display_mode *mode); };
/**
Hi!
Dne nedelja, 29. december 2019 ob 17:28:26 CET je roman.stratiienko@globallogic.com napisal(a):
From: Roman Stratiienko roman.stratiienko@globallogic.com
Create callback to update engine's registers on mode change.
Signed-off-by: Roman Stratiienko roman.stratiienko@globallogic.com
Reviewed-by: Jernej Skrabec jernej.skrabec@siol.net
Best regards, Jernej
v2:
- Split commit in 2 parts.
- Add description to mode_set callback
- Dropped 1 line from sun4i_crtc_mode_set_nofb()
- Add struct drm_display_mode declaration (fix build warning)
drivers/gpu/drm/sun4i/sun4i_crtc.c | 3 +++ drivers/gpu/drm/sun4i/sunxi_engine.h | 12 ++++++++++++ 2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c index 3a153648b369..f9c627d601c3 100644 --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c @@ -141,6 +141,9 @@ static void sun4i_crtc_mode_set_nofb(struct drm_crtc *crtc) struct sun4i_crtc *scrtc = drm_crtc_to_sun4i_crtc(crtc);
sun4i_tcon_mode_set(scrtc->tcon, encoder, mode);
- if (scrtc->engine->ops->mode_set)
scrtc->engine->ops->mode_set(scrtc->engine, mode);}
static const struct drm_crtc_helper_funcs sun4i_crtc_helper_funcs = { diff --git a/drivers/gpu/drm/sun4i/sunxi_engine.h b/drivers/gpu/drm/sun4i/sunxi_engine.h index 548710a936d5..44102783ee3c 100644 --- a/drivers/gpu/drm/sun4i/sunxi_engine.h +++ b/drivers/gpu/drm/sun4i/sunxi_engine.h @@ -9,6 +9,7 @@ struct drm_plane; struct drm_device; struct drm_crtc_state; +struct drm_display_mode;
struct sunxi_engine;
@@ -108,6 +109,17 @@ struct sunxi_engine_ops { * This function is optional. */ void (*vblank_quirk)(struct sunxi_engine *engine);
- /**
* @mode_set:** This callback is used to update engine registers that* responsible for display frame size other mode attributes.** This function is optional.*/- void (*mode_set)(struct sunxi_engine *engine,
struct drm_display_mode *mode);};
/**
From: Roman Stratiienko roman.stratiienko@globallogic.com
According to DRM documentation the only difference between PRIMARY and OVERLAY plane is that each CRTC must have PRIMARY plane and OVERLAY are optional.
Allow PRIMARY plane to have dimension different from full-screen.
Fixes: 90212fffa4fc ("drm/sun4i: Use values calculated by atomic check") Signed-off-by: Roman Stratiienko roman.stratiienko@globallogic.com --- v2: - Split commit in 2 parts - Add Fixes line to the commit message --- drivers/gpu/drm/sun4i/sun8i_mixer.c | 35 ++++++++++++++++++++++++++ drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 ---------------------- 2 files changed, 35 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index d306ad5dc093..5d90a95ff855 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -257,6 +257,40 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 format) return NULL; }
+static void sun8i_mode_set(struct sunxi_engine *engine, + struct drm_display_mode *mode) +{ + u32 dst_w = mode->crtc_hdisplay; + u32 dst_h = mode->crtc_vdisplay; + u32 outsize = SUN8I_MIXER_SIZE(dst_w, dst_h); + bool interlaced = false; + u32 val; + struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine); + u32 bld_base = sun8i_blender_base(mixer); + + DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n", + dst_w, dst_h); + regmap_write(mixer->engine.regs, + SUN8I_MIXER_GLOBAL_SIZE, + outsize); + regmap_write(mixer->engine.regs, + SUN8I_MIXER_BLEND_OUTSIZE(bld_base), outsize); + + interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE; + + if (interlaced) + val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED; + else + val = 0; + + regmap_update_bits(mixer->engine.regs, + SUN8I_MIXER_BLEND_OUTCTL(bld_base), + SUN8I_MIXER_BLEND_OUTCTL_INTERLACED, + val); + DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n", + interlaced ? "on" : "off"); +} + static void sun8i_mixer_commit(struct sunxi_engine *engine) { struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine); @@ -356,6 +390,7 @@ static struct drm_plane **sun8i_layers_init(struct drm_device *drm, static const struct sunxi_engine_ops sun8i_engine_ops = { .commit = sun8i_mixer_commit, .layers_init = sun8i_layers_init, + .mode_set = sun8i_mode_set, };
static struct regmap_config sun8i_mixer_regmap_config = { diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index ee7c13d8710f..23c2f4b68c89 100644 --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c @@ -72,36 +72,6 @@ static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel, insize = SUN8I_MIXER_SIZE(src_w, src_h); outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
- if (plane->type == DRM_PLANE_TYPE_PRIMARY) { - bool interlaced = false; - u32 val; - - DRM_DEBUG_DRIVER("Primary layer, updating global size W: %u H: %u\n", - dst_w, dst_h); - regmap_write(mixer->engine.regs, - SUN8I_MIXER_GLOBAL_SIZE, - outsize); - regmap_write(mixer->engine.regs, - SUN8I_MIXER_BLEND_OUTSIZE(bld_base), outsize); - - if (state->crtc) - interlaced = state->crtc->state->adjusted_mode.flags - & DRM_MODE_FLAG_INTERLACE; - - if (interlaced) - val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED; - else - val = 0; - - regmap_update_bits(mixer->engine.regs, - SUN8I_MIXER_BLEND_OUTCTL(bld_base), - SUN8I_MIXER_BLEND_OUTCTL_INTERLACED, - val); - - DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n", - interlaced ? "on" : "off"); - } - /* Set height and width */ DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n", state->src.x1 >> 16, state->src.y1 >> 16);
Hi!
Sorry that I missed few details in first review. Please take a look below.
Dne nedelja, 29. december 2019 ob 17:28:27 CET je roman.stratiienko@globallogic.com napisal(a):
From: Roman Stratiienko roman.stratiienko@globallogic.com
According to DRM documentation the only difference between PRIMARY and OVERLAY plane is that each CRTC must have PRIMARY plane and OVERLAY are optional.
Allow PRIMARY plane to have dimension different from full-screen.
Fixes: 90212fffa4fc ("drm/sun4i: Use values calculated by atomic check")
This fixes tag doesn't seem to be a good choice. First time where code in question was introduced was:
9d75b8c0b999 drm/sun4i: add support for Allwinner DE2 mixers
and it was later moved to sun8i_ui_layer.c in:
5bb5f5dafa1a drm/sun4i: Reorganize UI layer code in DE2
Not sure which one is better. You can also include both.
Signed-off-by: Roman Stratiienko roman.stratiienko@globallogic.com
v2:
- Split commit in 2 parts
- Add Fixes line to the commit message
drivers/gpu/drm/sun4i/sun8i_mixer.c | 35 ++++++++++++++++++++++++++ drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 30 ---------------------- 2 files changed, 35 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index d306ad5dc093..5d90a95ff855 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -257,6 +257,40 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 format) return NULL; }
+static void sun8i_mode_set(struct sunxi_engine *engine,
struct drm_display_mode *mode)+{
- u32 dst_w = mode->crtc_hdisplay;
- u32 dst_h = mode->crtc_vdisplay;
Now that you moved code in separate function, "dst_" prefix doesn't make sense anymore. Plain "width" and "height" work just fine.
- u32 outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
- bool interlaced = false;
No need to initialize above variable. This value is never used.
- u32 val;
- struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
- u32 bld_base = sun8i_blender_base(mixer);
Not extremely important, but can you move above two lines to the top? At least I prefer to have those lines sorted from longest to shortest as much as possible.
Once above comments are addressed, code is: Reviewed-by: Jernej Skrabec jernej.skrabec@siol.net
Best regards, Jernej
- DRM_DEBUG_DRIVER("Mode change, updating global size W: %u H: %u\n",
dst_w, dst_h);- regmap_write(mixer->engine.regs,
SUN8I_MIXER_GLOBAL_SIZE,outsize);- regmap_write(mixer->engine.regs,
SUN8I_MIXER_BLEND_OUTSIZE(bld_base), outsize);- interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE;
- if (interlaced)
val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;- else
val = 0;- regmap_update_bits(mixer->engine.regs,
SUN8I_MIXER_BLEND_OUTCTL(bld_base),SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,val);- DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
interlaced ? "on" : "off");+}
static void sun8i_mixer_commit(struct sunxi_engine *engine) { struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine); @@ -356,6 +390,7 @@ static struct drm_plane **sun8i_layers_init(struct drm_device *drm, static const struct sunxi_engine_ops sun8i_engine_ops = { .commit = sun8i_mixer_commit, .layers_init = sun8i_layers_init,
- .mode_set = sun8i_mode_set,
};
static struct regmap_config sun8i_mixer_regmap_config = { diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index ee7c13d8710f..23c2f4b68c89 100644 --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c @@ -72,36 +72,6 @@ static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel, insize = SUN8I_MIXER_SIZE(src_w, src_h); outsize = SUN8I_MIXER_SIZE(dst_w, dst_h);
- if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
bool interlaced = false;u32 val;DRM_DEBUG_DRIVER("Primary layer, updating global size
W: %u H: %u\n",
dst_w, dst_h);regmap_write(mixer->engine.regs,SUN8I_MIXER_GLOBAL_SIZE,outsize);regmap_write(mixer->engine.regs,SUN8I_MIXER_BLEND_OUTSIZE(bld_base),
outsize);
if (state->crtc)interlaced = state->crtc->state-adjusted_mode.flags
& DRM_MODE_FLAG_INTERLACE;if (interlaced)val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;elseval = 0;regmap_update_bits(mixer->engine.regs,
SUN8I_MIXER_BLEND_OUTCTL(bld_base),
SUN8I_MIXER_BLEND_OUTCTL_INTERLACED,
val);DRM_DEBUG_DRIVER("Switching display mixer interlaced
mode %s\n",
interlaced ? "on" : "off");- }
- /* Set height and width */ DRM_DEBUG_DRIVER("Layer source offset X: %d Y: %d\n", state->src.x1 >> 16, state->src.y1 >> 16);
From: Roman Stratiienko roman.stratiienko@globallogic.com
At system start blink of u-boot ghost framebuffer can be observed. Fix it.
Fixes: 9d75b8c0b999 ("drm/sun4i: add support for Allwinner DE2 mixers") Signed-off-by: Roman Stratiienko roman.stratiienko@globallogic.com Reviewed-by: Jernej Skrabec jernej.skrabec@siol.net --- v2: - Picked 'Reviewed-by' line - Added 'Fixes' line --- drivers/gpu/drm/sun4i/sun8i_mixer.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 5d90a95ff855..86711d8a9c84 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -576,6 +576,9 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base), SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
+ regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_DBUFF, + SUN8I_MIXER_GLOBAL_DBUFF_ENABLE); + return 0;
err_disable_bus_clk:
Please HOLD this until v3. I forgot about blender channel frame coords that updated according to zpos and can have gap between channels. I will also try to simplify this patch.
On Sun, Dec 29, 2019 at 6:28 PM roman.stratiienko@globallogic.com 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.
Fixes: f88c5ee77496 ("drm/sun4i: Implement zpos for DE2") Signed-off-by: Roman Stratiienko roman.stratiienko@globallogic.com
v2:
- Use SUN8I_MIXER_MAX_LAYERS macro
- Use plane_cnt instead of hard-coded number
- Put initialization of channel_zpos into for loop
- Add 'Fixes' line to the commit message
- Minor clean-ups
- Comments added
drivers/gpu/drm/sun4i/sun8i_mixer.c | 52 +++++++++++++++++++++++++- 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 | 39 +++---------------- 4 files changed, 66 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..d306ad5dc093 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -259,8 +259,54 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 format)
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 = 0;int channel_by_zpos[SUN8I_MIXER_MAX_LAYERS];u32 route = 0, pipe_ctl = 0;int plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num;DRM_DEBUG_DRIVER("Update blender routing\n");/* Assume that values in mixer->channel_zpos[] are unique except -1 */for (i = 0; i < plane_cnt; i++)channel_by_zpos[i] = -1;/* Sort by zpos */for (i = 0; i < plane_cnt; i++) {int zpos = mixer->channel_zpos[i];if (zpos >= 0 && zpos < plane_cnt)channel_by_zpos[zpos] = i;}/* Route enabled blending channels first */for (i = 0; i < plane_cnt; 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 remaining routing fields to match disabled channel indices */for (i = 0; i < SUN8I_MIXER_MAX_LAYERS && j < SUN8I_MIXER_MAX_LAYERS;i++) {if (mixer->channel_zpos[i] < 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);} @@ -485,10 +531,12 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, 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);diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index c6cc94057faf..b193d9d1db66 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -13,6 +13,8 @@ #include "sun8i_csc.h" #include "sunxi_engine.h"
+#define SUN8I_MIXER_MAX_LAYERS 5
#define SUN8I_MIXER_SIZE(w, h) (((h) - 1) << 16 | ((w) - 1)) #define SUN8I_MIXER_COORD(x, y) ((y) << 16 | (x))
@@ -176,6 +178,9 @@ struct sun8i_mixer {
struct clk *bus_clk; struct clk *mod_clk;
/* -1 means that layer is disabled */int channel_zpos[SUN8I_MIXER_MAX_LAYERS];};
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 c87fd842918e..ee7c13d8710f 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, @@ -265,11 +238,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, @@ -277,12 +248,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; }@@ -293,7 +263,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 = {
2.17.1
False alarm. Since DRM sets normalized ZPOS there should be no gaps and all should work as expected.
On Mon, Dec 30, 2019 at 1:25 PM Roman Stratiienko roman.stratiienko@globallogic.com wrote:
Please HOLD this until v3. I forgot about blender channel frame coords that updated according to zpos and can have gap between channels. I will also try to simplify this patch.
On Sun, Dec 29, 2019 at 6:28 PM roman.stratiienko@globallogic.com 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.
Fixes: f88c5ee77496 ("drm/sun4i: Implement zpos for DE2") Signed-off-by: Roman Stratiienko roman.stratiienko@globallogic.com
v2:
- Use SUN8I_MIXER_MAX_LAYERS macro
- Use plane_cnt instead of hard-coded number
- Put initialization of channel_zpos into for loop
- Add 'Fixes' line to the commit message
- Minor clean-ups
- Comments added
drivers/gpu/drm/sun4i/sun8i_mixer.c | 52 +++++++++++++++++++++++++- 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 | 39 +++---------------- 4 files changed, 66 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..d306ad5dc093 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -259,8 +259,54 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 format)
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 = 0;int channel_by_zpos[SUN8I_MIXER_MAX_LAYERS];u32 route = 0, pipe_ctl = 0;int plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num;DRM_DEBUG_DRIVER("Update blender routing\n");/* Assume that values in mixer->channel_zpos[] are unique except -1 */for (i = 0; i < plane_cnt; i++)channel_by_zpos[i] = -1;/* Sort by zpos */for (i = 0; i < plane_cnt; i++) {int zpos = mixer->channel_zpos[i];if (zpos >= 0 && zpos < plane_cnt)channel_by_zpos[zpos] = i;}/* Route enabled blending channels first */for (i = 0; i < plane_cnt; 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 remaining routing fields to match disabled channel indices */for (i = 0; i < SUN8I_MIXER_MAX_LAYERS && j < SUN8I_MIXER_MAX_LAYERS;i++) {if (mixer->channel_zpos[i] < 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);} @@ -485,10 +531,12 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, 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);diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index c6cc94057faf..b193d9d1db66 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -13,6 +13,8 @@ #include "sun8i_csc.h" #include "sunxi_engine.h"
+#define SUN8I_MIXER_MAX_LAYERS 5
#define SUN8I_MIXER_SIZE(w, h) (((h) - 1) << 16 | ((w) - 1)) #define SUN8I_MIXER_COORD(x, y) ((y) << 16 | (x))
@@ -176,6 +178,9 @@ struct sun8i_mixer {
struct clk *bus_clk; struct clk *mod_clk;
/* -1 means that layer is disabled */int channel_zpos[SUN8I_MIXER_MAX_LAYERS];};
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 c87fd842918e..ee7c13d8710f 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, @@ -265,11 +238,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, @@ -277,12 +248,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; }@@ -293,7 +263,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 = {
2.17.1
-- Best regards, Roman Stratiienko Global Logic Inc.
Hi!
Dne nedelja, 29. december 2019 ob 17:28:25 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.
Fixes: f88c5ee77496 ("drm/sun4i: Implement zpos for DE2") Signed-off-by: Roman Stratiienko roman.stratiienko@globallogic.com
v2:
- Use SUN8I_MIXER_MAX_LAYERS macro
- Use plane_cnt instead of hard-coded number
- Put initialization of channel_zpos into for loop
- Add 'Fixes' line to the commit message
- Minor clean-ups
- Comments added
drivers/gpu/drm/sun4i/sun8i_mixer.c | 52 +++++++++++++++++++++++++- 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 | 39 +++---------------- 4 files changed, 66 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 8b803eb903b8..d306ad5dc093 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -259,8 +259,54 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 format)
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 = 0;
- int channel_by_zpos[SUN8I_MIXER_MAX_LAYERS];
- u32 route = 0, pipe_ctl = 0;
- int plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num;
Can you please sort above lines by size (longest to shortest) as much as possible?
- DRM_DEBUG_DRIVER("Update blender routing\n");
- /* Assume that values in mixer->channel_zpos[] are unique except -1
*/
I would remove "Assume that" because it's guaranteed by zpos normalization in DRM core.
for (i = 0; i < plane_cnt; i++)
channel_by_zpos[i] = -1;/* Sort by zpos */
for (i = 0; i < plane_cnt; i++) {
int zpos = mixer->channel_zpos[i];if (zpos >= 0 && zpos < plane_cnt)channel_by_zpos[zpos] = i;}
/* Route enabled blending channels first */
for (i = 0; i < plane_cnt; 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 remaining routing fields to match disabled channel indices */
- for (i = 0; i < SUN8I_MIXER_MAX_LAYERS && j <
SUN8I_MIXER_MAX_LAYERS;
i++) {if (mixer->channel_zpos[i] < 0) {route |= i <<
SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(j);
j++;}- }
Is above for loop really necessary? If pipe is not enabled I don't think you need to set a route.
- 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);
} @@ -485,10 +531,12 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master, 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);diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index c6cc94057faf..b193d9d1db66 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -13,6 +13,8 @@ #include "sun8i_csc.h" #include "sunxi_engine.h"
+#define SUN8I_MIXER_MAX_LAYERS 5
#define SUN8I_MIXER_SIZE(w, h) (((h) - 1) << 16 |
((w) - 1))
#define SUN8I_MIXER_COORD(x, y) ((y) << 16 | (x))
@@ -176,6 +178,9 @@ struct sun8i_mixer {
struct clk *bus_clk; struct clk *mod_clk;
- /* -1 means that layer is disabled */
- int channel_zpos[SUN8I_MIXER_MAX_LAYERS];
};
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 c87fd842918e..ee7c13d8710f 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, @@ -265,11 +238,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, @@ -277,12 +248,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);@@ -293,7 +263,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
You should remove bld_base in this function too, now that it's not used anymore.
Best regards, Jernej
*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);@@ -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 = {
dri-devel@lists.freedesktop.org
-
Jernej Škrabec -
Roman Stratiienko -
roman.stratiienko＠globallogic.com