From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D3F5DC4708F for ; Wed, 2 Jun 2021 12:33:26 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 57A2E61396 for ; Wed, 2 Jun 2021 12:33:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 57A2E61396 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=shipmail.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id F37A56EC5F; Wed, 2 Jun 2021 12:33:24 +0000 (UTC) Received: from pio-pvt-msa2.bahnhof.se (pio-pvt-msa2.bahnhof.se [79.136.2.41]) by gabe.freedesktop.org (Postfix) with ESMTPS id 93E4489F4A for ; Wed, 2 Jun 2021 12:33:23 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by pio-pvt-msa2.bahnhof.se (Postfix) with ESMTP id DC5C940366; Wed, 2 Jun 2021 14:33:21 +0200 (CEST) Authentication-Results: pio-pvt-msa2.bahnhof.se; dkim=pass (1024-bit key; unprotected) header.d=shipmail.org header.i=@shipmail.org header.b="W9ittKG2"; dkim-atps=neutral X-Virus-Scanned: Debian amavisd-new at bahnhof.se Received: from pio-pvt-msa2.bahnhof.se ([127.0.0.1]) by localhost (pio-pvt-msa2.bahnhof.se [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id BNIVvHyoXX2C; Wed, 2 Jun 2021 14:33:20 +0200 (CEST) Received: by pio-pvt-msa2.bahnhof.se (Postfix) with ESMTPA id 24E943F9F1; Wed, 2 Jun 2021 14:33:19 +0200 (CEST) Received: from [192.168.0.209] (unknown [192.55.54.42]) by mail1.shipmail.org (Postfix) with ESMTPSA id 0250C36012A; Wed, 2 Jun 2021 14:33:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=shipmail.org; s=mail; t=1622637199; bh=baetTJtO7XG0Z2p5+jkL3rtWUCRjtn5lN4KdG1TKdeY=; h=Subject:To:References:From:Date:In-Reply-To:From; b=W9ittKG2OJUUz/qL9kuQcYlkg/GaEkHlUTD6TLpTnBhWvSS+DtTuhG29I323lbs/o C7EPQvatnhe3wQEECB/fM1AxX6+38DWpDuFTq8OULcpnMLeUUCqeTygczVVdEmeTun zlGFTKUbHUMB5yjNfPOcFuKutRabck2QgvUxB4Cg= Subject: Re: [PATCH 02/10] drm/ttm: flip over the range manager to self allocated nodes To: =?UTF-8?Q?Christian_K=c3=b6nig?= , matthew.auld@intel.com, dri-devel@lists.freedesktop.org References: <20210602100914.46246-1-christian.koenig@amd.com> <20210602100914.46246-2-christian.koenig@amd.com> <9b01d58f-6474-70de-4364-6adad59717a5@shipmail.org> <2354a311-c88f-04c5-0211-360c8116b811@gmail.com> From: =?UTF-8?Q?Thomas_Hellstr=c3=b6m_=28Intel=29?= Message-ID: <23afc41d-09ae-93f3-77b8-e18c8f72dd5a@shipmail.org> Date: Wed, 2 Jun 2021 14:33:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <2354a311-c88f-04c5-0211-360c8116b811@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 6/2/21 2:11 PM, Christian König wrote: > Am 02.06.21 um 13:44 schrieb Thomas Hellström (Intel): >> >> On 6/2/21 12:09 PM, Christian König wrote: >>> Start with the range manager to make the resource object the base >>> class for the allocated nodes. >>> >>> While at it cleanup a lot of the code around that. >>> >>> Signed-off-by: Christian König >>> Reviewed-by: Matthew Auld >>> --- >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  1 + >>>   drivers/gpu/drm/drm_gem_vram_helper.c   |  2 + >>>   drivers/gpu/drm/nouveau/nouveau_ttm.c   |  2 + >>>   drivers/gpu/drm/qxl/qxl_ttm.c           |  1 + >>>   drivers/gpu/drm/radeon/radeon_ttm.c     |  1 + >>>   drivers/gpu/drm/ttm/ttm_range_manager.c | 56 >>> ++++++++++++++++++------- >>>   drivers/gpu/drm/ttm/ttm_resource.c      | 26 ++++++++---- >>>   include/drm/ttm/ttm_bo_driver.h         | 26 ------------ >>>   include/drm/ttm/ttm_range_manager.h     | 43 +++++++++++++++++++ >>>   include/drm/ttm/ttm_resource.h          |  3 ++ >>>   10 files changed, 111 insertions(+), 50 deletions(-) >>>   create mode 100644 include/drm/ttm/ttm_range_manager.h >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>> index 69db89261650..df1f185faae9 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c >>> @@ -45,6 +45,7 @@ >>>   #include >>>   #include >>>   #include >>> +#include >>>     #include >>>   diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c >>> b/drivers/gpu/drm/drm_gem_vram_helper.c >>> index 83e7258c7f90..17a4c5d47b6a 100644 >>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c >>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c >>> @@ -17,6 +17,8 @@ >>>   #include >>>   #include >>>   +#include >>> + >>>   static const struct drm_gem_object_funcs drm_gem_vram_object_funcs; >>>     /** >>> diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c >>> b/drivers/gpu/drm/nouveau/nouveau_ttm.c >>> index 65430912ff72..b08b8efeefba 100644 >>> --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c >>> +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c >>> @@ -26,6 +26,8 @@ >>>   #include >>>   #include >>>   +#include >>> + >>>   #include "nouveau_drv.h" >>>   #include "nouveau_gem.h" >>>   #include "nouveau_mem.h" >>> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c >>> b/drivers/gpu/drm/qxl/qxl_ttm.c >>> index 8aa87b8edb9c..19fd39d9a00c 100644 >>> --- a/drivers/gpu/drm/qxl/qxl_ttm.c >>> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c >>> @@ -32,6 +32,7 @@ >>>   #include >>>   #include >>>   #include >>> +#include >>>     #include "qxl_drv.h" >>>   #include "qxl_object.h" >>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c >>> b/drivers/gpu/drm/radeon/radeon_ttm.c >>> index cdffa9b65108..ad2a5a791bba 100644 >>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c >>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c >>> @@ -45,6 +45,7 @@ >>>   #include >>>   #include >>>   #include >>> +#include >>>     #include "radeon_reg.h" >>>   #include "radeon.h" >>> diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c >>> b/drivers/gpu/drm/ttm/ttm_range_manager.c >>> index b9d5da6e6a81..ce5d07ca384c 100644 >>> --- a/drivers/gpu/drm/ttm/ttm_range_manager.c >>> +++ b/drivers/gpu/drm/ttm/ttm_range_manager.c >>> @@ -29,12 +29,13 @@ >>>    * Authors: Thomas Hellstrom >>>    */ >>>   -#include >>> +#include >>>   #include >>> +#include >>> +#include >>>   #include >>>   #include >>>   #include >>> -#include >>>     /* >>>    * Currently we use a spinlock for the lock, but a mutex *may* be >>> @@ -60,8 +61,8 @@ static int ttm_range_man_alloc(struct >>> ttm_resource_manager *man, >>>                      struct ttm_resource *mem) >>>   { >>>       struct ttm_range_manager *rman = to_range_manager(man); >>> +    struct ttm_range_mgr_node *node; >>>       struct drm_mm *mm = &rman->mm; >>> -    struct drm_mm_node *node; >>>       enum drm_mm_insert_mode mode; >>>       unsigned long lpfn; >>>       int ret; >>> @@ -70,7 +71,7 @@ static int ttm_range_man_alloc(struct >>> ttm_resource_manager *man, >>>       if (!lpfn) >>>           lpfn = man->size; >>>   -    node = kzalloc(sizeof(*node), GFP_KERNEL); >>> +    node = kzalloc(struct_size(node, mm_nodes, 1), GFP_KERNEL); >> >> I'm still a bit confused  about the situation where a driver wants to >> attach private data to a struct ttm_resource without having to >> re-implement its own range manager? >> >> Could be cached sg-tables, list of GPU bindings etc. Wouldn't work >> with the above unless we have a void *driver_private member on the >> struct ttm_resource. Is that the plan going forward here? Or that the >> driver actually does the re-implementation? > > I don't really understand your concern here. The basic idea is that > drivers use ttm_resource as a base class for their own implementation. > > See for example how nouveau does that: > > struct nouveau_mem { >         struct ttm_resource base; >         struct nouveau_cli *cli; >         u8 kind; >         u8 comp; >         struct nvif_mem mem; >         struct nvif_vma vma[2]; > }; > > The range manager is helping driver specific resource managers which > want to implement something drm_mm_nodes based. E.g. amdgpu_gtt_mgr > and amdgpu_vram_mgr, but it can also be used stand alone. > > The ttm_range_mgr_node can then be used as base class for this > functionality. I already want to move some more code from > amdgpu_vram_mgr.c into the range manager, but that is just minor > cleanup work. > Sure but if you embed a ttm_range_mgr_node in your struct i915_resource, and wanted to use the ttm range manager for it, it would allocate a struct ttm_range_mgr_node rather than a struct i915_resource? Or am I missing something? /Thomas > Regards, > Christian. > >> >> Thanks, >> >> Thomas >> >>