* [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock @ 2014-03-21 9:00 Jani Nikula 2014-03-21 9:29 ` Chris Wilson 2014-03-21 10:19 ` Daniel Vetter 0 siblings, 2 replies; 18+ messages in thread From: Jani Nikula @ 2014-03-21 9:00 UTC (permalink / raw) To: intel-gfx; +Cc: jani.nikula From: Ville Syrjälä <ville.syrjala@linux.intel.com> On SNB the BIOS provided WM memory latency values seem insufficient to handle high resolution displays. In this particular case the display mode was a 2560x1440@60Hz, which makes the pixel clock 241.5 MHz. It was empirically found that a memory latency value if 1.2 usec is enough to avoid underruns, whereas the BIOS provided value of 0.7 usec was clearly too low. Incidentally 1.2 usec is what the typical BIOS provided values are on IVB systems. Increase the WM memory latency values to 1.2 usec when encountering a display mode with pixel clock exceeding 225 MHz. 225 MHz was chosen as the threshold simply because that's the documented limit of SNB with HDMI, so one might surmise that the hardware may have been tested up to that frequency. In theory the latency shouldn't depend on the pixel clock at all. So it may be that we should just increase the latency values across the board for all SNB systems. But for now I'm inclined to limit this quirk to only those cases that are proven to need it, as doing otherwise might cause some increase in power consumption for everyone. Cc: Robert N <crshman@gmail.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- I'm just relaying the patch from bugzilla to the list as it seems to fix the bug. Don't ask me anything about it. BR, Jani. --- drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++ drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0b19afdfbaa7..b0ac92d8f461 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9933,6 +9933,16 @@ static int __intel_set_mode(struct drm_crtc *crtc, */ drm_calc_timestamping_constants(crtc, &pipe_config->adjusted_mode); + + + /* + * The BIOS provided WM memory latency values are often + * inadequate for high resolution displays. Adjust them. + * + * FIXME not sure 225MHz is a good threshold. + */ + if (IS_GEN6(dev) && pipe_config->adjusted_mode.crtc_clock > 225000) + snb_wm_latency_quirk(dev); } /* Only after disabling all output pipelines that will be changed can we diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 60ffad376390..b1d631c9dfa5 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -903,6 +903,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv); void intel_init_runtime_pm(struct drm_i915_private *dev_priv); void intel_fini_runtime_pm(struct drm_i915_private *dev_priv); void ilk_wm_get_hw_state(struct drm_device *dev); +void snb_wm_latency_quirk(struct drm_device *dev); /* intel_sdvo.c */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 39f3238bf1c3..226750452637 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2104,6 +2104,39 @@ static void ilk_setup_wm_latency(struct drm_device *dev) intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency); } +static bool ilk_increase_wm_latency(struct drm_i915_private *dev_priv, + uint16_t wm[5], uint16_t min) +{ + int level, max_level = ilk_wm_max_level(dev_priv->dev); + + if (wm[0] >= min) + return false; + + wm[0] = max(wm[0], min); + for (level = 1; level <= max_level; level++) + wm[level] = max_t(uint16_t, wm[level], DIV_ROUND_UP(min, 5)); + + return true; +} + +void snb_wm_latency_quirk(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + bool changed; + + changed = ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12) | + ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12) | + ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); + + if (!changed) + return; + + DRM_DEBUG_KMS("WM latency values increased due to high pixel clock\n"); + intel_print_wm_latency(dev, "Primary", dev_priv->wm.pri_latency); + intel_print_wm_latency(dev, "Sprite", dev_priv->wm.spr_latency); + intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency); +} + static void ilk_compute_wm_parameters(struct drm_crtc *crtc, struct ilk_pipe_wm_parameters *p, struct intel_wm_config *config) -- 1.7.9.5 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock 2014-03-21 9:00 [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock Jani Nikula @ 2014-03-21 9:29 ` Chris Wilson 2014-03-21 10:19 ` Daniel Vetter 1 sibling, 0 replies; 18+ messages in thread From: Chris Wilson @ 2014-03-21 9:29 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx On Fri, Mar 21, 2014 at 11:00:48AM +0200, Jani Nikula wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > On SNB the BIOS provided WM memory latency values seem insufficient to > handle high resolution displays. > > In this particular case the display mode was a 2560x1440@60Hz, which > makes the pixel clock 241.5 MHz. It was empirically found that a memory > latency value if 1.2 usec is enough to avoid underruns, whereas the BIOS > provided value of 0.7 usec was clearly too low. Incidentally 1.2 usec > is what the typical BIOS provided values are on IVB systems. > > Increase the WM memory latency values to 1.2 usec when encountering a > display mode with pixel clock exceeding 225 MHz. 225 MHz was chosen as > the threshold simply because that's the documented limit of SNB with > HDMI, so one might surmise that the hardware may have been tested up > to that frequency. In theory the latency shouldn't depend on the pixel > clock at all. So it may be that we should just increase the latency > values across the board for all SNB systems. But for now I'm inclined > to limit this quirk to only those cases that are proven to need it, > as doing otherwise might cause some increase in power consumption for > everyone. > > Cc: Robert N <crshman@gmail.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254 > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > I'm just relaying the patch from bugzilla to the list as it seems to fix > the bug. Don't ask me anything about it. > > BR, > Jani. > --- > drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_pm.c | 33 +++++++++++++++++++++++++++++++++ > 3 files changed, 44 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 0b19afdfbaa7..b0ac92d8f461 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9933,6 +9933,16 @@ static int __intel_set_mode(struct drm_crtc *crtc, > */ > drm_calc_timestamping_constants(crtc, > &pipe_config->adjusted_mode); > + > + > + /* > + * The BIOS provided WM memory latency values are often > + * inadequate for high resolution displays. Adjust them. > + * > + * FIXME not sure 225MHz is a good threshold. > + */ > + if (IS_GEN6(dev) && pipe_config->adjusted_mode.crtc_clock > 225000) > + snb_wm_latency_quirk(dev); > } > > /* Only after disabling all output pipelines that will be changed can we > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 60ffad376390..b1d631c9dfa5 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -903,6 +903,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv); > void intel_init_runtime_pm(struct drm_i915_private *dev_priv); > void intel_fini_runtime_pm(struct drm_i915_private *dev_priv); > void ilk_wm_get_hw_state(struct drm_device *dev); > +void snb_wm_latency_quirk(struct drm_device *dev); > > > /* intel_sdvo.c */ > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 39f3238bf1c3..226750452637 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2104,6 +2104,39 @@ static void ilk_setup_wm_latency(struct drm_device *dev) > intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency); > } > > +static bool ilk_increase_wm_latency(struct drm_i915_private *dev_priv, > + uint16_t wm[5], uint16_t min) > +{ > + int level, max_level = ilk_wm_max_level(dev_priv->dev); > + > + if (wm[0] >= min) > + return false; > + > + wm[0] = max(wm[0], min); > + for (level = 1; level <= max_level; level++) > + wm[level] = max_t(uint16_t, wm[level], DIV_ROUND_UP(min, 5)); > + > + return true; > +} > + > +void snb_wm_latency_quirk(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + bool changed; > + > + changed = ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12) | > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12) | > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); Unless I missed something this hereafter permanently bumps the latency for computations. At the 225MHz pixel clock, what are the differences in final WM values? -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock 2014-03-21 9:00 [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock Jani Nikula 2014-03-21 9:29 ` Chris Wilson @ 2014-03-21 10:19 ` Daniel Vetter 2014-03-21 17:28 ` Runyan, Arthur J 2014-05-08 12:09 ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB ville.syrjala 1 sibling, 2 replies; 18+ messages in thread From: Daniel Vetter @ 2014-03-21 10:19 UTC (permalink / raw) To: Jani Nikula, Arthur Ranyan, Syrjala, Ville; +Cc: intel-gfx On Fri, Mar 21, 2014 at 11:00:48AM +0200, Jani Nikula wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > On SNB the BIOS provided WM memory latency values seem insufficient to > handle high resolution displays. > > In this particular case the display mode was a 2560x1440@60Hz, which > makes the pixel clock 241.5 MHz. It was empirically found that a memory > latency value if 1.2 usec is enough to avoid underruns, whereas the BIOS > provided value of 0.7 usec was clearly too low. Incidentally 1.2 usec > is what the typical BIOS provided values are on IVB systems. > > Increase the WM memory latency values to 1.2 usec when encountering a > display mode with pixel clock exceeding 225 MHz. 225 MHz was chosen as > the threshold simply because that's the documented limit of SNB with > HDMI, so one might surmise that the hardware may have been tested up > to that frequency. In theory the latency shouldn't depend on the pixel > clock at all. So it may be that we should just increase the latency > values across the board for all SNB systems. But for now I'm inclined > to limit this quirk to only those cases that are proven to need it, > as doing otherwise might cause some increase in power consumption for > everyone. > > Cc: Robert N <crshman@gmail.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254 > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> I think we should just unconditionally increase the memory latency. ivb has a rather similar memory subsystem compared to snb, so just increasing it always to this limits sounds ok. For lower resolutions we might simply get away because there's more excess bandwidth available. Maybe Art can shed some light on this. -Daniel > > --- > > I'm just relaying the patch from bugzilla to the list as it seems to fix > the bug. Don't ask me anything about it. > > BR, > Jani. > --- > drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_pm.c | 33 +++++++++++++++++++++++++++++++++ > 3 files changed, 44 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 0b19afdfbaa7..b0ac92d8f461 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9933,6 +9933,16 @@ static int __intel_set_mode(struct drm_crtc *crtc, > */ > drm_calc_timestamping_constants(crtc, > &pipe_config->adjusted_mode); > + > + > + /* > + * The BIOS provided WM memory latency values are often > + * inadequate for high resolution displays. Adjust them. > + * > + * FIXME not sure 225MHz is a good threshold. > + */ > + if (IS_GEN6(dev) && pipe_config->adjusted_mode.crtc_clock > 225000) > + snb_wm_latency_quirk(dev); > } > > /* Only after disabling all output pipelines that will be changed can we > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 60ffad376390..b1d631c9dfa5 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -903,6 +903,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv); > void intel_init_runtime_pm(struct drm_i915_private *dev_priv); > void intel_fini_runtime_pm(struct drm_i915_private *dev_priv); > void ilk_wm_get_hw_state(struct drm_device *dev); > +void snb_wm_latency_quirk(struct drm_device *dev); > > > /* intel_sdvo.c */ > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 39f3238bf1c3..226750452637 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2104,6 +2104,39 @@ static void ilk_setup_wm_latency(struct drm_device *dev) > intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency); > } > > +static bool ilk_increase_wm_latency(struct drm_i915_private *dev_priv, > + uint16_t wm[5], uint16_t min) > +{ > + int level, max_level = ilk_wm_max_level(dev_priv->dev); > + > + if (wm[0] >= min) > + return false; > + > + wm[0] = max(wm[0], min); > + for (level = 1; level <= max_level; level++) > + wm[level] = max_t(uint16_t, wm[level], DIV_ROUND_UP(min, 5)); > + > + return true; > +} > + > +void snb_wm_latency_quirk(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + bool changed; > + > + changed = ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12) | > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12) | > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); > + > + if (!changed) > + return; > + > + DRM_DEBUG_KMS("WM latency values increased due to high pixel clock\n"); > + intel_print_wm_latency(dev, "Primary", dev_priv->wm.pri_latency); > + intel_print_wm_latency(dev, "Sprite", dev_priv->wm.spr_latency); > + intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency); > +} > + > static void ilk_compute_wm_parameters(struct drm_crtc *crtc, > struct ilk_pipe_wm_parameters *p, > struct intel_wm_config *config) > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock 2014-03-21 10:19 ` Daniel Vetter @ 2014-03-21 17:28 ` Runyan, Arthur J 2014-03-31 18:29 ` Robert Navarro 2014-05-08 12:09 ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB ville.syrjala 1 sibling, 1 reply; 18+ messages in thread From: Runyan, Arthur J @ 2014-03-21 17:28 UTC (permalink / raw) To: Daniel Vetter, Nikula, Jani, Syrjala, Ville; +Cc: intel-gfx Please check the DRAM configuration for the systems that fail. The higher latency is more likely with higher tRFC which is mainly found with 8 Gbit components. >-----Original Message----- >From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter >Sent: Friday, March 21, 2014 3:19 AM >To: Nikula, Jani; Runyan, Arthur J; Syrjala, Ville >Cc: intel-gfx >Subject: Re: [Intel-gfx] [PATCH] drm/i915: Increase WM memory latency values on SNB with >high pixel clock > >On Fri, Mar 21, 2014 at 11:00:48AM +0200, Jani Nikula wrote: >> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> On SNB the BIOS provided WM memory latency values seem insufficient to >> handle high resolution displays. >> >> In this particular case the display mode was a 2560x1440@60Hz, which >> makes the pixel clock 241.5 MHz. It was empirically found that a memory >> latency value if 1.2 usec is enough to avoid underruns, whereas the BIOS >> provided value of 0.7 usec was clearly too low. Incidentally 1.2 usec >> is what the typical BIOS provided values are on IVB systems. >> >> Increase the WM memory latency values to 1.2 usec when encountering a >> display mode with pixel clock exceeding 225 MHz. 225 MHz was chosen as >> the threshold simply because that's the documented limit of SNB with >> HDMI, so one might surmise that the hardware may have been tested up >> to that frequency. In theory the latency shouldn't depend on the pixel >> clock at all. So it may be that we should just increase the latency >> values across the board for all SNB systems. But for now I'm inclined >> to limit this quirk to only those cases that are proven to need it, >> as doing otherwise might cause some increase in power consumption for >> everyone. >> >> Cc: Robert N <crshman@gmail.com> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254 >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >I think we should just unconditionally increase the memory latency. ivb >has a rather similar memory subsystem compared to snb, so just increasing >it always to this limits sounds ok. For lower resolutions we might simply >get away because there's more excess bandwidth available. > >Maybe Art can shed some light on this. >-Daniel > >> >> --- >> >> I'm just relaying the patch from bugzilla to the list as it seems to fix >> the bug. Don't ask me anything about it. >> >> BR, >> Jani. >> --- >> drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++ >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> drivers/gpu/drm/i915/intel_pm.c | 33 +++++++++++++++++++++++++++++++++ >> 3 files changed, 44 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 0b19afdfbaa7..b0ac92d8f461 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -9933,6 +9933,16 @@ static int __intel_set_mode(struct drm_crtc *crtc, >> */ >> drm_calc_timestamping_constants(crtc, >> &pipe_config->adjusted_mode); >> + >> + >> + /* >> + * The BIOS provided WM memory latency values are often >> + * inadequate for high resolution displays. Adjust them. >> + * >> + * FIXME not sure 225MHz is a good threshold. >> + */ >> + if (IS_GEN6(dev) && pipe_config->adjusted_mode.crtc_clock > 225000) >> + snb_wm_latency_quirk(dev); >> } >> >> /* Only after disabling all output pipelines that will be changed can we >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 60ffad376390..b1d631c9dfa5 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -903,6 +903,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv); >> void intel_init_runtime_pm(struct drm_i915_private *dev_priv); >> void intel_fini_runtime_pm(struct drm_i915_private *dev_priv); >> void ilk_wm_get_hw_state(struct drm_device *dev); >> +void snb_wm_latency_quirk(struct drm_device *dev); >> >> >> /* intel_sdvo.c */ >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index 39f3238bf1c3..226750452637 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -2104,6 +2104,39 @@ static void ilk_setup_wm_latency(struct drm_device *dev) >> intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency); >> } >> >> +static bool ilk_increase_wm_latency(struct drm_i915_private *dev_priv, >> + uint16_t wm[5], uint16_t min) >> +{ >> + int level, max_level = ilk_wm_max_level(dev_priv->dev); >> + >> + if (wm[0] >= min) >> + return false; >> + >> + wm[0] = max(wm[0], min); >> + for (level = 1; level <= max_level; level++) >> + wm[level] = max_t(uint16_t, wm[level], DIV_ROUND_UP(min, 5)); >> + >> + return true; >> +} >> + >> +void snb_wm_latency_quirk(struct drm_device *dev) >> +{ >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + bool changed; >> + >> + changed = ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12) | >> + ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12) | >> + ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); >> + >> + if (!changed) >> + return; >> + >> + DRM_DEBUG_KMS("WM latency values increased due to high pixel clock\n"); >> + intel_print_wm_latency(dev, "Primary", dev_priv->wm.pri_latency); >> + intel_print_wm_latency(dev, "Sprite", dev_priv->wm.spr_latency); >> + intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency); >> +} >> + >> static void ilk_compute_wm_parameters(struct drm_crtc *crtc, >> struct ilk_pipe_wm_parameters *p, >> struct intel_wm_config *config) >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Daniel Vetter >Software Engineer, Intel Corporation >+41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock 2014-03-21 17:28 ` Runyan, Arthur J @ 2014-03-31 18:29 ` Robert Navarro 2014-04-14 9:16 ` Ville Syrjälä 0 siblings, 1 reply; 18+ messages in thread From: Robert Navarro @ 2014-03-31 18:29 UTC (permalink / raw) To: intel-gfx Runyan, Arthur J <arthur.j.runyan <at> intel.com> writes: > > Please check the DRAM configuration for the systems that fail. The higher latency is more likely with > higher tRFC which is mainly found with 8 Gbit components. > What other information do we need to get this included? The DRAM config, is this something that I have to/should check? How do I get this information to you? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock 2014-03-31 18:29 ` Robert Navarro @ 2014-04-14 9:16 ` Ville Syrjälä 0 siblings, 0 replies; 18+ messages in thread From: Ville Syrjälä @ 2014-04-14 9:16 UTC (permalink / raw) To: Robert Navarro; +Cc: intel-gfx On Mon, Mar 31, 2014 at 06:29:51PM +0000, Robert Navarro wrote: > Runyan, Arthur J <arthur.j.runyan <at> intel.com> writes: > > > > > Please check the DRAM configuration for the systems that fail. The higher > latency is more likely with > > higher tRFC which is mainly found with 8 Gbit components. > > > > What other information do we need to get this included? > > The DRAM config, is this something that I have to/should check? How do I get > this information to you? If decode-dimms works on your machine it might print out potentially interesting stuff. It doesn't seem to work on most of my machines. It does work on the x220 I have here though, so I'm assuming it should work on your x220 too. decode-dimms comes from the i2c-tools package. Here's what I get on mine: # modprobe i2c-i801 # modprobe eeprom # decode-dimms # decode-dimms version 6231 (2014-02-20 10:54:34 +0100) Memory Serial Presence Detect Decoder By Philip Edelbrock, Christian Zuckschwerdt, Burkart Lingner, Jean Delvare, Trent Piepho and others Decoding EEPROM: /sys/bus/i2c/drivers/eeprom/0-0050 Guessing DIMM is in bank 1 ---=== SPD EEPROM Information ===--- EEPROM CRC of bytes 0-116 OK (0xFC7B) # of bytes written to SDRAM EEPROM 176 Total number of bytes in EEPROM 256 Fundamental Memory type DDR3 SDRAM Module Type SO-DIMM ---=== Memory Characteristics ===--- Fine time base 1.000 ps Medium time base 0.125 ns Maximum module speed 1333 MHz (PC3-10600) Size 4096 MB Banks x Rows x Columns x Bits 8 x 15 x 10 x 64 Ranks 2 SDRAM Device Width 8 bits Bus Width Extension 0 bits tCL-tRCD-tRP-tRAS 9-9-9-24 Supported CAS Latencies (tCL) 9T, 8T, 7T, 6T, 5T ---=== Timing Parameters ===--- Minimum Write Recovery time (tWR) 15.000 ns Minimum Row Active to Row Active Delay (tRRD) 6.000 ns Minimum Active to Auto-Refresh Delay (tRC) 49.125 ns Minimum Recovery Delay (tRFC) 160.000 ns Minimum Write to Read CMD Delay (tWTR) 7.500 ns Minimum Read to Pre-charge CMD Delay (tRTP) 7.500 ns Minimum Four Activate Window Delay (tFAW) 30.000 ns ---=== Optional Features ===--- Operable voltages 1.5V RZQ/6 supported? Yes RZQ/7 supported? Yes DLL-Off Mode supported? Yes Operating temperature range 0-95 degrees C Refresh Rate in extended temp range 1X Auto Self-Refresh? No On-Die Thermal Sensor readout? No Partial Array Self-Refresh? No Thermal Sensor Accuracy Not implemented SDRAM Device Type Standard Monolithic ---=== Physical Characteristics ===--- Module Height (mm) 30 Module Thickness (mm) 2 front, 2 back Module Width (mm) 67.6 Module Reference Card F ---=== Manufacturer Data ===--- Module Manufacturer Samsung DRAM Manufacturer Samsung Manufacturing Location Code 0x03 Manufacturing Date 2012-W04 Assembly Serial Number 0x009B48F1 Part Number M471B5273DH0-CH9 Number of SDRAM DIMMs detected and decoded: 1 Just for figuring out what tRFC is I think the following would do it: # intel_reg_read 0x144298 0x144698 0x144a98 0x145d10 0x145e04 These are from the machines I have lying around on my desk: // dell xps (snb) # ./intel_reg_read 0x144298 0x144698 0x144a98 0x145d10 0x145e04 0x144298 : 0x5A6B1450 0x144698 : 0x5A6B1450 0x144A98 : 0x0 0x145D10 : 0x16040307 0x145E04 : 0x5 // x220 (snb) # ./intel_reg_read 0x144298 0x144698 0x144a98 0x145d10 0x145e04 0x144298 : 0x5A6B1450 0x144698 : 0x46B41004 0x144A98 : 0x0 0x145D10 : 0x16040307 0x145E04 : 0x5 // some ivb desktop # ./intel_reg_read 0x144298 0x144698 0x144a98 0x145d10 0x145e04 0x144298 : 0x5A6B1450 0x144698 : 0x5A6B1450 0x144A98 : 0x0 0x145D10 : 0x2010040C 0x145E04 : 0x5 So on for most things I get an answer of tRFC=107, which means 160ns if my math is any good. And that matches the SPD info from decode-dimms on the x220. The second channel for the x220 has something different but it actually looks like the reset value for the register which makes sense since decode-dimms says it only has one DIMM. No idea if my tRFC is particularly low or high. The latency values are quite different between the ivb and snb machines however. Also I must admit that I've never tried plugging in a 25x14 monitor to these machines, and in fact with the dell xps I can't since it doesn't even have a DP connector. -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] drm/i915: Increase WM memory latency values on SNB 2014-03-21 10:19 ` Daniel Vetter 2014-03-21 17:28 ` Runyan, Arthur J @ 2014-05-08 12:09 ` ville.syrjala 2014-05-09 15:23 ` Robert Navarro 2014-05-15 10:13 ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB Jani Nikula 1 sibling, 2 replies; 18+ messages in thread From: ville.syrjala @ 2014-05-08 12:09 UTC (permalink / raw) To: intel-gfx; +Cc: Jani Nikula, Robert N, Arthur Ranyan From: Ville Syrjälä <ville.syrjala@linux.intel.com> On SNB the BIOS provided WM memory latency values seem insufficient to handle high resolution displays. In this particular case the display mode was a 2560x1440@60Hz, which makes the pixel clock 241.5 MHz. It was empirically found that a memory latency value if 1.2 usec is enough to avoid underruns, whereas the BIOS provided value of 0.7 usec was clearly too low. Incidentally 1.2 usec is what the typical BIOS provided values are on IVB systems. Increase the WM memory latency values to at least 1.2 usec on SNB. Hopefully this won't have a significant effect on power consumption. v2: Increase the latency values regardless of the pixel clock Cc: Robert N <crshman@gmail.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_pm.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 834c49c..2809365 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2113,6 +2113,43 @@ static void intel_print_wm_latency(struct drm_device *dev, } } +static bool ilk_increase_wm_latency(struct drm_i915_private *dev_priv, + uint16_t wm[5], uint16_t min) +{ + int level, max_level = ilk_wm_max_level(dev_priv->dev); + + if (wm[0] >= min) + return false; + + wm[0] = max(wm[0], min); + for (level = 1; level <= max_level; level++) + wm[level] = max_t(uint16_t, wm[level], DIV_ROUND_UP(min, 5)); + + return true; +} + +static void snb_wm_latency_quirk(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + bool changed; + + /* + * The BIOS provided WM memory latency values are often + * inadequate for high resolution displays. Adjust them. + */ + changed = ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12) | + ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12) | + ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); + + if (!changed) + return; + + DRM_DEBUG_KMS("WM latency values increased to avoid potential underruns\n"); + intel_print_wm_latency(dev, "Primary", dev_priv->wm.pri_latency); + intel_print_wm_latency(dev, "Sprite", dev_priv->wm.spr_latency); + intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency); +} + static void ilk_setup_wm_latency(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -2130,6 +2167,9 @@ static void ilk_setup_wm_latency(struct drm_device *dev) intel_print_wm_latency(dev, "Primary", dev_priv->wm.pri_latency); intel_print_wm_latency(dev, "Sprite", dev_priv->wm.spr_latency); intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency); + + if (IS_GEN6(dev)) + snb_wm_latency_quirk(dev); } static void ilk_compute_wm_parameters(struct drm_crtc *crtc, -- 1.8.3.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB 2014-05-08 12:09 ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB ville.syrjala @ 2014-05-09 15:23 ` Robert Navarro 2014-05-09 15:38 ` [PATCH v2] drm/i915: Increase WM memory latency values?on SNB Ville Syrjälä 2014-05-15 10:13 ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB Jani Nikula 1 sibling, 1 reply; 18+ messages in thread From: Robert Navarro @ 2014-05-09 15:23 UTC (permalink / raw) To: intel-gfx Thanks for this Ville. Should this apply to 3.14 and 3.15? I'll try it on 3.15 first and report back. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm/i915: Increase WM memory latency values?on SNB 2014-05-09 15:23 ` Robert Navarro @ 2014-05-09 15:38 ` Ville Syrjälä 2014-05-09 17:46 ` Robert Navarro 0 siblings, 1 reply; 18+ messages in thread From: Ville Syrjälä @ 2014-05-09 15:38 UTC (permalink / raw) To: Robert Navarro; +Cc: intel-gfx On Fri, May 09, 2014 at 03:23:41PM +0000, Robert Navarro wrote: > Thanks for this Ville. > > Should this apply to 3.14 and 3.15? > > I'll try it on 3.15 first and report back. I think it should apply to 3.13+. If not directly then with a bit of manual frobbery. Which reminds me that we should perhaps slap a cc stable on it to get it included in 3.13+. For older kernels the patch would have to look totally different, so I'm not going to bother about those. -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm/i915: Increase WM memory latency values?on SNB 2014-05-09 15:38 ` [PATCH v2] drm/i915: Increase WM memory latency values?on SNB Ville Syrjälä @ 2014-05-09 17:46 ` Robert Navarro 2014-05-09 17:54 ` Ville Syrjälä 0 siblings, 1 reply; 18+ messages in thread From: Robert Navarro @ 2014-05-09 17:46 UTC (permalink / raw) To: intel-gfx Ville Syrjälä <ville.syrjala <at> linux.intel.com> writes: > I think it should apply to 3.13+. If not directly then with a bit of > manual frobbery. Which reminds me that we should perhaps slap a cc > stable on it to get it included in 3.13+. For older kernels the patch > would have to look totally different, so I'm not going to bother > about those. > Sounds good, one more question. Does this replace the previous patch or are they used together? _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm/i915: Increase WM memory latency values?on SNB 2014-05-09 17:46 ` Robert Navarro @ 2014-05-09 17:54 ` Ville Syrjälä 2014-05-14 0:55 ` Robert Navarro 0 siblings, 1 reply; 18+ messages in thread From: Ville Syrjälä @ 2014-05-09 17:54 UTC (permalink / raw) To: Robert Navarro; +Cc: intel-gfx On Fri, May 09, 2014 at 05:46:43PM +0000, Robert Navarro wrote: > Ville Syrjälä <ville.syrjala <at> linux.intel.com> writes: > > > I think it should apply to 3.13+. If not directly then with a bit of > > manual frobbery. Which reminds me that we should perhaps slap a cc > > stable on it to get it included in 3.13+. For older kernels the patch > > would have to look totally different, so I'm not going to bother > > about those. > > > > Sounds good, one more question. Does this replace the previous patch or are > they used together? Replace. -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm/i915: Increase WM memory latency values?on SNB 2014-05-09 17:54 ` Ville Syrjälä @ 2014-05-14 0:55 ` Robert Navarro 2014-05-15 14:18 ` Robert Navarro 0 siblings, 1 reply; 18+ messages in thread From: Robert Navarro @ 2014-05-14 0:55 UTC (permalink / raw) To: intel-gfx Ville Syrjälä <ville.syrjala <at> linux.intel.com> writes: > > Replace. > Finally got around to compiling this for my system, there were a few issues with the build scripts on the latest Ubuntu. Currently running 3.15.0-rc3-custom-drm-intel-nightly-bug70254+ with no issues thus far. I'll give a few more days just to make sure. Thanks for all your hard work so far! _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm/i915: Increase WM memory latency values?on SNB 2014-05-14 0:55 ` Robert Navarro @ 2014-05-15 14:18 ` Robert Navarro 0 siblings, 0 replies; 18+ messages in thread From: Robert Navarro @ 2014-05-15 14:18 UTC (permalink / raw) To: intel-gfx Robert Navarro <crshman <at> gmail.com> writes: > > Finally got around to compiling this for my system, there were a few issues > with the build scripts on the latest Ubuntu. > > Currently running 3.15.0-rc3-custom-drm-intel-nightly-bug70254+ with no > issues thus far. > > I'll give a few more days just to make sure. > > Thanks for all your hard work so far! > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx <at> lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > Just reporting back, I see you've included the fix, thanks! A few days (and really hot temperature days for the laptop) not a single flicker or anything. Thanks again for everyone's hard work in making this happen! ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB 2014-05-08 12:09 ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB ville.syrjala 2014-05-09 15:23 ` Robert Navarro @ 2014-05-15 10:13 ` Jani Nikula 2014-05-15 10:16 ` Chris Wilson 1 sibling, 1 reply; 18+ messages in thread From: Jani Nikula @ 2014-05-15 10:13 UTC (permalink / raw) To: ville.syrjala, intel-gfx; +Cc: Robert N, Arthur Ranyan On Thu, 08 May 2014, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > On SNB the BIOS provided WM memory latency values seem insufficient to > handle high resolution displays. > > In this particular case the display mode was a 2560x1440@60Hz, which > makes the pixel clock 241.5 MHz. It was empirically found that a memory > latency value if 1.2 usec is enough to avoid underruns, whereas the BIOS > provided value of 0.7 usec was clearly too low. Incidentally 1.2 usec > is what the typical BIOS provided values are on IVB systems. > > Increase the WM memory latency values to at least 1.2 usec on SNB. > Hopefully this won't have a significant effect on power consumption. > > v2: Increase the latency values regardless of the pixel clock > > Cc: Robert N <crshman@gmail.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70254 > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 834c49c..2809365 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2113,6 +2113,43 @@ static void intel_print_wm_latency(struct drm_device *dev, > } > } > > +static bool ilk_increase_wm_latency(struct drm_i915_private *dev_priv, > + uint16_t wm[5], uint16_t min) > +{ > + int level, max_level = ilk_wm_max_level(dev_priv->dev); > + > + if (wm[0] >= min) > + return false; > + > + wm[0] = max(wm[0], min); > + for (level = 1; level <= max_level; level++) > + wm[level] = max_t(uint16_t, wm[level], DIV_ROUND_UP(min, 5)); DIV_ROUND_UP(min, 5)? I don't get the units, and the code vs. comments in the wm code aren't helping. Please clarify. > + > + return true; > +} > + > +static void snb_wm_latency_quirk(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + bool changed; > + > + /* > + * The BIOS provided WM memory latency values are often > + * inadequate for high resolution displays. Adjust them. > + */ > + changed = ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12) | > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12) | > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); Nitpick, s/|/||/g for bools. BR, Jani. > + > + if (!changed) > + return; > + > + DRM_DEBUG_KMS("WM latency values increased to avoid potential underruns\n"); > + intel_print_wm_latency(dev, "Primary", dev_priv->wm.pri_latency); > + intel_print_wm_latency(dev, "Sprite", dev_priv->wm.spr_latency); > + intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency); > +} > + > static void ilk_setup_wm_latency(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -2130,6 +2167,9 @@ static void ilk_setup_wm_latency(struct drm_device *dev) > intel_print_wm_latency(dev, "Primary", dev_priv->wm.pri_latency); > intel_print_wm_latency(dev, "Sprite", dev_priv->wm.spr_latency); > intel_print_wm_latency(dev, "Cursor", dev_priv->wm.cur_latency); > + > + if (IS_GEN6(dev)) > + snb_wm_latency_quirk(dev); > } > > static void ilk_compute_wm_parameters(struct drm_crtc *crtc, > -- > 1.8.3.2 > -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB 2014-05-15 10:13 ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB Jani Nikula @ 2014-05-15 10:16 ` Chris Wilson 2014-05-15 10:34 ` Jani Nikula 0 siblings, 1 reply; 18+ messages in thread From: Chris Wilson @ 2014-05-15 10:16 UTC (permalink / raw) To: Jani Nikula; +Cc: Robert N, intel-gfx, Arthur Ranyan On Thu, May 15, 2014 at 01:13:21PM +0300, Jani Nikula wrote: > On Thu, 08 May 2014, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > +static void snb_wm_latency_quirk(struct drm_device *dev) > > +{ > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + bool changed; > > + > > + /* > > + * The BIOS provided WM memory latency values are often > > + * inadequate for high resolution displays. Adjust them. > > + */ > > + changed = ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12) | > > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12) | > > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); > > Nitpick, s/|/||/g for bools. Consider side effects. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB 2014-05-15 10:16 ` Chris Wilson @ 2014-05-15 10:34 ` Jani Nikula 2014-05-15 10:45 ` Ville Syrjälä 0 siblings, 1 reply; 18+ messages in thread From: Jani Nikula @ 2014-05-15 10:34 UTC (permalink / raw) To: Chris Wilson; +Cc: Robert N, intel-gfx, Arthur Ranyan On Thu, 15 May 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Thu, May 15, 2014 at 01:13:21PM +0300, Jani Nikula wrote: >> On Thu, 08 May 2014, ville.syrjala@linux.intel.com wrote: >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> > +static void snb_wm_latency_quirk(struct drm_device *dev) >> > +{ >> > + struct drm_i915_private *dev_priv = dev->dev_private; >> > + bool changed; >> > + >> > + /* >> > + * The BIOS provided WM memory latency values are often >> > + * inadequate for high resolution displays. Adjust them. >> > + */ >> > + changed = ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12) | >> > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12) | >> > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); >> >> Nitpick, s/|/||/g for bools. > > Consider side effects. Ugh I'm slow today. Some might claim business as usual. I'll hide somewhere. Before I head under the rock, may I say bitops on bools are still ugly? BR, Jani. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB 2014-05-15 10:34 ` Jani Nikula @ 2014-05-15 10:45 ` Ville Syrjälä 2014-05-15 10:58 ` Jani Nikula 0 siblings, 1 reply; 18+ messages in thread From: Ville Syrjälä @ 2014-05-15 10:45 UTC (permalink / raw) To: Jani Nikula; +Cc: Robert N, intel-gfx, Arthur Ranyan On Thu, May 15, 2014 at 01:34:44PM +0300, Jani Nikula wrote: > On Thu, 15 May 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > On Thu, May 15, 2014 at 01:13:21PM +0300, Jani Nikula wrote: > >> On Thu, 08 May 2014, ville.syrjala@linux.intel.com wrote: > >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> > +static void snb_wm_latency_quirk(struct drm_device *dev) > >> > +{ > >> > + struct drm_i915_private *dev_priv = dev->dev_private; > >> > + bool changed; > >> > + > >> > + /* > >> > + * The BIOS provided WM memory latency values are often > >> > + * inadequate for high resolution displays. Adjust them. > >> > + */ > >> > + changed = ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12) | > >> > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12) | > >> > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); > >> > >> Nitpick, s/|/||/g for bools. > > > > Consider side effects. > > Ugh I'm slow today. Some might claim business as usual. I'll hide > somewhere. > > Before I head under the rock, may I say bitops on bools are still ugly? I tend to use them but Paulo was also confused by them somewhere else in the watermark code, so maybe I should stop using them? I can rewrite as: changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12); changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12); changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); or just 3x if (ilk_increase_wm_latency(...)) changed = true; if that helps. -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] drm/i915: Increase WM memory latency values on SNB 2014-05-15 10:45 ` Ville Syrjälä @ 2014-05-15 10:58 ` Jani Nikula 0 siblings, 0 replies; 18+ messages in thread From: Jani Nikula @ 2014-05-15 10:58 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Robert N, intel-gfx, Arthur Ranyan On Thu, 15 May 2014, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Thu, May 15, 2014 at 01:34:44PM +0300, Jani Nikula wrote: >> On Thu, 15 May 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote: >> > On Thu, May 15, 2014 at 01:13:21PM +0300, Jani Nikula wrote: >> >> On Thu, 08 May 2014, ville.syrjala@linux.intel.com wrote: >> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com> >> >> > +static void snb_wm_latency_quirk(struct drm_device *dev) >> >> > +{ >> >> > + struct drm_i915_private *dev_priv = dev->dev_private; >> >> > + bool changed; >> >> > + >> >> > + /* >> >> > + * The BIOS provided WM memory latency values are often >> >> > + * inadequate for high resolution displays. Adjust them. >> >> > + */ >> >> > + changed = ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12) | >> >> > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12) | >> >> > + ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); >> >> >> >> Nitpick, s/|/||/g for bools. >> > >> > Consider side effects. >> >> Ugh I'm slow today. Some might claim business as usual. I'll hide >> somewhere. >> >> Before I head under the rock, may I say bitops on bools are still ugly? > > I tend to use them but Paulo was also confused by them somewhere else > in the watermark code, so maybe I should stop using them? > > I can rewrite as: > changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.pri_latency, 12); > changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.spr_latency, 12); > changed |= ilk_increase_wm_latency(dev_priv, dev_priv->wm.cur_latency, 12); > > or just 3x > if (ilk_increase_wm_latency(...)) > changed = true; > > if that helps. Too late, patch pushed to -fixes, thanks for the patch and ridicule. ;) You can send cleanups later for -next if you like. BR, Jani. > > -- > Ville Syrjälä > Intel OTC -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2014-05-15 14:18 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-21 9:00 [PATCH] drm/i915: Increase WM memory latency values on SNB with high pixel clock Jani Nikula 2014-03-21 9:29 ` Chris Wilson 2014-03-21 10:19 ` Daniel Vetter 2014-03-21 17:28 ` Runyan, Arthur J 2014-03-31 18:29 ` Robert Navarro 2014-04-14 9:16 ` Ville Syrjälä 2014-05-08 12:09 ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB ville.syrjala 2014-05-09 15:23 ` Robert Navarro 2014-05-09 15:38 ` [PATCH v2] drm/i915: Increase WM memory latency values?on SNB Ville Syrjälä 2014-05-09 17:46 ` Robert Navarro 2014-05-09 17:54 ` Ville Syrjälä 2014-05-14 0:55 ` Robert Navarro 2014-05-15 14:18 ` Robert Navarro 2014-05-15 10:13 ` [PATCH v2] drm/i915: Increase WM memory latency values on SNB Jani Nikula 2014-05-15 10:16 ` Chris Wilson 2014-05-15 10:34 ` Jani Nikula 2014-05-15 10:45 ` Ville Syrjälä 2014-05-15 10:58 ` Jani Nikula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox