[PATCH] drm/amdgpu: disable ring_muxer if mcbp is off

[Zhu, Jiadong]

[AMD Official Use Only - General]

Acked-by: Jiadong Zhu <Jiadong.Zhu at amd.com>

-----Original Message-----
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Christian König
Sent: Tuesday, February 27, 2024 12:02 AM
To: Alex Deucher <alexdeucher at gmail.com>; Pelloux-Prayer, Pierre-Eric <Pierre-eric.Pelloux-prayer at amd.com>
Cc: amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: disable ring_muxer if mcbp is off

Am 23.02.24 um 17:30 schrieb Alex Deucher:
> On Fri, Feb 23, 2024 at 4:48 AM Pierre-Eric Pelloux-Prayer
> <pierre-eric.pelloux-prayer at amd.com> wrote:
>> Using the ring_muxer without preemption adds overhead for no reason
>> since mcbp cannot be triggered.
>>
>> Moving back to a single queue in this case also helps when high
>> priority app are used: in this case the gpu_scheduler priority
>> handling will work as expected - much better than ring_muxer with its
>> 2 independant schedulers competing for the same hardware queue.
>>
>> This change requires moving amdgpu_device_set_mcbp above
>> amdgpu_device_ip_early_init because we use adev->gfx.mcbp.
>>
>> Signed-off-by: Pierre-Eric Pelloux-Prayer
>> <pierre-eric.pelloux-prayer at amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

Acked-by: Christian König <christian.koenig at amd.com>

>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 ++--
>>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      | 21 ++++++++++++---------
>>   2 files changed, 14 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index d534e192e260..40516d24026c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -4054,13 +4054,13 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>>                  return r;
>>          }
>>
>> +       amdgpu_device_set_mcbp(adev);
>> +
>>          /* early init functions */
>>          r = amdgpu_device_ip_early_init(adev);
>>          if (r)
>>                  return r;
>>
>> -       amdgpu_device_set_mcbp(adev);
>> -
>>          /* Get rid of things like offb */
>>          r = drm_aperture_remove_conflicting_pci_framebuffers(adev->pdev, &amdgpu_kms_driver);
>>          if (r)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> index 169d45268ef6..f682f830f7f6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> @@ -2080,7 +2080,7 @@ static int gfx_v9_0_sw_init(void *handle)
>>                  ring->doorbell_index =
>> adev->doorbell_index.gfx_ring0 << 1;
>>
>>                  /* disable scheduler on the real ring */
>> -               ring->no_scheduler = true;
>> +               ring->no_scheduler = adev->gfx.mcbp;
>>                  ring->vm_hub = AMDGPU_GFXHUB(0);
>>                  r = amdgpu_ring_init(adev, ring, 1024, &adev->gfx.eop_irq,
>>
>> AMDGPU_CP_IRQ_GFX_ME0_PIPE0_EOP, @@ -2090,7 +2090,7 @@ static int gfx_v9_0_sw_init(void *handle)
>>          }
>>
>>          /* set up the software rings */
>> -       if (adev->gfx.num_gfx_rings) {
>> +       if (adev->gfx.mcbp && adev->gfx.num_gfx_rings) {
>>                  for (i = 0; i < GFX9_NUM_SW_GFX_RINGS; i++) {
>>                          ring = &adev->gfx.sw_gfx_ring[i];
>>                          ring->ring_obj = NULL; @@ -2181,7 +2181,7 @@
>> static int gfx_v9_0_sw_fini(void *handle)
>>          int i;
>>          struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>
>> -       if (adev->gfx.num_gfx_rings) {
>> +       if (adev->gfx.mcbp && adev->gfx.num_gfx_rings) {
>>                  for (i = 0; i < GFX9_NUM_SW_GFX_RINGS; i++)
>>                          amdgpu_ring_fini(&adev->gfx.sw_gfx_ring[i]);
>>                  amdgpu_ring_mux_fini(&adev->gfx.muxer);
>> @@ -5910,11 +5910,14 @@ static int gfx_v9_0_eop_irq(struct
>> amdgpu_device *adev,
>>
>>          switch (me_id) {
>>          case 0:
>> -               if (adev->gfx.num_gfx_rings &&
>> -                   !amdgpu_mcbp_handle_trailing_fence_irq(&adev->gfx.muxer)) {
>> -                       /* Fence signals are handled on the software rings*/
>> -                       for (i = 0; i < GFX9_NUM_SW_GFX_RINGS; i++)
>> -                               amdgpu_fence_process(&adev->gfx.sw_gfx_ring[i]);
>> +               if (adev->gfx.num_gfx_rings) {
>> +                       if (!adev->gfx.mcbp) {
>> +                               amdgpu_fence_process(&adev->gfx.gfx_ring[0]);
>> +                       } else if (!amdgpu_mcbp_handle_trailing_fence_irq(&adev->gfx.muxer)) {
>> +                               /* Fence signals are handled on the software rings*/
>> +                               for (i = 0; i < GFX9_NUM_SW_GFX_RINGS; i++)
>> +                                       amdgpu_fence_process(&adev->gfx.sw_gfx_ring[i]);
>> +                       }
>>                  }
>>                  break;
>>          case 1:
>> @@ -7051,7 +7054,7 @@ static void gfx_v9_0_set_ring_funcs(struct amdgpu_device *adev)
>>          for (i = 0; i < adev->gfx.num_gfx_rings; i++)
>>                  adev->gfx.gfx_ring[i].funcs =
>> &gfx_v9_0_ring_funcs_gfx;
>>
>> -       if (adev->gfx.num_gfx_rings) {
>> +       if (adev->gfx.mcbp && adev->gfx.num_gfx_rings) {
>>                  for (i = 0; i < GFX9_NUM_SW_GFX_RINGS; i++)
>>                          adev->gfx.sw_gfx_ring[i].funcs = &gfx_v9_0_sw_ring_funcs_gfx;
>>          }
>> --
>> 2.40.1
>>