On 5/25/26 13:33, Timur Kristóf wrote:
> UVD 4.x and older can only access MSG, FEEDBACK buffers from a
> specific 256M VRAM segment that the VCPU BO is also located in.
> We already modify all placements of the given BO to ensure
> the BO is placed within this segment.
>
> Previously, it always assumed that the VCPU segment is
> the first 256M of VRAM, even though under some conditions
> the VCPU BO could be allocated outside this segment,
> which made UVD non-functional as the BOs were
> not inside the same segment as the UVD VCPU BO.
>
> Solve that by using the segment where the VCPU BO actually is.
>
> This fixes an issue with UVD failing to initialize on SI/CIK
> when resizable BAR is enabled and the VCPU BO is allocated
> in a different segment.
>
> v2:
> - For other BOs, keep using the same UVD segment as before.
>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/3851
> Signed-off-by: Timur Kristóf <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 33 ++++++++++++++++++-------
> 1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index 1e59ca924abe..480bf88def46 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -135,7 +135,7 @@ MODULE_FIRMWARE(FIRMWARE_VEGA12);
> MODULE_FIRMWARE(FIRMWARE_VEGA20);
>
> static void amdgpu_uvd_idle_work_handler(struct work_struct *work);
> -static void amdgpu_uvd_force_into_uvd_segment(struct amdgpu_bo *abo);
> +static void amdgpu_uvd_force_into_vcpu_segment(struct amdgpu_bo *abo);
>
> static int amdgpu_uvd_create_msg_bo_helper(struct amdgpu_device *adev,
> uint32_t size,
> @@ -158,7 +158,7 @@ static int amdgpu_uvd_create_msg_bo_helper(struct
> amdgpu_device *adev,
> amdgpu_bo_kunmap(bo);
> amdgpu_bo_unpin(bo);
> amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_VRAM);
> - amdgpu_uvd_force_into_uvd_segment(bo);
> + amdgpu_uvd_force_into_vcpu_segment(bo);
> r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
> if (r)
> goto err;
> @@ -550,6 +550,24 @@ void amdgpu_uvd_free_handles(struct amdgpu_device *adev,
> struct drm_file *filp)
> }
> }
>
> +static void amdgpu_uvd_force_into_vcpu_segment(struct amdgpu_bo *bo)
> +{
> + struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> + struct amdgpu_bo *vcpu_bo = adev->uvd.inst[0].vcpu_bo;
> + struct amdgpu_res_cursor vcpu_cur;
> +
> + amdgpu_res_first(vcpu_bo->tbo.resource, 0,
> + amdgpu_bo_size(vcpu_bo), &vcpu_cur);
> +
> + bo->placement.num_placement = 1;
> + bo->placement.placement = &bo->placements[0];
> + bo->placements[0].fpfn = ALIGN_DOWN(vcpu_cur.start, SZ_256M) >>
> PAGE_SHIFT;
> + bo->placements[0].lpfn = bo->placements[0].fpfn + (SZ_256M >>
> PAGE_SHIFT);
> + bo->placements[0].mem_type = vcpu_bo->tbo.resource->mem_type;
> + if (bo->placements[0].mem_type == TTM_PL_VRAM)
> + bo->placements[0].flags |= TTM_PL_FLAG_CONTIGUOUS;
You need to call ttm_bo_validate() here.
Apart from that looks good to me.
Regards,
Christian
> +}
> +
> static void amdgpu_uvd_force_into_uvd_segment(struct amdgpu_bo *abo)
> {
> int i;
> @@ -600,13 +618,10 @@ static int amdgpu_uvd_cs_pass1(struct amdgpu_uvd_cs_ctx
> *ctx)
> if (!ctx->parser->adev->uvd.address_64_bit) {
> /* check if it's a message or feedback command */
> cmd = amdgpu_ib_get_value(ctx->ib, ctx->idx) >> 1;
> - if (cmd == 0x0 || cmd == 0x3) {
> - /* yes, force it into VRAM */
> - uint32_t domain = AMDGPU_GEM_DOMAIN_VRAM;
> -
> - amdgpu_bo_placement_from_domain(bo, domain);
> - }
> - amdgpu_uvd_force_into_uvd_segment(bo);
> + if (cmd == 0x0 || cmd == 0x3)
> + amdgpu_uvd_force_into_vcpu_segment(bo);
> + else
> + amdgpu_uvd_force_into_uvd_segment(bo);
>
> r = ttm_bo_validate(&bo->tbo, &bo->placement, &tctx);
> }
