[PATCH 2/2] amd/display: add cursor check for YUV primary plane

[PATCH 2/2] amd/display: add cursor check for YUV primary plane

[Simon Ser]

The cursor plane can't be displayed if the primary plane isn't
using an RGB format. Reject such atomic commits so that user-space
can have a fallback instead of an invisible cursor.

In theory we could support YUV if the cursor is also YUV, but at the
moment only ARGB8888 cursors are supported.

Signed-off-by: Simon Ser <contact@emersion.fr>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Harry Wentland <hwentlan@amd.com>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +++++++
 1 file changed, 7 insertions(+)

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 4548b779bbce..f659e6cfdfcf 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -9239,6 +9239,13 @@ static int dm_check_crtc_cursor(struct drm_atomic_state *state,
 		return -EINVAL;
 	}
 
+	/* In theory we could probably support YUV cursors when the primary
+	 * plane uses a YUV format, but there's no use-case for it yet. */
+	if (new_primary_state->fb && new_primary_state->fb->format->is_yuv) {
+		drm_dbg_atomic(crtc->dev, "Cursor plane can't be used with YUV primary plane\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.30.1


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] amd/display: add cursor check for YUV primary plane

[Kazlauskas, Nicholas]

On 2021-02-19 11:19 a.m., Simon Ser wrote:
> The cursor plane can't be displayed if the primary plane isn't
> using an RGB format. Reject such atomic commits so that user-space
> can have a fallback instead of an invisible cursor.
> 
> In theory we could support YUV if the cursor is also YUV, but at the
> moment only ARGB8888 cursors are supported.

Patch 1 looks good, but this patch needs to be adjusted.

We can support cursor plane, but only if we have an overlay plane 
enabled that's using XRGB/ARGB.

This is what we do on Chrome OS for video playback:

Cursor Plane - ARGB8888
Overlay Plane - ARGB8888 Desktop/UI with cutout for video
Primary Plane - NV12 video

So this new check would break this usecase. It needs to check that there 
isn't an XRGB/ARGB plane at the top of the blending chain instead.

Regards,
Nicholas Kazlauskas

> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Harry Wentland <hwentlan@amd.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> 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 4548b779bbce..f659e6cfdfcf 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -9239,6 +9239,13 @@ static int dm_check_crtc_cursor(struct drm_atomic_state *state,
>   		return -EINVAL;
>   	}
>   
> +	/* In theory we could probably support YUV cursors when the primary
> +	 * plane uses a YUV format, but there's no use-case for it yet. */
> +	if (new_primary_state->fb && new_primary_state->fb->format->is_yuv) {
> +		drm_dbg_atomic(crtc->dev, "Cursor plane can't be used with YUV primary plane\n");
> +		return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] amd/display: add cursor check for YUV primary plane

[Simon Ser]

On Friday, February 19th, 2021 at 6:22 PM, Kazlauskas, Nicholas <nicholas.kazlauskas@amd.com> wrote:

> We can support cursor plane, but only if we have an overlay plane
> enabled that's using XRGB/ARGB.
>
> This is what we do on Chrome OS for video playback:
>
> Cursor Plane - ARGB8888
> Overlay Plane - ARGB8888 Desktop/UI with cutout for video
> Primary Plane - NV12 video
>
> So this new check would break this usecase. It needs to check that there
> isn't an XRGB/ARGB plane at the top of the blending chain instead.

Oh, interesting. I'll adjust the patch.

Related: how does this affect scaling? Right now there is a check that makes
sure the cursor plane scaling matches the primary plane's. Should we instead
check that the cursor plane scaling matches the top-most XRGB/ARGB plane's?
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] amd/display: add cursor check for YUV primary plane

[Kazlauskas, Nicholas]

On 2021-02-19 12:29 p.m., Simon Ser wrote:
> On Friday, February 19th, 2021 at 6:22 PM, Kazlauskas, Nicholas <nicholas.kazlauskas@amd.com> wrote:
> 
>> We can support cursor plane, but only if we have an overlay plane
>> enabled that's using XRGB/ARGB.
>>
>> This is what we do on Chrome OS for video playback:
>>
>> Cursor Plane - ARGB8888
>> Overlay Plane - ARGB8888 Desktop/UI with cutout for video
>> Primary Plane - NV12 video
>>
>> So this new check would break this usecase. It needs to check that there
>> isn't an XRGB/ARGB plane at the top of the blending chain instead.
> 
> Oh, interesting. I'll adjust the patch.
> 
> Related: how does this affect scaling? Right now there is a check that makes
> sure the cursor plane scaling matches the primary plane's. Should we instead
> check that the cursor plane scaling matches the top-most XRGB/ARGB plane's?
> 

Can't really do scaling on the cursor plane itself. It scales with the 
underlying pipe driving it so it'll only be correct if it matches that.

Primary plane isn't the correct check here since we always use the 
topmost pipe in the blending chain to draw the cursor - in the example I 
gave it'd have to match the overlay plane's scaling, not the primary 
plane's.

Regards,
Nicholas Kazlauskas

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] amd/display: add cursor check for YUV primary plane

[Simon Ser]

On Friday, February 19th, 2021 at 6:41 PM, Kazlauskas, Nicholas <nicholas.kazlauskas@amd.com> wrote:

> > Related: how does this affect scaling? Right now there is a check that makes
> > sure the cursor plane scaling matches the primary plane's. Should we instead
> > check that the cursor plane scaling matches the top-most XRGB/ARGB plane's?
> >
>
> Can't really do scaling on the cursor plane itself. It scales with the
> underlying pipe driving it so it'll only be correct if it matches that.
>
> Primary plane isn't the correct check here since we always use the
> topmost pipe in the blending chain to draw the cursor - in the example I
> gave it'd have to match the overlay plane's scaling, not the primary
> plane's.

OK, thanks for the info. That means the check I introduced some time
ago is wrong (it's right above my patch). I'll send a fix for that too.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx