Re: [PATCH 01/18] drm/ttm: Provide struct ttm_global for referencing TTM global state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 19.10.18 um 11:30 schrieb Christian König:
> In general I'm really graceful that you look into this, but you are just
> moving the complexity around instead of cleaning it up.
> 
> This whole global reference stuff is just going into the wrong direction.
> 
> Please let me clean that up instead,

Well, Ok.

One thing I noticed is that none of the drivers does anything with the
global TTM state. One thing that could be done is to remove the state
from the drivers and handle it entirely in ttm_bo_device.

Best regards
Thomas

> Christian.
> 
> Am 19.10.18 um 10:54 schrieb Thomas Zimmermann:
>> The new struct ttm_global provides drivers with TTM's global memory and
>> BO in a unified way. Initialization and release is handled internally.
>>
>> The functionality provided by struct ttm_global is currently
>> re-implemented
>> by a dozen individual DRM drivers using struct drm_global. The
>> implementation
>> of struct ttm_global is also built on top of drm_global, so it can
>> co-exists
>> with the existing drivers. Once all TTM-based drivers have been
>> converted to
>> struct ttm_global, the implementation of struct drm_global can be made
>> private to TTM.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
>> ---
>>   drivers/gpu/drm/ttm/Makefile     |  2 +-
>>   drivers/gpu/drm/ttm/ttm_global.c | 98 ++++++++++++++++++++++++++++++++
>>   include/drm/drm_global.h         |  8 +++
>>   include/drm/ttm/ttm_global.h     | 79 +++++++++++++++++++++++++
>>   4 files changed, 186 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/ttm/ttm_global.c
>>   create mode 100644 include/drm/ttm/ttm_global.h
>>
>> diff --git a/drivers/gpu/drm/ttm/Makefile b/drivers/gpu/drm/ttm/Makefile
>> index 01fc670ce7a2..b7272b26e9f3 100644
>> --- a/drivers/gpu/drm/ttm/Makefile
>> +++ b/drivers/gpu/drm/ttm/Makefile
>> @@ -5,7 +5,7 @@
>>   ttm-y := ttm_memory.o ttm_tt.o ttm_bo.o \
>>       ttm_bo_util.o ttm_bo_vm.o ttm_module.o \
>>       ttm_execbuf_util.o ttm_page_alloc.o ttm_bo_manager.o \
>> -    ttm_page_alloc_dma.o
>> +    ttm_page_alloc_dma.o ttm_global.o
>>   ttm-$(CONFIG_AGP) += ttm_agp_backend.o
>>     obj-$(CONFIG_DRM_TTM) += ttm.o
>> diff --git a/drivers/gpu/drm/ttm/ttm_global.c
>> b/drivers/gpu/drm/ttm/ttm_global.c
>> new file mode 100644
>> index 000000000000..ca9da0a46147
>> --- /dev/null
>> +++ b/drivers/gpu/drm/ttm/ttm_global.c
>> @@ -0,0 +1,98 @@
>> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
>> +/**************************************************************************
>>
>> + *
>> + * Copyright 2008-2009 VMware, Inc., Palo Alto, CA., USA
>> + * All Rights Reserved.
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> obtaining a
>> + * copy of this software and associated documentation files (the
>> + * "Software"), to deal in the Software without restriction, including
>> + * without limitation the rights to use, copy, modify, merge, publish,
>> + * distribute, sub license, and/or sell copies of the Software, and to
>> + * permit persons to whom the Software is furnished to do so, subject to
>> + * the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the
>> + * next paragraph) shall be included in all copies or substantial
>> portions
>> + * of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT
>> SHALL
>> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR
>> ANY CLAIM,
>> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE
>> OR THE
>> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
>> + *
>> +
>> **************************************************************************/
>>
>> +
>> +#include <drm/ttm/ttm_global.h>
>> +#include <drm/ttm/ttm_bo_driver.h>
>> +#include <drm/ttm/ttm_memory.h>
>> +#include <linux/kernel.h>
>> +
>> +static int ttm_global_init_mem(struct drm_global_reference *ref)
>> +{
>> +    BUG_ON(!ref->object);
>> +    return ttm_mem_global_init(ref->object);
>> +}
>> +
>> +static void ttm_global_release_mem(struct drm_global_reference *ref)
>> +{
>> +    BUG_ON(!ref->object);
>> +    ttm_mem_global_release(ref->object);
>> +}
>> +
>> +static int ttm_global_init_bo(struct drm_global_reference *ref)
>> +{
>> +    struct ttm_global *glob =
>> +        container_of(ref, struct ttm_global, bo_ref);
>> +    BUG_ON(!ref->object);
>> +    BUG_ON(!glob->mem_ref.object);
>> +    return ttm_bo_global_init(ref->object, glob->mem_ref.object);
>> +}
>> +
>> +static void ttm_global_release_bo(struct drm_global_reference *ref)
>> +{
>> +    BUG_ON(!ref->object);
>> +    ttm_bo_global_release(ref->object);
>> +}
>> +
>> +int ttm_global_init(struct ttm_global *glob)
>> +{
>> +    int ret;
>> +
>> +    glob->mem_ref.global_type = DRM_GLOBAL_TTM_MEM;
>> +    glob->mem_ref.size = sizeof(struct ttm_mem_global);
>> +    glob->bo_ref.object = NULL;
>> +    glob->mem_ref.init = &ttm_global_init_mem;
>> +    glob->mem_ref.release = &ttm_global_release_mem;
>> +    ret = drm_global_item_ref(&glob->mem_ref);
>> +    if (ret)
>> +        return ret;
>> +
>> +    glob->bo_ref.global_type = DRM_GLOBAL_TTM_BO;
>> +    glob->bo_ref.size = sizeof(struct ttm_bo_global);
>> +    glob->bo_ref.object = NULL;
>> +    glob->bo_ref.init = &ttm_global_init_bo;
>> +    glob->bo_ref.release = &ttm_global_release_bo;
>> +    ret = drm_global_item_ref(&glob->bo_ref);
>> +    if (ret)
>> +        goto err_drm_global_item_unref_mem;
>> +
>> +    return 0;
>> +
>> +err_drm_global_item_unref_mem:
>> +    drm_global_item_unref(&glob->mem_ref);
>> +    return ret;
>> +}
>> +
>> +EXPORT_SYMBOL(ttm_global_init);
>> +
>> +void ttm_global_release(struct ttm_global *glob)
>> +{
>> +    drm_global_item_unref(&glob->bo_ref);
>> +    drm_global_item_unref(&glob->mem_ref);
>> +}
>> +
>> +EXPORT_SYMBOL(ttm_global_release);
>> diff --git a/include/drm/drm_global.h b/include/drm/drm_global.h
>> index 3a830602a2e4..4482a9bbd6e9 100644
>> --- a/include/drm/drm_global.h
>> +++ b/include/drm/drm_global.h
>> @@ -28,8 +28,16 @@
>>    * Authors: Thomas Hellstrom <thellstrom-at-vmware-dot-com>
>>    */
>>   +/*
>> + *  The interfaces in this file are deprecated. Please use ttm_global
>> + *  from <drm/ttm/ttm_global.h> instead.
>> + */
>> +
>>   #ifndef _DRM_GLOBAL_H_
>>   #define _DRM_GLOBAL_H_
>> +
>> +#include <linux/types.h>
>> +
>>   enum drm_global_types {
>>       DRM_GLOBAL_TTM_MEM = 0,
>>       DRM_GLOBAL_TTM_BO,
>> diff --git a/include/drm/ttm/ttm_global.h b/include/drm/ttm/ttm_global.h
>> new file mode 100644
>> index 000000000000..06e791499f87
>> --- /dev/null
>> +++ b/include/drm/ttm/ttm_global.h
>> @@ -0,0 +1,79 @@
>> +/**************************************************************************
>>
>> + *
>> + * Copyright 2008-2009 VMware, Inc., Palo Alto, CA., USA
>> + * All Rights Reserved.
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> obtaining a
>> + * copy of this software and associated documentation files (the
>> + * "Software"), to deal in the Software without restriction, including
>> + * without limitation the rights to use, copy, modify, merge, publish,
>> + * distribute, sub license, and/or sell copies of the Software, and to
>> + * permit persons to whom the Software is furnished to do so, subject to
>> + * the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the
>> + * next paragraph) shall be included in all copies or substantial
>> portions
>> + * of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT
>> SHALL
>> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR
>> ANY CLAIM,
>> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE
>> OR THE
>> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
>> + *
>> +
>> **************************************************************************/
>>
>> +
>> +#ifndef _TTM_GLOBAL_H_
>> +#define _TTM_GLOBAL_H_
>> +
>> +#include <drm/drm_global.h>
>> +
>> +struct ttm_bo_global;
>> +
>> +/**
>> + * struct ttm_global - Stores references to global TTM state
>> + */
>> +struct ttm_global {
>> +    struct drm_global_reference mem_ref;
>> +    struct drm_global_reference bo_ref;
>> +};
>> +
>> +/**
>> + * ttm_global_init
>> + *
>> + * @glob: A pointer to a struct ttm_global that is about to be
>> initialized
>> + * @return Zero on success, or a negative error code otherwise.
>> + *
>> + * Initializes an instance of struct ttm_global with TTM's global state
>> + */
>> +int ttm_global_init(struct ttm_global *glob);
>> +
>> +/**
>> + * ttm_global_release
>> + *
>> + * @glob: A pointer to an instance of struct ttm_global
>> + *
>> + * Releases an initialized instance of struct ttm_global. If the
>> instance
>> + * constains the final references to the global memory and BO, the
>> global
>> + * structures are released as well.
>> + */
>> +void ttm_global_release(struct ttm_global *glob);
>> +
>> +/**
>> + * ttm_global_get_bo_global
>> + *
>> + * @glob A pointer to an instance of struct ttm_global
>> + * @return A refernce to the global BO.
>> + *
>> + * Returns the global BO. The BO should be forwarded to the
>> initialization
>> + * of a driver's TTM BO device.
>> + */
>> +static inline struct ttm_bo_global*
>> +ttm_global_get_bo_global(struct ttm_global *glob)
>> +{
>> +    return glob->bo_ref.object;
>> +}
>> +
>> +#endif /* _TTM_GLOBAL_H_ */
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstr. 5, D-90409 Nürnberg
Tel: +49-911-74053-0; Fax: +49-911-7417755;  https://www.suse.com/
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard,
Graham Norton, HRB 21284 (AG Nürnberg)

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux