| Message ID | 20240906180639.12218-2-tursulin@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amdgpu: No need for dynamic DRM priority? | expand |
Adding Leo as well. Am 06.09.24 um 20:06 schrieb Tvrtko Ursulin: > From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > > According to Christian the dynamic DRM priority override was only > interesting before the hardware priority (dona via > drm_sched_entity_modify_sched()) existed. Furthermore, both > overrides also only work somewhat on paper while in reality they are only > effective if the entity is idle, which is something userspace is unaware > of when using the AMDGPU_SCHED_OP_*_PRIORITY_OVERRIDE uapi. > > Therefore follow Christian's advice and remove this call completely. The only potential use case I can come up with would be for multimedia engines since we never implemented the hardware priority handling for them. @Leo do we have any use case relying on that? As far as I know it doesn't work for UVD/VCE anyway because those engines can't switch loads and for VCN we should probably just implement different hw priorities. Christian. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > Cc: Christian König <christian.koenig@amd.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > index c43d1b6e5d66..2480b3227dad 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > @@ -820,10 +820,6 @@ static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx, > struct drm_gpu_scheduler **scheds = NULL; > unsigned num_scheds; > > - /* set sw priority */ > - drm_sched_entity_set_priority(&aentity->entity, > - amdgpu_ctx_to_drm_sched_prio(priority)); > - > /* set hw priority */ > if (hw_ip == AMDGPU_HW_IP_COMPUTE || hw_ip == AMDGPU_HW_IP_GFX) { > hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);
On Mon, Sep 9, 2024 at 8:23 AM Christian König <christian.koenig@amd.com> wrote: > > Adding Leo as well. > > Am 06.09.24 um 20:06 schrieb Tvrtko Ursulin: > > From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > > > > According to Christian the dynamic DRM priority override was only > > interesting before the hardware priority (dona via > > drm_sched_entity_modify_sched()) existed. Furthermore, both > > overrides also only work somewhat on paper while in reality they are only > > effective if the entity is idle, which is something userspace is unaware > > of when using the AMDGPU_SCHED_OP_*_PRIORITY_OVERRIDE uapi. > > > > Therefore follow Christian's advice and remove this call completely. > > The only potential use case I can come up with would be for multimedia > engines since we never implemented the hardware priority handling for them. > > @Leo do we have any use case relying on that? As far as I know it > doesn't work for UVD/VCE anyway because those engines can't switch loads > and for VCN we should probably just implement different hw priorities. VCN has moved away from multiple queues to a single queue; and the next iteration is user queues. On the older hardware, there was a high priority queue, but IIRC, the firmware just prioritized fetches from that queue; there is still only one engine that can process the work. As such, I'd expect it to operate similarly to the drm scheduler. For both GFX and VCN, having a high priority queue, even if it was just in the drm scheduler, helped at least some workloads we had in the past. Alex > > Christian. > > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > > Cc: Christian König <christian.koenig@amd.com> > > Cc: Alex Deucher <alexander.deucher@amd.com> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 4 ---- > > 1 file changed, 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > > index c43d1b6e5d66..2480b3227dad 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c > > @@ -820,10 +820,6 @@ static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx, > > struct drm_gpu_scheduler **scheds = NULL; > > unsigned num_scheds; > > > > - /* set sw priority */ > > - drm_sched_entity_set_priority(&aentity->entity, > > - amdgpu_ctx_to_drm_sched_prio(priority)); > > - > > /* set hw priority */ > > if (hw_ip == AMDGPU_HW_IP_COMPUTE || hw_ip == AMDGPU_HW_IP_GFX) { > > hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip); >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c index c43d1b6e5d66..2480b3227dad 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c @@ -820,10 +820,6 @@ static void amdgpu_ctx_set_entity_priority(struct amdgpu_ctx *ctx, struct drm_gpu_scheduler **scheds = NULL; unsigned num_scheds; - /* set sw priority */ - drm_sched_entity_set_priority(&aentity->entity, - amdgpu_ctx_to_drm_sched_prio(priority)); - /* set hw priority */ if (hw_ip == AMDGPU_HW_IP_COMPUTE || hw_ip == AMDGPU_HW_IP_GFX) { hw_prio = amdgpu_ctx_get_hw_prio(ctx, hw_ip);