Re: [PATCH v2 13/16] drm/i915: Keep track of who disabled LP1+ watermarks

[Paulo Zanoni <przanoni@gmail.com>]

2014-05-22 11:48 GMT-03:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently ilk_disable_lp_wm() just disabled LP1+ watermarks directly.
> However there's nothing preventing someone else from re-enabling them
> immediately. To make sure sure LP1+ watermarks stay disabled for the
> intended period, keep track which pipes require the LP1+ watermarks
> to be disabled.
>
> v2: Rebase and s/intel_crtc/crtc/

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  6 ++++++
>  drivers/gpu/drm/i915/intel_display.c |  2 +-
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c      | 22 ++++++++++++++++++----
>  4 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1fe0cac..02ffbfc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1548,6 +1548,12 @@ struct drm_i915_private {
>                 struct ilk_wm_values hw;
>
>                 /*
> +                * bitmask of pipes that have requested
> +                * LP1+ watermarks to be disabled.
> +                */
> +               unsigned int lp_disabled;
> +
> +               /*
>                  * protects some dev_priv->wm and intel_crtc->wm
>                  * state as well as the actual hardware registers
>                  */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 311c0f0..879c649 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4002,7 +4002,7 @@ static void ilk_prepare_for_num_pipes_change(struct intel_crtc *crtc)
>
>         ilk_wm_synchronize(other_active_crtc);
>
> -       if (ilk_disable_lp_wm(dev))
> +       if (ilk_disable_lp_wm(crtc))
>                 intel_wait_for_vblank(dev, other_active_crtc->pipe);
>  }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 98f878f..2a3ad60 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1023,7 +1023,7 @@ void intel_program_watermarks_post(struct intel_crtc *crtc,
>                                    const struct intel_crtc_wm_config *config);
>  void ilk_wm_synchronize(struct intel_crtc *crtc);
>  void ilk_wm_pipe_post_disable(struct intel_crtc *crtc);
> -bool ilk_disable_lp_wm(struct drm_device *dev);
> +bool ilk_disable_lp_wm(struct intel_crtc *crtc);
>
>
>  /* intel_sdvo.c */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 18ea8b1..17f1769 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2428,6 +2428,7 @@ static void ilk_compute_wm_results(struct drm_device *dev,
>                                    enum intel_ddb_partitioning partitioning,
>                                    struct ilk_wm_values *results)
>  {
> +       struct drm_i915_private *dev_priv = dev->dev_private;
>         struct intel_crtc *intel_crtc;
>         int level, wm_lp;
>
> @@ -2451,7 +2452,7 @@ static void ilk_compute_wm_results(struct drm_device *dev,
>                         (r->pri_val << WM1_LP_SR_SHIFT) |
>                         r->cur_val;
>
> -               if (r->enable)
> +               if (r->enable && !dev_priv->wm.lp_disabled)
>                         results->wm_lp[wm_lp - 1] |= WM1_LP_SR_EN;
>
>                 if (INTEL_INFO(dev)->gen >= 8)
> @@ -2765,13 +2766,18 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
>         }
>  }
>
> -bool ilk_disable_lp_wm(struct drm_device *dev)
> +bool ilk_disable_lp_wm(struct intel_crtc *crtc)
>  {
> +       struct drm_device *dev = crtc->base.dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         bool changed;
>
>         mutex_lock(&dev_priv->wm.mutex);
> +
> +       dev_priv->wm.lp_disabled |= 1 << crtc->pipe;
> +
>         changed = _ilk_disable_lp_wm(dev_priv, WM_DIRTY_LP_ALL);
> +
>         mutex_unlock(&dev_priv->wm.mutex);
>
>         return changed;
> @@ -2889,15 +2895,20 @@ static void ilk_setup_pending_watermarks(struct intel_crtc *crtc,
>                                          u32 vbl_count)
>  {
>         struct drm_device *dev = crtc->base.dev;
> +       struct drm_i915_private *dev_priv = dev->dev_private;
>         enum pipe pipe = crtc->pipe;
>
>         WARN(!crtc->active, "pipe %c should be enabled\n",
>              pipe_name(pipe));
>
>         /* do the watermarks actually need changing? */
> -       if (!memcmp(&crtc->wm.pending, pipe_wm, sizeof(*pipe_wm)))
> +       if (!(dev_priv->wm.lp_disabled & (1 << pipe)) &&
> +           !memcmp(&crtc->wm.pending, pipe_wm, sizeof(*pipe_wm)))
>                 return;
>
> +       /* allow LP1+ watermarks again */
> +       dev_priv->wm.lp_disabled &= ~(1 << pipe);
> +
>         crtc->wm.pending = *pipe_wm;
>
>         spin_lock_irq(&crtc->wm.lock);
> @@ -3092,6 +3103,9 @@ void ilk_wm_pipe_post_disable(struct intel_crtc *crtc)
>         /* pending update (if any) got cancelled */
>         crtc->wm.pending = crtc->wm.active;
>
> +       /* allow LP1+ watermarks again */
> +       dev_priv->wm.lp_disabled &= ~(1 << crtc->pipe);
> +
>         ilk_update_watermarks(dev, true);
>
>         mutex_unlock(&dev_priv->wm.mutex);
> @@ -3145,7 +3159,7 @@ static int ilk_update_sprite_wm(struct intel_plane *plane,
>          *
>          * WaCxSRDisabledForSpriteScaling:ivb
>          */
> -       if (IS_IVYBRIDGE(dev) && config->spr.scaled && ilk_disable_lp_wm(dev))
> +       if (IS_IVYBRIDGE(dev) && config->spr.scaled && ilk_disable_lp_wm(crtc))
>                 intel_wait_for_vblank(dev, plane->pipe);
>
>         params.pri = config->pri;
> --
> 1.8.5.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx