Re: [Intel-gfx] [PATCH 02/11] drm/i915: Make the CRTC wrt. CSC state consistent during sanitize-disabling

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Apr 26, 2023 at 07:52:56PM +0300, Imre Deak wrote:
> intel_crtc_free_hw_state() frees all the Intel specific CSC blobs in the
> CRTC state, but the next memset() will only clear the corresponding
> pointers for the ones stored in intel_crtc_state:hw. Clear the ones
> stored in intel_crtc_state as well. Also sync the UAPI state with the HW
> state after the HW state was reset. This will reset the uapi.active
> flag as well, so no need to do this separately. Syncing the state will
> create a new umode blob, so move deleting the blob after the sync call.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_modeset_setup.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index eefa4018dc0c2..57d087de654f8 100644
> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> @@ -30,6 +30,8 @@
>  #include "intel_wm.h"
>  #include "skl_watermark.h"
>  
> +static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state);
> +
>  static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
>  					struct drm_modeset_acquire_ctx *ctx)
>  {
> @@ -88,13 +90,17 @@ static void intel_crtc_disable_noatomic(struct intel_crtc *crtc,
>  	crtc->active = false;
>  	crtc->base.enabled = false;
>  
> -	drm_WARN_ON(&i915->drm,
> -		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);
> -	crtc_state->uapi.active = false;
>  	crtc_state->uapi.connector_mask = 0;
>  	crtc_state->uapi.encoder_mask = 0;
> +
>  	intel_crtc_free_hw_state(crtc_state);
>  	memset(&crtc_state->hw, 0, sizeof(crtc_state->hw));
> +	crtc_state->pre_csc_lut = NULL;
> +	crtc_state->post_csc_lut = NULL;
> +	intel_crtc_copy_hw_to_uapi_state(crtc_state);
> +
> +	drm_WARN_ON(&i915->drm,
> +		    drm_atomic_set_mode_for_crtc(&crtc_state->uapi, NULL) < 0);

Hmm. Is there a reason we can't just do the full destroy+reset
thing that intel_modeset_readout_hw_state() does?

>  
>  	for_each_encoder_on_crtc(&i915->drm, &crtc->base, encoder)
>  		encoder->base.crtc = NULL;
> -- 
> 2.37.2

-- 
Ville Syrjälä
Intel