* RE: [PATCH v2] drm/amd/display: add Coverage blend mode for overlay plane
[not found] <20220513202200.4698-1-Sungjoon.Kim@amd.com>
@ 2022-05-13 20:23 ` Kim, Sung joon
2022-05-19 20:00 ` Melissa Wen
[not found] ` <8b8c0c85-1d79-5cfd-f0d2-ebc8b732e0c9@amd.com>
2 siblings, 0 replies; 5+ messages in thread
From: Kim, Sung joon @ 2022-05-13 20:23 UTC (permalink / raw)
To: Wentland, Harry, Li, Sun peng (Leo),
Siqueira, Rodrigo, Deucher, Alexander, Koenig, Christian
Cc: mwen, contact, amd-gfx, markyacoub
[AMD Official Use Only - General]
-----Original Message-----
From: Kim, Sung joon <Sungjoon.Kim@amd.com>
Sent: Friday, May 13, 2022 4:22 PM
To: Wentland, Harry <Harry.Wentland@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
Cc: amd-gfx@lists.freekdesktop.org; mwen@igalia.com; contact@emersion.fr; markyacoub@chromium.org; Kim, Sung joon <Sungjoon.Kim@amd.com>
Subject: [PATCH v2] drm/amd/display: add Coverage blend mode for overlay plane
Issue fixed: Overlay plane alpha channel blending is incorrect
Issue tracker: https://gitlab.freedesktop.org/drm/amd/-/issues/1769
According to the KMS man page, there is a "Coverage" alpha blend mode that assumes the pixel color values have NOT been pre-multiplied and will be done when the actual blending to the background color values happens.
Previously, this mode hasn't been enabled in our driver and it was assumed that all normal overlay planes are pre-multiplied by default.
When a 3rd party app is used to input a image in a specific format, e.g. PNG, as a source of a overlay plane to blend with the background primary plane, the pixel color values are not pre-multiplied. So by adding "Coverage" blend mode, our driver will support those cases.
Reference:
https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-composition-properties
Adding Coverage support also enables IGT kms_plane_alpha_blend Coverage subtests:
1. coverage-7efc
2. coverage-vs-premult-vs-constant
Signed-off-by: Sung Joon Kim <Sungjoon.Kim@amd.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 17 ++++++++---- .../gpu/drm/amd/display/dc/core/dc_surface.c | 2 ++
drivers/gpu/drm/amd/display/dc/dc.h | 2 ++
.../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 27 ++++++++++---------
.../drm/amd/display/dc/dcn20/dcn20_hwseq.c | 16 ++++++-----
5 files changed, 40 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 2ea20dd7fccf..af2b5d232742 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5380,17 +5380,19 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev,
static void
fill_blending_from_plane_state(const struct drm_plane_state *plane_state,
- bool *per_pixel_alpha, bool *global_alpha,
- int *global_alpha_value)
+ bool *per_pixel_alpha, bool *pre_multiplied_alpha,
+ bool *global_alpha, int *global_alpha_value)
{
*per_pixel_alpha = false;
+ *pre_multiplied_alpha = true;
*global_alpha = false;
*global_alpha_value = 0xff;
if (plane_state->plane->type != DRM_PLANE_TYPE_OVERLAY)
return;
- if (plane_state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI) {
+ if (plane_state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI ||
+ plane_state->pixel_blend_mode == DRM_MODE_BLEND_COVERAGE) {
static const uint32_t alpha_formats[] = {
DRM_FORMAT_ARGB8888,
DRM_FORMAT_RGBA8888,
@@ -5405,6 +5407,9 @@ fill_blending_from_plane_state(const struct drm_plane_state *plane_state,
break;
}
}
+
+ if (per_pixel_alpha && plane_state->pixel_blend_mode == DRM_MODE_BLEND_COVERAGE)
+ *pre_multiplied_alpha = false;
}
if (plane_state->alpha < 0xffff) {
@@ -5567,7 +5572,7 @@ fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
return ret;
fill_blending_from_plane_state(
- plane_state, &plane_info->per_pixel_alpha,
+ plane_state, &plane_info->per_pixel_alpha,
+&plane_info->pre_multiplied_alpha,
&plane_info->global_alpha, &plane_info->global_alpha_value);
return 0;
@@ -5614,6 +5619,7 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
dc_plane_state->tiling_info = plane_info.tiling_info;
dc_plane_state->visible = plane_info.visible;
dc_plane_state->per_pixel_alpha = plane_info.per_pixel_alpha;
+ dc_plane_state->pre_multiplied_alpha =
+plane_info.pre_multiplied_alpha;
dc_plane_state->global_alpha = plane_info.global_alpha;
dc_plane_state->global_alpha_value = plane_info.global_alpha_value;
dc_plane_state->dcc = plane_info.dcc;
@@ -7905,7 +7911,8 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
if (plane->type == DRM_PLANE_TYPE_OVERLAY &&
plane_cap && plane_cap->per_pixel_alpha) {
unsigned int blend_caps = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
- BIT(DRM_MODE_BLEND_PREMULTI);
+ BIT(DRM_MODE_BLEND_PREMULTI) |
+ BIT(DRM_MODE_BLEND_COVERAGE);
drm_plane_create_alpha_property(plane);
drm_plane_create_blend_mode_property(plane, blend_caps); diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
index e6b9c6a71841..5bc6ff2fa73e 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
@@ -61,6 +61,8 @@ static void dc_plane_construct(struct dc_context *ctx, struct dc_plane_state *pl
plane_state->blend_tf->type = TF_TYPE_BYPASS;
}
+ plane_state->pre_multiplied_alpha = true;
+
}
static void dc_plane_destruct(struct dc_plane_state *plane_state) diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
index 26c24db8f1da..c353842d5c40 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -1011,6 +1011,7 @@ struct dc_plane_state {
bool is_tiling_rotated;
bool per_pixel_alpha;
+ bool pre_multiplied_alpha;
bool global_alpha;
int global_alpha_value;
bool visible;
@@ -1045,6 +1046,7 @@ struct dc_plane_info {
bool horizontal_mirror;
bool visible;
bool per_pixel_alpha;
+ bool pre_multiplied_alpha;
bool global_alpha;
int global_alpha_value;
bool input_csc_enabled;
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index e02ac75afbf7..e3a62873c0e7 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -2550,12 +2550,21 @@ void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
blnd_cfg.overlap_only = false;
blnd_cfg.global_gain = 0xff;
- if (per_pixel_alpha && pipe_ctx->plane_state->global_alpha) {
- blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
- blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
- } else if (per_pixel_alpha) {
- blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
+ if (per_pixel_alpha) {
+ /* DCN1.0 has output CM before MPC which seems to screw with
+ * pre-multiplied alpha.
+ */
+ blnd_cfg.pre_multiplied_alpha = (is_rgb_cspace(
+ pipe_ctx->stream->output_color_space)
+ && pipe_ctx->plane_state->pre_multiplied_alpha);
+ if (pipe_ctx->plane_state->global_alpha) {
+ blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
+ blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
+ } else {
+ blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
+ }
} else {
+ blnd_cfg.pre_multiplied_alpha = false;
blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA;
}
@@ -2564,14 +2573,6 @@ void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
else
blnd_cfg.global_alpha = 0xff;
- /* DCN1.0 has output CM before MPC which seems to screw with
- * pre-multiplied alpha.
- */
- blnd_cfg.pre_multiplied_alpha = is_rgb_cspace(
- pipe_ctx->stream->output_color_space)
- && per_pixel_alpha;
-
-
/*
* TODO: remove hack
* Note: currently there is a bug in init_hw such that diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
index e1f87bd72e4a..c117830b8b9d 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
@@ -2346,12 +2346,16 @@ void dcn20_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
blnd_cfg.overlap_only = false;
blnd_cfg.global_gain = 0xff;
- if (per_pixel_alpha && pipe_ctx->plane_state->global_alpha) {
- blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
- blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
- } else if (per_pixel_alpha) {
- blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
+ if (per_pixel_alpha) {
+ blnd_cfg.pre_multiplied_alpha = pipe_ctx->plane_state->pre_multiplied_alpha;
+ if (pipe_ctx->plane_state->global_alpha) {
+ blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
+ blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
+ } else {
+ blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
+ }
} else {
+ blnd_cfg.pre_multiplied_alpha = false;
blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA;
}
@@ -2365,7 +2369,7 @@ void dcn20_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
blnd_cfg.top_gain = 0x1f000;
blnd_cfg.bottom_inside_gain = 0x1f000;
blnd_cfg.bottom_outside_gain = 0x1f000;
- blnd_cfg.pre_multiplied_alpha = per_pixel_alpha;
+
if (pipe_ctx->plane_state->format
== SURFACE_PIXEL_FORMAT_GRPH_RGBE_ALPHA)
blnd_cfg.pre_multiplied_alpha = false;
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm/amd/display: add Coverage blend mode for overlay plane
[not found] <20220513202200.4698-1-Sungjoon.Kim@amd.com>
2022-05-13 20:23 ` [PATCH v2] drm/amd/display: add Coverage blend mode for overlay plane Kim, Sung joon
@ 2022-05-19 20:00 ` Melissa Wen
[not found] ` <8b8c0c85-1d79-5cfd-f0d2-ebc8b732e0c9@amd.com>
2 siblings, 0 replies; 5+ messages in thread
From: Melissa Wen @ 2022-05-19 20:00 UTC (permalink / raw)
To: Sung Joon Kim
Cc: sunpeng.li, contact, rodrigo.siqueira, amd-gfx, markyacoub,
alexander.deucher, harry.wentland, christian.koenig
[-- Attachment #1: Type: text/plain, Size: 9931 bytes --]
On 05/13, Sung Joon Kim wrote:
> Issue fixed: Overlay plane alpha channel blending is incorrect
>
> Issue tracker: https://gitlab.freedesktop.org/drm/amd/-/issues/1769
>
> According to the KMS man page, there is a
> "Coverage" alpha blend mode that assumes the
> pixel color values have NOT been pre-multiplied
> and will be done when the actual blending to
> the background color values happens.
>
> Previously, this mode hasn't been enabled
> in our driver and it was assumed that all
> normal overlay planes are pre-multiplied
> by default.
>
> When a 3rd party app is used to input a image
> in a specific format, e.g. PNG, as a source
> of a overlay plane to blend with the background
> primary plane, the pixel color values are not
> pre-multiplied. So by adding "Coverage" blend
> mode, our driver will support those cases.
>
Hi,
overall lgtm. I would move those "Issue" points to here (after change
description).
> Reference:
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-composition-properties
>
> Adding Coverage support also enables IGT
> kms_plane_alpha_blend Coverage subtests:
> 1. coverage-7efc
> 2. coverage-vs-premult-vs-constant
.. and include a summary of changes from v1 here.
>
> Signed-off-by: Sung Joon Kim <Sungjoon.Kim@amd.com>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 17 ++++++++----
> .../gpu/drm/amd/display/dc/core/dc_surface.c | 2 ++
> drivers/gpu/drm/amd/display/dc/dc.h | 2 ++
> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 27 ++++++++++---------
> .../drm/amd/display/dc/dcn20/dcn20_hwseq.c | 16 ++++++-----
> 5 files changed, 40 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 2ea20dd7fccf..af2b5d232742 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5380,17 +5380,19 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev,
>
> static void
> fill_blending_from_plane_state(const struct drm_plane_state *plane_state,
> - bool *per_pixel_alpha, bool *global_alpha,
> - int *global_alpha_value)
> + bool *per_pixel_alpha, bool *pre_multiplied_alpha,
> + bool *global_alpha, int *global_alpha_value)
> {
> *per_pixel_alpha = false;
> + *pre_multiplied_alpha = true;
> *global_alpha = false;
> *global_alpha_value = 0xff;
>
> if (plane_state->plane->type != DRM_PLANE_TYPE_OVERLAY)
> return;
>
> - if (plane_state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI) {
> + if (plane_state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI ||
> + plane_state->pixel_blend_mode == DRM_MODE_BLEND_COVERAGE) {
> static const uint32_t alpha_formats[] = {
> DRM_FORMAT_ARGB8888,
> DRM_FORMAT_RGBA8888,
> @@ -5405,6 +5407,9 @@ fill_blending_from_plane_state(const struct drm_plane_state *plane_state,
> break;
> }
> }
> +
> + if (per_pixel_alpha && plane_state->pixel_blend_mode == DRM_MODE_BLEND_COVERAGE)
> + *pre_multiplied_alpha = false;
> }
>
> if (plane_state->alpha < 0xffff) {
> @@ -5567,7 +5572,7 @@ fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
> return ret;
>
> fill_blending_from_plane_state(
> - plane_state, &plane_info->per_pixel_alpha,
> + plane_state, &plane_info->per_pixel_alpha, &plane_info->pre_multiplied_alpha,
> &plane_info->global_alpha, &plane_info->global_alpha_value);
>
> return 0;
> @@ -5614,6 +5619,7 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
> dc_plane_state->tiling_info = plane_info.tiling_info;
> dc_plane_state->visible = plane_info.visible;
> dc_plane_state->per_pixel_alpha = plane_info.per_pixel_alpha;
> + dc_plane_state->pre_multiplied_alpha = plane_info.pre_multiplied_alpha;
> dc_plane_state->global_alpha = plane_info.global_alpha;
> dc_plane_state->global_alpha_value = plane_info.global_alpha_value;
> dc_plane_state->dcc = plane_info.dcc;
> @@ -7905,7 +7911,8 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
> if (plane->type == DRM_PLANE_TYPE_OVERLAY &&
> plane_cap && plane_cap->per_pixel_alpha) {
> unsigned int blend_caps = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
> - BIT(DRM_MODE_BLEND_PREMULTI);
> + BIT(DRM_MODE_BLEND_PREMULTI) |
> + BIT(DRM_MODE_BLEND_COVERAGE);
>
> drm_plane_create_alpha_property(plane);
> drm_plane_create_blend_mode_property(plane, blend_caps);
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> index e6b9c6a71841..5bc6ff2fa73e 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
> @@ -61,6 +61,8 @@ static void dc_plane_construct(struct dc_context *ctx, struct dc_plane_state *pl
> plane_state->blend_tf->type = TF_TYPE_BYPASS;
> }
>
> + plane_state->pre_multiplied_alpha = true;
> +
> }
>
> static void dc_plane_destruct(struct dc_plane_state *plane_state)
> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
> index 26c24db8f1da..c353842d5c40 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> @@ -1011,6 +1011,7 @@ struct dc_plane_state {
>
> bool is_tiling_rotated;
> bool per_pixel_alpha;
> + bool pre_multiplied_alpha;
> bool global_alpha;
> int global_alpha_value;
> bool visible;
> @@ -1045,6 +1046,7 @@ struct dc_plane_info {
> bool horizontal_mirror;
> bool visible;
> bool per_pixel_alpha;
> + bool pre_multiplied_alpha;
> bool global_alpha;
> int global_alpha_value;
> bool input_csc_enabled;
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index e02ac75afbf7..e3a62873c0e7 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -2550,12 +2550,21 @@ void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
> blnd_cfg.overlap_only = false;
> blnd_cfg.global_gain = 0xff;
>
> - if (per_pixel_alpha && pipe_ctx->plane_state->global_alpha) {
> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
> - blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
> - } else if (per_pixel_alpha) {
> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
> + if (per_pixel_alpha) {
> + /* DCN1.0 has output CM before MPC which seems to screw with
> + * pre-multiplied alpha.
> + */
> + blnd_cfg.pre_multiplied_alpha = (is_rgb_cspace(
> + pipe_ctx->stream->output_color_space)
> + && pipe_ctx->plane_state->pre_multiplied_alpha);
> + if (pipe_ctx->plane_state->global_alpha) {
> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
> + blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
> + } else {
> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
> + }
> } else {
> + blnd_cfg.pre_multiplied_alpha = false;
> blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA;
> }
>
> @@ -2564,14 +2573,6 @@ void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
> else
> blnd_cfg.global_alpha = 0xff;
>
> - /* DCN1.0 has output CM before MPC which seems to screw with
> - * pre-multiplied alpha.
> - */
> - blnd_cfg.pre_multiplied_alpha = is_rgb_cspace(
> - pipe_ctx->stream->output_color_space)
> - && per_pixel_alpha;
> -
> -
> /*
> * TODO: remove hack
> * Note: currently there is a bug in init_hw such that
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> index e1f87bd72e4a..c117830b8b9d 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> @@ -2346,12 +2346,16 @@ void dcn20_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
> blnd_cfg.overlap_only = false;
> blnd_cfg.global_gain = 0xff;
>
> - if (per_pixel_alpha && pipe_ctx->plane_state->global_alpha) {
> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
> - blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
> - } else if (per_pixel_alpha) {
> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
> + if (per_pixel_alpha) {
> + blnd_cfg.pre_multiplied_alpha = pipe_ctx->plane_state->pre_multiplied_alpha;
> + if (pipe_ctx->plane_state->global_alpha) {
> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
> + blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
> + } else {
> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
> + }
> } else {
> + blnd_cfg.pre_multiplied_alpha = false;
> blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA;
> }
>
> @@ -2365,7 +2369,7 @@ void dcn20_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
> blnd_cfg.top_gain = 0x1f000;
> blnd_cfg.bottom_inside_gain = 0x1f000;
> blnd_cfg.bottom_outside_gain = 0x1f000;
> - blnd_cfg.pre_multiplied_alpha = per_pixel_alpha;
> +
^ checkpatch complains about `trailing whitespace` here.
Anyway, after addressing these minor comments, the patch is
Reviewed-by: Melissa Wen <mwen@igalia.com>
Thanks a lot!
Next, I'm going to refactor the IGT coverage-7efc test to match the
changes done to alpha-7efc (pre-multiplied).
> if (pipe_ctx->plane_state->format
> == SURFACE_PIXEL_FORMAT_GRPH_RGBE_ALPHA)
> blnd_cfg.pre_multiplied_alpha = false;
> --
> 2.20.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm/amd/display: add Coverage blend mode for overlay plane
[not found] ` <8b8c0c85-1d79-5cfd-f0d2-ebc8b732e0c9@amd.com>
@ 2022-05-19 20:45 ` Harry Wentland
2022-05-20 17:50 ` Kim, Sung joon
0 siblings, 1 reply; 5+ messages in thread
From: Harry Wentland @ 2022-05-19 20:45 UTC (permalink / raw)
To: Sung Joon Kim, sunpeng.li, rodrigo.siqueira, alexander.deucher,
christian.koenig
Cc: mwen, contact, amd-gfx list, markyacoub
Fixing amd-gfx address.
On 2022-05-19 16:42, Harry Wentland wrote:
>
>
> On 2022-05-13 16:22, Sung Joon Kim wrote:
>> Issue fixed: Overlay plane alpha channel blending is incorrect
>>
>> Issue tracker: https://gitlab.freedesktop.org/drm/amd/-/issues/1769
>>
>> According to the KMS man page, there is a
>> "Coverage" alpha blend mode that assumes the
>> pixel color values have NOT been pre-multiplied
>> and will be done when the actual blending to
>> the background color values happens.
>>
>> Previously, this mode hasn't been enabled
>> in our driver and it was assumed that all
>> normal overlay planes are pre-multiplied
>> by default.
>>
>> When a 3rd party app is used to input a image
>> in a specific format, e.g. PNG, as a source
>> of a overlay plane to blend with the background
>> primary plane, the pixel color values are not
>> pre-multiplied. So by adding "Coverage" blend
>> mode, our driver will support those cases.
>>
>> Reference:
>> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-composition-properties
>>
>> Adding Coverage support also enables IGT
>> kms_plane_alpha_blend Coverage subtests:
>> 1. coverage-7efc
>> 2. coverage-vs-premult-vs-constant
>>
>> Signed-off-by: Sung Joon Kim <Sungjoon.Kim@amd.com>
>> ---
>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 17 ++++++++----
>> .../gpu/drm/amd/display/dc/core/dc_surface.c | 2 ++
>> drivers/gpu/drm/amd/display/dc/dc.h | 2 ++
>> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 27 ++++++++++---------
>> .../drm/amd/display/dc/dcn20/dcn20_hwseq.c | 16 ++++++-----
>> 5 files changed, 40 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 2ea20dd7fccf..af2b5d232742 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -5380,17 +5380,19 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev,
>>
>> static void
>> fill_blending_from_plane_state(const struct drm_plane_state *plane_state,
>> - bool *per_pixel_alpha, bool *global_alpha,
>> - int *global_alpha_value)
>> + bool *per_pixel_alpha, bool *pre_multiplied_alpha,
>> + bool *global_alpha, int *global_alpha_value)
>> {
>> *per_pixel_alpha = false;
>> + *pre_multiplied_alpha = true;
>> *global_alpha = false;
>> *global_alpha_value = 0xff;
>>
>> if (plane_state->plane->type != DRM_PLANE_TYPE_OVERLAY)
>> return;
>>
>> - if (plane_state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI) {
>> + if (plane_state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI ||
>> + plane_state->pixel_blend_mode == DRM_MODE_BLEND_COVERAGE) {
>> static const uint32_t alpha_formats[] = {
>> DRM_FORMAT_ARGB8888,
>> DRM_FORMAT_RGBA8888,
>> @@ -5405,6 +5407,9 @@ fill_blending_from_plane_state(const struct drm_plane_state *plane_state,
>> break;
>> }
>> }
>> +
>> + if (per_pixel_alpha && plane_state->pixel_blend_mode == DRM_MODE_BLEND_COVERAGE)
>> + *pre_multiplied_alpha = false;
>> }
>>
>> if (plane_state->alpha < 0xffff) {
>> @@ -5567,7 +5572,7 @@ fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
>> return ret;
>>
>> fill_blending_from_plane_state(
>> - plane_state, &plane_info->per_pixel_alpha,
>> + plane_state, &plane_info->per_pixel_alpha, &plane_info->pre_multiplied_alpha,
>> &plane_info->global_alpha, &plane_info->global_alpha_value);
>>
>> return 0;
>> @@ -5614,6 +5619,7 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
>> dc_plane_state->tiling_info = plane_info.tiling_info;
>> dc_plane_state->visible = plane_info.visible;
>> dc_plane_state->per_pixel_alpha = plane_info.per_pixel_alpha;
>> + dc_plane_state->pre_multiplied_alpha = plane_info.pre_multiplied_alpha;
>> dc_plane_state->global_alpha = plane_info.global_alpha;
>> dc_plane_state->global_alpha_value = plane_info.global_alpha_value;
>> dc_plane_state->dcc = plane_info.dcc;
>> @@ -7905,7 +7911,8 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
>> if (plane->type == DRM_PLANE_TYPE_OVERLAY &&
>> plane_cap && plane_cap->per_pixel_alpha) {
>> unsigned int blend_caps = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
>> - BIT(DRM_MODE_BLEND_PREMULTI);
>> + BIT(DRM_MODE_BLEND_PREMULTI) |
>> + BIT(DRM_MODE_BLEND_COVERAGE);
>>
>> drm_plane_create_alpha_property(plane);
>> drm_plane_create_blend_mode_property(plane, blend_caps);
>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
>> index e6b9c6a71841..5bc6ff2fa73e 100644
>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
>> @@ -61,6 +61,8 @@ static void dc_plane_construct(struct dc_context *ctx, struct dc_plane_state *pl
>> plane_state->blend_tf->type = TF_TYPE_BYPASS;
>> }
>>
>> + plane_state->pre_multiplied_alpha = true;
>> +
>> }
>>
>> static void dc_plane_destruct(struct dc_plane_state *plane_state)
>> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
>> index 26c24db8f1da..c353842d5c40 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dc.h
>> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
>> @@ -1011,6 +1011,7 @@ struct dc_plane_state {
>>
>> bool is_tiling_rotated;
>> bool per_pixel_alpha;
>> + bool pre_multiplied_alpha;
>> bool global_alpha;
>> int global_alpha_value;
>> bool visible;
>> @@ -1045,6 +1046,7 @@ struct dc_plane_info {
>> bool horizontal_mirror;
>> bool visible;
>> bool per_pixel_alpha;
>> + bool pre_multiplied_alpha;
>> bool global_alpha;
>> int global_alpha_value;
>> bool input_csc_enabled;
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
>> index e02ac75afbf7..e3a62873c0e7 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
>> @@ -2550,12 +2550,21 @@ void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
>> blnd_cfg.overlap_only = false;
>> blnd_cfg.global_gain = 0xff;
>>
>> - if (per_pixel_alpha && pipe_ctx->plane_state->global_alpha) {
>> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
>> - blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
>> - } else if (per_pixel_alpha) {
>> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
>> + if (per_pixel_alpha) {
>> + /* DCN1.0 has output CM before MPC which seems to screw with
>> + * pre-multiplied alpha.
>> + */
>> + blnd_cfg.pre_multiplied_alpha = (is_rgb_cspace(
>> + pipe_ctx->stream->output_color_space)
>> + && pipe_ctx->plane_state->pre_multiplied_alpha);
>> + if (pipe_ctx->plane_state->global_alpha) {
>> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
>> + blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
>> + } else {
>> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
>> + }
>> } else {
>> + blnd_cfg.pre_multiplied_alpha = false;
>> blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA;
>> }
>>
>> @@ -2564,14 +2573,6 @@ void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
>> else
>> blnd_cfg.global_alpha = 0xff;
>>
>> - /* DCN1.0 has output CM before MPC which seems to screw with
>> - * pre-multiplied alpha.
>> - */
>> - blnd_cfg.pre_multiplied_alpha = is_rgb_cspace(
>> - pipe_ctx->stream->output_color_space)
>> - && per_pixel_alpha;
>> -
>
> Could this break color management use-cases on DCN1.x for Windows?
> Dmytro Laktyushkin added this code 5 years ago. Maybe check with him.
>
> Overall I like this patch but let's see if we can confirm that
> this won't break DCN1.x on Windows.
>
> See commit ad32734699da4dd185405637459bf915a4f4cff6.
>
> Harry
>
>> -
>> /*
>> * TODO: remove hack
>> * Note: currently there is a bug in init_hw such that
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
>> index e1f87bd72e4a..c117830b8b9d 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
>> @@ -2346,12 +2346,16 @@ void dcn20_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
>> blnd_cfg.overlap_only = false;
>> blnd_cfg.global_gain = 0xff;
>>
>> - if (per_pixel_alpha && pipe_ctx->plane_state->global_alpha) {
>> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
>> - blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
>> - } else if (per_pixel_alpha) {
>> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
>> + if (per_pixel_alpha) {
>> + blnd_cfg.pre_multiplied_alpha = pipe_ctx->plane_state->pre_multiplied_alpha;
>> + if (pipe_ctx->plane_state->global_alpha) {
>> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
>> + blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
>> + } else {
>> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
>> + }
>> } else {
>> + blnd_cfg.pre_multiplied_alpha = false;
>> blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA;
>> }
>>
>> @@ -2365,7 +2369,7 @@ void dcn20_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
>> blnd_cfg.top_gain = 0x1f000;
>> blnd_cfg.bottom_inside_gain = 0x1f000;
>> blnd_cfg.bottom_outside_gain = 0x1f000;
>> - blnd_cfg.pre_multiplied_alpha = per_pixel_alpha;
>> +
>> if (pipe_ctx->plane_state->format
>> == SURFACE_PIXEL_FORMAT_GRPH_RGBE_ALPHA)
>> blnd_cfg.pre_multiplied_alpha = false;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2] drm/amd/display: add Coverage blend mode for overlay plane
2022-05-19 20:45 ` Harry Wentland
@ 2022-05-20 17:50 ` Kim, Sung joon
2022-05-20 17:55 ` Harry Wentland
0 siblings, 1 reply; 5+ messages in thread
From: Kim, Sung joon @ 2022-05-20 17:50 UTC (permalink / raw)
To: Wentland, Harry, Li, Sun peng (Leo),
Siqueira, Rodrigo, Deucher, Alexander, Koenig, Christian
Cc: mwen, contact, amd-gfx list, markyacoub
[AMD Official Use Only - General]
-----Original Message-----
From: Wentland, Harry <Harry.Wentland@amd.com>
Sent: Thursday, May 19, 2022 4:45 PM
To: Kim, Sung joon <Sungjoon.Kim@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
Cc: mwen@igalia.com; contact@emersion.fr; markyacoub@chromium.org; amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v2] drm/amd/display: add Coverage blend mode for overlay plane
Fixing amd-gfx address.
On 2022-05-19 16:42, Harry Wentland wrote:
>
>
> On 2022-05-13 16:22, Sung Joon Kim wrote:
>> Issue fixed: Overlay plane alpha channel blending is incorrect
>>
>> Issue tracker: https://gitlab.freedesktop.org/drm/amd/-/issues/1769
>>
>> According to the KMS man page, there is a "Coverage" alpha blend mode
>> that assumes the pixel color values have NOT been pre-multiplied and
>> will be done when the actual blending to the background color values
>> happens.
>>
>> Previously, this mode hasn't been enabled in our driver and it was
>> assumed that all normal overlay planes are pre-multiplied by default.
>>
>> When a 3rd party app is used to input a image in a specific format,
>> e.g. PNG, as a source of a overlay plane to blend with the background
>> primary plane, the pixel color values are not pre-multiplied. So by
>> adding "Coverage" blend mode, our driver will support those cases.
>>
>> Reference:
>> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-compositi
>> on-properties
>>
>> Adding Coverage support also enables IGT kms_plane_alpha_blend
>> Coverage subtests:
>> 1. coverage-7efc
>> 2. coverage-vs-premult-vs-constant
>>
>> Signed-off-by: Sung Joon Kim <Sungjoon.Kim@amd.com>
>> ---
>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 17 ++++++++----
>> .../gpu/drm/amd/display/dc/core/dc_surface.c | 2 ++
>> drivers/gpu/drm/amd/display/dc/dc.h | 2 ++
>> .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 27 ++++++++++---------
>> .../drm/amd/display/dc/dcn20/dcn20_hwseq.c | 16 ++++++-----
>> 5 files changed, 40 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 2ea20dd7fccf..af2b5d232742 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -5380,17 +5380,19 @@ fill_plane_buffer_attributes(struct
>> amdgpu_device *adev,
>>
>> static void
>> fill_blending_from_plane_state(const struct drm_plane_state *plane_state,
>> - bool *per_pixel_alpha, bool *global_alpha,
>> - int *global_alpha_value)
>> + bool *per_pixel_alpha, bool *pre_multiplied_alpha,
>> + bool *global_alpha, int *global_alpha_value)
>> {
>> *per_pixel_alpha = false;
>> + *pre_multiplied_alpha = true;
>> *global_alpha = false;
>> *global_alpha_value = 0xff;
>>
>> if (plane_state->plane->type != DRM_PLANE_TYPE_OVERLAY)
>> return;
>>
>> - if (plane_state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI) {
>> + if (plane_state->pixel_blend_mode == DRM_MODE_BLEND_PREMULTI ||
>> + plane_state->pixel_blend_mode == DRM_MODE_BLEND_COVERAGE) {
>> static const uint32_t alpha_formats[] = {
>> DRM_FORMAT_ARGB8888,
>> DRM_FORMAT_RGBA8888,
>> @@ -5405,6 +5407,9 @@ fill_blending_from_plane_state(const struct drm_plane_state *plane_state,
>> break;
>> }
>> }
>> +
>> + if (per_pixel_alpha && plane_state->pixel_blend_mode == DRM_MODE_BLEND_COVERAGE)
>> + *pre_multiplied_alpha = false;
>> }
>>
>> if (plane_state->alpha < 0xffff) {
>> @@ -5567,7 +5572,7 @@ fill_dc_plane_info_and_addr(struct amdgpu_device *adev,
>> return ret;
>>
>> fill_blending_from_plane_state(
>> - plane_state, &plane_info->per_pixel_alpha,
>> + plane_state, &plane_info->per_pixel_alpha,
>> +&plane_info->pre_multiplied_alpha,
>> &plane_info->global_alpha, &plane_info->global_alpha_value);
>>
>> return 0;
>> @@ -5614,6 +5619,7 @@ static int fill_dc_plane_attributes(struct amdgpu_device *adev,
>> dc_plane_state->tiling_info = plane_info.tiling_info;
>> dc_plane_state->visible = plane_info.visible;
>> dc_plane_state->per_pixel_alpha = plane_info.per_pixel_alpha;
>> + dc_plane_state->pre_multiplied_alpha =
>> +plane_info.pre_multiplied_alpha;
>> dc_plane_state->global_alpha = plane_info.global_alpha;
>> dc_plane_state->global_alpha_value = plane_info.global_alpha_value;
>> dc_plane_state->dcc = plane_info.dcc; @@ -7905,7 +7911,8 @@ static
>> int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm,
>> if (plane->type == DRM_PLANE_TYPE_OVERLAY &&
>> plane_cap && plane_cap->per_pixel_alpha) {
>> unsigned int blend_caps = BIT(DRM_MODE_BLEND_PIXEL_NONE) |
>> - BIT(DRM_MODE_BLEND_PREMULTI);
>> + BIT(DRM_MODE_BLEND_PREMULTI) |
>> + BIT(DRM_MODE_BLEND_COVERAGE);
>>
>> drm_plane_create_alpha_property(plane);
>> drm_plane_create_blend_mode_property(plane, blend_caps); diff
>> --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
>> b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
>> index e6b9c6a71841..5bc6ff2fa73e 100644
>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
>> @@ -61,6 +61,8 @@ static void dc_plane_construct(struct dc_context *ctx, struct dc_plane_state *pl
>> plane_state->blend_tf->type = TF_TYPE_BYPASS;
>> }
>>
>> + plane_state->pre_multiplied_alpha = true;
>> +
>> }
>>
>> static void dc_plane_destruct(struct dc_plane_state *plane_state)
>> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h
>> b/drivers/gpu/drm/amd/display/dc/dc.h
>> index 26c24db8f1da..c353842d5c40 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dc.h
>> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
>> @@ -1011,6 +1011,7 @@ struct dc_plane_state {
>>
>> bool is_tiling_rotated;
>> bool per_pixel_alpha;
>> + bool pre_multiplied_alpha;
>> bool global_alpha;
>> int global_alpha_value;
>> bool visible;
>> @@ -1045,6 +1046,7 @@ struct dc_plane_info {
>> bool horizontal_mirror;
>> bool visible;
>> bool per_pixel_alpha;
>> + bool pre_multiplied_alpha;
>> bool global_alpha;
>> int global_alpha_value;
>> bool input_csc_enabled;
>> diff --git
>> a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
>> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
>> index e02ac75afbf7..e3a62873c0e7 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
>> @@ -2550,12 +2550,21 @@ void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
>> blnd_cfg.overlap_only = false;
>> blnd_cfg.global_gain = 0xff;
>>
>> - if (per_pixel_alpha && pipe_ctx->plane_state->global_alpha) {
>> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
>> - blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
>> - } else if (per_pixel_alpha) {
>> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
>> + if (per_pixel_alpha) {
>> + /* DCN1.0 has output CM before MPC which seems to screw with
>> + * pre-multiplied alpha.
>> + */
>> + blnd_cfg.pre_multiplied_alpha = (is_rgb_cspace(
>> + pipe_ctx->stream->output_color_space)
>> + && pipe_ctx->plane_state->pre_multiplied_alpha);
>> + if (pipe_ctx->plane_state->global_alpha) {
>> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
>> + blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
>> + } else {
>> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
>> + }
>> } else {
>> + blnd_cfg.pre_multiplied_alpha = false;
>> blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA;
>> }
>>
>> @@ -2564,14 +2573,6 @@ void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
>> else
>> blnd_cfg.global_alpha = 0xff;
>>
>> - /* DCN1.0 has output CM before MPC which seems to screw with
>> - * pre-multiplied alpha.
>> - */
>> - blnd_cfg.pre_multiplied_alpha = is_rgb_cspace(
>> - pipe_ctx->stream->output_color_space)
>> - && per_pixel_alpha;
>> -
>
> Could this break color management use-cases on DCN1.x for Windows?
> Dmytro Laktyushkin added this code 5 years ago. Maybe check with him.
>
> Overall I like this patch but let's see if we can confirm that this
> won't break DCN1.x on Windows.
>
> See commit ad32734699da4dd185405637459bf915a4f4cff6.
>
> Harry
I spoke with Dmytro and he is okay with the change
Joon
>
>> -
>> /*
>> * TODO: remove hack
>> * Note: currently there is a bug in init_hw such that diff --git
>> a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
>> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
>> index e1f87bd72e4a..c117830b8b9d 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
>> @@ -2346,12 +2346,16 @@ void dcn20_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
>> blnd_cfg.overlap_only = false;
>> blnd_cfg.global_gain = 0xff;
>>
>> - if (per_pixel_alpha && pipe_ctx->plane_state->global_alpha) {
>> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
>> - blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
>> - } else if (per_pixel_alpha) {
>> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
>> + if (per_pixel_alpha) {
>> + blnd_cfg.pre_multiplied_alpha = pipe_ctx->plane_state->pre_multiplied_alpha;
>> + if (pipe_ctx->plane_state->global_alpha) {
>> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
>> + blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
>> + } else {
>> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
>> + }
>> } else {
>> + blnd_cfg.pre_multiplied_alpha = false;
>> blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA;
>> }
>>
>> @@ -2365,7 +2369,7 @@ void dcn20_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
>> blnd_cfg.top_gain = 0x1f000;
>> blnd_cfg.bottom_inside_gain = 0x1f000;
>> blnd_cfg.bottom_outside_gain = 0x1f000;
>> - blnd_cfg.pre_multiplied_alpha = per_pixel_alpha;
>> +
>> if (pipe_ctx->plane_state->format
>> == SURFACE_PIXEL_FORMAT_GRPH_RGBE_ALPHA)
>> blnd_cfg.pre_multiplied_alpha = false;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm/amd/display: add Coverage blend mode for overlay plane
2022-05-20 17:50 ` Kim, Sung joon
@ 2022-05-20 17:55 ` Harry Wentland
0 siblings, 0 replies; 5+ messages in thread
From: Harry Wentland @ 2022-05-20 17:55 UTC (permalink / raw)
To: Kim, Sung joon, Li, Sun peng (Leo),
Siqueira, Rodrigo, Deucher, Alexander, Koenig, Christian
Cc: mwen, contact, amd-gfx list, markyacoub
<snip>
>
> On 2022-05-19 16:42, Harry Wentland wrote:
>>
>>
>> On 2022-05-13 16:22, Sung Joon Kim wrote:
>>> Issue fixed: Overlay plane alpha channel blending is incorrect
>>>
>>> Issue tracker: https://gitlab.freedesktop.org/drm/amd/-/issues/1769
>>>
>>> According to the KMS man page, there is a "Coverage" alpha blend mode
>>> that assumes the pixel color values have NOT been pre-multiplied and
>>> will be done when the actual blending to the background color values
>>> happens.
>>>
>>> Previously, this mode hasn't been enabled in our driver and it was
>>> assumed that all normal overlay planes are pre-multiplied by default.
>>>
>>> When a 3rd party app is used to input a image in a specific format,
>>> e.g. PNG, as a source of a overlay plane to blend with the background
>>> primary plane, the pixel color values are not pre-multiplied. So by
>>> adding "Coverage" blend mode, our driver will support those cases.
>>>
>>> Reference:
>>> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-compositi
>>> on-properties
>>>
>>> Adding Coverage support also enables IGT kms_plane_alpha_blend
>>> Coverage subtests:
>>> 1. coverage-7efc
>>> 2. coverage-vs-premult-vs-constant
>>>
>>> Signed-off-by: Sung Joon Kim <Sungjoon.Kim@amd.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
<snip>
>>> @@ -2564,14 +2573,6 @@ void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
>>> else
>>> blnd_cfg.global_alpha = 0xff;
>>>
>>> - /* DCN1.0 has output CM before MPC which seems to screw with
>>> - * pre-multiplied alpha.
>>> - */
>>> - blnd_cfg.pre_multiplied_alpha = is_rgb_cspace(
>>> - pipe_ctx->stream->output_color_space)
>>> - && per_pixel_alpha;
>>> -
>>
>> Could this break color management use-cases on DCN1.x for Windows?
>> Dmytro Laktyushkin added this code 5 years ago. Maybe check with him.
>>
>> Overall I like this patch but let's see if we can confirm that this
>> won't break DCN1.x on Windows.
>>
>> See commit ad32734699da4dd185405637459bf915a4f4cff6.
>>
>> Harry
>
> I spoke with Dmytro and he is okay with the change
>
Thanks for checking.
Harry
> Joon
>>
>>> -
>>> /*
>>> * TODO: remove hack
>>> * Note: currently there is a bug in init_hw such that diff --git
>>> a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
>>> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
>>> index e1f87bd72e4a..c117830b8b9d 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
>>> @@ -2346,12 +2346,16 @@ void dcn20_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
>>> blnd_cfg.overlap_only = false;
>>> blnd_cfg.global_gain = 0xff;
>>>
>>> - if (per_pixel_alpha && pipe_ctx->plane_state->global_alpha) {
>>> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
>>> - blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
>>> - } else if (per_pixel_alpha) {
>>> - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
>>> + if (per_pixel_alpha) {
>>> + blnd_cfg.pre_multiplied_alpha = pipe_ctx->plane_state->pre_multiplied_alpha;
>>> + if (pipe_ctx->plane_state->global_alpha) {
>>> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN;
>>> + blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value;
>>> + } else {
>>> + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA;
>>> + }
>>> } else {
>>> + blnd_cfg.pre_multiplied_alpha = false;
>>> blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA;
>>> }
>>>
>>> @@ -2365,7 +2369,7 @@ void dcn20_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx)
>>> blnd_cfg.top_gain = 0x1f000;
>>> blnd_cfg.bottom_inside_gain = 0x1f000;
>>> blnd_cfg.bottom_outside_gain = 0x1f000;
>>> - blnd_cfg.pre_multiplied_alpha = per_pixel_alpha;
>>> +
>>> if (pipe_ctx->plane_state->format
>>> == SURFACE_PIXEL_FORMAT_GRPH_RGBE_ALPHA)
>>> blnd_cfg.pre_multiplied_alpha = false;
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-20 17:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20220513202200.4698-1-Sungjoon.Kim@amd.com>
2022-05-13 20:23 ` [PATCH v2] drm/amd/display: add Coverage blend mode for overlay plane Kim, Sung joon
2022-05-19 20:00 ` Melissa Wen
[not found] ` <8b8c0c85-1d79-5cfd-f0d2-ebc8b732e0c9@amd.com>
2022-05-19 20:45 ` Harry Wentland
2022-05-20 17:50 ` Kim, Sung joon
2022-05-20 17:55 ` Harry Wentland
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.