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:
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 <mdaen...@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.vet...@ffwll.ch>
Signed-off-by: Michel Dänzer <mdaen...@redhat.com>
Commit message agrees with my understand of this maze now, so:
Acked-by: Daniel Vetter <daniel.vet...@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.
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.
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 [...]
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.
Regards,
Nicholas Kazlauskas
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlaus...@amd.com>
Thanks!
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
