.[PATCH v4 0/4] drm/sun4i: Improve alpha processing

.[PATCH v4 0/4] drm/sun4i: Improve alpha processing

[Roman Stratiienko]

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(-)

[PATCH v4 1/4] drm/sun4i: Add alpha property for sun8i UI layer

[Roman Stratiienko]

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 <r.stratiienko@gmail.com>
Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
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

[PATCH v4 2/4] drm/sun4i: Add alpha property for sun8i and sun50i VI layer

[Roman Stratiienko]

DE3.0 VI layers supports plane-global alpha channel.
DE2.0 FCC block have GLOBAL_ALPHA register that can be used as alpha source
for blender.

Add alpha property to the DRM plane and connect it to the
corresponding registers in the mixer.

Do not add alpha property for V3s SOC that have DE2.0 and 2 VI planes.

Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>
Reviewed-by: Jernej Skrabec <jernej.skrabec@siol.net>

---
v2: Initial version by mistake
v3:
- Skip adding & applying alpha property if VI count > 1
V4:
- Changed author e-mail address to avoid mail rejecting
- Picked reviewed-by line
---
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 48 +++++++++++++++++++++-----
 drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 11 ++++++
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index 2344938be3e4..f2469b5e97ee 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -65,6 +65,36 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel,
 	}
 }
 
+static void sun8i_vi_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);
+
+	if (mixer->cfg->is_de3) {
+		mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
+		       SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK;
+		val = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA
+			(plane->state->alpha >> 8);
+
+		val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+			SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+			SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+		regmap_update_bits(mixer->engine.regs,
+				   SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
+								  overlay),
+				   mask, val);
+	} else if (mixer->cfg->vi_num == 1) {
+		regmap_update_bits(mixer->engine.regs,
+				   SUN8I_MIXER_FCC_GLOBAL_ALPHA_REG,
+				   SUN8I_MIXER_FCC_GLOBAL_ALPHA_MASK,
+				   SUN8I_MIXER_FCC_GLOBAL_ALPHA
+					(plane->state->alpha >> 8));
+	}
+}
+
 static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int channel,
 				       int overlay, struct drm_plane *plane,
 				       unsigned int zpos)
@@ -248,14 +278,6 @@ static int sun8i_vi_layer_update_formats(struct sun8i_mixer *mixer, int channel,
 			   SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay),
 			   SUN8I_MIXER_CHAN_VI_LAYER_ATTR_RGB_MODE, val);
 
-	/* It seems that YUV formats use global alpha setting. */
-	if (mixer->cfg->is_de3)
-		regmap_update_bits(mixer->engine.regs,
-				   SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
-								  overlay),
-				   SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK,
-				   SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA(0xff));
-
 	return 0;
 }
 
@@ -373,6 +395,8 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane *plane,
 
 	sun8i_vi_layer_update_coord(mixer, layer->channel,
 				    layer->overlay, plane, zpos);
+	sun8i_vi_layer_update_alpha(mixer, layer->channel,
+				    layer->overlay, plane);
 	sun8i_vi_layer_update_formats(mixer, layer->channel,
 				      layer->overlay, plane);
 	sun8i_vi_layer_update_buffer(mixer, layer->channel,
@@ -472,6 +496,14 @@ struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct drm_device *drm,
 
 	plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
 
+	if (mixer->cfg->vi_num == 1 || mixer->cfg->is_de3) {
+		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, index,
 					     0, plane_cnt - 1);
 	if (ret) {
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
index eaa6076f5dbc..48c399e1c86d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
@@ -29,14 +29,25 @@
 #define SUN8I_MIXER_CHAN_VI_VDS_UV(base) \
 		((base) + 0xfc)
 
+#define SUN8I_MIXER_FCC_GLOBAL_ALPHA_REG \
+		(0xAA000 + 0x90)
+
+#define SUN8I_MIXER_FCC_GLOBAL_ALPHA(x)			((x) << 24)
+#define SUN8I_MIXER_FCC_GLOBAL_ALPHA_MASK		GENMASK(31, 24)
+
 #define SUN8I_MIXER_CHAN_VI_LAYER_ATTR_EN		BIT(0)
 /* RGB mode should be set for RGB formats and cleared for YCbCr */
 #define SUN8I_MIXER_CHAN_VI_LAYER_ATTR_RGB_MODE		BIT(15)
 #define SUN8I_MIXER_CHAN_VI_LAYER_ATTR_FBFMT_OFFSET	8
 #define SUN8I_MIXER_CHAN_VI_LAYER_ATTR_FBFMT_MASK	GENMASK(12, 8)
+#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK	GENMASK(2, 1)
 #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK	GENMASK(31, 24)
 #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA(x)	((x) << 24)
 
+#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL	((0) << 1)
+#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_LAYER	((1) << 1)
+#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED	((2) << 1)
+
 #define SUN8I_MIXER_CHAN_VI_DS_N(x)			((x) << 16)
 #define SUN8I_MIXER_CHAN_VI_DS_M(x)			((x) << 0)
 
-- 
2.17.1

[PATCH v4 3/4] drm/sun4i: Add support for premulti/coverage blending mode handling

[Roman Stratiienko]

Argument:

All information below is author's understanding of underlying processes.
This information was obtained from DE2.0-3.0 datasheet analysis and
analysis of different DRM driver source code and commit messages.

Input buffers could have 2 ways of holding pixel alpha channel value.
1. Coverage means that in case of transparency - only alpha channel changes.
   Example of 50% transparent white pixel: R=0xff B=0xff B=0xff A=0x7f

2. Premultiply means that RGB pixel values should be dimmed proportional to
   alpha channel value. Alpha channel value has also to be set in this case.
   Example of 50% transparent white pixel: R=0x7f B=0x7f B=0x7f A=0x7f

Userspace informs DRM/KMS which blending type frame buffer uses with
'pixel blend mode' property.

Allwinner DE2.0 and DE3.0 have 2 block of image processing:
Overlay and Blending. Both should aware of blending type are used in the buffer.

Overlay block uses this information to:
1. Unify blending types if more then 1 overlay channel are used. It can unify it
   only as premultiplied by converting coverage to premultiplied.
2. Calculate correct pixel value in case of applying plane alpha.
   For coverage alpha only alpha channel should be affected:
     [Ro=Ri, Go=Gi, Bo=Bi, Ao=Ai*AGlobal]
   For premultiplied alpha all 4 channels should be affected:
     [Ro=Ri*AGlobal, Go=Gi*AGlobal, Bo=Bi*AGlobal, Ao=Ai*AGlobal]

Blending functional block should aware of blending type each pipe
channel uses. Otherwise image can't blend correctly.

In case we've specified premultiplied format for blending PIPE0, blender
converts premultiplied RGB values to original (divides by normalized Alpha).
In case for some reason pixel value after division exceeds 0xff, blender clamps
it to 0xff. [Was discovered in experimental way]
If image that passed through PIPE1-3 restored to coverage before mixing or used
in premultiplied form still require testing and out of scope of this patch.

Implementation:

1. Add blend property to UI channel
2. Add blend property to VI channel in case of DE3.0 used
3. Make all DE2.0 UI and DE3.0 VI overlay channels to use premultiply format.
   Mark all blending pipes as premultiply except DE2.0 VI plane.
4. If DRM plane uses coverage blending format, set blending mode register
   to convert it to premultiply.

Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>

--

v4:
- Initial version (Depends on unmerged patches from patchset)
---
 drivers/gpu/drm/sun4i/sun8i_mixer.h    |  2 ++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 33 ++++++++++++++++++-----
 drivers/gpu/drm/sun4i/sun8i_ui_layer.h |  5 ++++
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 36 +++++++++++++++++++++-----
 drivers/gpu/drm/sun4i/sun8i_vi_layer.h |  5 ++++
 5 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index 915479cc3077..8a18372938d5 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -70,6 +70,8 @@
 #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n)	(0xf << ((n) << 2))
 #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)	((n) << 2)
 
+#define SUN8I_MIXER_BLEND_PREMULTIPLY_EN(pipe)	BIT(pipe)
+
 #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACED	BIT(1)
 
 #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)	BIT(ch)
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 5278032567a3..dd6145f80c36 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -73,10 +73,12 @@ 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)
+					int overlay, struct drm_plane *plane,
+					unsigned int zpos)
 {
-	u32 mask, val, ch_base;
+	u32 mask, val, ch_base, bld_base;
 
+	bld_base = sun8i_blender_base(mixer);
 	ch_base = sun8i_channel_base(mixer, channel);
 
 	mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
@@ -84,13 +86,27 @@ static void sun8i_ui_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
 
 	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;
+	if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
+		val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER;
+	} else {
+		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;
+
+		if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_COVERAGE)
+			val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREMUL;
+		else
+			val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI;
+	}
 
 	regmap_update_bits(mixer->engine.regs,
 			   SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
 			   mask, val);
+
+	regmap_update_bits(mixer->engine.regs,
+			   SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
+			   SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
+			   SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos));
 }
 
 static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
@@ -280,7 +296,7 @@ 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);
+				    layer->overlay, plane, zpos);
 	sun8i_ui_layer_update_formats(mixer, layer->channel,
 				      layer->overlay, plane);
 	sun8i_ui_layer_update_buffer(mixer, layer->channel,
@@ -361,6 +377,11 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct drm_device *drm,
 		return ERR_PTR(ret);
 	}
 
+	drm_plane_create_blend_mode_property(&layer->plane,
+					     BIT(DRM_MODE_BLEND_PREMULTI) |
+					     BIT(DRM_MODE_BLEND_COVERAGE) |
+					     BIT(DRM_MODE_BLEND_PIXEL_NONE));
+
 	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 e3e32ee1178d..c5136f4841bc 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
@@ -41,6 +41,11 @@
 #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_BLEND_MASK	GENMASK(16, 17)
+
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COVERAGE		((0) << 16)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREMUL		((1) << 16)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI		((2) << 16)
 
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL		((0) << 1)
 #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER		((1) << 1)
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index f2469b5e97ee..e6d8a539614f 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -66,11 +66,13 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel,
 }
 
 static void sun8i_vi_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
-					int overlay, struct drm_plane *plane)
+					int overlay, struct drm_plane *plane,
+					unsigned int zpos)
 {
-	u32 mask, val, ch_base;
+	u32 mask, val, ch_base, bld_base;
 
 	ch_base = sun8i_channel_base(mixer, channel);
+	bld_base = sun8i_blender_base(mixer);
 
 	if (mixer->cfg->is_de3) {
 		mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
@@ -78,9 +80,18 @@ static void sun8i_vi_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
 		val = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA
 			(plane->state->alpha >> 8);
 
-		val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
-			SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL :
-			SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+		if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
+			val |= SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_LAYER;
+		} else {
+			val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) ?
+				SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL :
+				SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED;
+
+			if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_COVERAGE)
+				val |= SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREMUL;
+			else
+				val |= SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_PREMULTI;
+		}
 
 		regmap_update_bits(mixer->engine.regs,
 				   SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
@@ -93,6 +104,13 @@ static void sun8i_vi_layer_update_alpha(struct sun8i_mixer *mixer, int channel,
 				   SUN8I_MIXER_FCC_GLOBAL_ALPHA
 					(plane->state->alpha >> 8));
 	}
+
+	regmap_update_bits(mixer->engine.regs,
+			   SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
+			   SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
+			   (mixer->cfg->is_de3) ?
+				SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
+
 }
 
 static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int channel,
@@ -396,7 +414,7 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane *plane,
 	sun8i_vi_layer_update_coord(mixer, layer->channel,
 				    layer->overlay, plane, zpos);
 	sun8i_vi_layer_update_alpha(mixer, layer->channel,
-				    layer->overlay, plane);
+				    layer->overlay, plane, zpos);
 	sun8i_vi_layer_update_formats(mixer, layer->channel,
 				      layer->overlay, plane);
 	sun8i_vi_layer_update_buffer(mixer, layer->channel,
@@ -504,6 +522,12 @@ struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct drm_device *drm,
 		}
 	}
 
+	if (mixer->cfg->is_de3)
+		drm_plane_create_blend_mode_property
+			(&layer->plane, BIT(DRM_MODE_BLEND_PREMULTI) |
+			 BIT(DRM_MODE_BLEND_COVERAGE) |
+			 BIT(DRM_MODE_BLEND_PIXEL_NONE));
+
 	ret = drm_plane_create_zpos_property(&layer->plane, index,
 					     0, plane_cnt - 1);
 	if (ret) {
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
index 48c399e1c86d..a1cf0ff16543 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
@@ -43,6 +43,11 @@
 #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK	GENMASK(2, 1)
 #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK	GENMASK(31, 24)
 #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA(x)	((x) << 24)
+#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_MASK	GENMASK(16, 17)
+
+#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COVERAGE		((0) << 16)
+#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREMUL	((1) << 16)
+#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_PREMULTI		((2) << 16)
 
 #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL	((0) << 1)
 #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_LAYER	((1) << 1)
-- 
2.17.1

Re: [PATCH v4 3/4] drm/sun4i: Add support for premulti/coverage blending mode handling

[Jernej Škrabec]

Hi!

Dne ponedeljek, 02. marec 2020 ob 11:31:37 CET je Roman Stratiienko 
napisal(a):
> Argument:
> 
> All information below is author's understanding of underlying processes.
> This information was obtained from DE2.0-3.0 datasheet analysis and
> analysis of different DRM driver source code and commit messages.
> 
> Input buffers could have 2 ways of holding pixel alpha channel value.
> 1. Coverage means that in case of transparency - only alpha channel changes.
> Example of 50% transparent white pixel: R=0xff B=0xff B=0xff A=0x7f
> 
> 2. Premultiply means that RGB pixel values should be dimmed proportional to
>    alpha channel value. Alpha channel value has also to be set in this case.
> Example of 50% transparent white pixel: R=0x7f B=0x7f B=0x7f A=0x7f
> 
> Userspace informs DRM/KMS which blending type frame buffer uses with
> 'pixel blend mode' property.
> 
> Allwinner DE2.0 and DE3.0 have 2 block of image processing:
> Overlay and Blending. Both should aware of blending type are used in the
> buffer.
> 
> Overlay block uses this information to:
> 1. Unify blending types if more then 1 overlay channel are used. It can
> unify it only as premultiplied by converting coverage to premultiplied.
> 2. Calculate correct pixel value in case of applying plane alpha.
>    For coverage alpha only alpha channel should be affected:
>      [Ro=Ri, Go=Gi, Bo=Bi, Ao=Ai*AGlobal]
>    For premultiplied alpha all 4 channels should be affected:
>      [Ro=Ri*AGlobal, Go=Gi*AGlobal, Bo=Bi*AGlobal, Ao=Ai*AGlobal]
> 
> Blending functional block should aware of blending type each pipe
> channel uses. Otherwise image can't blend correctly.
> 
> In case we've specified premultiplied format for blending PIPE0, blender
> converts premultiplied RGB values to original (divides by normalized Alpha).
> In case for some reason pixel value after division exceeds 0xff, blender
> clamps it to 0xff. [Was discovered in experimental way]
> If image that passed through PIPE1-3 restored to coverage before mixing or
> used in premultiplied form still require testing and out of scope of this
> patch.
> 
> Implementation:
> 
> 1. Add blend property to UI channel
> 2. Add blend property to VI channel in case of DE3.0 used
> 3. Make all DE2.0 UI and DE3.0 VI overlay channels to use premultiply
> format. Mark all blending pipes as premultiply except DE2.0 VI plane.
> 4. If DRM plane uses coverage blending format, set blending mode register
>    to convert it to premultiply.

Regarding 3 and 4 - do we really need to convert everything to premultiply? 
Currently only one overlay is used in each channel, so there is no need to 
convert coverage to premultiply if I understand datasheet correctly. Just mark 
it accordingly in BLD premultiply control register.

> 
> Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>
> 
> --
> 
> v4:
> - Initial version (Depends on unmerged patches from patchset)
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.h    |  2 ++
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 33 ++++++++++++++++++-----
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.h |  5 ++++
>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 36 +++++++++++++++++++++-----
>  drivers/gpu/drm/sun4i/sun8i_vi_layer.h |  5 ++++
>  5 files changed, 69 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> b/drivers/gpu/drm/sun4i/sun8i_mixer.h index 915479cc3077..8a18372938d5
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> @@ -70,6 +70,8 @@
>  #define SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(n)	(0xf << ((n) << 2))
>  #define SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(n)	((n) << 2)
> 
> +#define SUN8I_MIXER_BLEND_PREMULTIPLY_EN(pipe)	BIT(pipe)
> +
>  #define SUN8I_MIXER_BLEND_OUTCTL_INTERLACED	BIT(1)
> 
>  #define SUN50I_MIXER_BLEND_CSC_CTL_EN(ch)	BIT(ch)
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 5278032567a3..dd6145f80c36
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -73,10 +73,12 @@ 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)
> +					int overlay, struct 
drm_plane *plane,
> +					unsigned int zpos)
>  {
> -	u32 mask, val, ch_base;
> +	u32 mask, val, ch_base, bld_base;
> 
> +	bld_base = sun8i_blender_base(mixer);
>  	ch_base = sun8i_channel_base(mixer, channel);
> 
>  	mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
> @@ -84,13 +86,27 @@ static void sun8i_ui_layer_update_alpha(struct
> sun8i_mixer *mixer, int channel,
> 
>  	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;
> +	if (plane->state->pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> +		val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER;
> +	} else {
> +		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;
> +
> +		if (plane->state->pixel_blend_mode == 
DRM_MODE_BLEND_COVERAGE)
> +			val |= 
SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREMUL;
> +		else
> +			val |= 
SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI;
> +	}
> 
>  	regmap_update_bits(mixer->engine.regs,
>  			   SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, 
overlay),
>  			   mask, val);
> +
> +	regmap_update_bits(mixer->engine.regs,
> +			   SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
> +			   SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
> +			   SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos));

As you already figured out in previous patch [1], register values based on zpos 
can only be properly determined in sun8i_mixer_commit(). To be honest, I would 
like to see patch [1] to go in before this.

Note: Comment is also applicable to VI layer changes.

Best regards,
Jernej

[1] https://patchwork.freedesktop.org/patch/346998/

>  }
> 
>  static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int
> channel, @@ -280,7 +296,7 @@ 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);
> +				    layer->overlay, plane, zpos);
>  	sun8i_ui_layer_update_formats(mixer, layer->channel,
>  				      layer->overlay, plane);
>  	sun8i_ui_layer_update_buffer(mixer, layer->channel,
> @@ -361,6 +377,11 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct
> drm_device *drm, return ERR_PTR(ret);
>  	}
> 
> +	drm_plane_create_blend_mode_property(&layer->plane,
> +					     
BIT(DRM_MODE_BLEND_PREMULTI) |
> +					     
BIT(DRM_MODE_BLEND_COVERAGE) |
> +					     
BIT(DRM_MODE_BLEND_PIXEL_NONE));
> +
>  	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 e3e32ee1178d..c5136f4841bc
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
> @@ -41,6 +41,11 @@
>  #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_BLEND_MASK	GENMASK(16, 17)
> +
> +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COVERAGE		
((0) << 16)
> +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREMUL		
((1) << 16)
> +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI		
((2) << 16)
> 
>  #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_PIXEL		
((0) << 1)
>  #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER		
((1) << 1)
> diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index f2469b5e97ee..e6d8a539614f
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> @@ -66,11 +66,13 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer
> *mixer, int channel, }
> 
>  static void sun8i_vi_layer_update_alpha(struct sun8i_mixer *mixer, int
> channel, -					int overlay, struct 
drm_plane *plane)
> +					int overlay, struct 
drm_plane *plane,
> +					unsigned int zpos)
>  {
> -	u32 mask, val, ch_base;
> +	u32 mask, val, ch_base, bld_base;
> 
>  	ch_base = sun8i_channel_base(mixer, channel);
> +	bld_base = sun8i_blender_base(mixer);
> 
>  	if (mixer->cfg->is_de3) {
>  		mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
> @@ -78,9 +80,18 @@ static void sun8i_vi_layer_update_alpha(struct
> sun8i_mixer *mixer, int channel, val =
> SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA
>  			(plane->state->alpha >> 8);
> 
> -		val |= (plane->state->alpha == DRM_BLEND_ALPHA_OPAQUE) 
?
> -			
SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL :
> -			
SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED;
> +		if (plane->state->pixel_blend_mode == 
DRM_MODE_BLEND_PIXEL_NONE) {
> +			val |= 
SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_LAYER;
> +		} else {
> +			val |= (plane->state->alpha == 
DRM_BLEND_ALPHA_OPAQUE) ?
> +				
SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL :
> +				
SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED;
> +
> +			if (plane->state->pixel_blend_mode == 
DRM_MODE_BLEND_COVERAGE)
> +				val |= 
SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREMUL;
> +			else
> +				val |= 
SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_PREMULTI;
> +		}
> 
>  		regmap_update_bits(mixer->engine.regs,
>  				   
SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
> @@ -93,6 +104,13 @@ static void sun8i_vi_layer_update_alpha(struct
> sun8i_mixer *mixer, int channel, SUN8I_MIXER_FCC_GLOBAL_ALPHA
>  					(plane->state->alpha 
>> 8));
>  	}
> +
> +	regmap_update_bits(mixer->engine.regs,
> +			   SUN8I_MIXER_BLEND_PREMULTIPLY(bld_base),
> +			   SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos),
> +			   (mixer->cfg->is_de3) ?
> +				
SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
> +
>  }
> 
>  static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int
> channel, @@ -396,7 +414,7 @@ static void
> sun8i_vi_layer_atomic_update(struct drm_plane *plane,
> sun8i_vi_layer_update_coord(mixer, layer->channel,
>  				    layer->overlay, plane, zpos);
>  	sun8i_vi_layer_update_alpha(mixer, layer->channel,
> -				    layer->overlay, plane);
> +				    layer->overlay, plane, zpos);
>  	sun8i_vi_layer_update_formats(mixer, layer->channel,
>  				      layer->overlay, plane);
>  	sun8i_vi_layer_update_buffer(mixer, layer->channel,
> @@ -504,6 +522,12 @@ struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct
> drm_device *drm, }
>  	}
> 
> +	if (mixer->cfg->is_de3)
> +		drm_plane_create_blend_mode_property
> +			(&layer->plane, BIT(DRM_MODE_BLEND_PREMULTI) 
|
> +			 BIT(DRM_MODE_BLEND_COVERAGE) |
> +			 BIT(DRM_MODE_BLEND_PIXEL_NONE));
> +
>  	ret = drm_plane_create_zpos_property(&layer->plane, index,
>  					     0, plane_cnt - 
1);
>  	if (ret) {
> diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
> b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h index 48c399e1c86d..a1cf0ff16543
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
> @@ -43,6 +43,11 @@
>  #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK	GENMASK(2, 1)
>  #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK	GENMASK(31, 24)
>  #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA(x)	((x) << 24)
> +#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_MASK	GENMASK(16, 17)
> +
> +#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COVERAGE		
((0) << 16)
> +#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREMUL	((1) << 16)
> +#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_PREMULTI		
((2) << 16)
> 
>  #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_PIXEL	((0) << 1)
>  #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_LAYER	((1) << 1)

[PATCH v4 4/4] RFC: drm/sun4i: Process alpha channel of most bottom layer

[Roman Stratiienko]

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 <r.stratiienko@gmail.com>

---

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

Re: [PATCH v4 4/4] RFC: drm/sun4i: Process alpha channel of most bottom layer

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1613 bytes --]

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 <r.stratiienko@gmail.com>
>
> ---
>
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 4/4] RFC: drm/sun4i: Process alpha channel of most bottom layer

[Jernej Škrabec]

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 <r.stratiienko@gmail.com>
> 
> ---
> 
> 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);
> 
>  }