2021-11-16 15:50:43

by Rob Clark

[permalink] [raw]
Subject: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder

From: Rob Clark <[email protected]>

drm_sched_job_add_dependency() could drop the last ref, so we need to do
the dma_fence_get() first.

Cc: Christian König <[email protected]>
Fixes: 9c2ba265352a drm/scheduler: ("use new iterator in drm_sched_job_add_implicit_dependencies v2")
Signed-off-by: Rob Clark <[email protected]>
---
Applies on top of "drm/scheduler: fix drm_sched_job_add_implicit_dependencies"
but I don't think that has a stable commit sha yet.

drivers/gpu/drm/scheduler/sched_main.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
index 94fe51b3caa2..f91fb31ab7a7 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -704,12 +704,13 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job,
int ret;

dma_resv_for_each_fence(&cursor, obj->resv, write, fence) {
- ret = drm_sched_job_add_dependency(job, fence);
- if (ret)
- return ret;
-
/* Make sure to grab an additional ref on the added fence */
dma_fence_get(fence);
+ ret = drm_sched_job_add_dependency(job, fence);
+ if (ret) {
+ dma_fence_put(fence);
+ return ret;
+ }
}
return 0;
}
--
2.33.1



2021-11-16 18:31:11

by Amit Pundir

[permalink] [raw]
Subject: Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder

On Tue, 16 Nov 2021 at 21:21, Rob Clark <[email protected]> wrote:
>
> From: Rob Clark <[email protected]>
>
> drm_sched_job_add_dependency() could drop the last ref, so we need to do
> the dma_fence_get() first.
>

It fixed the splats I saw on RB5 (sm8250 | A650). Thanks.

Tested-by: Amit Pundir <[email protected]>

> Cc: Christian König <[email protected]>
> Fixes: 9c2ba265352a drm/scheduler: ("use new iterator in drm_sched_job_add_implicit_dependencies v2")
> Signed-off-by: Rob Clark <[email protected]>
> ---
> Applies on top of "drm/scheduler: fix drm_sched_job_add_implicit_dependencies"
> but I don't think that has a stable commit sha yet.
>
> drivers/gpu/drm/scheduler/sched_main.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
> index 94fe51b3caa2..f91fb31ab7a7 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -704,12 +704,13 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job,
> int ret;
>
> dma_resv_for_each_fence(&cursor, obj->resv, write, fence) {
> - ret = drm_sched_job_add_dependency(job, fence);
> - if (ret)
> - return ret;
> -
> /* Make sure to grab an additional ref on the added fence */
> dma_fence_get(fence);
> + ret = drm_sched_job_add_dependency(job, fence);
> + if (ret) {
> + dma_fence_put(fence);
> + return ret;
> + }
> }
> return 0;
> }
> --
> 2.33.1
>

2021-11-17 07:27:11

by Christian König

[permalink] [raw]
Subject: Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder

Am 16.11.21 um 19:30 schrieb Amit Pundir:
> On Tue, 16 Nov 2021 at 21:21, Rob Clark <[email protected]> wrote:
>> From: Rob Clark <[email protected]>
>>
>> drm_sched_job_add_dependency() could drop the last ref, so we need to do
>> the dma_fence_get() first.
>>
> It fixed the splats I saw on RB5 (sm8250 | A650). Thanks.
>
> Tested-by: Amit Pundir <[email protected]>

I've added my rb, pushed this with the original fix to drm-misc-fixes
and cleaned up the obvious fallout between drm-misc-fixes and
drm-misc-next in drm-tip.

Thanks for the help and sorry for the noise,
Christian.

>
>> Cc: Christian König <[email protected]>
>> Fixes: 9c2ba265352a drm/scheduler: ("use new iterator in drm_sched_job_add_implicit_dependencies v2")
>> Signed-off-by: Rob Clark <[email protected]>
>> ---
>> Applies on top of "drm/scheduler: fix drm_sched_job_add_implicit_dependencies"
>> but I don't think that has a stable commit sha yet.
>>
>> drivers/gpu/drm/scheduler/sched_main.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
>> index 94fe51b3caa2..f91fb31ab7a7 100644
>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>> @@ -704,12 +704,13 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job,
>> int ret;
>>
>> dma_resv_for_each_fence(&cursor, obj->resv, write, fence) {
>> - ret = drm_sched_job_add_dependency(job, fence);
>> - if (ret)
>> - return ret;
>> -
>> /* Make sure to grab an additional ref on the added fence */
>> dma_fence_get(fence);
>> + ret = drm_sched_job_add_dependency(job, fence);
>> + if (ret) {
>> + dma_fence_put(fence);
>> + return ret;
>> + }
>> }
>> return 0;
>> }
>> --
>> 2.33.1
>>


2021-11-18 01:23:23

by Steev Klimaszewski

[permalink] [raw]
Subject: Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder


On 11/17/21 1:27 AM, Christian König wrote:
> Am 16.11.21 um 19:30 schrieb Amit Pundir:
>> On Tue, 16 Nov 2021 at 21:21, Rob Clark <[email protected]> wrote:
>>> From: Rob Clark <[email protected]>
>>>
>>> drm_sched_job_add_dependency() could drop the last ref, so we need
>>> to do
>>> the dma_fence_get() first.
>>>
>> It fixed the splats I saw on RB5 (sm8250 | A650). Thanks.
>>
>> Tested-by: Amit Pundir <[email protected]>
>
> I've added my rb, pushed this with the original fix to drm-misc-fixes
> and cleaned up the obvious fallout between drm-misc-fixes and
> drm-misc-next in drm-tip.
>
> Thanks for the help and sorry for the noise,
> Christian.
>
I've run into this splat on the Lenovo Yoga C630 on 5.16-rc1 - are these
2 patches (which fix it) going to be heading to 5.16 or were they
targeted at 5.17?

-- steev


2021-11-18 03:04:00

by Rob Clark

[permalink] [raw]
Subject: Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder

On Wed, Nov 17, 2021 at 5:23 PM Steev Klimaszewski <[email protected]> wrote:
>
>
> On 11/17/21 1:27 AM, Christian König wrote:
> > Am 16.11.21 um 19:30 schrieb Amit Pundir:
> >> On Tue, 16 Nov 2021 at 21:21, Rob Clark <[email protected]> wrote:
> >>> From: Rob Clark <[email protected]>
> >>>
> >>> drm_sched_job_add_dependency() could drop the last ref, so we need
> >>> to do
> >>> the dma_fence_get() first.
> >>>
> >> It fixed the splats I saw on RB5 (sm8250 | A650). Thanks.
> >>
> >> Tested-by: Amit Pundir <[email protected]>
> >
> > I've added my rb, pushed this with the original fix to drm-misc-fixes
> > and cleaned up the obvious fallout between drm-misc-fixes and
> > drm-misc-next in drm-tip.
> >
> > Thanks for the help and sorry for the noise,
> > Christian.
> >
> I've run into this splat on the Lenovo Yoga C630 on 5.16-rc1 - are these
> 2 patches (which fix it) going to be heading to 5.16 or were they
> targeted at 5.17?

these should be for v5.16

BR,
-R

2021-11-18 06:59:42

by Christian König

[permalink] [raw]
Subject: Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder

Am 18.11.21 um 04:09 schrieb Rob Clark:
> On Wed, Nov 17, 2021 at 5:23 PM Steev Klimaszewski <[email protected]> wrote:
>>
>> On 11/17/21 1:27 AM, Christian König wrote:
>>> Am 16.11.21 um 19:30 schrieb Amit Pundir:
>>>> On Tue, 16 Nov 2021 at 21:21, Rob Clark <[email protected]> wrote:
>>>>> From: Rob Clark <[email protected]>
>>>>>
>>>>> drm_sched_job_add_dependency() could drop the last ref, so we need
>>>>> to do
>>>>> the dma_fence_get() first.
>>>>>
>>>> It fixed the splats I saw on RB5 (sm8250 | A650). Thanks.
>>>>
>>>> Tested-by: Amit Pundir <[email protected]>
>>> I've added my rb, pushed this with the original fix to drm-misc-fixes
>>> and cleaned up the obvious fallout between drm-misc-fixes and
>>> drm-misc-next in drm-tip.
>>>
>>> Thanks for the help and sorry for the noise,
>>> Christian.
>>>
>> I've run into this splat on the Lenovo Yoga C630 on 5.16-rc1 - are these
>> 2 patches (which fix it) going to be heading to 5.16 or were they
>> targeted at 5.17?
> these should be for v5.16

Yeah, they are already queued up for -rc2.

Regards,
Christian.

>
> BR,
> -R