AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV
@ 2025-10-09  3:30 Ellen Pan
  2025-10-09 12:26 ` Christian König
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ellen Pan @ 2025-10-09  3:30 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alexander.Deucher, Christian.Koenig, Shravankumar.Gande,
	Ellen Pan

- During guest driver init, asa VFs receive PF msg to
	init dynamic critical region(v2), VFs reuse fw_vram_usage_*
	 from ttm to store critical region tables in a 5MB chunk.

Signed-off-by: Ellen Pan <yunru.pan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 33 +++++++++++++-----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c |  8 ++++++
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 96bd0185f936..4ba34ba74671 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1943,23 +1943,26 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 	if (r)
 		return r;
 
-	/*
-	 *The reserved vram for driver must be pinned to the specified
-	 *place on the VRAM, so reserve it early.
-	 */
-	r = amdgpu_ttm_drv_reserve_vram_init(adev);
-	if (r)
-		return r;
-
-	/*
-	 * only NAVI10 and onwards ASIC support for IP discovery.
-	 * If IP discovery enabled, a block of memory should be
-	 * reserved for IP discovey.
-	 */
-	if (adev->mman.discovery_bin) {
-		r = amdgpu_ttm_reserve_tmr(adev);
+	/* VFs using dynamic critical regions(v2) won't need to reserve for below memory */
+	if (!amdgpu_sriov_vf(adev) || (adev->virt.req_init_data_ver != GPU_CRIT_REGION_V2)) {
+		/*
+		 *The reserved vram for driver must be pinned to the specified
+		 *place on the VRAM, so reserve it early.
+		 */
+		r = amdgpu_ttm_drv_reserve_vram_init(adev);
 		if (r)
 			return r;
+
+		/*
+		 * only NAVI10 and onwards ASIC support for IP discovery.
+		 * If IP discovery enabled, a block of memory should be
+		 * reserved for IP discovey.
+		 */
+		if (adev->mman.discovery_bin) {
+			r = amdgpu_ttm_reserve_tmr(adev);
+			if (r)
+				return r;
+		}
 	}
 
 	/* allocate memory as required for VGA
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 46c19e96086a..e9dbab53cb06 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -931,6 +931,14 @@ int amdgpu_virt_init_critical_region(struct amdgpu_device *adev)
 		adev->virt.crit_region_sizes_kb[AMD_SRIOV_MSG_BAD_PAGE_INFO_TABLE_ID] =
 			init_data_hdr->bad_page_size_in_kb;
 
+		/* reserved memory starts from crit region base offset with the size of 5MB */
+		adev->mman.fw_vram_usage_start_offset = adev->virt.crit_region_base_offset;
+		adev->mman.fw_vram_usage_size = adev->virt.crit_region_size_in_kb << 10;
+		DRM_INFO("critical region v%d requested to reserve memory start at %08x with %d KB.\n",
+			  init_data_hdr->version,
+			  adev->mman.fw_vram_usage_start_offset,
+			  adev->mman.fw_vram_usage_size >> 10);
+
 		adev->virt.init_data_done = true;
 		break;
 	default:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV
  2025-10-09  3:30 [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV Ellen Pan
@ 2025-10-09 12:26 ` Christian König
  2025-10-09 12:52 ` Alex Deucher
  2025-10-09 13:59 ` Lazar, Lijo
  2 siblings, 0 replies; 7+ messages in thread
From: Christian König @ 2025-10-09 12:26 UTC (permalink / raw)
  To: Ellen Pan, amd-gfx; +Cc: Alexander.Deucher, Shravankumar.Gande

On 09.10.25 05:30, Ellen Pan wrote:
> - During guest driver init, asa VFs receive PF msg to
> 	init dynamic critical region(v2), VFs reuse fw_vram_usage_*
> 	 from ttm to store critical region tables in a 5MB chunk.
> 
> Signed-off-by: Ellen Pan <yunru.pan@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 33 +++++++++++++-----------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c |  8 ++++++
>  2 files changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 96bd0185f936..4ba34ba74671 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1943,23 +1943,26 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>  	if (r)
>  		return r;
>  
> -	/*
> -	 *The reserved vram for driver must be pinned to the specified
> -	 *place on the VRAM, so reserve it early.
> -	 */
> -	r = amdgpu_ttm_drv_reserve_vram_init(adev);
> -	if (r)
> -		return r;
> -
> -	/*
> -	 * only NAVI10 and onwards ASIC support for IP discovery.
> -	 * If IP discovery enabled, a block of memory should be
> -	 * reserved for IP discovey.
> -	 */
> -	if (adev->mman.discovery_bin) {
> -		r = amdgpu_ttm_reserve_tmr(adev);
> +	/* VFs using dynamic critical regions(v2) won't need to reserve for below memory */
> +	if (!amdgpu_sriov_vf(adev) || (adev->virt.req_init_data_ver != GPU_CRIT_REGION_V2)) {

That comment says what is done, but not why it is done.

> +		/*
> +		 *The reserved vram for driver must be pinned to the specified
> +		 *place on the VRAM, so reserve it early.
> +		 */
> +		r = amdgpu_ttm_drv_reserve_vram_init(adev);
>  		if (r)
>  			return r;
> +
> +		/*
> +		 * only NAVI10 and onwards ASIC support for IP discovery.
> +		 * If IP discovery enabled, a block of memory should be
> +		 * reserved for IP discovey.

While at it there are a couple of typos in the comment, please fix.

> +		 */
> +		if (adev->mman.discovery_bin) {
> +			r = amdgpu_ttm_reserve_tmr(adev);
> +			if (r)
> +				return r;
> +		}
>  	}
>  
>  	/* allocate memory as required for VGA
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 46c19e96086a..e9dbab53cb06 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -931,6 +931,14 @@ int amdgpu_virt_init_critical_region(struct amdgpu_device *adev)
>  		adev->virt.crit_region_sizes_kb[AMD_SRIOV_MSG_BAD_PAGE_INFO_TABLE_ID] =
>  			init_data_hdr->bad_page_size_in_kb;
>  
> +		/* reserved memory starts from crit region base offset with the size of 5MB */
> +		adev->mman.fw_vram_usage_start_offset = adev->virt.crit_region_base_offset;
> +		adev->mman.fw_vram_usage_size = adev->virt.crit_region_size_in_kb << 10;
> +		DRM_INFO("critical region v%d requested to reserve memory start at %08x with %d KB.\n",
> +			  init_data_hdr->version,
> +			  adev->mman.fw_vram_usage_start_offset,
> +			  adev->mman.fw_vram_usage_size >> 10);
> +

Same as on the other patch, using dev_info might be more adequate than DRM_INFO.

Regards,
Christian.

>  		adev->virt.init_data_done = true;
>  		break;
>  	default:


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV
  2025-10-09  3:30 [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV Ellen Pan
  2025-10-09 12:26 ` Christian König
@ 2025-10-09 12:52 ` Alex Deucher
  2025-10-09 13:59 ` Lazar, Lijo
  2 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2025-10-09 12:52 UTC (permalink / raw)
  To: Ellen Pan
  Cc: amd-gfx, Alexander.Deucher, Christian.Koenig, Shravankumar.Gande

On Wed, Oct 8, 2025 at 11:46 PM Ellen Pan <yunru.pan@amd.com> wrote:
>
> - During guest driver init, asa VFs receive PF msg to
>         init dynamic critical region(v2), VFs reuse fw_vram_usage_*
>          from ttm to store critical region tables in a 5MB chunk.
>
> Signed-off-by: Ellen Pan <yunru.pan@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 33 +++++++++++++-----------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c |  8 ++++++
>  2 files changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 96bd0185f936..4ba34ba74671 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1943,23 +1943,26 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>         if (r)
>                 return r;
>
> -       /*
> -        *The reserved vram for driver must be pinned to the specified
> -        *place on the VRAM, so reserve it early.
> -        */
> -       r = amdgpu_ttm_drv_reserve_vram_init(adev);
> -       if (r)
> -               return r;
> -
> -       /*
> -        * only NAVI10 and onwards ASIC support for IP discovery.
> -        * If IP discovery enabled, a block of memory should be
> -        * reserved for IP discovey.
> -        */
> -       if (adev->mman.discovery_bin) {
> -               r = amdgpu_ttm_reserve_tmr(adev);
> +       /* VFs using dynamic critical regions(v2) won't need to reserve for below memory */
> +       if (!amdgpu_sriov_vf(adev) || (adev->virt.req_init_data_ver != GPU_CRIT_REGION_V2)) {
> +               /*
> +                *The reserved vram for driver must be pinned to the specified
> +                *place on the VRAM, so reserve it early.
> +                */
> +               r = amdgpu_ttm_drv_reserve_vram_init(adev);
>                 if (r)
>                         return r;
> +
> +               /*
> +                * only NAVI10 and onwards ASIC support for IP discovery.
> +                * If IP discovery enabled, a block of memory should be
> +                * reserved for IP discovey.
> +                */
> +               if (adev->mman.discovery_bin) {
> +                       r = amdgpu_ttm_reserve_tmr(adev);
> +                       if (r)
> +                               return r;
> +               }
>         }
>
>         /* allocate memory as required for VGA
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 46c19e96086a..e9dbab53cb06 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -931,6 +931,14 @@ int amdgpu_virt_init_critical_region(struct amdgpu_device *adev)
>                 adev->virt.crit_region_sizes_kb[AMD_SRIOV_MSG_BAD_PAGE_INFO_TABLE_ID] =
>                         init_data_hdr->bad_page_size_in_kb;
>
> +               /* reserved memory starts from crit region base offset with the size of 5MB */
> +               adev->mman.fw_vram_usage_start_offset = adev->virt.crit_region_base_offset;
> +               adev->mman.fw_vram_usage_size = adev->virt.crit_region_size_in_kb << 10;
> +               DRM_INFO("critical region v%d requested to reserve memory start at %08x with %d KB.\n",

Please use dev_info() here so we have proper multi-gpu support.

Alex

> +                         init_data_hdr->version,
> +                         adev->mman.fw_vram_usage_start_offset,
> +                         adev->mman.fw_vram_usage_size >> 10);
> +
>                 adev->virt.init_data_done = true;
>                 break;
>         default:
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV
  2025-10-09  3:30 [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV Ellen Pan
  2025-10-09 12:26 ` Christian König
  2025-10-09 12:52 ` Alex Deucher
@ 2025-10-09 13:59 ` Lazar, Lijo
  2 siblings, 0 replies; 7+ messages in thread
From: Lazar, Lijo @ 2025-10-09 13:59 UTC (permalink / raw)
  To: Pan, Ellen, amd-gfx@lists.freedesktop.org
  Cc: Deucher, Alexander, Koenig, Christian, Gande, Shravan kumar,
	Pan, Ellen

[Public]

>-----Original Message-----
>From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Ellen
>Pan
>Sent: Thursday, October 9, 2025 9:01 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
><Christian.Koenig@amd.com>; Gande, Shravan kumar
><Shravankumar.Gande@amd.com>; Pan, Ellen <Yunru.Pan@amd.com>
>Subject: [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic
>critical region in SRIOV
>
>- During guest driver init, asa VFs receive PF msg to
>       init dynamic critical region(v2), VFs reuse fw_vram_usage_*
>        from ttm to store critical region tables in a 5MB chunk.
>
>Signed-off-by: Ellen Pan <yunru.pan@amd.com>
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  | 33 +++++++++++++----------
>-  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c |  8 ++++++
> 2 files changed, 26 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>index 96bd0185f936..4ba34ba74671 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>@@ -1943,23 +1943,26 @@ int amdgpu_ttm_init(struct amdgpu_device
>*adev)
>       if (r)
>               return r;
>
>-      /*
>-       *The reserved vram for driver must be pinned to the specified
>-       *place on the VRAM, so reserve it early.
>-       */
>-      r = amdgpu_ttm_drv_reserve_vram_init(adev);
>-      if (r)
>-              return r;
>-
>-      /*
>-       * only NAVI10 and onwards ASIC support for IP discovery.
>-       * If IP discovery enabled, a block of memory should be
>-       * reserved for IP discovey.
>-       */
>-      if (adev->mman.discovery_bin) {
>-              r = amdgpu_ttm_reserve_tmr(adev);
>+      /* VFs using dynamic critical regions(v2) won't need to reserve for
>below memory */
>+      if (!amdgpu_sriov_vf(adev) || (adev->virt.req_init_data_ver !=
>GPU_CRIT_REGION_V2)) {
>+              /*
>+               *The reserved vram for driver must be pinned to the specified
>+               *place on the VRAM, so reserve it early.
>+               */
>+              r = amdgpu_ttm_drv_reserve_vram_init(adev);
>               if (r)
>                       return r;
>+
>+              /*
>+               * only NAVI10 and onwards ASIC support for IP discovery.
>+               * If IP discovery enabled, a block of memory should be
>+               * reserved for IP discovey.
>+               */
>+              if (adev->mman.discovery_bin) {
>+                      r = amdgpu_ttm_reserve_tmr(adev);
>+                      if (r)
>+                              return r;
>+              }
>       }
>
>       /* allocate memory as required for VGA diff --git
>a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>index 46c19e96086a..e9dbab53cb06 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>@@ -931,6 +931,14 @@ int amdgpu_virt_init_critical_region(struct
>amdgpu_device *adev)
>               adev-
>>virt.crit_region_sizes_kb[AMD_SRIOV_MSG_BAD_PAGE_INFO_TABLE_ID] =
>                       init_data_hdr->bad_page_size_in_kb;
>
>+              /* reserved memory starts from crit region base offset with the
>size of 5MB */
>+              adev->mman.fw_vram_usage_start_offset = adev-
>>virt.crit_region_base_offset;
>+              adev->mman.fw_vram_usage_size = adev-
>>virt.crit_region_size_in_kb << 10;
>+              DRM_INFO("critical region v%d requested to reserve memory
>start at %08x with %d KB.\n",
>+                        init_data_hdr->version,
>+                        adev->mman.fw_vram_usage_start_offset,
>+                        adev->mman.fw_vram_usage_size >> 10);
>+

[lijo]
There is another place where this gets set - amdgpu_atomfirmware_allocate_fb_v2_2. Is the assumption that vbios table no longer will have the flags set?

Thanks,
Lijo

>               adev->virt.init_data_done = true;
>               break;
>       default:
>--
>2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV
  2025-10-10  4:43 [PATCH 1/6] drm/amdgpu: Updated naming of SRIOV critical region offsets/sizes with _V1 suffix Ellen Pan
@ 2025-10-10  4:43 ` Ellen Pan
  2025-10-10 10:33   ` Lazar, Lijo
  2025-10-10 15:10   ` Alex Deucher
  0 siblings, 2 replies; 7+ messages in thread
From: Ellen Pan @ 2025-10-10  4:43 UTC (permalink / raw)
  To: amd-gfx
  Cc: Alexander.Deucher, Christian.Koenig, Lijo.Lazar, Jeffrey.Chan,
	Ellen Pan

- During guest driver init, asa VFs receive PF msg to
	init dynamic critical region(v2), VFs reuse fw_vram_usage_*
	 from ttm to store critical region tables in a 5MB chunk.

Signed-off-by: Ellen Pan <yunru.pan@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c  | 29 ++++++++--------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c       | 33 +++++++++++--------
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c      |  9 +++++
 3 files changed, 44 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index c7d32fb216e4..636385c80f64 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -181,19 +181,22 @@ int amdgpu_atomfirmware_allocate_fb_scratch(struct amdgpu_device *adev)
 	u8 frev, crev;
 	int usage_bytes = 0;
 
-	if (amdgpu_atom_parse_data_header(ctx, index, NULL, &frev, &crev, &data_offset)) {
-		if (frev == 2 && crev == 1) {
-			fw_usage_v2_1 =
-				(struct vram_usagebyfirmware_v2_1 *)(ctx->bios + data_offset);
-			amdgpu_atomfirmware_allocate_fb_v2_1(adev,
-					fw_usage_v2_1,
-					&usage_bytes);
-		} else if (frev >= 2 && crev >= 2) {
-			fw_usage_v2_2 =
-				(struct vram_usagebyfirmware_v2_2 *)(ctx->bios + data_offset);
-			amdgpu_atomfirmware_allocate_fb_v2_2(adev,
-					fw_usage_v2_2,
-					&usage_bytes);
+	/* Skip atomfirmware allocation for SRIOV VFs when dynamic crit regn is enabled */
+	if (!(amdgpu_sriov_vf(adev) && adev->virt.is_dynamic_crit_regn_enabled)) {
+		if (amdgpu_atom_parse_data_header(ctx, index, NULL, &frev, &crev, &data_offset)) {
+			if (frev == 2 && crev == 1) {
+				fw_usage_v2_1 =
+					(struct vram_usagebyfirmware_v2_1 *)(ctx->bios + data_offset);
+				amdgpu_atomfirmware_allocate_fb_v2_1(adev,
+						fw_usage_v2_1,
+						&usage_bytes);
+			} else if (frev >= 2 && crev >= 2) {
+				fw_usage_v2_2 =
+					(struct vram_usagebyfirmware_v2_2 *)(ctx->bios + data_offset);
+				amdgpu_atomfirmware_allocate_fb_v2_2(adev,
+						fw_usage_v2_2,
+						&usage_bytes);
+			}
 		}
 	}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 96bd0185f936..c0810c53703c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1943,23 +1943,28 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 	if (r)
 		return r;
 
-	/*
-	 *The reserved vram for driver must be pinned to the specified
-	 *place on the VRAM, so reserve it early.
-	 */
-	r = amdgpu_ttm_drv_reserve_vram_init(adev);
-	if (r)
-		return r;
-
-	/*
-	 * only NAVI10 and onwards ASIC support for IP discovery.
-	 * If IP discovery enabled, a block of memory should be
-	 * reserved for IP discovey.
+	/* For VFs with dynamic critical regions (v2), fw_reserve_vram is already
+	 * reserved, so no additional memory reservation is required below.
 	 */
-	if (adev->mman.discovery_bin) {
-		r = amdgpu_ttm_reserve_tmr(adev);
+	if (!amdgpu_sriov_vf(adev) || (adev->virt.req_init_data_ver != GPU_CRIT_REGION_V2)) {
+		/*
+		 * The reserved VRAM for the driver must be pinned to a specific
+		 * location in VRAM, so reserve it early.
+		 */
+		r = amdgpu_ttm_drv_reserve_vram_init(adev);
 		if (r)
 			return r;
+
+		/*
+		 * only NAVI10 and later ASICs support IP discovery.
+		 * If IP discovery is enabled, a block of memory should be
+		 * reserved for it.
+		 */
+		if (adev->mman.discovery_bin) {
+			r = amdgpu_ttm_reserve_tmr(adev);
+			if (r)
+				return r;
+		}
 	}
 
 	/* allocate memory as required for VGA
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 6eca5e8a7375..461e83728594 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -940,6 +940,15 @@ int amdgpu_virt_init_critical_region(struct amdgpu_device *adev)
 		adev->virt.crit_regn_tbl[AMD_SRIOV_MSG_BAD_PAGE_INFO_TABLE_ID].size_kb =
 			init_data_hdr->bad_page_size_in_kb;
 
+		/* reserved memory starts from crit region base offset with the size of 5MB */
+		adev->mman.fw_vram_usage_start_offset = adev->virt.crit_regn.offset;
+		adev->mman.fw_vram_usage_size = adev->virt.crit_regn.size_kb << 10;
+		dev_info(adev->dev,
+			"critical region v%d requested to reserve memory start at %08x with %d KB.\n",
+			  init_data_hdr->version,
+			  adev->mman.fw_vram_usage_start_offset,
+			  adev->mman.fw_vram_usage_size >> 10);
+
 		adev->virt.is_dynamic_crit_regn_enabled = true;
 		break;
 	default:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* RE: [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV
  2025-10-10  4:43 ` [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV Ellen Pan
@ 2025-10-10 10:33   ` Lazar, Lijo
  2025-10-10 15:10   ` Alex Deucher
  1 sibling, 0 replies; 7+ messages in thread
From: Lazar, Lijo @ 2025-10-10 10:33 UTC (permalink / raw)
  To: Pan, Ellen, amd-gfx@lists.freedesktop.org
  Cc: Deucher, Alexander, Koenig, Christian, Chan, Hing Pong

[AMD Official Use Only - AMD Internal Distribution Only]

>-----Original Message-----
>From: Pan, Ellen <Yunru.Pan@amd.com>
>Sent: Friday, October 10, 2025 10:13 AM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
><Christian.Koenig@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Chan, Hing
>Pong <Jeffrey.Chan@amd.com>; Pan, Ellen <Yunru.Pan@amd.com>
>Subject: [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical
>region in SRIOV
>
>- During guest driver init, asa VFs receive PF msg to
>       init dynamic critical region(v2), VFs reuse fw_vram_usage_*
>        from ttm to store critical region tables in a 5MB chunk.
>
>Signed-off-by: Ellen Pan <yunru.pan@amd.com>
>---
> .../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c  | 29 ++++++++--------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c       | 33 +++++++++++--------
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c      |  9 +++++
> 3 files changed, 44 insertions(+), 27 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
>index c7d32fb216e4..636385c80f64 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
>@@ -181,19 +181,22 @@ int amdgpu_atomfirmware_allocate_fb_scratch(struct
>amdgpu_device *adev)
>       u8 frev, crev;
>       int usage_bytes = 0;
>
>-      if (amdgpu_atom_parse_data_header(ctx, index, NULL, &frev, &crev,
>&data_offset)) {
>-              if (frev == 2 && crev == 1) {
>-                      fw_usage_v2_1 =
>-                              (struct vram_usagebyfirmware_v2_1 *)(ctx->bios
>+ data_offset);
>-                      amdgpu_atomfirmware_allocate_fb_v2_1(adev,
>-                                      fw_usage_v2_1,
>-                                      &usage_bytes);
>-              } else if (frev >= 2 && crev >= 2) {
>-                      fw_usage_v2_2 =
>-                              (struct vram_usagebyfirmware_v2_2 *)(ctx->bios
>+ data_offset);
>-                      amdgpu_atomfirmware_allocate_fb_v2_2(adev,
>-                                      fw_usage_v2_2,
>-                                      &usage_bytes);
>+      /* Skip atomfirmware allocation for SRIOV VFs when dynamic crit regn is
>enabled */
>+      if (!(amdgpu_sriov_vf(adev) && adev-
>>virt.is_dynamic_crit_regn_enabled)) {
>+              if (amdgpu_atom_parse_data_header(ctx, index, NULL, &frev,
>&crev, &data_offset)) {
>+                      if (frev == 2 && crev == 1) {
>+                              fw_usage_v2_1 =
>+                                      (struct vram_usagebyfirmware_v2_1
>*)(ctx->bios + data_offset);
>+                              amdgpu_atomfirmware_allocate_fb_v2_1(adev,
>+                                              fw_usage_v2_1,
>+                                              &usage_bytes);
>+                      } else if (frev >= 2 && crev >= 2) {
>+                              fw_usage_v2_2 =
>+                                      (struct vram_usagebyfirmware_v2_2
>*)(ctx->bios + data_offset);
>+                              amdgpu_atomfirmware_allocate_fb_v2_2(adev,
>+                                              fw_usage_v2_2,
>+                                              &usage_bytes);
>+                      }
>               }
>       }
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>index 96bd0185f936..c0810c53703c 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>@@ -1943,23 +1943,28 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>       if (r)
>               return r;
>
>-      /*
>-       *The reserved vram for driver must be pinned to the specified
>-       *place on the VRAM, so reserve it early.
>-       */
>-      r = amdgpu_ttm_drv_reserve_vram_init(adev);
>-      if (r)
>-              return r;
>-
>-      /*
>-       * only NAVI10 and onwards ASIC support for IP discovery.
>-       * If IP discovery enabled, a block of memory should be
>-       * reserved for IP discovey.
>+      /* For VFs with dynamic critical regions (v2), fw_reserve_vram is already
>+       * reserved, so no additional memory reservation is required below.
>        */
>-      if (adev->mman.discovery_bin) {
>-              r = amdgpu_ttm_reserve_tmr(adev);
>+      if (!amdgpu_sriov_vf(adev) || (adev->virt.req_init_data_ver !=
>GPU_CRIT_REGION_V2)) {
>+              /*
>+               * The reserved VRAM for the driver must be pinned to a specific
>+               * location in VRAM, so reserve it early.
>+               */
>+              r = amdgpu_ttm_drv_reserve_vram_init(adev);
>               if (r)
>                       return r;
[lijo]

Since you are skipping FW usage tables, isn't drv_vram_usage_size expected to be 0 anyway? The discovery one below will need a separate check though for now.

Thanks,
Lijo

>+
>+              /*
>+               * only NAVI10 and later ASICs support IP discovery.
>+               * If IP discovery is enabled, a block of memory should be
>+               * reserved for it.
>+               */
>+              if (adev->mman.discovery_bin) {
>+                      r = amdgpu_ttm_reserve_tmr(adev);
>+                      if (r)
>+                              return r;
>+              }
>       }
>
>       /* allocate memory as required for VGA diff --git
>a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>index 6eca5e8a7375..461e83728594 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>@@ -940,6 +940,15 @@ int amdgpu_virt_init_critical_region(struct
>amdgpu_device *adev)
>               adev-
>>virt.crit_regn_tbl[AMD_SRIOV_MSG_BAD_PAGE_INFO_TABLE_ID].size_kb =
>                       init_data_hdr->bad_page_size_in_kb;
>
>+              /* reserved memory starts from crit region base offset with the
>size of 5MB */
>+              adev->mman.fw_vram_usage_start_offset = adev-
>>virt.crit_regn.offset;
>+              adev->mman.fw_vram_usage_size = adev->virt.crit_regn.size_kb
><< 10;
>+              dev_info(adev->dev,
>+                      "critical region v%d requested to reserve memory start at
>%08x with %d KB.\n",
>+                        init_data_hdr->version,
>+                        adev->mman.fw_vram_usage_start_offset,
>+                        adev->mman.fw_vram_usage_size >> 10);
>+
>               adev->virt.is_dynamic_crit_regn_enabled = true;
>               break;
>       default:
>--
>2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV
  2025-10-10  4:43 ` [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV Ellen Pan
  2025-10-10 10:33   ` Lazar, Lijo
@ 2025-10-10 15:10   ` Alex Deucher
  1 sibling, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2025-10-10 15:10 UTC (permalink / raw)
  To: Ellen Pan
  Cc: amd-gfx, Alexander.Deucher, Christian.Koenig, Lijo.Lazar,
	Jeffrey.Chan

On Fri, Oct 10, 2025 at 1:21 AM Ellen Pan <yunru.pan@amd.com> wrote:
>
> - During guest driver init, asa VFs receive PF msg to
>         init dynamic critical region(v2), VFs reuse fw_vram_usage_*
>          from ttm to store critical region tables in a 5MB chunk.
>
> Signed-off-by: Ellen Pan <yunru.pan@amd.com>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c  | 29 ++++++++--------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c       | 33 +++++++++++--------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c      |  9 +++++
>  3 files changed, 44 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> index c7d32fb216e4..636385c80f64 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> @@ -181,19 +181,22 @@ int amdgpu_atomfirmware_allocate_fb_scratch(struct amdgpu_device *adev)
>         u8 frev, crev;
>         int usage_bytes = 0;
>
> -       if (amdgpu_atom_parse_data_header(ctx, index, NULL, &frev, &crev, &data_offset)) {
> -               if (frev == 2 && crev == 1) {
> -                       fw_usage_v2_1 =
> -                               (struct vram_usagebyfirmware_v2_1 *)(ctx->bios + data_offset);
> -                       amdgpu_atomfirmware_allocate_fb_v2_1(adev,
> -                                       fw_usage_v2_1,
> -                                       &usage_bytes);
> -               } else if (frev >= 2 && crev >= 2) {
> -                       fw_usage_v2_2 =
> -                               (struct vram_usagebyfirmware_v2_2 *)(ctx->bios + data_offset);
> -                       amdgpu_atomfirmware_allocate_fb_v2_2(adev,
> -                                       fw_usage_v2_2,
> -                                       &usage_bytes);
> +       /* Skip atomfirmware allocation for SRIOV VFs when dynamic crit regn is enabled */
> +       if (!(amdgpu_sriov_vf(adev) && adev->virt.is_dynamic_crit_regn_enabled)) {
> +               if (amdgpu_atom_parse_data_header(ctx, index, NULL, &frev, &crev, &data_offset)) {

Do these tables actually exist on a V2 setup?  If not or if the
version numbers are invalid, the current code should handle this
already and there is no need for an extra check here.

Alex

> +                       if (frev == 2 && crev == 1) {
> +                               fw_usage_v2_1 =
> +                                       (struct vram_usagebyfirmware_v2_1 *)(ctx->bios + data_offset);
> +                               amdgpu_atomfirmware_allocate_fb_v2_1(adev,
> +                                               fw_usage_v2_1,
> +                                               &usage_bytes);
> +                       } else if (frev >= 2 && crev >= 2) {
> +                               fw_usage_v2_2 =
> +                                       (struct vram_usagebyfirmware_v2_2 *)(ctx->bios + data_offset);
> +                               amdgpu_atomfirmware_allocate_fb_v2_2(adev,
> +                                               fw_usage_v2_2,
> +                                               &usage_bytes);
> +                       }
>                 }
>         }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 96bd0185f936..c0810c53703c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1943,23 +1943,28 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>         if (r)
>                 return r;
>
> -       /*
> -        *The reserved vram for driver must be pinned to the specified
> -        *place on the VRAM, so reserve it early.
> -        */
> -       r = amdgpu_ttm_drv_reserve_vram_init(adev);
> -       if (r)
> -               return r;
> -
> -       /*
> -        * only NAVI10 and onwards ASIC support for IP discovery.
> -        * If IP discovery enabled, a block of memory should be
> -        * reserved for IP discovey.
> +       /* For VFs with dynamic critical regions (v2), fw_reserve_vram is already
> +        * reserved, so no additional memory reservation is required below.
>          */
> -       if (adev->mman.discovery_bin) {
> -               r = amdgpu_ttm_reserve_tmr(adev);
> +       if (!amdgpu_sriov_vf(adev) || (adev->virt.req_init_data_ver != GPU_CRIT_REGION_V2)) {
> +               /*
> +                * The reserved VRAM for the driver must be pinned to a specific
> +                * location in VRAM, so reserve it early.
> +                */
> +               r = amdgpu_ttm_drv_reserve_vram_init(adev);
>                 if (r)
>                         return r;
> +
> +               /*
> +                * only NAVI10 and later ASICs support IP discovery.
> +                * If IP discovery is enabled, a block of memory should be
> +                * reserved for it.
> +                */
> +               if (adev->mman.discovery_bin) {
> +                       r = amdgpu_ttm_reserve_tmr(adev);
> +                       if (r)
> +                               return r;
> +               }
>         }
>
>         /* allocate memory as required for VGA
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 6eca5e8a7375..461e83728594 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -940,6 +940,15 @@ int amdgpu_virt_init_critical_region(struct amdgpu_device *adev)
>                 adev->virt.crit_regn_tbl[AMD_SRIOV_MSG_BAD_PAGE_INFO_TABLE_ID].size_kb =
>                         init_data_hdr->bad_page_size_in_kb;
>
> +               /* reserved memory starts from crit region base offset with the size of 5MB */
> +               adev->mman.fw_vram_usage_start_offset = adev->virt.crit_regn.offset;
> +               adev->mman.fw_vram_usage_size = adev->virt.crit_regn.size_kb << 10;
> +               dev_info(adev->dev,
> +                       "critical region v%d requested to reserve memory start at %08x with %d KB.\n",
> +                         init_data_hdr->version,
> +                         adev->mman.fw_vram_usage_start_offset,
> +                         adev->mman.fw_vram_usage_size >> 10);
> +
>                 adev->virt.is_dynamic_crit_regn_enabled = true;
>                 break;
>         default:
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-10-10 15:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-09  3:30 [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV Ellen Pan
2025-10-09 12:26 ` Christian König
2025-10-09 12:52 ` Alex Deucher
2025-10-09 13:59 ` Lazar, Lijo
  -- strict thread matches above, loose matches on Subject: below --
2025-10-10  4:43 [PATCH 1/6] drm/amdgpu: Updated naming of SRIOV critical region offsets/sizes with _V1 suffix Ellen Pan
2025-10-10  4:43 ` [PATCH 4/6] drm/amdgpu: Reuse fw_vram_usage_* for dynamic critical region in SRIOV Ellen Pan
2025-10-10 10:33   ` Lazar, Lijo
2025-10-10 15:10   ` Alex Deucher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox