From: Thomas Hellstrom <thellstrom@vmware.com>
To: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 2/4] drm/ttm: remove persistent_swap_storage argument from ttm_bo_init/ttm_bo_create
Date: Wed, 07 Nov 2012 13:36:08 +0100 [thread overview]
Message-ID: <509A55B8.7050701@vmware.com> (raw)
In-Reply-To: <1352238594-14991-2-git-send-email-marcin.slusarz@gmail.com>
On 11/06/2012 10:49 PM, Marcin Slusarz wrote:
> All drivers pass NULL here. ttm_buffer_object's field can still be set after
> init, just like nouveau does.
>
> Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> ---
> drivers/gpu/drm/ast/ast_ttm.c | 7 +++----
> drivers/gpu/drm/cirrus/cirrus_ttm.c | 2 +-
> drivers/gpu/drm/mgag200/mgag200_ttm.c | 2 +-
> drivers/gpu/drm/nouveau/nouveau_bo.c | 7 +++----
> drivers/gpu/drm/radeon/radeon_object.c | 6 +++---
> drivers/gpu/drm/ttm/ttm_bo.c | 7 ++-----
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 3 +--
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 8 +++-----
> include/drm/ttm/ttm_bo_api.h | 16 +++-------------
> 9 files changed, 20 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
> index 0a54f65..c6dcc84 100644
> --- a/drivers/gpu/drm/ast/ast_ttm.c
> +++ b/drivers/gpu/drm/ast/ast_ttm.c
> @@ -354,10 +354,9 @@ int ast_bo_create(struct drm_device *dev, int size, int align,
> acc_size = ttm_bo_dma_acc_size(&ast->ttm.bdev, size,
> sizeof(struct ast_bo));
>
> - ret = ttm_bo_init(&ast->ttm.bdev, &astbo->bo, size,
> - ttm_bo_type_device, &astbo->placement,
> - align >> PAGE_SHIFT, false, NULL, acc_size,
> - NULL, ast_bo_ttm_destroy);
> + ret = ttm_bo_init(&ast->ttm.bdev, &astbo->bo, size, ttm_bo_type_device,
> + &astbo->placement, align >> PAGE_SHIFT, false,
> + acc_size, NULL, ast_bo_ttm_destroy);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c b/drivers/gpu/drm/cirrus/cirrus_ttm.c
> index 90d7701..65e665f 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
> @@ -361,7 +361,7 @@ int cirrus_bo_create(struct drm_device *dev, int size, int align,
>
> ret = ttm_bo_init(&cirrus->ttm.bdev, &cirrusbo->bo, size,
> ttm_bo_type_device, &cirrusbo->placement,
> - align >> PAGE_SHIFT, false, NULL, acc_size,
> + align >> PAGE_SHIFT, false, acc_size,
> NULL, cirrus_bo_ttm_destroy);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c
> index 49d60a6..be121d1 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
> @@ -355,7 +355,7 @@ int mgag200_bo_create(struct drm_device *dev, int size, int align,
>
> ret = ttm_bo_init(&mdev->ttm.bdev, &mgabo->bo, size,
> ttm_bo_type_device, &mgabo->placement,
> - align >> PAGE_SHIFT, false, NULL, acc_size,
> + align >> PAGE_SHIFT, false, acc_size,
> NULL, mgag200_bo_ttm_destroy);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 3cbf1a8..82f0f3c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -223,10 +223,9 @@ nouveau_bo_new(struct drm_device *dev, int size, int align,
> acc_size = ttm_bo_dma_acc_size(&drm->ttm.bdev, size,
> sizeof(struct nouveau_bo));
>
> - ret = ttm_bo_init(&drm->ttm.bdev, &nvbo->bo, size,
> - type, &nvbo->placement,
> - align >> PAGE_SHIFT, false, NULL, acc_size, sg,
> - nouveau_bo_del_ttm);
> + ret = ttm_bo_init(&drm->ttm.bdev, &nvbo->bo, size, type,
> + &nvbo->placement, align >> PAGE_SHIFT, false,
> + acc_size, sg, nouveau_bo_del_ttm);
> if (ret) {
> /* ttm will call nouveau_bo_del_ttm if it fails.. */
> return ret;
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index 0665845..834b291 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -139,9 +139,9 @@ int radeon_bo_create(struct radeon_device *rdev,
> radeon_ttm_placement_from_domain(bo, domain);
> /* Kernel allocation are uninterruptible */
> down_read(&rdev->pm.mclk_lock);
> - r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
> - &bo->placement, page_align, !kernel, NULL,
> - acc_size, sg, &radeon_ttm_bo_destroy);
> + r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type, &bo->placement,
> + page_align, !kernel, acc_size, sg,
> + &radeon_ttm_bo_destroy);
> up_read(&rdev->pm.mclk_lock);
> if (unlikely(r != 0)) {
> return r;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 27a2d3f..412486c 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1180,7 +1180,6 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
> struct ttm_placement *placement,
> uint32_t page_alignment,
> bool interruptible,
> - struct file *persistent_swap_storage,
> size_t acc_size,
> struct sg_table *sg,
> void (*destroy) (struct ttm_buffer_object *))
> @@ -1234,7 +1233,7 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
> bo->priv_flags = 0;
> bo->mem.placement = (TTM_PL_FLAG_SYSTEM | TTM_PL_FLAG_CACHED);
> bo->seq_valid = false;
> - bo->persistent_swap_storage = persistent_swap_storage;
> + bo->persistent_swap_storage = NULL;
> bo->acc_size = acc_size;
> bo->sg = sg;
> atomic_inc(&bo->glob->bo_count);
> @@ -1304,7 +1303,6 @@ int ttm_bo_create(struct ttm_bo_device *bdev,
> struct ttm_placement *placement,
> uint32_t page_alignment,
> bool interruptible,
> - struct file *persistent_swap_storage,
> struct ttm_buffer_object **p_bo)
> {
> struct ttm_buffer_object *bo;
> @@ -1317,8 +1315,7 @@ int ttm_bo_create(struct ttm_bo_device *bdev,
>
> acc_size = ttm_bo_acc_size(bdev, size, sizeof(struct ttm_buffer_object));
> ret = ttm_bo_init(bdev, bo, size, type, placement, page_alignment,
> - interruptible, persistent_swap_storage, acc_size,
> - NULL, NULL);
> + interruptible, acc_size, NULL, NULL);
> if (likely(ret == 0))
> *p_bo = bo;
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 9f37b72..fcd9107 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -292,8 +292,7 @@ static int vmw_dummy_query_bo_create(struct vmw_private *dev_priv)
> PAGE_SIZE,
> ttm_bo_type_device,
> &vmw_vram_sys_placement,
> - 0, false, NULL,
> - &dev_priv->dummy_query_bo);
> + 0, false, &dev_priv->dummy_query_bo);
> }
>
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> index be87124..4396fd8 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -955,7 +955,7 @@ int vmw_surface_evict(struct vmw_private *dev_priv,
> ret = ttm_bo_create(&dev_priv->bdev, srf->backup_size,
> ttm_bo_type_device,
> &vmw_srf_placement, 0, true,
> - NULL, &srf->backup);
> + &srf->backup);
> if (unlikely(ret != 0))
> return ret;
> }
> @@ -1564,10 +1564,8 @@ int vmw_dmabuf_init(struct vmw_private *dev_priv,
>
> INIT_LIST_HEAD(&vmw_bo->validate_list);
>
> - ret = ttm_bo_init(bdev, &vmw_bo->base, size,
> - ttm_bo_type_device, placement,
> - 0, interruptible,
> - NULL, acc_size, NULL, bo_free);
> + ret = ttm_bo_init(bdev, &vmw_bo->base, size, ttm_bo_type_device,
> + placement, 0, interruptible, acc_size, NULL, bo_free);
> return ret;
> }
>
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 9a71fda..641bd13 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -157,7 +157,9 @@ struct ttm_tt;
> * @mem: structure describing current placement.
> * @persistent_swap_storage: Usually the swap storage is deleted for buffers
> * pinned in physical memory. If this behaviour is not desired, this member
> - * holds a pointer to a persistent shmem object.
> + * holds a pointer to a persistent shmem object. Typically, this would
> + * point to the shmem object backing a GEM object if TTM is used to back a
> + * GEM user interface.
> * @ttm: TTM structure holding system pages.
> * @evicted: Whether the object was evicted without user-space knowing.
> * @cpu_writes: For synchronization. Number of cpu writers.
> @@ -471,11 +473,6 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device *bdev,
> * @page_alignment: Data alignment in pages.
> * @interruptible: If needing to sleep to wait for GPU resources,
> * sleep interruptible.
> - * @persistent_swap_storage: Usually the swap storage is deleted for buffers
> - * pinned in physical memory. If this behaviour is not desired, this member
> - * holds a pointer to a persistent shmem object. Typically, this would
> - * point to the shmem object backing a GEM object if TTM is used to back a
> - * GEM user interface.
> * @acc_size: Accounted size for this object.
> * @destroy: Destroy function. Use NULL for kfree().
> *
> @@ -501,7 +498,6 @@ extern int ttm_bo_init(struct ttm_bo_device *bdev,
> struct ttm_placement *placement,
> uint32_t page_alignment,
> bool interrubtible,
> - struct file *persistent_swap_storage,
> size_t acc_size,
> struct sg_table *sg,
> void (*destroy) (struct ttm_buffer_object *));
> @@ -517,11 +513,6 @@ extern int ttm_bo_init(struct ttm_bo_device *bdev,
> * @page_alignment: Data alignment in pages.
> * @interruptible: If needing to sleep while waiting for GPU resources,
> * sleep interruptible.
> - * @persistent_swap_storage: Usually the swap storage is deleted for buffers
> - * pinned in physical memory. If this behaviour is not desired, this member
> - * holds a pointer to a persistent shmem object. Typically, this would
> - * point to the shmem object backing a GEM object if TTM is used to back a
> - * GEM user interface.
> * @p_bo: On successful completion *p_bo points to the created object.
> *
> * This function allocates a ttm_buffer_object, and then calls ttm_bo_init
> @@ -538,7 +529,6 @@ extern int ttm_bo_create(struct ttm_bo_device *bdev,
> struct ttm_placement *placement,
> uint32_t page_alignment,
> bool interruptible,
> - struct file *persistent_swap_storage,
> struct ttm_buffer_object **p_bo);
>
> /**
I'm not convinced about this one. All gem-aware drivers could
potentially use it, so we should instead figure out whether the
fact that they don't is an oversight.
/Thomas
prev parent reply other threads:[~2012-11-07 12:47 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-06 21:49 [PATCH 2/4] drm/ttm: remove persistent_swap_storage argument from ttm_bo_init/ttm_bo_create Marcin Slusarz
2012-11-07 12:36 ` Thomas Hellstrom [this message]
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=509A55B8.7050701@vmware.com \
--to=thellstrom@vmware.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=marcin.slusarz@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.