Commit d693def4 authored by Thomas Zimmermann's avatar Thomas Zimmermann
Browse files

drm: Remove obsolete GEM and PRIME callbacks from struct drm_driver 



Several GEM and PRIME callbacks have been deprecated in favor of
per-instance GEM object functions. Remove the callbacks as they are
now unused. The only exception is .gem_prime_mmap, which is still
in use by several drivers.

What is also gone is gem_vm_ops in struct drm_driver. All drivers now
use struct drm_gem_object_funcs.vm_ops instead.

While at it, the patch also improves error handling around calls
to .free and .get_sg_table callbacks.

v3:
	* restore default call to drm_gem_prime_export() in
	  drm_gem_prime_handle_to_fd()
	* return -ENOSYS if get_sg_table is not set
	* drop all checks for obj->funcs
	* clean up TODO list and documentation
v2:
	* update related TODO item (Sam)

Signed-off-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
Acked-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: default avatarChristian König <christian.koenig@amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200923102159.24084-23-tzimmermann@suse.de
parent c85dce1b

Documentation/gpu/drm-mm.rst

+2 −2
Original line number Diff line number Diff line
@@ -182,11 +182,11 @@ acquired and release by calling drm_gem_object_get() and drm_gem_object_put() 
respectively.



When the last reference to a GEM object is released the GEM core calls

the :c:type:`struct drm_driver <drm_driver>` gem_free_object_unlocked

the :c:type:`struct drm_gem_object_funcs <gem_object_funcs>` free

operation. That operation is mandatory for GEM-enabled drivers and must

free the GEM object and all associated resources.



void (\*gem_free_object) (struct drm_gem_object \*obj); Drivers are

void (\*free) (struct drm_gem_object \*obj); Drivers are

responsible for freeing all GEM object resources. This includes the

resources created by the GEM core, which need to be released with

drm_gem_object_release().

Documentation/gpu/todo.rst

+3 −6
Original line number Diff line number Diff line
@@ -149,7 +149,7 @@ have to keep track of that lock and either call ``unreference`` or 
``unreference_locked`` depending upon context.



Core GEM doesn't have a need for ``struct_mutex`` any more since kernel 4.8,

and there's a ``gem_free_object_unlocked`` callback for any drivers which are

and there's a GEM object ``free`` callback for any drivers which are

entirely ``struct_mutex`` free.



For drivers that need ``struct_mutex`` it should be replaced with a driver-
@@ -289,11 +289,8 @@ struct drm_gem_object_funcs 
---------------------------



GEM objects can now have a function table instead of having the callbacks on the

DRM driver struct. This is now the preferred way and drivers can be moved over.



We also need a 2nd version of the CMA define that doesn't require the

vmapping to be present (different hook for prime importing). Plus this needs to

be rolled out to all drivers using their own implementations, too.

DRM driver struct. This is now the preferred way. Callbacks in drivers have been

converted, except for struct drm_driver.gem_prime_mmap.



Level: Intermediate

drivers/gpu/drm/drm_gem.c

+17 −36
Original line number Diff line number Diff line
@@ -247,12 +247,9 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) 
{

	struct drm_file *file_priv = data;

	struct drm_gem_object *obj = ptr;

	struct drm_device *dev = obj->dev;



	if (obj->funcs && obj->funcs->close)

	if (obj->funcs->close)

		obj->funcs->close(obj, file_priv);

	else if (dev->driver->gem_close_object)

		dev->driver->gem_close_object(obj, file_priv);



	drm_gem_remove_prime_handles(obj, file_priv);

	drm_vma_node_revoke(&obj->vma_node, file_priv);
@@ -403,14 +400,10 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, 
	if (ret)

		goto err_remove;



	if (obj->funcs && obj->funcs->open) {

	if (obj->funcs->open) {

		ret = obj->funcs->open(obj, file_priv);

		if (ret)

			goto err_revoke;

	} else if (dev->driver->gem_open_object) {

		ret = dev->driver->gem_open_object(obj, file_priv);

		if (ret)

			goto err_revoke;

	}



	*handlep = handle;
@@ -982,12 +975,11 @@ drm_gem_object_free(struct kref *kref) 
{

	struct drm_gem_object *obj =

		container_of(kref, struct drm_gem_object, refcount);

	struct drm_device *dev = obj->dev;



	if (obj->funcs)

	if (WARN_ON(!obj->funcs->free))

		return;



	obj->funcs->free(obj);

	else if (dev->driver->gem_free_object_unlocked)

		dev->driver->gem_free_object_unlocked(obj);

}

EXPORT_SYMBOL(drm_gem_object_free);
@@ -1049,9 +1041,9 @@ EXPORT_SYMBOL(drm_gem_vm_close); 
 * @obj_size: the object size to be mapped, in bytes

 * @vma: VMA for the area to be mapped

 *

 * Set up the VMA to prepare mapping of the GEM object using the gem_vm_ops

 * provided by the driver. Depending on their requirements, drivers can either

 * provide a fault handler in their gem_vm_ops (in which case any accesses to

 * Set up the VMA to prepare mapping of the GEM object using the GEM object's

 * vm_ops. Depending on their requirements, GEM objects can either

 * provide a fault handler in their vm_ops (in which case any accesses to

 * the object will be trapped, to perform migration, GTT binding, surface

 * register allocation, or performance monitoring), or mmap the buffer memory

 * synchronously after calling drm_gem_mmap_obj.
@@ -1065,12 +1057,11 @@ EXPORT_SYMBOL(drm_gem_vm_close); 
 * callers must verify access restrictions before calling this helper.

 *

 * Return 0 or success or -EINVAL if the object size is smaller than the VMA

 * size, or if no gem_vm_ops are provided.

 * size, or if no vm_ops are provided.

 */

int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,

		     struct vm_area_struct *vma)

{

	struct drm_device *dev = obj->dev;

	int ret;



	/* Check for valid size. */
@@ -1085,7 +1076,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, 
	 */

	drm_gem_object_get(obj);



	if (obj->funcs && obj->funcs->mmap) {

	if (obj->funcs->mmap) {

		ret = obj->funcs->mmap(obj, vma);

		if (ret) {

			drm_gem_object_put(obj);
@@ -1093,10 +1084,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size, 
		}

		WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));

	} else {

		if (obj->funcs && obj->funcs->vm_ops)

		if (obj->funcs->vm_ops)

			vma->vm_ops = obj->funcs->vm_ops;

		else if (dev->driver->gem_vm_ops)

			vma->vm_ops = dev->driver->gem_vm_ops;

		else {

			drm_gem_object_put(obj);

			return -EINVAL;
@@ -1198,36 +1187,30 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent, 
	drm_printf_indent(p, indent, "imported=%s\n",

			  obj->import_attach ? "yes" : "no");



	if (obj->funcs && obj->funcs->print_info)

	if (obj->funcs->print_info)

		obj->funcs->print_info(p, indent, obj);

}



int drm_gem_pin(struct drm_gem_object *obj)

{

	if (obj->funcs && obj->funcs->pin)

	if (obj->funcs->pin)

		return obj->funcs->pin(obj);

	else if (obj->dev->driver->gem_prime_pin)

		return obj->dev->driver->gem_prime_pin(obj);

	else

		return 0;

}



void drm_gem_unpin(struct drm_gem_object *obj)

{

	if (obj->funcs && obj->funcs->unpin)

	if (obj->funcs->unpin)

		obj->funcs->unpin(obj);

	else if (obj->dev->driver->gem_prime_unpin)

		obj->dev->driver->gem_prime_unpin(obj);

}



void *drm_gem_vmap(struct drm_gem_object *obj)

{

	void *vaddr;



	if (obj->funcs && obj->funcs->vmap)

	if (obj->funcs->vmap)

		vaddr = obj->funcs->vmap(obj);

	else if (obj->dev->driver->gem_prime_vmap)

		vaddr = obj->dev->driver->gem_prime_vmap(obj);

	else

		vaddr = ERR_PTR(-EOPNOTSUPP);
@@ -1242,10 +1225,8 @@ void drm_gem_vunmap(struct drm_gem_object *obj, void *vaddr) 
	if (!vaddr)

		return;



	if (obj->funcs && obj->funcs->vunmap)

	if (obj->funcs->vunmap)

		obj->funcs->vunmap(obj, vaddr);

	else if (obj->dev->driver->gem_prime_vunmap)

		obj->dev->driver->gem_prime_vunmap(obj, vaddr);

}



/**

drivers/gpu/drm/drm_gem_cma_helper.c

+4 −4
Original line number Diff line number Diff line
@@ -171,7 +171,7 @@ drm_gem_cma_create_with_handle(struct drm_file *file_priv, 
 * GEM object state and frees the memory used to store the object itself.

 * If the buffer is imported and the virtual address is set, it is released.

 * Drivers using the CMA helpers should set this as their

 * &drm_driver.gem_free_object_unlocked callback.

 * &drm_gem_object_funcs.free callback.

 */

void drm_gem_cma_free_object(struct drm_gem_object *gem_obj)

{
@@ -419,7 +419,7 @@ EXPORT_SYMBOL(drm_gem_cma_print_info); 
 *

 * This function exports a scatter/gather table suitable for PRIME usage by

 * calling the standard DMA mapping API. Drivers using the CMA helpers should

 * set this as their &drm_driver.gem_prime_get_sg_table callback.

 * set this as their &drm_gem_object_funcs.get_sg_table callback.

 *

 * Returns:

 * A pointer to the scatter/gather table of pinned pages or NULL on failure.
@@ -542,7 +542,7 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_prime_mmap); 
 * virtual address space. Since the CMA buffers are already mapped into the

 * kernel virtual address space this simply returns the cached virtual

 * address. Drivers using the CMA helpers should set this as their DRM

 * driver's &drm_driver.gem_prime_vmap callback.

 * driver's &drm_gem_object_funcs.vmap callback.

 *

 * Returns:

 * The kernel virtual address of the CMA GEM object's backing store.
@@ -564,7 +564,7 @@ EXPORT_SYMBOL_GPL(drm_gem_cma_prime_vmap); 
 * This function removes a buffer exported via DRM PRIME from the kernel's

 * virtual address space. This is a no-op because CMA buffers cannot be

 * unmapped from kernel space. Drivers using the CMA helpers should set this

 * as their &drm_driver.gem_prime_vunmap callback.

 * as their &drm_gem_object_funcs.vunmap callback.

 */

void drm_gem_cma_prime_vunmap(struct drm_gem_object *obj, void *vaddr)

{

drivers/gpu/drm/drm_prime.c

+7 −7
Original line number Diff line number Diff line
@@ -386,8 +386,6 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, 


	if (obj->funcs && obj->funcs->export)

		dmabuf = obj->funcs->export(obj, flags);

	else if (dev->driver->gem_prime_export)

		dmabuf = dev->driver->gem_prime_export(obj, flags);

	else

		dmabuf = drm_gem_prime_export(obj, flags);

	if (IS_ERR(dmabuf)) {
@@ -419,7 +417,7 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev, 
 * This is the PRIME export function which must be used mandatorily by GEM

 * drivers to ensure correct lifetime management of the underlying GEM object.

 * The actual exporting from GEM object to a dma-buf is done through the

 * &drm_driver.gem_prime_export driver callback.

 * &drm_gem_object_funcs.export callback.

 */

int drm_gem_prime_handle_to_fd(struct drm_device *dev,

			       struct drm_file *file_priv, uint32_t handle,
@@ -621,10 +619,12 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach, 
	if (WARN_ON(dir == DMA_NONE))

		return ERR_PTR(-EINVAL);



	if (obj->funcs)

	if (WARN_ON(!obj->funcs->get_sg_table))

		return ERR_PTR(-ENOSYS);



	sgt = obj->funcs->get_sg_table(obj);

	else

		sgt = obj->dev->driver->gem_prime_get_sg_table(obj);

	if (IS_ERR(sgt))

		return sgt;



	if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,

			      DMA_ATTR_SKIP_CPU_SYNC)) {

CRA Git | Maintained and supported by SUSTech CRA and CCSE