On Wed, Apr 3, 2024 at 2:28 AM Zack Rusin <zack.rusin@xxxxxxxxxxxx> wrote:
>
> The conditional was supposed to prevent enabling of a crtc state
> without a set primary plane. Accidently it also prevented disabling
> crtc state with a set primary plane. Neither is correct.
>
> Fix the conditional and just driver-warn when a crtc state has been
> enabled without a primary plane which will help debug broken userspace.
>
> Fixes IGT's kms_atomic_interruptible and kms_atomic_transition tests.
>
> Signed-off-by: Zack Rusin <zack.rusin@xxxxxxxxxxxx>
> Fixes: 06ec41909e31 ("drm/vmwgfx: Add and connect CRTC helper functions")
> Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@xxxxxxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: <stable@xxxxxxxxxxxxxxx> # v4.12+
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e33e5993d8fc..13b2820cae51 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -931,6 +931,7 @@ int vmw_du_cursor_plane_atomic_check(struct drm_plane *plane,
> int vmw_du_crtc_atomic_check(struct drm_crtc *crtc,
> struct drm_atomic_state *state)
> {
> + struct vmw_private *vmw = vmw_priv(crtc->dev);
> struct drm_crtc_state *new_state = drm_atomic_get_new_crtc_state(state,
> crtc);
> struct vmw_display_unit *du = vmw_crtc_to_du(new_state->crtc);
> @@ -938,9 +939,13 @@ int vmw_du_crtc_atomic_check(struct drm_crtc *crtc,
> bool has_primary = new_state->plane_mask &
> drm_plane_mask(crtc->primary);
>
> - /* We always want to have an active plane with an active CRTC */
> - if (has_primary != new_state->enable)
> - return -EINVAL;
> + /*
> + * This is fine in general, but broken userspace might expect
> + * some actual rendering so give a clue as why it's blank.
> + */
> + if (new_state->enable && !has_primary)
> + drm_dbg_driver(&vmw->drm,
> + "CRTC without a primary plane will be blank.\n");
>
>
> if (new_state->connector_mask != connector_mask &&
> --
> 2.40.1
>
LGTM!
Reviewed-by: Martin Krastev <martin.krastev@xxxxxxxxxxxx>
Regards,
Martin
