[PATCH 06/16] drm/amdgpu: accommodate DOMAIN/PL_DOORBELL
Shashank Sharma
shashank.sharma at amd.com
Thu Mar 30 13:48:28 UTC 2023
On 30/03/2023 15:45, Luben Tuikov wrote:
> On 2023-03-30 09:43, Shashank Sharma wrote:
>> On 30/03/2023 15:33, Luben Tuikov wrote:
>>> On 2023-03-30 07:14, Christian König wrote:
>>>> Am 29.03.23 um 17:47 schrieb Shashank Sharma:
>>>>> From: Alex Deucher <alexander.deucher at amd.com>
>>>>>
>>>>> This patch adds changes:
>>>>> - to accommodate the new GEM domain DOORBELL
>>>>> - to accommodate the new TTM PL DOORBELL
>>>>>
>>>>> in order to manage doorbell pages as GEM object.
>>>>>
>>>>> V2: Addressed reviwe comments from Christian
>>>>> - drop the doorbell changes for pinning/unpinning
>>>>> - drop the doorbell changes for dma-buf map
>>>>> - drop the doorbell changes for sgt
>>>>> - no need to handle TTM_PL_FLAG_CONTIGUOUS for doorbell
>>>>> - add caching type for doorbell
>>>>>
>>>>> Cc: Alex Deucher <alexander.deucher at amd.com>
>>>>> Cc: Christian Koenig <christian.koenig at amd.com>
>>>>>
>>>>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>>>>> Signed-off-by: Shashank Sharma <shashank.sharma at amd.com>
>>> Generally there are no empty lines in the tag list. Perhaps remove it?
>> I would prefer to keep it, to highlight the CC parts.
> I've never seen a commit with them separated. Perhaps follow Linux custom?
IIRC This is not against Linux patch formatting/message body guidelines.
- Shashank
> Regards,
> Luben
>
>> - Shashank
>>
>>> Regards,
>>> Luben
>>>
>>>>> ---
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 ++++++++++-
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 2 ++
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 16 +++++++++++++++-
>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 +
>>>>> 4 files changed, 28 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> index 4e684c2afc70..0ec080e240ad 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>>> @@ -147,6 +147,14 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain)
>>>>> c++;
>>>>> }
>>>>>
>>>>> + if (domain & AMDGPU_GEM_DOMAIN_DOORBELL) {
>>>>> + places[c].fpfn = 0;
>>>>> + places[c].lpfn = 0;
>>>>> + places[c].mem_type = AMDGPU_PL_DOORBELL;
>>>>> + places[c].flags = 0;
>>>>> + c++;
>>>>> + }
>>>>> +
>>>>> if (domain & AMDGPU_GEM_DOMAIN_GTT) {
>>>>> places[c].fpfn = 0;
>>>>> places[c].lpfn = 0;
>>>>> @@ -466,7 +474,7 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev,
>>>>> goto fail;
>>>>> }
>>>>>
>>>>> - /* TODO add more domains checks, such as AMDGPU_GEM_DOMAIN_CPU */
>>>>> + /* TODO add more domains checks, such as AMDGPU_GEM_DOMAIN_CPU, AMDGPU_GEM_DOMAIN_DOORBELL */
>>>>> return true;
>>>>>
>>>>> fail:
>>>>> @@ -1013,6 +1021,7 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo)
>>>>> } else if (bo->tbo.resource->mem_type == TTM_PL_TT) {
>>>>> atomic64_sub(amdgpu_bo_size(bo), &adev->gart_pin_size);
>>>>> }
>>>>> +
>>>> Unrelated newline, probably just a leftover.
>>>>
>>>> Apart from that the patch is Reviewed-by: Christian König
>>>> <christian.koenig at amd.com>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>> }
>>>>>
>>>>> static const char *amdgpu_vram_names[] = {
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
>>>>> index 5c4f93ee0c57..3c988cc406e4 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
>>>>> @@ -90,6 +90,7 @@ static inline void amdgpu_res_first(struct ttm_resource *res,
>>>>> cur->node = block;
>>>>> break;
>>>>> case TTM_PL_TT:
>>>>> + case AMDGPU_PL_DOORBELL:
>>>>> node = to_ttm_range_mgr_node(res)->mm_nodes;
>>>>> while (start >= node->size << PAGE_SHIFT)
>>>>> start -= node++->size << PAGE_SHIFT;
>>>>> @@ -152,6 +153,7 @@ static inline void amdgpu_res_next(struct amdgpu_res_cursor *cur, uint64_t size)
>>>>> cur->size = min(amdgpu_vram_mgr_block_size(block), cur->remaining);
>>>>> break;
>>>>> case TTM_PL_TT:
>>>>> + case AMDGPU_PL_DOORBELL:
>>>>> node = cur->node;
>>>>>
>>>>> cur->node = ++node;
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> index 55e0284b2bdd..6f61491ef3dd 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>>> @@ -128,6 +128,7 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
>>>>> case AMDGPU_PL_GDS:
>>>>> case AMDGPU_PL_GWS:
>>>>> case AMDGPU_PL_OA:
>>>>> + case AMDGPU_PL_DOORBELL:
>>>>> placement->num_placement = 0;
>>>>> placement->num_busy_placement = 0;
>>>>> return;
>>>>> @@ -500,9 +501,11 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
>>>>> if (old_mem->mem_type == AMDGPU_PL_GDS ||
>>>>> old_mem->mem_type == AMDGPU_PL_GWS ||
>>>>> old_mem->mem_type == AMDGPU_PL_OA ||
>>>>> + old_mem->mem_type == AMDGPU_PL_DOORBELL ||
>>>>> new_mem->mem_type == AMDGPU_PL_GDS ||
>>>>> new_mem->mem_type == AMDGPU_PL_GWS ||
>>>>> - new_mem->mem_type == AMDGPU_PL_OA) {
>>>>> + new_mem->mem_type == AMDGPU_PL_OA ||
>>>>> + new_mem->mem_type == AMDGPU_PL_DOORBELL) {
>>>>> /* Nothing to save here */
>>>>> ttm_bo_move_null(bo, new_mem);
>>>>> goto out;
>>>>> @@ -586,6 +589,12 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,
>>>>> mem->bus.offset += adev->gmc.aper_base;
>>>>> mem->bus.is_iomem = true;
>>>>> break;
>>>>> + case AMDGPU_PL_DOORBELL:
>>>>> + mem->bus.offset = mem->start << PAGE_SHIFT;
>>>>> + mem->bus.offset += adev->doorbell.base;
>>>>> + mem->bus.is_iomem = true;
>>>>> + mem->bus.caching = ttm_uncached;
>>>>> + break;
>>>>> default:
>>>>> return -EINVAL;
>>>>> }
>>>>> @@ -600,6 +609,10 @@ static unsigned long amdgpu_ttm_io_mem_pfn(struct ttm_buffer_object *bo,
>>>>>
>>>>> amdgpu_res_first(bo->resource, (u64)page_offset << PAGE_SHIFT, 0,
>>>>> &cursor);
>>>>> +
>>>>> + if (bo->resource->mem_type == AMDGPU_PL_DOORBELL)
>>>>> + return ((uint64_t)(adev->doorbell.base + cursor.start)) >> PAGE_SHIFT;
>>>>> +
>>>>> return (adev->gmc.aper_base + cursor.start) >> PAGE_SHIFT;
>>>>> }
>>>>>
>>>>> @@ -1267,6 +1280,7 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_resource *mem)
>>>>> flags |= AMDGPU_PTE_VALID;
>>>>>
>>>>> if (mem && (mem->mem_type == TTM_PL_TT ||
>>>>> + mem->mem_type == AMDGPU_PL_DOORBELL ||
>>>>> mem->mem_type == AMDGPU_PL_PREEMPT)) {
>>>>> flags |= AMDGPU_PTE_SYSTEM;
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> index e2cd5894afc9..761cd6b2b942 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>>>> @@ -33,6 +33,7 @@
>>>>> #define AMDGPU_PL_GWS (TTM_PL_PRIV + 1)
>>>>> #define AMDGPU_PL_OA (TTM_PL_PRIV + 2)
>>>>> #define AMDGPU_PL_PREEMPT (TTM_PL_PRIV + 3)
>>>>> +#define AMDGPU_PL_DOORBELL (TTM_PL_PRIV + 4)
>>>>>
>>>>> #define AMDGPU_GTT_MAX_TRANSFER_SIZE 512
>>>>> #define AMDGPU_GTT_NUM_TRANSFER_WINDOWS 2
More information about the amd-gfx
mailing list