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

["Jindal, Sonika" <sonika.jindal@intel.com>]

On 3/26/2015 5:09 PM, R, Durgadoss wrote:
>> -----Original Message-----
>> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Sonika Jindal
>> Sent: Thursday, March 26, 2015 1:57 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Vivi, Rodrigo
>> Subject: [Intel-gfx] [PATCH] drm/i915/skl: Enabling PSR2 SU with frame sync
>>
>> 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.
>>
>> v2: Add 3200x2000 resolution restriction with PSR2, move psr2_support to i915_psr
>> struct, add aux_frame_sync to independently control aux frame sync, rename the
>> TP2 TIME macro for 2500us (Rodrigo, Siva)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h  |    2 ++
>> drivers/gpu/drm/i915/i915_reg.h  |   14 ++++++++++++++
>> drivers/gpu/drm/i915/intel_dp.c  |   15 +++++++++++++++
>> drivers/gpu/drm/i915/intel_psr.c |   34 +++++++++++++++++++++++++++++++++-
>> 4 files changed, 64 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index eb38cd1..65d3dae 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -879,6 +879,8 @@ struct i915_psr {
>> 	struct delayed_work work;
>> 	unsigned busy_frontbuffer_bits;
>> 	bool link_standby;
>> +	bool psr2_support;
>> +	bool aux_frame_sync;
>> };
>>
>> enum intel_pch {
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 5b84ee6..0b42ef3 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_2500	(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..b0ae45e 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -3786,6 +3786,21 @@ 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);
>> +			dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
>> +			/* PSR2 needs frame sync as well */
>> +			dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
>> +			DRM_DEBUG_KMS("PSR2 %s on sink",
>> +				dev_priv->psr.psr2_support ? "supported" : "not supported");
>> +		}
>> 	}
>>
>> 	/* Training Pattern 3 support, both source and sink */
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>> index b9f40c2..d52623d 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 (dev_priv->psr.aux_frame_sync)
>> +		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,
>> @@ -232,6 +253,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>> 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> 	struct drm_device *dev = dig_port->base.base.dev;
>> 	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
>> +
>> 	uint32_t max_sleep_time = 0x1f;
>> 	/* Lately it was identified that depending on panel idle frame count
>> 	 * calculated at HW can be off by 1. So let's use what came
>> @@ -255,6 +278,12 @@ 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);
>> +
>> +	/* PSR2 is restricted to work with panel resolutions upto 3200x2000 */
>> +	if (dev_priv->psr.psr2_support && crtc->config->pipe_src_w <= 3200
>> +		&& crtc->config->pipe_src_h <= 2000)
>> +		I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE |
>> +				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
>
> Do we need this size check every time _enable() is called ?
> Or is it sufficient to do it once ?
Yes, it is sufficient to check it once after the pipe is enabled. I will 
move it to intel_psr_enable which is called once, and send out a patch 
in a while.
>
> Also, if this size restriction is  not specific to HSW, isn't match_conditions()
> a better place for this check ?
>
It is specific to psr2. In match_conditions we mainly check if psr 
support is possible. I think this check can remain in intel_psr_enable.

> Thanks,
> Durga
>
>> }
>>
>> static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
>> @@ -364,6 +393,9 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>> 	if (HAS_DDI(dev)) {
>> 		hsw_psr_setup_vsc(intel_dp);
>>
>> +		if (dev_priv->psr.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);
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> 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