diff mbox series

[2/2] drm/amdpgu: Use VRAM domain in UVD IB test

Message ID 88539BB5-333A-4019-A411-D09859291404@amd.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/ttm: Fix a deadlock if the target BO is not idle during swap | expand

Commit Message

Pan, Xinhui Sept. 3, 2021, 6:49 a.m. UTC 
Like vce/vcn does, visible VRAM is ok for ib test.
And in ib test stage, VRAM is sufficient.

Signed-off-by: xinhui pan <xinhui.pan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Christian König Sept. 3, 2021, 7:04 a.m. UTC | #1 
Am 03.09.21 um 08:49 schrieb Pan, Xinhui:
> Like vce/vcn does, visible VRAM is ok for ib test.
> And in ib test stage, VRAM is sufficient.

NAK, that won't work for older hw generations (e.g. SI, maybe CIK as 
well) where the IBs must be in a specific GTT hardware window.

Christian.

>
> Signed-off-by: xinhui pan <xinhui.pan@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index d451c359606a..1c099b79d12c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -1178,7 +1178,7 @@ int amdgpu_uvd_get_create_msg(struct amdgpu_ring *ring, uint32_t handle,
>   	int r, i;
>   
>   	r = amdgpu_bo_create_reserved(adev, 1024, PAGE_SIZE,
> -				      AMDGPU_GEM_DOMAIN_GTT,
> +				      AMDGPU_GEM_DOMAIN_VRAM,
>   				      &bo, NULL, (void **)&msg);
>   	if (r)
>   		return r;
> @@ -1210,7 +1210,7 @@ int amdgpu_uvd_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
>   	int r, i;
>   
>   	r = amdgpu_bo_create_reserved(adev, 1024, PAGE_SIZE,
> -				      AMDGPU_GEM_DOMAIN_GTT,
> +				      AMDGPU_GEM_DOMAIN_VRAM,
>   				      &bo, NULL, (void **)&msg);
>   	if (r)
>   		return r;
Pan, Xinhui Sept. 3, 2021, 7:15 a.m. UTC | #2 
在 2021/9/3 15:04，“Koenig, Christian”<Christian.Koenig@amd.com> 写入:

    Am 03.09.21 um 08:49 schrieb Pan, Xinhui:
    > Like vce/vcn does, visible VRAM is ok for ib test.
    > And in ib test stage, VRAM is sufficient.
    
    NAK, that won't work for older hw generations (e.g. SI, maybe CIK as 
    well) where the IBs must be in a specific GTT hardware window.
    
    Christian.
  
Not has older HW on hand for test.
But the uvd code says below. Looks like IBs should be in specific VRAM range[0 - 256MB]?
        if (!ring->adev->uvd.address_64_bit) {
                struct ttm_operation_ctx ctx = { true, false };

                amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_VRAM);
                amdgpu_uvd_force_into_uvd_segment(bo);
                r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
                if (r)
                        goto err; 
        }
  
    >
    > Signed-off-by: xinhui pan <xinhui.pan@amd.com>
    > ---
    >   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 4 ++--
    >   1 file changed, 2 insertions(+), 2 deletions(-)
    >
    > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
    > index d451c359606a..1c099b79d12c 100644
    > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
    > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
    > @@ -1178,7 +1178,7 @@ int amdgpu_uvd_get_create_msg(struct amdgpu_ring *ring, uint32_t handle,
    >   	int r, i;
    >   
    >   	r = amdgpu_bo_create_reserved(adev, 1024, PAGE_SIZE,
    > -				      AMDGPU_GEM_DOMAIN_GTT,
    > +				      AMDGPU_GEM_DOMAIN_VRAM,
    >   				      &bo, NULL, (void **)&msg);
    >   	if (r)
    >   		return r;
    > @@ -1210,7 +1210,7 @@ int amdgpu_uvd_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
    >   	int r, i;
    >   
    >   	r = amdgpu_bo_create_reserved(adev, 1024, PAGE_SIZE,
    > -				      AMDGPU_GEM_DOMAIN_GTT,
    > +				      AMDGPU_GEM_DOMAIN_VRAM,
    >   				      &bo, NULL, (void **)&msg);
    >   	if (r)
    >   		return r;
Christian König Sept. 3, 2021, 7:47 a.m. UTC | #3 
Am 03.09.21 um 09:15 schrieb Pan, Xinhui:
>
> 在 2021/9/3 15:04，“Koenig, Christian”<Christian.Koenig@amd.com> 写入:
>
>      Am 03.09.21 um 08:49 schrieb Pan, Xinhui:
>      > Like vce/vcn does, visible VRAM is ok for ib test.
>      > And in ib test stage, VRAM is sufficient.
>      
>      NAK, that won't work for older hw generations (e.g. SI, maybe CIK as
>      well) where the IBs must be in a specific GTT hardware window.
>      
>      Christian.
>    
> Not has older HW on hand for test.
> But the uvd code says below. Looks like IBs should be in specific VRAM range[0 - 256MB]?

Well, it's a long time that I locked into this.

The old UVD hardware has multiple 256MiB "windows". One is for message 
and feedback buffers which *must* be in VRAM and another one is for the 
DPB which should be in VRAM.

The IB on the other hand always had to be in GTT (except for 15+ year 
old AGP systems where it should be in VRAM as well for coherency reasons).

Could be that my memory is failing me and on SI/CIK both GTT and VRAM 
would work. But key point is don't touch this without extensive testing, 
it's really old stuff and was extremely painful to get working everywhere.

Christian.

>          if (!ring->adev->uvd.address_64_bit) {
>                  struct ttm_operation_ctx ctx = { true, false };
>
>                  amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_VRAM);
>                  amdgpu_uvd_force_into_uvd_segment(bo);
>                  r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
>                  if (r)
>                          goto err;
>          }
>    
>      >
>      > Signed-off-by: xinhui pan <xinhui.pan@amd.com>
>      > ---
>      >   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 4 ++--
>      >   1 file changed, 2 insertions(+), 2 deletions(-)
>      >
>      > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>      > index d451c359606a..1c099b79d12c 100644
>      > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>      > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>      > @@ -1178,7 +1178,7 @@ int amdgpu_uvd_get_create_msg(struct amdgpu_ring *ring, uint32_t handle,
>      >   	int r, i;
>      >
>      >   	r = amdgpu_bo_create_reserved(adev, 1024, PAGE_SIZE,
>      > -				      AMDGPU_GEM_DOMAIN_GTT,
>      > +				      AMDGPU_GEM_DOMAIN_VRAM,
>      >   				      &bo, NULL, (void **)&msg);
>      >   	if (r)
>      >   		return r;
>      > @@ -1210,7 +1210,7 @@ int amdgpu_uvd_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
>      >   	int r, i;
>      >
>      >   	r = amdgpu_bo_create_reserved(adev, 1024, PAGE_SIZE,
>      > -				      AMDGPU_GEM_DOMAIN_GTT,
>      > +				      AMDGPU_GEM_DOMAIN_VRAM,
>      >   				      &bo, NULL, (void **)&msg);
>      >   	if (r)
>      >   		return r;
>      
>      
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index d451c359606a..1c099b79d12c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -1178,7 +1178,7 @@  int amdgpu_uvd_get_create_msg(struct amdgpu_ring *ring, uint32_t handle,
 	int r, i;
 
 	r = amdgpu_bo_create_reserved(adev, 1024, PAGE_SIZE,
-				      AMDGPU_GEM_DOMAIN_GTT,
+				      AMDGPU_GEM_DOMAIN_VRAM,
 				      &bo, NULL, (void **)&msg);
 	if (r)
 		return r;
@@ -1210,7 +1210,7 @@  int amdgpu_uvd_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
 	int r, i;
 
 	r = amdgpu_bo_create_reserved(adev, 1024, PAGE_SIZE,
-				      AMDGPU_GEM_DOMAIN_GTT,
+				      AMDGPU_GEM_DOMAIN_VRAM,
 				      &bo, NULL, (void **)&msg);
 	if (r)
 		return r;