| Message ID | 20200819103952.19083-1-chris@chris-wilson.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] drm/i915/gem: Replace reloc chain with terminator on error unwind | expand |
Hi! > > > > Thanks for the patches. I assume this should fix problem from > > > > "5.9-rc1: graphics regression moved from -next to mainline" thread. > > > > > > > > I have applied them over current -next, and my machine seems to be > > > > working so far (but uptime is less than 30 minutes). > > > > > > > > If the machine still works tommorow, I'll assume problem is solved. > > > > > > Aye, best wait until we have to start competing with Chromium for > > > memory... The suspicion is that it was the resource allocation failure > > > path. > > > > Yep, my machines are low on memory. > > > > But ... test did not work that well. I have dead X and blinking > > screen. Machine still works reasonably well over ssh, so I guess > > that's an improvement. > > Well my last remaining 32bit gen3 device is currently pushing up the > daises, so could you try removing the attempt to use WC? Something like > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -955,10 +955,7 @@ static u32 *__reloc_gpu_map(struct reloc_cache *cache, > { > u32 *map; > > - map = i915_gem_object_pin_map(pool->obj, > - cache->has_llc ? > - I915_MAP_FORCE_WB : > - I915_MAP_FORCE_WC); > + map = i915_gem_object_pin_map(pool->obj, I915_MAP_FORCE_WB); > > on top of the previous patch. Faultinjection didn't turn up anything in > eb_relocate_vma, so we need to dig deeper. With this on top of other patches, it works. Tested-by: Pavel Machek <pavel@ucw.cz> Best regards, Pavel
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 24a1486d2dc5..a09f04eee417 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -972,21 +972,6 @@ static int reloc_gpu_chain(struct reloc_cache *cache) if (err) goto out_pool; - 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) - *cmd++ = MI_BATCH_BUFFER_START_GEN8; - else if (cache->gen >= 6) - *cmd++ = MI_BATCH_BUFFER_START; - else - *cmd++ = MI_BATCH_BUFFER_START | MI_BATCH_GTT; - *cmd++ = lower_32_bits(batch->node.start); - *cmd++ = upper_32_bits(batch->node.start); /* Always 0 for gen<8 */ - i915_gem_object_flush_map(cache->rq_vma->obj); - i915_gem_object_unpin_map(cache->rq_vma->obj); - cache->rq_vma = NULL; - err = intel_gt_buffer_pool_mark_active(pool, rq); if (err == 0) { i915_vma_lock(batch); @@ -999,15 +984,31 @@ static int reloc_gpu_chain(struct reloc_cache *cache) if (err) goto out_pool; + 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) + *cmd++ = MI_BATCH_BUFFER_START_GEN8; + else if (cache->gen >= 6) + *cmd++ = MI_BATCH_BUFFER_START; + else + *cmd++ = MI_BATCH_BUFFER_START | MI_BATCH_GTT; + *cmd++ = lower_32_bits(batch->node.start); + *cmd++ = upper_32_bits(batch->node.start); /* Always 0 for gen<8 */ + cmd = i915_gem_object_pin_map(batch->obj, cache->has_llc ? I915_MAP_FORCE_WB : I915_MAP_FORCE_WC); if (IS_ERR(cmd)) { + /* We will replace the BBS with BBE upon flushing the rq */ err = PTR_ERR(cmd); goto out_pool; } + i915_gem_object_flush_map(cache->rq_vma->obj); + i915_gem_object_unpin_map(cache->rq_vma->obj); + /* Return with batch mapping (cmd) still pinned */ cache->rq_cmd = cmd; cache->rq_size = 0;
If we hit an error during construction of the reloc chain, we need to replace the chain into the next batch with the terminator so that upon flushing the relocations so far, we do not execute a hanging batch. Reported-by: Pavel Machek <pavel@ucw.cz> Fixes: 964a9b0f611e ("drm/i915/gem: Use chained reloc batches") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Pavel Machek <pavel@ucw.cz> Cc: <stable@vger.kernel.org> # v5.8+ --- .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-)