From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 11/37] drm/i915/gem: Move the 'cached' info to i915_execbuffer
Date: Wed, 5 Aug 2020 14:29:38 +0100 [thread overview]
Message-ID: <1cd7fe46-f6b0-ab55-8e56-f64abf55fcfe@linux.intel.com> (raw)
In-Reply-To: <20200805122231.23313-12-chris@chris-wilson.co.uk>
On 05/08/2020 13:22, Chris Wilson wrote:
> The reloc_cache contains some details that are used outside of the
> relocation handling, so lift those out of the embeddded struct into the
> principle struct i915_execbuffer.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 61 +++++++++++--------
> .../i915/gem/selftests/i915_gem_execbuffer.c | 6 +-
> 2 files changed, 37 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index e7e16c62df1c..e9ef0c287fd9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -261,11 +261,6 @@ struct i915_execbuffer {
> */
> struct reloc_cache {
> struct drm_mm_node node; /** temporary GTT binding */
> - unsigned int gen; /** Cached value of INTEL_GEN */
> - bool use_64bit_reloc : 1;
> - bool has_llc : 1;
> - bool has_fence : 1;
> - bool needs_unfenced : 1;
>
> struct intel_context *ce;
>
> @@ -283,6 +278,12 @@ struct i915_execbuffer {
> u32 batch_len; /** Length of batch within object */
> u32 batch_flags; /** Flags composed for emit_bb_start() */
>
> + unsigned int gen; /** Cached value of INTEL_GEN */
> + bool use_64bit_reloc : 1;
> + bool has_llc : 1;
> + bool has_fence : 1;
> + bool needs_unfenced : 1;
> +
> /**
> * Indicate either the size of the hastable used to resolve
> * relocation handles, or if negative that we are using a direct
> @@ -540,11 +541,11 @@ eb_validate_vma(struct i915_execbuffer *eb,
> */
> entry->offset = gen8_noncanonical_addr(entry->offset);
>
> - if (!eb->reloc_cache.has_fence) {
> + if (!eb->has_fence) {
> entry->flags &= ~EXEC_OBJECT_NEEDS_FENCE;
> } else {
> if ((entry->flags & EXEC_OBJECT_NEEDS_FENCE ||
> - eb->reloc_cache.needs_unfenced) &&
> + eb->needs_unfenced) &&
> i915_gem_object_is_tiled(vma->obj))
> entry->flags |= EXEC_OBJECT_NEEDS_GTT | __EXEC_OBJECT_NEEDS_MAP;
> }
> @@ -592,7 +593,7 @@ eb_add_vma(struct i915_execbuffer *eb,
> if (entry->relocation_count &&
> !(ev->flags & EXEC_OBJECT_PINNED))
> ev->flags |= __EXEC_OBJECT_NEEDS_BIAS;
> - if (eb->reloc_cache.has_fence)
> + if (eb->has_fence)
> ev->flags |= EXEC_OBJECT_NEEDS_FENCE;
>
> eb->batch = ev;
> @@ -995,15 +996,19 @@ relocation_target(const struct drm_i915_gem_relocation_entry *reloc,
> return gen8_canonical_addr((int)reloc->delta + target->node.start);
> }
>
> -static void reloc_cache_init(struct reloc_cache *cache,
> - struct drm_i915_private *i915)
> +static void eb_info_init(struct i915_execbuffer *eb,
> + struct drm_i915_private *i915)
> {
> /* Must be a variable in the struct to allow GCC to unroll. */
> - cache->gen = INTEL_GEN(i915);
> - cache->has_llc = HAS_LLC(i915);
> - cache->use_64bit_reloc = HAS_64BIT_RELOC(i915);
> - cache->has_fence = cache->gen < 4;
> - cache->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;
> + eb->gen = INTEL_GEN(i915);
> + eb->has_llc = HAS_LLC(i915);
> + eb->use_64bit_reloc = HAS_64BIT_RELOC(i915);
> + eb->has_fence = eb->gen < 4;
> + eb->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;
> +}
> +
> +static void reloc_cache_init(struct reloc_cache *cache)
> +{
> cache->node.flags = 0;
> cache->rq = NULL;
> cache->target = NULL;
> @@ -1011,8 +1016,9 @@ static void reloc_cache_init(struct reloc_cache *cache,
>
> #define RELOC_TAIL 4
>
> -static int reloc_gpu_chain(struct reloc_cache *cache)
> +static int reloc_gpu_chain(struct i915_execbuffer *eb)
> {
> + struct reloc_cache *cache = &eb->reloc_cache;
> struct intel_gt_buffer_pool_node *pool;
> struct i915_request *rq = cache->rq;
> struct i915_vma *batch;
> @@ -1036,9 +1042,9 @@ static int reloc_gpu_chain(struct reloc_cache *cache)
> GEM_BUG_ON(cache->rq_size + RELOC_TAIL > PAGE_SIZE / sizeof(u32));
> cmd = cache->rq_cmd + cache->rq_size;
> *cmd++ = MI_ARB_CHECK;
> - if (cache->gen >= 8)
> + if (eb->gen >= 8)
> *cmd++ = MI_BATCH_BUFFER_START_GEN8;
> - else if (cache->gen >= 6)
> + else if (eb->gen >= 6)
> *cmd++ = MI_BATCH_BUFFER_START;
> else
> *cmd++ = MI_BATCH_BUFFER_START | MI_BATCH_GTT;
> @@ -1061,7 +1067,7 @@ static int reloc_gpu_chain(struct reloc_cache *cache)
> goto out_pool;
>
> cmd = i915_gem_object_pin_map(batch->obj,
> - cache->has_llc ?
> + eb->has_llc ?
> I915_MAP_FORCE_WB :
> I915_MAP_FORCE_WC);
> if (IS_ERR(cmd)) {
> @@ -1110,9 +1116,9 @@ static void __i915_request_add(struct i915_request *rq,
> __i915_request_queue(rq, attr);
> }
>
> -static unsigned int reloc_bb_flags(const struct reloc_cache *cache)
> +static unsigned int reloc_bb_flags(const struct i915_execbuffer *eb)
> {
> - return cache->gen > 5 ? 0 : I915_DISPATCH_SECURE;
> + return eb->gen > 5 ? 0 : I915_DISPATCH_SECURE;
> }
>
> static int reloc_gpu_flush(struct i915_execbuffer *eb)
> @@ -1143,7 +1149,7 @@ static int reloc_gpu_flush(struct i915_execbuffer *eb)
> err = rq->engine->emit_bb_start(rq,
> rq->batch->node.start,
> PAGE_SIZE,
> - reloc_bb_flags(cache));
> + reloc_bb_flags(eb));
> if (err)
> i915_request_set_error_once(rq, err);
>
> @@ -1191,7 +1197,7 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
> return PTR_ERR(pool);
>
> cmd = i915_gem_object_pin_map(pool->obj,
> - cache->has_llc ?
> + eb->has_llc ?
> I915_MAP_FORCE_WB :
> I915_MAP_FORCE_WC);
> if (IS_ERR(cmd)) {
> @@ -1285,7 +1291,7 @@ static u32 *reloc_gpu(struct i915_execbuffer *eb,
>
> if (unlikely(cache->rq_size + len >
> PAGE_SIZE / sizeof(u32) - RELOC_TAIL)) {
> - err = reloc_gpu_chain(cache);
> + err = reloc_gpu_chain(eb);
> if (unlikely(err)) {
> i915_request_set_error_once(cache->rq, err);
> return ERR_PTR(err);
> @@ -1318,7 +1324,7 @@ static int __reloc_entry_gpu(struct i915_execbuffer *eb,
> u64 offset,
> u64 target_addr)
> {
> - const unsigned int gen = eb->reloc_cache.gen;
> + const unsigned int gen = eb->gen;
> unsigned int len;
> u32 *batch;
> u64 addr;
> @@ -1465,7 +1471,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
>
> /* Check that the relocation address is valid... */
> if (unlikely(reloc->offset >
> - ev->vma->size - (eb->reloc_cache.use_64bit_reloc ? 8 : 4))) {
> + ev->vma->size - (eb->use_64bit_reloc ? 8 : 4))) {
> drm_dbg(&i915->drm, "Relocation beyond object bounds: "
> "target %d offset %d size %d.\n",
> reloc->target_handle,
> @@ -2676,7 +2682,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
> eb.exec = exec;
>
> eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
> - reloc_cache_init(&eb.reloc_cache, eb.i915);
> + eb_info_init(&eb, eb.i915);
> + reloc_cache_init(&eb.reloc_cache);
>
> eb.buffer_count = args->buffer_count;
> eb.batch_start_offset = args->batch_start_offset;
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> index d87f572048e8..13f17d6b5c66 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> @@ -23,8 +23,7 @@ static int __igt_gpu_reloc(struct i915_execbuffer *eb,
> struct drm_i915_gem_object *obj)
> {
> const unsigned int offsets[] = { 8, 3, 0 };
> - const u64 mask =
> - GENMASK_ULL(eb->reloc_cache.use_64bit_reloc ? 63 : 31, 0);
> + const u64 mask = GENMASK_ULL(eb->use_64bit_reloc ? 63 : 31, 0);
> const u32 *map = page_mask_bits(obj->mm.mapping);
> struct i915_request *rq;
> struct i915_vma *vma;
> @@ -126,8 +125,9 @@ static int igt_gpu_reloc(void *arg)
> goto err_scratch;
> }
>
> + eb_info_init(&eb, eb.i915);
> for_each_uabi_engine(eb.engine, eb.i915) {
> - reloc_cache_init(&eb.reloc_cache, eb.i915);
> + reloc_cache_init(&eb.reloc_cache);
> memset(map, POISON_INUSE, 4096);
>
> intel_engine_pm_get(eb.engine);
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-08-05 13:29 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-05 12:21 [Intel-gfx] [PATCH 00/37] Replace obj->mm.lock with reservation_ww_class Chris Wilson
2020-08-05 12:21 ` [Intel-gfx] [PATCH 01/37] drm/i915/gem: Reduce context termination list iteration guard to RCU Chris Wilson
2020-08-05 15:02 ` Tvrtko Ursulin
2020-08-05 12:21 ` [Intel-gfx] [PATCH 02/37] drm/i915/gt: Protect context lifetime with RCU Chris Wilson
2020-08-05 15:03 ` Tvrtko Ursulin
2020-08-06 10:14 ` Chris Wilson
2020-08-05 12:21 ` [Intel-gfx] [PATCH 03/37] drm/i915/gt: Free stale request on destroying the virtual engine Chris Wilson
2020-08-05 15:05 ` Tvrtko Ursulin
2020-08-06 10:44 ` Chris Wilson
2020-08-05 12:21 ` [Intel-gfx] [PATCH 04/37] drm/i915/gt: Defer enabling the breadcrumb interrupt to after submission Chris Wilson
2020-08-05 12:21 ` [Intel-gfx] [PATCH 05/37] drm/i915/gt: Track signaled breadcrumbs outside of the breadcrumb spinlock Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 06/37] drm/i915/gt: Don't cancel the interrupt shadow too early Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 07/37] drm/i915/gt: Split the breadcrumb spinlock between global and contexts Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 08/37] drm/i915/gem: Don't drop the timeline lock during execbuf Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 09/37] drm/i915/gem: Rename execbuf.bind_link to unbound_link Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 10/37] drm/i915/gem: Rename the list of relocations to reloc_list Chris Wilson
2020-08-05 13:26 ` Tvrtko Ursulin
2020-08-05 12:22 ` [Intel-gfx] [PATCH 11/37] drm/i915/gem: Move the 'cached' info to i915_execbuffer Chris Wilson
2020-08-05 13:29 ` Tvrtko Ursulin [this message]
2020-08-05 12:22 ` [Intel-gfx] [PATCH 12/37] drm/i915/gem: Break apart the early i915_vma_pin from execbuf object lookup Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 13/37] drm/i915/gem: Remove the call for no-evict i915_vma_pin Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 14/37] drm/i915: Serialise i915_vma_pin_inplace() with i915_vma_unbind() Chris Wilson
2020-08-05 13:56 ` Tvrtko Ursulin
2020-08-05 12:22 ` [Intel-gfx] [PATCH 15/37] drm/i915: Add list_for_each_entry_safe_continue_reverse Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 16/37] drm/i915: Always defer fenced work to the worker Chris Wilson
2020-08-05 13:58 ` Tvrtko Ursulin
2020-08-05 12:22 ` [Intel-gfx] [PATCH 17/37] drm/i915/gem: Assign context id for async work Chris Wilson
2020-08-05 13:59 ` Tvrtko Ursulin
2020-08-05 12:22 ` [Intel-gfx] [PATCH 18/37] drm/i915/gem: Separate the ww_mutex walker into its own list Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 19/37] drm/i915/gem: Asynchronous GTT unbinding Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 20/37] drm/i915/gem: Bind the fence async for execbuf Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 21/37] drm/i915/gem: Include cmdparser in common execbuf pinning Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 22/37] drm/i915/gem: Include secure batch " Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 23/37] drm/i915/gem: Manage GTT placement bias (starting offset) explicitly Chris Wilson
2020-08-05 14:16 ` Tvrtko Ursulin
2020-08-05 12:22 ` [Intel-gfx] [PATCH 24/37] drm/i915/gem: Reintroduce multiple passes for reloc processing Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 25/37] drm/i915: Add an implementation for common reservation_ww_class locking Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 26/37] drm/i915/gem: Pull execbuf dma resv under a single critical section Chris Wilson
2020-08-05 15:42 ` Thomas Hellström (Intel)
2020-08-05 12:22 ` [Intel-gfx] [PATCH 27/37] drm/i915/gtt: map the PD up front Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 28/37] drm/i915: Acquire the object lock around page directories Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 29/37] drm/i915/gem: Replace i915_gem_object.mm.mutex with reservation_ww_class Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 30/37] drm/i915: Hold wakeref for the duration of the vma GGTT binding Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 31/37] drm/i915/gt: Refactor heartbeat request construction and submission Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 32/37] drm/i915: Specialise GGTT binding Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 33/37] drm/i915/gt: Acquire backing storage for the context Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 34/37] drm/i915/gt: Push the wait for the context to bound to the request Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 35/37] drm/i915: Remove unused i915_gem_evict_vm() Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 36/37] drm/i915/display: Drop object lock from intel_unpin_fb_vma Chris Wilson
2020-08-05 12:22 ` [Intel-gfx] [PATCH 37/37] drm/i915/gem: Delay attach mmu-notifier until we acquire the pinned userptr Chris Wilson
2020-08-05 12:41 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Replace obj->mm.lock with reservation_ww_class Patchwork
2020-08-05 12:42 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-08-05 13:00 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-08-05 16:22 ` [Intel-gfx] [PATCH 00/37] " Thomas Hellström (Intel)
2020-08-06 9:21 ` Tvrtko Ursulin
2020-08-06 11:55 ` Daniel Vetter
2020-08-06 13:10 ` Tvrtko Ursulin
2020-08-10 9:51 ` Chris Wilson
2020-09-03 14:25 ` Tvrtko Ursulin
2020-08-05 17:44 ` [Intel-gfx] ✗ Fi.CI.IGT: failure for " Patchwork
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=1cd7fe46-f6b0-ab55-8e56-f64abf55fcfe@linux.intel.com \
--to=tvrtko.ursulin@linux.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