Re: [PATCH 50/70] drm/loongsoon: Move hardware reset to CRTC creation

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Broken S3 Resume due to missing CRTC hardware reset
--

--- Patch [50]: [PATCH 50/70] drm/loongsoon: Move hardware reset to CRTC creation ---
commit 841a0290c125afb8611341788938204f1fca139c
Author: Maxime Ripard <[email protected]>
drm/loongsoon: Move hardware reset to CRTC creation

This commit moves the hardware-level CRTC reset out of the atomic reset
hook and into the CRTC initialization functions.

> diff --git a/drivers/gpu/drm/loongson/lsdc_crtc.c b/drivers/gpu/drm/loongson/lsdc_crtc.c
> --- a/drivers/gpu/drm/loongson/lsdc_crtc.c
> +++ b/drivers/gpu/drm/loongson/lsdc_crtc.c
> @@ -403,9 +401,6 @@ static void lsdc_crtc_reset(struct drm_crtc *crtc)
>  		__drm_atomic_helper_crtc_reset(crtc, NULL);
>  	else
>  		__drm_atomic_helper_crtc_reset(crtc, &priv_crtc_state->base);
> -
> -	/* Reset the CRTC hardware, this is required for S3 support */
> -	ops->reset(lcrtc);
>  }

[Severity: High]
Since drm_mode_config_reset() is typically called via drm_atomic_helper_resume()
during system resume, does removing the hardware reset from this hook prevent
the CRTC from being correctly reinitialized after waking from sleep?

[ ... ]

> @@ -983,6 +978,9 @@ int ls7a1000_crtc_init(struct drm_device *ddev,
>  
>  	drm_crtc_enable_color_mgmt(crtc, 0, false, 256);
>  
> +	/* Reset the CRTC hardware, this is required for S3 support */
> +	lcrtc->hw_ops->reset(lcrtc);
> +
>  	return 0;
>  }

[Severity: High]
The comment explicitly notes this reset is required for S3 support, but
ls7a1000_crtc_init() only executes during device probe.

During S3 resume, the hardware registers will likely contain garbage values. If
the hardware reset is no longer called during resume, and the atomic commit
path merely performs a read-modify-write to enable the output, won't this
preserve those garbage bits and cause the display hardware to hang or fail
to resume?

(The same question applies to the identical change in ls7a2000_crtc_init below).

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=50