[PATCH 09/16] drm/amdgpu: create kernel doorbell page
Alex Deucher
alexdeucher at gmail.com
Thu Mar 30 15:04:55 UTC 2023
On Thu, Mar 30, 2023 at 10:48 AM Shashank Sharma
<shashank.sharma at amd.com> wrote:
>
>
> On 30/03/2023 16:42, Christian König wrote:
> > Am 30.03.23 um 16:39 schrieb Alex Deucher:
> >> On Thu, Mar 30, 2023 at 7:49 AM Shashank Sharma
> >> <shashank.sharma at amd.com> wrote:
> >>>
> >>> On 30/03/2023 13:30, Christian König wrote:
> >>>>
> >>>> Am 29.03.23 um 17:47 schrieb Shashank Sharma:
> >>>>> From: Shashank Sharma <contactshashanksharma at gmail.com>
> >>>>>
> >>>>> This patch:
> >>>>> - creates a doorbell page for graphics driver usages.
> >>>>> - removes the adev->doorbell.ptr variable, replaces it with
> >>>>> kernel-doorbell-bo's cpu address.
> >>>>>
> >>>>> Cc: Alex Deucher <alexander.deucher at amd.com>
> >>>>> Cc: Christian Koenig <christian.koenig at amd.com>
> >>>>> Signed-off-by: Shashank Sharma <shashank.sharma at amd.com>
> >>>>> ---
> >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h | 16 ++++++-
> >>>>> .../gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c | 44
> >>>>> +++++++++++++++----
> >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 +++
> >>>>> 3 files changed, 57 insertions(+), 10 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> >>>>> index 6581b78fe438..10a9bb10e974 100644
> >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell.h
> >>>>> @@ -49,10 +49,13 @@ struct amdgpu_doorbell {
> >>>>> /* doorbell mmio */
> >>>>> resource_size_t base;
> >>>>> resource_size_t size;
> >>>>> - u32 __iomem *ptr;
> >>>>> + u32 __iomem *ptr;
> >>>> This one can probably go away if we use the pointer from
> >>>> amdgpu_bo_create_kernel().
> >>> We started like that, but later realized that the cpu_addr from
> >>> create_kernel() will just limit us
> >>>
> >>> to that object only, whereas we are keeping this ptr to ioremap the
> >>> whole doorbell space in one shot.
> >> Why do we need that? For the kernel driver, we'd only need to mmap
> >> the page used for kernel doorbells. Then each user app would mmap its
> >> doorbell page.
> >
> > Yes, that is exactly my concern as well.
> >
> > The kernel needs a fixed number of doorbells allocated for its
> > internal use. Everything else should probably use the normal BO API.
> >
> > For KFD we can use the BO API internal in the kernel, but that is
> > certainly completely different to the kernel allocations.
> >
> There are 3 different kernel doorbell clients here:
>
> - graphics driver
>
> - mes (for aggregated doorbells and kernel ring test)
>
> - kfd (for kernel ring test and KIQ)
>
>
> The fix num_doorbells are just for kernel graphics driver, but We are
> allocating doorbell pages for each of those, and they all need to be
> mapped.
>
> Please see patch 12-16 for these details.
To me, it's clearer that resources managed by ttm are consistently
dealt with. Regardless of whether we are talking about VRAM or OA or
GDS or GTT or doorbells. When the kernel driver needs a page of vram
resources, it calls amdgpu_bo_create_kernel(), then if it needs a CPU
mmap, it calls amdgpu_bo_kmp(). Doorbells should be the same. In
this case, the KGD would call amdgpu_bo_create_kernel(...DOORBELL..),
then create a mmap with amdgpu_bo_kmap() and then use that pointer for
its use. KFD would do the same. User mode doorbell allocations would
be consistent as well.
Alex
>
> - Shashank
>
> > Christian.
> >
> >>
> >> Alex
> >>
> >>> - Shashank
> >>>
> >>>>> /* Number of doorbells reserved for amdgpu kernel driver */
> >>>>> u32 num_kernel_doorbells;
> >>>>> +
> >>>>> + /* For kernel doorbell pages */
> >>>>> + struct amdgpu_doorbell_obj kernel_doorbells;
> >>>>> };
> >>>>> /* Reserved doorbells for amdgpu (including multimedia).
> >>>>> @@ -369,6 +372,17 @@ void amdgpu_doorbell_free_page(struct
> >>>>> amdgpu_device *adev,
> >>>>> int amdgpu_doorbell_alloc_page(struct amdgpu_device *adev,
> >>>>> struct amdgpu_doorbell_obj *db_obj);
> >>>>> +/**
> >>>>> + * amdgpu_doorbell_create_kernel_doorbells - Create kernel doorbells
> >>>>> for graphics
> >>>>> + *
> >>>>> + * @adev: amdgpu_device pointer
> >>>>> + *
> >>>>> + * Creates doorbells for graphics driver
> >>>>> + *
> >>>>> + * returns 0 on success, error otherwise.
> >>>>> + */
> >>>>> +int amdgpu_doorbell_create_kernel_doorbells(struct amdgpu_device
> >>>>> *adev);
> >>>>> +
> >>>>> #define RDOORBELL32(index) amdgpu_mm_rdoorbell(adev, (index))
> >>>>> #define WDOORBELL32(index, v) amdgpu_mm_wdoorbell(adev,
> >>>>> (index), (v))
> >>>>> #define RDOORBELL64(index) amdgpu_mm_rdoorbell64(adev, (index))
> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
> >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
> >>>>> index 8be15b82b545..b46fe8b1378d 100644
> >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
> >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_doorbell_mgr.c
> >>>>> @@ -160,6 +160,38 @@ int amdgpu_doorbell_alloc_page(struct
> >>>>> amdgpu_device *adev,
> >>>>> return 0;
> >>>>> }
> >>>>> +/**
> >>>>> + * amdgpu_doorbell_create_kernel_doorbells - Create kernel doorbells
> >>>>> for graphics
> >>>>> + *
> >>>>> + * @adev: amdgpu_device pointer
> >>>>> + *
> >>>>> + * Creates doorbells for graphics driver
> >>>>> + *
> >>>>> + * returns 0 on success, error otherwise.
> >>>>> + */
> >>>>> +int amdgpu_doorbell_create_kernel_doorbells(struct amdgpu_device
> >>>>> *adev)
> >>>>> +{
> >>>>> + int r;
> >>>>> + struct amdgpu_doorbell_obj *kernel_doorbells =
> >>>>> &adev->doorbell.kernel_doorbells;
> >>>>> +
> >>>>> + kernel_doorbells->doorbell_bitmap =
> >>>>> bitmap_zalloc(adev->doorbell.num_kernel_doorbells,
> >>>>> + GFP_KERNEL);
> >>>>> + if (!kernel_doorbells->doorbell_bitmap) {
> >>>>> + DRM_ERROR("Failed to create kernel doorbell bitmap\n");
> >>>>> + return -ENOMEM;
> >>>>> + }
> >>>>> +
> >>>>> + kernel_doorbells->size = adev->doorbell.num_kernel_doorbells *
> >>>>> sizeof(u32);
> >>>>> + r = amdgpu_doorbell_alloc_page(adev, kernel_doorbells);
> >>>>> + if (r) {
> >>>>> + bitmap_free(kernel_doorbells->doorbell_bitmap);
> >>>>> + DRM_ERROR("Failed to allocate kernel doorbells,
> >>>>> err=%d\n", r);
> >>>>> + return r;
> >>>>> + }
> >>>>> +
> >>>>> + return 0;
> >>>>> +}
> >>>>> +
> >>>>> /*
> >>>>> * GPU doorbell aperture helpers function.
> >>>>> */
> >>>>> @@ -179,7 +211,6 @@ int amdgpu_device_doorbell_init(struct
> >>>>> amdgpu_device *adev)
> >>>>> adev->doorbell.base = 0;
> >>>>> adev->doorbell.size = 0;
> >>>>> adev->doorbell.num_kernel_doorbells = 0;
> >>>>> - adev->doorbell.ptr = NULL;
> >>>>> return 0;
> >>>>> }
> >>>>> @@ -208,12 +239,7 @@ int amdgpu_device_doorbell_init(struct
> >>>>> amdgpu_device *adev)
> >>>>> if (adev->asic_type >= CHIP_VEGA10)
> >>>>> adev->doorbell.num_kernel_doorbells += 0x400;
> >>>>> - adev->doorbell.ptr = ioremap(adev->doorbell.base,
> >>>>> - adev->doorbell.num_kernel_doorbells *
> >>>>> - sizeof(u32));
> >>>>> - if (adev->doorbell.ptr == NULL)
> >>>>> - return -ENOMEM;
> >>>>> -
> >>>>> + adev->doorbell.ptr = ioremap(adev->doorbell.base,
> >>>>> adev->doorbell.size);
> >>>>> return 0;
> >>>>> }
> >>>>> @@ -226,6 +252,6 @@ int amdgpu_device_doorbell_init(struct
> >>>>> amdgpu_device *adev)
> >>>>> */
> >>>>> void amdgpu_device_doorbell_fini(struct amdgpu_device *adev)
> >>>>> {
> >>>>> - iounmap(adev->doorbell.ptr);
> >>>>> - adev->doorbell.ptr = NULL;
> >>>>> + bitmap_free(adev->doorbell.kernel_doorbells.doorbell_bitmap);
> >>>>> + amdgpu_doorbell_free_page(adev,
> >>>>> &adev->doorbell.kernel_doorbells);
> >>>>> }
> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >>>>> index 203d77a20507..75c6852845c4 100644
> >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >>>>> @@ -1866,6 +1866,13 @@ int amdgpu_ttm_init(struct amdgpu_device
> >>>>> *adev)
> >>>>> return r;
> >>>>> }
> >>>>> + /* Create a boorbell page for kernel usages */
> >>>>> + r = amdgpu_doorbell_create_kernel_doorbells(adev);
> >>>>> + if (r) {
> >>>>> + DRM_ERROR("Failed to initialize kernel doorbells. \n");
> >>>>> + return r;
> >>>>> + }
> >>>>> +
> >>>>> /* Initialize preemptible memory pool */
> >>>>> r = amdgpu_preempt_mgr_init(adev);
> >>>>> if (r) {
> >
More information about the amd-gfx
mailing list