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

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

[ville.syrjala]

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");
 }
 
-- 
2.0.4

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

[PATCH 2/2] drm/i915: Drop WaRsForcewakeWaitTC0:vlv

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

GEN6_GT_THREAD_STATUS_REG doesn't seem to exist on VLV. Reads just give
0x0 no matter what the state of the render and media wells.

There was also some hint in the Gunit HAS that thread status not being
needed on VLV, and hence dropped when bringing stuff over from the IVB
design. Not really a definite comment about the specific register itself
though.

Also the w/a itself is no longer listed for VLV in the database. It was
there some time ago in the past, but I guess someone figured out the
mistake and dropped it.

So let's just drop it from the code as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index f318c03..3aa5c65 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -223,10 +223,6 @@ static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
 					FORCEWAKE_ACK_TIMEOUT_MS))
 			DRM_ERROR("Timed out: waiting for media to ack.\n");
 	}
-
-	/* WaRsForcewakeWaitTC0:vlv */
-	if (!IS_CHERRYVIEW(dev_priv->dev))
-		__gen6_gt_wait_for_thread_c0(dev_priv);
 }
 
 static void __vlv_force_wake_put(struct drm_i915_private *dev_priv,
-- 
2.0.4

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

Re: [PATCH 2/2] drm/i915: Drop WaRsForcewakeWaitTC0:vlv

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
BYT: pass/total=291/291->291/291
PNV: pass/total=356/356->356/356
ILK: pass/total=372/372->367/372
IVB: pass/total=544/546->545/546
SNB: pass/total=423/425->424/425
HSW: pass/total=579/579->579/579
BDW: pass/total=433/435->434/435
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
ILK: Intel_gpu_tools, igt_kms_flip_plain-flip-fb-recreate-interruptible, PASS(4, M26) -> DMESG_WARN(2, M26)PASS(2, M26)
ILK: Intel_gpu_tools, igt_kms_flip_wf_vblank-vs-modeset-interruptible, DMESG_WARN(1, M26)PASS(3, M6M26) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_pipe_crc_basic_bad-nb-words-1, PASS(4, M6M26) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_setmode_invalid-clone-exclusive-crtc, PASS(4, M26) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, FAIL(6, M26)DMESG_FAIL(1, M26)TIMEOUT(32, M37M6M26)PASS(1, M26) -> FAIL(1, M26)DMESG_FAIL(1, M26)TIMEOUT(2, M26)
IVB: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(9, M4M21M34M45)PASS(16, M21M34M4M45) -> NSPT(1, M4)PASS(3, M4)
SNB: Intel_gpu_tools, igt_gem_concurrent_blit_gpuX-bcs-overwrite-source-forked, FAIL(1, M22)PASS(21, M35M22) -> PASS(4, M35)
BDW: Intel_gpu_tools, igt_gem_reset_stats_ban-bsd, DMESG_WARN(1, M30)PASS(24, M30M42M28) -> PASS(4, M30)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Drop WaRsForcewakeWaitTC0:vlv

[Daniel Vetter]

On Tue, Nov 18, 2014 at 02:10:35PM +0530, Deepak S wrote:
> 
> On Friday 14 November 2014 01:42 AM, ville.syrjala@linux.intel.com wrote:
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >GEN6_GT_THREAD_STATUS_REG doesn't seem to exist on VLV. Reads just give
> >0x0 no matter what the state of the render and media wells.
> >
> >There was also some hint in the Gunit HAS that thread status not being
> >needed on VLV, and hence dropped when bringing stuff over from the IVB
> >design. Not really a definite comment about the specific register itself
> >though.
> >
> >Also the w/a itself is no longer listed for VLV in the database. It was
> >there some time ago in the past, but I guess someone figured out the
> >mistake and dropped it.
> >
> >So let's just drop it from the code as well.
> >
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_uncore.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> >index f318c03..3aa5c65 100644
> >--- a/drivers/gpu/drm/i915/intel_uncore.c
> >+++ b/drivers/gpu/drm/i915/intel_uncore.c
> >@@ -223,10 +223,6 @@ static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
> >  					FORCEWAKE_ACK_TIMEOUT_MS))
> >  			DRM_ERROR("Timed out: waiting for media to ack.\n");
> >  	}
> >-
> >-	/* WaRsForcewakeWaitTC0:vlv */
> >-	if (!IS_CHERRYVIEW(dev_priv->dev))
> >-		__gen6_gt_wait_for_thread_c0(dev_priv);
> >  }
> >  static void __vlv_force_wake_put(struct drm_i915_private *dev_priv,
> 
> I did not find WA in DB.
> S0 :) - Reviewed-by: Deepak S<deepak.s@linux.intel.com>

Both merged, thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Drop WaRsForcewakeWaitTC0:vlv

[Deepak S]

On Friday 14 November 2014 01:42 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> GEN6_GT_THREAD_STATUS_REG doesn't seem to exist on VLV. Reads just give
> 0x0 no matter what the state of the render and media wells.
>
> There was also some hint in the Gunit HAS that thread status not being
> needed on VLV, and hence dropped when bringing stuff over from the IVB
> design. Not really a definite comment about the specific register itself
> though.
>
> Also the w/a itself is no longer listed for VLV in the database. It was
> there some time ago in the past, but I guess someone figured out the
> mistake and dropped it.
>
> So let's just drop it from the code as well.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_uncore.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index f318c03..3aa5c65 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -223,10 +223,6 @@ static void __vlv_force_wake_get(struct drm_i915_private *dev_priv,
>   					FORCEWAKE_ACK_TIMEOUT_MS))
>   			DRM_ERROR("Timed out: waiting for media to ack.\n");
>   	}
> -
> -	/* WaRsForcewakeWaitTC0:vlv */
> -	if (!IS_CHERRYVIEW(dev_priv->dev))
> -		__gen6_gt_wait_for_thread_c0(dev_priv);
>   }
>   
>   static void __vlv_force_wake_put(struct drm_i915_private *dev_priv,

I did not find WA in DB.
S0 :) - 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

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

[Deepak S]

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