[RFC PATCH v3 07/11] drm, cgroup: Add TTM buffer allocation stats
The drm resource being measured is the TTM (Translation Table Manager)
buffers. TTM manages different types of memory that a GPU might access.
These memory types include dedicated Video RAM (VRAM) and host/system
memory accessible through IOMMU (GART/GTT). TTM is currently used by
multiple drm drivers (amd, ast, bochs, cirrus, hisilicon, maga200,
nouveau, qxl, virtio, vmwgfx.)
drm.memory.stats
A read-only nested-keyed file which exists on all cgroups.
Each entry is keyed by the drm device's major:minor. The
following nested keys are defined.
== =
system Host/system memory
tt Host memory used by the drm device (GTT/GART)
vram Video RAM used by the drm device
priv Other drm device, vendor specific memory
== =
Reading returns the following::
226:0 system=0 tt=0 vram=0 priv=0
226:1 system=0 tt=9035776 vram=17768448 priv=16809984
226:2 system=0 tt=9035776 vram=17768448 priv=16809984
drm.memory.evict.stats
A read-only flat-keyed file which exists on all cgroups. Each
entry is keyed by the drm device's major:minor.
Total number of evictions.
Change-Id: Ice2c4cc845051229549bebeb6aa2d7d6153bdf6a
Signed-off-by: Kenny Ho
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 +-
drivers/gpu/drm/ttm/ttm_bo.c| 30 +++
drivers/gpu/drm/ttm/ttm_bo_util.c | 4 +
include/drm/drm_cgroup.h| 19
include/drm/ttm/ttm_bo_api.h| 2 +
include/drm/ttm/ttm_bo_driver.h | 8 ++
include/linux/cgroup_drm.h | 4 +
kernel/cgroup/drm.c | 113
8 files changed, 182 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e9ecc3953673..a8dfc78ed45f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1678,8 +1678,9 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
mutex_init(&adev->mman.gtt_window_lock);
/* No others user of address space so set it to 0 */
- r = ttm_bo_device_init(&adev->mman.bdev,
+ r = ttm_bo_device_init_tmp(&adev->mman.bdev,
&amdgpu_bo_driver,
+ adev->ddev,
adev->ddev->anon_inode->i_mapping,
adev->need_dma32);
if (r) {
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2845fceb2fbd..e9f70547f0ad 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -42,6 +43,7 @@
#include
#include
#include
+#include
static void ttm_bo_global_kobj_release(struct kobject *kobj);
@@ -151,6 +153,10 @@ static void ttm_bo_release_list(struct kref *list_kref)
struct ttm_bo_device *bdev = bo->bdev;
size_t acc_size = bo->acc_size;
+ if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for
all
+ drmcgrp_unchg_mem(bo);
+ put_drmcgrp(bo->drmcgrp);
+
BUG_ON(kref_read(&bo->list_kref));
BUG_ON(kref_read(&bo->kref));
BUG_ON(atomic_read(&bo->cpu_writers));
@@ -353,6 +359,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
if (bo->mem.mem_type == TTM_PL_SYSTEM) {
if (bdev->driver->move_notify)
bdev->driver->move_notify(bo, evict, mem);
+ if (bo->bdev->ddev != NULL) // TODO: remove after ddev
initiazlied for all
+ drmcgrp_mem_track_move(bo, evict, mem);
bo->mem = *mem;
mem->mm_node = NULL;
goto moved;
@@ -361,6 +369,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
if (bdev->driver->move_notify)
bdev->driver->move_notify(bo, evict, mem);
+ if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for
all
+ drmcgrp_mem_track_move(bo, evict, mem);
if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
@@ -374,6 +384,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object
*bo,
if (bdev->driver->move_notify) {
swap(*mem, bo->mem);
bdev->driver->move_notify(bo, false, mem);
+ if (bo->bdev->ddev != NULL) // TODO: remove after ddev
initiazlied for all
+ drmcgrp_mem_track_move(bo, evict, mem);
swap(*mem, bo->mem);
}
Re: [RFC PATCH v3 07/11] drm, cgroup: Add TTM buffer allocation stats
On Wed, Jun 26, 2019 at 11:05:18AM -0400, Kenny Ho wrote:
> The drm resource being measured is the TTM (Translation Table Manager)
> buffers. TTM manages different types of memory that a GPU might access.
> These memory types include dedicated Video RAM (VRAM) and host/system
> memory accessible through IOMMU (GART/GTT). TTM is currently used by
> multiple drm drivers (amd, ast, bochs, cirrus, hisilicon, maga200,
> nouveau, qxl, virtio, vmwgfx.)
>
> drm.memory.stats
> A read-only nested-keyed file which exists on all cgroups.
> Each entry is keyed by the drm device's major:minor. The
> following nested keys are defined.
>
> == =
> system Host/system memory
Shouldn't that be covered by gem bo stats already? Also, system memory is
definitely something a lot of non-ttm drivers want to be able to track, so
that needs to be separate from ttm.
> tt Host memory used by the drm device (GTT/GART)
> vram Video RAM used by the drm device
> priv Other drm device, vendor specific memory
So what's "priv". In general I think we need some way to register the
different kinds of memory, e.g. stuff not in your list:
- multiple kinds of vram (like numa-style gpus)
- cma (for all those non-ttm drivers that's a big one, it's like system
memory but also totally different)
- any carveouts and stuff
> == =
>
> Reading returns the following::
>
> 226:0 system=0 tt=0 vram=0 priv=0
> 226:1 system=0 tt=9035776 vram=17768448 priv=16809984
> 226:2 system=0 tt=9035776 vram=17768448 priv=16809984
>
> drm.memory.evict.stats
> A read-only flat-keyed file which exists on all cgroups. Each
> entry is keyed by the drm device's major:minor.
>
> Total number of evictions.
>
> Change-Id: Ice2c4cc845051229549bebeb6aa2d7d6153bdf6a
> Signed-off-by: Kenny Ho
I think with all the ttm refactoring going on I think we need to de-ttm
the interface functions here a bit. With Gerd Hoffmans series you can just
use a gem_bo pointer here, so what's left to do is have some extracted
structure for tracking memory types. I think Brian Welty has some ideas
for this, even in patch form. Would be good to keep him on cc at least for
the next version. We'd need to explicitly hand in the ttm_mem_reg (or
whatever the specific thing is going to be).
-Daniel
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 +-
> drivers/gpu/drm/ttm/ttm_bo.c| 30 +++
> drivers/gpu/drm/ttm/ttm_bo_util.c | 4 +
> include/drm/drm_cgroup.h| 19
> include/drm/ttm/ttm_bo_api.h| 2 +
> include/drm/ttm/ttm_bo_driver.h | 8 ++
> include/linux/cgroup_drm.h | 4 +
> kernel/cgroup/drm.c | 113
> 8 files changed, 182 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index e9ecc3953673..a8dfc78ed45f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1678,8 +1678,9 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
> mutex_init(&adev->mman.gtt_window_lock);
>
> /* No others user of address space so set it to 0 */
> - r = ttm_bo_device_init(&adev->mman.bdev,
> + r = ttm_bo_device_init_tmp(&adev->mman.bdev,
> &amdgpu_bo_driver,
> +adev->ddev,
> adev->ddev->anon_inode->i_mapping,
> adev->need_dma32);
> if (r) {
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 2845fceb2fbd..e9f70547f0ad 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -34,6 +34,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -42,6 +43,7 @@
> #include
> #include
> #include
> +#include
>
> static void ttm_bo_global_kobj_release(struct kobject *kobj);
>
> @@ -151,6 +153,10 @@ static void ttm_bo_release_list(struct kref *list_kref)
> struct ttm_bo_device *bdev = bo->bdev;
> size_t acc_size = bo->acc_size;
>
> + if (bo->bdev->ddev != NULL) // TODO: remove after ddev initiazlied for
> all
> + drmcgrp_unchg_mem(bo);
> + put_drmcgrp(bo->drmcgrp);
> +
> BUG_ON(kref_read(&bo->list_kref));
> BUG_ON(kref_read(&bo->kref));
> BUG_ON(atomic_read(&bo->cpu_writers));
> @@ -353,6 +359,8 @@ static int ttm_bo_handle_move_mem(struct
> ttm_buffer_object *bo,
> if (bo->mem.mem_type == TTM_PL_SYSTEM) {
> if (bdev->driver->move_notify)
> bdev->driver->move_notify(bo, evict, mem);
> +
Re: [RFC PATCH v3 07/11] drm, cgroup: Add TTM buffer allocation stats
On Wed, Jun 26, 2019 at 12:12 PM Daniel Vetter wrote: > > On Wed, Jun 26, 2019 at 11:05:18AM -0400, Kenny Ho wrote: > > drm.memory.stats > > A read-only nested-keyed file which exists on all cgroups. > > Each entry is keyed by the drm device's major:minor. The > > following nested keys are defined. > > > > == = > > system Host/system memory > > Shouldn't that be covered by gem bo stats already? Also, system memory is > definitely something a lot of non-ttm drivers want to be able to track, so > that needs to be separate from ttm. The gem bo stats covers all of these type. I am treat the gem stats as more of the front end and a hard limit and this set of stats as the backing store which can be of various type. How does non-ttm drivers identify various memory types? > > tt Host memory used by the drm device (GTT/GART) > > vram Video RAM used by the drm device > > priv Other drm device, vendor specific memory > > So what's "priv". In general I think we need some way to register the > different kinds of memory, e.g. stuff not in your list: > > - multiple kinds of vram (like numa-style gpus) > - cma (for all those non-ttm drivers that's a big one, it's like system > memory but also totally different) > - any carveouts and stuff privs are vendor specific, which is why I have truncated it. For example, AMD has AMDGPU_PL_GDS, GWS, OA https://elixir.bootlin.com/linux/v5.2-rc6/source/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h#L30 Since we are using keyed file type, we should be able to support vendor specific memory type but I am not sure if this is acceptable to cgroup upstream. This is why I stick to the 3 memory type that is common across all ttm drivers. > I think with all the ttm refactoring going on I think we need to de-ttm > the interface functions here a bit. With Gerd Hoffmans series you can just > use a gem_bo pointer here, so what's left to do is have some extracted > structure for tracking memory types. I think Brian Welty has some ideas > for this, even in patch form. Would be good to keep him on cc at least for > the next version. We'd need to explicitly hand in the ttm_mem_reg (or > whatever the specific thing is going to be). I assume Gerd Hoffman's series you are referring to is this one? https://www.spinics.net/lists/dri-devel/msg215056.html I can certainly keep an eye out for Gerd's refactoring while refactoring other parts of this RFC. I have added Brian and Gerd to the thread for awareness. Regards, Kenny ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH v3 07/11] drm, cgroup: Add TTM buffer allocation stats
On Thu, Jun 27, 2019 at 12:06:13AM -0400, Kenny Ho wrote: > On Wed, Jun 26, 2019 at 12:12 PM Daniel Vetter wrote: > > > > On Wed, Jun 26, 2019 at 11:05:18AM -0400, Kenny Ho wrote: > > > drm.memory.stats > > > A read-only nested-keyed file which exists on all cgroups. > > > Each entry is keyed by the drm device's major:minor. The > > > following nested keys are defined. > > > > > > == = > > > system Host/system memory > > > > Shouldn't that be covered by gem bo stats already? Also, system memory is > > definitely something a lot of non-ttm drivers want to be able to track, so > > that needs to be separate from ttm. > The gem bo stats covers all of these type. I am treat the gem stats > as more of the front end and a hard limit and this set of stats as the > backing store which can be of various type. How does non-ttm drivers > identify various memory types? Not explicitly, they generally just have one. I think i915 currently has two, system and carveout (with vram getting added). > > > tt Host memory used by the drm device (GTT/GART) > > > vram Video RAM used by the drm device > > > priv Other drm device, vendor specific memory > > > > So what's "priv". In general I think we need some way to register the > > different kinds of memory, e.g. stuff not in your list: > > > > - multiple kinds of vram (like numa-style gpus) > > - cma (for all those non-ttm drivers that's a big one, it's like system > > memory but also totally different) > > - any carveouts and stuff > privs are vendor specific, which is why I have truncated it. For > example, AMD has AMDGPU_PL_GDS, GWS, OA > https://elixir.bootlin.com/linux/v5.2-rc6/source/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h#L30 > > Since we are using keyed file type, we should be able to support > vendor specific memory type but I am not sure if this is acceptable to > cgroup upstream. This is why I stick to the 3 memory type that is > common across all ttm drivers. I think we'll need custom memory pools, not just priv, and I guess some naming scheme for them. I think just exposing them as amd-gws, amd-oa, amd-gds would make sense. Another thing I wonder about is multi-gpu cards, with multiple gpus and each their own vram and other device-specific resources. For those we'd have node0.vram and node1.vram too (on top of maybe an overall vram node, not sure). > > I think with all the ttm refactoring going on I think we need to de-ttm > > the interface functions here a bit. With Gerd Hoffmans series you can just > > use a gem_bo pointer here, so what's left to do is have some extracted > > structure for tracking memory types. I think Brian Welty has some ideas > > for this, even in patch form. Would be good to keep him on cc at least for > > the next version. We'd need to explicitly hand in the ttm_mem_reg (or > > whatever the specific thing is going to be). > > I assume Gerd Hoffman's series you are referring to is this one? > https://www.spinics.net/lists/dri-devel/msg215056.html There's a newer one, much more complete, but yes that's the work. > I can certainly keep an eye out for Gerd's refactoring while > refactoring other parts of this RFC. > > I have added Brian and Gerd to the thread for awareness. btw just realized that maybe building the interfaces on top of ttm_mem_reg is maybe not the best. That's what you're using right now, but in a way that's just the ttm internal detail of how the backing storage is allocated. I think the structure we need to abstract away is ttm_mem_type_manager, without any of the actual management details. btw reminds me: I guess it would be good to have a per-type .total read-only exposed, so that userspace has an idea of how much there is? ttm is trying to be agnostic to the allocator that's used to manage a memory type/resource, so doesn't even know that. But I think something we need to expose to admins, otherwise they can't meaningfully set limits. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH v3 07/11] drm, cgroup: Add TTM buffer allocation stats
On Thu, Jun 27, 2019 at 2:01 AM Daniel Vetter wrote: > > btw reminds me: I guess it would be good to have a per-type .total > read-only exposed, so that userspace has an idea of how much there is? > ttm is trying to be agnostic to the allocator that's used to manage a > memory type/resource, so doesn't even know that. But I think something we > need to expose to admins, otherwise they can't meaningfully set limits. I don't think I understand this bit, do you mean total across multiple GPU of the same mem type? Or do you mean the total available per GPU (or something else?) Regards, Kenny ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH v3 07/11] drm, cgroup: Add TTM buffer allocation stats
On Thu, Jun 27, 2019 at 04:17:09PM -0400, Kenny Ho wrote: > On Thu, Jun 27, 2019 at 2:01 AM Daniel Vetter wrote: > > btw reminds me: I guess it would be good to have a per-type .total > > read-only exposed, so that userspace has an idea of how much there is? > > ttm is trying to be agnostic to the allocator that's used to manage a > > memory type/resource, so doesn't even know that. But I think something we > > need to expose to admins, otherwise they can't meaningfully set limits. > > I don't think I understand this bit, do you mean total across multiple > GPU of the same mem type? Or do you mean the total available per GPU > (or something else?) Total for a given type on a given cpu. E.g. maybe you want to give 50% of your vram to one cgroup, and the other 50% to the other cgroup. For that you need to know how much vram you have. And expecting people to lspci and then look at wikipedia for how much vram that chip should have (or something like that) isn't great. Hence 0.vram.total, 0.tt.total, and so on (also for all the other gpu minors ofc). For system memory we probably don't want to provide a total, since that's already a value that's easy to obtain from various sources. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH v3 07/11] drm, cgroup: Add TTM buffer allocation stats
On 6/26/2019 11:01 PM, Daniel Vetter wrote: > On Thu, Jun 27, 2019 at 12:06:13AM -0400, Kenny Ho wrote: >> On Wed, Jun 26, 2019 at 12:12 PM Daniel Vetter wrote: >>> >>> I think with all the ttm refactoring going on I think we need to de-ttm >>> the interface functions here a bit. With Gerd Hoffmans series you can just >>> use a gem_bo pointer here, so what's left to do is have some extracted >>> structure for tracking memory types. I think Brian Welty has some ideas >>> for this, even in patch form. Would be good to keep him on cc at least for >>> the next version. We'd need to explicitly hand in the ttm_mem_reg (or >>> whatever the specific thing is going to be). >> >> I assume Gerd Hoffman's series you are referring to is this one? >> https://www.spinics.net/lists/dri-devel/msg215056.html > > There's a newer one, much more complete, but yes that's the work. > >> I can certainly keep an eye out for Gerd's refactoring while >> refactoring other parts of this RFC. >> >> I have added Brian and Gerd to the thread for awareness. > > btw just realized that maybe building the interfaces on top of ttm_mem_reg > is maybe not the best. That's what you're using right now, but in a way > that's just the ttm internal detail of how the backing storage is > allocated. I think the structure we need to abstract away is > ttm_mem_type_manager, without any of the actual management details. > Any de-ttm refactoring should probably not spam all the cgroups folks. So I removed cgroups list. As Daniel mentioned, some of us are looking at possible refactoring of TTM for reuse in i915 driver. Here is a brief summary of some ideas to be considered: 1) refactor part of ttm_mem_type_manager into a new drm_mem_type_region. Really, should then move the array from ttm_bo_device.man[] into drm_device. Relevant to drm_cgroup, you could then perhaps access these stats through drm_device and don't need the mem_stats array in drmcgrp_device_resource. 1a) doing this right means replacing TTM_PL_XXX memory types with new DRM defines. But could keep the TTM ones as redefinition of (new) DRM ones. Probably those private ones (TTM_PL_PRIV) make this difficult. All of the above could be eventually leveraged by the vram support being implemented now in i915 driver. 2) refactor ttm_mem_reg + ttm_bus_placement into something generic for any GEM object, maybe call it drm_gem_object_placement. ttm_mem_reg could remain as a wrapper for TTM drivers. This hasn't been broadly discussed with intel-gfx folks, so not sure this fits well into i915 or not. Relevant to drm_cgroup, maybe this function: drmcgrp_mem_track_move(struct ttm_buffer_object *old_bo, bool evict, struct ttm_mem_reg *new_mem) could potentially become: drmcgrp_mem_track_move(struct drm_gem_object *old_bo, bool evict, struct drm_gem_object_placement *new_place) Though from ttm_mem_reg, you look to only be using mem_type and size. I think Daniel is noting that ttm_mem_reg wasn't truly needed here, so you could just pass in the mem_type and size instead. Would appreciate any feedback (positive or negative) on above Perhaps this should move to a new thread? I could send out basic RFC patches for (1) if helpful but as it touches all the TTM drivers, nice to hear some feedback first. Anyway, this doesn't necessarily need to block forward progress on drm_cgroup, as refactoring into common base structures could happen incrementally. Thanks, -Brian ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH v3 07/11] drm, cgroup: Add TTM buffer allocation stats
On Fri, Jun 28, 2019 at 3:16 AM Welty, Brian wrote: > On 6/26/2019 11:01 PM, Daniel Vetter wrote: > > On Thu, Jun 27, 2019 at 12:06:13AM -0400, Kenny Ho wrote: > >> On Wed, Jun 26, 2019 at 12:12 PM Daniel Vetter wrote: > >>> > >>> I think with all the ttm refactoring going on I think we need to de-ttm > >>> the interface functions here a bit. With Gerd Hoffmans series you can just > >>> use a gem_bo pointer here, so what's left to do is have some extracted > >>> structure for tracking memory types. I think Brian Welty has some ideas > >>> for this, even in patch form. Would be good to keep him on cc at least for > >>> the next version. We'd need to explicitly hand in the ttm_mem_reg (or > >>> whatever the specific thing is going to be). > >> > >> I assume Gerd Hoffman's series you are referring to is this one? > >> https://www.spinics.net/lists/dri-devel/msg215056.html > > > > There's a newer one, much more complete, but yes that's the work. > > > >> I can certainly keep an eye out for Gerd's refactoring while > >> refactoring other parts of this RFC. > >> > >> I have added Brian and Gerd to the thread for awareness. > > > > btw just realized that maybe building the interfaces on top of ttm_mem_reg > > is maybe not the best. That's what you're using right now, but in a way > > that's just the ttm internal detail of how the backing storage is > > allocated. I think the structure we need to abstract away is > > ttm_mem_type_manager, without any of the actual management details. > > > > Any de-ttm refactoring should probably not spam all the cgroups folks. > So I removed cgroups list. > > As Daniel mentioned, some of us are looking at possible refactoring of TTM > for reuse in i915 driver. > Here is a brief summary of some ideas to be considered: > > 1) refactor part of ttm_mem_type_manager into a new drm_mem_type_region. > Really, should then move the array from ttm_bo_device.man[] into > drm_device. > > Relevant to drm_cgroup, you could then perhaps access these stats through > drm_device and don't need the mem_stats array in drmcgrp_device_resource. > > 1a) doing this right means replacing TTM_PL_XXX memory types with new DRM > defines. But could keep the TTM ones as redefinition of (new) DRM ones. > Probably those private ones (TTM_PL_PRIV) make this difficult. > > All of the above could be eventually leveraged by the vram support being > implemented now in i915 driver. > > 2) refactor ttm_mem_reg + ttm_bus_placement into something generic for > any GEM object, maybe call it drm_gem_object_placement. > ttm_mem_reg could remain as a wrapper for TTM drivers. > This hasn't been broadly discussed with intel-gfx folks, so not sure > this fits well into i915 or not. > > Relevant to drm_cgroup, maybe this function: > drmcgrp_mem_track_move(struct ttm_buffer_object *old_bo, bool evict, > struct ttm_mem_reg *new_mem) > could potentially become: > drmcgrp_mem_track_move(struct drm_gem_object *old_bo, bool evict, > struct drm_gem_object_placement *new_place) > > Though from ttm_mem_reg, you look to only be using mem_type and size. > I think Daniel is noting that ttm_mem_reg wasn't truly needed here, so > you could just pass in the mem_type and size instead. Yeah I think the relevant part of your refactoring is creating a more abstract memory type/resource thing (not the individual allocations from it which ttm calls regions and I get confused about that every time). I think that abstraction should also have a field for the total (which I think cgroups also needs, both as read-only information and starting value). ttm would that put somewhere into the ttm_mem_type_manager, i915 would put it somewhere else, cma based drivers could perhaps expose the cma heap like that (if it's exclusive to the gpu at least). > Would appreciate any feedback (positive or negative) on above > Perhaps this should move to a new thread? I could send out basic RFC > patches for (1) if helpful but as it touches all the TTM drivers, nice to > hear some feedback first. > Anyway, this doesn't necessarily need to block forward progress on drm_cgroup, > as refactoring into common base structures could happen incrementally. Yeah I think new dri-devel thread with a totally unpolished rfc as draft plus this as the intro would be good. That way we can ground this a bit better in actual code. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
