Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jan 22, 2015 at 02:30:54PM +0530, Sonika Jindal wrote:
> Mainly taking care of some register offsets, otherwise things are similar to
> hsw. Also, programming ddi aux to use hardcoded values for psr data select.
> 
> v2: introduce  EDP_PSR_AUX_BASE macro (Chris)
> v3: Moving to HW tracking for SKL+ platforms, so activating source psr during
> psr_enabling and then avoiding psr entries and exits for each frontbuffer
> updates.
> v4: Using SKL DDI AUX regs instead of changing PSR_AUX regs definition (Rodrigo)
> 
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          |    3 ++-
>  drivers/gpu/drm/i915/i915_reg.h          |    5 +++++
>  drivers/gpu/drm/i915/intel_frontbuffer.c |    7 +++++--
>  drivers/gpu/drm/i915/intel_psr.c         |   26 ++++++++++++++++++++++++--
>  4 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 66f0c60..3d24872 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2371,7 +2371,8 @@ struct drm_i915_cmd_table {
>  #define HAS_DDI(dev)		(INTEL_INFO(dev)->has_ddi)
>  #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
>  #define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev) || \
> -				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
> +				 IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
> +				 IS_SKYLAKE(dev))
>  #define HAS_RUNTIME_PM(dev)	(IS_GEN6(dev) || IS_HASWELL(dev) || \
>  				 IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
>  #define HAS_RC6(dev)		(INTEL_INFO(dev)->gen >= 6)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a39bb03..a6f06fa 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3748,6 +3748,11 @@ enum punit_power_well {
>  #define   DP_AUX_CH_CTL_PRECHARGE_TEST	    (1 << 11)
>  #define   DP_AUX_CH_CTL_BIT_CLOCK_2X_MASK    (0x7ff)
>  #define   DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT   0
> +#define   DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL	(1 << 14)
> +#define   DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL	(1 << 13)
> +#define   DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL	(1 << 12)
> +#define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (1f << 5)
> +#define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
>  #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index 79f6d72..010d550 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -156,7 +156,9 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
>  
>  	intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
>  
> -	intel_psr_invalidate(dev, obj->frontbuffer_bits);
> +
> +	if (INTEL_INFO(dev)->gen < 9)
> +		intel_psr_invalidate(dev, obj->frontbuffer_bits);
>  }
>  
>  /**
> @@ -182,7 +184,8 @@ void intel_frontbuffer_flush(struct drm_device *dev,
>  
>  	intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
>  
> -	intel_psr_flush(dev, frontbuffer_bits);
> +	if (INTEL_INFO(dev)->gen < 9)
> +		intel_psr_flush(dev, frontbuffer_bits);

Again no, not going to take wholesale filtering of the sw invalidate
paths. This needs to be properly tested and pushed down into the psr
specific invalidate/flush functions on a per-function basis.

I've dropped these two hunks and merged the patch.
-Daniel

>  
>  	/*
>  	 * FIXME: Unconditional fbc flushing here is a rather gross hack and
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index dd0e6e0..4867d5a 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -142,6 +142,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>  	struct drm_device *dev = dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint32_t aux_clock_divider;
> +	uint32_t aux_data_reg, aux_ctl_reg;
>  	int precharge = 0x3;
>  	bool only_standby = dev_priv->vbt.psr.full_link;
>  	static const uint8_t aux_msg[] = {
> @@ -168,16 +169,34 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>  		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>  				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
>  
> +	aux_data_reg = (INTEL_INFO(dev)->gen >= 9) ?
> +				DPA_AUX_CH_DATA1 : EDP_PSR_AUX_DATA1(dev);
> +	aux_ctl_reg = (INTEL_INFO(dev)->gen >= 9) ?
> +				DPA_AUX_CH_CTL : EDP_PSR_AUX_CTL(dev);
> +
>  	/* Setup AUX registers */
>  	for (i = 0; i < sizeof(aux_msg); i += 4)
> -		I915_WRITE(EDP_PSR_AUX_DATA1(dev) + i,
> +		I915_WRITE(aux_data_reg + i,
>  			   intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
>  
> -	I915_WRITE(EDP_PSR_AUX_CTL(dev),
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		uint32_t val;
> +
> +		val = I915_READ(aux_ctl_reg);
> +		val &= ~DP_AUX_CH_CTL_TIME_OUT_MASK;
> +		val |= DP_AUX_CH_CTL_TIME_OUT_1600us;
> +		val &= ~DP_AUX_CH_CTL_MESSAGE_SIZE_MASK;
> +		val |= (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);
> +		/* Use hardcoded data values for PSR */
> +		val &= ~DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL;
> +		I915_WRITE(aux_ctl_reg, val);
> +	} else {
> +		I915_WRITE(aux_ctl_reg,
>  		   DP_AUX_CH_CTL_TIME_OUT_400us |
>  		   (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
>  		   (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
>  		   (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
> +	}
>  }
>  
>  static void vlv_psr_enable_source(struct intel_dp *intel_dp)
> @@ -355,6 +374,9 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>  
>  		/* Enable PSR on the panel */
>  		hsw_psr_enable_sink(intel_dp);
> +
> +		if (INTEL_INFO(dev)->gen >= 9)
> +			intel_psr_activate(intel_dp);
>  	} else {
>  		vlv_psr_setup_vsc(intel_dp);
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx