diff mbox series

drm/vmwgfx: Don't destroy Screen Target when CRTC is enabled but inactive

Message ID 20240531203358.26677-1-ian.forbes@broadcom.com (mailing list archive)
State New, archived
Headers show
Series drm/vmwgfx: Don't destroy Screen Target when CRTC is enabled but inactive | expand

Commit Message

Ian Forbes May 31, 2024, 8:33 p.m. UTC
drm_crtc_helper_funcs::atomic_disable can be called even when the CRTC is
still enabled. This can occur when the mode changes or the CRTC is set as
inactive.

In the case where the CRTC is being set as inactive we only want to
blank the screen. The Screen Target should remain intact as long as the
mode has not changed and CRTC is enabled.

This fixes a bug with GDM where locking the screen results in a permanent
black screen because the Screen Target is no longer defined.

Fixes: 7b0062036c3b ("drm/vmwgfx: Implement virtual crc generation")
Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Martin Krastev June 3, 2024, 1:02 p.m. UTC | #1
On Fri, May 31, 2024 at 11:34 PM Ian Forbes <ian.forbes@broadcom.com> wrote:
>
> drm_crtc_helper_funcs::atomic_disable can be called even when the CRTC is
> still enabled. This can occur when the mode changes or the CRTC is set as
> inactive.
>
> In the case where the CRTC is being set as inactive we only want to
> blank the screen. The Screen Target should remain intact as long as the
> mode has not changed and CRTC is enabled.
>
> This fixes a bug with GDM where locking the screen results in a permanent
> black screen because the Screen Target is no longer defined.
>
> Fixes: 7b0062036c3b ("drm/vmwgfx: Implement virtual crc generation")
> Signed-off-by: Ian Forbes <ian.forbes@broadcom.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index 2041c4d48daa..d2f523191314 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -414,6 +414,7 @@ static void vmw_stdu_crtc_atomic_disable(struct drm_crtc *crtc,
>  {
>         struct vmw_private *dev_priv;
>         struct vmw_screen_target_display_unit *stdu;
> +       struct drm_crtc_state *new_crtc_state;
>         int ret;
>
>         if (!crtc) {
> @@ -423,6 +424,7 @@ static void vmw_stdu_crtc_atomic_disable(struct drm_crtc *crtc,
>
>         stdu     = vmw_crtc_to_stdu(crtc);
>         dev_priv = vmw_priv(crtc->dev);
> +       new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>
>         if (dev_priv->vkms_enabled)
>                 drm_crtc_vblank_off(crtc);
> @@ -434,6 +436,14 @@ static void vmw_stdu_crtc_atomic_disable(struct drm_crtc *crtc,
>
>                 (void) vmw_stdu_update_st(dev_priv, stdu);
>
> +               /* Don't destroy the Screen Target if we are only setting the
> +                * display as inactive
> +                */
> +               if (new_crtc_state->enable &&
> +                   !new_crtc_state->active &&
> +                   !new_crtc_state->mode_changed)
> +                       return;
> +
>                 ret = vmw_stdu_destroy_st(dev_priv, stdu);
>                 if (ret)
>                         DRM_ERROR("Failed to destroy Screen Target\n");
> --
> 2.34.1
>

LGTM!

Reviewed-by: Martin Krastev <martin.krastev@broadcom.com>

Regards,
Martin
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 2041c4d48daa..d2f523191314 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -414,6 +414,7 @@  static void vmw_stdu_crtc_atomic_disable(struct drm_crtc *crtc,
 {
 	struct vmw_private *dev_priv;
 	struct vmw_screen_target_display_unit *stdu;
+	struct drm_crtc_state *new_crtc_state;
 	int ret;
 
 	if (!crtc) {
@@ -423,6 +424,7 @@  static void vmw_stdu_crtc_atomic_disable(struct drm_crtc *crtc,
 
 	stdu     = vmw_crtc_to_stdu(crtc);
 	dev_priv = vmw_priv(crtc->dev);
+	new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 
 	if (dev_priv->vkms_enabled)
 		drm_crtc_vblank_off(crtc);
@@ -434,6 +436,14 @@  static void vmw_stdu_crtc_atomic_disable(struct drm_crtc *crtc,
 
 		(void) vmw_stdu_update_st(dev_priv, stdu);
 
+		/* Don't destroy the Screen Target if we are only setting the
+		 * display as inactive
+		 */
+		if (new_crtc_state->enable &&
+		    !new_crtc_state->active &&
+		    !new_crtc_state->mode_changed)
+			return;
+
 		ret = vmw_stdu_destroy_st(dev_priv, stdu);
 		if (ret)
 			DRM_ERROR("Failed to destroy Screen Target\n");