Re: [PATCH 3/7] drm/amdgpu/vce1: Correct firmware offset mask

["Christian König" <christian.koenig@amd.com>]

On 4/20/26 14:10, Timur Kristóf wrote:
> It's 0x0fffffff and not 0x7fffffff.
> 
> Fixes: d4a640d4b9f3 ("drm/amdgpu/vce1: Implement VCE1 IP block (v2)")
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/vce_v1_0.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> index 2fe931366985a..ce993b57b0e9f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> @@ -313,17 +313,17 @@ static int vce_v1_0_mc_resume(struct amdgpu_device *adev)
>  
>  	offset =  adev->vce.gpu_addr + AMDGPU_VCE_FIRMWARE_OFFSET;
>  	size = VCE_V1_0_FW_SIZE;
> -	WREG32(mmVCE_VCPU_CACHE_OFFSET0, offset & 0x7fffffff);
> +	WREG32(mmVCE_VCPU_CACHE_OFFSET0, offset & 0x0fffffff);
>  	WREG32(mmVCE_VCPU_CACHE_SIZE0, size);
>  
>  	offset += size;
>  	size = VCE_V1_0_STACK_SIZE;
> -	WREG32(mmVCE_VCPU_CACHE_OFFSET1, offset & 0x7fffffff);
> +	WREG32(mmVCE_VCPU_CACHE_OFFSET1, offset & 0x0fffffff);
>  	WREG32(mmVCE_VCPU_CACHE_SIZE1, size);
>  
>  	offset += size;
>  	size = VCE_V1_0_DATA_SIZE;
> -	WREG32(mmVCE_VCPU_CACHE_OFFSET2, offset & 0x7fffffff);
> +	WREG32(mmVCE_VCPU_CACHE_OFFSET2, offset & 0x0fffffff);

We actually have that as VCE_VCPU_CACHE_OFFSET*__OFFSET_MASK in the headers, would probably be a good idea to use that one instead.

Additional to that limiting the value actually doesn't make much sense, that just hides the problem when we really get an offset which is to large.

We should probably rather have a WARN_ON(offset & ~VCE_VCPU_CACHE_OFFSET2__OFFSET_MASK) directly above the register write.

Apart from that the patch looks good to me.

Regards,
Christian.

>  	WREG32(mmVCE_VCPU_CACHE_SIZE2, size);
>  
>  	WREG32_P(mmVCE_LMI_CTRL2, 0x0, ~0x100);
> @@ -531,7 +531,7 @@ static int vce_v1_0_early_init(struct amdgpu_ip_block *ip_block)
>  static int vce_v1_0_ensure_vcpu_bo_32bit_addr(struct amdgpu_device *adev)
>  {
>  	u64 bo_size = amdgpu_bo_size(adev->vce.vcpu_bo);
> -	u64 max_vcpu_bo_addr = 0xffffffff - bo_size;
> +	u64 max_vcpu_bo_addr = 0x0fffffff - bo_size;
>  	u64 num_pages = ALIGN(bo_size, AMDGPU_GPU_PAGE_SIZE) / AMDGPU_GPU_PAGE_SIZE;
>  	u64 pa = amdgpu_gmc_vram_pa(adev, adev->vce.vcpu_bo);
>  	u64 flags = AMDGPU_PTE_READABLE | AMDGPU_PTE_WRITEABLE | AMDGPU_PTE_VALID;