Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 19/21] drm/i915/gt: Use indices for writing into relative timelines
Date: Thu, 10 Dec 2020 11:16:44 -0800	[thread overview]
Message-ID: <20201210191644.GA6255@sdutt-i7> (raw)
In-Reply-To: <20201210080240.24529-19-chris@chris-wilson.co.uk>

On Thu, Dec 10, 2020 at 08:02:38AM +0000, Chris Wilson wrote:
> Relative timelines are relative to either the global or per-process
> HWSP, and so we can replace the absolute addressing with store-index
> variants for position invariance.
> 

Can you explain the benifit of relative addressing? Why can't we also
use absolute? If we can always use absolute, I don't see the point
complicating the breadcrumb code.

Matt

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 98 +++++++++++++++++-------
>  drivers/gpu/drm/i915/gt/intel_timeline.h | 12 +++
>  2 files changed, 82 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index ed88dc4de72c..386da26816d0 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -502,7 +502,19 @@ gen8_emit_fini_breadcrumb_tail(struct i915_request *rq, u32 *cs)
>  
>  static u32 *emit_xcs_breadcrumb(struct i915_request *rq, u32 *cs)
>  {
> -	return gen8_emit_ggtt_write(cs, rq->fence.seqno, hwsp_offset(rq), 0);
> +	struct intel_timeline *tl = rcu_dereference_protected(rq->timeline, 1);
> +	unsigned int flags = MI_FLUSH_DW_OP_STOREDW;
> +	u32 offset = hwsp_offset(rq);
> +
> +	if (intel_timeline_is_relative(tl)) {
> +		offset = offset_in_page(offset);
> +		flags |= MI_FLUSH_DW_STORE_INDEX;
> +	}
> +	GEM_BUG_ON(offset & 7);
> +	if (intel_timeline_is_global(tl))
> +		offset |= MI_FLUSH_DW_USE_GTT;
> +
> +	return __gen8_emit_flush_dw(cs, rq->fence.seqno, offset, flags);
>  }
>  
>  u32 *gen8_emit_fini_breadcrumb_xcs(struct i915_request *rq, u32 *cs)
> @@ -512,6 +524,18 @@ u32 *gen8_emit_fini_breadcrumb_xcs(struct i915_request *rq, u32 *cs)
>  
>  u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>  {
> +	struct intel_timeline *tl = rcu_dereference_protected(rq->timeline, 1);
> +	unsigned int flags = PIPE_CONTROL_FLUSH_ENABLE | PIPE_CONTROL_CS_STALL;
> +	u32 offset = hwsp_offset(rq);
> +
> +	if (intel_timeline_is_relative(tl)) {
> +		offset = offset_in_page(offset);
> +		flags |= PIPE_CONTROL_STORE_DATA_INDEX;
> +	}
> +	GEM_BUG_ON(offset & 7);
> +	if (intel_timeline_is_global(tl))
> +		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
> +
>  	cs = gen8_emit_pipe_control(cs,
>  				    PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
>  				    PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> @@ -519,26 +543,33 @@ u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>  				    0);
>  
>  	/* XXX flush+write+CS_STALL all in one upsets gem_concurrent_blt:kbl */
> -	cs = gen8_emit_ggtt_write_rcs(cs,
> -				      rq->fence.seqno,
> -				      hwsp_offset(rq),
> -				      PIPE_CONTROL_FLUSH_ENABLE |
> -				      PIPE_CONTROL_CS_STALL);
> +	cs = __gen8_emit_write_rcs(cs, rq->fence.seqno, offset, 0, flags);
>  
>  	return gen8_emit_fini_breadcrumb_tail(rq, cs);
>  }
>  
>  u32 *gen11_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>  {
> -	cs = gen8_emit_ggtt_write_rcs(cs,
> -				      rq->fence.seqno,
> -				      hwsp_offset(rq),
> -				      PIPE_CONTROL_CS_STALL |
> -				      PIPE_CONTROL_TILE_CACHE_FLUSH |
> -				      PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> -				      PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> -				      PIPE_CONTROL_DC_FLUSH_ENABLE |
> -				      PIPE_CONTROL_FLUSH_ENABLE);
> +	struct intel_timeline *tl = rcu_dereference_protected(rq->timeline, 1);
> +	u32 offset = hwsp_offset(rq);
> +	unsigned int flags;
> +
> +	flags = (PIPE_CONTROL_CS_STALL |
> +		 PIPE_CONTROL_TILE_CACHE_FLUSH |
> +		 PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> +		 PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> +		 PIPE_CONTROL_DC_FLUSH_ENABLE |
> +		 PIPE_CONTROL_FLUSH_ENABLE);
> +
> +	if (intel_timeline_is_relative(tl)) {
> +		offset = offset_in_page(offset);
> +		flags |= PIPE_CONTROL_STORE_DATA_INDEX;
> +	}
> +	GEM_BUG_ON(offset & 7);
> +	if (intel_timeline_is_global(tl))
> +		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
> +
> +	cs = __gen8_emit_write_rcs(cs, rq->fence.seqno, offset, 0, flags);
>  
>  	return gen8_emit_fini_breadcrumb_tail(rq, cs);
>  }
> @@ -601,19 +632,30 @@ u32 *gen12_emit_fini_breadcrumb_xcs(struct i915_request *rq, u32 *cs)
>  
>  u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>  {
> -	cs = gen12_emit_ggtt_write_rcs(cs,
> -				       rq->fence.seqno,
> -				       hwsp_offset(rq),
> -				       PIPE_CONTROL0_HDC_PIPELINE_FLUSH,
> -				       PIPE_CONTROL_CS_STALL |
> -				       PIPE_CONTROL_TILE_CACHE_FLUSH |
> -				       PIPE_CONTROL_FLUSH_L3 |
> -				       PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> -				       PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> -				       /* Wa_1409600907:tgl */
> -				       PIPE_CONTROL_DEPTH_STALL |
> -				       PIPE_CONTROL_DC_FLUSH_ENABLE |
> -				       PIPE_CONTROL_FLUSH_ENABLE);
> +	struct intel_timeline *tl = rcu_dereference_protected(rq->timeline, 1);
> +	u32 offset = hwsp_offset(rq);
> +	unsigned int flags;
> +
> +	flags = (PIPE_CONTROL_CS_STALL |
> +		 PIPE_CONTROL_TILE_CACHE_FLUSH |
> +		 PIPE_CONTROL_FLUSH_L3 |
> +		 PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> +		 PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> +		 /* Wa_1409600907:tgl */
> +		 PIPE_CONTROL_DEPTH_STALL |
> +		 PIPE_CONTROL_DC_FLUSH_ENABLE |
> +		 PIPE_CONTROL_FLUSH_ENABLE);
> +
> +	if (intel_timeline_is_relative(tl)) {
> +		offset = offset_in_page(offset);
> +		flags |= PIPE_CONTROL_STORE_DATA_INDEX;
> +	}
> +	GEM_BUG_ON(offset & 7);
> +	if (intel_timeline_is_global(tl))
> +		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
> +
> +	cs = __gen8_emit_write_rcs(cs, rq->fence.seqno, offset,
> +				   PIPE_CONTROL0_HDC_PIPELINE_FLUSH, flags);
>  
>  	return gen12_emit_fini_breadcrumb_tail(rq, cs);
>  }
> diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.h b/drivers/gpu/drm/i915/gt/intel_timeline.h
> index 69250de3a814..a3bdbff62e96 100644
> --- a/drivers/gpu/drm/i915/gt/intel_timeline.h
> +++ b/drivers/gpu/drm/i915/gt/intel_timeline.h
> @@ -79,6 +79,18 @@ intel_timeline_has_initial_breadcrumb(const struct intel_timeline *tl)
>  	return tl->mode == INTEL_TIMELINE_ABSOLUTE;
>  }
>  
> +static inline bool
> +intel_timeline_is_relative(const struct intel_timeline *tl)
> +{
> +	return tl->mode != INTEL_TIMELINE_ABSOLUTE;
> +}
> +
> +static inline bool
> +intel_timeline_is_global(const struct intel_timeline *tl)
> +{
> +	return tl->mode != INTEL_TIMELINE_CONTEXT;
> +}
> +
>  static inline int __intel_timeline_sync_set(struct intel_timeline *tl,
>  					    u64 context, u32 seqno)
>  {
> -- 
> 2.20.1
> 
> _______________________________________________
> 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

  reply	other threads:[~2020-12-10 19:22 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-10  8:02 [Intel-gfx] [PATCH 01/21] drm/i915/gt: Mark legacy ring context as lost Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 02/21] drm/i915/gt: Wean workaround selftests off GEM context Chris Wilson
2020-12-10 17:04   ` Mika Kuoppala
2020-12-10  8:02 ` [Intel-gfx] [PATCH 03/21] drm/i915/gt: Replace direct submit with direct call to tasklet Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 04/21] drm/i915/gt: Use virtual_engine during execlists_dequeue Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 05/21] drm/i915/gt: Decouple inflight virtual engines Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 06/21] drm/i915/gt: Defer schedule_out until after the next dequeue Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 07/21] drm/i915/gt: Remove virtual breadcrumb before transfer Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 08/21] drm/i915/gt: Shrink the critical section for irq signaling Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 09/21] drm/i915/gt: Resubmit the virtual engine on schedule-out Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 10/21] drm/i915/gt: Simplify virtual engine handling for execlists_hold() Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 11/21] drm/i915/gt: ce->inflight updates are now serialised Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 12/21] drm/i915/gem: Drop free_work for GEM contexts Chris Wilson
2020-12-10 18:50   ` Matthew Brost
2020-12-10  8:02 ` [Intel-gfx] [PATCH 13/21] drm/i915/gt: Track the overall awake/busy time Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 14/21] drm/i915: Encode fence specific waitqueue behaviour into the wait.flags Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 15/21] drm/i915/gt: Track all timelines created using the HWSP Chris Wilson
2020-12-10 18:28   ` Matthew Brost
2020-12-10  8:02 ` [Intel-gfx] [PATCH 16/21] drm/i915/gt: Wrap intel_timeline.has_initial_breadcrumb Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 17/21] drm/i915/gt: Track timeline GGTT offset separately from subpage offset Chris Wilson
2020-12-10 21:37   ` Matthew Brost
2020-12-10  8:02 ` [Intel-gfx] [PATCH 18/21] drm/i915/gt: Add timeline "mode" Chris Wilson
2020-12-10 19:28   ` Matthew Brost
2020-12-10 21:00     ` Chris Wilson
2020-12-10 21:18       ` Matthew Brost
2020-12-10  8:02 ` [Intel-gfx] [PATCH 19/21] drm/i915/gt: Use indices for writing into relative timelines Chris Wilson
2020-12-10 19:16   ` Matthew Brost [this message]
2020-12-10 21:05     ` Chris Wilson
2020-12-10 21:51       ` Matthew Brost
2020-12-10  8:02 ` [Intel-gfx] [PATCH 20/21] drm/i915/selftests: Exercise relative timeline modes Chris Wilson
2020-12-10  8:02 ` [Intel-gfx] [PATCH 21/21] drm/i915/gt: Use ppHWSP for unshared non-semaphore related timelines Chris Wilson
2020-12-10 21:28   ` Matthew Brost
2020-12-10  8:31 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [01/21] drm/i915/gt: Mark legacy ring context as lost Patchwork
2020-12-10  8:34 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2020-12-10  8:59 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-12-10 12:18 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-12-10 16:39 ` [Intel-gfx] [PATCH 01/21] " Mika Kuoppala
2020-12-10 17:04 ` Matthew Brost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201210191644.GA6255@sdutt-i7 \
    --to=matthew.brost@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox