diff mbox series

[05/15] drm/i915/gt: Ignore forcewake acquisition for posting_reads

Message ID 20190703091726.11690-5-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [01/15] drm/i915/selftests: Common live setup/teardown | expand

Commit Message

Chris Wilson July 3, 2019, 9:17 a.m. UTC
We don't care about the result of the read, so it may be garbage, we
only care that the mmio is flushed. As such, we can forgo using an
individual forcewake and lock around any posting-read for an engine.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tvrtko Ursulin July 3, 2019, 11:26 a.m. UTC | #1
On 03/07/2019 10:17, Chris Wilson wrote:
> We don't care about the result of the read, so it may be garbage, we
> only care that the mmio is flushed. As such, we can forgo using an
> individual forcewake and lock around any posting-read for an engine.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index 557b08b13feb..0331e9ac2485 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -51,7 +51,7 @@ struct drm_printer;
>   #define ENGINE_READ16(...)	__ENGINE_READ_OP(read16, __VA_ARGS__)
>   #define ENGINE_READ(...)	__ENGINE_READ_OP(read, __VA_ARGS__)
>   #define ENGINE_READ_FW(...)	__ENGINE_READ_OP(read_fw, __VA_ARGS__)
> -#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read, __VA_ARGS__)
> +#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read_fw, __VA_ARGS__)
>   #define ENGINE_POSTING_READ16(...) __ENGINE_READ_OP(posting_read16, __VA_ARGS__)
>   
>   #define ENGINE_READ64(engine__, lower_reg__, upper_reg__) \
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Can we apply this to all posting reads? (intel_uncore_posting_read*)

Regards,

Tvrtko
Chris Wilson July 3, 2019, 11:44 a.m. UTC | #2
Quoting Tvrtko Ursulin (2019-07-03 12:26:36)
> 
> On 03/07/2019 10:17, Chris Wilson wrote:
> > We don't care about the result of the read, so it may be garbage, we
> > only care that the mmio is flushed. As such, we can forgo using an
> > individual forcewake and lock around any posting-read for an engine.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/gt/intel_engine.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> > index 557b08b13feb..0331e9ac2485 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> > @@ -51,7 +51,7 @@ struct drm_printer;
> >   #define ENGINE_READ16(...)  __ENGINE_READ_OP(read16, __VA_ARGS__)
> >   #define ENGINE_READ(...)    __ENGINE_READ_OP(read, __VA_ARGS__)
> >   #define ENGINE_READ_FW(...) __ENGINE_READ_OP(read_fw, __VA_ARGS__)
> > -#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read, __VA_ARGS__)
> > +#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read_fw, __VA_ARGS__)
> >   #define ENGINE_POSTING_READ16(...) __ENGINE_READ_OP(posting_read16, __VA_ARGS__)
> >   
> >   #define ENGINE_READ64(engine__, lower_reg__, upper_reg__) \
> > 
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Can we apply this to all posting reads? (intel_uncore_posting_read*)

I briefly considered it, but was too lazy to think beyond the current
set. What gave me pause for concern was intel_gt_flush_ggtt_writes()
where we have to worry about overlapping mmio access causing gen7 to
explode, generalising the lock drop is tricky.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
index 557b08b13feb..0331e9ac2485 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine.h
@@ -51,7 +51,7 @@  struct drm_printer;
 #define ENGINE_READ16(...)	__ENGINE_READ_OP(read16, __VA_ARGS__)
 #define ENGINE_READ(...)	__ENGINE_READ_OP(read, __VA_ARGS__)
 #define ENGINE_READ_FW(...)	__ENGINE_READ_OP(read_fw, __VA_ARGS__)
-#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read, __VA_ARGS__)
+#define ENGINE_POSTING_READ(...) __ENGINE_READ_OP(posting_read_fw, __VA_ARGS__)
 #define ENGINE_POSTING_READ16(...) __ENGINE_READ_OP(posting_read16, __VA_ARGS__)
 
 #define ENGINE_READ64(engine__, lower_reg__, upper_reg__) \