* [PATCH] drm/i915: Use i915_gem_object_ggtt_pin_ww for reloc_iomap
@ 2022-05-11 11:52 Maarten Lankhorst
2022-05-11 18:23 ` Matthew Auld
0 siblings, 1 reply; 4+ messages in thread
From: Maarten Lankhorst @ 2022-05-11 11:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Mateusz Jończyk, Hans de Goede, Matthew Auld, dri-devel
Instead of its own path, use the common path when it doesn't result
in evicting any vma. This fixes the case where we don't wait for
binding.
Fixes: b5cfe6f7a6e1 ("drm/i915: Remove short-term pins from execbuf, v6.")
Cc: Matthew Auld <matthew.auld@intel.com>
Reported-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 498b458fd784..919d01082909 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1262,14 +1262,12 @@ static void *reloc_iomap(struct i915_vma *batch,
* Only attempt to pin the batch buffer to ggtt if the current batch
* is not inside ggtt, or the batch buffer is not misplaced.
*/
- if (!i915_is_ggtt(batch->vm)) {
+ if (!i915_is_ggtt(batch->vm) ||
+ !i915_vma_misplaced(batch, 0, 0, PIN_MAPPABLE)) {
vma = i915_gem_object_ggtt_pin_ww(obj, &eb->ww, NULL, 0, 0,
PIN_MAPPABLE |
PIN_NONBLOCK /* NOWARN */ |
PIN_NOEVICT);
- } else if (i915_vma_is_map_and_fenceable(batch)) {
- __i915_vma_pin(batch);
- vma = batch;
}
if (vma == ERR_PTR(-EDEADLK))
--
2.36.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Use i915_gem_object_ggtt_pin_ww for reloc_iomap
2022-05-11 11:52 [PATCH] drm/i915: Use i915_gem_object_ggtt_pin_ww for reloc_iomap Maarten Lankhorst
@ 2022-05-11 18:23 ` Matthew Auld
2022-05-11 18:38 ` Maarten Lankhorst
0 siblings, 1 reply; 4+ messages in thread
From: Matthew Auld @ 2022-05-11 18:23 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
Cc: Mateusz Jończyk, Hans de Goede, dri-devel
On 11/05/2022 12:52, Maarten Lankhorst wrote:
> Instead of its own path, use the common path when it doesn't result
> in evicting any vma. This fixes the case where we don't wait for
> binding.
>
https://gitlab.freedesktop.org/drm/intel/-/issues/5806
If I'm reading that correctly waiting for the bind doesn't seem to help?
> Fixes: b5cfe6f7a6e1 ("drm/i915: Remove short-term pins from execbuf, v6.")
> Cc: Matthew Auld <matthew.auld@intel.com>
> Reported-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 498b458fd784..919d01082909 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -1262,14 +1262,12 @@ static void *reloc_iomap(struct i915_vma *batch,
> * Only attempt to pin the batch buffer to ggtt if the current batch
> * is not inside ggtt, or the batch buffer is not misplaced.
> */
> - if (!i915_is_ggtt(batch->vm)) {
> + if (!i915_is_ggtt(batch->vm) ||
> + !i915_vma_misplaced(batch, 0, 0, PIN_MAPPABLE)) {
> vma = i915_gem_object_ggtt_pin_ww(obj, &eb->ww, NULL, 0, 0,
> PIN_MAPPABLE |
> PIN_NONBLOCK /* NOWARN */ |
> PIN_NOEVICT);
> - } else if (i915_vma_is_map_and_fenceable(batch)) {
> - __i915_vma_pin(batch);
> - vma = batch;
> }
>
> if (vma == ERR_PTR(-EDEADLK))
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Use i915_gem_object_ggtt_pin_ww for reloc_iomap
2022-05-11 18:23 ` Matthew Auld
@ 2022-05-11 18:38 ` Maarten Lankhorst
2022-05-12 16:14 ` Matthew Auld
0 siblings, 1 reply; 4+ messages in thread
From: Maarten Lankhorst @ 2022-05-11 18:38 UTC (permalink / raw)
To: Matthew Auld, intel-gfx; +Cc: Mateusz Jończyk, Hans de Goede, dri-devel
Op 11-05-2022 om 20:23 schreef Matthew Auld:
> On 11/05/2022 12:52, Maarten Lankhorst wrote:
>> Instead of its own path, use the common path when it doesn't result
>> in evicting any vma. This fixes the case where we don't wait for
>> binding.
>>
>
> https://gitlab.freedesktop.org/drm/intel/-/issues/5806
>
> If I'm reading that correctly waiting for the bind doesn't seem to help?
I suspect the actual pinning there might do some stuff that we are not doing.
It was working before the change, and manually calling pin caused the failure, so I reverted it back to what was working before. It was specifically the manual pin code that was failing.
I can change the commit message if it helps.
~Maarten
>> Fixes: b5cfe6f7a6e1 ("drm/i915: Remove short-term pins from execbuf, v6.")
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Reported-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
>> Tested-by: Hans de Goede <hdegoede@redhat.com>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> index 498b458fd784..919d01082909 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> @@ -1262,14 +1262,12 @@ static void *reloc_iomap(struct i915_vma *batch,
>> * Only attempt to pin the batch buffer to ggtt if the current batch
>> * is not inside ggtt, or the batch buffer is not misplaced.
>> */
>> - if (!i915_is_ggtt(batch->vm)) {
>> + if (!i915_is_ggtt(batch->vm) ||
>> + !i915_vma_misplaced(batch, 0, 0, PIN_MAPPABLE)) {
>> vma = i915_gem_object_ggtt_pin_ww(obj, &eb->ww, NULL, 0, 0,
>> PIN_MAPPABLE |
>> PIN_NONBLOCK /* NOWARN */ |
>> PIN_NOEVICT);
>> - } else if (i915_vma_is_map_and_fenceable(batch)) {
>> - __i915_vma_pin(batch);
>> - vma = batch;
>> }
>> if (vma == ERR_PTR(-EDEADLK))
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Use i915_gem_object_ggtt_pin_ww for reloc_iomap
2022-05-11 18:38 ` Maarten Lankhorst
@ 2022-05-12 16:14 ` Matthew Auld
0 siblings, 0 replies; 4+ messages in thread
From: Matthew Auld @ 2022-05-12 16:14 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
Cc: Mateusz Jończyk, Hans de Goede, dri-devel
On 11/05/2022 19:38, Maarten Lankhorst wrote:
> Op 11-05-2022 om 20:23 schreef Matthew Auld:
>> On 11/05/2022 12:52, Maarten Lankhorst wrote:
>>> Instead of its own path, use the common path when it doesn't result
>>> in evicting any vma. This fixes the case where we don't wait for
>>> binding.
>>>
>>
>> https://gitlab.freedesktop.org/drm/intel/-/issues/5806
>>
>> If I'm reading that correctly waiting for the bind doesn't seem to help?
>
> I suspect the actual pinning there might do some stuff that we are not doing.
>
> It was working before the change, and manually calling pin caused the failure, so I reverted it back to what was working before. It was specifically the manual pin code that was failing.
>
> I can change the commit message if it helps.
Hmm strange. With the commit message updated,
Acked-by: Matthew Auld <matthew.auld@intel.com>
>
> ~Maarten
>
>>> Fixes: b5cfe6f7a6e1 ("drm/i915: Remove short-term pins from execbuf, v6.")
>>> Cc: Matthew Auld <matthew.auld@intel.com>
>>> Reported-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
>>> Tested-by: Hans de Goede <hdegoede@redhat.com>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 ++----
>>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>> index 498b458fd784..919d01082909 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>> @@ -1262,14 +1262,12 @@ static void *reloc_iomap(struct i915_vma *batch,
>>> * Only attempt to pin the batch buffer to ggtt if the current batch
>>> * is not inside ggtt, or the batch buffer is not misplaced.
>>> */
>>> - if (!i915_is_ggtt(batch->vm)) {
>>> + if (!i915_is_ggtt(batch->vm) ||
>>> + !i915_vma_misplaced(batch, 0, 0, PIN_MAPPABLE)) {
>>> vma = i915_gem_object_ggtt_pin_ww(obj, &eb->ww, NULL, 0, 0,
>>> PIN_MAPPABLE |
>>> PIN_NONBLOCK /* NOWARN */ |
>>> PIN_NOEVICT);
>>> - } else if (i915_vma_is_map_and_fenceable(batch)) {
>>> - __i915_vma_pin(batch);
>>> - vma = batch;
>>> }
>>> if (vma == ERR_PTR(-EDEADLK))
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-12 16:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-11 11:52 [PATCH] drm/i915: Use i915_gem_object_ggtt_pin_ww for reloc_iomap Maarten Lankhorst
2022-05-11 18:23 ` Matthew Auld
2022-05-11 18:38 ` Maarten Lankhorst
2022-05-12 16:14 ` Matthew Auld
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).