On Wed, Feb 10, 2021 at 08:52:29AM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 09.02.21 um 11:54 schrieb Daniel Vetter:
> > *: vmwgfx is the only non-gem driver, but there's plans to move at least
> > vmwgfx internals (maybe not the uapi, we'll see) over to gem. Once that's
> > done it's truly all gpu memory.
>
> Do you have a URL to the discussion?
>
> While I recent worked on GEM, I thought that vmwgfx could easily switch to
> the GEM internals without adopting the interface.
Zack Rusin pinged me on irc I think, not sure there's anything on
dri-devel. zackr on freenode. I think he's working on this already.
> Personally, I think we should consider renaming struct drm_gem_object et al.
> It's not strictly GEM UAPI, but nowadays anything memory-related. Maybe
> drm_mem_object would fit.
I think abbrevations we've created that have been around for long enough
can stay. Otherwise we'd need to rename drm into something less confusing
too :-)
So gem just becomes the buffer based memory management thing in drm, which
is the accelerator and display framework in upstream. And ttm is our
memory manager for discrete gpus - ttm means translation table manager,
and that part just got moved out as a helper so drivers call we needed :-)
I mean we haven't even managed to rename the cma helpers to dma helpers.
The one thing we did manage is rename struct fence to struct dma_fence,
because no prefix was just really bad naming accident.
Cheers, Daniel
>
> Best regards
> Thomas
>
> > > ---
> > > drivers/gpu/drm/drm_gem.c | 89 +++++++++++++++++++++++++++++++++++++++
> > > include/drm/drm_gem.h | 17 ++++++++
> > > 2 files changed, 106 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > > index c2ce78c4edc3..a12da41eaafe 100644
> > > --- a/drivers/gpu/drm/drm_gem.c
> > > +++ b/drivers/gpu/drm/drm_gem.c
> > > @@ -29,6 +29,7 @@
> > > #include <linux/slab.h>
> > > #include <linux/mm.h>
> > > #include <linux/uaccess.h>
> > > +#include <linux/cgroup_drm.h>
> > > #include <linux/fs.h>
> > > #include <linux/file.h>
> > > #include <linux/module.h>
> > > @@ -112,6 +113,89 @@ drm_gem_init(struct drm_device *dev)
> > > return drmm_add_action(dev, drm_gem_init_release, NULL);
> > > }
> > > +/**
> > > + * drm_gem_object_set_cgroup - associate GEM object with a cgroup
> > > + * @obj: GEM object which is being associated with a cgroup
> > > + * @task: task associated with process control group to use
> > > + *
> > > + * This will acquire a reference on cgroup and use for charging GEM
> > > + * memory allocations.
> > > + * This helper could be extended in future to migrate charges to another
> > > + * cgroup, print warning if this usage occurs.
> > > + */
> > > +void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
> > > + struct task_struct *task)
> > > +{
> > > + /* if object has existing cgroup, we migrate the charge... */
> > > + if (obj->drmcg) {
> > > + pr_warn("DRM: need to migrate cgroup charge of %lld\n",
> > > + atomic64_read(&obj->drmcg_bytes_charged));
> > > + }
> > > + obj->drmcg = drmcg_get(task);
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_object_set_cgroup);
> > > +
> > > +/**
> > > + * drm_gem_object_unset_cgroup - clear GEM object's associated cgroup
> > > + * @obj: GEM object
> > > + *
> > > + * This will release a reference on cgroup.
> > > + */
> > > +void drm_gem_object_unset_cgroup(struct drm_gem_object *obj)
> > > +{
> > > + WARN_ON(atomic64_read(&obj->drmcg_bytes_charged));
> > > + drmcg_put(obj->drmcg);
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_object_unset_cgroup);
> > > +
> > > +/**
> > > + * drm_gem_object_charge_mem - try charging size bytes to DRM cgroup
> > > + * @obj: GEM object which is being charged
> > > + * @size: number of bytes to charge
> > > + *
> > > + * Try to charge @size bytes to GEM object's associated DRM cgroup. This
> > > + * will fail if a successful charge would cause the current device memory
> > > + * usage to go above the cgroup's GPU memory maximum limit.
> > > + *
> > > + * Returns 0 on success. Otherwise, an error code is returned.
> > > + */
> > > +int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size)
> > > +{
> > > + int ret;
> > > +
> > > + ret = drm_cgroup_try_charge(obj->drmcg, obj->dev,
> > > + DRMCG_TYPE_MEM_CURRENT, size);
> > > + if (!ret)
> > > + atomic64_add(size, &obj->drmcg_bytes_charged);
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_object_charge_mem);
> > > +
> > > +/**
> > > + * drm_gem_object_uncharge_mem - uncharge size bytes from DRM cgroup
> > > + * @obj: GEM object which is being uncharged
> > > + * @size: number of bytes to uncharge
> > > + *
> > > + * Uncharge @size bytes from the DRM cgroup associated with specified
> > > + * GEM object.
> > > + *
> > > + * Returns 0 on success. Otherwise, an error code is returned.
> > > + */
> > > +void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size)
> > > +{
> > > + u64 charged = atomic64_read(&obj->drmcg_bytes_charged);
> > > +
> > > + if (WARN_ON(!charged))
> > > + return;
> > > + if (WARN_ON(size > charged))
> > > + size = charged;
> > > +
> > > + atomic64_sub(size, &obj->drmcg_bytes_charged);
> > > + drm_cgroup_uncharge(obj->drmcg, obj->dev, DRMCG_TYPE_MEM_CURRENT,
> > > + size);
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_object_uncharge_mem);
> > > +
> > > /**
> > > * drm_gem_object_init - initialize an allocated shmem-backed GEM object
> > > * @dev: drm_device the object should be initialized for
> > > @@ -156,6 +240,8 @@ void drm_gem_private_object_init(struct drm_device *dev,
> > > obj->dev = dev;
> > > obj->filp = NULL;
> > > + drm_gem_object_set_cgroup(obj, current);
> > > +
> > > kref_init(&obj->refcount);
> > > obj->handle_count = 0;
> > > obj->size = size;
> > > @@ -950,6 +1036,9 @@ drm_gem_object_release(struct drm_gem_object *obj)
> > > dma_resv_fini(&obj->_resv);
> > > drm_gem_free_mmap_offset(obj);
> > > +
> > > + /* Release reference on cgroup used with GEM object charging */
> > > + drm_gem_object_unset_cgroup(obj);
> > > }
> > > EXPORT_SYMBOL(drm_gem_object_release);
> > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> > > index 240049566592..06ea10fc17bc 100644
> > > --- a/include/drm/drm_gem.h
> > > +++ b/include/drm/drm_gem.h
> > > @@ -37,6 +37,7 @@
> > > #include <linux/kref.h>
> > > #include <linux/dma-resv.h>
> > > +#include <drm/drm_cgroup.h>
> > > #include <drm/drm_vma_manager.h>
> > > struct dma_buf_map;
> > > @@ -222,6 +223,17 @@ struct drm_gem_object {
> > > */
> > > struct file *filp;
> > > + /**
> > > + * @drmcg:
> > > + *
> > > + * cgroup used for charging GEM object page allocations against. This
> > > + * is set to the current cgroup during GEM object creation.
> > > + * Charging policy is up to the DRM driver to implement and should be
> > > + * charged when allocating backing store from device memory.
> > > + */
> > > + struct drmcg *drmcg;
> > > + atomic64_t drmcg_bytes_charged;
> > > +
> > > /**
> > > * @vma_node:
> > > *
> > > @@ -417,4 +429,9 @@ int drm_gem_fence_array_add_implicit(struct xarray *fence_array,
> > > int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
> > > u32 handle, u64 *offset);
> > > +void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
> > > + struct task_struct *task);
> > > +void drm_gem_object_unset_cgroup(struct drm_gem_object *obj);
> > > +int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size);
> > > +void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size);
> > > #endif /* __DRM_GEM_H__ */
> > > --
> > > 2.20.1
> > >
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
