All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: puck.chen@hisilicon.com, dri-devel@lists.freedesktop.org,
	z.liuxinliang@hisilicon.com, kong.kongxinwei@hisilicon.com,
	kraxel@redhat.com, zourongrong@gmail.com, airlied@redhat.com,
	sam@ravnborg.org
Subject: Re: [PATCH 2/3] drm/vram-helper: Remove BO device from public interface
Date: Fri, 13 Dec 2019 11:08:28 +0100	[thread overview]
Message-ID: <20191213100828.GC624164@phenom.ffwll.local> (raw)
In-Reply-To: <20191211180832.7159-3-tzimmermann@suse.de>

On Wed, Dec 11, 2019 at 07:08:31PM +0100, Thomas Zimmermann wrote:
> TTM is an implementation detail of the VRAM helpers and therefore
> shouldn't be exposed to the callers. There's only one correct value
> for the BO device anyway, which is the one stored in the DRM device.
> 
> So remove struct ttm_bo_device from the VRAM-helper interface and
> use the device's VRAM manager unconditionally. The GEM initializer
> function fails if the VRAM manager has not been initialized.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

If/when someone is really bored I think might be really nice to create
something like drm_device.gem (or .mm) like we have for
drm_device.mode_config ... drm_device is an unwieldy monster.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/ast/ast_mode.c              |  3 +--
>  drivers/gpu/drm/drm_gem_vram_helper.c       | 21 +++++++++------------
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c |  2 +-
>  drivers/gpu/drm/mgag200/mgag200_cursor.c    |  3 +--
>  drivers/gpu/drm/mgag200/mgag200_drv.c       |  3 +--
>  include/drm/drm_gem_vram_helper.h           |  2 --
>  6 files changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 26336642dd59..f4fbdab29bb7 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1144,8 +1144,7 @@ static int ast_cursor_init(struct drm_device *dev)
>  	size = roundup(AST_HWC_SIZE + AST_HWC_SIGNATURE_SIZE, PAGE_SIZE);
>  
>  	for (i = 0; i < ARRAY_SIZE(ast->cursor.gbo); ++i) {
> -		gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
> -					  size, 0);
> +		gbo = drm_gem_vram_create(dev, size, 0);
>  		if (IS_ERR(gbo)) {
>  			ret = PTR_ERR(gbo);
>  			goto err_drm_gem_vram_put;
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 4908f1281002..b760fd27f3c0 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -92,13 +92,18 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo,
>  }
>  
>  static int drm_gem_vram_init(struct drm_device *dev,
> -			     struct ttm_bo_device *bdev,
>  			     struct drm_gem_vram_object *gbo,
>  			     size_t size, unsigned long pg_align)
>  {
> +	struct drm_vram_mm *vmm = dev->vram_mm;
> +	struct ttm_bo_device *bdev;
>  	int ret;
>  	size_t acc_size;
>  
> +	if (WARN_ONCE(!vmm, "VRAM MM not initialized"))
> +		return -EINVAL;
> +	bdev = &vmm->bdev;
> +
>  	gbo->bo.base.funcs = &drm_gem_vram_object_funcs;
>  
>  	ret = drm_gem_object_init(dev, &gbo->bo.base, size);
> @@ -126,7 +131,6 @@ static int drm_gem_vram_init(struct drm_device *dev,
>  /**
>   * drm_gem_vram_create() - Creates a VRAM-backed GEM object
>   * @dev:		the DRM device
> - * @bdev:		the TTM BO device backing the object
>   * @size:		the buffer size in bytes
>   * @pg_align:		the buffer's alignment in multiples of the page size
>   *
> @@ -135,7 +139,6 @@ static int drm_gem_vram_init(struct drm_device *dev,
>   * an ERR_PTR()-encoded error code otherwise.
>   */
>  struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
> -						struct ttm_bo_device *bdev,
>  						size_t size,
>  						unsigned long pg_align)
>  {
> @@ -146,7 +149,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
>  	if (!gbo)
>  		return ERR_PTR(-ENOMEM);
>  
> -	ret = drm_gem_vram_init(dev, bdev, gbo, size, pg_align);
> +	ret = drm_gem_vram_init(dev, gbo, size, pg_align);
>  	if (ret < 0)
>  		goto err_kfree;
>  
> @@ -480,7 +483,6 @@ EXPORT_SYMBOL(drm_gem_vram_vunmap);
>  	Helper for implementing &struct drm_driver.dumb_create
>   * @file:		the DRM file
>   * @dev:		the DRM device
> - * @bdev:		the TTM BO device managing the buffer object
>   * @pg_align:		the buffer's alignment in multiples of the page size
>   * @args:		the arguments as provided to \
>  				&struct drm_driver.dumb_create
> @@ -496,7 +498,6 @@ EXPORT_SYMBOL(drm_gem_vram_vunmap);
>   */
>  int drm_gem_vram_fill_create_dumb(struct drm_file *file,
>  				  struct drm_device *dev,
> -				  struct ttm_bo_device *bdev,
>  				  unsigned long pg_align,
>  				  struct drm_mode_create_dumb *args)
>  {
> @@ -512,7 +513,7 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file,
>  	if (!size)
>  		return -EINVAL;
>  
> -	gbo = drm_gem_vram_create(dev, bdev, size, pg_align);
> +	gbo = drm_gem_vram_create(dev, size, pg_align);
>  	if (IS_ERR(gbo))
>  		return PTR_ERR(gbo);
>  
> @@ -604,11 +605,7 @@ int drm_gem_vram_driver_dumb_create(struct drm_file *file,
>  				    struct drm_device *dev,
>  				    struct drm_mode_create_dumb *args)
>  {
> -	if (WARN_ONCE(!dev->vram_mm, "VRAM MM not initialized"))
> -		return -EINVAL;
> -
> -	return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev, 0,
> -					     args);
> +	return drm_gem_vram_fill_create_dumb(file, dev, 0, args);
>  }
>  EXPORT_SYMBOL(drm_gem_vram_driver_dumb_create);
>  
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> index 16f5cd2c1b3b..4a5bde80045a 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> @@ -58,7 +58,7 @@ int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel,
>  	if (size == 0)
>  		return -EINVAL;
>  
> -	gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev, size, 0);
> +	gbo = drm_gem_vram_create(dev, size, 0);
>  	if (IS_ERR(gbo)) {
>  		ret = PTR_ERR(gbo);
>  		if (ret != -ERESTARTSYS)
> diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
> index dd54fd507e13..d491edd317ff 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
> @@ -208,8 +208,7 @@ int mgag200_cursor_init(struct mga_device *mdev)
>  		return -ENOMEM;
>  
>  	for (i = 0; i < ncursors; ++i) {
> -		gbo = drm_gem_vram_create(dev, &dev->vram_mm->bdev,
> -					  size, 0);
> +		gbo = drm_gem_vram_create(dev, size, 0);
>  		if (IS_ERR(gbo)) {
>  			ret = PTR_ERR(gbo);
>  			goto err_drm_gem_vram_put;
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index dc125838f5d1..1b5b60da2e62 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -120,8 +120,7 @@ int mgag200_driver_dumb_create(struct drm_file *file,
>  	if (mgag200_pin_bo_at_0(mdev))
>  		pg_align = PFN_UP(mdev->mc.vram_size);
>  
> -	return drm_gem_vram_fill_create_dumb(file, dev, &dev->vram_mm->bdev,
> -					     pg_align, args);
> +	return drm_gem_vram_fill_create_dumb(file, dev, pg_align, args);
>  }
>  
>  static struct drm_driver driver = {
> diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
> index 90736427285e..9341f54383b3 100644
> --- a/include/drm/drm_gem_vram_helper.h
> +++ b/include/drm/drm_gem_vram_helper.h
> @@ -93,7 +93,6 @@ static inline struct drm_gem_vram_object *drm_gem_vram_of_gem(
>  }
>  
>  struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
> -						struct ttm_bo_device *bdev,
>  						size_t size,
>  						unsigned long pg_align);
>  void drm_gem_vram_put(struct drm_gem_vram_object *gbo);
> @@ -109,7 +108,6 @@ void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo, void *vaddr);
>  
>  int drm_gem_vram_fill_create_dumb(struct drm_file *file,
>  				  struct drm_device *dev,
> -				  struct ttm_bo_device *bdev,
>  				  unsigned long pg_align,
>  				  struct drm_mode_create_dumb *args);
>  
> -- 
> 2.24.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2019-12-13 10:08 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-11 18:08 [PATCH 0/3] drm/vram-helper: Various cleanups Thomas Zimmermann
2019-12-11 18:08 ` [PATCH 1/3] drm/vram-helper: Remove interruptible flag from public interface Thomas Zimmermann
2019-12-13 10:05   ` Daniel Vetter
2019-12-11 18:08 ` [PATCH 2/3] drm/vram-helper: Remove BO device " Thomas Zimmermann
2019-12-13 10:08   ` Daniel Vetter [this message]
2019-12-11 18:08 ` [PATCH 3/3] drm/vram-helper: Support struct drm_driver.gem_create_object Thomas Zimmermann
2019-12-11 19:11   ` Sam Ravnborg
2019-12-12  5:39     ` Thomas Zimmermann
2019-12-13 10:23       ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191213100828.GC624164@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kong.kongxinwei@hisilicon.com \
    --cc=kraxel@redhat.com \
    --cc=puck.chen@hisilicon.com \
    --cc=sam@ravnborg.org \
    --cc=tzimmermann@suse.de \
    --cc=z.liuxinliang@hisilicon.com \
    --cc=zourongrong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.