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

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> 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@linux.intel.com>
> Fixes: 14a6bbf9e535 ("drm/i915: Replace irq_seqno_barrier on hws write with a clflush")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@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);
> +	}
>  }
>  
>  /*
> -- 
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx