Re: [PATCH] drm/i915/skl: Enabling PSR2 SU with frame sync

["Vivi, Rodrigo" <rodrigo.vivi@intel.com>]

On Mon, 2015-03-23 at 10:14 +0100, Daniel Vetter wrote:
> On Fri, Mar 20, 2015 at 11:26:19PM +0000, Vivi, Rodrigo wrote:
> > Hi Sonika,
> > 
> > on the previous email I forgot to tell that we also need to add
> > somewhere a check to avoid PSR for resolutions bigger than 3200x2000.
> > 
> > Although most of displays that we have that support psr are 3200x1800 it
> > is better to think a way to protect now than later.
> 
> Is that a restriction just for PSR2 (because the hw doesn't have more
> space for line checksums) or also for the existing PSR code? If it's just
> for PSR2 we can easily extend the check Sonika is adding in this patch to
> also check for that and it's all done.

Spec just list this limitation on PSR2.

> -Daniel
> 
> > 
> > Thanks,
> > Rodrigo.
> > 
> > On Fri, 2015-03-20 at 11:27 +0530, Sonika Jindal wrote:
> > > We make use of HW tracking for Selective update region and enable frame sync on
> > > sink. We use hardware's hardcoded data values for frame sync and GTC.
> > > 
> > > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h  |   14 ++++++++++++++
> > >  drivers/gpu/drm/i915/intel_dp.c  |   16 ++++++++++++++++
> > >  drivers/gpu/drm/i915/intel_drv.h |    2 ++
> > >  drivers/gpu/drm/i915/intel_psr.c |   30 +++++++++++++++++++++++++++++-
> > >  4 files changed, 61 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 5b84ee6..7e4f6f0 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -2745,6 +2745,20 @@ enum skl_disp_power_wells {
> > >  #define   EDP_PSR_DEBUG_MASK_MEMUP	(1<<26)
> > >  #define   EDP_PSR_DEBUG_MASK_HPD	(1<<25)
> > >  
> > > +#define EDP_PSR2_CTL			0x6f900
> > > +#define   EDP_PSR2_ENABLE		(1<<31)
> > > +#define   EDP_SU_TRACK_ENABLE		(1<<30)
> > > +#define   EDP_MAX_SU_DISABLE_TIME(t)	((t)<<20)
> > > +#define   EDP_MAX_SU_DISABLE_TIME_MASK	(0x1f<<20)
> > > +#define   EDP_PSR2_TP2_TIME_500		(0<<8)
> > > +#define   EDP_PSR2_TP2_TIME_100		(1<<8)
> > > +#define   EDP_PSR2_TP2_TIME_250		(2<<8)
> > > +#define   EDP_PSR2_TP2_TIME_50		(3<<8)
> > > +#define   EDP_PSR2_TP2_TIME_MASK	(3<<8)
> > > +#define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
> > > +#define   EDP_PSR2_FRAME_BEFORE_SU_MASK	(0xf<<4)
> > > +#define   EDP_PSR2_IDLE_MASK		0xf
> > > +
> > >  /* VGA port control */
> > >  #define ADPA			0x61100
> > >  #define PCH_ADPA                0xe1100
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 3967af1..dedd8ad 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -3786,6 +3786,22 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> > >  			dev_priv->psr.sink_support = true;
> > >  			DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
> > >  		}
> > > +
> > > +		if (INTEL_INFO(dev)->gen >= 9 &&
> > > +			(intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
> > > +			uint8_t frame_sync_cap;
> > > +
> > > +			dev_priv->psr.sink_support = true;
> > > +			intel_dp_dpcd_read_wake(&intel_dp->aux,
> > > +					DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> > > +					&frame_sync_cap, 1);
> > > +			/* PSR2 needs frame sync as well */
> > > +			if (frame_sync_cap) {
> > > +				DRM_DEBUG_KMS("PSR2 supported on sink");
> > > +				intel_dp->psr2_support = true;
> > > +			} else
> > > +				intel_dp->psr2_support = false;
> > > +		}
> > >  	}
> > >  
> > >  	/* Training Pattern 3 support, both source and sink */
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 8bb18e5..ed1b0a5 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -663,6 +663,8 @@ struct intel_dp {
> > >  	struct intel_dp_mst_encoder *mst_encoders[I915_MAX_PIPES];
> > >  	struct drm_dp_mst_topology_mgr mst_mgr;
> > >  
> > > +	bool psr2_support;
> > > +
> > >  	uint32_t (*get_aux_clock_divider)(struct intel_dp *dp, int index);
> > >  	/*
> > >  	 * This function returns the value we have to program the AUX_CTL
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > index b9f40c2..99dbc73 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -117,6 +117,19 @@ static void vlv_psr_setup_vsc(struct intel_dp *intel_dp)
> > >  	I915_WRITE(VLV_VSCSDP(pipe), val);
> > >  }
> > >  
> > > +static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp)
> > > +{
> > > +	struct edp_vsc_psr psr_vsc;
> > > +
> > > +	/* Prepare VSC Header for SU as per EDP 1.4 spec, Table 6.11 */
> > > +	memset(&psr_vsc, 0, sizeof(psr_vsc));
> > > +	psr_vsc.sdp_header.HB0 = 0;
> > > +	psr_vsc.sdp_header.HB1 = 0x7;
> > > +	psr_vsc.sdp_header.HB2 = 0x3;
> > > +	psr_vsc.sdp_header.HB3 = 0xb;
> > > +	intel_psr_write_vsc(intel_dp, &psr_vsc);
> > > +}
> > > +
> > >  static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
> > >  {
> > >  	struct edp_vsc_psr psr_vsc;
> > > @@ -165,6 +178,12 @@ 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);
> > >  
> > > +	/* Enable AUX frame sync at sink */
> > > +	if (intel_dp->psr2_support)
> > > +		drm_dp_dpcd_writeb(&intel_dp->aux,
> > > +				DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
> > > +				DP_AUX_FRAME_SYNC_ENABLE);
> > > +
> > >  	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) ?
> > > @@ -183,8 +202,10 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
> > >  		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 */
> > > +		/* Use hardcoded data values for PSR, frame sync and GTC */
> > >  		val &= ~DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL;
> > > +		val &= ~DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL;
> > > +		val &= ~DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL;
> > >  		I915_WRITE(aux_ctl_reg, val);
> > >  	} else {
> > >  		I915_WRITE(aux_ctl_reg,
> > > @@ -255,6 +276,10 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
> > >  		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
> > >  		   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
> > >  		   EDP_PSR_ENABLE);
> > > +
> > > +	if (intel_dp->psr2_support)
> 
> Well here I mean.

Agree.

> 
> Cheers, Daniel
> 
> > > +		I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE |
> > > +				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
> > >  }
> > >  
> > >  static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
> > > @@ -364,6 +389,9 @@ void intel_psr_enable(struct intel_dp *intel_dp)
> > >  	if (HAS_DDI(dev)) {
> > >  		hsw_psr_setup_vsc(intel_dp);
> > >  
> > > +		if (intel_dp->psr2_support)
> > > +			skl_psr_setup_su_vsc(intel_dp);
> > > +
> > >  		/* Avoid continuous PSR exit by masking memup and hpd */
> > >  		I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP |
> > >  			   EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx