Re: [PATCH 50/62] [v5] drm/i915/bdw: Support eDP PSR

[Ben Widawsky <benjamin.widawsky@intel.com>]

This is a bug. Someone needs to send me back to C-programmer school.

Bits 26:25 are reserved in the spec. Furthermore, there shouldn't be a
functional difference since link_entry_time =
EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES   (0<<25). So you found the bug, but I
think the solution is actually:

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c512d78..2c0ceb4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1723,7 +1723,7 @@ static void intel_edp_psr_enable_source(struct intel_dp *intel_dp)
                val |= EDP_PSR_LINK_DISABLE;
 
        I915_WRITE(EDP_PSR_CTL(dev), val |
-                  IS_BROADWELL(dev) ? 0 : link_entry_time |
+                  (IS_BROADWELL(dev) ? 0 : link_entry_time) |
                   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
                   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
                   EDP_PSR_ENABLE);




On Tue, Mar 04, 2014 at 09:31:28AM +0000, Kumar, Kiran S wrote:
> Hi Ben,
> 
> Can you please let me know the reason for explicit about not setting min link entry time for BDW. During my PSR testing on BDW, I found perf counter not getting increment and SRD control is setting to 0x0 with the following check:
>  	IS_BROADWELL(dev) ? 0 : link_entry_time
> 
> When I remove and used only "link_entry_time" without check for BDW, PSR worked fine. (perf counter started incrementing)
> 
> Thanks
> Kiran
> 
> -----Original Message-----
> From: intel-gfx-bounces@lists.freedesktop.org [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Ben Widawsky
> Sent: Tuesday, November 05, 2013 12:15 PM
> To: Intel GFX
> Cc: Nikula, Jani; Ben Widawsky; Widawsky, Benjamin
> Subject: [Intel-gfx] [PATCH 50/62] [v5] drm/i915/bdw: Support eDP PSR
> 
> Broadwell PSR support is a superset of Haswell. With this simple register base calculation, everything that worked on HSW for eDP PSR should work on BDW.
> 
> Note that Broadwell provides additional PSR support. This is not addressed at this time.
> 
> v2: Make the HAS_PSR include BDW
> 
> v3: Use the correct offset (I had incorrectly used one from my faulty
> brain) (Art!)
> 
> v4: It helps if you git add
> 
> v5: Be explicit about not setting min link entry time for BDW. This should be no functional change over v4 (Jani)
> 
> Reviewed-by: Art Runyan <arthur.j.runyan@intel.com>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 2 +-
>  drivers/gpu/drm/i915/i915_reg.h | 4 ++--  drivers/gpu/drm/i915/intel_dp.c | 3 ++-
>  3 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f222eb4..dc79a0f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1808,7 +1808,7 @@ struct drm_i915_file_private {
>  #define HAS_DDI(dev)		(INTEL_INFO(dev)->has_ddi)
>  #define HAS_POWER_WELL(dev)	(IS_HASWELL(dev) || IS_GEN8(dev))
>  #define HAS_FPGA_DBG_UNCLAIMED(dev)	(INTEL_INFO(dev)->has_fpga_dbg)
> -#define HAS_PSR(dev)		(IS_HASWELL(dev))
> +#define HAS_PSR(dev)		(IS_HASWELL(dev) || IS_BROADWELL(dev))
>  
>  #define INTEL_PCH_DEVICE_ID_MASK		0xff00
>  #define INTEL_PCH_IBX_DEVICE_ID_TYPE		0x3b00
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ba1fe7e..3761c80 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1959,8 +1959,8 @@
>  #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)  #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
>  
> -/* HSW eDP PSR registers */
> -#define EDP_PSR_BASE(dev)			0x64800
> +/* HSW+ eDP PSR registers */
> +#define EDP_PSR_BASE(dev)                       (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
>  #define EDP_PSR_CTL(dev)			(EDP_PSR_BASE(dev) + 0)
>  #define   EDP_PSR_ENABLE			(1<<31)
>  #define   EDP_PSR_LINK_DISABLE			(0<<27)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 7725f81..6e4246f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1603,6 +1603,7 @@ static void intel_edp_psr_enable_source(struct intel_dp *intel_dp)
>  	uint32_t max_sleep_time = 0x1f;
>  	uint32_t idle_frames = 1;
>  	uint32_t val = 0x0;
> +	const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
>  
>  	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
>  		val |= EDP_PSR_LINK_STANDBY;
> @@ -1613,7 +1614,7 @@ static void intel_edp_psr_enable_source(struct intel_dp *intel_dp)
>  		val |= EDP_PSR_LINK_DISABLE;
>  
>  	I915_WRITE(EDP_PSR_CTL(dev), val |
> -		   EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES |
> +		   IS_BROADWELL(dev) ? 0 : link_entry_time |
>  		   max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT |
>  		   idle_frames << EDP_PSR_IDLE_FRAME_SHIFT |
>  		   EDP_PSR_ENABLE);
> --
> 1.8.4.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Ben Widawsky, Intel Open Source Technology Center