[PATCH 4/5] drm/vmwgfx: Fix crtc's atomic check conditional

[Martin Krastev]

On Wed, Apr 3, 2024 at 2:28 AM Zack Rusin <zack.rusin at broadcom.com> 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 at broadcom.com>
> Fixes: 06ec41909e31 ("drm/vmwgfx: Add and connect CRTC helper functions")
> Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list at broadcom.com>
> Cc: dri-devel at lists.freedesktop.org
> Cc: <stable at vger.kernel.org> # 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 at broadcom.com>

Regards,
Martin