Re: [PATCH 10/10] drm/i915: Remove crtc->active from crtc_enable callbacks

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

On Thu, Oct 11, 2018 at 12:04:57PM +0200, Maarten Lankhorst wrote:
> Now that most of the driver is atomic, we no longer need to worry
> about setting crtc->active right before actually enabling the pipe.

Hmm. I think we need at least that wait_for_vblank_if_active() change
we discussed. And maybe there's something in the watermark code that
depends on this being more or less accurate indication of thw hw state.
That probably needs some actual thought.

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 21 +--------------------
>  1 file changed, 1 insertion(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ad2c207d18bb..0e4bdd5c337e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5585,9 +5585,6 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  	struct intel_atomic_state *old_intel_state =
>  		to_intel_atomic_state(old_state);
>  
> -	if (WARN_ON(intel_crtc->active))
> -		return;
> -
>  	/*
>  	 * Sometimes spurious CPU pipe underruns happen during FDI
>  	 * training, at least with VGA+HDMI cloning. Suppress them.
> @@ -5617,8 +5614,6 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	ironlake_set_pipeconf(pipe_config);
>  
> -	intel_crtc->active = true;
> -
>  	intel_encoders_pre_enable(crtc, pipe_config, old_state);
>  
>  	if (pipe_config->has_pch_encoder) {
> @@ -5715,9 +5710,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	bool psl_clkgate_wa;
>  	u32 pipe_chicken;
>  
> -	if (WARN_ON(intel_crtc->active))
> -		return;
> -
>  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
>  
>  	if (pipe_config->shared_dpll)
> @@ -5754,8 +5746,6 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_color_set_csc(&pipe_config->base);
>  
> -	intel_crtc->active = true;
> -
>  	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
>  	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
>  			 pipe_config->pch_pfit.enabled;
> @@ -6067,9 +6057,6 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int pipe = intel_crtc->pipe;
>  
> -	if (WARN_ON(intel_crtc->active))
> -		return;
> -
>  	if (intel_crtc_has_dp_encoder(pipe_config))
>  		intel_dp_set_m_n(pipe_config, M1_N1);
>  
> @@ -6085,8 +6072,6 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_color_set_csc(&pipe_config->base);
>  
> -	intel_crtc->active = true;
> -
>  	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
>  
>  	intel_encoders_pre_pll_enable(crtc, pipe_config, old_state);
> @@ -6135,9 +6120,6 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum pipe pipe = intel_crtc->pipe;
>  
> -	if (WARN_ON(intel_crtc->active))
> -		return;
> -
>  	i9xx_set_pll_dividers(pipe_config);
>  
>  	if (intel_crtc_has_dp_encoder(pipe_config))
> @@ -6148,8 +6130,6 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	i9xx_set_pipeconf(pipe_config);
>  
> -	intel_crtc->active = true;
> -
>  	if (!IS_GEN2(dev_priv))
>  		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
>  
> @@ -12525,6 +12505,7 @@ static void intel_update_crtc(struct drm_crtc *crtc,
>  
>  	if (modeset) {
>  		update_scanline_offset(pipe_config);
> +		intel_crtc->active = true;
>  		dev_priv->display.crtc_enable(pipe_config, state);
>  
>  		/* vblanks work again, re-enable pipe CRC. */
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx