[PATCH 0/7] VCE1 fixes

[PATCH 0/7] VCE1 fixes

[Timur Kristóf]

Fix various small issues regarding VCE1
and the workaround to ensure the VCPU BO
has a low 32-bit address.

Timur Kristóf (7):
  drm/amdgpu: Add alignment to amdgpu_gtt_mgr_alloc_entries()
  drm/amdgpu/vce: Align VCPU BO to nearest power of two
  drm/amdgpu/vce1: Correct firmware offset mask
  drm/amdgpu/vce1: Remove superfluous address check
  drm/amdgpu/vce1: Check if VRAM address is lower than GART.
  drm/amdgpu/vce1: Don't repeat GTT MGR node allocation
  drm/amdgpu/vce1: Align VCPU BO GART address to nearest power of two

 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c |  5 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c     |  3 +-
 drivers/gpu/drm/amd/amdgpu/vce_v1_0.c       | 32 ++++++++++++++-------
 5 files changed, 28 insertions(+), 16 deletions(-)

-- 
2.53.0

[PATCH 1/7] drm/amdgpu: Add alignment to amdgpu_gtt_mgr_alloc_entries()

[Timur Kristóf]

Add an argument to amdgpu_gtt_mgr_alloc_entries() so that
the caller can specify an alignment.

This is a pre-requisite for fixing the workaround for
ensuring that the VCE1 VCPU BO has a low 32-bit address.

Fixes: 66a80158aa2a ("amdgpu/vce: use amdgpu_gtt_mgr_alloc_entries")
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 5 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     | 2 +-
 drivers/gpu/drm/amd/amdgpu/vce_v1_0.c       | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 9b0bcf6aca445..4fea81479264f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -188,6 +188,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
  * @mgr: The GTT manager object
  * @mm_node: The drm mm node to return the new allocation node information
  * @num_pages: The number of pages for the new allocation
+ * @alignment: Alignment of the allocation (in pages)
  * @mode: The new allocation mode
  *
  * Helper to dynamic alloc GART entries to map memory not accociated with
@@ -195,7 +196,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
  */
 int amdgpu_gtt_mgr_alloc_entries(struct amdgpu_gtt_mgr *mgr,
 				 struct drm_mm_node *mm_node,
-				 u64 num_pages,
+				 u64 num_pages, u64 alignment,
 				 enum drm_mm_insert_mode mode)
 {
 	struct amdgpu_device *adev = container_of(mgr, typeof(*adev), mman.gtt_mgr);
@@ -203,7 +204,7 @@ int amdgpu_gtt_mgr_alloc_entries(struct amdgpu_gtt_mgr *mgr,
 
 	spin_lock(&mgr->lock);
 	r = drm_mm_insert_node_in_range(&mgr->mm, mm_node, num_pages,
-					0, GART_ENTRY_WITHOUT_BO_COLOR, 0,
+					alignment, GART_ENTRY_WITHOUT_BO_COLOR, 0,
 					adev->gmc.gart_size >> PAGE_SHIFT,
 					mode);
 	spin_unlock(&mgr->lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 4c7d1917d9bb1..b7b6c4469937b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2026,7 +2026,7 @@ static int amdgpu_ttm_buffer_entity_init(struct amdgpu_gtt_mgr *mgr,
 		return 0;
 
 	num_pages = num_gart_windows * AMDGPU_GTT_MAX_TRANSFER_SIZE;
-	r = amdgpu_gtt_mgr_alloc_entries(mgr, &entity->gart_node, num_pages,
+	r = amdgpu_gtt_mgr_alloc_entries(mgr, &entity->gart_node, num_pages, 0,
 					 DRM_MM_INSERT_BEST);
 	if (r) {
 		drm_sched_entity_destroy(&entity->base);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 2d72fa2172745..09a524be64c3d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -148,7 +148,7 @@ void amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr);
 
 int amdgpu_gtt_mgr_alloc_entries(struct amdgpu_gtt_mgr *mgr,
 				 struct drm_mm_node *mm_node,
-				 u64 num_pages,
+				 u64 num_pages, u64 alignment,
 				 enum drm_mm_insert_mode mode);
 void amdgpu_gtt_mgr_free_entries(struct amdgpu_gtt_mgr *mgr,
 				 struct drm_mm_node *mm_node);
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
index 5b7b46d242c6d..2fe931366985a 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
@@ -539,7 +539,7 @@ static int vce_v1_0_ensure_vcpu_bo_32bit_addr(struct amdgpu_device *adev)
 	int r;
 
 	r = amdgpu_gtt_mgr_alloc_entries(&adev->mman.gtt_mgr,
-					 &adev->vce.gart_node, num_pages,
+					 &adev->vce.gart_node, num_pages, 0,
 					 DRM_MM_INSERT_LOW);
 	if (r)
 		return r;
-- 
2.53.0

Re: [PATCH 1/7] drm/amdgpu: Add alignment to amdgpu_gtt_mgr_alloc_entries()

[Christian König]

On 4/20/26 14:10, Timur Kristóf wrote:
> Add an argument to amdgpu_gtt_mgr_alloc_entries() so that
> the caller can specify an alignment.
> 
> This is a pre-requisite for fixing the workaround for
> ensuring that the VCE1 VCPU BO has a low 32-bit address.
> 
> Fixes: 66a80158aa2a ("amdgpu/vce: use amdgpu_gtt_mgr_alloc_entries")
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 5 +++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     | 2 +-
>  drivers/gpu/drm/amd/amdgpu/vce_v1_0.c       | 2 +-
>  4 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index 9b0bcf6aca445..4fea81479264f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -188,6 +188,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
>   * @mgr: The GTT manager object
>   * @mm_node: The drm mm node to return the new allocation node information
>   * @num_pages: The number of pages for the new allocation
> + * @alignment: Alignment of the allocation (in pages)
>   * @mode: The new allocation mode
>   *
>   * Helper to dynamic alloc GART entries to map memory not accociated with
> @@ -195,7 +196,7 @@ static void amdgpu_gtt_mgr_del(struct ttm_resource_manager *man,
>   */
>  int amdgpu_gtt_mgr_alloc_entries(struct amdgpu_gtt_mgr *mgr,
>  				 struct drm_mm_node *mm_node,
> -				 u64 num_pages,
> +				 u64 num_pages, u64 alignment,
>  				 enum drm_mm_insert_mode mode)
>  {
>  	struct amdgpu_device *adev = container_of(mgr, typeof(*adev), mman.gtt_mgr);
> @@ -203,7 +204,7 @@ int amdgpu_gtt_mgr_alloc_entries(struct amdgpu_gtt_mgr *mgr,
>  
>  	spin_lock(&mgr->lock);
>  	r = drm_mm_insert_node_in_range(&mgr->mm, mm_node, num_pages,
> -					0, GART_ENTRY_WITHOUT_BO_COLOR, 0,
> +					alignment, GART_ENTRY_WITHOUT_BO_COLOR, 0,
>  					adev->gmc.gart_size >> PAGE_SHIFT,
>  					mode);
>  	spin_unlock(&mgr->lock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 4c7d1917d9bb1..b7b6c4469937b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -2026,7 +2026,7 @@ static int amdgpu_ttm_buffer_entity_init(struct amdgpu_gtt_mgr *mgr,
>  		return 0;
>  
>  	num_pages = num_gart_windows * AMDGPU_GTT_MAX_TRANSFER_SIZE;
> -	r = amdgpu_gtt_mgr_alloc_entries(mgr, &entity->gart_node, num_pages,
> +	r = amdgpu_gtt_mgr_alloc_entries(mgr, &entity->gart_node, num_pages, 0,
>  					 DRM_MM_INSERT_BEST);
>  	if (r) {
>  		drm_sched_entity_destroy(&entity->base);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index 2d72fa2172745..09a524be64c3d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -148,7 +148,7 @@ void amdgpu_gtt_mgr_recover(struct amdgpu_gtt_mgr *mgr);
>  
>  int amdgpu_gtt_mgr_alloc_entries(struct amdgpu_gtt_mgr *mgr,
>  				 struct drm_mm_node *mm_node,
> -				 u64 num_pages,
> +				 u64 num_pages, u64 alignment,
>  				 enum drm_mm_insert_mode mode);
>  void amdgpu_gtt_mgr_free_entries(struct amdgpu_gtt_mgr *mgr,
>  				 struct drm_mm_node *mm_node);
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> index 5b7b46d242c6d..2fe931366985a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> @@ -539,7 +539,7 @@ static int vce_v1_0_ensure_vcpu_bo_32bit_addr(struct amdgpu_device *adev)
>  	int r;
>  
>  	r = amdgpu_gtt_mgr_alloc_entries(&adev->mman.gtt_mgr,
> -					 &adev->vce.gart_node, num_pages,
> +					 &adev->vce.gart_node, num_pages, 0,
>  					 DRM_MM_INSERT_LOW);
>  	if (r)
>  		return r;

[PATCH 2/7] drm/amdgpu/vce: Align VCPU BO to nearest power of two

[Timur Kristóf]

VCE accesses memory, including its firmware, through a BAR.
It works slightly differently on each generation.
In case of VCE1, the start address of this BAR is zero
and we can't change it due to the firmware validation mechanism.

Align the VCE VCPU BO to the nearest power of two after the
byte size of the BO in order to prevent it from crossing the
boundaries of its BAR.

Fixes: d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index efdebd9c0a1f3..0658f13f0bdf7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -218,7 +218,8 @@ int amdgpu_vce_sw_init(struct amdgpu_device *adev, unsigned long size)
 	if (!adev->vce.fw)
 		return -ENOENT;
 
-	r = amdgpu_bo_create_kernel(adev, size, PAGE_SIZE,
+	r = amdgpu_bo_create_kernel(adev, size,
+				    roundup_pow_of_two(size),
 				    AMDGPU_GEM_DOMAIN_VRAM |
 				    AMDGPU_GEM_DOMAIN_GTT,
 				    &adev->vce.vcpu_bo,
-- 
2.53.0

Re: [PATCH 2/7] drm/amdgpu/vce: Align VCPU BO to nearest power of two

[Christian König]

On 4/20/26 14:10, Timur Kristóf wrote:
> VCE accesses memory, including its firmware, through a BAR.
> It works slightly differently on each generation.
> In case of VCE1, the start address of this BAR is zero
> and we can't change it due to the firmware validation mechanism.
> 
> Align the VCE VCPU BO to the nearest power of two after the
> byte size of the BO in order to prevent it from crossing the
> boundaries of its BAR.
> 
> Fixes: d38ceaf99ed0 ("drm/amdgpu: add core driver (v4)")
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index efdebd9c0a1f3..0658f13f0bdf7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -218,7 +218,8 @@ int amdgpu_vce_sw_init(struct amdgpu_device *adev, unsigned long size)
>  	if (!adev->vce.fw)
>  		return -ENOENT;
>  
> -	r = amdgpu_bo_create_kernel(adev, size, PAGE_SIZE,
> +	r = amdgpu_bo_create_kernel(adev, size,
> +				    roundup_pow_of_two(size),
>  				    AMDGPU_GEM_DOMAIN_VRAM |
>  				    AMDGPU_GEM_DOMAIN_GTT,

I don't think that this is a good idea. The VRAM BO should never cross a BAR boundary.

What could be is that the GTT BO does that, but approach that is only available on VCE4 anyway.

I think we should rather restrict falling back to GTT to VCE4 only.

Regards,
Christian.

>  				    &adev->vce.vcpu_bo,

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

[Timur Kristóf]

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);
 	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;
-- 
2.53.0

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

[Christian König]

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;

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

[Timur Kristóf]

On Monday, April 20, 2026 2:23:18 PM Central European Summer Time Christian 
König wrote:
> 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.

Hi Christian,

Thanks, I agree and I'll add the necessary changes.
That being said, with some further testing it seems that VCE fails to 
initialize when the gpu_addr is 128 MiB or more, which makes me think that we 
should use 0x07ffffff instead of 0x0fffffff. Does that sound reasonable to you?

Thanks,
Timur


> 
> >  	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;

[PATCH 4/7] drm/amdgpu/vce1: Remove superfluous address check

[Timur Kristóf]

The same thing is already checked a few lines above.

Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/vce_v1_0.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
index ce993b57b0e9f..35caef5a8102e 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
@@ -553,8 +553,6 @@ static int vce_v1_0_ensure_vcpu_bo_32bit_addr(struct amdgpu_device *adev)
 	amdgpu_gart_map_vram_range(adev, pa, adev->vce.gart_node.start,
 				   num_pages, flags, adev->gart.ptr);
 	adev->vce.gpu_addr = adev->gmc.gart_start + vce_gart_start_offs;
-	if (adev->vce.gpu_addr > max_vcpu_bo_addr)
-		return -EINVAL;
 
 	return 0;
 }
-- 
2.53.0

Re: [PATCH 4/7] drm/amdgpu/vce1: Remove superfluous address check

[Christian König]

On 4/20/26 14:10, Timur Kristóf wrote:
> The same thing is already checked a few lines above.
> 
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/vce_v1_0.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> index ce993b57b0e9f..35caef5a8102e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> @@ -553,8 +553,6 @@ static int vce_v1_0_ensure_vcpu_bo_32bit_addr(struct amdgpu_device *adev)
>  	amdgpu_gart_map_vram_range(adev, pa, adev->vce.gart_node.start,
>  				   num_pages, flags, adev->gart.ptr);
>  	adev->vce.gpu_addr = adev->gmc.gart_start + vce_gart_start_offs;
> -	if (adev->vce.gpu_addr > max_vcpu_bo_addr)
> -		return -EINVAL;
>  
>  	return 0;
>  }

[PATCH 5/7] drm/amdgpu/vce1: Check if VRAM address is lower than GART.

[Timur Kristóf]

Previously, I had assumed this was not possible
so it was OK to not handle it, but now we got a report
from a user who has a board that is configured this way.

When the VCPU BO is already located in a low 32-bit address
in VRAM (eg. when VRAM is mapped to the low address space),
don't do the workaround.

Fixes: 66a80158aa2a ("amdgpu/vce: use amdgpu_gtt_mgr_alloc_entries")
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/vce_v1_0.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
index 35caef5a8102e..b7b6096c1a1fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
@@ -538,6 +538,9 @@ static int vce_v1_0_ensure_vcpu_bo_32bit_addr(struct amdgpu_device *adev)
 	u64 vce_gart_start_offs;
 	int r;
 
+	if (adev->gmc.vram_start < adev->gmc.gart_start)
+		return amdgpu_bo_gpu_offset(adev->vce.vcpu_bo) <= max_vcpu_bo_addr ? 0 : -EINVAL;
+
 	r = amdgpu_gtt_mgr_alloc_entries(&adev->mman.gtt_mgr,
 					 &adev->vce.gart_node, num_pages, 0,
 					 DRM_MM_INSERT_LOW);
-- 
2.53.0

Re: [PATCH 5/7] drm/amdgpu/vce1: Check if VRAM address is lower than GART.

[Christian König]

On 4/20/26 14:10, Timur Kristóf wrote:
> Previously, I had assumed this was not possible
> so it was OK to not handle it, but now we got a report
> from a user who has a board that is configured this way.
> 
> When the VCPU BO is already located in a low 32-bit address
> in VRAM (eg. when VRAM is mapped to the low address space),
> don't do the workaround.
> 
> Fixes: 66a80158aa2a ("amdgpu/vce: use amdgpu_gtt_mgr_alloc_entries")
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/vce_v1_0.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> index 35caef5a8102e..b7b6096c1a1fd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> @@ -538,6 +538,9 @@ static int vce_v1_0_ensure_vcpu_bo_32bit_addr(struct amdgpu_device *adev)
>  	u64 vce_gart_start_offs;
>  	int r;
>  
> +	if (adev->gmc.vram_start < adev->gmc.gart_start)
> +		return amdgpu_bo_gpu_offset(adev->vce.vcpu_bo) <= max_vcpu_bo_addr ? 0 : -EINVAL;
> +
>  	r = amdgpu_gtt_mgr_alloc_entries(&adev->mman.gtt_mgr,
>  					 &adev->vce.gart_node, num_pages, 0,
>  					 DRM_MM_INSERT_LOW);

[PATCH 6/7] drm/amdgpu/vce1: Don't repeat GTT MGR node allocation

[Timur Kristóf]

Only allocate entries from the GTT manager when the
VCE GTT node is not allocated yet. This prevents the
possibility of allocating them multiple times, which
causes issues during GPU reset and suspend/resume.

Fixes: 66a80158aa2a ("amdgpu/vce: use amdgpu_gtt_mgr_alloc_entries")
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/vce_v1_0.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
index b7b6096c1a1fd..100aa48204c77 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
@@ -541,11 +541,13 @@ static int vce_v1_0_ensure_vcpu_bo_32bit_addr(struct amdgpu_device *adev)
 	if (adev->gmc.vram_start < adev->gmc.gart_start)
 		return amdgpu_bo_gpu_offset(adev->vce.vcpu_bo) <= max_vcpu_bo_addr ? 0 : -EINVAL;
 
-	r = amdgpu_gtt_mgr_alloc_entries(&adev->mman.gtt_mgr,
-					 &adev->vce.gart_node, num_pages, 0,
-					 DRM_MM_INSERT_LOW);
-	if (r)
-		return r;
+	if (!drm_mm_node_allocated(&adev->vce.gart_node)) {
+		r = amdgpu_gtt_mgr_alloc_entries(&adev->mman.gtt_mgr,
+						 &adev->vce.gart_node, num_pages, 0,
+						 DRM_MM_INSERT_LOW);
+		if (r)
+			return r;
+	}
 
 	vce_gart_start_offs = amdgpu_gtt_node_to_byte_offset(&adev->vce.gart_node);
 
-- 
2.53.0

Re: [PATCH 6/7] drm/amdgpu/vce1: Don't repeat GTT MGR node allocation

[Christian König]

On 4/20/26 14:10, Timur Kristóf wrote:
> Only allocate entries from the GTT manager when the
> VCE GTT node is not allocated yet. This prevents the
> possibility of allocating them multiple times, which
> causes issues during GPU reset and suspend/resume.
> 
> Fixes: 66a80158aa2a ("amdgpu/vce: use amdgpu_gtt_mgr_alloc_entries")
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/vce_v1_0.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> index b7b6096c1a1fd..100aa48204c77 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> @@ -541,11 +541,13 @@ static int vce_v1_0_ensure_vcpu_bo_32bit_addr(struct amdgpu_device *adev)
>  	if (adev->gmc.vram_start < adev->gmc.gart_start)
>  		return amdgpu_bo_gpu_offset(adev->vce.vcpu_bo) <= max_vcpu_bo_addr ? 0 : -EINVAL;
>  
> -	r = amdgpu_gtt_mgr_alloc_entries(&adev->mman.gtt_mgr,
> -					 &adev->vce.gart_node, num_pages, 0,
> -					 DRM_MM_INSERT_LOW);
> -	if (r)
> -		return r;
> +	if (!drm_mm_node_allocated(&adev->vce.gart_node)) {
> +		r = amdgpu_gtt_mgr_alloc_entries(&adev->mman.gtt_mgr,
> +						 &adev->vce.gart_node, num_pages, 0,
> +						 DRM_MM_INSERT_LOW);
> +		if (r)
> +			return r;
> +	}
>  
>  	vce_gart_start_offs = amdgpu_gtt_node_to_byte_offset(&adev->vce.gart_node);
>  

[PATCH 7/7] drm/amdgpu/vce1: Align VCPU BO GART address to nearest power of two

[Timur Kristóf]

VCE accesses memory, including its firmware, through a BAR.
It works slightly differently on each generation.
In case of VCE1, the start address of this BAR is zero
and we can't change it due to the firmware validation mechanism.

Align the GART address of the VCPU BO like the VRAM address,
in order to prevent it from crossing the boundaries of its BAR.

This fixes VCE1 initialization failure after suspend/resume.

Fixes: 66a80158aa2a ("amdgpu/vce: use amdgpu_gtt_mgr_alloc_entries")
Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/vce_v1_0.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
index 100aa48204c77..9ddd635449873 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
@@ -527,11 +527,17 @@ static int vce_v1_0_early_init(struct amdgpu_ip_block *ip_block)
  * To accomodate that, we put GART to the LOW address range
  * and reserve some GART pages where we map the VCPU BO,
  * so that it gets a 32-bit address.
+ *
+ * VCE accesses memory, including its firmware, through a BAR.
+ * It works slightly differently on each generation.
+ * In case of VCE1, the start address of this BAR is zero
+ * and we can't change it due to the firmware validation mechanism.
  */
 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 = 0x0fffffff - bo_size;
+	u64 aligned_size = roundup_pow_of_two(bo_size);
+	u64 max_vcpu_bo_addr = 0x0fffffff - aligned_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;
@@ -543,7 +549,8 @@ static int vce_v1_0_ensure_vcpu_bo_32bit_addr(struct amdgpu_device *adev)
 
 	if (!drm_mm_node_allocated(&adev->vce.gart_node)) {
 		r = amdgpu_gtt_mgr_alloc_entries(&adev->mman.gtt_mgr,
-						 &adev->vce.gart_node, num_pages, 0,
+						 &adev->vce.gart_node, num_pages,
+						 aligned_size >> AMDGPU_GPU_PAGE_SHIFT,
 						 DRM_MM_INSERT_LOW);
 		if (r)
 			return r;
-- 
2.53.0

Re: [PATCH 7/7] drm/amdgpu/vce1: Align VCPU BO GART address to nearest power of two

[Christian König]

On 4/20/26 14:10, Timur Kristóf wrote:
> VCE accesses memory, including its firmware, through a BAR.
> It works slightly differently on each generation.
> In case of VCE1, the start address of this BAR is zero
> and we can't change it due to the firmware validation mechanism.
> 
> Align the GART address of the VCPU BO like the VRAM address,
> in order to prevent it from crossing the boundaries of its BAR.
> 
> This fixes VCE1 initialization failure after suspend/resume.
> 
> Fixes: 66a80158aa2a ("amdgpu/vce: use amdgpu_gtt_mgr_alloc_entries")
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/vce_v1_0.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> index 100aa48204c77..9ddd635449873 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
> @@ -527,11 +527,17 @@ static int vce_v1_0_early_init(struct amdgpu_ip_block *ip_block)
>   * To accomodate that, we put GART to the LOW address range
>   * and reserve some GART pages where we map the VCPU BO,
>   * so that it gets a 32-bit address.
> + *
> + * VCE accesses memory, including its firmware, through a BAR.
> + * It works slightly differently on each generation.
> + * In case of VCE1, the start address of this BAR is zero
> + * and we can't change it due to the firmware validation mechanism.
>   */
>  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 = 0x0fffffff - bo_size;
> +	u64 aligned_size = roundup_pow_of_two(bo_size);
> +	u64 max_vcpu_bo_addr = 0x0fffffff - aligned_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;
> @@ -543,7 +549,8 @@ static int vce_v1_0_ensure_vcpu_bo_32bit_addr(struct amdgpu_device *adev)
>  
>  	if (!drm_mm_node_allocated(&adev->vce.gart_node)) {
>  		r = amdgpu_gtt_mgr_alloc_entries(&adev->mman.gtt_mgr,
> -						 &adev->vce.gart_node, num_pages, 0,
> +						 &adev->vce.gart_node, num_pages,
> +						 aligned_size >> AMDGPU_GPU_PAGE_SHIFT,
>  						 DRM_MM_INSERT_LOW);
>  		if (r)
>  			return r;