Re: [PATCH 1/2] drm/i915: Drop the HSW special case from __gen6_gt_wait_for_thread_c0()

From: Deepak S <deepak.s@intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/i915: Drop the HSW special case from __gen6_gt_wait_for_thread_c0()
Date: Tue, 18 Nov 2014 14:21:39 +0530	[thread overview]
Message-ID: <546B089B.8080302@intel.com> (raw)
In-Reply-To: <1415909573-8402-1-git-send-email-ville.syrjala@linux.intel.com>

On Friday 14 November 2014 01:42 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Bits [18:16] of GEN6_GT_THREAD_STATUS_REG have always had the same
> meaning since SNB. So treating them as something special for HSW doesn't
> make sense to me.
>
> Also the bits *seem* to work exactly the same way on IVB, HSW GT2 and
> HSW GT3. At least intel_reg_read gives the identical results on all
> platforms with and without forcewake.
>
> Also the HSW PM guide rev 0.99 (ww05 2013) doesn't say anything about
> those bits. It just says to poll for bits [2:0]. As does the more recent
> BDW PM guide.
>
> So just drop the HSW special case and treat all platforms the same way.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h     |  1 -
>   drivers/gpu/drm/i915/intel_uncore.c | 10 ++--------
>   2 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 97d3479..3de58ac 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2313,7 +2313,6 @@ enum punit_power_well {
>   
>   #define GEN6_GT_THREAD_STATUS_REG 0x13805c
>   #define GEN6_GT_THREAD_STATUS_CORE_MASK 0x7
> -#define GEN6_GT_THREAD_STATUS_CORE_MASK_HSW (0x7 | (0x07 << 16))
>   
>   #define GEN6_GT_PERF_STATUS	(MCHBAR_MIRROR_BASE_SNB + 0x5948)
>   #define GEN6_RP_STATE_LIMITS	(MCHBAR_MIRROR_BASE_SNB + 0x5994)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 6a0c3fb..f318c03 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -49,17 +49,11 @@ assert_device_not_suspended(struct drm_i915_private *dev_priv)
>   
>   static void __gen6_gt_wait_for_thread_c0(struct drm_i915_private *dev_priv)
>   {
> -	u32 gt_thread_status_mask;
> -
> -	if (IS_HASWELL(dev_priv->dev))
> -		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK_HSW;
> -	else
> -		gt_thread_status_mask = GEN6_GT_THREAD_STATUS_CORE_MASK;
> -
>   	/* w/a for a sporadic read returning 0 by waiting for the GT
>   	 * thread to wake up.
>   	 */
> -	if (wait_for_atomic_us((__raw_i915_read32(dev_priv, GEN6_GT_THREAD_STATUS_REG) & gt_thread_status_mask) == 0, 500))
> +	if (wait_for_atomic_us((__raw_i915_read32(dev_priv, GEN6_GT_THREAD_STATUS_REG) &
> +				GEN6_GT_THREAD_STATUS_CORE_MASK) == 0, 500))
>   		DRM_ERROR("GT thread status wait timed out\n");
>   }
>   

Yes, Just polling for Bits [2..0] should be good.

Reviewed-by: Deepak S<deepak.s@linux.intel.com>

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