[PATCH 3/3] drm/amdgpu: Remove the while loop from amdgpu_job_prepare_job

Alex Deucher alexdeucher at gmail.com
Tue Oct 8 13:36:00 UTC 2024


On Tue, Oct 8, 2024 at 4:20 AM Tvrtko Ursulin <tvrtko.ursulin at igalia.com> wrote:
>
>
> On 07/10/2024 15:39, Alex Deucher wrote:
> > On Mon, Oct 7, 2024 at 8:52 AM Tvrtko Ursulin <tvrtko.ursulin at igalia.com> wrote:
> >>
> >>
> >> On 04/10/2024 15:15, Alex Deucher wrote:
> >>> Applied.  Thanks!
> >>
> >> Thanks Alex!
> >>
> >> Could you perhaps also merge
> >> https://lore.kernel.org/amd-gfx/20240813135712.82611-1-tursulin@igalia.com/
> >> via your tree? If it still applies that is.
> >
> > Just picked it up.  Thanks!
>
> Did you get only 1/2? 2/2 needs more work/review you think or?

I picked up both.  The second patch was still in the CI pipeline when
I pushed out an updated branch yesterday.  I'll push an updated tree
today.

Alex

>
> Regards,
>
> Tvrtko
>
> > Alex
> >
> >>
> >> Regards,
> >>
> >> Tvrtko
> >>
> >>> On Fri, Oct 4, 2024 at 3:28 AM Tvrtko Ursulin <tvrtko.ursulin at igalia.com> wrote:
> >>>>
> >>>>
> >>>> On 24/09/2024 13:06, Christian König wrote:
> >>>>> Am 24.09.24 um 11:51 schrieb Tvrtko Ursulin:
> >>>>>> From: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
> >>>>>>
> >>>>>> While loop makes it sound like amdgpu_vmid_grab() potentially needs to be
> >>>>>> called multiple times to produce a fence, while in reality all code paths
> >>>>>> either return an error, assign a valid job->vmid or assign a vmid which
> >>>>>> will be valid once the returned fence signals.
> >>>>>>
> >>>>>> Therefore we can remove the loop to make it clear the call does not need
> >>>>>> to be repeated.
> >>>>>>
> >>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
> >>>>>> Cc: Christian König <christian.koenig at amd.com>
> >>>>>
> >>>>> Oh yeah that's a leftover from when we still had the dependency handling
> >>>>> inside all this.
> >>>>>
> >>>>> Reviewed-by: Christian König <christian.koenig at amd.com> for the whole
> >>>>> series.
> >>>>
> >>>> Thanks - CC Alex if you could merge the trivial series please?
> >>>>
> >>>> Regards,
> >>>>
> >>>> Tvrtko
> >>>>
> >>>>>> ---
> >>>>>> I stared for a good while, going back and forth, and couldn't see that
> >>>>>> the
> >>>>>> while loop is needed. But maybe I missed something?
> >>>>>> ---
> >>>>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 2 +-
> >>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> >>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> >>>>>> index d11cb0ad8c49..85f10b59d09c 100644
> >>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> >>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> >>>>>> @@ -356,7 +356,7 @@ amdgpu_job_prepare_job(struct drm_sched_job
> >>>>>> *sched_job,
> >>>>>>         if (job->gang_submit)
> >>>>>>             fence = amdgpu_device_switch_gang(ring->adev,
> >>>>>> job->gang_submit);
> >>>>>> -    while (!fence && job->vm && !job->vmid) {
> >>>>>> +    if (!fence && job->vm && !job->vmid) {
> >>>>>>             r = amdgpu_vmid_grab(job->vm, ring, job, &fence);
> >>>>>>             if (r) {
> >>>>>>                 dev_err(ring->adev->dev, "Error getting VM ID (%d)\n", r);
> >>>>>


More information about the amd-gfx mailing list