[PATCH 09/16] drm/amdgpu: create kernel doorbell page
Christian König
christian.koenig at amd.com
Thu Mar 30 14:53:12 UTC 2023
Am 30.03.23 um 16:48 schrieb Shashank Sharma:
>
> 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.
Yeah and exactly that doesn't sound like a good idea to me. In general
we should only distinct between static and dynamic allocations and *not*
different use cases of those.
What do you mean exactly with graphics driver and aggregated doorbells?
For the MES in kernel test we should probably just allocate a single
doorbell page and use it statically like userspace would do. E.g. with
fixed indexes etc...
Regards,
Christian.
>
> Please see patch 12-16 for these details.
>
> - 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