All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/skl: Enabling PSR on Skylake
@ 2015-01-22  9:00 Sonika Jindal
  2015-01-22 17:48 ` shuang.he
  2015-01-28 16:02 ` Daniel Vetter
  0 siblings, 2 replies; 16+ messages in thread
From: Sonika Jindal @ 2015-01-22  9:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: rodrigo.vivi

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);
 
 	/*
 	 * 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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-22  9:00 [PATCH] drm/i915/skl: Enabling PSR on Skylake Sonika Jindal
@ 2015-01-22 17:48 ` shuang.he
  2015-01-28 16:02 ` Daniel Vetter
  1 sibling, 0 replies; 16+ messages in thread
From: shuang.he @ 2015-01-22 17:48 UTC (permalink / raw)
  To: shuang.he, ethan.gao, intel-gfx, sonika.jindal

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5623
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              353/353              352/353
ILK                                  353/353              353/353
SNB                                  400/422              400/422
IVB                                  487/487              487/487
BYT                                  296/296              296/296
HSW                                  508/508              508/508
BDW                                  401/402              401/402
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gen3_render_linear_blits      PASS(3, M25M23)      CRASH(1, M23)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-22  9:00 [PATCH] drm/i915/skl: Enabling PSR on Skylake Sonika Jindal
  2015-01-22 17:48 ` shuang.he
@ 2015-01-28 16:02 ` Daniel Vetter
  2015-01-29  3:57   ` sonika
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2015-01-28 16:02 UTC (permalink / raw)
  To: Sonika Jindal; +Cc: intel-gfx, rodrigo.vivi

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-28 16:02 ` Daniel Vetter
@ 2015-01-29  3:57   ` sonika
  2015-01-29 16:11     ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: sonika @ 2015-01-29  3:57 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, rodrigo.vivi


On Wednesday 28 January 2015 09:32 PM, Daniel Vetter wrote:
> 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
Hi Daniel,
Even SW tracking doesn't work in many cases, like I reported earlier in 
ubuntu login screen where we don't get frontbuffer flushes and we don't 
enter PSR at all with SW tracking.
I see similar behavior even in fbcon mode. So, I am not sure how you can 
say that SW tracking is the only right way.
If there are cases where HW tracking fails (and I know a few), we need 
to fix them. I can move this gen check to the intel_psr_* function if 
that is the major concern.
-Sonika
>>   
>>   	/*
>>   	 * 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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-29  3:57   ` sonika
@ 2015-01-29 16:11     ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2015-01-29 16:11 UTC (permalink / raw)
  To: sonika; +Cc: intel-gfx, rodrigo.vivi

On Thu, Jan 29, 2015 at 09:27:14AM +0530, sonika wrote:
> 
> On Wednesday 28 January 2015 09:32 PM, Daniel Vetter wrote:
> >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
> Hi Daniel,
> Even SW tracking doesn't work in many cases, like I reported earlier in
> ubuntu login screen where we don't get frontbuffer flushes and we don't
> enter PSR at all with SW tracking.
> I see similar behavior even in fbcon mode. So, I am not sure how you can say
> that SW tracking is the only right way.

In some cases the sw tracking isn't especially accurate (cpu based
frontbuffer rendering to the gtt). Paulo has seen a similar issue with
fbc, and since hw tracking works correctly for that case his patches
filter that source of sw invalidates out. Paulo should be back from his
vacation next week, so please ping him when he's back.

> If there are cases where HW tracking fails (and I know a few), we need to
> fix them. I can move this gen check to the intel_psr_* function if that is
> the major concern.

Yeah, the check should be pushed down imo, in the core sw tracking code
it's a bit in the wrong layer.
-Daniel
-- 
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-21  8:55             ` sonika
@ 2015-01-21  9:42               ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2015-01-21  9:42 UTC (permalink / raw)
  To: sonika; +Cc: intel-gfx, Vivi, Rodrigo

On Wed, Jan 21, 2015 at 02:25:23PM +0530, sonika wrote:
> 
> On Wednesday 21 January 2015 02:07 PM, Daniel Vetter wrote:
> >On Tue, Jan 20, 2015 at 02:01:25PM -0800, Rodrigo Vivi wrote:
> >>Hi Sonika, for the login screen my guess is that blinking cursor
> >>waiting for password blocks psr entry.
> >>
> >>For test cases I started the test case enhancement but my psr work is
> >>paused this month for bug maintainance. Any help is welcome.
> >>I was planing also to take a look on new Paulo's fbc tests cases to
> >>see if we could use some pieces on PSR validation. might be worth to
> >>take a look.
> >>
> >>But anyway, for HW tracking I would only consider when I use a KDE for
> >>a while without missing any screen updates and seeing perf counter
> >>increasing. But if you tell this is what happening I trust you and we
> >>can add support for that since it is disabled by default and validate
> >>that carefully also on SKL when trying to enable it by default
> >>everywhere.
> >I'm definitely leaning the other way: Before we take out the sw tracking
> >for some platforms I want the full psr validation to be done (since
> >otherwise no one will do it again and for skl+1 we have the same hilarious
> >saga again). And if we figure out that the hw tracking has indeed become
> >better and is now useful, then we can do the same thing like Paulo's fbc
> >patches and ignore some sw invalidate events selectively. But only when
> >the tests are solid, we're sure we have full coverage and that the hw
> >really works for that use-case.
> >-Daniel
> Ok, I like the idea of selectively ignoring sw invalidates. And then PSR2's
> HW tracking will still hold good
> in those cases. I think we can have a module parameter (hw_tracking) instead
> of checking for platform
> everywhere.

I don't think a hw_tracking variable is all that useful, since I expect
every platform to work slightly different. If you look at the psr code we
already have some platform checks for specific issues (e.g. sprite flips
don't invalidate on hsw but do on bdw). Imo much better to just have
platform base checks which filter out specific sw-invalidate sources.

Please also look at Paulo's fbc series which adds more information to the
invalidate callbacks so that we can filter more precisely.
-Daniel
-- 
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-21  8:37           ` Daniel Vetter
@ 2015-01-21  8:55             ` sonika
  2015-01-21  9:42               ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: sonika @ 2015-01-21  8:55 UTC (permalink / raw)
  To: Daniel Vetter, Rodrigo Vivi; +Cc: intel-gfx, Vivi, Rodrigo


On Wednesday 21 January 2015 02:07 PM, Daniel Vetter wrote:
> On Tue, Jan 20, 2015 at 02:01:25PM -0800, Rodrigo Vivi wrote:
>> Hi Sonika, for the login screen my guess is that blinking cursor
>> waiting for password blocks psr entry.
>>
>> For test cases I started the test case enhancement but my psr work is
>> paused this month for bug maintainance. Any help is welcome.
>> I was planing also to take a look on new Paulo's fbc tests cases to
>> see if we could use some pieces on PSR validation. might be worth to
>> take a look.
>>
>> But anyway, for HW tracking I would only consider when I use a KDE for
>> a while without missing any screen updates and seeing perf counter
>> increasing. But if you tell this is what happening I trust you and we
>> can add support for that since it is disabled by default and validate
>> that carefully also on SKL when trying to enable it by default
>> everywhere.
> I'm definitely leaning the other way: Before we take out the sw tracking
> for some platforms I want the full psr validation to be done (since
> otherwise no one will do it again and for skl+1 we have the same hilarious
> saga again). And if we figure out that the hw tracking has indeed become
> better and is now useful, then we can do the same thing like Paulo's fbc
> patches and ignore some sw invalidate events selectively. But only when
> the tests are solid, we're sure we have full coverage and that the hw
> really works for that use-case.
> -Daniel
Ok, I like the idea of selectively ignoring sw invalidates. And then 
PSR2's HW tracking will still hold good
in those cases. I think we can have a module parameter (hw_tracking) 
instead of checking for platform
everywhere.
Please let me know if it sounds better.
-Sonika


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-20 22:01         ` Rodrigo Vivi
  2015-01-21  4:53           ` sonika
@ 2015-01-21  8:37           ` Daniel Vetter
  2015-01-21  8:55             ` sonika
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2015-01-21  8:37 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Vivi, Rodrigo

On Tue, Jan 20, 2015 at 02:01:25PM -0800, Rodrigo Vivi wrote:
> Hi Sonika, for the login screen my guess is that blinking cursor
> waiting for password blocks psr entry.
> 
> For test cases I started the test case enhancement but my psr work is
> paused this month for bug maintainance. Any help is welcome.
> I was planing also to take a look on new Paulo's fbc tests cases to
> see if we could use some pieces on PSR validation. might be worth to
> take a look.
> 
> But anyway, for HW tracking I would only consider when I use a KDE for
> a while without missing any screen updates and seeing perf counter
> increasing. But if you tell this is what happening I trust you and we
> can add support for that since it is disabled by default and validate
> that carefully also on SKL when trying to enable it by default
> everywhere.

I'm definitely leaning the other way: Before we take out the sw tracking
for some platforms I want the full psr validation to be done (since
otherwise no one will do it again and for skl+1 we have the same hilarious
saga again). And if we figure out that the hw tracking has indeed become
better and is now useful, then we can do the same thing like Paulo's fbc
patches and ignore some sw invalidate events selectively. But only when
the tests are solid, we're sure we have full coverage and that the hw
really works for that use-case.
-Daniel
-- 
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-20 22:01         ` Rodrigo Vivi
@ 2015-01-21  4:53           ` sonika
  2015-01-21  8:37           ` Daniel Vetter
  1 sibling, 0 replies; 16+ messages in thread
From: sonika @ 2015-01-21  4:53 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Vivi, Rodrigo


On Wednesday 21 January 2015 03:31 AM, Rodrigo Vivi wrote:
> Hi Sonika, for the login screen my guess is that blinking cursor
> waiting for password blocks psr entry.
But I thought it is the psr exit which is taking time which is why it 
takes time
to actually show up the characters
> For test cases I started the test case enhancement but my psr work is
> paused this month for bug maintainance. Any help is welcome.
> I was planing also to take a look on new Paulo's fbc tests cases to
> see if we could use some pieces on PSR validation. might be worth to
> take a look.
Can we test scenarios like delayed psr exits (like the one I mentioned) 
with test case?
If yes, I can work with you offline to enhance the igt.
> But anyway, for HW tracking I would only consider when I use a KDE for
> a while without missing any screen updates and seeing perf counter
> increasing. But if you tell this is what happening I trust you and we
> can add support for that since it is disabled by default and validate
> that carefully also on SKL when trying to enable it by default
> everywhere.
Hmm, I have not tested with KDE yet. I have been testing with ubuntu 
with gnome.
And it works very smooth there. I will do more experiments with KDE too.
>
> I'm going to review the code more carefully later...
>
> On Tue, Jan 20, 2015 at 3:49 AM, Jindal, Sonika <sonika.jindal@intel.com> wrote:
>> Is somebody working on enhancing the psr testcase?
>> -Sonika
>>
>> -----Original Message-----
>> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
>> Sent: Tuesday, January 20, 2015 3:20 PM
>> To: Jindal, Sonika
>> Cc: Daniel Vetter; intel-gfx; Vivi, Rodrigo
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Enabling PSR on Skylake
>>
>> On Mon, Jan 19, 2015 at 05:10:58PM +0530, sonika wrote:
>>> On Saturday 17 January 2015 09:54 AM, Daniel Vetter wrote:
>>>> On Fri, Jan 16, 2015 at 02:07:26PM +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.
>>>>>
>>>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/i915_drv.h          |    3 ++-
>>>>>   drivers/gpu/drm/i915/i915_reg.h          |   19 +++++++++++++------
>>>>>   drivers/gpu/drm/i915/intel_frontbuffer.c |    7 +++++--
>>>>>   drivers/gpu/drm/i915/intel_psr.c         |   18 +++++++++++++++++-
>>>>>   4 files changed, 37 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>>>> b/drivers/gpu/drm/i915/i915_drv.h index 42c69ca..ee5cd3b 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>> @@ -2355,7 +2355,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 a828cf5..068c8da 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>>> @@ -2619,12 +2619,14 @@ enum skl_disp_power_wells {
>>>>>   #define   EDP_PSR_TP1_TIME_0us (3<<4)
>>>>>   #define   EDP_PSR_IDLE_FRAME_SHIFT 0
>>>>>
>>>>> -#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_BASE(dev) + 0x10) -#define
>>>>> EDP_PSR_AUX_DATA1(dev) (EDP_PSR_BASE(dev) + 0x14) -#define
>>>>> EDP_PSR_AUX_DATA2(dev) (EDP_PSR_BASE(dev) + 0x18) -#define
>>>>> EDP_PSR_AUX_DATA3(dev) (EDP_PSR_BASE(dev) + 0x1c) -#define
>>>>> EDP_PSR_AUX_DATA4(dev) (EDP_PSR_BASE(dev) + 0x20) -#define
>>>>> EDP_PSR_AUX_DATA5(dev) (EDP_PSR_BASE(dev) + 0x24)
>>>>> +#define EDP_PSR_AUX_BASE(dev) (INTEL_INFO(dev)->gen >= 9 ? \
>>>>> + 0x64000 : EDP_PSR_BASE(dev))
>>>>> +#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_AUX_BASE(dev) + 0x10) #define
>>>>> +EDP_PSR_AUX_DATA1(dev) (EDP_PSR_AUX_BASE(dev) + 0x14) #define
>>>>> +EDP_PSR_AUX_DATA2(dev) (EDP_PSR_AUX_BASE(dev) + 0x18) #define
>>>>> +EDP_PSR_AUX_DATA3(dev) (EDP_PSR_AUX_BASE(dev) + 0x1c) #define
>>>>> +EDP_PSR_AUX_DATA4(dev) (EDP_PSR_AUX_BASE(dev) + 0x20) #define
>>>>> +EDP_PSR_AUX_DATA5(dev) (EDP_PSR_AUX_BASE(dev) + 0x24)
>>>>>
>>>>>   #define EDP_PSR_STATUS_CTL(dev) (EDP_PSR_BASE(dev) + 0x40)
>>>>>   #define   EDP_PSR_STATUS_STATE_MASK (7<<29)
>>>>> @@ -3771,6 +3773,11 @@ enum skl_disp_power_wells {
>>>>>   #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);
>>>> I'm pretty sure the hw isn't good enough yet to detect everything,
>>>> unfortunately the testcase is also not quite ready yet. In any case
>>>> these changes should be moved into the inel_psr_* functions and in a
>>>> separate patch.
>>> When ubuntu boots up and stops at the login screen, the text I enter
>>> comes slowly, looks like psr exit and entry is not very prompt at that time.
>>> But when I try with SW tracking, I see that PSR doesn't get enabled at
>>> all at that point!
>>> Also, I don't see calls to frontbuffer invalidate and flush at that point.
>>> So, with SW tracking too we will have cases where we don't enter psr.
>>> Any inputs on why frontbuffer calls are not received during login screen?
>>>
>>> Please note that after login, it works fine if I type something on the
>>> terminal or do some other things.
>> I have no idea why or what exactly the ubuntu screen does in it's login function. Just please note that there are _lots_ more desktop environments on linux than just ubunut's flavour of gnome, so we really need to have that psr testcase working (which covers them all) instead of manual testing.
>> -Daniel
>>
>>>> Also someone needs to re-review the psr igt testcase to make sure it
>>>> covers everything. Ville could do that too since he's done the fbc
>>>> testcase.
>>>> -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..6d2cdb8 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_psr.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>>>>> @@ -173,11 +173,24 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>>>>>    I915_WRITE(EDP_PSR_AUX_DATA1(dev) + 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(EDP_PSR_AUX_CTL(dev)); 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(EDP_PSR_AUX_CTL(dev), val); } else {
>>>>> + I915_WRITE(EDP_PSR_AUX_CTL(dev),
>>>>>      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 +368,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
>
>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-20 11:49       ` Jindal, Sonika
@ 2015-01-20 22:01         ` Rodrigo Vivi
  2015-01-21  4:53           ` sonika
  2015-01-21  8:37           ` Daniel Vetter
  0 siblings, 2 replies; 16+ messages in thread
From: Rodrigo Vivi @ 2015-01-20 22:01 UTC (permalink / raw)
  To: Jindal, Sonika; +Cc: intel-gfx, Vivi, Rodrigo

Hi Sonika, for the login screen my guess is that blinking cursor
waiting for password blocks psr entry.

For test cases I started the test case enhancement but my psr work is
paused this month for bug maintainance. Any help is welcome.
I was planing also to take a look on new Paulo's fbc tests cases to
see if we could use some pieces on PSR validation. might be worth to
take a look.

But anyway, for HW tracking I would only consider when I use a KDE for
a while without missing any screen updates and seeing perf counter
increasing. But if you tell this is what happening I trust you and we
can add support for that since it is disabled by default and validate
that carefully also on SKL when trying to enable it by default
everywhere.

I'm going to review the code more carefully later...

On Tue, Jan 20, 2015 at 3:49 AM, Jindal, Sonika <sonika.jindal@intel.com> wrote:
> Is somebody working on enhancing the psr testcase?
> -Sonika
>
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Tuesday, January 20, 2015 3:20 PM
> To: Jindal, Sonika
> Cc: Daniel Vetter; intel-gfx; Vivi, Rodrigo
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Enabling PSR on Skylake
>
> On Mon, Jan 19, 2015 at 05:10:58PM +0530, sonika wrote:
>>
>> On Saturday 17 January 2015 09:54 AM, Daniel Vetter wrote:
>> >On Fri, Jan 16, 2015 at 02:07:26PM +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.
>> >>
>> >>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> >>---
>> >>  drivers/gpu/drm/i915/i915_drv.h          |    3 ++-
>> >>  drivers/gpu/drm/i915/i915_reg.h          |   19 +++++++++++++------
>> >>  drivers/gpu/drm/i915/intel_frontbuffer.c |    7 +++++--
>> >>  drivers/gpu/drm/i915/intel_psr.c         |   18 +++++++++++++++++-
>> >>  4 files changed, 37 insertions(+), 10 deletions(-)
>> >>
>> >>diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> >>b/drivers/gpu/drm/i915/i915_drv.h index 42c69ca..ee5cd3b 100644
>> >>--- a/drivers/gpu/drm/i915/i915_drv.h
>> >>+++ b/drivers/gpu/drm/i915/i915_drv.h
>> >>@@ -2355,7 +2355,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 a828cf5..068c8da 100644
>> >>--- a/drivers/gpu/drm/i915/i915_reg.h
>> >>+++ b/drivers/gpu/drm/i915/i915_reg.h
>> >>@@ -2619,12 +2619,14 @@ enum skl_disp_power_wells {
>> >>  #define   EDP_PSR_TP1_TIME_0us (3<<4)
>> >>  #define   EDP_PSR_IDLE_FRAME_SHIFT 0
>> >>
>> >>-#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_BASE(dev) + 0x10) -#define
>> >>EDP_PSR_AUX_DATA1(dev) (EDP_PSR_BASE(dev) + 0x14) -#define
>> >>EDP_PSR_AUX_DATA2(dev) (EDP_PSR_BASE(dev) + 0x18) -#define
>> >>EDP_PSR_AUX_DATA3(dev) (EDP_PSR_BASE(dev) + 0x1c) -#define
>> >>EDP_PSR_AUX_DATA4(dev) (EDP_PSR_BASE(dev) + 0x20) -#define
>> >>EDP_PSR_AUX_DATA5(dev) (EDP_PSR_BASE(dev) + 0x24)
>> >>+#define EDP_PSR_AUX_BASE(dev) (INTEL_INFO(dev)->gen >= 9 ? \
>> >>+ 0x64000 : EDP_PSR_BASE(dev))
>> >>+#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_AUX_BASE(dev) + 0x10) #define
>> >>+EDP_PSR_AUX_DATA1(dev) (EDP_PSR_AUX_BASE(dev) + 0x14) #define
>> >>+EDP_PSR_AUX_DATA2(dev) (EDP_PSR_AUX_BASE(dev) + 0x18) #define
>> >>+EDP_PSR_AUX_DATA3(dev) (EDP_PSR_AUX_BASE(dev) + 0x1c) #define
>> >>+EDP_PSR_AUX_DATA4(dev) (EDP_PSR_AUX_BASE(dev) + 0x20) #define
>> >>+EDP_PSR_AUX_DATA5(dev) (EDP_PSR_AUX_BASE(dev) + 0x24)
>> >>
>> >>  #define EDP_PSR_STATUS_CTL(dev) (EDP_PSR_BASE(dev) + 0x40)
>> >>  #define   EDP_PSR_STATUS_STATE_MASK (7<<29)
>> >>@@ -3771,6 +3773,11 @@ enum skl_disp_power_wells {
>> >>  #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);
>> >I'm pretty sure the hw isn't good enough yet to detect everything,
>> >unfortunately the testcase is also not quite ready yet. In any case
>> >these changes should be moved into the inel_psr_* functions and in a
>> >separate patch.
>> When ubuntu boots up and stops at the login screen, the text I enter
>> comes slowly, looks like psr exit and entry is not very prompt at that time.
>> But when I try with SW tracking, I see that PSR doesn't get enabled at
>> all at that point!
>> Also, I don't see calls to frontbuffer invalidate and flush at that point.
>> So, with SW tracking too we will have cases where we don't enter psr.
>> Any inputs on why frontbuffer calls are not received during login screen?
>>
>> Please note that after login, it works fine if I type something on the
>> terminal or do some other things.
>
> I have no idea why or what exactly the ubuntu screen does in it's login function. Just please note that there are _lots_ more desktop environments on linux than just ubunut's flavour of gnome, so we really need to have that psr testcase working (which covers them all) instead of manual testing.
> -Daniel
>
>> >Also someone needs to re-review the psr igt testcase to make sure it
>> >covers everything. Ville could do that too since he's done the fbc
>> >testcase.
>> >-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..6d2cdb8 100644
>> >>--- a/drivers/gpu/drm/i915/intel_psr.c
>> >>+++ b/drivers/gpu/drm/i915/intel_psr.c
>> >>@@ -173,11 +173,24 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>> >>   I915_WRITE(EDP_PSR_AUX_DATA1(dev) + 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(EDP_PSR_AUX_CTL(dev)); 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(EDP_PSR_AUX_CTL(dev), val); } else {
>> >>+ I915_WRITE(EDP_PSR_AUX_CTL(dev),
>> >>     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 +368,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



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-20  9:50     ` Daniel Vetter
@ 2015-01-20 11:49       ` Jindal, Sonika
  2015-01-20 22:01         ` Rodrigo Vivi
  0 siblings, 1 reply; 16+ messages in thread
From: Jindal, Sonika @ 2015-01-20 11:49 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, Vivi, Rodrigo

Is somebody working on enhancing the psr testcase?
-Sonika

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Tuesday, January 20, 2015 3:20 PM
To: Jindal, Sonika
Cc: Daniel Vetter; intel-gfx; Vivi, Rodrigo
Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Enabling PSR on Skylake

On Mon, Jan 19, 2015 at 05:10:58PM +0530, sonika wrote:
> 
> On Saturday 17 January 2015 09:54 AM, Daniel Vetter wrote:
> >On Fri, Jan 16, 2015 at 02:07:26PM +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.
> >>
> >>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/i915_drv.h          |    3 ++-
> >>  drivers/gpu/drm/i915/i915_reg.h          |   19 +++++++++++++------
> >>  drivers/gpu/drm/i915/intel_frontbuffer.c |    7 +++++--
> >>  drivers/gpu/drm/i915/intel_psr.c         |   18 +++++++++++++++++-
> >>  4 files changed, 37 insertions(+), 10 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> >>b/drivers/gpu/drm/i915/i915_drv.h index 42c69ca..ee5cd3b 100644
> >>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>@@ -2355,7 +2355,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 a828cf5..068c8da 100644
> >>--- a/drivers/gpu/drm/i915/i915_reg.h
> >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> >>@@ -2619,12 +2619,14 @@ enum skl_disp_power_wells {
> >>  #define   EDP_PSR_TP1_TIME_0us (3<<4)
> >>  #define   EDP_PSR_IDLE_FRAME_SHIFT 0
> >>
> >>-#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_BASE(dev) + 0x10) -#define 
> >>EDP_PSR_AUX_DATA1(dev) (EDP_PSR_BASE(dev) + 0x14) -#define 
> >>EDP_PSR_AUX_DATA2(dev) (EDP_PSR_BASE(dev) + 0x18) -#define 
> >>EDP_PSR_AUX_DATA3(dev) (EDP_PSR_BASE(dev) + 0x1c) -#define 
> >>EDP_PSR_AUX_DATA4(dev) (EDP_PSR_BASE(dev) + 0x20) -#define 
> >>EDP_PSR_AUX_DATA5(dev) (EDP_PSR_BASE(dev) + 0x24)
> >>+#define EDP_PSR_AUX_BASE(dev) (INTEL_INFO(dev)->gen >= 9 ? \
> >>+ 0x64000 : EDP_PSR_BASE(dev))
> >>+#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_AUX_BASE(dev) + 0x10) #define 
> >>+EDP_PSR_AUX_DATA1(dev) (EDP_PSR_AUX_BASE(dev) + 0x14) #define 
> >>+EDP_PSR_AUX_DATA2(dev) (EDP_PSR_AUX_BASE(dev) + 0x18) #define 
> >>+EDP_PSR_AUX_DATA3(dev) (EDP_PSR_AUX_BASE(dev) + 0x1c) #define 
> >>+EDP_PSR_AUX_DATA4(dev) (EDP_PSR_AUX_BASE(dev) + 0x20) #define 
> >>+EDP_PSR_AUX_DATA5(dev) (EDP_PSR_AUX_BASE(dev) + 0x24)
> >>
> >>  #define EDP_PSR_STATUS_CTL(dev) (EDP_PSR_BASE(dev) + 0x40)
> >>  #define   EDP_PSR_STATUS_STATE_MASK (7<<29)
> >>@@ -3771,6 +3773,11 @@ enum skl_disp_power_wells {
> >>  #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);
> >I'm pretty sure the hw isn't good enough yet to detect everything, 
> >unfortunately the testcase is also not quite ready yet. In any case 
> >these changes should be moved into the inel_psr_* functions and in a 
> >separate patch.
> When ubuntu boots up and stops at the login screen, the text I enter 
> comes slowly, looks like psr exit and entry is not very prompt at that time.
> But when I try with SW tracking, I see that PSR doesn't get enabled at 
> all at that point!
> Also, I don't see calls to frontbuffer invalidate and flush at that point.
> So, with SW tracking too we will have cases where we don't enter psr.
> Any inputs on why frontbuffer calls are not received during login screen?
> 
> Please note that after login, it works fine if I type something on the 
> terminal or do some other things.

I have no idea why or what exactly the ubuntu screen does in it's login function. Just please note that there are _lots_ more desktop environments on linux than just ubunut's flavour of gnome, so we really need to have that psr testcase working (which covers them all) instead of manual testing.
-Daniel

> >Also someone needs to re-review the psr igt testcase to make sure it 
> >covers everything. Ville could do that too since he's done the fbc 
> >testcase.
> >-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..6d2cdb8 100644
> >>--- a/drivers/gpu/drm/i915/intel_psr.c
> >>+++ b/drivers/gpu/drm/i915/intel_psr.c
> >>@@ -173,11 +173,24 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
> >>   I915_WRITE(EDP_PSR_AUX_DATA1(dev) + 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(EDP_PSR_AUX_CTL(dev)); 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(EDP_PSR_AUX_CTL(dev), val); } else { 
> >>+ I915_WRITE(EDP_PSR_AUX_CTL(dev),
> >>     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 +368,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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-19 11:40   ` sonika
@ 2015-01-20  9:50     ` Daniel Vetter
  2015-01-20 11:49       ` Jindal, Sonika
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2015-01-20  9:50 UTC (permalink / raw)
  To: sonika; +Cc: intel-gfx, Rodrigo Vivi

On Mon, Jan 19, 2015 at 05:10:58PM +0530, sonika wrote:
> 
> On Saturday 17 January 2015 09:54 AM, Daniel Vetter wrote:
> >On Fri, Jan 16, 2015 at 02:07:26PM +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.
> >>
> >>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/i915_drv.h          |    3 ++-
> >>  drivers/gpu/drm/i915/i915_reg.h          |   19 +++++++++++++------
> >>  drivers/gpu/drm/i915/intel_frontbuffer.c |    7 +++++--
> >>  drivers/gpu/drm/i915/intel_psr.c         |   18 +++++++++++++++++-
> >>  4 files changed, 37 insertions(+), 10 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >>index 42c69ca..ee5cd3b 100644
> >>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>@@ -2355,7 +2355,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 a828cf5..068c8da 100644
> >>--- a/drivers/gpu/drm/i915/i915_reg.h
> >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> >>@@ -2619,12 +2619,14 @@ enum skl_disp_power_wells {
> >>  #define   EDP_PSR_TP1_TIME_0us (3<<4)
> >>  #define   EDP_PSR_IDLE_FRAME_SHIFT 0
> >>
> >>-#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_BASE(dev) + 0x10)
> >>-#define EDP_PSR_AUX_DATA1(dev) (EDP_PSR_BASE(dev) + 0x14)
> >>-#define EDP_PSR_AUX_DATA2(dev) (EDP_PSR_BASE(dev) + 0x18)
> >>-#define EDP_PSR_AUX_DATA3(dev) (EDP_PSR_BASE(dev) + 0x1c)
> >>-#define EDP_PSR_AUX_DATA4(dev) (EDP_PSR_BASE(dev) + 0x20)
> >>-#define EDP_PSR_AUX_DATA5(dev) (EDP_PSR_BASE(dev) + 0x24)
> >>+#define EDP_PSR_AUX_BASE(dev) (INTEL_INFO(dev)->gen >= 9 ? \
> >>+ 0x64000 : EDP_PSR_BASE(dev))
> >>+#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_AUX_BASE(dev) + 0x10)
> >>+#define EDP_PSR_AUX_DATA1(dev) (EDP_PSR_AUX_BASE(dev) + 0x14)
> >>+#define EDP_PSR_AUX_DATA2(dev) (EDP_PSR_AUX_BASE(dev) + 0x18)
> >>+#define EDP_PSR_AUX_DATA3(dev) (EDP_PSR_AUX_BASE(dev) + 0x1c)
> >>+#define EDP_PSR_AUX_DATA4(dev) (EDP_PSR_AUX_BASE(dev) + 0x20)
> >>+#define EDP_PSR_AUX_DATA5(dev) (EDP_PSR_AUX_BASE(dev) + 0x24)
> >>
> >>  #define EDP_PSR_STATUS_CTL(dev) (EDP_PSR_BASE(dev) + 0x40)
> >>  #define   EDP_PSR_STATUS_STATE_MASK (7<<29)
> >>@@ -3771,6 +3773,11 @@ enum skl_disp_power_wells {
> >>  #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);
> >I'm pretty sure the hw isn't good enough yet to detect everything,
> >unfortunately the testcase is also not quite ready yet. In any case these
> >changes should be moved into the inel_psr_* functions and in a separate
> >patch.
> When ubuntu boots up and stops at the login screen, the text I enter comes
> slowly, looks like psr exit and entry is not very prompt at that time.
> But when I try with SW tracking, I see that PSR doesn't get enabled at all
> at that point!
> Also, I don't see calls to frontbuffer invalidate and flush at that point.
> So, with SW tracking too we will have cases where we don't enter psr.
> Any inputs on why frontbuffer calls are not received during login screen?
> 
> Please note that after login, it works fine if I type something on the
> terminal or do some other
> things.

I have no idea why or what exactly the ubuntu screen does in it's login
function. Just please note that there are _lots_ more desktop environments
on linux than just ubunut's flavour of gnome, so we really need to have
that psr testcase working (which covers them all) instead of manual
testing.
-Daniel

> >Also someone needs to re-review the psr igt testcase to make sure it
> >covers everything. Ville could do that too since he's done the fbc
> >testcase.
> >-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..6d2cdb8 100644
> >>--- a/drivers/gpu/drm/i915/intel_psr.c
> >>+++ b/drivers/gpu/drm/i915/intel_psr.c
> >>@@ -173,11 +173,24 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
> >>   I915_WRITE(EDP_PSR_AUX_DATA1(dev) + 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(EDP_PSR_AUX_CTL(dev));
> >>+ 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(EDP_PSR_AUX_CTL(dev), val);
> >>+ } else {
> >>+ I915_WRITE(EDP_PSR_AUX_CTL(dev),
> >>     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 +368,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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-17  4:24 ` Daniel Vetter
@ 2015-01-19 11:40   ` sonika
  2015-01-20  9:50     ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: sonika @ 2015-01-19 11:40 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, Rodrigo Vivi


On Saturday 17 January 2015 09:54 AM, Daniel Vetter wrote:
> On Fri, Jan 16, 2015 at 02:07:26PM +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.
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h          |    3 ++-
>>   drivers/gpu/drm/i915/i915_reg.h          |   19 +++++++++++++------
>>   drivers/gpu/drm/i915/intel_frontbuffer.c |    7 +++++--
>>   drivers/gpu/drm/i915/intel_psr.c         |   18 +++++++++++++++++-
>>   4 files changed, 37 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 42c69ca..ee5cd3b 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2355,7 +2355,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 a828cf5..068c8da 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2619,12 +2619,14 @@ enum skl_disp_power_wells {
>>   #define   EDP_PSR_TP1_TIME_0us (3<<4)
>>   #define   EDP_PSR_IDLE_FRAME_SHIFT 0
>>
>> -#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_BASE(dev) + 0x10)
>> -#define EDP_PSR_AUX_DATA1(dev) (EDP_PSR_BASE(dev) + 0x14)
>> -#define EDP_PSR_AUX_DATA2(dev) (EDP_PSR_BASE(dev) + 0x18)
>> -#define EDP_PSR_AUX_DATA3(dev) (EDP_PSR_BASE(dev) + 0x1c)
>> -#define EDP_PSR_AUX_DATA4(dev) (EDP_PSR_BASE(dev) + 0x20)
>> -#define EDP_PSR_AUX_DATA5(dev) (EDP_PSR_BASE(dev) + 0x24)
>> +#define EDP_PSR_AUX_BASE(dev) (INTEL_INFO(dev)->gen >= 9 ? \
>> + 0x64000 : EDP_PSR_BASE(dev))
>> +#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_AUX_BASE(dev) + 0x10)
>> +#define EDP_PSR_AUX_DATA1(dev) (EDP_PSR_AUX_BASE(dev) + 0x14)
>> +#define EDP_PSR_AUX_DATA2(dev) (EDP_PSR_AUX_BASE(dev) + 0x18)
>> +#define EDP_PSR_AUX_DATA3(dev) (EDP_PSR_AUX_BASE(dev) + 0x1c)
>> +#define EDP_PSR_AUX_DATA4(dev) (EDP_PSR_AUX_BASE(dev) + 0x20)
>> +#define EDP_PSR_AUX_DATA5(dev) (EDP_PSR_AUX_BASE(dev) + 0x24)
>>
>>   #define EDP_PSR_STATUS_CTL(dev) (EDP_PSR_BASE(dev) + 0x40)
>>   #define   EDP_PSR_STATUS_STATE_MASK (7<<29)
>> @@ -3771,6 +3773,11 @@ enum skl_disp_power_wells {
>>   #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);
> I'm pretty sure the hw isn't good enough yet to detect everything,
> unfortunately the testcase is also not quite ready yet. In any case these
> changes should be moved into the inel_psr_* functions and in a separate
> patch.
When ubuntu boots up and stops at the login screen, the text I enter comes
slowly, looks like psr exit and entry is not very prompt at that time.
But when I try with SW tracking, I see that PSR doesn't get enabled at 
all at that point!
Also, I don't see calls to frontbuffer invalidate and flush at that point.
So, with SW tracking too we will have cases where we don't enter psr.
Any inputs on why frontbuffer calls are not received during login screen?

Please note that after login, it works fine if I type something on the 
terminal or do some other
things.
> Also someone needs to re-review the psr igt testcase to make sure it
> covers everything. Ville could do that too since he's done the fbc
> testcase.
> -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..6d2cdb8 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -173,11 +173,24 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>>    I915_WRITE(EDP_PSR_AUX_DATA1(dev) + 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(EDP_PSR_AUX_CTL(dev));
>> + 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(EDP_PSR_AUX_CTL(dev), val);
>> + } else {
>> + I915_WRITE(EDP_PSR_AUX_CTL(dev),
>>      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 +368,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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-16  8:37 Sonika Jindal
  2015-01-16 15:06 ` shuang.he
@ 2015-01-17  4:24 ` Daniel Vetter
  2015-01-19 11:40   ` sonika
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2015-01-17  4:24 UTC (permalink / raw)
  To: Sonika Jindal; +Cc: intel-gfx, Rodrigo Vivi

On Fri, Jan 16, 2015 at 02:07:26PM +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.
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          |    3 ++-
>  drivers/gpu/drm/i915/i915_reg.h          |   19 +++++++++++++------
>  drivers/gpu/drm/i915/intel_frontbuffer.c |    7 +++++--
>  drivers/gpu/drm/i915/intel_psr.c         |   18 +++++++++++++++++-
>  4 files changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 42c69ca..ee5cd3b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2355,7 +2355,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 a828cf5..068c8da 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2619,12 +2619,14 @@ enum skl_disp_power_wells {
>  #define   EDP_PSR_TP1_TIME_0us (3<<4)
>  #define   EDP_PSR_IDLE_FRAME_SHIFT 0
>
> -#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_BASE(dev) + 0x10)
> -#define EDP_PSR_AUX_DATA1(dev) (EDP_PSR_BASE(dev) + 0x14)
> -#define EDP_PSR_AUX_DATA2(dev) (EDP_PSR_BASE(dev) + 0x18)
> -#define EDP_PSR_AUX_DATA3(dev) (EDP_PSR_BASE(dev) + 0x1c)
> -#define EDP_PSR_AUX_DATA4(dev) (EDP_PSR_BASE(dev) + 0x20)
> -#define EDP_PSR_AUX_DATA5(dev) (EDP_PSR_BASE(dev) + 0x24)
> +#define EDP_PSR_AUX_BASE(dev) (INTEL_INFO(dev)->gen >= 9 ? \
> + 0x64000 : EDP_PSR_BASE(dev))
> +#define EDP_PSR_AUX_CTL(dev) (EDP_PSR_AUX_BASE(dev) + 0x10)
> +#define EDP_PSR_AUX_DATA1(dev) (EDP_PSR_AUX_BASE(dev) + 0x14)
> +#define EDP_PSR_AUX_DATA2(dev) (EDP_PSR_AUX_BASE(dev) + 0x18)
> +#define EDP_PSR_AUX_DATA3(dev) (EDP_PSR_AUX_BASE(dev) + 0x1c)
> +#define EDP_PSR_AUX_DATA4(dev) (EDP_PSR_AUX_BASE(dev) + 0x20)
> +#define EDP_PSR_AUX_DATA5(dev) (EDP_PSR_AUX_BASE(dev) + 0x24)
>
>  #define EDP_PSR_STATUS_CTL(dev) (EDP_PSR_BASE(dev) + 0x40)
>  #define   EDP_PSR_STATUS_STATE_MASK (7<<29)
> @@ -3771,6 +3773,11 @@ enum skl_disp_power_wells {
>  #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);

I'm pretty sure the hw isn't good enough yet to detect everything,
unfortunately the testcase is also not quite ready yet. In any case these
changes should be moved into the inel_psr_* functions and in a separate
patch.

Also someone needs to re-review the psr igt testcase to make sure it
covers everything. Ville could do that too since he's done the fbc
testcase.
-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..6d2cdb8 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -173,11 +173,24 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>   I915_WRITE(EDP_PSR_AUX_DATA1(dev) + 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(EDP_PSR_AUX_CTL(dev));
> + 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(EDP_PSR_AUX_CTL(dev), val);
> + } else {
> + I915_WRITE(EDP_PSR_AUX_CTL(dev),
>     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 +368,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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
  2015-01-16  8:37 Sonika Jindal
@ 2015-01-16 15:06 ` shuang.he
  2015-01-17  4:24 ` Daniel Vetter
  1 sibling, 0 replies; 16+ messages in thread
From: shuang.he @ 2015-01-16 15:06 UTC (permalink / raw)
  To: shuang.he, ethan.gao, intel-gfx, sonika.jindal

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 5591
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              353/353              352/353
ILK                                  353/353              353/353
SNB                                  400/422              400/422
IVB                                  487/487              487/487
BYT                                  296/296              296/296
HSW              +20-8              487/508              499/508
BDW                                  401/402              401/402
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gen3_render_linear_blits      PASS(2, M25M23)      CRASH(1, M23)
*HSW  igt_kms_cursor_crc_cursor-size-change      NSPT(1, M19)TIMEOUT(1, M40)PASS(1, M20)      PASS(1, M40)
*HSW  igt_kms_fence_pin_leak      NSPT(1, M19)DMESG_WARN(1, M40)PASS(1, M20)      PASS(1, M40)
 HSW  igt_kms_flip_dpms-vs-vblank-race      DMESG_WARN(2, M20M40)PASS(1, M19)      DMESG_WARN(1, M40)
 HSW  igt_kms_flip_flip-vs-dpms-off-vs-modeset      DMESG_WARN(1, M40)PASS(2, M19M20)      DMESG_WARN(1, M40)
*HSW  igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip      NSPT(1, M19)TIMEOUT(1, M40)PASS(1, M20)      PASS(1, M40)
*HSW  igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip      NSPT(1, M19)TIMEOUT(1, M40)PASS(1, M20)      FAIL(1, M40)
*HSW  igt_kms_plane_plane-panning-top-left-pipe-B-plane-1      TIMEOUT(1, M40)PASS(1, M19)      DMESG_FAIL(1, M40)
 HSW  igt_kms_plane_plane-panning-top-left-pipe-C-plane-1      TIMEOUT(1, M40)PASS(1, M19)      TIMEOUT(1, M40)
*HSW  igt_pm_lpsp_non-edp      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_cursor      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_cursor-dpms      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_dpms-mode-unset-non-lpsp      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_dpms-non-lpsp      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_drm-resources-equal      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_fences      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_fences-dpms      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_gem-execbuf      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_gem-mmap-cpu      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_gem-mmap-gtt      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_gem-pread      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_i2c      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_modeset-non-lpsp      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_modeset-non-lpsp-stress-no-wait      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_pci-d3-state      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_pm_rpm_rte      NSPT(1, M19)PASS(1, M20)      PASS(1, M40)
*HSW  igt_kms_flip_absolute-wf_vblank-interruptible      PASS(1, M19)      DMESG_WARN(1, M40)
*HSW  igt_kms_flip_blocking-absolute-wf_vblank      PASS(1, M19)      DMESG_WARN(1, M40)
*HSW  igt_kms_flip_plain-flip-fb-recreate      PASS(1, M19)      DMESG_WARN(1, M40)
*HSW  igt_kms_flip_plain-flip-ts-check-interruptible      PASS(1, M19)      DMESG_WARN(1, M40)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] drm/i915/skl: Enabling PSR on Skylake
@ 2015-01-16  8:37 Sonika Jindal
  2015-01-16 15:06 ` shuang.he
  2015-01-17  4:24 ` Daniel Vetter
  0 siblings, 2 replies; 16+ messages in thread
From: Sonika Jindal @ 2015-01-16  8:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: rodrigo.vivi

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.

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          |    3 ++-
 drivers/gpu/drm/i915/i915_reg.h          |   19 +++++++++++++------
 drivers/gpu/drm/i915/intel_frontbuffer.c |    7 +++++--
 drivers/gpu/drm/i915/intel_psr.c         |   18 +++++++++++++++++-
 4 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 42c69ca..ee5cd3b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2355,7 +2355,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 a828cf5..068c8da 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2619,12 +2619,14 @@ enum skl_disp_power_wells {
 #define   EDP_PSR_TP1_TIME_0us			(3<<4)
 #define   EDP_PSR_IDLE_FRAME_SHIFT		0
 
-#define EDP_PSR_AUX_CTL(dev)			(EDP_PSR_BASE(dev) + 0x10)
-#define EDP_PSR_AUX_DATA1(dev)			(EDP_PSR_BASE(dev) + 0x14)
-#define EDP_PSR_AUX_DATA2(dev)			(EDP_PSR_BASE(dev) + 0x18)
-#define EDP_PSR_AUX_DATA3(dev)			(EDP_PSR_BASE(dev) + 0x1c)
-#define EDP_PSR_AUX_DATA4(dev)			(EDP_PSR_BASE(dev) + 0x20)
-#define EDP_PSR_AUX_DATA5(dev)			(EDP_PSR_BASE(dev) + 0x24)
+#define EDP_PSR_AUX_BASE(dev)		(INTEL_INFO(dev)->gen >= 9 ? \
+						0x64000 : EDP_PSR_BASE(dev))
+#define EDP_PSR_AUX_CTL(dev)		(EDP_PSR_AUX_BASE(dev) + 0x10)
+#define EDP_PSR_AUX_DATA1(dev)		(EDP_PSR_AUX_BASE(dev) + 0x14)
+#define EDP_PSR_AUX_DATA2(dev)		(EDP_PSR_AUX_BASE(dev) + 0x18)
+#define EDP_PSR_AUX_DATA3(dev)		(EDP_PSR_AUX_BASE(dev) + 0x1c)
+#define EDP_PSR_AUX_DATA4(dev)		(EDP_PSR_AUX_BASE(dev) + 0x20)
+#define EDP_PSR_AUX_DATA5(dev)		(EDP_PSR_AUX_BASE(dev) + 0x24)
 
 #define EDP_PSR_STATUS_CTL(dev)			(EDP_PSR_BASE(dev) + 0x40)
 #define   EDP_PSR_STATUS_STATE_MASK		(7<<29)
@@ -3771,6 +3773,11 @@ enum skl_disp_power_wells {
 #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);
 
 	/*
 	 * 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..6d2cdb8 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -173,11 +173,24 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 		I915_WRITE(EDP_PSR_AUX_DATA1(dev) + 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(EDP_PSR_AUX_CTL(dev));
+		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(EDP_PSR_AUX_CTL(dev), val);
+	} else {
+		I915_WRITE(EDP_PSR_AUX_CTL(dev),
 		   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 +368,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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-01-29 16:10 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-22  9:00 [PATCH] drm/i915/skl: Enabling PSR on Skylake Sonika Jindal
2015-01-22 17:48 ` shuang.he
2015-01-28 16:02 ` Daniel Vetter
2015-01-29  3:57   ` sonika
2015-01-29 16:11     ` Daniel Vetter
  -- strict thread matches above, loose matches on Subject: below --
2015-01-16  8:37 Sonika Jindal
2015-01-16 15:06 ` shuang.he
2015-01-17  4:24 ` Daniel Vetter
2015-01-19 11:40   ` sonika
2015-01-20  9:50     ` Daniel Vetter
2015-01-20 11:49       ` Jindal, Sonika
2015-01-20 22:01         ` Rodrigo Vivi
2015-01-21  4:53           ` sonika
2015-01-21  8:37           ` Daniel Vetter
2015-01-21  8:55             ` sonika
2015-01-21  9:42               ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.