From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 50/62] [v5] drm/i915/bdw: Support eDP PSR Date: Tue, 4 Mar 2014 22:31:58 -0800 Message-ID: <20140305063158.GA21767@intel.com> References: <874n7scv6a.fsf@intel.com> <1383633905-18166-1-git-send-email-benjamin.widawsky@intel.com> <8506592BE1D7B64680288B9B8A90F319017D6B97@BGSMSX101.gar.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-pa0-f53.google.com (mail-pa0-f53.google.com [209.85.220.53]) by gabe.freedesktop.org (Postfix) with ESMTP id 0A7DDFA240 for ; Tue, 4 Mar 2014 22:32:03 -0800 (PST) Received: by mail-pa0-f53.google.com with SMTP id ld10so656798pab.40 for ; Tue, 04 Mar 2014 22:32:02 -0800 (PST) Content-Disposition: inline In-Reply-To: <8506592BE1D7B64680288B9B8A90F319017D6B97@BGSMSX101.gar.corp.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: "Kumar, Kiran S" Cc: "Nikula, Jani" , Intel GFX , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org 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 > Reviewed-by: Jani Nikula > Signed-off-by: Ben Widawsky > --- > 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