* [PATCH] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling @ 2015-11-25 15:11 ville.syrjala 2015-11-25 15:17 ` Ville Syrjälä 2015-11-26 20:55 ` [PATCH 1/2] drm/i915: Add HAS_PCH_LPT_H() ville.syrjala 0 siblings, 2 replies; 7+ messages in thread From: ville.syrjala @ 2015-11-25 15:11 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> LPT/WPT only have transcoder A, so we shouldn't look at FIFO underruns for transocoder B/C. And more importnatnly we should not consider the state of underrun reporting for transcoders B/C when checking whether we can enable the south error interrupt. The whole thing is a bit of mess since we store the underrun reporting state for transcoder A under intel_crtc for pipe A, irrespective of which pipe may actually be driving the transcoder. But I figured trying to change that would result in more churn. Caveat: totally untested due to lack of hw, but might explain some of the HSW/BDW BAT fails... Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_fifo_underrun.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c index bda526660e20..04bf625a1b6c 100644 --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c @@ -48,6 +48,13 @@ * The code also supports underrun detection on the PCH transcoder. */ +static bool cpt_transcoder_exists(struct drm_i915_private *dev_priv, + enum transcoder pch_transcoder) +{ + /* HSW/BDW have only transcoder A */ + return !HAS_DDI(dev_priv) || pch_transcoder == TRANSCODER_A; +} + static bool ivb_can_enable_err_int(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -69,13 +76,16 @@ static bool ivb_can_enable_err_int(struct drm_device *dev) static bool cpt_can_enable_serr_int(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - enum pipe pipe; - struct intel_crtc *crtc; + enum transcoder pch_transcoder; assert_spin_locked(&dev_priv->irq_lock); - for_each_pipe(dev_priv, pipe) { - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); + for_each_pipe(dev_priv, pch_transcoder) { + struct intel_crtc *crtc = + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pch_transcoder]); + + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) + continue; if (crtc->pch_fifo_underrun_disabled) return false; @@ -206,6 +216,9 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc *crtc) assert_spin_locked(&dev_priv->irq_lock); + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) + return; + if ((serr_int & SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) == 0) return; @@ -222,6 +235,9 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev, { struct drm_i915_private *dev_priv = dev->dev_private; + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) + return; + if (enable) { I915_WRITE(SERR_INT, SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)); -- 2.4.10 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling 2015-11-25 15:11 [PATCH] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling ville.syrjala @ 2015-11-25 15:17 ` Ville Syrjälä 2015-11-26 20:55 ` [PATCH 1/2] drm/i915: Add HAS_PCH_LPT_H() ville.syrjala 1 sibling, 0 replies; 7+ messages in thread From: Ville Syrjälä @ 2015-11-25 15:17 UTC (permalink / raw) To: intel-gfx On Wed, Nov 25, 2015 at 05:11:23PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > LPT/WPT only have transcoder A, so we shouldn't look at FIFO underruns > for transocoder B/C. And more importnatnly we should not consider > the state of underrun reporting for transcoders B/C when checking > whether we can enable the south error interrupt. > > The whole thing is a bit of mess since we store the underrun reporting > state for transcoder A under intel_crtc for pipe A, irrespective of > which pipe may actually be driving the transcoder. But I figured trying > to change that would result in more churn. > > Caveat: totally untested due to lack of hw, but might explain some of > the HSW/BDW BAT fails... Hmm. Actually it can't since we don't use the CRCs that get signalled via the south error interrupt. Back to the drawing board... > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_fifo_underrun.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c > index bda526660e20..04bf625a1b6c 100644 > --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c > +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c > @@ -48,6 +48,13 @@ > * The code also supports underrun detection on the PCH transcoder. > */ > > +static bool cpt_transcoder_exists(struct drm_i915_private *dev_priv, > + enum transcoder pch_transcoder) > +{ > + /* HSW/BDW have only transcoder A */ > + return !HAS_DDI(dev_priv) || pch_transcoder == TRANSCODER_A; > +} > + > static bool ivb_can_enable_err_int(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -69,13 +76,16 @@ static bool ivb_can_enable_err_int(struct drm_device *dev) > static bool cpt_can_enable_serr_int(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > - enum pipe pipe; > - struct intel_crtc *crtc; > + enum transcoder pch_transcoder; > > assert_spin_locked(&dev_priv->irq_lock); > > - for_each_pipe(dev_priv, pipe) { > - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > + for_each_pipe(dev_priv, pch_transcoder) { > + struct intel_crtc *crtc = > + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pch_transcoder]); > + > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > + continue; > > if (crtc->pch_fifo_underrun_disabled) > return false; > @@ -206,6 +216,9 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc *crtc) > > assert_spin_locked(&dev_priv->irq_lock); > > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > + return; > + > if ((serr_int & SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) == 0) > return; > > @@ -222,6 +235,9 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev, > { > struct drm_i915_private *dev_priv = dev->dev_private; > > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > + return; > + > if (enable) { > I915_WRITE(SERR_INT, > SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)); > -- > 2.4.10 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] drm/i915: Add HAS_PCH_LPT_H() 2015-11-25 15:11 [PATCH] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling ville.syrjala 2015-11-25 15:17 ` Ville Syrjälä @ 2015-11-26 20:55 ` ville.syrjala 2015-11-26 20:55 ` [PATCH v2 2/2] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling ville.syrjala 1 sibling, 1 reply; 7+ messages in thread From: ville.syrjala @ 2015-11-26 20:55 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> We have HAS_PCH_LPT_LP() already, so add HAS_PCH_LPT_H() and use it where appropriate. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_dp.c | 2 +- drivers/gpu/drm/i915/intel_panel.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bc865e234df2..9ab3e25ddf38 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2615,6 +2615,7 @@ struct drm_i915_cmd_table { #define HAS_PCH_SPT(dev) (INTEL_PCH_TYPE(dev) == PCH_SPT) #define HAS_PCH_LPT(dev) (INTEL_PCH_TYPE(dev) == PCH_LPT) #define HAS_PCH_LPT_LP(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) +#define HAS_PCH_LPT_H(dev) (__I915__(dev)->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT) #define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX) #define HAS_PCH_NOP(dev) (INTEL_PCH_TYPE(dev) == PCH_NOP) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 634dba423ae9..644e2fc9c67d 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -711,7 +711,7 @@ static uint32_t hsw_get_aux_clock_divider(struct intel_dp *intel_dp, int index) if (index) return 0; return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000); - } else if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) { + } else if (HAS_PCH_LPT_H(dev_priv)) { /* Workaround for non-ULT HSW */ switch (index) { case 0: return 63; diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index a24df35e11e7..d4c81df318b6 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1300,7 +1300,7 @@ static u32 lpt_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz) else mul = 128; - if (dev_priv->pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) + if (HAS_PCH_LPT_H(dev_priv)) clock = MHz(135); /* LPT:H */ else clock = MHz(24); /* LPT:LP */ -- 2.4.10 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling 2015-11-26 20:55 ` [PATCH 1/2] drm/i915: Add HAS_PCH_LPT_H() ville.syrjala @ 2015-11-26 20:55 ` ville.syrjala 2015-11-30 8:25 ` Daniel Vetter 0 siblings, 1 reply; 7+ messages in thread From: ville.syrjala @ 2015-11-26 20:55 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> LPT/WPT only have transcoder A, so we shouldn't look at FIFO underruns for transocoder B/C. And more importnatnly we should not consider the state of underrun reporting for transcoders B/C when checking whether we can enable the south error interrupt. The whole thing is a bit of mess since we store the underrun reporting state for transcoder A under intel_crtc for pipe A, irrespective of which pipe may actually be driving the transcoder. But I figured trying to change that would result in more churn. Caveat: Still untested v2: Use HAS_PCH_LPT_H instead of HAS_DDI Use cpt_check_pch_fifo_underruns() on LPT-H/WPT-H too Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_fifo_underrun.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c index bda526660e20..3d3acc8b8367 100644 --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c @@ -48,6 +48,14 @@ * The code also supports underrun detection on the PCH transcoder. */ +static bool cpt_transcoder_exists(struct drm_i915_private *dev_priv, + enum transcoder pch_transcoder) +{ + /* LPT-H/WPT-H have only transcoder A */ + return HAS_PCH_CPT(dev_priv) || + (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == TRANSCODER_A); +} + static bool ivb_can_enable_err_int(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -69,13 +77,16 @@ static bool ivb_can_enable_err_int(struct drm_device *dev) static bool cpt_can_enable_serr_int(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - enum pipe pipe; - struct intel_crtc *crtc; + enum transcoder pch_transcoder; assert_spin_locked(&dev_priv->irq_lock); - for_each_pipe(dev_priv, pipe) { - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); + for_each_pipe(dev_priv, pch_transcoder) { + struct intel_crtc *crtc = + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pch_transcoder]); + + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) + continue; if (crtc->pch_fifo_underrun_disabled) return false; @@ -206,6 +217,9 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc *crtc) assert_spin_locked(&dev_priv->irq_lock); + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) + return; + if ((serr_int & SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) == 0) return; @@ -222,6 +236,9 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev, { struct drm_i915_private *dev_priv = dev->dev_private; + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) + return; + if (enable) { I915_WRITE(SERR_INT, SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)); @@ -436,7 +453,7 @@ void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv) if (crtc->pch_fifo_underrun_disabled) continue; - if (HAS_PCH_CPT(dev_priv)) + if (HAS_PCH_CPT(dev_priv) || HAS_PCH_LPT_H(dev_priv)) cpt_check_pch_fifo_underruns(crtc); } -- 2.4.10 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling 2015-11-26 20:55 ` [PATCH v2 2/2] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling ville.syrjala @ 2015-11-30 8:25 ` Daniel Vetter 2015-11-30 14:08 ` Ville Syrjälä 0 siblings, 1 reply; 7+ messages in thread From: Daniel Vetter @ 2015-11-30 8:25 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx On Thu, Nov 26, 2015 at 10:55:53PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > LPT/WPT only have transcoder A, so we shouldn't look at FIFO underruns > for transocoder B/C. And more importnatnly we should not consider > the state of underrun reporting for transcoders B/C when checking > whether we can enable the south error interrupt. > > The whole thing is a bit of mess since we store the underrun reporting > state for transcoder A under intel_crtc for pipe A, irrespective of > which pipe may actually be driving the transcoder. But I figured trying > to change that would result in more churn. > > Caveat: Still untested > > v2: Use HAS_PCH_LPT_H instead of HAS_DDI > Use cpt_check_pch_fifo_underruns() on LPT-H/WPT-H too > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> I've battled hsw lpt fifo underrun issues last week and never seen a fifo underrun on pipe B/C. Have you seen them anywhere really? If not I think we can skip this patch here, since I think I tracked it all down. -Daniel > --- > drivers/gpu/drm/i915/intel_fifo_underrun.c | 27 ++++++++++++++++++++++----- > 1 file changed, 22 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c > index bda526660e20..3d3acc8b8367 100644 > --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c > +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c > @@ -48,6 +48,14 @@ > * The code also supports underrun detection on the PCH transcoder. > */ > > +static bool cpt_transcoder_exists(struct drm_i915_private *dev_priv, > + enum transcoder pch_transcoder) > +{ > + /* LPT-H/WPT-H have only transcoder A */ > + return HAS_PCH_CPT(dev_priv) || > + (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == TRANSCODER_A); > +} > + > static bool ivb_can_enable_err_int(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -69,13 +77,16 @@ static bool ivb_can_enable_err_int(struct drm_device *dev) > static bool cpt_can_enable_serr_int(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > - enum pipe pipe; > - struct intel_crtc *crtc; > + enum transcoder pch_transcoder; > > assert_spin_locked(&dev_priv->irq_lock); > > - for_each_pipe(dev_priv, pipe) { > - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > + for_each_pipe(dev_priv, pch_transcoder) { > + struct intel_crtc *crtc = > + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pch_transcoder]); > + > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > + continue; > > if (crtc->pch_fifo_underrun_disabled) > return false; > @@ -206,6 +217,9 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc *crtc) > > assert_spin_locked(&dev_priv->irq_lock); > > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > + return; > + > if ((serr_int & SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) == 0) > return; > > @@ -222,6 +236,9 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev, > { > struct drm_i915_private *dev_priv = dev->dev_private; > > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > + return; > + > if (enable) { > I915_WRITE(SERR_INT, > SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)); > @@ -436,7 +453,7 @@ void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv) > if (crtc->pch_fifo_underrun_disabled) > continue; > > - if (HAS_PCH_CPT(dev_priv)) > + if (HAS_PCH_CPT(dev_priv) || HAS_PCH_LPT_H(dev_priv)) > cpt_check_pch_fifo_underruns(crtc); > } > > -- > 2.4.10 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation 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] 7+ messages in thread
* Re: [PATCH v2 2/2] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling 2015-11-30 8:25 ` Daniel Vetter @ 2015-11-30 14:08 ` Ville Syrjälä 2015-11-30 14:16 ` Daniel Vetter 0 siblings, 1 reply; 7+ messages in thread From: Ville Syrjälä @ 2015-11-30 14:08 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Mon, Nov 30, 2015 at 09:25:03AM +0100, Daniel Vetter wrote: > On Thu, Nov 26, 2015 at 10:55:53PM +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > LPT/WPT only have transcoder A, so we shouldn't look at FIFO underruns > > for transocoder B/C. And more importnatnly we should not consider > > the state of underrun reporting for transcoders B/C when checking > > whether we can enable the south error interrupt. > > > > The whole thing is a bit of mess since we store the underrun reporting > > state for transcoder A under intel_crtc for pipe A, irrespective of > > which pipe may actually be driving the transcoder. But I figured trying > > to change that would result in more churn. > > > > Caveat: Still untested > > > > v2: Use HAS_PCH_LPT_H instead of HAS_DDI > > Use cpt_check_pch_fifo_underruns() on LPT-H/WPT-H too > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > I've battled hsw lpt fifo underrun issues last week and never seen a fifo > underrun on pipe B/C. Have you seen them anywhere really? No, I've not seen it. We mark underrun detection as disabled for active pipes in intel_sanitize_crtc(), and we never turn it back on except for transcoder A, so I'm thinking we'll never actually enable the south error interrup on LPT if a non-pch port is enabled on boot. Note that so far I didn't check if that's actually the case, but based on the code I can't come to any other conclusion. > > If not I think we can skip this patch here, since I think I tracked it all > down. > -Daniel > > > --- > > drivers/gpu/drm/i915/intel_fifo_underrun.c | 27 ++++++++++++++++++++++----- > > 1 file changed, 22 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c > > index bda526660e20..3d3acc8b8367 100644 > > --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c > > +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c > > @@ -48,6 +48,14 @@ > > * The code also supports underrun detection on the PCH transcoder. > > */ > > > > +static bool cpt_transcoder_exists(struct drm_i915_private *dev_priv, > > + enum transcoder pch_transcoder) > > +{ > > + /* LPT-H/WPT-H have only transcoder A */ > > + return HAS_PCH_CPT(dev_priv) || > > + (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == TRANSCODER_A); > > +} > > + > > static bool ivb_can_enable_err_int(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > @@ -69,13 +77,16 @@ static bool ivb_can_enable_err_int(struct drm_device *dev) > > static bool cpt_can_enable_serr_int(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > - enum pipe pipe; > > - struct intel_crtc *crtc; > > + enum transcoder pch_transcoder; > > > > assert_spin_locked(&dev_priv->irq_lock); > > > > - for_each_pipe(dev_priv, pipe) { > > - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > > + for_each_pipe(dev_priv, pch_transcoder) { > > + struct intel_crtc *crtc = > > + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pch_transcoder]); > > + > > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > > + continue; > > > > if (crtc->pch_fifo_underrun_disabled) > > return false; > > @@ -206,6 +217,9 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc *crtc) > > > > assert_spin_locked(&dev_priv->irq_lock); > > > > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > > + return; > > + > > if ((serr_int & SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) == 0) > > return; > > > > @@ -222,6 +236,9 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev, > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > > > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > > + return; > > + > > if (enable) { > > I915_WRITE(SERR_INT, > > SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)); > > @@ -436,7 +453,7 @@ void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv) > > if (crtc->pch_fifo_underrun_disabled) > > continue; > > > > - if (HAS_PCH_CPT(dev_priv)) > > + if (HAS_PCH_CPT(dev_priv) || HAS_PCH_LPT_H(dev_priv)) > > cpt_check_pch_fifo_underruns(crtc); > > } > > > > -- > > 2.4.10 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling 2015-11-30 14:08 ` Ville Syrjälä @ 2015-11-30 14:16 ` Daniel Vetter 0 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2015-11-30 14:16 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On Mon, Nov 30, 2015 at 04:08:34PM +0200, Ville Syrjälä wrote: > On Mon, Nov 30, 2015 at 09:25:03AM +0100, Daniel Vetter wrote: > > On Thu, Nov 26, 2015 at 10:55:53PM +0200, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > LPT/WPT only have transcoder A, so we shouldn't look at FIFO underruns > > > for transocoder B/C. And more importnatnly we should not consider > > > the state of underrun reporting for transcoders B/C when checking > > > whether we can enable the south error interrupt. > > > > > > The whole thing is a bit of mess since we store the underrun reporting > > > state for transcoder A under intel_crtc for pipe A, irrespective of > > > which pipe may actually be driving the transcoder. But I figured trying > > > to change that would result in more churn. > > > > > > Caveat: Still untested > > > > > > v2: Use HAS_PCH_LPT_H instead of HAS_DDI > > > Use cpt_check_pch_fifo_underruns() on LPT-H/WPT-H too > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > I've battled hsw lpt fifo underrun issues last week and never seen a fifo > > underrun on pipe B/C. Have you seen them anywhere really? > > No, I've not seen it. We mark underrun detection as disabled for active > pipes in intel_sanitize_crtc(), and we never turn it back on except for > transcoder A, so I'm thinking we'll never actually enable the south > error interrup on LPT if a non-pch port is enabled on boot. Hm right this is broken for pch ... I guess we need an IS_HSW(dev) : PCH_A : pipe in there to get this right. I do get plenty pch fifo underruns here, but I guess my bios boots with vga on pipe A and hence I get away with this bug ;-) > Note that so far I didn't check if that's actually the case, but based > on the code I can't come to any other conclusion. Yeah seems indeed broken, but I think simpler to fix in the crtc sanitize code. -Daniel > > > > > If not I think we can skip this patch here, since I think I tracked it all > > down. > > -Daniel > > > > > --- > > > drivers/gpu/drm/i915/intel_fifo_underrun.c | 27 ++++++++++++++++++++++----- > > > 1 file changed, 22 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c > > > index bda526660e20..3d3acc8b8367 100644 > > > --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c > > > +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c > > > @@ -48,6 +48,14 @@ > > > * The code also supports underrun detection on the PCH transcoder. > > > */ > > > > > > +static bool cpt_transcoder_exists(struct drm_i915_private *dev_priv, > > > + enum transcoder pch_transcoder) > > > +{ > > > + /* LPT-H/WPT-H have only transcoder A */ > > > + return HAS_PCH_CPT(dev_priv) || > > > + (HAS_PCH_LPT_H(dev_priv) && pch_transcoder == TRANSCODER_A); > > > +} > > > + > > > static bool ivb_can_enable_err_int(struct drm_device *dev) > > > { > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > @@ -69,13 +77,16 @@ static bool ivb_can_enable_err_int(struct drm_device *dev) > > > static bool cpt_can_enable_serr_int(struct drm_device *dev) > > > { > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > - enum pipe pipe; > > > - struct intel_crtc *crtc; > > > + enum transcoder pch_transcoder; > > > > > > assert_spin_locked(&dev_priv->irq_lock); > > > > > > - for_each_pipe(dev_priv, pipe) { > > > - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > > > + for_each_pipe(dev_priv, pch_transcoder) { > > > + struct intel_crtc *crtc = > > > + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pch_transcoder]); > > > + > > > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > > > + continue; > > > > > > if (crtc->pch_fifo_underrun_disabled) > > > return false; > > > @@ -206,6 +217,9 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc *crtc) > > > > > > assert_spin_locked(&dev_priv->irq_lock); > > > > > > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > > > + return; > > > + > > > if ((serr_int & SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) == 0) > > > return; > > > > > > @@ -222,6 +236,9 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev, > > > { > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > > > > + if (!cpt_transcoder_exists(dev_priv, pch_transcoder)) > > > + return; > > > + > > > if (enable) { > > > I915_WRITE(SERR_INT, > > > SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)); > > > @@ -436,7 +453,7 @@ void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv) > > > if (crtc->pch_fifo_underrun_disabled) > > > continue; > > > > > > - if (HAS_PCH_CPT(dev_priv)) > > > + if (HAS_PCH_CPT(dev_priv) || HAS_PCH_LPT_H(dev_priv)) > > > cpt_check_pch_fifo_underruns(crtc); > > > } > > > > > > -- > > > 2.4.10 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > Ville Syrjälä > Intel OTC -- Daniel Vetter Software Engineer, Intel Corporation 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] 7+ messages in thread
end of thread, other threads:[~2015-11-30 14:16 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-11-25 15:11 [PATCH] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling ville.syrjala 2015-11-25 15:17 ` Ville Syrjälä 2015-11-26 20:55 ` [PATCH 1/2] drm/i915: Add HAS_PCH_LPT_H() ville.syrjala 2015-11-26 20:55 ` [PATCH v2 2/2] drm/i915: Ignore transcoder B/C on LPT/WPT FIFO underrun handling ville.syrjala 2015-11-30 8:25 ` Daniel Vetter 2015-11-30 14:08 ` Ville Syrjälä 2015-11-30 14:16 ` 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.