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@xxxxxxxxx>
> ---
> 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
