diff mbox series

[1/3] drm/ttm: Provide struct ttm_global for referencing TTM global state

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

Commit Message

Thomas Zimmermann Oct. 18, 2018, 4:27 p.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

Huang Rui Oct. 19, 2018, 3:30 a.m. UTC | #1 
On Fri, Oct 19, 2018 at 12:27:50AM +0800, Thomas Zimmermann wrote:
> 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;
> +

We would better add a protection here to make sure the glob is not NULL.

if (!glob)
        return -EINVAL;

Others, look good for me.

Thanks,
Ray

> +	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_ */
> -- 
> 2.19.1
>
Thomas Zimmermann Oct. 19, 2018, 7:38 a.m. UTC | #2 
Hi,

thanks for the review.

Am 19.10.18 um 05:30 schrieb Huang Rui:
> On Fri, Oct 19, 2018 at 12:27:50AM +0800, Thomas Zimmermann wrote:
>> +int ttm_global_init(struct ttm_global *glob)
>> +{
>> +	int ret;
>> +
> 
> We would better add a protection here to make sure the glob is not NULL.
> 
> if (!glob)
>         return -EINVAL;

Passing a NULL pointer is a programming error, not a runtime error. I'd
rather use BUG_ON() for this test. Ok?

Best regards
Thomas

> 
> Others, look good for me.
> 
> Thanks,
> Ray
> 
>> +	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_ */
>> -- 
>> 2.19.1
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Daniel Vetter Oct. 19, 2018, 7:49 a.m. UTC | #3 
On Fri, Oct 19, 2018 at 9:38 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi,
>
> thanks for the review.
>
> Am 19.10.18 um 05:30 schrieb Huang Rui:
> > On Fri, Oct 19, 2018 at 12:27:50AM +0800, Thomas Zimmermann wrote:
> >> +int ttm_global_init(struct ttm_global *glob)
> >> +{
> >> +    int ret;
> >> +
> >
> > We would better add a protection here to make sure the glob is not NULL.
> >
> > if (!glob)
> >         return -EINVAL;
>
> Passing a NULL pointer is a programming error, not a runtime error. I'd
> rather use BUG_ON() for this test. Ok?

if (WARN_ON(!glob)
    return -EINVAL;

instead of BUG_ON is much friendly for debugging. Especially in gfx
drivers, where the entire init runs while holding a few critical
kernel locks (console_lock, among others), and so will result in a
very silent death of the entire machine.
-Daniel

>
> Best regards
> Thomas
>
> >
> > Others, look good for me.
> >
> > Thanks,
> > Ray
> >
> >> +    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_ */
> >> --
> >> 2.19.1
> >>
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>
> --
> 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)
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Christian König Oct. 19, 2018, 7:51 a.m. UTC | #4 
Am 19.10.18 um 09:49 schrieb Daniel Vetter:
> On Fri, Oct 19, 2018 at 9:38 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Hi,
>>
>> thanks for the review.
>>
>> Am 19.10.18 um 05:30 schrieb Huang Rui:
>>> On Fri, Oct 19, 2018 at 12:27:50AM +0800, Thomas Zimmermann wrote:
>>>> +int ttm_global_init(struct ttm_global *glob)
>>>> +{
>>>> +    int ret;
>>>> +
>>> We would better add a protection here to make sure the glob is not NULL.
>>>
>>> if (!glob)
>>>          return -EINVAL;
>> Passing a NULL pointer is a programming error, not a runtime error. I'd
>> rather use BUG_ON() for this test. Ok?
> if (WARN_ON(!glob)
>      return -EINVAL;
>
> instead of BUG_ON is much friendly for debugging. Especially in gfx
> drivers, where the entire init runs while holding a few critical
> kernel locks (console_lock, among others), and so will result in a
> very silent death of the entire machine.

Yeah, additional to that a BUG_ON a NULL pointer is rather pointless.

Dereferencing the NULL pointer has pretty much the same effect as a 
BUG_ON :)

Christian.

> -Daniel
>
>> Best regards
>> Thomas
>>
>>> Others, look good for me.
>>>
>>> Thanks,
>>> Ray
>>>
>>>> +    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_ */
>>>> --
>>>> 2.19.1
>>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>> --
>> 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)
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
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_ */