Re: [PATCH v2] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is

From: "Michel Dänzer" <michel@daenzer.net>
To: "Kazlauskas, Nicholas" <nicholas.kazlauskas@amd.com>,
	Daniel Vetter <daniel@ffwll.ch>,
	Harry Wentland <hwentlan@amd.com>
Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is
Date: Mon, 14 Sep 2020 17:44:30 +0200	[thread overview]
Message-ID: <6fe69e5e-853a-0ea4-8ef7-0754bc25e43d@daenzer.net> (raw)
In-Reply-To: <26ce07b5-d225-547d-cec6-b2f87bdd9f12@amd.com>

On 2020-09-14 5:33 p.m., Kazlauskas, Nicholas wrote:
> On 2020-09-14 11:22 a.m., Michel Dänzer wrote:
>> On 2020-09-14 4:37 p.m., Kazlauskas, Nicholas wrote:
>>> On 2020-09-14 3:52 a.m., Michel Dänzer wrote:
>>>>
>>>> P.S. Since DCN doesn't make a distinction between primary or overlay 
>>>> planes in hardware, could ChromiumOS achieve the same effect with 
>>>> only the primary plane instead of only an overlay plane? If so, 
>>>> maybe there's no need for the "black plane hack".
>>>>
>>>>
>>>
>>> I only know that atomictest tries to enable this configuration. Not 
>>> sure if ChromiumOS or other "real" userspace tries this configuration.
>>
>> Someone mentioned that ChromiumOS uses this for video playback with 
>> black bars (when the video aspect ratio doesn't match the display's).
> 
> We only expose support for NV12 on the primary plane so we wouldn't be 
> hitting this case at least.

Interesting, so if we're lucky this really won't affect any real-world apps.

>>> We can always go back to lying to userspace about the cursor being
>>> visible if the commit fails in that case I guess [...]
>>
>> I'm not sure what you mean by that / how it could work.
> 
> Dropping the check you added in this patch:
> 
> +    if (state->enable &&
> +        !(state->plane_mask & drm_plane_mask(crtc->primary)))
>           return -EINVAL;
> 
> That way we can still allow the cursor plane to be enabled while 
> primary/overlay are not, it's just not going to be actually visible on 
> the screen. It'll fail some IGT tests but nothing really uses this 
> configuration as more than just a temporary state.

As Daniel pointed out in <20200901075432.GW2352366@phenom.ffwll.local> 
in the v1 patch thread, that won't fly, since atomic userspace can 
legitimately expect the cursor plane to be visible in that case.

-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx