Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
"None", "Pre-multiplied", "Coverage"
Add the blend mode property and route it to the appropriate registers.
Note:
"force_premulti" parameter was added to handle multi-overlay channel
cases in future changes. It must be set to true for cases when more
than 1 overlay layer is used within a channel and at least one of the
overlay layers within a group uses premultiplied blending mode.
Test:
Manually tested all the modes using kmsxx python wrapper with and
without 'force_premulti' flag enabled.
Signed-off-by: Roman Stratiienko <[email protected]>
---
drivers/gpu/drm/sun4i/sun8i_mixer.h | 2 ++
drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 48 ++++++++++++++++++++-----
drivers/gpu/drm/sun4i/sun8i_ui_layer.h | 5 +++
drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 49 ++++++++++++++++++++++----
drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 5 +++
5 files changed, 94 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index ebfc276b2464..5c05907e26fb 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -65,6 +65,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 6ccbbca3176d..29c0d9cca19a 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -58,24 +58,46 @@ 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, bool force_premulti)
{
- u32 mask, val, ch_base;
+ u32 mask, val, ch_base, bld_base;
+ bool in_premulti, out_premulti;
+ bld_base = sun8i_blender_base(mixer);
ch_base = sun8i_channel_base(mixer, channel);
+ in_premulti = plane->state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI;
+ out_premulti = force_premulti || in_premulti;
+
mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
- SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
+ SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK |
+ SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_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;
+ 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 (in_premulti)
+ val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI;
+ }
+
+ if (!in_premulti && out_premulti)
+ val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM;
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),
+ out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
}
static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int channel,
@@ -274,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, false);
sun8i_ui_layer_update_formats(mixer, layer->channel,
layer->overlay, plane);
sun8i_ui_layer_update_buffer(mixer, layer->channel,
@@ -332,8 +354,8 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct drm_device *drm,
{
enum drm_plane_type type = DRM_PLANE_TYPE_OVERLAY;
int channel = mixer->cfg->vi_num + index;
+ unsigned int plane_cnt, blend_modes;
struct sun8i_ui_layer *layer;
- unsigned int plane_cnt;
int ret;
layer = devm_kzalloc(drm->dev, sizeof(*layer), GFP_KERNEL);
@@ -362,6 +384,16 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct drm_device *drm,
return ERR_PTR(ret);
}
+ blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
+ BIT(DRM_MODE_BLEND_COVERAGE) |
+ BIT(DRM_MODE_BLEND_PIXEL_NONE);
+
+ ret = drm_plane_create_blend_mode_property(&layer->plane, blend_modes);
+ if (ret) {
+ dev_err(drm->dev, "Couldn't add blend mode 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 43c48cf7bc51..bd3c30e8058d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
@@ -46,6 +46,11 @@
#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER ((1) << 1)
#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_MASK GENMASK(17, 16)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COVERAGE ((0) << 16)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM ((1) << 16)
+#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI ((2) << 16)
+
struct sun8i_mixer;
struct sun8i_ui_layer {
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index 662ba1018cc4..6581fc7d9668 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -52,21 +52,38 @@ 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, bool force_premulti)
{
- u32 mask, val, ch_base;
+ u32 mask, val, ch_base, bld_base;
+ bool in_premulti = false, out_premulti = false;
ch_base = sun8i_channel_base(mixer, channel);
+ bld_base = sun8i_blender_base(mixer);
if (mixer->cfg->is_de3) {
+ in_premulti = plane->state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI;
+ out_premulti = force_premulti || in_premulti;
+
mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
- SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK;
+ SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK |
+ SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_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;
+ 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 (in_premulti)
+ val |= SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_PREMULTI;
+ }
+
+ if (!in_premulti && out_premulti)
+ val |= SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREM;
regmap_update_bits(mixer->engine.regs,
SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
@@ -79,6 +96,11 @@ 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),
+ out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) : 0);
}
static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int channel,
@@ -408,7 +430,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, false);
sun8i_vi_layer_update_formats(mixer, layer->channel,
layer->overlay, plane);
sun8i_vi_layer_update_buffer(mixer, layer->channel,
@@ -563,6 +585,19 @@ struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct drm_device *drm,
}
}
+ if (mixer->cfg->is_de3) {
+ unsigned int blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
+ BIT(DRM_MODE_BLEND_COVERAGE) |
+ BIT(DRM_MODE_BLEND_PIXEL_NONE);
+
+ ret = drm_plane_create_blend_mode_property(&layer->plane,
+ blend_modes);
+ if (ret) {
+ dev_err(drm->dev, "Couldn't add blend mode 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 9939a4cc7a52..ccf91f09f1fe 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
@@ -44,6 +44,11 @@
#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(17, 16)
+#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COVERAGE ((0) << 16)
+#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREM ((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)
#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
--
2.30.2
Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko napisal(a):
> Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
> "None", "Pre-multiplied", "Coverage"
>
> Add the blend mode property and route it to the appropriate registers.
>
> Note:
> "force_premulti" parameter was added to handle multi-overlay channel
> cases in future changes. It must be set to true for cases when more
> than 1 overlay layer is used within a channel and at least one of the
> overlay layers within a group uses premultiplied blending mode.
Please remove this parameter. It's nothing special, so it can be easily added
once it's actually needed. For now, it only complicates code.
>
> Test:
> Manually tested all the modes using kmsxx python wrapper with and
> without 'force_premulti' flag enabled.
>
> Signed-off-by: Roman Stratiienko <[email protected]>
> ---
> drivers/gpu/drm/sun4i/sun8i_mixer.h | 2 ++
> drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 48 ++++++++++++++++++++-----
> drivers/gpu/drm/sun4i/sun8i_ui_layer.h | 5 +++
> drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 49 ++++++++++++++++++++++----
> drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 5 +++
> 5 files changed, 94 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..5c05907e26fb
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> @@ -65,6 +65,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 6ccbbca3176d..29c0d9cca19a
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -58,24 +58,46 @@ 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,
bool force_premulti)
> {
> - u32 mask, val, ch_base;
> + u32 mask, val, ch_base, bld_base;
> + bool in_premulti, out_premulti;
>
> + bld_base = sun8i_blender_base(mixer);
> ch_base = sun8i_channel_base(mixer, channel);
>
> + in_premulti = plane->state->pixel_blend_mode ==
DRM_MODE_BLEND_PREMULTI;
> + out_premulti = force_premulti || in_premulti;
> +
> mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
> - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
> + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK |
> + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_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;
> + 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 (in_premulti)
> + val |=
SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI;
> + }
> +
> + if (!in_premulti && out_premulti)
> + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM;
>
> 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),
> + out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) :
0);
> }
>
> static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int
> channel, @@ -274,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,
false);
> sun8i_ui_layer_update_formats(mixer, layer->channel,
> layer->overlay, plane);
> sun8i_ui_layer_update_buffer(mixer, layer->channel,
> @@ -332,8 +354,8 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct
> drm_device *drm, {
> enum drm_plane_type type = DRM_PLANE_TYPE_OVERLAY;
> int channel = mixer->cfg->vi_num + index;
> + unsigned int plane_cnt, blend_modes;
> struct sun8i_ui_layer *layer;
> - unsigned int plane_cnt;
> int ret;
>
> layer = devm_kzalloc(drm->dev, sizeof(*layer), GFP_KERNEL);
> @@ -362,6 +384,16 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct
> drm_device *drm, return ERR_PTR(ret);
> }
>
> + blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
> + BIT(DRM_MODE_BLEND_COVERAGE) |
> + BIT(DRM_MODE_BLEND_PIXEL_NONE);
> +
> + ret = drm_plane_create_blend_mode_property(&layer->plane,
blend_modes);
> + if (ret) {
> + dev_err(drm->dev, "Couldn't add blend mode
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 43c48cf7bc51..bd3c30e8058d
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
> @@ -46,6 +46,11 @@
> #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER ((1) << 1)
> #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
>
> +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_MASK GENMASK(17, 16)
> +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COVERAGE ((0) << 16)
> +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM ((1) << 16)
> +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI ((2) << 16)
> +
> struct sun8i_mixer;
>
> struct sun8i_ui_layer {
> diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index 662ba1018cc4..6581fc7d9668
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> @@ -52,21 +52,38 @@ 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,
bool force_premulti)
> {
> - u32 mask, val, ch_base;
> + u32 mask, val, ch_base, bld_base;
> + bool in_premulti = false, out_premulti = false;
>
> ch_base = sun8i_channel_base(mixer, channel);
> + bld_base = sun8i_blender_base(mixer);
>
> if (mixer->cfg->is_de3) {
> + in_premulti = plane->state->pixel_blend_mode ==
DRM_MODE_BLEND_PREMULTI;
> + out_premulti = force_premulti || in_premulti;
> +
> mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
> - SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK;
> + SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK
|
> + SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_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;
> + 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 (in_premulti)
> + val |=
SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_PREMULTI;
> + }
> +
> + if (!in_premulti && out_premulti)
> + val |=
SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREM;
>
> regmap_update_bits(mixer->engine.regs,
>
SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
> @@ -79,6 +96,11 @@ 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),
> + out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) :
0);
Shouldn't be above block inside DE3 if block?
Looks good otherwise.
Best regards,
Jernej
> }
>
> static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int
> channel, @@ -408,7 +430,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,
false);
> sun8i_vi_layer_update_formats(mixer, layer->channel,
> layer->overlay, plane);
> sun8i_vi_layer_update_buffer(mixer, layer->channel,
> @@ -563,6 +585,19 @@ struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct
> drm_device *drm, }
> }
>
> + if (mixer->cfg->is_de3) {
> + unsigned int blend_modes = BIT(DRM_MODE_BLEND_PREMULTI)
|
> +
BIT(DRM_MODE_BLEND_COVERAGE) |
> +
BIT(DRM_MODE_BLEND_PIXEL_NONE);
> +
> + ret = drm_plane_create_blend_mode_property(&layer-
>plane,
> +
blend_modes);
> + if (ret) {
> + dev_err(drm->dev, "Couldn't add blend mode
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 9939a4cc7a52..ccf91f09f1fe
> 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
> @@ -44,6 +44,11 @@
> #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(17, 16)
> +#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COVERAGE ((0) << 16)
> +#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREM ((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)
> #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
Hello Jernej,
Thank you for having a look.
вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec <[email protected]>:
>
> Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko napisal(a):
> > Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
> > "None", "Pre-multiplied", "Coverage"
> >
> > Add the blend mode property and route it to the appropriate registers.
> >
> > Note:
> > "force_premulti" parameter was added to handle multi-overlay channel
> > cases in future changes. It must be set to true for cases when more
> > than 1 overlay layer is used within a channel and at least one of the
> > overlay layers within a group uses premultiplied blending mode.
>
> Please remove this parameter. It's nothing special, so it can be easily added
> once it's actually needed. For now, it only complicates code.
I would prefer keeping it if you do not have any strong opinion against it.
I am working now on exposing all overlays, so it will be needed soon anyway.
Also it helps to better understand the COV2PREMULT mode which has not
the best description in the datasheet. Only after testing this
register using devmem I became confident on its purpose.
>
> >
> > Test:
> > Manually tested all the modes using kmsxx python wrapper with and
> > without 'force_premulti' flag enabled.
> >
> > Signed-off-by: Roman Stratiienko <[email protected]>
> > ---
> > drivers/gpu/drm/sun4i/sun8i_mixer.h | 2 ++
> > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 48 ++++++++++++++++++++-----
> > drivers/gpu/drm/sun4i/sun8i_ui_layer.h | 5 +++
> > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 49 ++++++++++++++++++++++----
> > drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 5 +++
> > 5 files changed, 94 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..5c05907e26fb
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > @@ -65,6 +65,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 6ccbbca3176d..29c0d9cca19a
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > @@ -58,24 +58,46 @@ 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,
> bool force_premulti)
> > {
> > - u32 mask, val, ch_base;
> > + u32 mask, val, ch_base, bld_base;
> > + bool in_premulti, out_premulti;
> >
> > + bld_base = sun8i_blender_base(mixer);
> > ch_base = sun8i_channel_base(mixer, channel);
> >
> > + in_premulti = plane->state->pixel_blend_mode ==
> DRM_MODE_BLEND_PREMULTI;
> > + out_premulti = force_premulti || in_premulti;
> > +
> > mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
> > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
> > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK |
> > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_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;
> > + 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 (in_premulti)
> > + val |=
> SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI;
> > + }
> > +
> > + if (!in_premulti && out_premulti)
> > + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM;
> >
> > 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),
> > + out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) :
> 0);
> > }
> >
> > static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int
> > channel, @@ -274,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,
> false);
> > sun8i_ui_layer_update_formats(mixer, layer->channel,
> > layer->overlay, plane);
> > sun8i_ui_layer_update_buffer(mixer, layer->channel,
> > @@ -332,8 +354,8 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct
> > drm_device *drm, {
> > enum drm_plane_type type = DRM_PLANE_TYPE_OVERLAY;
> > int channel = mixer->cfg->vi_num + index;
> > + unsigned int plane_cnt, blend_modes;
> > struct sun8i_ui_layer *layer;
> > - unsigned int plane_cnt;
> > int ret;
> >
> > layer = devm_kzalloc(drm->dev, sizeof(*layer), GFP_KERNEL);
> > @@ -362,6 +384,16 @@ struct sun8i_ui_layer *sun8i_ui_layer_init_one(struct
> > drm_device *drm, return ERR_PTR(ret);
> > }
> >
> > + blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
> > + BIT(DRM_MODE_BLEND_COVERAGE) |
> > + BIT(DRM_MODE_BLEND_PIXEL_NONE);
> > +
> > + ret = drm_plane_create_blend_mode_property(&layer->plane,
> blend_modes);
> > + if (ret) {
> > + dev_err(drm->dev, "Couldn't add blend mode
> 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 43c48cf7bc51..bd3c30e8058d
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
> > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
> > @@ -46,6 +46,11 @@
> > #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER ((1) << 1)
> > #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
> >
> > +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_MASK GENMASK(17, 16)
> > +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COVERAGE ((0) << 16)
> > +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM ((1) << 16)
> > +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI ((2) << 16)
> > +
> > struct sun8i_mixer;
> >
> > struct sun8i_ui_layer {
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index 662ba1018cc4..6581fc7d9668
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > @@ -52,21 +52,38 @@ 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,
> bool force_premulti)
> > {
> > - u32 mask, val, ch_base;
> > + u32 mask, val, ch_base, bld_base;
> > + bool in_premulti = false, out_premulti = false;
> >
> > ch_base = sun8i_channel_base(mixer, channel);
> > + bld_base = sun8i_blender_base(mixer);
> >
> > if (mixer->cfg->is_de3) {
> > + in_premulti = plane->state->pixel_blend_mode ==
> DRM_MODE_BLEND_PREMULTI;
> > + out_premulti = force_premulti || in_premulti;
> > +
> > mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
> > - SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK;
> > + SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK
> |
> > + SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_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;
> > + 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 (in_premulti)
> > + val |=
> SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_PREMULTI;
> > + }
> > +
> > + if (!in_premulti && out_premulti)
> > + val |=
> SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREM;
> >
> > regmap_update_bits(mixer->engine.regs,
> >
> SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
> > @@ -79,6 +96,11 @@ 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),
> > + out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) :
> 0);
>
> Shouldn't be above block inside DE3 if block?
No, we have to update the value to non-premulti in case it was set to
premulti by another layer.
Regards,
Roman.
>
> Looks good otherwise.
>
> Best regards,
> Jernej
>
> > }
> >
> > static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int
> > channel, @@ -408,7 +430,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,
> false);
> > sun8i_vi_layer_update_formats(mixer, layer->channel,
> > layer->overlay, plane);
> > sun8i_vi_layer_update_buffer(mixer, layer->channel,
> > @@ -563,6 +585,19 @@ struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct
> > drm_device *drm, }
> > }
> >
> > + if (mixer->cfg->is_de3) {
> > + unsigned int blend_modes = BIT(DRM_MODE_BLEND_PREMULTI)
> |
> > +
> BIT(DRM_MODE_BLEND_COVERAGE) |
> > +
> BIT(DRM_MODE_BLEND_PIXEL_NONE);
> > +
> > + ret = drm_plane_create_blend_mode_property(&layer-
> >plane,
> > +
> blend_modes);
> > + if (ret) {
> > + dev_err(drm->dev, "Couldn't add blend mode
> 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 9939a4cc7a52..ccf91f09f1fe
> > 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
> > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
> > @@ -44,6 +44,11 @@
> > #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(17, 16)
> > +#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COVERAGE ((0) << 16)
> > +#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREM ((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)
> > #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
>
>
>
>
On Mon, Jun 06, 2022 at 11:17:20AM +0300, Roman Stratiienko wrote:
> Hello Jernej,
>
> Thank you for having a look.
>
> вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec <[email protected]>:
> >
> > Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko napisal(a):
> > > Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
> > > "None", "Pre-multiplied", "Coverage"
> > >
> > > Add the blend mode property and route it to the appropriate registers.
> > >
> > > Note:
> > > "force_premulti" parameter was added to handle multi-overlay channel
> > > cases in future changes. It must be set to true for cases when more
> > > than 1 overlay layer is used within a channel and at least one of the
> > > overlay layers within a group uses premultiplied blending mode.
> >
> > Please remove this parameter. It's nothing special, so it can be easily added
> > once it's actually needed. For now, it only complicates code.
>
> I would prefer keeping it if you do not have any strong opinion against it.
>
> I am working now on exposing all overlays, so it will be needed soon anyway.
KMS assumes pre-multiplied alpha anyway, so we shouldn't need a
parameter to force it: we're always going to force it.
> Also it helps to better understand the COV2PREMULT mode which has not
> the best description in the datasheet. Only after testing this
> register using devmem I became confident on its purpose.
Sounds like a good job for a comment in the source code.
Maxime
On Mon, Jun 06, 2022 at 11:20:06AM +0200, Maxime Ripard wrote:
> On Mon, Jun 06, 2022 at 11:17:20AM +0300, Roman Stratiienko wrote:
> > Hello Jernej,
> >
> > Thank you for having a look.
> >
> > вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec <[email protected]>:
> > >
> > > Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko napisal(a):
> > > > Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
> > > > "None", "Pre-multiplied", "Coverage"
> > > >
> > > > Add the blend mode property and route it to the appropriate registers.
> > > >
> > > > Note:
> > > > "force_premulti" parameter was added to handle multi-overlay channel
> > > > cases in future changes. It must be set to true for cases when more
> > > > than 1 overlay layer is used within a channel and at least one of the
> > > > overlay layers within a group uses premultiplied blending mode.
> > >
> > > Please remove this parameter. It's nothing special, so it can be easily added
> > > once it's actually needed. For now, it only complicates code.
> >
> > I would prefer keeping it if you do not have any strong opinion against it.
> >
> > I am working now on exposing all overlays, so it will be needed soon anyway.
>
> KMS assumes pre-multiplied alpha anyway, so we shouldn't need a
> parameter to force it: we're always going to force it.
My bad, I got confused with your other patch.
Still, I agree with Jernej, if it's not needed it only complicates the
code for no particular reason. If you need it at some point in the
future, add it then. Otherwise, it's hard to reason about, since we
don't know what are the expectations that those future patches will
bring.
Maxime
Hi Maxime,
пн, 6 июн. 2022 г. в 12:22, Maxime Ripard <[email protected]>:
>
> On Mon, Jun 06, 2022 at 11:20:06AM +0200, Maxime Ripard wrote:
> > On Mon, Jun 06, 2022 at 11:17:20AM +0300, Roman Stratiienko wrote:
> > > Hello Jernej,
> > >
> > > Thank you for having a look.
> > >
> > > вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec <[email protected]>:
> > > >
> > > > Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko napisal(a):
> > > > > Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
> > > > > "None", "Pre-multiplied", "Coverage"
> > > > >
> > > > > Add the blend mode property and route it to the appropriate registers.
> > > > >
> > > > > Note:
> > > > > "force_premulti" parameter was added to handle multi-overlay channel
> > > > > cases in future changes. It must be set to true for cases when more
> > > > > than 1 overlay layer is used within a channel and at least one of the
> > > > > overlay layers within a group uses premultiplied blending mode.
> > > >
> > > > Please remove this parameter. It's nothing special, so it can be easily added
> > > > once it's actually needed. For now, it only complicates code.
> > >
> > > I would prefer keeping it if you do not have any strong opinion against it.
> > >
> > > I am working now on exposing all overlays, so it will be needed soon anyway.
> >
> > KMS assumes pre-multiplied alpha anyway, so we shouldn't need a
> > parameter to force it: we're always going to force it.
>
> My bad, I got confused with your other patch.
>
> Still, I agree with Jernej, if it's not needed it only complicates the
> code for no particular reason. If you need it at some point in the
> future, add it then. Otherwise, it's hard to reason about, since we
> don't know what are the expectations that those future patches will
> bring.
Well, existing code already has some sort of support for the sub-overlays:
For example 'int overlay, ' parameter is always 0, but it still passed through
the call stack. Therefore this patch is just aligned with already existing
traditions to keep sub-overlays in mind.
>
> Maxime
Regards,
Roman
Dne ponedeljek, 06. junij 2022 ob 10:17:20 CEST je Roman Stratiienko
napisal(a):
> Hello Jernej,
>
> Thank you for having a look.
>
> вс, 5 июн. 2022 г. в 23:37, Jernej Škrabec <[email protected]>:
> >
> > Dne nedelja, 05. junij 2022 ob 17:47:31 CEST je Roman Stratiienko
napisal(a):
> > > Allwinner DE2 and DE3 hardware support 3 pixel blend modes:
> > > "None", "Pre-multiplied", "Coverage"
> > >
> > > Add the blend mode property and route it to the appropriate registers.
> > >
> > > Note:
> > > "force_premulti" parameter was added to handle multi-overlay channel
> > > cases in future changes. It must be set to true for cases when more
> > > than 1 overlay layer is used within a channel and at least one of the
> > > overlay layers within a group uses premultiplied blending mode.
> >
> > Please remove this parameter. It's nothing special, so it can be easily
added
> > once it's actually needed. For now, it only complicates code.
>
> I would prefer keeping it if you do not have any strong opinion against it.
Actually I do. Patch will be smaller and easier to follow if there is no extra
variables with fixed values in it.
>
> I am working now on exposing all overlays, so it will be needed soon anyway.
Well, it will just be one patch more there, if at all.
Regards,
Jernej
>
> Also it helps to better understand the COV2PREMULT mode which has not
> the best description in the datasheet. Only after testing this
> register using devmem I became confident on its purpose.
>
> >
> > >
> > > Test:
> > > Manually tested all the modes using kmsxx python wrapper with and
> > > without 'force_premulti' flag enabled.
> > >
> > > Signed-off-by: Roman Stratiienko <[email protected]>
> > > ---
> > > drivers/gpu/drm/sun4i/sun8i_mixer.h | 2 ++
> > > drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 48 ++++++++++++++++++++-----
> > > drivers/gpu/drm/sun4i/sun8i_ui_layer.h | 5 +++
> > > drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 49 ++++++++++++++++++++++----
> > > drivers/gpu/drm/sun4i/sun8i_vi_layer.h | 5 +++
> > > 5 files changed, 94 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > > b/drivers/gpu/drm/sun4i/sun8i_mixer.h index ebfc276b2464..5c05907e26fb
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> > > @@ -65,6 +65,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 6ccbbca3176d..
29c0d9cca19a
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> > > @@ -58,24 +58,46 @@ 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,
> > bool force_premulti)
> > > {
> > > - u32 mask, val, ch_base;
> > > + u32 mask, val, ch_base, bld_base;
> > > + bool in_premulti, out_premulti;
> > >
> > > + bld_base = sun8i_blender_base(mixer);
> > > ch_base = sun8i_channel_base(mixer, channel);
> > >
> > > + in_premulti = plane->state->pixel_blend_mode ==
> > DRM_MODE_BLEND_PREMULTI;
> > > + out_premulti = force_premulti || in_premulti;
> > > +
> > > mask = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK |
> > > - SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK;
> > > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK |
> > > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_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;
> > > + 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 (in_premulti)
> > > + val |=
> > SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI;
> > > + }
> > > +
> > > + if (!in_premulti && out_premulti)
> > > + val |= SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM;
> > >
> > > 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),
> > > + out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) :
> > 0);
> > > }
> > >
> > > static int sun8i_ui_layer_update_coord(struct sun8i_mixer *mixer, int
> > > channel, @@ -274,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,
> > false);
> > > sun8i_ui_layer_update_formats(mixer, layer->channel,
> > > layer->overlay, plane);
> > > sun8i_ui_layer_update_buffer(mixer, layer->channel,
> > > @@ -332,8 +354,8 @@ struct sun8i_ui_layer
*sun8i_ui_layer_init_one(struct
> > > drm_device *drm, {
> > > enum drm_plane_type type = DRM_PLANE_TYPE_OVERLAY;
> > > int channel = mixer->cfg->vi_num + index;
> > > + unsigned int plane_cnt, blend_modes;
> > > struct sun8i_ui_layer *layer;
> > > - unsigned int plane_cnt;
> > > int ret;
> > >
> > > layer = devm_kzalloc(drm->dev, sizeof(*layer), GFP_KERNEL);
> > > @@ -362,6 +384,16 @@ struct sun8i_ui_layer
*sun8i_ui_layer_init_one(struct
> > > drm_device *drm, return ERR_PTR(ret);
> > > }
> > >
> > > + blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
> > > + BIT(DRM_MODE_BLEND_COVERAGE) |
> > > + BIT(DRM_MODE_BLEND_PIXEL_NONE);
> > > +
> > > + ret = drm_plane_create_blend_mode_property(&layer->plane,
> > blend_modes);
> > > + if (ret) {
> > > + dev_err(drm->dev, "Couldn't add blend mode
> > 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
43c48cf7bc51..bd3c30e8058d
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.h
> > > @@ -46,6 +46,11 @@
> > > #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_LAYER
((1) << 1)
> > > #define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
> > >
> > > +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_MASK GENMASK(17, 16)
> > > +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COVERAGE ((0) << 16)
> > > +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_COV2PREM ((1) << 16)
> > > +#define SUN8I_MIXER_CHAN_UI_LAYER_ATTR_BLEND_PREMULTI ((2) << 16)
> > > +
> > > struct sun8i_mixer;
> > >
> > > struct sun8i_ui_layer {
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index
662ba1018cc4..6581fc7d9668
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > @@ -52,21 +52,38 @@ 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,
> > bool force_premulti)
> > > {
> > > - u32 mask, val, ch_base;
> > > + u32 mask, val, ch_base, bld_base;
> > > + bool in_premulti = false, out_premulti = false;
> > >
> > > ch_base = sun8i_channel_base(mixer, channel);
> > > + bld_base = sun8i_blender_base(mixer);
> > >
> > > if (mixer->cfg->is_de3) {
> > > + in_premulti = plane->state->pixel_blend_mode ==
> > DRM_MODE_BLEND_PREMULTI;
> > > + out_premulti = force_premulti || in_premulti;
> > > +
> > > mask = SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MASK |
> > > - SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK;
> > > + SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_MASK
> > |
> > > + SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_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;
> > > + 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 (in_premulti)
> > > + val |=
> > SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_PREMULTI;
> > > + }
> > > +
> > > + if (!in_premulti && out_premulti)
> > > + val |=
> > SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREM;
> > >
> > > regmap_update_bits(mixer->engine.regs,
> > >
> > SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base,
> > > @@ -79,6 +96,11 @@ 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),
> > > + out_premulti ? SUN8I_MIXER_BLEND_PREMULTIPLY_EN(zpos) :
> > 0);
> >
> > Shouldn't be above block inside DE3 if block?
>
> No, we have to update the value to non-premulti in case it was set to
> premulti by another layer.
>
>
> Regards,
> Roman.
>
> >
> > Looks good otherwise.
> >
> > Best regards,
> > Jernej
> >
> > > }
> > >
> > > static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int
> > > channel, @@ -408,7 +430,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,
> > false);
> > > sun8i_vi_layer_update_formats(mixer, layer->channel,
> > > layer->overlay, plane);
> > > sun8i_vi_layer_update_buffer(mixer, layer->channel,
> > > @@ -563,6 +585,19 @@ struct sun8i_vi_layer
*sun8i_vi_layer_init_one(struct
> > > drm_device *drm, }
> > > }
> > >
> > > + if (mixer->cfg->is_de3) {
> > > + unsigned int blend_modes = BIT(DRM_MODE_BLEND_PREMULTI)
> > |
> > > +
> > BIT(DRM_MODE_BLEND_COVERAGE) |
> > > +
> > BIT(DRM_MODE_BLEND_PIXEL_NONE);
> > > +
> > > + ret = drm_plane_create_blend_mode_property(&layer-
> > >plane,
> > > +
> > blend_modes);
> > > + if (ret) {
> > > + dev_err(drm->dev, "Couldn't add blend mode
> > 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
9939a4cc7a52..ccf91f09f1fe
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.h
> > > @@ -44,6 +44,11 @@
> > > #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(17, 16)
> > > +#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COVERAGE ((0) << 16)
> > > +#define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_BLEND_COV2PREM ((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)
> > > #define SUN50I_MIXER_CHAN_VI_LAYER_ATTR_ALPHA_MODE_COMBINED ((2) << 1)
> >
> >
> >
> >
>