diff mbox series

[i-g-t] lib: Only avoid relocations with full-ppgtt

Message ID 20200925084148.351563-1-chris@chris-wilson.co.uk
State New
Headers show
Series [i-g-t] lib: Only avoid relocations with full-ppgtt | expand

Commit Message

Chris Wilson Sept. 25, 2020, 8:41 a.m. UTC
We can only assigned random addresses with impunity if we know we have
complete control over the ppGTT. If the ppGTT is shared, either aliased
with the global GTT or simply the global GTT on ancient HW, then we have
to be careful in case they are objects already fixed in place inside the
GTT, e.g. active framebuffers. As the relocation code is still
available, only enforce no-relocations by default when the context has
its own ppGTT.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2491
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 lib/intel_batchbuffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Zbigniew Kempczyński Sept. 25, 2020, 9:25 a.m. UTC | #1
On Fri, Sep 25, 2020 at 09:41:48AM +0100, Chris Wilson wrote:
> We can only assigned random addresses with impunity if we know we have
> complete control over the ppGTT. If the ppGTT is shared, either aliased
> with the global GTT or simply the global GTT on ancient HW, then we have
> to be careful in case they are objects already fixed in place inside the
> GTT, e.g. active framebuffers. As the relocation code is still
> available, only enforce no-relocations by default when the context has
> its own ppGTT.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2491
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> ---
>  lib/intel_batchbuffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> index be764646e..42d7cd251 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -1306,7 +1306,7 @@ __intel_bb_create(int i915, uint32_t size, bool do_relocs)
>   */
>  struct intel_bb *intel_bb_create(int i915, uint32_t size)
>  {
> -	return __intel_bb_create(i915, size, false);
> +	return __intel_bb_create(i915, size, !gem_uses_full_ppgtt(i915));
>  }
>  
>  /**
> -- 
> 2.28.0

If we won't enforce EXEC_OBJECT_PINNED and set I915_EXEC_NO_RELOC for execbuf
still driver will try to use offsets configured in the objects array? I thought
NO_RELOC + !PINNED will relocate not properly configured objects but I'm likely
wrong. I don't have much experience with !ppgtt tables but if they are shared
I guess we can rely on kernel relocations only. So

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
diff mbox series

Patch

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index be764646e..42d7cd251 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -1306,7 +1306,7 @@  __intel_bb_create(int i915, uint32_t size, bool do_relocs)
  */
 struct intel_bb *intel_bb_create(int i915, uint32_t size)
 {
-	return __intel_bb_create(i915, size, false);
+	return __intel_bb_create(i915, size, !gem_uses_full_ppgtt(i915));
 }
 
 /**