From: "Zhang, Jerry (Junwei)" <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>
To: Chunming Zhou <david1.zhou-5C7GfCeVMHo@public.gmane.org>,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 1/2] drm/amdgpu: add amdgpu_bo_param
Date: Tue, 17 Apr 2018 14:12:00 +0800 [thread overview]
Message-ID: <5AD59030.5060307@amd.com> (raw)
In-Reply-To: <20180417055406.19902-1-david1.zhou-5C7GfCeVMHo@public.gmane.org>
On 04/17/2018 01:54 PM, Chunming Zhou wrote:
> amdgpu_bo_create has too many parameters, and used in
> too many places. Collect them to one structure.
Good cleanup.
feel free to add my RB for the series.
Reviewed-by: Junwei Zhang <Jerry.Zhang@amd.com>
>
> Change-Id: Ib2aa98ee37a70f3cb0d61eef1d336e89187554d5
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> Reviewed-by: Huang Rui <ray.huang@amd.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 75 +++++++++++++++++-------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 9 ++++
> 2 files changed, 51 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 24f582c696cc..b33a7fdea7f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -341,27 +341,25 @@ static bool amdgpu_bo_validate_size(struct amdgpu_device *adev,
> return false;
> }
>
> -static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size,
> - int byte_align, u32 domain,
> - u64 flags, enum ttm_bo_type type,
> - struct reservation_object *resv,
> +static int amdgpu_bo_do_create(struct amdgpu_device *adev,
> + struct amdgpu_bo_param *bp,
> struct amdgpu_bo **bo_ptr)
> {
> struct ttm_operation_ctx ctx = {
> - .interruptible = (type != ttm_bo_type_kernel),
> + .interruptible = (bp->type != ttm_bo_type_kernel),
> .no_wait_gpu = false,
> - .resv = resv,
> + .resv = bp->resv,
> .flags = TTM_OPT_FLAG_ALLOW_RES_EVICT
> };
> struct amdgpu_bo *bo;
> - unsigned long page_align;
> + unsigned long page_align, size = bp->size;
> size_t acc_size;
> int r;
>
> - page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT;
> + page_align = roundup(bp->byte_align, PAGE_SIZE) >> PAGE_SHIFT;
> size = ALIGN(size, PAGE_SIZE);
>
> - if (!amdgpu_bo_validate_size(adev, size, domain))
> + if (!amdgpu_bo_validate_size(adev, size, bp->domain))
> return -ENOMEM;
>
> *bo_ptr = NULL;
> @@ -375,18 +373,18 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size,
> drm_gem_private_object_init(adev->ddev, &bo->gem_base, size);
> INIT_LIST_HEAD(&bo->shadow_list);
> INIT_LIST_HEAD(&bo->va);
> - bo->preferred_domains = domain & (AMDGPU_GEM_DOMAIN_VRAM |
> - AMDGPU_GEM_DOMAIN_GTT |
> - AMDGPU_GEM_DOMAIN_CPU |
> - AMDGPU_GEM_DOMAIN_GDS |
> - AMDGPU_GEM_DOMAIN_GWS |
> - AMDGPU_GEM_DOMAIN_OA);
> + bo->preferred_domains = bp->domain & (AMDGPU_GEM_DOMAIN_VRAM |
> + AMDGPU_GEM_DOMAIN_GTT |
> + AMDGPU_GEM_DOMAIN_CPU |
> + AMDGPU_GEM_DOMAIN_GDS |
> + AMDGPU_GEM_DOMAIN_GWS |
> + AMDGPU_GEM_DOMAIN_OA);
> bo->allowed_domains = bo->preferred_domains;
> - if (type != ttm_bo_type_kernel &&
> + if (bp->type != ttm_bo_type_kernel &&
> bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
> bo->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
>
> - bo->flags = flags;
> + bo->flags = bp->flags;
>
> #ifdef CONFIG_X86_32
> /* XXX: Write-combined CPU mappings of GTT seem broken on 32-bit
> @@ -417,11 +415,11 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size,
> #endif
>
> bo->tbo.bdev = &adev->mman.bdev;
> - amdgpu_ttm_placement_from_domain(bo, domain);
> + amdgpu_ttm_placement_from_domain(bo, bp->domain);
>
> - r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, type,
> + r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
> &bo->placement, page_align, &ctx, acc_size,
> - NULL, resv, &amdgpu_ttm_bo_destroy);
> + NULL, bp->resv, &amdgpu_ttm_bo_destroy);
> if (unlikely(r != 0))
> return r;
>
> @@ -433,10 +431,10 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size,
> else
> amdgpu_cs_report_moved_bytes(adev, ctx.bytes_moved, 0);
>
> - if (type == ttm_bo_type_kernel)
> + if (bp->type == ttm_bo_type_kernel)
> bo->tbo.priority = 1;
>
> - if (flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
> + if (bp->flags & AMDGPU_GEM_CREATE_VRAM_CLEARED &&
> bo->tbo.mem.placement & TTM_PL_FLAG_VRAM) {
> struct dma_fence *fence;
>
> @@ -449,20 +447,20 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, unsigned long size,
> bo->tbo.moving = dma_fence_get(fence);
> dma_fence_put(fence);
> }
> - if (!resv)
> + if (!bp->resv)
> amdgpu_bo_unreserve(bo);
> *bo_ptr = bo;
>
> trace_amdgpu_bo_create(bo);
>
> /* Treat CPU_ACCESS_REQUIRED only as a hint if given by UMD */
> - if (type == ttm_bo_type_device)
> + if (bp->type == ttm_bo_type_device)
> bo->flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>
> return 0;
>
> fail_unreserve:
> - if (!resv)
> + if (!bp->resv)
> ww_mutex_unlock(&bo->tbo.resv->lock);
> amdgpu_bo_unref(&bo);
> return r;
> @@ -472,16 +470,21 @@ static int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
> unsigned long size, int byte_align,
> struct amdgpu_bo *bo)
> {
> + struct amdgpu_bo_param bp = {
> + .size = size,
> + .byte_align = byte_align,
> + .domain = AMDGPU_GEM_DOMAIN_GTT,
> + .flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC |
> + AMDGPU_GEM_CREATE_SHADOW,
> + .type = ttm_bo_type_kernel,
> + .resv = bo->tbo.resv
> + };
> int r;
>
> if (bo->shadow)
> return 0;
>
> - r = amdgpu_bo_do_create(adev, size, byte_align, AMDGPU_GEM_DOMAIN_GTT,
> - AMDGPU_GEM_CREATE_CPU_GTT_USWC |
> - AMDGPU_GEM_CREATE_SHADOW,
> - ttm_bo_type_kernel,
> - bo->tbo.resv, &bo->shadow);
> + r = amdgpu_bo_do_create(adev, &bp, &bo->shadow);
> if (!r) {
> bo->shadow->parent = amdgpu_bo_ref(bo);
> mutex_lock(&adev->shadow_list_lock);
> @@ -498,11 +501,17 @@ int amdgpu_bo_create(struct amdgpu_device *adev, unsigned long size,
> struct reservation_object *resv,
> struct amdgpu_bo **bo_ptr)
> {
> - uint64_t parent_flags = flags & ~AMDGPU_GEM_CREATE_SHADOW;
> + struct amdgpu_bo_param bp = {
> + .size = size,
> + .byte_align = byte_align,
> + .domain = domain,
> + .flags = flags & ~AMDGPU_GEM_CREATE_SHADOW,
> + .type = type,
> + .resv = resv
> + };
> int r;
>
> - r = amdgpu_bo_do_create(adev, size, byte_align, domain,
> - parent_flags, type, resv, bo_ptr);
> + r = amdgpu_bo_do_create(adev, &bp, bo_ptr);
> if (r)
> return r;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 1e9fe85abcbb..4bb6f0a8d799 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -33,6 +33,15 @@
>
> #define AMDGPU_BO_INVALID_OFFSET LONG_MAX
>
> +struct amdgpu_bo_param {
> + unsigned long size;
> + int byte_align;
> + u32 domain;
> + u64 flags;
> + enum ttm_bo_type type;
> + struct reservation_object *resv;
> +};
> +
> /* bo virtual addresses in a vm */
> struct amdgpu_bo_va_mapping {
> struct amdgpu_bo_va *bo_va;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2018-04-17 6:12 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-17 5:54 [PATCH 1/2] drm/amdgpu: add amdgpu_bo_param Chunming Zhou
[not found] ` <20180417055406.19902-1-david1.zhou-5C7GfCeVMHo@public.gmane.org>
2018-04-17 5:54 ` [PATCH 2/2] drm/amdgpu: use amdgpu_bo_param for amdgpu_bo_create v2 Chunming Zhou
[not found] ` <20180417055406.19902-2-david1.zhou-5C7GfCeVMHo@public.gmane.org>
2018-04-17 10:12 ` Christian König
2018-04-17 6:12 ` Zhang, Jerry (Junwei) [this message]
-- strict thread matches above, loose matches on Subject: below --
2018-04-16 11:13 [PATCH 1/2] drm/amdgpu: add amdgpu_bo_param Chunming Zhou
[not found] ` <20180416111305.640-1-david1.zhou-5C7GfCeVMHo@public.gmane.org>
2018-04-16 11:41 ` Huang Rui
2018-04-16 12:23 ` Christian König
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=5AD59030.5060307@amd.com \
--to=jerry.zhang-5c7gfcevmho@public.gmane.org \
--cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
--cc=david1.zhou-5C7GfCeVMHo@public.gmane.org \
/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.