* [PATCH] drm/i915: Rename global latency_ns variable
@ 2014-09-03 10:56 Chris Wilson
2014-09-03 11:17 ` Ville Syrjälä
0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2014-09-03 10:56 UTC (permalink / raw)
To: intel-gfx
We use the variable name latency_ns in both the local lowlevel wm
calculation routines and at the global level. Rename the global value to
reduce shadow warnings and future confusion.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_pm.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a2340e87dd20..425d5e4052ae 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -872,7 +872,7 @@ void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
* A value of 5us seems to be a good balance; safe for very low end
* platforms but not overly aggressive on lower latency configs.
*/
-static const int latency_ns = 5000;
+static const int pessimal_latency_ns = 5000;
static int i9xx_get_fifo_size(struct drm_device *dev, int plane)
{
@@ -1387,14 +1387,14 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
vlv_update_drain_latency(crtc);
if (g4x_compute_wm0(dev, PIPE_A,
- &valleyview_wm_info, latency_ns,
- &valleyview_cursor_wm_info, latency_ns,
+ &valleyview_wm_info, pessimal_latency_ns,
+ &valleyview_cursor_wm_info, pessimal_latency_ns,
&planea_wm, &cursora_wm))
enabled |= 1 << PIPE_A;
if (g4x_compute_wm0(dev, PIPE_B,
- &valleyview_wm_info, latency_ns,
- &valleyview_cursor_wm_info, latency_ns,
+ &valleyview_wm_info, pessimal_latency_ns,
+ &valleyview_cursor_wm_info, pessimal_latency_ns,
&planeb_wm, &cursorb_wm))
enabled |= 1 << PIPE_B;
@@ -1453,20 +1453,20 @@ static void cherryview_update_wm(struct drm_crtc *crtc)
vlv_update_drain_latency(crtc);
if (g4x_compute_wm0(dev, PIPE_A,
- &valleyview_wm_info, latency_ns,
- &valleyview_cursor_wm_info, latency_ns,
+ &valleyview_wm_info, pessimal_latency_ns,
+ &valleyview_cursor_wm_info, pessimal_latency_ns,
&planea_wm, &cursora_wm))
enabled |= 1 << PIPE_A;
if (g4x_compute_wm0(dev, PIPE_B,
- &valleyview_wm_info, latency_ns,
- &valleyview_cursor_wm_info, latency_ns,
+ &valleyview_wm_info, pessimal_latency_ns,
+ &valleyview_cursor_wm_info, pessimal_latency_ns,
&planeb_wm, &cursorb_wm))
enabled |= 1 << PIPE_B;
if (g4x_compute_wm0(dev, PIPE_C,
- &valleyview_wm_info, latency_ns,
- &valleyview_cursor_wm_info, latency_ns,
+ &valleyview_wm_info, pessimal_latency_ns,
+ &valleyview_cursor_wm_info, pessimal_latency_ns,
&planec_wm, &cursorc_wm))
enabled |= 1 << PIPE_C;
@@ -1559,14 +1559,14 @@ static void g4x_update_wm(struct drm_crtc *crtc)
bool cxsr_enabled;
if (g4x_compute_wm0(dev, PIPE_A,
- &g4x_wm_info, latency_ns,
- &g4x_cursor_wm_info, latency_ns,
+ &g4x_wm_info, pessimal_latency_ns,
+ &g4x_cursor_wm_info, pessimal_latency_ns,
&planea_wm, &cursora_wm))
enabled |= 1 << PIPE_A;
if (g4x_compute_wm0(dev, PIPE_B,
- &g4x_wm_info, latency_ns,
- &g4x_cursor_wm_info, latency_ns,
+ &g4x_wm_info, pessimal_latency_ns,
+ &g4x_cursor_wm_info, pessimal_latency_ns,
&planeb_wm, &cursorb_wm))
enabled |= 1 << PIPE_B;
@@ -1709,7 +1709,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
wm_info, fifo_size, cpp,
- latency_ns);
+ pessimal_latency_ns);
enabled = crtc;
} else {
planea_wm = fifo_size - wm_info->guard_size;
@@ -1731,7 +1731,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
planeb_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
wm_info, fifo_size, cpp,
- latency_ns);
+ pessimal_latency_ns);
if (enabled == NULL)
enabled = crtc;
else
@@ -1827,7 +1827,7 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
&i845_wm_info,
dev_priv->display.get_fifo_size(dev, 0),
- 4, latency_ns);
+ 4, pessimal_latency_ns);
fwater_lo = I915_READ(FW_BLC) & ~0xfff;
fwater_lo |= (3<<8) | planea_wm;
--
2.1.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Rename global latency_ns variable
2014-09-03 10:56 [PATCH] drm/i915: Rename global latency_ns variable Chris Wilson
@ 2014-09-03 11:17 ` Ville Syrjälä
2014-09-03 13:12 ` Daniel Vetter
0 siblings, 1 reply; 3+ messages in thread
From: Ville Syrjälä @ 2014-09-03 11:17 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, Sep 03, 2014 at 11:56:07AM +0100, Chris Wilson wrote:
> We use the variable name latency_ns in both the local lowlevel wm
> calculation routines and at the global level. Rename the global value to
> reduce shadow warnings and future confusion.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
This has confused me occasioanlly in the past. Though I'm not sure if
it would be better to move it into local scope instead since we already
have the sr_lantency_ns in there for some platforms. Also I'm not sure
5us is really a good value.
But anyway we can massage it further if anyone cares enough. In the
meantime this makes things clearer at least, so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 36 ++++++++++++++++++------------------
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a2340e87dd20..425d5e4052ae 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -872,7 +872,7 @@ void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
> * A value of 5us seems to be a good balance; safe for very low end
> * platforms but not overly aggressive on lower latency configs.
> */
> -static const int latency_ns = 5000;
> +static const int pessimal_latency_ns = 5000;
>
> static int i9xx_get_fifo_size(struct drm_device *dev, int plane)
> {
> @@ -1387,14 +1387,14 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
> vlv_update_drain_latency(crtc);
>
> if (g4x_compute_wm0(dev, PIPE_A,
> - &valleyview_wm_info, latency_ns,
> - &valleyview_cursor_wm_info, latency_ns,
> + &valleyview_wm_info, pessimal_latency_ns,
> + &valleyview_cursor_wm_info, pessimal_latency_ns,
> &planea_wm, &cursora_wm))
> enabled |= 1 << PIPE_A;
>
> if (g4x_compute_wm0(dev, PIPE_B,
> - &valleyview_wm_info, latency_ns,
> - &valleyview_cursor_wm_info, latency_ns,
> + &valleyview_wm_info, pessimal_latency_ns,
> + &valleyview_cursor_wm_info, pessimal_latency_ns,
> &planeb_wm, &cursorb_wm))
> enabled |= 1 << PIPE_B;
>
> @@ -1453,20 +1453,20 @@ static void cherryview_update_wm(struct drm_crtc *crtc)
> vlv_update_drain_latency(crtc);
>
> if (g4x_compute_wm0(dev, PIPE_A,
> - &valleyview_wm_info, latency_ns,
> - &valleyview_cursor_wm_info, latency_ns,
> + &valleyview_wm_info, pessimal_latency_ns,
> + &valleyview_cursor_wm_info, pessimal_latency_ns,
> &planea_wm, &cursora_wm))
> enabled |= 1 << PIPE_A;
>
> if (g4x_compute_wm0(dev, PIPE_B,
> - &valleyview_wm_info, latency_ns,
> - &valleyview_cursor_wm_info, latency_ns,
> + &valleyview_wm_info, pessimal_latency_ns,
> + &valleyview_cursor_wm_info, pessimal_latency_ns,
> &planeb_wm, &cursorb_wm))
> enabled |= 1 << PIPE_B;
>
> if (g4x_compute_wm0(dev, PIPE_C,
> - &valleyview_wm_info, latency_ns,
> - &valleyview_cursor_wm_info, latency_ns,
> + &valleyview_wm_info, pessimal_latency_ns,
> + &valleyview_cursor_wm_info, pessimal_latency_ns,
> &planec_wm, &cursorc_wm))
> enabled |= 1 << PIPE_C;
>
> @@ -1559,14 +1559,14 @@ static void g4x_update_wm(struct drm_crtc *crtc)
> bool cxsr_enabled;
>
> if (g4x_compute_wm0(dev, PIPE_A,
> - &g4x_wm_info, latency_ns,
> - &g4x_cursor_wm_info, latency_ns,
> + &g4x_wm_info, pessimal_latency_ns,
> + &g4x_cursor_wm_info, pessimal_latency_ns,
> &planea_wm, &cursora_wm))
> enabled |= 1 << PIPE_A;
>
> if (g4x_compute_wm0(dev, PIPE_B,
> - &g4x_wm_info, latency_ns,
> - &g4x_cursor_wm_info, latency_ns,
> + &g4x_wm_info, pessimal_latency_ns,
> + &g4x_cursor_wm_info, pessimal_latency_ns,
> &planeb_wm, &cursorb_wm))
> enabled |= 1 << PIPE_B;
>
> @@ -1709,7 +1709,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
> adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
> wm_info, fifo_size, cpp,
> - latency_ns);
> + pessimal_latency_ns);
> enabled = crtc;
> } else {
> planea_wm = fifo_size - wm_info->guard_size;
> @@ -1731,7 +1731,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
> adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> planeb_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
> wm_info, fifo_size, cpp,
> - latency_ns);
> + pessimal_latency_ns);
> if (enabled == NULL)
> enabled = crtc;
> else
> @@ -1827,7 +1827,7 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
> planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
> &i845_wm_info,
> dev_priv->display.get_fifo_size(dev, 0),
> - 4, latency_ns);
> + 4, pessimal_latency_ns);
> fwater_lo = I915_READ(FW_BLC) & ~0xfff;
> fwater_lo |= (3<<8) | planea_wm;
>
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Rename global latency_ns variable
2014-09-03 11:17 ` Ville Syrjälä
@ 2014-09-03 13:12 ` Daniel Vetter
0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2014-09-03 13:12 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Wed, Sep 03, 2014 at 02:17:52PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 03, 2014 at 11:56:07AM +0100, Chris Wilson wrote:
> > We use the variable name latency_ns in both the local lowlevel wm
> > calculation routines and at the global level. Rename the global value to
> > reduce shadow warnings and future confusion.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> This has confused me occasioanlly in the past. Though I'm not sure if
> it would be better to move it into local scope instead since we already
> have the sr_lantency_ns in there for some platforms. Also I'm not sure
> 5us is really a good value.
I've had some patches to clean up the gmch wm calculation code since
there's a few other really confusing things in there. But it's also a lot
of tricky/fragile code :(
> But anyway we can massage it further if anyone cares enough. In the
> meantime this makes things clearer at least, so:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Queued for -next, thanks for the patch.
-Daniel
>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 36 ++++++++++++++++++------------------
> > 1 file changed, 18 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index a2340e87dd20..425d5e4052ae 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -872,7 +872,7 @@ void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
> > * A value of 5us seems to be a good balance; safe for very low end
> > * platforms but not overly aggressive on lower latency configs.
> > */
> > -static const int latency_ns = 5000;
> > +static const int pessimal_latency_ns = 5000;
> >
> > static int i9xx_get_fifo_size(struct drm_device *dev, int plane)
> > {
> > @@ -1387,14 +1387,14 @@ static void valleyview_update_wm(struct drm_crtc *crtc)
> > vlv_update_drain_latency(crtc);
> >
> > if (g4x_compute_wm0(dev, PIPE_A,
> > - &valleyview_wm_info, latency_ns,
> > - &valleyview_cursor_wm_info, latency_ns,
> > + &valleyview_wm_info, pessimal_latency_ns,
> > + &valleyview_cursor_wm_info, pessimal_latency_ns,
> > &planea_wm, &cursora_wm))
> > enabled |= 1 << PIPE_A;
> >
> > if (g4x_compute_wm0(dev, PIPE_B,
> > - &valleyview_wm_info, latency_ns,
> > - &valleyview_cursor_wm_info, latency_ns,
> > + &valleyview_wm_info, pessimal_latency_ns,
> > + &valleyview_cursor_wm_info, pessimal_latency_ns,
> > &planeb_wm, &cursorb_wm))
> > enabled |= 1 << PIPE_B;
> >
> > @@ -1453,20 +1453,20 @@ static void cherryview_update_wm(struct drm_crtc *crtc)
> > vlv_update_drain_latency(crtc);
> >
> > if (g4x_compute_wm0(dev, PIPE_A,
> > - &valleyview_wm_info, latency_ns,
> > - &valleyview_cursor_wm_info, latency_ns,
> > + &valleyview_wm_info, pessimal_latency_ns,
> > + &valleyview_cursor_wm_info, pessimal_latency_ns,
> > &planea_wm, &cursora_wm))
> > enabled |= 1 << PIPE_A;
> >
> > if (g4x_compute_wm0(dev, PIPE_B,
> > - &valleyview_wm_info, latency_ns,
> > - &valleyview_cursor_wm_info, latency_ns,
> > + &valleyview_wm_info, pessimal_latency_ns,
> > + &valleyview_cursor_wm_info, pessimal_latency_ns,
> > &planeb_wm, &cursorb_wm))
> > enabled |= 1 << PIPE_B;
> >
> > if (g4x_compute_wm0(dev, PIPE_C,
> > - &valleyview_wm_info, latency_ns,
> > - &valleyview_cursor_wm_info, latency_ns,
> > + &valleyview_wm_info, pessimal_latency_ns,
> > + &valleyview_cursor_wm_info, pessimal_latency_ns,
> > &planec_wm, &cursorc_wm))
> > enabled |= 1 << PIPE_C;
> >
> > @@ -1559,14 +1559,14 @@ static void g4x_update_wm(struct drm_crtc *crtc)
> > bool cxsr_enabled;
> >
> > if (g4x_compute_wm0(dev, PIPE_A,
> > - &g4x_wm_info, latency_ns,
> > - &g4x_cursor_wm_info, latency_ns,
> > + &g4x_wm_info, pessimal_latency_ns,
> > + &g4x_cursor_wm_info, pessimal_latency_ns,
> > &planea_wm, &cursora_wm))
> > enabled |= 1 << PIPE_A;
> >
> > if (g4x_compute_wm0(dev, PIPE_B,
> > - &g4x_wm_info, latency_ns,
> > - &g4x_cursor_wm_info, latency_ns,
> > + &g4x_wm_info, pessimal_latency_ns,
> > + &g4x_cursor_wm_info, pessimal_latency_ns,
> > &planeb_wm, &cursorb_wm))
> > enabled |= 1 << PIPE_B;
> >
> > @@ -1709,7 +1709,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
> > adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> > planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
> > wm_info, fifo_size, cpp,
> > - latency_ns);
> > + pessimal_latency_ns);
> > enabled = crtc;
> > } else {
> > planea_wm = fifo_size - wm_info->guard_size;
> > @@ -1731,7 +1731,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
> > adjusted_mode = &to_intel_crtc(crtc)->config.adjusted_mode;
> > planeb_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
> > wm_info, fifo_size, cpp,
> > - latency_ns);
> > + pessimal_latency_ns);
> > if (enabled == NULL)
> > enabled = crtc;
> > else
> > @@ -1827,7 +1827,7 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
> > planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
> > &i845_wm_info,
> > dev_priv->display.get_fifo_size(dev, 0),
> > - 4, latency_ns);
> > + 4, pessimal_latency_ns);
> > fwater_lo = I915_READ(FW_BLC) & ~0xfff;
> > fwater_lo |= (3<<8) | planea_wm;
> >
> > --
> > 2.1.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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] 3+ messages in thread
end of thread, other threads:[~2014-09-03 13:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-03 10:56 [PATCH] drm/i915: Rename global latency_ns variable Chris Wilson
2014-09-03 11:17 ` Ville Syrjälä
2014-09-03 13:12 ` 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.