[Intel-gfx] [PATCH] drm/i915: Hold forcewake for whole of punit communication

[Chris Wilson]

On Wed, Feb 15, 2017 at 01:33:01PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 15, 2017 at 10:41:29AM +0000, Chris Wilson wrote:
> > Take the forcewake for punit access and hold it across our waits to
> > ensure that the powerwell is not dropped by a spurious sleep.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_sideband.c | 55 ++++++++++++++++++++++-------------
> >  1 file changed, 34 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> > index 9f782b5eb6e6..ce6d166d67ea 100644
> > --- a/drivers/gpu/drm/i915/intel_sideband.c
> > +++ b/drivers/gpu/drm/i915/intel_sideband.c
> > @@ -39,42 +39,55 @@
> >  /* Private register write, double-word addressing, non-posted */
> >  #define SB_CRWRDA_NP	0x07
> >  
> > -static int vlv_sideband_rw(struct drm_i915_private *dev_priv, u32 devfn,
> > -			   u32 port, u32 opcode, u32 addr, u32 *val)
> > +static int vlv_sideband_rw(struct drm_i915_private *dev_priv,
> > +			   u32 devfn, u32 port, u32 opcode, u32 addr,
> > +			   u32 *val)
> >  {
> > -	u32 cmd, be = 0xf, bar = 0;
> > -	bool is_read = (opcode == SB_MRD_NP || opcode == SB_CRRDDA_NP);
> > +	const bool is_read = (opcode == SB_MRD_NP || opcode == SB_CRRDDA_NP);
> > +	enum forcewake_domains fw_domains;
> > +	int err = 0;
> >  
> > -	cmd = (devfn << IOSF_DEVFN_SHIFT) | (opcode << IOSF_OPCODE_SHIFT) |
> > -		(port << IOSF_PORT_SHIFT) | (be << IOSF_BYTE_ENABLES_SHIFT) |
> > -		(bar << IOSF_BAR_SHIFT);
> > +	lockdep_assert_held(&dev_priv->sb_lock);
> >  
> > -	WARN_ON(!mutex_is_locked(&dev_priv->sb_lock));
> > +	fw_domains = intel_uncore_forcewake_for_reg(dev_priv,
> > +						    VLV_IOSF_DOORBELL_REQ,
> 
> The mailbox lives in the display register range, and so doesn't need
> forcewake.

That's what I actually thought... So we could even bypass the fw
mmio interface. Probably not worth it.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre