[Intel-gfx] [PATCH 1/3] drm/i915: Fixup intel_write_status_page() for old CPUs without clflush

[Ville Syrjälä]

On Fri, Mar 24, 2017 at 09:05:13PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 24, 2017 at 04:35:38PM +0000, Chris Wilson wrote:
> > Note all of our target platforms have clflush. For those without, just
> > assume the status page is sufficiently coherent that we do not need our
> > paranoia.
> > 
> > Reported-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Fixes: 14a6bbf9e535 ("drm/i915: Replace irq_seqno_barrier on hws write with a clflush")
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ringbuffer.h | 22 +++++++++++++++-------
> >  1 file changed, 15 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index 2ecb41788fb6..b5ce6692ed8a 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -454,14 +454,22 @@ intel_read_status_page(struct intel_engine_cs *engine, int reg)
> >  }
> >  
> >  static inline void
> > -intel_write_status_page(struct intel_engine_cs *engine,
> > -			int reg, u32 value)
> > +intel_write_status_page(struct intel_engine_cs *engine, int reg, u32 value)
> >  {
> > -	mb();
> > -	clflush(&engine->status_page.page_addr[reg]);
> > -	engine->status_page.page_addr[reg] = value;
> > -	clflush(&engine->status_page.page_addr[reg]);
> > -	mb();
> > +	/* Writing into the status page should be done sparingly. Since
> > +	 * we do when we are uncertain of the device state, we take a bit
> > +	 * if extra paranoia to try and ensure that the HWS takes the value
> > +	 * we give and that it doesn't end up trapped inside the CPU!
> > +	 */
> > +	if (static_cpu_has(X86_FEATURE_CLFLUSH)) {
> > +		mb();
> > +		clflush(&engine->status_page.page_addr[reg]);
> > +		engine->status_page.page_addr[reg] = value;
> > +		clflush(&engine->status_page.page_addr[reg]);
> > +		mb();
> > +	} else {
> > +		WRITE_ONCE(engine->status_page.page_addr[reg], value);
> > +	}
> 
> Fixes the illegal opcode explosion.
> Tested-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Unfortunately the GPU still dies as soon as I fire up glxgears. 4.9
> seems to be working just fine, so looks like a kernel regression.

And bisected down to

commit 944397f04f24eaf05125896dcb601c0e1c917879
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Mon Jan 9 16:16:11 2017 +0000

    drm/i915: Store required fence size/alignment for GGTT vma


Doesn't revert cleanly so can't tell whether there are other
issues piled on top or not.

-- 
Ville Syrjälä
Intel OTC