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

[Kazlauskas, Nicholas]

On 2020-09-14 3:52 a.m., Michel Dänzer wrote:
> On 2020-09-07 9:57 a.m., Daniel Vetter wrote:
>> On Fri, Sep 04, 2020 at 12:43:04PM +0200, Michel Dänzer wrote:
>>> From: Michel Dänzer <mdaenzer at redhat.com>
>>>
>>> Don't check drm_crtc_state::active for this either, per its
>>> documentation in include/drm/drm_crtc.h:
>>>
>>>   * Hence drivers must not consult @active in their various
>>>   * &drm_mode_config_funcs.atomic_check callback to reject an atomic
>>>   * commit.
>>>
>>> atomic_remove_fb disables the CRTC as needed for disabling the primary
>>> plane.
>>>
>>> This prevents at least the following problems if the primary plane gets
>>> disabled (e.g. due to destroying the FB assigned to the primary plane,
>>> as happens e.g. with mutter in Wayland mode):
>>>
>>> * The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID
>>>    (which enables the cursor plane).
>>> * If the cursor plane was enabled, changing the legacy DPMS property
>>>    value from off to on returned EINVAL.
>>>
>>> v2:
>>> * Minor changes to code comment and commit log, per review feedback.
>>>
>>> GitLab: 
>>> https://gitlab.gnome.org/GNOME/mutter/-/issues/1108 
>>>
>>> GitLab: 
>>> https://gitlab.gnome.org/GNOME/mutter/-/issues/1165 
>>>
>>> GitLab: 
>>> https://gitlab.gnome.org/GNOME/mutter/-/issues/1344 
>>>
>>> Suggested-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>>> Signed-off-by: Michel Dänzer <mdaenzer at redhat.com>
>>
>> Commit message agrees with my understand of this maze now, so:
>>
>> Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> Thanks Daniel!
> 
> 
> Nick / Harry, any more feedback? If not, can you apply this?
> 
> 
> 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.

Maybe for now this can go in and if something breaks we can deal with 
the fallout then. We can always go back to lying to userspace about the 
cursor being visible if the commit fails in that case I guess since the 
blank plane hack is going to add a significant amount of complexity to DM.

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>

Regards,
Nicholas Kazlauskas