On Thu, 26 Jun 2014 18:23:57 +0100
john.c.harri...@intel.com wrote:

> From: John Harrison <john.c.harri...@intel.com>
> 
> The previous code was not correctly masking the value of the GTFIFOCTL 
> register,
> leading to overruns and the message "MMIO read or write has been dropped". In
> addition, the checks were repeated in several different places. This commit
> replaces these various checks with a simple (inline) function to encapsulate 
> the
> read-and-mask operation. In addition, it adds a custom wait-for-fifo function
> for VLV, as the timing parameters are somewhat different from those on earlier
> chips.
> ---
>  drivers/gpu/drm/i915/intel_uncore.c |   49 
> ++++++++++++++++++++++++++++++-----
>  1 file changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 871c284..6a3dddf 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -47,6 +47,12 @@ assert_device_not_suspended(struct drm_i915_private 
> *dev_priv)
>            "Device suspended\n");
>  }
>  
> +static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
> +{
> +     u32 count = __raw_i915_read32(dev_priv, GTFIFOCTL);
> +     return count & GT_FIFO_FREE_ENTRIES_MASK;
> +}
> +
>  static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
>  {
>       u32 gt_thread_status_mask;
> @@ -154,6 +160,28 @@ static void __gen7_gt_force_wake_mt_put(struct 
> drm_i915_private *dev_priv,
>               gen6_gt_check_fifodbg(dev_priv);
>  }
>  
> +static int __vlv_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
> +{
> +     u32 free = fifo_free_entries(dev_priv);
> +     int loop1, loop2;
> +
> +     for (loop1 = 0; loop1 < 5000 && free < GT_FIFO_NUM_RESERVED_ENTRIES; ) {
> +             for (loop2 = 0; loop2 < 1000 && free < 
> GT_FIFO_NUM_RESERVED_ENTRIES; loop2 += 10) {
> +                     udelay(10);
> +                     free = fifo_free_entries(dev_priv);
> +             }
> +             loop1 += loop2;
> +             if (loop1 > 1000 || free < 48)
> +                     DRM_DEBUG("after %d us, the FIFO has %d slots", loop1, 
> free);
> +     }
> +
> +     dev_priv->uncore.fifo_count = free;
> +     if (WARN(free < GT_FIFO_NUM_RESERVED_ENTRIES,
> +             "FIFO has insufficient space (%d slots)", free))
> +             return -1;
> +     return 0;
> +}
> +
>  static int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
>  {
>       int ret = 0;
> @@ -161,16 +189,15 @@ static int __gen6_gt_wait_for_fifo(struct 
> drm_i915_private *dev_priv)
>       /* On VLV, FIFO will be shared by both SW and HW.
>        * So, we need to read the FREE_ENTRIES everytime */
>       if (IS_VALLEYVIEW(dev_priv->dev))
> -             dev_priv->uncore.fifo_count =
> -                     __raw_i915_read32(dev_priv, GTFIFOCTL) &
> -                                             GT_FIFO_FREE_ENTRIES_MASK;
> +             return __vlv_gt_wait_for_fifo(dev_priv);
>  
>       if (dev_priv->uncore.fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
>               int loop = 500;
> -             u32 fifo = __raw_i915_read32(dev_priv, GTFIFOCTL) & 
> GT_FIFO_FREE_ENTRIES_MASK;
> +             u32 fifo = fifo_free_entries(dev_priv);
> +
>               while (fifo <= GT_FIFO_NUM_RESERVED_ENTRIES && loop--) {
>                       udelay(10);
> -                     fifo = __raw_i915_read32(dev_priv, GTFIFOCTL) & 
> GT_FIFO_FREE_ENTRIES_MASK;
> +                     fifo = fifo_free_entries(dev_priv);
>               }
>               if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
>                       ++ret;
> @@ -194,6 +221,11 @@ static void vlv_force_wake_reset(struct drm_i915_private 
> *dev_priv)
>  static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
>                                               int fw_engine)
>  {
> +#if  1
> +     if (__gen6_gt_wait_for_fifo(dev_priv))
> +             gen6_gt_check_fifodbg(dev_priv);
> +#endif
> +
>       /* Check for Render Engine */
>       if (FORCEWAKE_RENDER & fw_engine) {
>               if (wait_for_atomic((__raw_i915_read32(dev_priv,
> @@ -238,6 +270,10 @@ static void __vlv_force_wake_get(struct drm_i915_private 
> *dev_priv,
>  static void __vlv_force_wake_put(struct drm_i915_private *dev_priv,
>                                       int fw_engine)
>  {
> +#if  1
> +     if (__gen6_gt_wait_for_fifo(dev_priv))
> +             gen6_gt_check_fifodbg(dev_priv);
> +#endif
>  
>       /* Check for Render Engine */
>       if (FORCEWAKE_RENDER & fw_engine)
> @@ -355,8 +391,7 @@ static void intel_uncore_forcewake_reset(struct 
> drm_device *dev, bool restore)
>  
>               if (IS_GEN6(dev) || IS_GEN7(dev))
>                       dev_priv->uncore.fifo_count =
> -                             __raw_i915_read32(dev_priv, GTFIFOCTL) &
> -                             GT_FIFO_FREE_ENTRIES_MASK;
> +                             fifo_free_entries(dev_priv);
>       } else {
>               dev_priv->uncore.forcewake_count = 0;
>               dev_priv->uncore.fw_rendercount = 0;

It would be best to split out the free_entries cleanup (a good one)
from the vlv bug fix, and also drop the #if 1s.

With that done:

Reviewed-by: Jesse Barnes <jbar...@virtuousgeek.org>

-- 
Jesse Barnes, 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