AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Pan, Xinhui" <Xinhui.Pan@amd.com>
To: "Koenig, Christian" <Christian.Koenig@amd.com>,
	"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>
Cc: "Deucher, Alexander" <Alexander.Deucher@amd.com>,
	"Kuehling, Felix" <Felix.Kuehling@amd.com>
Subject: Re: [PATCH 1/2] drm//amdgpu: Add job fence to resv conditionally
Date: Fri, 13 Mar 2020 13:52:59 +0000	[thread overview]
Message-ID: <CA9D4D39-AF04-4F89-A418-B8A76D99F7C2@amd.com> (raw)
In-Reply-To: <a910a52f-b1c7-1cc3-18c5-6048a1074552@amd.com>


[-- Attachment #1.1: Type: text/plain, Size: 4044 bytes --]

Yep, will send v3.

Thanks
xinhui

发件人: "Koenig, Christian" <Christian.Koenig@amd.com>
日期: 2020年3月13日 星期五 21:46
收件人: "Pan, Xinhui" <Xinhui.Pan@amd.com>, "amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>
抄送: "Deucher, Alexander" <Alexander.Deucher@amd.com>, "Kuehling, Felix" <Felix.Kuehling@amd.com>
主题: Re: [PATCH 1/2] drm//amdgpu: Add job fence to resv conditionally

Yeah, but this is still the wrong resv object :)

See the object passed to amdgpu_vm_sdma_prepare() is the one of the BO which is mapped into the page tables and NOT the one of the page tables.

You need to use p->vm->root.base.bo->tbo.base.resv here.

Regards,
Christian.

Am 13.03.20 um 14:43 schrieb Pan, Xinhui:

[AMD Official Use Only - Internal Distribution Only]

page table BOs share same resv.It should be ok using any of them, root bo resv or bo resv.
I forgot to unref bos which cause problems. not good at rebasing...


________________________________
From: Koenig, Christian <Christian.Koenig@amd.com><mailto:Christian.Koenig@amd.com>
Sent: Friday, March 13, 2020 9:34:42 PM
To: Pan, Xinhui <Xinhui.Pan@amd.com><mailto:Xinhui.Pan@amd.com>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org><mailto:amd-gfx@lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com><mailto:Alexander.Deucher@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com><mailto:Felix.Kuehling@amd.com>
Subject: Re: [PATCH 1/2] drm//amdgpu: Add job fence to resv conditionally

Am 13.03.20 um 12:53 schrieb xinhui pan:
> If a job need sync the bo resv, it is likely that bo need the job fence
> to sync with others.

That won't work because this is the wrong resv object :)

You added the fence to the mapped BO and not the page table.

No wonder that this doesn't work,
Christian.

>
> Cc: Christian König <christian.koenig@amd.com><mailto:christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com><mailto:alexander.deucher@amd.com>
> Cc: Felix Kuehling <Felix.Kuehling@amd.com><mailto:Felix.Kuehling@amd.com>
> Suggested-by: Christian König <christian.koenig@amd.com><mailto:christian.koenig@amd.com>
> Signed-off-by: xinhui pan <xinhui.pan@amd.com><mailto:xinhui.pan@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h      | 5 +++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 9 +++++++++
>   2 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index b5705fcfc935..ca6021b4200b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -226,6 +226,11 @@ struct amdgpu_vm_update_params {
>         * @num_dw_left: number of dw left for the IB
>         */
>        unsigned int num_dw_left;
> +
> +     /**
> +      * @resv: sync the resv and add job fence to it conditionally.
> +      */
> +     struct dma_resv *resv;
>   };
>
>   struct amdgpu_vm_update_funcs {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> index 4cc7881f438c..0cfac59bff36 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
> @@ -70,6 +70,8 @@ static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
>
>        p->num_dw_left = ndw;
>
> +     p->resv = resv;
> +
>        if (!resv)
>                return 0;
>
> @@ -111,6 +113,13 @@ static int amdgpu_vm_sdma_commit(struct amdgpu_vm_update_params *p,
>                swap(p->vm->last_delayed, tmp);
>        dma_fence_put(tmp);
>
> +     /* add job fence to resv.
> +      * MM notifier path is an exception as we can not grab the
> +      * resv lock.
> +      */
> +     if (!p->direct && p->resv)
> +             dma_resv_add_shared_fence(p->resv, f);
> +
>        if (fence && !p->direct)
>                swap(*fence, f);
>        dma_fence_put(f);



[-- Attachment #1.2: Type: text/html, Size: 12269 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  reply	other threads:[~2020-03-13 13:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-13 11:53 [PATCH 1/2] drm//amdgpu: Add job fence to resv conditionally xinhui pan
2020-03-13 11:53 ` [PATCH 2/2] drm/amdgpu: unref the bo after job submit xinhui pan
2020-03-13 12:20 ` [PATCH 1/2] drm//amdgpu: Add job fence to resv conditionally Pan, Xinhui
2020-03-13 13:34 ` Christian König
2020-03-13 13:43   ` Pan, Xinhui
2020-03-13 13:46     ` Christian König
2020-03-13 13:52       ` Pan, Xinhui [this message]
2020-03-13 13:43   ` Pan, Xinhui

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=CA9D4D39-AF04-4F89-A418-B8A76D99F7C2@amd.com \
    --to=xinhui.pan@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Christian.Koenig@amd.com \
    --cc=Felix.Kuehling@amd.com \
    --cc=amd-gfx@lists.freedesktop.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox