On Tue, Oct 8, 2024 at 4:20 AM Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx> wrote: > > > On 07/10/2024 15:39, Alex Deucher wrote: > > On Mon, Oct 7, 2024 at 8:52 AM Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx> 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@xxxxxxxxxx/ > >> 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@xxxxxxxxxx> 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@xxxxxxxxxx> > >>>>>> > >>>>>> 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@xxxxxxxxxx> > >>>>>> Cc: Christian König <christian.koenig@xxxxxxx> > >>>>> > >>>>> Oh yeah that's a leftover from when we still had the dependency handling > >>>>> inside all this. > >>>>> > >>>>> Reviewed-by: Christian König <christian.koenig@xxxxxxx> 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); > >>>>>
