linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder
@ 2021-11-16 15:55 Rob Clark
  2021-11-16 18:30 ` Amit Pundir
  0 siblings, 1 reply; 6+ messages in thread
From: Rob Clark @ 2021-11-16 15:55 UTC (permalink / raw)
  To: dri-devel
  Cc: Daniel Vetter, linux-arm-msm, Rob Clark, Christian König,
	Andrey Grodzovsky, David Airlie, open list

From: Rob Clark <robdclark@chromium.org>

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

Cc: Christian König <christian.koenig@amd.com>
Fixes: 9c2ba265352a drm/scheduler: ("use new iterator in drm_sched_job_add_implicit_dependencies v2")
Signed-off-by: Rob Clark <robdclark@chromium.org>
---
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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder
  2021-11-16 15:55 [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder Rob Clark
@ 2021-11-16 18:30 ` Amit Pundir
  2021-11-17  7:27   ` Christian König
  0 siblings, 1 reply; 6+ messages in thread
From: Amit Pundir @ 2021-11-16 18:30 UTC (permalink / raw)
  To: Rob Clark
  Cc: dri-devel, Daniel Vetter, linux-arm-msm, Rob Clark,
	Christian König, Andrey Grodzovsky, David Airlie, open list

On Tue, 16 Nov 2021 at 21:21, Rob Clark <robdclark@gmail.com> wrote:
>
> From: Rob Clark <robdclark@chromium.org>
>
> 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 <amit.pundir@linaro.org>

> Cc: Christian König <christian.koenig@amd.com>
> Fixes: 9c2ba265352a drm/scheduler: ("use new iterator in drm_sched_job_add_implicit_dependencies v2")
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> 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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder
  2021-11-16 18:30 ` Amit Pundir
@ 2021-11-17  7:27   ` Christian König
  2021-11-18  1:23     ` Steev Klimaszewski
  0 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2021-11-17  7:27 UTC (permalink / raw)
  To: Amit Pundir, Rob Clark
  Cc: dri-devel, Daniel Vetter, linux-arm-msm, Rob Clark,
	Andrey Grodzovsky, David Airlie, open list

Am 16.11.21 um 19:30 schrieb Amit Pundir:
> On Tue, 16 Nov 2021 at 21:21, Rob Clark <robdclark@gmail.com> wrote:
>> From: Rob Clark <robdclark@chromium.org>
>>
>> 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 <amit.pundir@linaro.org>

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 <christian.koenig@amd.com>
>> Fixes: 9c2ba265352a drm/scheduler: ("use new iterator in drm_sched_job_add_implicit_dependencies v2")
>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>> ---
>> 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
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder
  2021-11-17  7:27   ` Christian König
@ 2021-11-18  1:23     ` Steev Klimaszewski
  2021-11-18  3:09       ` Rob Clark
  0 siblings, 1 reply; 6+ messages in thread
From: Steev Klimaszewski @ 2021-11-18  1:23 UTC (permalink / raw)
  To: Christian König, Amit Pundir, Rob Clark
  Cc: dri-devel, Daniel Vetter, linux-arm-msm, Rob Clark,
	Andrey Grodzovsky, David Airlie, open list


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 <robdclark@gmail.com> wrote:
>>> From: Rob Clark <robdclark@chromium.org>
>>>
>>> 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 <amit.pundir@linaro.org>
>
> 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder
  2021-11-18  1:23     ` Steev Klimaszewski
@ 2021-11-18  3:09       ` Rob Clark
  2021-11-18  6:59         ` Christian König
  0 siblings, 1 reply; 6+ messages in thread
From: Rob Clark @ 2021-11-18  3:09 UTC (permalink / raw)
  To: Steev Klimaszewski
  Cc: Christian König, Amit Pundir, dri-devel, Daniel Vetter,
	linux-arm-msm, Rob Clark, Andrey Grodzovsky, David Airlie,
	open list

On Wed, Nov 17, 2021 at 5:23 PM Steev Klimaszewski <steev@kali.org> 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 <robdclark@gmail.com> wrote:
> >>> From: Rob Clark <robdclark@chromium.org>
> >>>
> >>> 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 <amit.pundir@linaro.org>
> >
> > 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder
  2021-11-18  3:09       ` Rob Clark
@ 2021-11-18  6:59         ` Christian König
  0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2021-11-18  6:59 UTC (permalink / raw)
  To: Rob Clark, Steev Klimaszewski
  Cc: Amit Pundir, dri-devel, Daniel Vetter, linux-arm-msm, Rob Clark,
	Andrey Grodzovsky, David Airlie, open list

Am 18.11.21 um 04:09 schrieb Rob Clark:
> On Wed, Nov 17, 2021 at 5:23 PM Steev Klimaszewski <steev@kali.org> 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 <robdclark@gmail.com> wrote:
>>>>> From: Rob Clark <robdclark@chromium.org>
>>>>>
>>>>> 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 <amit.pundir@linaro.org>
>>> 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-18  6:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-16 15:55 [PATCH] drm/scheduler: fix drm_sched_job_add_implicit_dependencies harder Rob Clark
2021-11-16 18:30 ` Amit Pundir
2021-11-17  7:27   ` Christian König
2021-11-18  1:23     ` Steev Klimaszewski
2021-11-18  3:09       ` Rob Clark
2021-11-18  6:59         ` Christian König

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).