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: [email protected]
> [mailto:[email protected]] 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 <[email protected]>
> Reviewed-by: Jani Nikula <[email protected]>
> Signed-off-by: Ben Widawsky <[email protected]>
> ---
> 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
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
