On Thu, 26 Jun 2014 18:23:57 +0100
John.C.Harrison@xxxxxxxxx wrote:
> From: John Harrison <John.C.Harrison@xxxxxxxxx>
>
> 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 <jbarnes@xxxxxxxxxxxxxxxx>
--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
