On Thu, Nov 9, 2023 at 4:35 PM Luben Tuikov <ltuikov89@xxxxxxxxx> wrote:
>
> On 2023-11-09 11:13, Alex Deucher wrote:
> > Ping?
> >
> > On Wed, Nov 8, 2023 at 1:42 PM Alex Deucher <alexander.deucher@xxxxxxx> wrote:
> >>
> >> We need kernel scheduling entities to deal with handle clean up
> >> if apps are not cleaned up properly. With commit 56e449603f0ac5
> >> ("drm/sched: Convert the GPU scheduler to variable number of run-queues")
> >> the scheduler entities have to be created after scheduler init, so
> >> change the ordering to fix this.
> >>
> >> v2: Leave logic in UVD and VCE code
> >>
> >> Fixes: 56e449603f0ac5 ("drm/sched: Convert the GPU scheduler to variable number of run-queues")
> >> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> >> Cc: ltuikov89@xxxxxxxxx
>
> Reviewed-by: Luben Tuikov <ltuikov89@xxxxxxxxx>
Thanks Luben!
Alex
>
> Regards,
> Luben
>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++++++++++
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 22 ++++++++++----------
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h | 2 +-
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 24 +++++++++++-----------
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h | 2 +-
> >> drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c | 2 --
> >> drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c | 2 --
> >> drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c | 2 --
> >> drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 2 --
> >> drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c | 4 ----
> >> drivers/gpu/drm/amd/amdgpu/vce_v2_0.c | 2 --
> >> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 2 --
> >> drivers/gpu/drm/amd/amdgpu/vce_v4_0.c | 5 -----
> >> 13 files changed, 37 insertions(+), 46 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index 43a95feba884..03e669c34033 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -2499,6 +2499,18 @@ static int amdgpu_device_init_schedulers(struct amdgpu_device *adev)
> >> ring->name);
> >> return r;
> >> }
> >> + r = amdgpu_uvd_entity_init(adev, ring);
> >> + if (r) {
> >> + DRM_ERROR("Failed to create UVD scheduling entity on ring %s.\n",
> >> + ring->name);
> >> + return r;
> >> + }
> >> + r = amdgpu_vce_entity_init(adev, ring);
> >> + if (r) {
> >> + DRM_ERROR("Failed to create VCE scheduling entity on ring %s.\n",
> >> + ring->name);
> >> + return r;
> >> + }
> >> }
> >>
> >> amdgpu_xcp_update_partition_sched_list(adev);
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> >> index 815b7c34ed33..65949cc7abb9 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> >> @@ -399,20 +399,20 @@ int amdgpu_uvd_sw_fini(struct amdgpu_device *adev)
> >> *
> >> * @adev: amdgpu_device pointer
> >> *
> >> + * Initialize the entity used for handle management in the kernel driver.
> >> */
> >> -int amdgpu_uvd_entity_init(struct amdgpu_device *adev)
> >> +int amdgpu_uvd_entity_init(struct amdgpu_device *adev, struct amdgpu_ring *ring)
> >> {
> >> - struct amdgpu_ring *ring;
> >> - struct drm_gpu_scheduler *sched;
> >> - int r;
> >> + if (ring == &adev->uvd.inst[0].ring) {
> >> + struct drm_gpu_scheduler *sched = &ring->sched;
> >> + int r;
> >>
> >> - ring = &adev->uvd.inst[0].ring;
> >> - sched = &ring->sched;
> >> - r = drm_sched_entity_init(&adev->uvd.entity, DRM_SCHED_PRIORITY_NORMAL,
> >> - &sched, 1, NULL);
> >> - if (r) {
> >> - DRM_ERROR("Failed setting up UVD kernel entity.\n");
> >> - return r;
> >> + r = drm_sched_entity_init(&adev->uvd.entity, DRM_SCHED_PRIORITY_NORMAL,
> >> + &sched, 1, NULL);
> >> + if (r) {
> >> + DRM_ERROR("Failed setting up UVD kernel entity.\n");
> >> + return r;
> >> + }
> >> }
> >>
> >> return 0;
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> >> index a9f342537c68..9dfad2f48ef4 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
> >> @@ -73,7 +73,7 @@ struct amdgpu_uvd {
> >>
> >> int amdgpu_uvd_sw_init(struct amdgpu_device *adev);
> >> int amdgpu_uvd_sw_fini(struct amdgpu_device *adev);
> >> -int amdgpu_uvd_entity_init(struct amdgpu_device *adev);
> >> +int amdgpu_uvd_entity_init(struct amdgpu_device *adev, struct amdgpu_ring *ring);
> >> int amdgpu_uvd_prepare_suspend(struct amdgpu_device *adev);
> >> int amdgpu_uvd_suspend(struct amdgpu_device *adev);
> >> int amdgpu_uvd_resume(struct amdgpu_device *adev);
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> >> index 1904edf68407..0954447f689d 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> >> @@ -231,20 +231,20 @@ int amdgpu_vce_sw_fini(struct amdgpu_device *adev)
> >> *
> >> * @adev: amdgpu_device pointer
> >> *
> >> + * Initialize the entity used for handle management in the kernel driver.
> >> */
> >> -int amdgpu_vce_entity_init(struct amdgpu_device *adev)
> >> +int amdgpu_vce_entity_init(struct amdgpu_device *adev, struct amdgpu_ring *ring)
> >> {
> >> - struct amdgpu_ring *ring;
> >> - struct drm_gpu_scheduler *sched;
> >> - int r;
> >> -
> >> - ring = &adev->vce.ring[0];
> >> - sched = &ring->sched;
> >> - r = drm_sched_entity_init(&adev->vce.entity, DRM_SCHED_PRIORITY_NORMAL,
> >> - &sched, 1, NULL);
> >> - if (r != 0) {
> >> - DRM_ERROR("Failed setting up VCE run queue.\n");
> >> - return r;
> >> + if (ring == &adev->vce.ring[0]) {
> >> + struct drm_gpu_scheduler *sched = &ring->sched;
> >> + int r;
> >> +
> >> + r = drm_sched_entity_init(&adev->vce.entity, DRM_SCHED_PRIORITY_NORMAL,
> >> + &sched, 1, NULL);
> >> + if (r != 0) {
> >> + DRM_ERROR("Failed setting up VCE run queue.\n");
> >> + return r;
> >> + }
> >> }
> >>
> >> return 0;
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> >> index ea680fc9a6c3..6e53f872d084 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.h
> >> @@ -55,7 +55,7 @@ struct amdgpu_vce {
> >>
> >> int amdgpu_vce_sw_init(struct amdgpu_device *adev, unsigned long size);
> >> int amdgpu_vce_sw_fini(struct amdgpu_device *adev);
> >> -int amdgpu_vce_entity_init(struct amdgpu_device *adev);
> >> +int amdgpu_vce_entity_init(struct amdgpu_device *adev, struct amdgpu_ring *ring);
> >> int amdgpu_vce_suspend(struct amdgpu_device *adev);
> >> int amdgpu_vce_resume(struct amdgpu_device *adev);
> >> void amdgpu_vce_free_handles(struct amdgpu_device *adev, struct drm_file *filp);
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
> >> index 58a8f78c003c..a6006f231c65 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
> >> @@ -577,8 +577,6 @@ static int uvd_v3_1_sw_init(void *handle)
> >> ptr += ucode_len;
> >> memcpy(&adev->uvd.keyselect, ptr, 4);
> >>
> >> - r = amdgpu_uvd_entity_init(adev);
> >> -
> >> return r;
> >> }
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> >> index d3b1e31f5450..1aa09ad7bbe3 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
> >> @@ -127,8 +127,6 @@ static int uvd_v4_2_sw_init(void *handle)
> >> if (r)
> >> return r;
> >>
> >> - r = amdgpu_uvd_entity_init(adev);
> >> -
> >> return r;
> >> }
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> >> index 5a8116437abf..f8b229b75435 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> >> @@ -125,8 +125,6 @@ static int uvd_v5_0_sw_init(void *handle)
> >> if (r)
> >> return r;
> >>
> >> - r = amdgpu_uvd_entity_init(adev);
> >> -
> >> return r;
> >> }
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> >> index 74c09230aeb3..a9a6880f44e3 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> >> @@ -432,8 +432,6 @@ static int uvd_v6_0_sw_init(void *handle)
> >> }
> >> }
> >>
> >> - r = amdgpu_uvd_entity_init(adev);
> >> -
> >> return r;
> >> }
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> >> index 1c42cf10cc29..6068b784dc69 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
> >> @@ -480,10 +480,6 @@ static int uvd_v7_0_sw_init(void *handle)
> >> if (r)
> >> return r;
> >>
> >> - r = amdgpu_uvd_entity_init(adev);
> >> - if (r)
> >> - return r;
> >> -
> >> r = amdgpu_virt_alloc_mm_table(adev);
> >> if (r)
> >> return r;
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> >> index 67eb01fef789..a08e7abca423 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v2_0.c
> >> @@ -441,8 +441,6 @@ static int vce_v2_0_sw_init(void *handle)
> >> return r;
> >> }
> >>
> >> - r = amdgpu_vce_entity_init(adev);
> >> -
> >> return r;
> >> }
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> >> index 18f6e62af339..f4760748d349 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> >> @@ -450,8 +450,6 @@ static int vce_v3_0_sw_init(void *handle)
> >> return r;
> >> }
> >>
> >> - r = amdgpu_vce_entity_init(adev);
> >> -
> >> return r;
> >> }
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> >> index e0b70cd3b697..06d787385ad4 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> >> @@ -486,11 +486,6 @@ static int vce_v4_0_sw_init(void *handle)
> >> return r;
> >> }
> >>
> >> -
> >> - r = amdgpu_vce_entity_init(adev);
> >> - if (r)
> >> - return r;
> >> -
> >> r = amdgpu_virt_alloc_mm_table(adev);
> >> if (r)
> >> return r;
> >> --
> >> 2.41.0
> >>
