Re: [PATCH 4/5] drm/i915: Use only blitter forcewake

["Kamble, Sagar A" <sagar.a.kamble@xxxxxxxxx>]

On 9/11/2015 7:32 PM, Ville Syrjälä wrote:
> On Fri, Sep 11, 2015 at 07:24:31PM +0530, Kamble, Sagar A wrote:
> > On 9/11/2015 7:07 PM, Ville Syrjälä wrote:
> > > On Sun, Aug 23, 2015 at 05:52:50PM +0530, Sagar Arun Kamble wrote:
> > > > Coarse power gating is disabled prior to BXT B0 and till SKL E0,
> > > > hence even for render and media well registers blitter forcewake request
> > > > need to be used.
> > > >
> > > > Change-Id: Ibfa8abf02b4d27ca1fcd68fc9e98c2daade7c286
> > > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
> > > > ---
> > > >    drivers/gpu/drm/i915/i915_debugfs.c |  5 +++-
> > > >    drivers/gpu/drm/i915/i915_drv.h     |  1 +
> > > >    drivers/gpu/drm/i915/intel_uncore.c | 53 ++++++++++++++++++++++++++++++-------
> > > >    3 files changed, 48 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > index 7a28de5..8eea452 100644
> > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > > @@ -1509,7 +1509,10 @@ static int gen6_drpc_info(struct seq_file *m)
> > > >    	intel_runtime_pm_get(dev_priv);
> > > >    
> > > >    	spin_lock_irq(&dev_priv->uncore.lock);
> > > > -	forcewake_count = dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count;
> > > > +	if (use_blitter_forcewake(dev))
> > > > +		forcewake_count = dev_priv->uncore.fw_domain[FW_DOMAIN_ID_BLITTER].wake_count;
> > > > +	else
> > > > +		forcewake_count = dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count;
> > > >    	spin_unlock_irq(&dev_priv->uncore.lock);
> > > >    
> > > >    	if (forcewake_count) {
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index e0f3f05..c127175 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -2691,6 +2691,7 @@ extern void intel_uncore_check_errors(struct drm_device *dev);
> > > >    extern void intel_uncore_fini(struct drm_device *dev);
> > > >    extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore);
> > > >    const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id);
> > > > +bool use_blitter_forcewake(struct drm_device *dev);
> > > >    void intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
> > > >    				enum forcewake_domains domains);
> > > >    void intel_uncore_forcewake_put(struct drm_i915_private *dev_priv,
> > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > > > index 2df03b1..b7b6612 100644
> > > > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > > > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > > > @@ -63,6 +63,19 @@ intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id)
> > > >    	return "unknown";
> > > >    }
> > > >    
> > > > +bool use_blitter_forcewake(struct drm_device *dev)
> > > > +{
> > > > +	/*
> > > > +	 * Due to WaRsDisableCoarsePowerGating, Only blitter forcewake need to
> > > > +	 * be used on platforms previous to BXT B0 and until SKL E0.
> > > > +	*/
> > > You say only blitter forcewake need be used. OK, but how does that imply
> > > that you need to grab the blitter forcewake for render/media wells as
> > > well?
> > Ok. I need to reword the commit message. Change is to grab blitter fw
> > for all register accesses in GT.
> Where is that documented BTW? I couldn't find anything solid.
This is not documented as part of WaRsDisableCoarsePowerGating.
But if Coarse Power Gating is disabled it is as good as legacy RC6 flow 
like in HSW/BDW.
And hence we need to take blitter forcewake for all accesses.
> > > > +	if ((IS_BROXTON(dev) && (INTEL_REVID(dev) < BXT_REVID_B0)) ||
> > > > +		(IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_E0)))
> > > > +		return true;
> > > > +	else
> > > > +		return false;
> > > > +}
> > > > +
> > > >    static void
> > > >    assert_device_not_suspended(struct drm_i915_private *dev_priv)
> > > >    {
> > > > @@ -129,6 +142,9 @@ fw_domains_get(struct drm_i915_private *dev_priv, enum forcewake_domains fw_doma
> > > >    	struct intel_uncore_forcewake_domain *d;
> > > >    	enum forcewake_domain_id id;
> > > >    
> > > > +	WARN_ON(use_blitter_forcewake(dev_priv->dev) &&
> > > > +			(fw_domains & ~FORCEWAKE_BLITTER));
> > > > +
> > > >    	for_each_fw_domain_mask(d, fw_domains, dev_priv, id) {
> > > >    		fw_domain_wait_ack_clear(d);
> > > >    		fw_domain_get(d);
> > > > @@ -142,6 +158,9 @@ fw_domains_put(struct drm_i915_private *dev_priv, enum forcewake_domains fw_doma
> > > >    	struct intel_uncore_forcewake_domain *d;
> > > >    	enum forcewake_domain_id id;
> > > >    
> > > > +	WARN_ON(use_blitter_forcewake(dev_priv->dev) &&
> > > > +			(fw_domains & ~FORCEWAKE_BLITTER));
> > > > +
> > > >    	for_each_fw_domain_mask(d, fw_domains, dev_priv, id) {
> > > >    		fw_domain_put(d);
> > > >    		fw_domain_posting_read(d);
> > > > @@ -313,6 +332,11 @@ void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
> > > >    	if (fw)
> > > >    		dev_priv->uncore.funcs.force_wake_put(dev_priv, fw);
> > > >    
> > > > +	if (use_blitter_forcewake(dev) && !restore) {
> > > > +		__raw_i915_write32(dev_priv, FORCEWAKE_RENDER_GEN9, _MASKED_BIT_DISABLE(0xffff));
> > > > +		__raw_i915_write32(dev_priv, FORCEWAKE_MEDIA_GEN9, _MASKED_BIT_DISABLE(0xffff));
> > > > +	}
> > > > +
> > > >    	fw_domains_reset(dev_priv, FORCEWAKE_ALL);
> > > >    
> > > >    	if (restore) { /* If reset with a user forcewake, try to restore */
> > > > @@ -780,8 +804,11 @@ gen9_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \
> > > >    		fw_engine = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
> > > >    	else \
> > > >    		fw_engine = FORCEWAKE_BLITTER; \
> > > > -	if (fw_engine) \
> > > > +	if (fw_engine) { \
> > > > +		if (use_blitter_forcewake(dev_priv->dev)) \
> > > > +			fw_engine = FORCEWAKE_BLITTER; \
> > > >    		__force_wake_get(dev_priv, fw_engine); \
> > > > +	} \
> > > >    	val = __raw_i915_read##x(dev_priv, reg); \
> > > >    	GEN6_READ_FOOTER; \
> > > >    }
> > > > @@ -994,8 +1021,11 @@ gen9_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, \
> > > >    		fw_engine = FORCEWAKE_RENDER | FORCEWAKE_MEDIA; \
> > > >    	else \
> > > >    		fw_engine = FORCEWAKE_BLITTER; \
> > > > -	if (fw_engine) \
> > > > +	if (fw_engine) { \
> > > > +		if (use_blitter_forcewake(dev_priv->dev)) \
> > > > +			fw_engine = FORCEWAKE_BLITTER; \
> > > >    		__force_wake_get(dev_priv, fw_engine); \
> > > > +	} \
> > > >    	__raw_i915_write##x(dev_priv, reg, val); \
> > > >    	GEN6_WRITE_FOOTER; \
> > > >    }
> > > > @@ -1106,14 +1136,17 @@ static void intel_uncore_fw_domains_init(struct drm_device *dev)
> > > >    	if (IS_GEN9(dev)) {
> > > >    		dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
> > > >    		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
> > > > -		fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
> > > > -			       FORCEWAKE_RENDER_GEN9,
> > > > -			       FORCEWAKE_ACK_RENDER_GEN9);
> > > > -		fw_domain_init(dev_priv, FW_DOMAIN_ID_BLITTER,
> > > > -			       FORCEWAKE_BLITTER_GEN9,
> > > > -			       FORCEWAKE_ACK_BLITTER_GEN9);
> > > > -		fw_domain_init(dev_priv, FW_DOMAIN_ID_MEDIA,
> > > > -			       FORCEWAKE_MEDIA_GEN9, FORCEWAKE_ACK_MEDIA_GEN9);
> > > > +		if (!use_blitter_forcewake(dev)) {
> > > > +			fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
> > > > +					FORCEWAKE_RENDER_GEN9,
> > > > +					FORCEWAKE_ACK_RENDER_GEN9);
> > > > +			fw_domain_init(dev_priv, FW_DOMAIN_ID_MEDIA,
> > > > +					FORCEWAKE_MEDIA_GEN9,
> > > > +					FORCEWAKE_ACK_MEDIA_GEN9);
> > > > +		}
> > > > +                fw_domain_init(dev_priv, FW_DOMAIN_ID_BLITTER,
> > > > +                               FORCEWAKE_BLITTER_GEN9,
> > > > +                               FORCEWAKE_ACK_BLITTER_GEN9);
> > > >    	} else if (IS_VALLEYVIEW(dev)) {
> > > >    		dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
> > > >    		if (!IS_CHERRYVIEW(dev))
> > > > --
> > > > 1.9.1
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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