[Intel-gfx] [PATCH] drm/i915: Add functions to force psr exit

[Daniel Vetter]

On Mon, Jul 15, 2013 at 05:29:15PM -0300, Rodrigo Vivi wrote:
> PSR tracking engine in HSW doesn't detect automagically some directly copy area
> operations through scanout so we will have to kick it manually and
> reschedule it to come back to normal operation as soon as possible.
> 
> v2: Before PSR Hook. Don't force it when busy yet.
> v3/v4: Solved small conflict.
> v5: setup once function was already added on previous commit.
> v6: Use POSTING_READ to make sure the mutex works as a write barrier as
>     suggested by Chris Wilson
> 
> CC: Chris Wilson <chris at chris-wilson.co.uk>
> Reviewed-by: Shobhit Kumar <shobhit.kumar at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>  drivers/gpu/drm/i915/intel_dp.c  | 48 ++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  3 +++
>  3 files changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3bca337..dc10345 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1840,6 +1840,7 @@
>  #define   EDP_PSR_PERF_CNT_MASK		0xffffff
>  
>  #define EDP_PSR_DEBUG_CTL		0x64860
> +#define   EDP_PSR_DEBUG_FORCE_EXIT	(3<<30)
>  #define   EDP_PSR_DEBUG_MASK_LPSP	(1<<27)
>  #define   EDP_PSR_DEBUG_MASK_MEMUP	(1<<26)
>  #define   EDP_PSR_DEBUG_MASK_HPD	(1<<25)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3c9473c..47e1676 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1393,6 +1393,50 @@ bool intel_edp_is_psr_enabled(struct drm_device *dev)
>  	return I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
>  }
>  
> +static void intel_edp_psr_delayed_normal_work(struct work_struct *__work)
> +{
> +	struct intel_dp *intel_dp = container_of(to_delayed_work(__work),
> +						 struct intel_dp,
> +						 edp_psr_delayed_normal_work);
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	mutex_lock(&intel_dp->psr_exit_mutex);
> +	I915_WRITE(EDP_PSR_DEBUG_CTL, I915_READ(EDP_PSR_DEBUG_CTL) &
> +		   ~EDP_PSR_DEBUG_FORCE_EXIT);
> +	POSTING_READ(EDP_PSR_DEBUG_CTL);
> +	mutex_unlock(&intel_dp->psr_exit_mutex);
> +}
> +
> +void intel_edp_psr_force_exit(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_encoder *encoder;
> +	struct intel_dp *intel_dp = NULL;
> +
> +	if (!intel_edp_is_psr_enabled(dev))
> +		return;
> +
> +	list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head)
> +		if (encoder->type == INTEL_OUTPUT_EDP)
> +			intel_dp = enc_to_intel_dp(&encoder->base);
> +
> +	if (!intel_dp)
> +		return;
> +
> +	if (WARN_ON(!intel_dp->psr_setup_done))
> +		return;

If you have to dig out your data like this it usually means that it's at
the wrong spot. Imo we should track a psr_possible bit in the pipe_config
and a psr_enabled (and the locking for the runtime mutex) in the crtc
itself. Also, the psr_setup_done thing is another hint that we should move
this into the crtc.

Second I prefer if functions with tricky tie-in with existing code are
used in the same patch they're created - if it's split over two patches
review is a bit a pain. But if I read the follow-up patch correctly we
call this function from the busy_ioctl unconditionally, which will
obviously result in tons of falls postives - e.g. libdrm uses this ioctl
to manage it's buffer cache.

I think I'll punt on this patch and merge just the parts from the next one
for normal backbuffer rendering with pageflips model.
-Daniel

> +
> +	mutex_lock(&intel_dp->psr_exit_mutex);
> +	I915_WRITE(EDP_PSR_DEBUG_CTL, I915_READ(EDP_PSR_DEBUG_CTL) |
> +		   EDP_PSR_DEBUG_FORCE_EXIT);
> +	POSTING_READ(EDP_PSR_DEBUG_CTL);
> +	mutex_unlock(&intel_dp->psr_exit_mutex);
> +
> +	schedule_delayed_work(&intel_dp->edp_psr_delayed_normal_work,
> +			      msecs_to_jiffies(100));
> +}
> +
>  static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp,
>  				    struct edp_vsc_psr *vsc_psr)
>  {
> @@ -1443,6 +1487,10 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp)
>  	I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP |
>  		   EDP_PSR_DEBUG_MASK_HPD);
>  
> +	INIT_DELAYED_WORK(&intel_dp->edp_psr_delayed_normal_work,
> +			  intel_edp_psr_delayed_normal_work);
> +	mutex_init(&intel_dp->psr_exit_mutex);
> +
>  	intel_dp->psr_setup_done = true;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0f52362..e47f3f3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -499,6 +499,8 @@ struct intel_dp {
>  	int backlight_off_delay;
>  	struct delayed_work panel_vdd_work;
>  	bool want_panel_vdd;
> +	struct delayed_work edp_psr_delayed_normal_work;
> +	struct mutex psr_exit_mutex;
>  	bool psr_setup_done;
>  	struct intel_connector *attached_connector;
>  };
> @@ -839,5 +841,6 @@ extern void intel_edp_psr_enable(struct intel_dp *intel_dp);
>  extern void intel_edp_psr_disable(struct intel_dp *intel_dp);
>  extern void intel_edp_psr_update(struct drm_device *dev);
>  extern bool intel_edp_is_psr_enabled(struct drm_device *dev);
> +extern void intel_edp_psr_force_exit(struct drm_device *dev);
>  
>  #endif /* __INTEL_DRV_H__ */
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch