[PATCH] drm/colorop: get DATA blob ref at duplicate_state

[Xaver Hugl]

Hi Harry,

I applied this patch and the two issues I mentioned before are gone.
I noticed a new problem though: Changes in the COLOR_PIPELINE value
aren't always applied immediately. For testing I played an HDR video
on an SDR screen with the work/zamundaaa/drm-colorop KWin branch, and
made it fullscreen. When I do that, the video is reinterpreted as sRGB
until the pipeline actually gets used at some later point in time, and
when opening another app to interrupt direct scanout while the
pipeline is being applied, the previous pipeline keeps being applied
until it switches to bypass. It often applies the proper value when
the cursor plane gets enabled or disabled, but that doesn't seem to be
reliable and it sometimes happens without cursor changes.
I checked with drm_info that KWin has actually set COLOR_PIPELINE to
the pipeline or bypass, so I'm confident that this issue is on the
kernel side.

When the pipeline is actually applied, it works correctly (outside of
some banding, but that's expected with the current state of the KWin
branch).


Am Di., 10. Sept. 2024 um 21:24 Uhr schrieb Harry Wentland
<harry.wentland at amd.com>:
>
> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
> ---
>
> It was a stupid mistake on my part. The duplicate_state
> function needs to take a reference to the blob.
>
> This should fix it. Please give it a try if you can.
> I'll roll it into the patch that introduces the bug in
> my v6.
>
> Harry
>
>  drivers/gpu/drm/drm_colorop.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
> index 86f7f084a73f..fd1cd934df48 100644
> --- a/drivers/gpu/drm/drm_colorop.c
> +++ b/drivers/gpu/drm/drm_colorop.c
> @@ -401,6 +401,9 @@ static void __drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colo
>  {
>         memcpy(state, colorop->state, sizeof(*state));
>
> +       if (state->data)
> +               drm_property_blob_get(state->data);
> +
>         state->bypass = true;
>  }
>
> --
> 2.46.0
>