Re: [PATCH 12/31] drm/i915: Fix PSR initialization.

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Nov 05, 2015 at 10:50:04AM -0800, Rodrigo Vivi wrote:
> PSR is still disabled by default, but even passing i915.enable_psr=1
> at this point we weren't able to get PSR working because with
> fastboot by default in place we weren't executing the path that enables
> encoder and consequently PSR.
> 
> Now with psr_ready in place and PSR using crtc signature we can move
> its enable/disable sequences from the encoder enable to the post
> atomic modeset functions.
> 
> i915.enable_psr parameter is still used to enable/disable psr feature
> on the next primary plane update. So current test cases that relies
> on this flow still works.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c     |  2 --
>  drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c      |  5 -----
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  4 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index b8f8dee..36db970 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2404,7 +2404,6 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
>  			intel_dp_stop_link_train(intel_dp);
>  
>  		intel_edp_backlight_on(intel_dp);
> -		intel_psr_enable(intel_crtc);
>  		intel_edp_drrs_enable(intel_dp);
>  	}
>  
> @@ -2432,7 +2431,6 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  
>  		intel_edp_drrs_disable(intel_dp);
> -		intel_psr_disable(intel_crtc);
>  		intel_edp_backlight_off(intel_dp);
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 869929d..f67e2ee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4687,6 +4687,9 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
>  	if (atomic->enable_ips)
>  		intel_ips_enable(crtc);
>  
> +	if (atomic->enable_psr)
> +		intel_psr_enable(crtc);

What we need here is a post-modeset fixup hook for encoders/connectors.
That would avoid the layering violation of going through a crtc and then
doing the crtc->encoder lookup you do in the previous patch. And we have
other uses for this, e.g. mipi self refresh, fixing up infoframes and all
those things.
-Daniel

> +
>  	if (atomic->post_enable_primary)
>  		intel_post_enable_primary(&crtc->base);
>  
> @@ -4705,6 +4708,9 @@ static void intel_pre_plane_update(struct intel_crtc *crtc)
>  	if (crtc->atomic.disable_ips)
>  		intel_ips_disable_if_alone(crtc);
>  
> +	if (crtc->atomic.disable_psr)
> +		intel_psr_disable(crtc);
> +
>  	if (atomic->pre_disable_primary)
>  		intel_pre_disable_primary(&crtc->base);
>  
> @@ -11560,9 +11566,18 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  			intel_crtc->atomic.disable_ips = true;
>  
>  			intel_crtc->atomic.disable_fbc = true;
> +
> +			intel_crtc->atomic.disable_psr = true;
>  		}
>  		if (visible && intel_crtc->config->ips_ready)
>  			intel_crtc->atomic.enable_ips = true;
> +
> +		if (visible && intel_crtc->config->psr_ready) {
> +			if (i915.enable_psr)
> +				intel_crtc->atomic.enable_psr = true;
> +			else
> +				intel_crtc->atomic.disable_psr = true;
> +		}
>  		/*
>  		 * FBC does not work on some platforms for rotated
>  		 * planes, so disable it when rotation is not 0 and
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 92f59cc..f0ee497 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2373,9 +2373,6 @@ static void intel_disable_dp(struct intel_encoder *encoder)
>  	if (crtc->config->has_audio)
>  		intel_audio_codec_disable(encoder);
>  
> -	if (HAS_PSR(dev) && !HAS_DDI(dev))
> -		intel_psr_disable(crtc);
> -
>  	/* Make sure the panel is off before trying to change the mode. But also
>  	 * ensure that we have vdd while we switch off the panel. */
>  	intel_edp_panel_vdd_on(intel_dp);
> @@ -2629,10 +2626,8 @@ static void g4x_enable_dp(struct intel_encoder *encoder)
>  static void vlv_enable_dp(struct intel_encoder *encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> -	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
>  
>  	intel_edp_backlight_on(intel_dp);
> -	intel_psr_enable(crtc);
>  }
>  
>  static void g4x_pre_enable_dp(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cafe4c1..d599d54 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -531,6 +531,7 @@ struct intel_crtc_atomic_commit {
>  	/* Sleepable operations to perform before commit */
>  	bool disable_fbc;
>  	bool disable_ips;
> +	bool disable_psr;
>  	bool disable_cxsr;
>  	bool pre_disable_primary;
>  	bool update_wm_pre, update_wm_post;
> @@ -540,6 +541,7 @@ struct intel_crtc_atomic_commit {
>  	bool wait_vblank;
>  	bool update_fbc;
>  	bool enable_ips;
> +	bool enable_psr;
>  	bool post_enable_primary;
>  	unsigned update_sprite_watermarks;
>  };
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx