Re: [PATCH v2] drm/amd/display: add Coverage blend mode for overlay plane

[Harry Wentland <harry.wentland@amd.com>]

<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;
>>
>