Patches 1-2 already reviewed and ready to be applied.
Patch 4 is RFC and require more testing on real hardware.
[PATCH v4 1/4] drm/sun4i: Add alpha property for sun8i UI layer
[PATCH v4 2/4] drm/sun4i: Add alpha property for sun8i and sun50i VI
[PATCH v4 3/4] drm/sun4i: Add support for premulti/coverage blending
[PATCH v4 4/4] RFC: drm/sun4i: Process alpha channel of most bottom layer
drivers/gpu/drm/sun4i/sun8i_mixer.h | 2 +
drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 50 ++++++++++++++++
drivers/gpu/drm/sun4i/sun8i_ui_layer.h | 10 ++++
drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 72 +++++++++++++++++++++---
drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 16 ++++++
5 files changed, 142 insertions(+), 8 deletions(-)
DE2.0 and DE3.0 UI layers supports plane-global alpha channel.
Add alpha property to the DRM plane and connect it to the
corresponding registers in mixer.
Signed-off-by: Roman Stratiienko <[email protected]>
Reviewed-by: Jernej Skrabec <[email protected]>
---
v2: Initial commit by mistake
v3:
- Picked `reviewed-by` line
V4:
- Changed author e-mail to avoid mail rejecting.
---
drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 29 ++++++++++++++++++++++++++
drivers/gpu/drm/sun4i/sun8i_ui_layer.h | 5 +++++
2 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 19f42004cebe..5278032567a3 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -72,6 +72,27 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel,
}
}
+static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
+ int overlay, struct drm_plane *plane)
+{
+ u32 mask, val, ch_base;
+
+ ch_base = sun8i_channel_base(mixer, channel);
+
+ mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
+ SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
+
+ val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(plane->state->alpha >> 8);
+
+ val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+ SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+ SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+ regmap_update_bits(mixer->engine.regs,
+ SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
+ mask, val);
+}
+
static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
int overlay, struct drm_plane *plane,
unsigned int zpos)
@@ -258,6 +279,8 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane *plane,
sun8i_ui_layer_update_coord(mixer, layer->channel,
layer->overlay, plane, zpos);
+ sun8i_ui_layer_update_alpha(mixer, layer->channel,
+ layer->overlay, plane);
sun8i_ui_layer_update_formats(mixer, layer->channel,
layer->overlay, plane);
sun8i_ui_layer_update_buffer(mixer, layer->channel,
@@ -332,6 +355,12 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct drm_device *drm,
plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
+ ret = drm_plane_create_alpha_property(&layer->plane);
+ if (ret) {
+ dev_err(drm->dev, "Couldn't add alpha property\n");
+ return ERR_PTR(ret);
+ }
+
ret = drm_plane_create_zpos_property(&layer->plane, channel,
0, plane_cnt - 1);
if (ret) {
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
index f4ab1cf6cded..e3e32ee1178d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
@@ -40,6 +40,11 @@
#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_MASK GENMASK(12, 8)
#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_OFFSET 8
#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK GENMASK(31, 24)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA(x) ((x) << 24)
+
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL ((0) << 1)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER ((1) << 1)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
struct sun8i_mixer;
--
2.17.1
Allwinner display engine blender consists of 3 pipelined blending units.
PIPE0->\
BLD0-\
PIPE1->/ BLD1-\
PIPE2->------/ BLD2->OUT
PIPE3->------------/
This pipeline produces incorrect composition if PIPE0 buffer has alpha.
Correct solution is to add one more blending step and mix PIPE0 with
background, but it is not supported by the hardware.
Use premultiplied alpha buffer of PIPE0 overlay channel as is.
In this case we got same effect as mixing PIPE0 with black background.
Signed-off-by: Roman Stratiienko <[email protected]>
---
v4:
- Initial version, depends on other unmerged patches in the patchset.
---
drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 2 +-
drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index dd6145f80c36..d94f4d8b9128 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -106,7 +106,7 @@ static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
regmap_update_bits(mixer->engine.regs,
SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
- SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos));
+ zpos ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
}
static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index e6d8a539614f..68a6843db4ab 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -108,7 +108,7 @@ static void sun8i_vi_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
regmap_update_bits(mixer->engine.regs,
SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
- (mixer->cfg->is_de3) ?
+ (zpos != 0 && mixer->cfg->is_de3) ?
SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
}
--
2.17.1
On Mon, Mar 02, 2020 at 12:31:38PM +0200, Roman Stratiienko wrote:
> Allwinner display engine blender consists of 3 pipelined blending units.
>
> PIPE0->\
> BLD0-\
> PIPE1->/ BLD1-\
> PIPE2->------/ BLD2->OUT
> PIPE3->------------/
>
> This pipeline produces incorrect composition if PIPE0 buffer has alpha.
Why? What happens in that case?
> Correct solution is to add one more blending step and mix PIPE0 with
> background, but it is not supported by the hardware.
>
> Use premultiplied alpha buffer of PIPE0 overlay channel as is.
> In this case we got same effect as mixing PIPE0 with black background.
>
> Signed-off-by: Roman Stratiienko <[email protected]>
>
> ---
>
> v4:
> - Initial version, depends on other unmerged patches in the patchset.
> ---
> drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 2 +-
> drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> index dd6145f80c36..d94f4d8b9128 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -106,7 +106,7 @@ static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
> regmap_update_bits(mixer->engine.regs,
> SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
> SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
> - SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos));
> + zpos ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
Can you really use the zpos here? What happens if the zpos doesn't
match the pipe?
Maxime
Hi!
Dne ponedeljek, 02. marec 2020 ob 11:31:38 CET je Roman Stratiienko
napisal(a):
> Allwinner display engine blender consists of 3 pipelined blending units.
>
> PIPE0->\
> BLD0-\
> PIPE1->/ BLD1-\
> PIPE2->------/ BLD2->OUT
> PIPE3->------------/
>
> This pipeline produces incorrect composition if PIPE0 buffer has alpha.
I always thought that if bottom layer has alpha, it's blended with background
color, which is set to opaque black. If that is not the case, can you solve
this by changing blending formula located in BLD control registers (offsets
0x90, 0x94, 0x98 and 0x9c)?
Best regards,
Jernej
> Correct solution is to add one more blending step and mix PIPE0 with
> background, but it is not supported by the hardware.
>
> Use premultiplied alpha buffer of PIPE0 overlay channel as is.
> In this case we got same effect as mixing PIPE0 with black background.
>
> Signed-off-by: Roman Stratiienko <[email protected]>
>
> ---
>
> v4:
> - Initial version, depends on other unmerged patches in the patchset.
> ---
> drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 2 +-
> drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index dd6145f80c36..d94f4d8b9128
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -106,7 +106,7 @@ static void sun8i_ui_layer_update_alpha(struct
> sun8i_mixer *mixer, int channel, regmap_update_bits(mixer->engine.regs,
> SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
> SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
> - SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos));
> + zpos ?
SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
> }
>
> static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int
> channel, diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index e6d8a539614f..68a6843db4ab
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> @@ -108,7 +108,7 @@ static void sun8i_vi_layer_update_alpha(struct
> sun8i_mixer *mixer, int channel, regmap_update_bits(mixer->engine.regs,
> SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
> SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
> - (mixer->cfg->is_de3) ?
> + (zpos != 0 && mixer->cfg->is_de3) ?
>
SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
>
> }