[RFC 1/3] drm/driver: Add defaults for .gem_prime_export/import callbacks

[Noralf Trønnes]

Den 22.09.2018 12.10, skrev Daniel Vetter:
> On Fri, Sep 21, 2018 at 06:42:28PM +0200, Noralf Trønnes wrote:
>> The majority of drivers use drm_gem_prime_export() and
>> drm_gem_prime_import() for these callbacks so let's make them the
>> default.
>>
>> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
>> ---
>>   Documentation/gpu/todo.rst  |  7 +++++++
>>   drivers/gpu/drm/drm_prime.c | 10 ++++++++--
>>   include/drm/drm_drv.h       |  4 ++++
>>   3 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
>> index 77c2b3c25565..39748e22dea8 100644
>> --- a/Documentation/gpu/todo.rst
>> +++ b/Documentation/gpu/todo.rst
>> @@ -234,6 +234,13 @@ efficient.
>>   
>>   Contact: Daniel Vetter
>>   
>> +Defaults for .gem_prime_import and export
>> +-----------------------------------------
>> +
>> +Most drivers don't need to set drm_driver->gem_prime_import and
>> +->gem_prime_export now that drm_gem_prime_import() and drm_gem_prime_export()
>> +are the default.
>> +
>>   Core refactorings
>>   =================
>>   
>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
>> index 3f0205fc0a1a..6721571749ff 100644
>> --- a/drivers/gpu/drm/drm_prime.c
>> +++ b/drivers/gpu/drm/drm_prime.c
>> @@ -559,7 +559,10 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev,
>>   		return dmabuf;
>>   	}
>>   
>> -	dmabuf = dev->driver->gem_prime_export(dev, obj, flags);
>> +	if (dev->driver->gem_prime_export)
>> +		dmabuf = dev->driver->gem_prime_export(dev, obj, flags);
>> +	else
>> +		dmabuf = drm_gem_prime_export(dev, obj, flags);
> Hm, this one here operates on a drm_gem_object (the dev parameter is kinda
> redundant). I think this would look neat in the callback structure you're
> adding in the next patch. There's also a bunch of gem drivers overwriting
> this one, so would make your callbacks structure more generally useful.

I'm not following you here.
What I do is to add a default for the 29 drivers that have this set to
drm_gem_prime_export(). For the 8 drivers that have their own thing, I
have the funcs->export callback that they can use.

Combined with the next patch it looks like this:

     if (dev->driver->gem_prime_export)
         dmabuf = dev->driver->gem_prime_export(dev, obj, flags);
     else if (obj->funcs && obj->funcs->export)
         dmabuf = obj->funcs->export(obj, flags);
     else
         dmabuf = drm_gem_prime_export(dev, obj, flags);

Noralf.

> Otherwise lgtm.
> -Daniel
>
>>   	if (IS_ERR(dmabuf)) {
>>   		/* normally the created dma-buf takes ownership of the ref,
>>   		 * but if that fails then drop the ref
>> @@ -792,7 +795,10 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>>   
>>   	/* never seen this one, need to import */
>>   	mutex_lock(&dev->object_name_lock);
>> -	obj = dev->driver->gem_prime_import(dev, dma_buf);
>> +	if (dev->driver->gem_prime_import)
>> +		obj = dev->driver->gem_prime_import(dev, dma_buf);
>> +	else
>> +		obj = drm_gem_prime_import(dev, dma_buf);
>>   	if (IS_ERR(obj)) {
>>   		ret = PTR_ERR(obj);
>>   		goto out_unlock;
>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>> index 8830e3de3a86..1162145f7f68 100644
>> --- a/include/drm/drm_drv.h
>> +++ b/include/drm/drm_drv.h
>> @@ -471,6 +471,8 @@ struct drm_driver {
>>   	 * @gem_prime_export:
>>   	 *
>>   	 * export GEM -> dmabuf
>> +	 *
>> +	 * This defaults to drm_gem_prime_export() if not set.
>>   	 */
>>   	struct dma_buf * (*gem_prime_export)(struct drm_device *dev,
>>   				struct drm_gem_object *obj, int flags);
>> @@ -478,6 +480,8 @@ struct drm_driver {
>>   	 * @gem_prime_import:
>>   	 *
>>   	 * import dmabuf -> GEM
>> +	 *
>> +	 * This defaults to drm_gem_prime_import() if not set.
>>   	 */
>>   	struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev,
>>   				struct dma_buf *dma_buf);
>> -- 
>> 2.15.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel