Re: [PATCH] drm/i915: add multi-threaded forcewake support

[Jesse Barnes <jbarnes@virtuousgeek.org>]

[-- Attachment #1.1: Type: text/plain, Size: 4769 bytes --]

On Mon, 21 Nov 2011 14:55:51 +0800
"Fang, XunX" <xunx.fang@intel.com> wrote:
>  void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
> @@ -902,8 +924,9 @@ MODULE_LICENSE("GPL and additional rights");
>  /* We give fast paths for the really cool registers */
>  #define NEEDS_FORCE_WAKE(dev_priv, reg) \
>  	(((dev_priv)->info->gen >= 6) && \
> -	((reg) < 0x40000) && \
> -	((reg) != FORCEWAKE))
> +	 ((reg) < 0x40000) &&		 \
> +	 ((reg) != FORCEWAKE) &&	 \
> +	 ((reg) != ECOBUS))

Both instances of this macro need to drop the ECOBUS check now that we
know it's in the GT power well.

>  
>  #define __i915_read(x, y) \
>  u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4a9c1b9..8ba88cf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -107,6 +107,7 @@ struct opregion_header;
>  struct opregion_acpi;
>  struct opregion_swsci;
>  struct opregion_asle;
> +struct drm_i915_private;
>  
>  struct intel_opregion {
>  	struct opregion_header *header;
> @@ -221,6 +222,8 @@ struct drm_i915_display_funcs {
>  			  struct drm_i915_gem_object *obj);
>  	int (*update_plane)(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  			    int x, int y);
> +	void (*force_wake_get)(struct drm_i915_private *dev_priv);
> +	void (*force_wake_put)(struct drm_i915_private *dev_priv);
>  	/* clock updates for mode set */
>  	/* cursor updates */
>  	/* render clock increase/decrease */
> @@ -1308,6 +1311,11 @@ extern void gen6_set_rps(struct drm_device *dev, u8 val);
>  extern void intel_detect_pch(struct drm_device *dev);
>  extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
>  
> +extern void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv);
> +extern void __gen6_gt_force_wake_mt_get(struct drm_i915_private *dev_priv);
> +extern void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv);
> +extern void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv);
> +
>  /* overlay */
>  #ifdef CONFIG_DEBUG_FS
>  extern struct intel_overlay_error_state *intel_overlay_capture_error_state(struct drm_device *dev);
> @@ -1352,8 +1360,9 @@ void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv);
>  /* We give fast paths for the really cool registers */
>  #define NEEDS_FORCE_WAKE(dev_priv, reg) \
>  	(((dev_priv)->info->gen >= 6) && \
> -	((reg) < 0x40000) && \
> -	((reg) != FORCEWAKE))
> +	 ((reg) < 0x40000) &&		 \
> +	 ((reg) != FORCEWAKE) &&	 \
> +	 ((reg) != ECOBUS))
>  
>  #define __i915_read(x, y) \
>  	u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b080cc8..8990057 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3449,6 +3449,10 @@
>  
>  #define  FORCEWAKE				0xA18C
>  #define  FORCEWAKE_ACK				0x130090
> +#define  FORCEWAKE_MT				0xa188 /* multi-threaded */
> +#define  FORCEWAKE_MT_ACK			0x130040
> +#define  ECOBUS					0xa180
> +#define    FORCEWAKE_MT_ENABLE			(1<<5)
>  
>  #define  GT_FIFO_FREE_ENTRIES			0x120008
>  #define    GT_FIFO_NUM_RESERVED_ENTRIES		20
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e77a863..633c693 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8491,6 +8491,28 @@ static void intel_init_display(struct drm_device *dev)
>  
>  	/* For FIFO watermark updates */
>  	if (HAS_PCH_SPLIT(dev)) {
> +		dev_priv->display.force_wake_get = __gen6_gt_force_wake_get;
> +		dev_priv->display.force_wake_put = __gen6_gt_force_wake_put;
> +
> +		/* IVB configs may use multi-threaded forcewake */
> +		if (IS_IVYBRIDGE(dev)) {
> +			u32	ecobus;
> +
> +			mutex_lock(&dev->struct_mutex);
> +			__gen6_gt_force_wake_mt_get(dev_priv);
> +			ecobus = I915_READ(ECOBUS);
> +			__gen6_gt_force_wake_mt_put(dev_priv);
> +			mutex_unlock(&dev->struct_mutex);
> +
> +			if (ecobus & FORCEWAKE_MT_ENABLE) {
> +				DRM_DEBUG_KMS("Using MT version of forcewake\n");
> +				dev_priv->display.force_wake_get =
> +					__gen6_gt_force_wake_mt_get;
> +				dev_priv->display.force_wake_put =
> +					__gen6_gt_force_wake_mt_put;
> +			}
> +		}

I think we need:

	I915_WRITE(GEN6_RC_STATE, 0);
	I915_WRITE(GEN6_RC_SLEEP, 0);

above the force_wake_get here.  And as long as it comes before
enable_rps, we'll properly re-enable RC6 and turbo later.

Anyone want to test with the suggested changes and report back?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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