On Mon, Apr 21, 2014 at 01:34:10PM +0530, deepa...@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> 
> Streamline the CHV forcewake functions just like was done for VLV.
> 
> This will also fix a bug in accessing the common well registers,
> where we'd end up trying to wake up the wells too many times
> since we'd call force_wake_get/put twice per register access, with
> FORCEFAKE_ALL both times.
> 
> Reviewed-by: Mika Kuoppala <mika.kuopp...@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

Already reviewed. I guess this patch makes what I was requesting earlier
a bit harder to implement. :-(

> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 88 
> ++++++++++++++-----------------------
>  1 file changed, 32 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 11741e4..f1264e2 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -619,35 +619,22 @@ chv_read##x(struct drm_i915_private *dev_priv, off_t 
> reg, bool trace) { \
>       unsigned fwengine = 0; \
>       REG_READ_HEADER(x); \
>       if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) { \
> -             fwengine = FORCEWAKE_RENDER; \
> -     } \
> -     else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) { \
> -             fwengine = FORCEWAKE_MEDIA; \
> -     } \
> -     else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) { \
> -             fwengine = FORCEWAKE_ALL; \
> -     } \
> -     if (FORCEWAKE_RENDER & fwengine) { \
> -             if (dev_priv->uncore.fw_rendercount++ == 0) \
> -                     (dev_priv)->uncore.funcs.force_wake_get(dev_priv, \
> -                                                             fwengine); \
> -     } \
> -     if (FORCEWAKE_MEDIA & fwengine) { \
> -             if (dev_priv->uncore.fw_mediacount++ == 0) \
> -                     (dev_priv)->uncore.funcs.force_wake_get(dev_priv, \
> -                                                             fwengine); \
> +             if (dev_priv->uncore.fw_rendercount == 0) \
> +                     fwengine = FORCEWAKE_RENDER; \
> +     } else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) { \
> +             if (dev_priv->uncore.fw_mediacount == 0) \
> +                     fwengine = FORCEWAKE_MEDIA; \
> +     } else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) { \
> +             if (dev_priv->uncore.fw_rendercount == 0) \
> +                     fwengine |= FORCEWAKE_RENDER; \
> +             if (dev_priv->uncore.fw_mediacount == 0) \
> +                     fwengine |= FORCEWAKE_MEDIA; \
>       } \
> +     if (fwengine) \
> +             dev_priv->uncore.funcs.force_wake_get(dev_priv, fwengine); \
>       val = __raw_i915_read##x(dev_priv, reg); \
> -     if (FORCEWAKE_RENDER & fwengine) { \
> -             if (--dev_priv->uncore.fw_rendercount == 0) \
> -                     (dev_priv)->uncore.funcs.force_wake_put(dev_priv, \
> -                                                             fwengine); \
> -     } \
> -     if (FORCEWAKE_MEDIA & fwengine) { \
> -             if (--dev_priv->uncore.fw_mediacount == 0) \
> -                     (dev_priv)->uncore.funcs.force_wake_put(dev_priv, \
> -                                                             fwengine); \
> -     } \
> +     if (fwengine) \
> +             dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
>       REG_READ_FOOTER; \
>  }
>  
> @@ -781,38 +768,27 @@ gen8_write##x(struct drm_i915_private *dev_priv, off_t 
> reg, u##x val, bool trace
>  static void \
>  chv_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool 
> trace) { \
>       unsigned fwengine = 0; \
> -     bool __needs_put = !is_gen8_shadowed(dev_priv, reg); \
> +     bool shadowed = is_gen8_shadowed(dev_priv, reg); \
>       REG_WRITE_HEADER; \
> -     if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) { \
> -             fwengine = FORCEWAKE_RENDER; \
> -     } \
> -     else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) { \
> -             fwengine = FORCEWAKE_MEDIA; \
> -     } \
> -     else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) { \
> -             fwengine = FORCEWAKE_ALL; \
> -     } \
> -     if (__needs_put && (FORCEWAKE_RENDER & fwengine)) { \
> -                     if (dev_priv->uncore.fw_rendercount++ == 0) \
> -                             
> (dev_priv)->uncore.funcs.force_wake_get(dev_priv, \
> -                                                                     
> fwengine); \
> -     } \
> -     if (__needs_put && (FORCEWAKE_MEDIA & fwengine)) { \
> -             if (dev_priv->uncore.fw_mediacount++ == 0) \
> -                     (dev_priv)->uncore.funcs.force_wake_get(dev_priv, \
> -                                                             fwengine); \
> +     if (!shadowed) { \
> +             if (FORCEWAKE_CHV_RENDER_RANGE_OFFSET(reg)) { \
> +                     if (dev_priv->uncore.fw_rendercount == 0) \
> +                             fwengine = FORCEWAKE_RENDER; \
> +             } else if (FORCEWAKE_CHV_MEDIA_RANGE_OFFSET(reg)) { \
> +                     if (dev_priv->uncore.fw_mediacount == 0) \
> +                             fwengine = FORCEWAKE_MEDIA; \
> +             } else if (FORCEWAKE_CHV_COMMON_RANGE_OFFSET(reg)) { \
> +                     if (dev_priv->uncore.fw_rendercount == 0) \
> +                             fwengine |= FORCEWAKE_RENDER; \
> +                     if (dev_priv->uncore.fw_mediacount == 0) \
> +                             fwengine |= FORCEWAKE_MEDIA; \
> +             } \

Nice target here we could extract out as common between read/write.

>       } \
> +     if (fwengine) \
> +             dev_priv->uncore.funcs.force_wake_get(dev_priv, fwengine); \
>       __raw_i915_write##x(dev_priv, reg, val); \
> -     if (__needs_put && (FORCEWAKE_RENDER & fwengine)) { \
> -                     if (--dev_priv->uncore.fw_rendercount == 0) \
> -                             
> (dev_priv)->uncore.funcs.force_wake_put(dev_priv, \
> -                                                                     
> fwengine); \
> -     } \
> -     if (__needs_put && (FORCEWAKE_MEDIA & fwengine)) { \
> -             if (--dev_priv->uncore.fw_mediacount == 0) \
> -                     (dev_priv)->uncore.funcs.force_wake_put(dev_priv, \
> -                                                             fwengine); \
> -     } \
> +     if (fwengine) \
> +             dev_priv->uncore.funcs.force_wake_put(dev_priv, fwengine); \
>       REG_WRITE_FOOTER; \
>  }
>  

Reviewed-by: Ben Widawsky <b...@bwidawsk.net>

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to