[PATCH 1/2] drm/i915: fix broken build

[PATCH 1/2] drm/i915: fix broken build

[Matthew Auld]

I guess this was missed in the conversion or something.

Fixes: 7bc80a5462c3 ("dma-buf: add enum dma_resv_usage v4")
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_deps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_deps.c b/drivers/gpu/drm/i915/i915_deps.c
index 999210b37325..297b8e4e42ee 100644
--- a/drivers/gpu/drm/i915/i915_deps.c
+++ b/drivers/gpu/drm/i915/i915_deps.c
@@ -226,7 +226,7 @@ int i915_deps_add_resv(struct i915_deps *deps, struct dma_resv *resv,
 	struct dma_fence *fence;
 
 	dma_resv_assert_held(resv);
-	dma_resv_for_each_fence(&iter, resv, true, fence) {
+	dma_resv_for_each_fence(&iter, resv, dma_resv_usage_rw(true), fence) {
 		int ret = i915_deps_add_dependency(deps, fence, ctx);
 
 		if (ret)
-- 
2.34.1

[PATCH 2/2] drm/i915: fix i915_gem_object_wait_moving_fence

[Matthew Auld]

All of CI is just failing with the following, which prevents loading of
the module:

    i915 0000:03:00.0: [drm] *ERROR* Scratch setup failed

Best guess is that this comes from the pin_map() for the scratch page,
which does an i915_gem_object_wait_moving_fence() somewhere. It looks
like this now calls into dma_resv_wait_timeout() which can return the
remaining timeout, leading to the caller thinking this is an error.

Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/gem/i915_gem_object.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 2998d895a6b3..1c88d4121658 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -772,9 +772,14 @@ int i915_gem_object_get_moving_fence(struct drm_i915_gem_object *obj,
 int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object *obj,
 				      bool intr)
 {
+	long ret;
+
 	assert_object_held(obj);
-	return dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
-				     intr, MAX_SCHEDULE_TIMEOUT);
+
+	ret = dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
+				    intr, MAX_SCHEDULE_TIMEOUT);
+
+	return ret < 0 ? ret : 0;
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
-- 
2.34.1

Re: [PATCH 1/2] drm/i915: fix broken build

[Christian König]

Am 07.04.22 um 18:45 schrieb Matthew Auld:
> I guess this was missed in the conversion or something.
>
> Fixes: 7bc80a5462c3 ("dma-buf: add enum dma_resv_usage v4")
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

My best guess is that this is a rebase/merge conflict. I'm 100% sure 
i915 was compiling fine before I pushed the patch.

Anyway Reviewed-by: Christian König <christian.koenig@amd.com> for the 
series.

Thanks,
Christian.

> ---
>   drivers/gpu/drm/i915/i915_deps.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_deps.c b/drivers/gpu/drm/i915/i915_deps.c
> index 999210b37325..297b8e4e42ee 100644
> --- a/drivers/gpu/drm/i915/i915_deps.c
> +++ b/drivers/gpu/drm/i915/i915_deps.c
> @@ -226,7 +226,7 @@ int i915_deps_add_resv(struct i915_deps *deps, struct dma_resv *resv,
>   	struct dma_fence *fence;
>   
>   	dma_resv_assert_held(resv);
> -	dma_resv_for_each_fence(&iter, resv, true, fence) {
> +	dma_resv_for_each_fence(&iter, resv, dma_resv_usage_rw(true), fence) {
>   		int ret = i915_deps_add_dependency(deps, fence, ctx);
>   
>   		if (ret)

Re: [PATCH 1/2] drm/i915: fix broken build

[Matthew Auld]

On 07/04/2022 17:49, Christian König wrote:
> Am 07.04.22 um 18:45 schrieb Matthew Auld:
>> I guess this was missed in the conversion or something.
>>
>> Fixes: 7bc80a5462c3 ("dma-buf: add enum dma_resv_usage v4")
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> My best guess is that this is a rebase/merge conflict. I'm 100% sure 
> i915 was compiling fine before I pushed the patch.

That was my thinking also, but building drm-misc-next I get the same error:

drivers/gpu/drm/i915/i915_deps.c: In function ‘i915_deps_add_resv’:
drivers/gpu/drm/i915/i915_deps.c:229:46: error: implicit conversion from 
‘enum <anonymous>’ to ‘enum dma_resv_usage’ [-Werror=enum-conversion]
   229 |         dma_resv_for_each_fence(&iter, resv, true, fence) {
       |                                              ^~~~
./include/linux/dma-resv.h:297:47: note: in definition of macro 
‘dma_resv_for_each_fence’
   297 |         for (dma_resv_iter_begin(cursor, obj, usage),   \
       |                                               ^~~~~

> 
> Anyway Reviewed-by: Christian König <christian.koenig@amd.com> for the 
> series.

Thanks.

> 
> Thanks,
> Christian.
> 
>> ---
>>   drivers/gpu/drm/i915/i915_deps.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_deps.c 
>> b/drivers/gpu/drm/i915/i915_deps.c
>> index 999210b37325..297b8e4e42ee 100644
>> --- a/drivers/gpu/drm/i915/i915_deps.c
>> +++ b/drivers/gpu/drm/i915/i915_deps.c
>> @@ -226,7 +226,7 @@ int i915_deps_add_resv(struct i915_deps *deps, 
>> struct dma_resv *resv,
>>       struct dma_fence *fence;
>>       dma_resv_assert_held(resv);
>> -    dma_resv_for_each_fence(&iter, resv, true, fence) {
>> +    dma_resv_for_each_fence(&iter, resv, dma_resv_usage_rw(true), 
>> fence) {
>>           int ret = i915_deps_add_dependency(deps, fence, ctx);
>>           if (ret)
> 

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix i915_gem_object_wait_moving_fence

[Lucas De Marchi]

On Thu, Apr 07, 2022 at 05:45:32PM +0100, Matthew Auld wrote:
>All of CI is just failing with the following, which prevents loading of
>the module:
>
>    i915 0000:03:00.0: [drm] *ERROR* Scratch setup failed
>
>Best guess is that this comes from the pin_map() for the scratch page,
>which does an i915_gem_object_wait_moving_fence() somewhere. It looks
>like this now calls into dma_resv_wait_timeout() which can return the
>remaining timeout, leading to the caller thinking this is an error.
>
>Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")
>Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>Cc: Christian König <christian.koenig@amd.com>
>Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

This indeed brings CI back to life.


Acked-by: Lucas De Marchi <lucas.demarchi@intel.com>


thanks
Lucas De Marchi

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix i915_gem_object_wait_moving_fence

[Lucas De Marchi]

On Thu, Apr 07, 2022 at 05:45:32PM +0100, Matthew Auld wrote:
>All of CI is just failing with the following, which prevents loading of
>the module:
>
>    i915 0000:03:00.0: [drm] *ERROR* Scratch setup failed
>
>Best guess is that this comes from the pin_map() for the scratch page,
>which does an i915_gem_object_wait_moving_fence() somewhere. It looks
>like this now calls into dma_resv_wait_timeout() which can return the
>remaining timeout, leading to the caller thinking this is an error.
>
>Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")
>Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>Cc: Christian König <christian.koenig@amd.com>
>Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>---
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>index 2998d895a6b3..1c88d4121658 100644
>--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
>+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>@@ -772,9 +772,14 @@ int i915_gem_object_get_moving_fence(struct drm_i915_gem_object *obj,
> int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object *obj,
> 				      bool intr)
> {
>+	long ret;
>+
> 	assert_object_held(obj);
>-	return dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
>-				     intr, MAX_SCHEDULE_TIMEOUT);
>+
>+	ret = dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
>+				    intr, MAX_SCHEDULE_TIMEOUT);
>+
>+	return ret < 0 ? ret : 0;

shouldn't == 0 also be an error since it would be a timeout?

Lucas De Marchi

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix i915_gem_object_wait_moving_fence

[Matthew Auld]

On 08/04/2022 06:00, Lucas De Marchi wrote:
> On Thu, Apr 07, 2022 at 05:45:32PM +0100, Matthew Auld wrote:
>> All of CI is just failing with the following, which prevents loading of
>> the module:
>>
>>    i915 0000:03:00.0: [drm] *ERROR* Scratch setup failed
>>
>> Best guess is that this comes from the pin_map() for the scratch page,
>> which does an i915_gem_object_wait_moving_fence() somewhere. It looks
>> like this now calls into dma_resv_wait_timeout() which can return the
>> remaining timeout, leading to the caller thinking this is an error.
>>
>> Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>> drivers/gpu/drm/i915/gem/i915_gem_object.c | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c 
>> b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> index 2998d895a6b3..1c88d4121658 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> @@ -772,9 +772,14 @@ int i915_gem_object_get_moving_fence(struct 
>> drm_i915_gem_object *obj,
>> int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object *obj,
>>                       bool intr)
>> {
>> +    long ret;
>> +
>>     assert_object_held(obj);
>> -    return dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
>> -                     intr, MAX_SCHEDULE_TIMEOUT);
>> +
>> +    ret = dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
>> +                    intr, MAX_SCHEDULE_TIMEOUT);
>> +
>> +    return ret < 0 ? ret : 0;
> 
> shouldn't == 0 also be an error since it would be a timeout?

Hmm, I guess so...

> 
> Lucas De Marchi

Re: [PATCH 1/2] drm/i915: fix broken build

[Matthew Auld]

On 07/04/2022 17:49, Christian König wrote:
> Am 07.04.22 um 18:45 schrieb Matthew Auld:
>> I guess this was missed in the conversion or something.
>>
>> Fixes: 7bc80a5462c3 ("dma-buf: add enum dma_resv_usage v4")
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> My best guess is that this is a rebase/merge conflict. I'm 100% sure 
> i915 was compiling fine before I pushed the patch.
> 
> Anyway Reviewed-by: Christian König <christian.koenig@amd.com> for the 
> series.

Christian, could you merge the first patch? I need to re-spin the second 
patch it seems.

> 
> Thanks,
> Christian.
> 
>> ---
>>   drivers/gpu/drm/i915/i915_deps.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_deps.c 
>> b/drivers/gpu/drm/i915/i915_deps.c
>> index 999210b37325..297b8e4e42ee 100644
>> --- a/drivers/gpu/drm/i915/i915_deps.c
>> +++ b/drivers/gpu/drm/i915/i915_deps.c
>> @@ -226,7 +226,7 @@ int i915_deps_add_resv(struct i915_deps *deps, 
>> struct dma_resv *resv,
>>       struct dma_fence *fence;
>>       dma_resv_assert_held(resv);
>> -    dma_resv_for_each_fence(&iter, resv, true, fence) {
>> +    dma_resv_for_each_fence(&iter, resv, dma_resv_usage_rw(true), 
>> fence) {
>>           int ret = i915_deps_add_dependency(deps, fence, ctx);
>>           if (ret)
> 

Re: [PATCH 1/2] drm/i915: fix broken build

[Christian König]

Am 08.04.22 um 10:32 schrieb Matthew Auld:
> On 07/04/2022 17:49, Christian König wrote:
>> Am 07.04.22 um 18:45 schrieb Matthew Auld:
>>> I guess this was missed in the conversion or something.
>>>
>>> Fixes: 7bc80a5462c3 ("dma-buf: add enum dma_resv_usage v4")
>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>>> Cc: Christian König <christian.koenig@amd.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> My best guess is that this is a rebase/merge conflict. I'm 100% sure 
>> i915 was compiling fine before I pushed the patch.
>>
>> Anyway Reviewed-by: Christian König <christian.koenig@amd.com> for 
>> the series.
>
> Christian, could you merge the first patch? I need to re-spin the 
> second patch it seems.

Pushed.

Christian.

>
>>
>> Thanks,
>> Christian.
>>
>>> ---
>>>   drivers/gpu/drm/i915/i915_deps.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_deps.c 
>>> b/drivers/gpu/drm/i915/i915_deps.c
>>> index 999210b37325..297b8e4e42ee 100644
>>> --- a/drivers/gpu/drm/i915/i915_deps.c
>>> +++ b/drivers/gpu/drm/i915/i915_deps.c
>>> @@ -226,7 +226,7 @@ int i915_deps_add_resv(struct i915_deps *deps, 
>>> struct dma_resv *resv,
>>>       struct dma_fence *fence;
>>>       dma_resv_assert_held(resv);
>>> -    dma_resv_for_each_fence(&iter, resv, true, fence) {
>>> +    dma_resv_for_each_fence(&iter, resv, dma_resv_usage_rw(true), 
>>> fence) {
>>>           int ret = i915_deps_add_dependency(deps, fence, ctx);
>>>           if (ret)
>>

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix i915_gem_object_wait_moving_fence

[Tvrtko Ursulin]

On 07/04/2022 17:45, Matthew Auld wrote:
> All of CI is just failing with the following, which prevents loading of
> the module:
> 
>      i915 0000:03:00.0: [drm] *ERROR* Scratch setup failed
> 
> Best guess is that this comes from the pin_map() for the scratch page,
> which does an i915_gem_object_wait_moving_fence() somewhere. It looks
> like this now calls into dma_resv_wait_timeout() which can return the
> remaining timeout, leading to the caller thinking this is an error.
> 
> Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")

Has this one went in bypassing i915 CI and merged via drm-misc-next? If 
so I think it's the 2nd large disruption to i915 CI flows recently so 
the lesson here is try not to bypass i915 CI when merging i915 patches.

In this particular example, unless there were merge conflicts causing 
the series not to apply against drm-tip, it should have been doable to 
copy intel-gfx on all patches and so get the CI results. (Even if just 
with --subject-prefix=CI && --suppress-cc=all before merging.)

The second question is which branch to merge through, on which I think 
i915 maintainers would have liked to be consulted.

Regards,

Tvrtko

> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_object.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index 2998d895a6b3..1c88d4121658 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -772,9 +772,14 @@ int i915_gem_object_get_moving_fence(struct drm_i915_gem_object *obj,
>   int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object *obj,
>   				      bool intr)
>   {
> +	long ret;
> +
>   	assert_object_held(obj);
> -	return dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
> -				     intr, MAX_SCHEDULE_TIMEOUT);
> +
> +	ret = dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
> +				    intr, MAX_SCHEDULE_TIMEOUT);
> +
> +	return ret < 0 ? ret : 0;
>   }
>   
>   #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix i915_gem_object_wait_moving_fence

[Christian König]

Am 08.04.22 um 11:08 schrieb Tvrtko Ursulin:
>
> On 07/04/2022 17:45, Matthew Auld wrote:
>> All of CI is just failing with the following, which prevents loading of
>> the module:
>>
>>      i915 0000:03:00.0: [drm] *ERROR* Scratch setup failed
>>
>> Best guess is that this comes from the pin_map() for the scratch page,
>> which does an i915_gem_object_wait_moving_fence() somewhere. It looks
>> like this now calls into dma_resv_wait_timeout() which can return the
>> remaining timeout, leading to the caller thinking this is an error.
>>
>> Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")
>
> Has this one went in bypassing i915 CI and merged via drm-misc-next? 
> If so I think it's the 2nd large disruption to i915 CI flows recently 
> so the lesson here is try not to bypass i915 CI when merging i915 
> patches.
>
> In this particular example, unless there were merge conflicts causing 
> the series not to apply against drm-tip, it should have been doable to 
> copy intel-gfx on all patches and so get the CI results. (Even if just 
> with --subject-prefix=CI && --suppress-cc=all before merging.)

Exactly that was the problem. I didn't got any usable CI results for 
this set because it always caused merge conflicts between i915 and 
drm-misc-next in drm-tip.

Regards,
Christian.

>
> The second question is which branch to merge through, on which I think 
> i915 maintainers would have liked to be consulted.
>
> Regards,
>
> Tvrtko
>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>   drivers/gpu/drm/i915/gem/i915_gem_object.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c 
>> b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> index 2998d895a6b3..1c88d4121658 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> @@ -772,9 +772,14 @@ int i915_gem_object_get_moving_fence(struct 
>> drm_i915_gem_object *obj,
>>   int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object *obj,
>>                         bool intr)
>>   {
>> +    long ret;
>> +
>>       assert_object_held(obj);
>> -    return dma_resv_wait_timeout(obj->base. resv, 
>> DMA_RESV_USAGE_KERNEL,
>> -                     intr, MAX_SCHEDULE_TIMEOUT);
>> +
>> +    ret = dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
>> +                    intr, MAX_SCHEDULE_TIMEOUT);
>> +
>> +    return ret < 0 ? ret : 0;
>>   }
>>     #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix i915_gem_object_wait_moving_fence

[Tvrtko Ursulin]

On 08/04/2022 10:12, Christian König wrote:
> Am 08.04.22 um 11:08 schrieb Tvrtko Ursulin:
>>
>> On 07/04/2022 17:45, Matthew Auld wrote:
>>> All of CI is just failing with the following, which prevents loading of
>>> the module:
>>>
>>>      i915 0000:03:00.0: [drm] *ERROR* Scratch setup failed
>>>
>>> Best guess is that this comes from the pin_map() for the scratch page,
>>> which does an i915_gem_object_wait_moving_fence() somewhere. It looks
>>> like this now calls into dma_resv_wait_timeout() which can return the
>>> remaining timeout, leading to the caller thinking this is an error.
>>>
>>> Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")
>>
>> Has this one went in bypassing i915 CI and merged via drm-misc-next? 
>> If so I think it's the 2nd large disruption to i915 CI flows recently 
>> so the lesson here is try not to bypass i915 CI when merging i915 
>> patches.
>>
>> In this particular example, unless there were merge conflicts causing 
>> the series not to apply against drm-tip, it should have been doable to 
>> copy intel-gfx on all patches and so get the CI results. (Even if just 
>> with --subject-prefix=CI && --suppress-cc=all before merging.)
> 
> Exactly that was the problem. I didn't got any usable CI results for 
> this set because it always caused merge conflicts between i915 and 
> drm-misc-next in drm-tip.

Then a staged approach should be used. First merge the core stuff and 
when we backmerge to drm-intel(-gt)-next send the i915 parts out.

Because knock on effect of such large of a CI fire too many many people 
on our side is very significant.

Regards,

Tvrtko

> 
> Regards,
> Christian.
> 
>>
>> The second question is which branch to merge through, on which I think 
>> i915 maintainers would have liked to be consulted.
>>
>> Regards,
>>
>> Tvrtko
>>
>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>>> Cc: Christian König <christian.koenig@amd.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> ---
>>>   drivers/gpu/drm/i915/gem/i915_gem_object.c | 9 +++++++--
>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c 
>>> b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>> index 2998d895a6b3..1c88d4121658 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>> @@ -772,9 +772,14 @@ int i915_gem_object_get_moving_fence(struct 
>>> drm_i915_gem_object *obj,
>>>   int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object *obj,
>>>                         bool intr)
>>>   {
>>> +    long ret;
>>> +
>>>       assert_object_held(obj);
>>> -    return dma_resv_wait_timeout(obj->base. resv, 
>>> DMA_RESV_USAGE_KERNEL,
>>> -                     intr, MAX_SCHEDULE_TIMEOUT);
>>> +
>>> +    ret = dma_resv_wait_timeout(obj->base. resv, DMA_RESV_USAGE_KERNEL,
>>> +                    intr, MAX_SCHEDULE_TIMEOUT);
>>> +
>>> +    return ret < 0 ? ret : 0;
>>>   }
>>>     #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> 

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix i915_gem_object_wait_moving_fence

[Christian König]

Am 08.04.22 um 11:23 schrieb Tvrtko Ursulin:
>
> On 08/04/2022 10:12, Christian König wrote:
>> Am 08.04.22 um 11:08 schrieb Tvrtko Ursulin:
>>>
>>> On 07/04/2022 17:45, Matthew Auld wrote:
>>>> All of CI is just failing with the following, which prevents 
>>>> loading of
>>>> the module:
>>>>
>>>>      i915 0000:03:00.0: [drm] *ERROR* Scratch setup failed
>>>>
>>>> Best guess is that this comes from the pin_map() for the scratch page,
>>>> which does an i915_gem_object_wait_moving_fence() somewhere. It looks
>>>> like this now calls into dma_resv_wait_timeout() which can return the
>>>> remaining timeout, leading to the caller thinking this is an error.
>>>>
>>>> Fixes: 1d7f5e6c5240 ("drm/i915: drop bo->moving dependency")
>>>
>>> Has this one went in bypassing i915 CI and merged via drm-misc-next? 
>>> If so I think it's the 2nd large disruption to i915 CI flows 
>>> recently so the lesson here is try not to bypass i915 CI when 
>>> merging i915 patches.
>>>
>>> In this particular example, unless there were merge conflicts 
>>> causing the series not to apply against drm-tip, it should have been 
>>> doable to copy intel-gfx on all patches and so get the CI results. 
>>> (Even if just with --subject-prefix=CI && --suppress-cc=all before 
>>> merging.)
>>
>> Exactly that was the problem. I didn't got any usable CI results for 
>> this set because it always caused merge conflicts between i915 and 
>> drm-misc-next in drm-tip.
>
> Then a staged approach should be used. First merge the core stuff and 
> when we backmerge to drm-intel(-gt)-next send the i915 parts out.
>
> Because knock on effect of such large of a CI fire too many many 
> people on our side is very significant.

Sorry for that. I thought we had everything covered in drm-tip, but 
looks like it still broke.

BTW: Why is the CI system failing?

Regards,
Christian.

>
> Regards,
>
> Tvrtko
>
>>
>> Regards,
>> Christian.
>>
>>>
>>> The second question is which branch to merge through, on which I 
>>> think i915 maintainers would have liked to be consulted.
>>>
>>> Regards,
>>>
>>> Tvrtko
>>>
>>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>>>> Cc: Christian König <christian.koenig@amd.com>
>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> ---
>>>>   drivers/gpu/drm/i915/gem/i915_gem_object.c | 9 +++++++--
>>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c 
>>>> b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>>> index 2998d895a6b3..1c88d4121658 100644
>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>>> @@ -772,9 +772,14 @@ int i915_gem_object_get_moving_fence(struct 
>>>> drm_i915_gem_object *obj,
>>>>   int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object 
>>>> *obj,
>>>>                         bool intr)
>>>>   {
>>>> +    long ret;
>>>> +
>>>>       assert_object_held(obj);
>>>> -    return dma_resv_wait_timeout(obj->base. resv, 
>>>> DMA_RESV_USAGE_KERNEL,
>>>> -                     intr, MAX_SCHEDULE_TIMEOUT);
>>>> +
>>>> +    ret = dma_resv_wait_timeout(obj->base. resv, 
>>>> DMA_RESV_USAGE_KERNEL,
>>>> +                    intr, MAX_SCHEDULE_TIMEOUT);
>>>> +
>>>> +    return ret < 0 ? ret : 0;
>>>>   }
>>>>     #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
>>