* [PATCH 2/7] drm/i915/psr: Try to program link training times correctly [not found] <1463590036-17824-1-git-send-email-daniel.vetter@ffwll.ch> @ 2016-05-18 16:47 ` Daniel Vetter 2016-05-18 17:39 ` [Intel-gfx] " Ville Syrjälä [not found] ` <573D9A5A.4010004@intel.com> 0 siblings, 2 replies; 5+ messages in thread From: Daniel Vetter @ 2016-05-18 16:47 UTC (permalink / raw) To: Intel Graphics Development Cc: Daniel Vetter, Lyude, stable, Rodrigo Vivi, Sonika Jindal, Durgadoss R, Pandiyan, Dhinakaran, Daniel Vetter Oops. Hw default for programming these fields to 0 is "skip link training". Display won't take that too well usually. v2: Unbotch the math a bit. v3: Drop debug hunk. Tested-by: Lyude <cpaul@redhat.com> Cc: Lyude <cpaul@redhat.com> Cc: stable@vger.kernel.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176 Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Sonika Jindal <sonika.jindal@intel.com> Cc: Durgadoss R <durgadoss.r@intel.com> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index c3abae4bc596..a788d1e9589b 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -280,7 +280,10 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp) * with the 5 or 6 idle patterns. */ uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames); - uint32_t val = 0x0; + uint32_t val = EDP_PSR_ENABLE; + + val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; + val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; if (IS_HASWELL(dev)) val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES; @@ -288,14 +291,50 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp) if (dev_priv->psr.link_standby) val |= EDP_PSR_LINK_STANDBY; - I915_WRITE(EDP_PSR_CTL, val | - max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT | - idle_frames << EDP_PSR_IDLE_FRAME_SHIFT | - EDP_PSR_ENABLE); + if (dev_priv->vbt.psr.tp1_wakeup_time > 5) + val |= EDP_PSR_TP1_TIME_2500us; + else if (dev_priv->vbt.psr.tp1_wakeup_time > 1) + val |= EDP_PSR_TP1_TIME_500us; + else if (dev_priv->vbt.psr.tp1_wakeup_time > 0) + val |= EDP_PSR_TP1_TIME_100us; + else + val |= EDP_PSR_TP1_TIME_0us; + + if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5) + val |= EDP_PSR_TP2_TP3_TIME_2500us; + else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1) + val |= EDP_PSR_TP2_TP3_TIME_500us; + else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0) + val |= EDP_PSR_TP2_TP3_TIME_100us; + else + val |= EDP_PSR_TP2_TP3_TIME_0us; + + if (intel_dp_source_supports_hbr2(intel_dp) && + drm_dp_tps3_supported(intel_dp->dpcd)) + val |= EDP_PSR_TP1_TP3_SEL; + else + val |= EDP_PSR_TP1_TP2_SEL; + + I915_WRITE(EDP_PSR_CTL, val); + + if (!dev_priv->psr.psr2_support) + return; + + /* FIXME: selective update is probably totally broken because it doesn't + * mesh at all with our frontbuffer tracking. And the hw alone isn't + * good enough. */ + val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE; + + if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5) + val |= EDP_PSR2_TP2_TIME_2500; + else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1) + val |= EDP_PSR2_TP2_TIME_500; + else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0) + val |= EDP_PSR2_TP2_TIME_100; + else + val |= EDP_PSR2_TP2_TIME_50; - if (dev_priv->psr.psr2_support) - I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE | - EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100); + I915_WRITE(EDP_PSR2_CTL, val); } static bool intel_psr_match_conditions(struct intel_dp *intel_dp) -- 2.8.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [PATCH 2/7] drm/i915/psr: Try to program link training times correctly 2016-05-18 16:47 ` [PATCH 2/7] drm/i915/psr: Try to program link training times correctly Daniel Vetter @ 2016-05-18 17:39 ` Ville Syrjälä 2016-05-18 18:04 ` Daniel Vetter [not found] ` <573D9A5A.4010004@intel.com> 1 sibling, 1 reply; 5+ messages in thread From: Ville Syrjälä @ 2016-05-18 17:39 UTC (permalink / raw) To: Daniel Vetter Cc: Intel Graphics Development, Daniel Vetter, stable, Pandiyan, Dhinakaran, Rodrigo Vivi On Wed, May 18, 2016 at 06:47:11PM +0200, Daniel Vetter wrote: > Oops. Hw default for programming these fields to 0 is "skip link > training". Display won't take that too well usually. s/skip/500 usec/ > > v2: Unbotch the math a bit. > > v3: Drop debug hunk. > > Tested-by: Lyude <cpaul@redhat.com> > Cc: Lyude <cpaul@redhat.com> > Cc: stable@vger.kernel.org > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176 > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Sonika Jindal <sonika.jindal@intel.com> > Cc: Durgadoss R <durgadoss.r@intel.com> > Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 47 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > index c3abae4bc596..a788d1e9589b 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -280,7 +280,10 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp) > * with the 5 or 6 idle patterns. > */ > uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames); > - uint32_t val = 0x0; > + uint32_t val = EDP_PSR_ENABLE; > + > + val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; > + val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; > > if (IS_HASWELL(dev)) > val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES; > @@ -288,14 +291,50 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp) > if (dev_priv->psr.link_standby) > val |= EDP_PSR_LINK_STANDBY; > > - I915_WRITE(EDP_PSR_CTL, val | > - max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT | > - idle_frames << EDP_PSR_IDLE_FRAME_SHIFT | > - EDP_PSR_ENABLE); > + if (dev_priv->vbt.psr.tp1_wakeup_time > 5) > + val |= EDP_PSR_TP1_TIME_2500us; > + else if (dev_priv->vbt.psr.tp1_wakeup_time > 1) > + val |= EDP_PSR_TP1_TIME_500us; > + else if (dev_priv->vbt.psr.tp1_wakeup_time > 0) > + val |= EDP_PSR_TP1_TIME_100us; > + else > + val |= EDP_PSR_TP1_TIME_0us; > + > + if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5) > + val |= EDP_PSR_TP2_TP3_TIME_2500us; > + else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1) > + val |= EDP_PSR_TP2_TP3_TIME_500us; > + else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0) > + val |= EDP_PSR_TP2_TP3_TIME_100us; > + else > + val |= EDP_PSR_TP2_TP3_TIME_0us; The current vbt spec is confusing. But after some head scratching this does look correct. Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> > + > + if (intel_dp_source_supports_hbr2(intel_dp) && > + drm_dp_tps3_supported(intel_dp->dpcd)) > + val |= EDP_PSR_TP1_TP3_SEL; > + else > + val |= EDP_PSR_TP1_TP2_SEL; > + > + I915_WRITE(EDP_PSR_CTL, val); > + > + if (!dev_priv->psr.psr2_support) > + return; > + > + /* FIXME: selective update is probably totally broken because it doesn't > + * mesh at all with our frontbuffer tracking. And the hw alone isn't > + * good enough. */ > + val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE; > + > + if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5) > + val |= EDP_PSR2_TP2_TIME_2500; > + else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1) > + val |= EDP_PSR2_TP2_TIME_500; > + else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0) > + val |= EDP_PSR2_TP2_TIME_100; > + else > + val |= EDP_PSR2_TP2_TIME_50; > > - if (dev_priv->psr.psr2_support) > - I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE | > - EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100); > + I915_WRITE(EDP_PSR2_CTL, val); > } > > static bool intel_psr_match_conditions(struct intel_dp *intel_dp) > -- > 2.8.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [PATCH 2/7] drm/i915/psr: Try to program link training times correctly 2016-05-18 17:39 ` [Intel-gfx] " Ville Syrjälä @ 2016-05-18 18:04 ` Daniel Vetter 2016-05-18 18:09 ` Ville Syrjälä 0 siblings, 1 reply; 5+ messages in thread From: Daniel Vetter @ 2016-05-18 18:04 UTC (permalink / raw) To: Ville Syrjälä Cc: Intel Graphics Development, Daniel Vetter, stable, Pandiyan, Dhinakaran, Rodrigo Vivi On Wed, May 18, 2016 at 7:39 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Wed, May 18, 2016 at 06:47:11PM +0200, Daniel Vetter wrote: >> Oops. Hw default for programming these fields to 0 is "skip link >> training". Display won't take that too well usually. > > s/skip/500 usec/ Yeah, my reading skills have reached an all time low ;-) But we confirmed on irc that the hardcoded 500usec was indeed wrong, since the fixed machines now run on 2.5ms of link training time. I'll update the commit message when merging to reflect that. Also Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Tested-by: fritsch@kodi.tv -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [PATCH 2/7] drm/i915/psr: Try to program link training times correctly 2016-05-18 18:04 ` Daniel Vetter @ 2016-05-18 18:09 ` Ville Syrjälä 0 siblings, 0 replies; 5+ messages in thread From: Ville Syrjälä @ 2016-05-18 18:09 UTC (permalink / raw) To: Daniel Vetter Cc: Intel Graphics Development, Daniel Vetter, stable, Pandiyan, Dhinakaran, Rodrigo Vivi On Wed, May 18, 2016 at 08:04:02PM +0200, Daniel Vetter wrote: > On Wed, May 18, 2016 at 7:39 PM, Ville Syrj�l� > <ville.syrjala@linux.intel.com> wrote: > > On Wed, May 18, 2016 at 06:47:11PM +0200, Daniel Vetter wrote: > >> Oops. Hw default for programming these fields to 0 is "skip link > >> training". Display won't take that too well usually. > > > > s/skip/500 usec/ > > Yeah, my reading skills have reached an all time low ;-) But we > confirmed on irc that the hardcoded 500usec was indeed wrong, since > the fixed machines now run on 2.5ms of link training time. I'll update > the commit message when merging to reflect that. Also > > Tested-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> That can be slapped onto the entire series. > Tested-by: fritsch@kodi.tv > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <573D9A5A.4010004@intel.com>]
* Re: [PATCH 2/7] drm/i915/psr: Try to program link training times correctly [not found] ` <573D9A5A.4010004@intel.com> @ 2016-05-20 7:33 ` Daniel Vetter 0 siblings, 0 replies; 5+ messages in thread From: Daniel Vetter @ 2016-05-20 7:33 UTC (permalink / raw) To: Jindal, Sonika Cc: Daniel Vetter, Intel Graphics Development, Lyude, stable, Rodrigo Vivi, Durgadoss R, Pandiyan, Dhinakaran, Daniel Vetter On Thu, May 19, 2016 at 04:20:02PM +0530, Jindal, Sonika wrote: > > > On 5/18/2016 10:17 PM, Daniel Vetter wrote: > >Oops. Hw default for programming these fields to 0 is "skip link > >training". Display won't take that too well usually. > But we were defaulting it to value 0, which means 500us for both TP1 and TP2 > or TP3 time. > I dont think it means skip link training. This is just to set the time for > the patterns. > Skip aux handshake can happen if bit 12 of SRD_CTL is set. > > Does this solution help in fixing the bug mentioned here? See the other thread, I misread and yes it does help. -Daniel > > > > >v2: Unbotch the math a bit. > > > >v3: Drop debug hunk. > > > >Tested-by: Lyude <cpaul@redhat.com> > >Cc: Lyude <cpaul@redhat.com> > >Cc: stable@vger.kernel.org > >Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95176 > >Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > >Cc: Sonika Jindal <sonika.jindal@intel.com> > >Cc: Durgadoss R <durgadoss.r@intel.com> > >Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> > >Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > >--- > > drivers/gpu/drm/i915/intel_psr.c | 55 ++++++++++++++++++++++++++++++++++------ > > 1 file changed, 47 insertions(+), 8 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > >index c3abae4bc596..a788d1e9589b 100644 > >--- a/drivers/gpu/drm/i915/intel_psr.c > >+++ b/drivers/gpu/drm/i915/intel_psr.c > >@@ -280,7 +280,10 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp) > > * with the 5 or 6 idle patterns. > > */ > > uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames); > >- uint32_t val = 0x0; > >+ uint32_t val = EDP_PSR_ENABLE; > >+ > >+ val |= max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT; > >+ val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT; > > if (IS_HASWELL(dev)) > > val |= EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES; > >@@ -288,14 +291,50 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp) > > if (dev_priv->psr.link_standby) > > val |= EDP_PSR_LINK_STANDBY; > >- I915_WRITE(EDP_PSR_CTL, val | > >- max_sleep_time << EDP_PSR_MAX_SLEEP_TIME_SHIFT | > >- idle_frames << EDP_PSR_IDLE_FRAME_SHIFT | > >- EDP_PSR_ENABLE); > >+ if (dev_priv->vbt.psr.tp1_wakeup_time > 5) > >+ val |= EDP_PSR_TP1_TIME_2500us; > >+ else if (dev_priv->vbt.psr.tp1_wakeup_time > 1) > >+ val |= EDP_PSR_TP1_TIME_500us; > >+ else if (dev_priv->vbt.psr.tp1_wakeup_time > 0) > >+ val |= EDP_PSR_TP1_TIME_100us; > >+ else > >+ val |= EDP_PSR_TP1_TIME_0us; > >+ > >+ if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5) > >+ val |= EDP_PSR_TP2_TP3_TIME_2500us; > >+ else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1) > >+ val |= EDP_PSR_TP2_TP3_TIME_500us; > >+ else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0) > >+ val |= EDP_PSR_TP2_TP3_TIME_100us; > >+ else > >+ val |= EDP_PSR_TP2_TP3_TIME_0us; > >+ > >+ if (intel_dp_source_supports_hbr2(intel_dp) && > >+ drm_dp_tps3_supported(intel_dp->dpcd)) > >+ val |= EDP_PSR_TP1_TP3_SEL; > >+ else > >+ val |= EDP_PSR_TP1_TP2_SEL; > >+ > >+ I915_WRITE(EDP_PSR_CTL, val); > >+ > >+ if (!dev_priv->psr.psr2_support) > >+ return; > >+ > >+ /* FIXME: selective update is probably totally broken because it doesn't > >+ * mesh at all with our frontbuffer tracking. And the hw alone isn't > >+ * good enough. */ > >+ val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE; > >+ > >+ if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5) > >+ val |= EDP_PSR2_TP2_TIME_2500; > >+ else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 1) > >+ val |= EDP_PSR2_TP2_TIME_500; > >+ else if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 0) > >+ val |= EDP_PSR2_TP2_TIME_100; > >+ else > >+ val |= EDP_PSR2_TP2_TIME_50; > >- if (dev_priv->psr.psr2_support) > >- I915_WRITE(EDP_PSR2_CTL, EDP_PSR2_ENABLE | > >- EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100); > >+ I915_WRITE(EDP_PSR2_CTL, val); > > } > > static bool intel_psr_match_conditions(struct intel_dp *intel_dp) > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-20 7:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1463590036-17824-1-git-send-email-daniel.vetter@ffwll.ch>
2016-05-18 16:47 ` [PATCH 2/7] drm/i915/psr: Try to program link training times correctly Daniel Vetter
2016-05-18 17:39 ` [Intel-gfx] " Ville Syrjälä
2016-05-18 18:04 ` Daniel Vetter
2016-05-18 18:09 ` Ville Syrjälä
[not found] ` <573D9A5A.4010004@intel.com>
2016-05-20 7:33 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox