[Intel-gfx] [PATCH 01/19] drm/i915: Pass crtc to intel_update_watermarks()

Paulo Zanoni przanoni at gmail.com
Fri Aug 30 22:09:49 CEST 2013


2013/8/30  <ville.syrjala at linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Passing the appropriate crtc to intel_update_watermarks() should help
> in avoiding needless work in the future.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

I like the fact that now we're passing the CRTC, but <bikeshed> my
only worry is that now some functions overwrite the "crtc" we pass as
argument, so this might be confusing and maybe lead to bugs in the
future: perhaps the argument could be called unused_crtc or
ignored_crtc, then we'd keep the "crtc" variables they already have
</bikeshed>. But it's just a bikeshed, so with or without changes:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>.


> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++---------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c      | 43 +++++++++++++++++++++---------------
>  drivers/gpu/drm/i915/intel_sprite.c  |  6 ++---
>  5 files changed, 40 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7594356..ab46eb3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -357,7 +357,7 @@ struct drm_i915_display_funcs {
>                           int target, int refclk,
>                           struct dpll *match_clock,
>                           struct dpll *best_clock);
> -       void (*update_wm)(struct drm_device *dev);
> +       void (*update_wm)(struct drm_crtc *crtc);
>         void (*update_sprite_wm)(struct drm_plane *plane,
>                                  struct drm_crtc *crtc,
>                                  uint32_t sprite_width, int pixel_size,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f526ea9..13856ec 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3278,7 +3278,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>         intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
>         intel_set_pch_fifo_underrun_reporting(dev, pipe, true);
>
> -       intel_update_watermarks(dev);
> +       intel_update_watermarks(crtc);
>
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 if (encoder->pre_enable)
> @@ -3388,7 +3388,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>         if (intel_crtc->config.has_pch_encoder)
>                 intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
>
> -       intel_update_watermarks(dev);
> +       intel_update_watermarks(crtc);
>
>         if (intel_crtc->config.has_pch_encoder)
>                 dev_priv->display.fdi_link_train(crtc);
> @@ -3520,7 +3520,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>         }
>
>         intel_crtc->active = false;
> -       intel_update_watermarks(dev);
> +       intel_update_watermarks(crtc);
>
>         mutex_lock(&dev->struct_mutex);
>         intel_update_fbc(dev);
> @@ -3577,7 +3577,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>         }
>
>         intel_crtc->active = false;
> -       intel_update_watermarks(dev);
> +       intel_update_watermarks(crtc);
>
>         mutex_lock(&dev->struct_mutex);
>         intel_update_fbc(dev);
> @@ -3677,7 +3677,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>                 return;
>
>         intel_crtc->active = true;
> -       intel_update_watermarks(dev);
> +       intel_update_watermarks(crtc);
>
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 if (encoder->pre_pll_enable)
> @@ -3722,7 +3722,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>                 return;
>
>         intel_crtc->active = true;
> -       intel_update_watermarks(dev);
> +       intel_update_watermarks(crtc);
>
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 if (encoder->pre_enable)
> @@ -3806,7 +3806,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>
>         intel_crtc->active = false;
>         intel_update_fbc(dev);
> -       intel_update_watermarks(dev);
> +       intel_update_watermarks(crtc);
>  }
>
>  static void i9xx_crtc_off(struct drm_crtc *crtc)
> @@ -4972,7 +4972,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>
>         ret = intel_pipe_set_base(crtc, x, y, fb);
>
> -       intel_update_watermarks(dev);
> +       intel_update_watermarks(crtc);
>
>         return ret;
>  }
> @@ -5860,7 +5860,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>
>         ret = intel_pipe_set_base(crtc, x, y, fb);
>
> -       intel_update_watermarks(dev);
> +       intel_update_watermarks(crtc);
>
>         return ret;
>  }
> @@ -6316,7 +6316,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>
>         ret = intel_pipe_set_base(crtc, x, y, fb);
>
> -       intel_update_watermarks(dev);
> +       intel_update_watermarks(crtc);
>
>         return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a38f5b2..8b132e2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -714,7 +714,7 @@ extern void hsw_fdi_link_train(struct drm_crtc *crtc);
>  extern void intel_ddi_init(struct drm_device *dev, enum port port);
>
>  /* For use by IVB LP watermark workaround in intel_sprite.c */
> -extern void intel_update_watermarks(struct drm_device *dev);
> +extern void intel_update_watermarks(struct drm_crtc *crtc);
>  extern void intel_update_sprite_watermarks(struct drm_plane *plane,
>                                            struct drm_crtc *crtc,
>                                            uint32_t sprite_width, int pixel_size,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6b1d003..774c57f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1087,10 +1087,10 @@ static struct drm_crtc *single_enabled_crtc(struct drm_device *dev)
>         return enabled;
>  }
>
> -static void pineview_update_wm(struct drm_device *dev)
> +static void pineview_update_wm(struct drm_crtc *crtc)
>  {
> +       struct drm_device *dev = crtc->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       struct drm_crtc *crtc;
>         const struct cxsr_latency *latency;
>         u32 reg;
>         unsigned long wm;
> @@ -1365,8 +1365,9 @@ static void vlv_update_drain_latency(struct drm_device *dev)
>
>  #define single_plane_enabled(mask) is_power_of_2(mask)
>
> -static void valleyview_update_wm(struct drm_device *dev)
> +static void valleyview_update_wm(struct drm_crtc *crtc)
>  {
> +       struct drm_device *dev = crtc->dev;
>         static const int sr_latency_ns = 12000;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         int planea_wm, planeb_wm, cursora_wm, cursorb_wm;
> @@ -1424,8 +1425,9 @@ static void valleyview_update_wm(struct drm_device *dev)
>                    (cursor_sr << DSPFW_CURSOR_SR_SHIFT));
>  }
>
> -static void g4x_update_wm(struct drm_device *dev)
> +static void g4x_update_wm(struct drm_crtc *crtc)
>  {
> +       struct drm_device *dev = crtc->dev;
>         static const int sr_latency_ns = 12000;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         int planea_wm, planeb_wm, cursora_wm, cursorb_wm;
> @@ -1476,10 +1478,10 @@ static void g4x_update_wm(struct drm_device *dev)
>                    (cursor_sr << DSPFW_CURSOR_SR_SHIFT));
>  }
>
> -static void i965_update_wm(struct drm_device *dev)
> +static void i965_update_wm(struct drm_crtc *crtc)
>  {
> +       struct drm_device *dev = crtc->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       struct drm_crtc *crtc;
>         int srwm = 1;
>         int cursor_sr = 16;
>
> @@ -1541,8 +1543,9 @@ static void i965_update_wm(struct drm_device *dev)
>         I915_WRITE(DSPFW3, (cursor_sr << DSPFW_CURSOR_SR_SHIFT));
>  }
>
> -static void i9xx_update_wm(struct drm_device *dev)
> +static void i9xx_update_wm(struct drm_crtc *crtc)
>  {
> +       struct drm_device *dev = crtc->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         const struct intel_watermark_params *wm_info;
>         uint32_t fwater_lo;
> @@ -1550,7 +1553,7 @@ static void i9xx_update_wm(struct drm_device *dev)
>         int cwm, srwm = 1;
>         int fifo_size;
>         int planea_wm, planeb_wm;
> -       struct drm_crtc *crtc, *enabled = NULL;
> +       struct drm_crtc *enabled = NULL;
>
>         if (IS_I945GM(dev))
>                 wm_info = &i945_wm_info;
> @@ -1658,10 +1661,10 @@ static void i9xx_update_wm(struct drm_device *dev)
>         }
>  }
>
> -static void i830_update_wm(struct drm_device *dev)
> +static void i830_update_wm(struct drm_crtc *crtc)
>  {
> +       struct drm_device *dev = crtc->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       struct drm_crtc *crtc;
>         uint32_t fwater_lo;
>         int planea_wm;
>
> @@ -1785,8 +1788,9 @@ static bool ironlake_compute_srwm(struct drm_device *dev, int level, int plane,
>                                    display, cursor);
>  }
>
> -static void ironlake_update_wm(struct drm_device *dev)
> +static void ironlake_update_wm(struct drm_crtc *crtc)
>  {
> +       struct drm_device *dev = crtc->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         int fbc_wm, plane_wm, cursor_wm;
>         unsigned int enabled;
> @@ -1868,8 +1872,9 @@ static void ironlake_update_wm(struct drm_device *dev)
>          */
>  }
>
> -static void sandybridge_update_wm(struct drm_device *dev)
> +static void sandybridge_update_wm(struct drm_crtc *crtc)
>  {
> +       struct drm_device *dev = crtc->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         int latency = dev_priv->wm.pri_latency[0] * 100;        /* In unit 0.1us */
>         u32 val;
> @@ -1970,8 +1975,9 @@ static void sandybridge_update_wm(struct drm_device *dev)
>                    cursor_wm);
>  }
>
> -static void ivybridge_update_wm(struct drm_device *dev)
> +static void ivybridge_update_wm(struct drm_crtc *crtc)
>  {
> +       struct drm_device *dev = crtc->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         int latency = dev_priv->wm.pri_latency[0] * 100;        /* In unit 0.1us */
>         u32 val;
> @@ -2841,8 +2847,9 @@ static void hsw_write_wm_values(struct drm_i915_private *dev_priv,
>                 I915_WRITE(WM3_LP_ILK, results->wm_lp[2]);
>  }
>
> -static void haswell_update_wm(struct drm_device *dev)
> +static void haswell_update_wm(struct drm_crtc *crtc)
>  {
> +       struct drm_device *dev = crtc->dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
>         struct hsw_pipe_wm_parameters params[3];
> @@ -2879,7 +2886,7 @@ static void haswell_update_sprite_wm(struct drm_plane *plane,
>         intel_plane->wm.horiz_pixels = sprite_width;
>         intel_plane->wm.bytes_per_pixel = pixel_size;
>
> -       haswell_update_wm(plane->dev);
> +       haswell_update_wm(crtc);
>  }
>
>  static bool
> @@ -3076,12 +3083,12 @@ static void sandybridge_update_sprite_wm(struct drm_plane *plane,
>   * We don't use the sprite, so we can ignore that.  And on Crestline we have
>   * to set the non-SR watermarks to 8.
>   */
> -void intel_update_watermarks(struct drm_device *dev)
> +void intel_update_watermarks(struct drm_crtc *crtc)
>  {
> -       struct drm_i915_private *dev_priv = dev->dev_private;
> +       struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>
>         if (dev_priv->display.update_wm)
> -               dev_priv->display.update_wm(dev);
> +               dev_priv->display.update_wm(crtc);
>  }
>
>  void intel_update_sprite_watermarks(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 78b621c..a8a72337 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -285,7 +285,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>                 dev_priv->sprite_scaling_enabled |= 1 << pipe;
>
>                 if (!scaling_was_enabled) {
> -                       intel_update_watermarks(dev);
> +                       intel_update_watermarks(crtc);
>                         intel_wait_for_vblank(dev, pipe);
>                 }
>                 sprscale = SPRITE_SCALE_ENABLE | (src_w << 16) | src_h;
> @@ -320,7 +320,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>
>         /* potentially re-enable LP watermarks */
>         if (scaling_was_enabled && !dev_priv->sprite_scaling_enabled)
> -               intel_update_watermarks(dev);
> +               intel_update_watermarks(crtc);
>  }
>
>  static void
> @@ -346,7 +346,7 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
>
>         /* potentially re-enable LP watermarks */
>         if (scaling_was_enabled && !dev_priv->sprite_scaling_enabled)
> -               intel_update_watermarks(dev);
> +               intel_update_watermarks(crtc);
>  }
>
>  static int
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni



More information about the Intel-gfx mailing list