Re: [PATCH 2/3] drm/i915: Implement sema idle msg disable for all rings

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Mar 31, 2014 at 07:23:20PM +0200, Daniel Vetter wrote:
> On Mon, Mar 31, 2014 at 06:17:17PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Previously in
> >  commit 295e8bb73a4785b65db6655fbf6ad57c4177b551
> >  Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >  Date:   Thu Feb 27 21:59:01 2014 +0200
> > 
> >     drm/i915: Disable semaphore wait event idle message on BDW
> > 
> > I failed to notice that all rings have their own copy of the bit that
> > disables the semaphore wait even idle message. So that patch only succeeded
> > in disabling it for the render ring. Instead we should set the bit for all
> > rings.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 8 ++++++++
> >  drivers/gpu/drm/i915/i915_reg.h | 2 ++
> >  drivers/gpu/drm/i915/intel_pm.c | 3 ---
> >  3 files changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 33bbaa0..84a7171 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4372,6 +4372,14 @@ static int i915_gem_init_rings(struct drm_device *dev)
> >  			goto cleanup_blt_ring;
> >  	}
> >  
> > +	if (IS_GEN8(dev)) {
> > +		struct intel_ring_buffer *ring;
> > +		int i;
> > +
> > +		for_each_ring(ring, dev_priv, i)
> > +			I915_WRITE(RING_RC_PSMI_CONTROL(ring),
> > +				   _MASKED_BIT_ENABLE(GEN8_RC_SEMA_IDLE_MSG_DISABLE));
> > +	}
> 
> Why move this to here? Is this one of those bits which get reset on ring
> init? If that's the case I think we really need to have a w/a checker to
> make sure that after driver load, suspend/resume and gpu reset we always
> have the same set of workarounds ...

Cause I needed ring->mmio_base to be there and I couldn't be bothered to
find a better place. Now that I looked a bit, I suppose init_ring_common()
might be the right place for it. I have no idea when it gets reset.

But hold on, now that I look at the spec again it seems the bit isn't there
for the other rings after all. I must have been doubly blind when I wrote
the patch. So let's just drop it.

-- 
Ville Syrjälä
Intel OTC