* [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic
@ 2019-12-29 16:28 roman.stratiienko
2019-12-29 16:28 ` [PATCH v2 2/4] drm/sun4i: Add mode_set callback to the engine roman.stratiienko
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: roman.stratiienko @ 2019-12-29 16:28 UTC (permalink / raw)
To: mripard, dri-devel, linux-arm-kernel, linux-kernel, jernej.skrabec
Cc: Roman Stratiienko
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/4] drm/sun4i: Add mode_set callback to the engine
2019-12-29 16:28 [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic roman.stratiienko
@ 2019-12-29 16:28 ` roman.stratiienko
2019-12-31 11:24 ` Jernej Škrabec
2019-12-29 16:28 ` [PATCH v2 3/4] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame roman.stratiienko
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: roman.stratiienko @ 2019-12-29 16:28 UTC (permalink / raw)
To: mripard, dri-devel, linux-arm-kernel, linux-kernel, jernej.skrabec
Cc: Roman Stratiienko
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);
};
/**
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/4] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame.
2019-12-29 16:28 [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic roman.stratiienko
2019-12-29 16:28 ` [PATCH v2 2/4] drm/sun4i: Add mode_set callback to the engine roman.stratiienko
@ 2019-12-29 16:28 ` roman.stratiienko
2019-12-31 12:06 ` Jernej Škrabec
2019-12-29 16:28 ` [PATCH v2 4/4] drm/sun4i: Update mixer's internal registers after initialization roman.stratiienko
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: roman.stratiienko @ 2019-12-29 16:28 UTC (permalink / raw)
To: mripard, dri-devel, linux-arm-kernel, linux-kernel, jernej.skrabec
Cc: Roman Stratiienko
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);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 4/4] drm/sun4i: Update mixer's internal registers after initialization
2019-12-29 16:28 [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic roman.stratiienko
2019-12-29 16:28 ` [PATCH v2 2/4] drm/sun4i: Add mode_set callback to the engine roman.stratiienko
2019-12-29 16:28 ` [PATCH v2 3/4] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame roman.stratiienko
@ 2019-12-29 16:28 ` roman.stratiienko
2019-12-30 11:25 ` [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic Roman Stratiienko
2019-12-31 13:52 ` Jernej Škrabec
4 siblings, 0 replies; 9+ messages in thread
From: roman.stratiienko @ 2019-12-29 16:28 UTC (permalink / raw)
To: mripard, dri-devel, linux-arm-kernel, linux-kernel, jernej.skrabec
Cc: Roman Stratiienko
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:
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic
2019-12-29 16:28 [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic roman.stratiienko
` (2 preceding siblings ...)
2019-12-29 16:28 ` [PATCH v2 4/4] drm/sun4i: Update mixer's internal registers after initialization roman.stratiienko
@ 2019-12-30 11:25 ` Roman Stratiienko
2019-12-30 17:39 ` Roman Stratiienko
2019-12-31 13:52 ` Jernej Škrabec
4 siblings, 1 reply; 9+ messages in thread
From: Roman Stratiienko @ 2019-12-30 11:25 UTC (permalink / raw)
To: Maxime Ripard, dri-devel, linux-arm-kernel, linux-kernel,
Jernej Škrabec
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.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic
2019-12-30 11:25 ` [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic Roman Stratiienko
@ 2019-12-30 17:39 ` Roman Stratiienko
0 siblings, 0 replies; 9+ messages in thread
From: Roman Stratiienko @ 2019-12-30 17:39 UTC (permalink / raw)
To: Maxime Ripard, dri-devel, linux-arm-kernel, linux-kernel,
Jernej Škrabec
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.
--
Best regards,
Roman Stratiienko
Global Logic Inc.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/4] drm/sun4i: Add mode_set callback to the engine
2019-12-29 16:28 ` [PATCH v2 2/4] drm/sun4i: Add mode_set callback to the engine roman.stratiienko
@ 2019-12-31 11:24 ` Jernej Škrabec
0 siblings, 0 replies; 9+ messages in thread
From: Jernej Škrabec @ 2019-12-31 11:24 UTC (permalink / raw)
To: mripard, dri-devel, linux-arm-kernel, linux-kernel, roman.stratiienko
Cc: Roman Stratiienko
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);
> };
>
> /**
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/4] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame.
2019-12-29 16:28 ` [PATCH v2 3/4] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame roman.stratiienko
@ 2019-12-31 12:06 ` Jernej Škrabec
0 siblings, 0 replies; 9+ messages in thread
From: Jernej Škrabec @ 2019-12-31 12:06 UTC (permalink / raw)
To: mripard, dri-devel, linux-arm-kernel, linux-kernel, roman.stratiienko
Cc: Roman Stratiienko
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;
> - 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);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic
2019-12-29 16:28 [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic roman.stratiienko
` (3 preceding siblings ...)
2019-12-30 11:25 ` [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic Roman Stratiienko
@ 2019-12-31 13:52 ` Jernej Škrabec
4 siblings, 0 replies; 9+ messages in thread
From: Jernej Škrabec @ 2019-12-31 13:52 UTC (permalink / raw)
To: mripard, dri-devel, linux-arm-kernel, linux-kernel, roman.stratiienko
Cc: Roman Stratiienko
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);
> 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
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);
> 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 = {
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-12-31 13:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-29 16:28 [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic roman.stratiienko
2019-12-29 16:28 ` [PATCH v2 2/4] drm/sun4i: Add mode_set callback to the engine roman.stratiienko
2019-12-31 11:24 ` Jernej Škrabec
2019-12-29 16:28 ` [PATCH v2 3/4] drm/sun4i: Use CRTC size instead of PRIMARY plane size as mixer frame roman.stratiienko
2019-12-31 12:06 ` Jernej Škrabec
2019-12-29 16:28 ` [PATCH v2 4/4] drm/sun4i: Update mixer's internal registers after initialization roman.stratiienko
2019-12-30 11:25 ` [PATCH v2 1/4] drm/sun4i: Reimplement plane z position setting logic Roman Stratiienko
2019-12-30 17:39 ` Roman Stratiienko
2019-12-31 13:52 ` Jernej Škrabec
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).