Re: [Intel-gfx] [PATCH 2/2] drm/i915: refactor eb_get_batch()

[Chris Wilson]

On Fri, Jul 15, 2016 at 09:03:40AM +0100, Dave Gordon wrote:
> On 14/07/16 15:03, Chris Wilson wrote:
> >On Thu, Jul 14, 2016 at 02:12:55PM +0100, Dave Gordon wrote:
> >>On 13/07/16 13:44, Chris Wilson wrote:
> >>>On Wed, Jul 13, 2016 at 02:38:16PM +0200, Daniel Vetter wrote:
> On Thu, Jun 30, 2016 at 04:12:49PM +0100, Dave Gordon wrote:
> >Precursor for fix to secure batch execution. We will need to be able to
> >retrieve the batch VMA (as well as the batch itself) from the eb list,
> >so this patch extracts that part of eb_get_batch() into a separate
> >function, and moves both parts to a more logical place in the file, near
> >where the eb list is created.
> >
> >Also, it may not be obvious, but the current execbuffer2 ioctl interface
> >requires that the buffer object containing the batch-to-be-executed be
> >the LAST entry in the exec2_list[] array (I expected it to be the 
> >first!).
> >
> >To clarify this, we can replace the rather obscure construct
> > "list_entry(eb->vmas.prev, ...)"
> >in the old version of eb_get_batch() with the equivalent but more 
> >explicit
> > "list_last_entry(>vmas,...)"
> >in the new eb_get_batch_vma() and of course add an explanatory comment.
> >
> >Signed-off-by: Dave Gordon 
> 
> I have no context on the secure batch fix you're talking about, but this
> here makes sense as an independent cleanup.
> >>>
> >>>It won't help though, so this is just churn for no purpose.
> >>>-Chris
> >>
> >>At the very least, it replaces a confusing construct with
> >>a comprehensible one annotated with an explanatory comment.
> >
> >No. It deepens a confusion in the code that I've been trying to get
> >removed over the last couple of years.
> 
> ?
> 
> I was referring to the list_{last_}entry() change. That's definitely
> a clarification as to how things work now. Of course, if you're
> planning to make the batch the first object rather than the last, I
> won't object. But whichever it is, let's use the
> most-appropriately-named of the available list functions when we
> pick an item from a list. And comment why or what it's doing.
> 
> >>Separating finding the VMA for the batch from finding the batch itself
> >>also improves clarity and costs nothing (compiler inlines it anyway).
> >
> >No. That's the confusion you have here. The object is irrelevant.
> 
> Ah, so we have a function to return an irrelevant object. Let's just
> delete it then ;)
> 
> Do you think we /should/ just get rid of eb_get_batch()? Maybe just
> have eb_get_batch_vma() return the VMA to the [single] caller
> i915_gem_do_execbuffer() instead, and then have /that/ do both
> the flag-setting ugliness and the indirection to the object (which
> evidently is not irrelevant to it) ?
> 
> >>Comprehensibility -- and hence maintainability -- is always
> >>a worthwhile purpose :)
> >
> >s/comprehensibility/greater confusion/
> 
> Spoken like a true Discordian ;)
> 
> >> BTW, do the comments in this code from patch
> >>
> >>d23db88 drm/i915: Prevent negative relocation deltas from wrapping
> >>
> >>still apply? 'Cos I think it's pretty ugly to be setting a flag
> >>on a VMA as a side-effect of a "lookup" type operation :( Surely
> >>cleaner to do that sort of think at the top level i.e. inside
> >>i915_gem_do_execbuffer() ?
> >
> >The comment is wrong since the practice is more widespread and it is
> >a particular hw bug on Ivybridge.
> >-Chris
> 
> Another reason to move it out to the caller and update the comments
> in the process!

Then review the patches that have already been posted several times on
the list to do everything above.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: refactor eb_get_batch()

[Dave Gordon]

On 14/07/16 15:03, Chris Wilson wrote:

On Thu, Jul 14, 2016 at 02:12:55PM +0100, Dave Gordon wrote:

On 13/07/16 13:44, Chris Wilson wrote:

On Wed, Jul 13, 2016 at 02:38:16PM +0200, Daniel Vetter wrote:

On Thu, Jun 30, 2016 at 04:12:49PM +0100, Dave Gordon wrote:

Precursor for fix to secure batch execution. We will need to be able to
retrieve the batch VMA (as well as the batch itself) from the eb list,
so this patch extracts that part of eb_get_batch() into a separate
function, and moves both parts to a more logical place in the file, near
where the eb list is created.

Also, it may not be obvious, but the current execbuffer2 ioctl interface
requires that the buffer object containing the batch-to-be-executed be
the LAST entry in the exec2_list[] array (I expected it to be the first!).

To clarify this, we can replace the rather obscure construct
"list_entry(eb->vmas.prev, ...)"
in the old version of eb_get_batch() with the equivalent but more explicit
"list_last_entry(>vmas,...)"
in the new eb_get_batch_vma() and of course add an explanatory comment.

Signed-off-by: Dave Gordon 


I have no context on the secure batch fix you're talking about, but this
here makes sense as an independent cleanup.


It won't help though, so this is just churn for no purpose.
-Chris


At the very least, it replaces a confusing construct with
a comprehensible one annotated with an explanatory comment.


No. It deepens a confusion in the code that I've been trying to get
removed over the last couple of years.


?

I was referring to the list_{last_}entry() change. That's definitely a 
clarification as to how things work now. Of course, if you're planning 
to make the batch the first object rather than the last, I won't object. 
But whichever it is, let's use the most-appropriately-named of the 
available list functions when we pick an item from a list. And comment 
why or what it's doing.



Separating finding the VMA for the batch from finding the batch itself
also improves clarity and costs nothing (compiler inlines it anyway).


No. That's the confusion you have here. The object is irrelevant.


Ah, so we have a function to return an irrelevant object. Let's just 
delete it then ;)


Do you think we /should/ just get rid of eb_get_batch()? Maybe just
have eb_get_batch_vma() return the VMA to the [single] caller
i915_gem_do_execbuffer() instead, and then have /that/ do both
the flag-setting ugliness and the indirection to the object (which
evidently is not irrelevant to it) ?


Comprehensibility -- and hence maintainability -- is always
a worthwhile purpose :)


s/comprehensibility/greater confusion/


Spoken like a true Discordian ;)


> BTW, do the comments in this code from patch


d23db88 drm/i915: Prevent negative relocation deltas from wrapping

still apply? 'Cos I think it's pretty ugly to be setting a flag
on a VMA as a side-effect of a "lookup" type operation :( Surely
cleaner to do that sort of think at the top level i.e. inside
i915_gem_do_execbuffer() ?


The comment is wrong since the practice is more widespread and it is
a particular hw bug on Ivybridge.
-Chris


Another reason to move it out to the caller and update the comments in 
the process!


.Dave.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: refactor eb_get_batch()

[Chris Wilson]

On Thu, Jul 14, 2016 at 02:12:55PM +0100, Dave Gordon wrote:
> On 13/07/16 13:44, Chris Wilson wrote:
> >On Wed, Jul 13, 2016 at 02:38:16PM +0200, Daniel Vetter wrote:
> >>On Thu, Jun 30, 2016 at 04:12:49PM +0100, Dave Gordon wrote:
> >>>Precursor for fix to secure batch execution. We will need to be able to
> >>>retrieve the batch VMA (as well as the batch itself) from the eb list,
> >>>so this patch extracts that part of eb_get_batch() into a separate
> >>>function, and moves both parts to a more logical place in the file, near
> >>>where the eb list is created.
> >>>
> >>>Also, it may not be obvious, but the current execbuffer2 ioctl interface
> >>>requires that the buffer object containing the batch-to-be-executed be
> >>>the LAST entry in the exec2_list[] array (I expected it to be the first!).
> >>>
> >>>To clarify this, we can replace the rather obscure construct
> >>>   "list_entry(eb->vmas.prev, ...)"
> >>>in the old version of eb_get_batch() with the equivalent but more explicit
> >>>   "list_last_entry(>vmas,...)"
> >>>in the new eb_get_batch_vma() and of course add an explanatory comment.
> >>>
> >>>Signed-off-by: Dave Gordon 
> >>
> >>I have no context on the secure batch fix you're talking about, but this
> >>here makes sense as an independent cleanup.
> >
> >It won't help though, so this is just churn for no purpose.
> >-Chris
> 
> At the very least, it replaces a confusing construct with
> a comprehensible one annotated with an explanatory comment.

No. It deepens a confusion in the code that I've been trying to get
removed over the last couple of years.
 
> Separating finding the VMA for the batch from finding the batch itself
> also improves clarity and costs nothing (compiler inlines it anyway).

No. That's the confusion you have here. The object is irrelevant.
 
> Comprehensibility -- and hence maintainability -- is always
> a worthwhile purpose :)

s/comprehensibility/greater confusion/

> BTW, do the comments in this code from patch
> 
> d23db88 drm/i915: Prevent negative relocation deltas from wrapping
> 
> still apply? 'Cos I think it's pretty ugly to be setting a flag
> on a VMA as a side-effect of a "lookup" type operation :( Surely
> cleaner to do that sort of think at the top level i.e. inside
> i915_gem_do_execbuffer() ?

The comment is wrong since the practice is more widespread and it is
a particular hw bug on Ivybridge.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: refactor eb_get_batch()

[Dave Gordon]

On 13/07/16 13:44, Chris Wilson wrote:

On Wed, Jul 13, 2016 at 02:38:16PM +0200, Daniel Vetter wrote:

On Thu, Jun 30, 2016 at 04:12:49PM +0100, Dave Gordon wrote:

Precursor for fix to secure batch execution. We will need to be able to
retrieve the batch VMA (as well as the batch itself) from the eb list,
so this patch extracts that part of eb_get_batch() into a separate
function, and moves both parts to a more logical place in the file, near
where the eb list is created.

Also, it may not be obvious, but the current execbuffer2 ioctl interface
requires that the buffer object containing the batch-to-be-executed be
the LAST entry in the exec2_list[] array (I expected it to be the first!).

To clarify this, we can replace the rather obscure construct
"list_entry(eb->vmas.prev, ...)"
in the old version of eb_get_batch() with the equivalent but more explicit
"list_last_entry(>vmas,...)"
in the new eb_get_batch_vma() and of course add an explanatory comment.

Signed-off-by: Dave Gordon 


I have no context on the secure batch fix you're talking about, but this
here makes sense as an independent cleanup.


It won't help though, so this is just churn for no purpose.
-Chris


At the very least, it replaces a confusing construct with
a comprehensible one annotated with an explanatory comment.

Separating finding the VMA for the batch from finding the batch itself
also improves clarity and costs nothing (compiler inlines it anyway).

Comprehensibility -- and hence maintainability -- is always
a worthwhile purpose :)

BTW, do the comments in this code from patch

d23db88 drm/i915: Prevent negative relocation deltas from wrapping

still apply? 'Cos I think it's pretty ugly to be setting a flag
on a VMA as a side-effect of a "lookup" type operation :( Surely
cleaner to do that sort of think at the top level i.e. inside
i915_gem_do_execbuffer() ?

.Dave.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: refactor eb_get_batch()

[Chris Wilson]

On Wed, Jul 13, 2016 at 02:38:16PM +0200, Daniel Vetter wrote:
> On Thu, Jun 30, 2016 at 04:12:49PM +0100, Dave Gordon wrote:
> > Precursor for fix to secure batch execution. We will need to be able to
> > retrieve the batch VMA (as well as the batch itself) from the eb list,
> > so this patch extracts that part of eb_get_batch() into a separate
> > function, and moves both parts to a more logical place in the file, near
> > where the eb list is created.
> > 
> > Also, it may not be obvious, but the current execbuffer2 ioctl interface
> > requires that the buffer object containing the batch-to-be-executed be
> > the LAST entry in the exec2_list[] array (I expected it to be the first!).
> > 
> > To clarify this, we can replace the rather obscure construct
> > "list_entry(eb->vmas.prev, ...)"
> > in the old version of eb_get_batch() with the equivalent but more explicit
> > "list_last_entry(>vmas,...)"
> > in the new eb_get_batch_vma() and of course add an explanatory comment.
> > 
> > Signed-off-by: Dave Gordon 
> 
> I have no context on the secure batch fix you're talking about, but this
> here makes sense as an independent cleanup.

It won't help though, so this is just churn for no purpose.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: refactor eb_get_batch()

[Daniel Vetter]

On Thu, Jun 30, 2016 at 04:12:49PM +0100, Dave Gordon wrote:
> Precursor for fix to secure batch execution. We will need to be able to
> retrieve the batch VMA (as well as the batch itself) from the eb list,
> so this patch extracts that part of eb_get_batch() into a separate
> function, and moves both parts to a more logical place in the file, near
> where the eb list is created.
> 
> Also, it may not be obvious, but the current execbuffer2 ioctl interface
> requires that the buffer object containing the batch-to-be-executed be
> the LAST entry in the exec2_list[] array (I expected it to be the first!).
> 
> To clarify this, we can replace the rather obscure construct
>   "list_entry(eb->vmas.prev, ...)"
> in the old version of eb_get_batch() with the equivalent but more explicit
>   "list_last_entry(>vmas,...)"
> in the new eb_get_batch_vma() and of course add an explanatory comment.
> 
> Signed-off-by: Dave Gordon 

I have no context on the secure batch fix you're talking about, but this
here makes sense as an independent cleanup.

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 49 
> ++
>  1 file changed, 29 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 608fdc4..eea8b1f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -186,6 +186,35 @@ struct eb_vmas {
>   return ret;
>  }
>  
> +static inline struct i915_vma *
> +eb_get_batch_vma(struct eb_vmas *eb)
> +{
> + /* The batch is always the LAST item in the VMA list */
> + struct i915_vma *vma = list_last_entry(>vmas, typeof(*vma), 
> exec_list);
> +
> + return vma;
> +}
> +
> +static struct drm_i915_gem_object *
> +eb_get_batch(struct eb_vmas *eb)
> +{
> + struct i915_vma *vma = eb_get_batch_vma(eb);
> +
> + /*
> +  * SNA is doing fancy tricks with compressing batch buffers, which leads
> +  * to negative relocation deltas. Usually that works out ok since the
> +  * relocate address is still positive, except when the batch is placed
> +  * very low in the GTT. Ensure this doesn't happen.
> +  *
> +  * Note that actual hangs have only been observed on gen7, but for
> +  * paranoia do it everywhere.
> +  */
> + if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
> + vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
> +
> + return vma->obj;
> +}
> +
>  static struct i915_vma *eb_get_vma(struct eb_vmas *eb, unsigned long handle)
>  {
>   if (eb->and < 0) {
> @@ -1341,26 +1370,6 @@ static bool only_mappable_for_reloc(unsigned int flags)
>   return file_priv->bsd_ring;
>  }
>  
> -static struct drm_i915_gem_object *
> -eb_get_batch(struct eb_vmas *eb)
> -{
> - struct i915_vma *vma = list_entry(eb->vmas.prev, typeof(*vma), 
> exec_list);
> -
> - /*
> -  * SNA is doing fancy tricks with compressing batch buffers, which leads
> -  * to negative relocation deltas. Usually that works out ok since the
> -  * relocate address is still positive, except when the batch is placed
> -  * very low in the GTT. Ensure this doesn't happen.
> -  *
> -  * Note that actual hangs have only been observed on gen7, but for
> -  * paranoia do it everywhere.
> -  */
> - if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
> - vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
> -
> - return vma->obj;
> -}
> -
>  #define I915_USER_RINGS (4)
>  
>  static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = {
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/2] drm/i915: refactor eb_get_batch()

[Dave Gordon]

Precursor for fix to secure batch execution. We will need to be able to
retrieve the batch VMA (as well as the batch itself) from the eb list,
so this patch extracts that part of eb_get_batch() into a separate
function, and moves both parts to a more logical place in the file, near
where the eb list is created.

Also, it may not be obvious, but the current execbuffer2 ioctl interface
requires that the buffer object containing the batch-to-be-executed be
the LAST entry in the exec2_list[] array (I expected it to be the first!).

To clarify this, we can replace the rather obscure construct
"list_entry(eb->vmas.prev, ...)"
in the old version of eb_get_batch() with the equivalent but more explicit
"list_last_entry(>vmas,...)"
in the new eb_get_batch_vma() and of course add an explanatory comment.

Signed-off-by: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 49 ++
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1bb1f25..f6724ae 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -186,6 +186,35 @@ struct eb_vmas {
return ret;
 }
 
+static inline struct i915_vma *
+eb_get_batch_vma(struct eb_vmas *eb)
+{
+   /* The batch is always the LAST item in the VMA list */
+   struct i915_vma *vma = list_last_entry(>vmas, typeof(*vma), 
exec_list);
+
+   return vma;
+}
+
+static struct drm_i915_gem_object *
+eb_get_batch(struct eb_vmas *eb)
+{
+   struct i915_vma *vma = eb_get_batch_vma(eb);
+
+   /*
+* SNA is doing fancy tricks with compressing batch buffers, which leads
+* to negative relocation deltas. Usually that works out ok since the
+* relocate address is still positive, except when the batch is placed
+* very low in the GTT. Ensure this doesn't happen.
+*
+* Note that actual hangs have only been observed on gen7, but for
+* paranoia do it everywhere.
+*/
+   if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
+   vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
+
+   return vma->obj;
+}
+
 static struct i915_vma *eb_get_vma(struct eb_vmas *eb, unsigned long handle)
 {
if (eb->and < 0) {
@@ -1341,26 +1370,6 @@ static bool only_mappable_for_reloc(unsigned int flags)
return file_priv->bsd_ring;
 }
 
-static struct drm_i915_gem_object *
-eb_get_batch(struct eb_vmas *eb)
-{
-   struct i915_vma *vma = list_entry(eb->vmas.prev, typeof(*vma), 
exec_list);
-
-   /*
-* SNA is doing fancy tricks with compressing batch buffers, which leads
-* to negative relocation deltas. Usually that works out ok since the
-* relocate address is still positive, except when the batch is placed
-* very low in the GTT. Ensure this doesn't happen.
-*
-* Note that actual hangs have only been observed on gen7, but for
-* paranoia do it everywhere.
-*/
-   if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
-   vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
-
-   return vma->obj;
-}
-
 #define I915_USER_RINGS (4)
 
 static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = {
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/2] drm/i915: refactor eb_get_batch()

[Dave Gordon]

Precursor for fix to secure batch execution. We will need to be able to
retrieve the batch VMA (as well as the batch itself) from the eb list,
so this patch extracts that part of eb_get_batch() into a separate
function, and moves both parts to a more logical place in the file, near
where the eb list is created.

Also, it may not be obvious, but the current execbuffer2 ioctl interface
requires that the buffer object containing the batch-to-be-executed be
the LAST entry in the exec2_list[] array (I expected it to be the first!).

To clarify this, we can replace the rather obscure construct
"list_entry(eb->vmas.prev, ...)"
in the old version of eb_get_batch() with the equivalent but more explicit
"list_last_entry(>vmas,...)"
in the new eb_get_batch_vma() and of course add an explanatory comment.

Signed-off-by: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 49 ++
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 608fdc4..eea8b1f 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -186,6 +186,35 @@ struct eb_vmas {
return ret;
 }
 
+static inline struct i915_vma *
+eb_get_batch_vma(struct eb_vmas *eb)
+{
+   /* The batch is always the LAST item in the VMA list */
+   struct i915_vma *vma = list_last_entry(>vmas, typeof(*vma), 
exec_list);
+
+   return vma;
+}
+
+static struct drm_i915_gem_object *
+eb_get_batch(struct eb_vmas *eb)
+{
+   struct i915_vma *vma = eb_get_batch_vma(eb);
+
+   /*
+* SNA is doing fancy tricks with compressing batch buffers, which leads
+* to negative relocation deltas. Usually that works out ok since the
+* relocate address is still positive, except when the batch is placed
+* very low in the GTT. Ensure this doesn't happen.
+*
+* Note that actual hangs have only been observed on gen7, but for
+* paranoia do it everywhere.
+*/
+   if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
+   vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
+
+   return vma->obj;
+}
+
 static struct i915_vma *eb_get_vma(struct eb_vmas *eb, unsigned long handle)
 {
if (eb->and < 0) {
@@ -1341,26 +1370,6 @@ static bool only_mappable_for_reloc(unsigned int flags)
return file_priv->bsd_ring;
 }
 
-static struct drm_i915_gem_object *
-eb_get_batch(struct eb_vmas *eb)
-{
-   struct i915_vma *vma = list_entry(eb->vmas.prev, typeof(*vma), 
exec_list);
-
-   /*
-* SNA is doing fancy tricks with compressing batch buffers, which leads
-* to negative relocation deltas. Usually that works out ok since the
-* relocate address is still positive, except when the batch is placed
-* very low in the GTT. Ensure this doesn't happen.
-*
-* Note that actual hangs have only been observed on gen7, but for
-* paranoia do it everywhere.
-*/
-   if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
-   vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
-
-   return vma->obj;
-}
-
 #define I915_USER_RINGS (4)
 
 static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = {
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx