diff mbox series

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

Message ID 20181019085423.28159-2-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series Provide a nice interface for TTM global state | expand

Commit Message

Thomas Zimmermann Oct. 19, 2018, 8:54 a.m. UTC 
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@suse.de>
---
 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

Comments

Christian König Oct. 19, 2018, 9:30 a.m. UTC | #1 
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,
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@suse.de>
> ---
>   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 Oct. 19, 2018, 9:41 a.m. UTC | #2 
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@suse.de>
>> ---
>>   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_ */
>
Christian König Oct. 19, 2018, 9:45 a.m. UTC | #3 
Am 19.10.18 um 11:41 schrieb Thomas Zimmermann:
> 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.

Yeah, exactly that's the point and you can even go a step further.

All what's in those "global" states are actually static information.

I can't work on much else today because I locked myself out of the AMD 
VPN. So going to give that a try.

Thanks,
Christian.

>
> 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@suse.de>
>>> ---
>>>    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_ */
diff mbox series

Patch

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_ */