[PATCH] drm/amdgpu: Add comments to gmc structure

[PATCH] drm/amdgpu: Add comments to gmc structure

[Zeng, Oak]

Explain fields like aper_base, agp_start etc. The definition
of those fields are confusing as they are from different view
(CPU or GPU). Add comments for easier understand.

Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index 555d8e5..8003201 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -127,18 +127,43 @@ struct amdgpu_xgmi {
 };
 
 struct amdgpu_gmc {
+	/* FB's physical address in MMIO space (for CPU to
+	 * map FB). This is different compared to the apg/
+	 * gart/vram_start/end field as the later is from
+	 * GPU's view and aper_base is from CPU's view.
+	 */
 	resource_size_t		aper_size;
 	resource_size_t		aper_base;
 	/* for some chips with <= 32MB we need to lie
 	 * about vram size near mc fb location */
 	u64			mc_vram_size;
 	u64			visible_vram_size;
+	/* APG aperture start and end in MC address space
+	 * Driver find a hole in the MC address space
+	 * to place AGP by setting MC_VM_AGP_BOT/TOP registers
+	 * Under VMID0, logical address == MC address
+	 * AGP aperture is used to simulate FB in ZFB case
+	 */
 	u64			agp_size;
 	u64			agp_start;
 	u64			agp_end;
+	/* GART aperture start and end in MC address space
+	 * Driver find a hole in the MC address space
+	 * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
+	 * registers
+	 * Under VMID0, logical address inside GART aperture will
+	 * be translated through gpuvm gart page table to access
+	 * paged system memory
+	 */
 	u64			gart_size;
 	u64			gart_start;
 	u64			gart_end;
+	/* Frame buffer aperture of this GPU device. Different from
+	 * fb_start (see below), this only covers the local GPU device.
+	 * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
+	 * and calculate vram_start of this local device by adding an
+	 * offset inside the XGMI hive.
+	 */
 	u64			vram_start;
 	u64			vram_end;
 	/* FB region , it's same as local vram region in single GPU, in XGMI
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: Add comments to gmc structure

[Zeng, Oak]

Explain fields like aper_base, agp_start etc. The definition
of those fields are confusing as they are from different view
(CPU or GPU). Add comments for easier understand.

Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index 555d8e5..8003201 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -127,18 +127,43 @@ struct amdgpu_xgmi {
 };
 
 struct amdgpu_gmc {
+	/* FB's physical address in MMIO space (for CPU to
+	 * map FB). This is different compared to the apg/
+	 * gart/vram_start/end field as the later is from
+	 * GPU's view and aper_base is from CPU's view.
+	 */
 	resource_size_t		aper_size;
 	resource_size_t		aper_base;
 	/* for some chips with <= 32MB we need to lie
 	 * about vram size near mc fb location */
 	u64			mc_vram_size;
 	u64			visible_vram_size;
+	/* APG aperture start and end in MC address space
+	 * Driver find a hole in the MC address space
+	 * to place AGP by setting MC_VM_AGP_BOT/TOP registers
+	 * Under VMID0, logical address == MC address
+	 * AGP aperture is used to simulate FB in ZFB case
+	 */
 	u64			agp_size;
 	u64			agp_start;
 	u64			agp_end;
+	/* GART aperture start and end in MC address space
+	 * Driver find a hole in the MC address space
+	 * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
+	 * registers
+	 * Under VMID0, logical address inside GART aperture will
+	 * be translated through gpuvm gart page table to access
+	 * paged system memory
+	 */
 	u64			gart_size;
 	u64			gart_start;
 	u64			gart_end;
+	/* Frame buffer aperture of this GPU device. Different from
+	 * fb_start (see below), this only covers the local GPU device.
+	 * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
+	 * and calculate vram_start of this local device by adding an
+	 * offset inside the XGMI hive.
+	 */
 	u64			vram_start;
 	u64			vram_end;
 	/* FB region , it's same as local vram region in single GPU, in XGMI
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add comments to gmc structure

[Alex Deucher]

On Wed, Nov 6, 2019 at 12:21 PM Zeng, Oak <Oak.Zeng@amd.com> wrote:
>
> Explain fields like aper_base, agp_start etc. The definition
> of those fields are confusing as they are from different view
> (CPU or GPU). Add comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>

A few comments below, otherwise looks good to me.

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..8003201 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,43 @@ struct amdgpu_xgmi {
>  };
>
>  struct amdgpu_gmc {
> +       /* FB's physical address in MMIO space (for CPU to
> +        * map FB). This is different compared to the apg/

apg -> agp

> +        * gart/vram_start/end field as the later is from
> +        * GPU's view and aper_base is from CPU's view.
> +        */
>         resource_size_t         aper_size;
>         resource_size_t         aper_base;
>         /* for some chips with <= 32MB we need to lie
>          * about vram size near mc fb location */
>         u64                     mc_vram_size;
>         u64                     visible_vram_size;
> +       /* APG aperture start and end in MC address space

APG -> AGP

> +        * Driver find a hole in the MC address space
> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +        * Under VMID0, logical address == MC address
> +        * AGP aperture is used to simulate FB in ZFB case
> +        */

You may want to add a comment that the AGP aperture just maps to
physical bus or IOVA addresses on the platform.  It's also used for
page tables in system memory.

>         u64                     agp_size;
>         u64                     agp_start;
>         u64                     agp_end;
> +       /* GART aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +        * registers
> +        * Under VMID0, logical address inside GART aperture will
> +        * be translated through gpuvm gart page table to access
> +        * paged system memory
> +        */
>         u64                     gart_size;
>         u64                     gart_start;
>         u64                     gart_end;
> +       /* Frame buffer aperture of this GPU device. Different from
> +        * fb_start (see below), this only covers the local GPU device.
> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +        * and calculate vram_start of this local device by adding an
> +        * offset inside the XGMI hive.
> +        */
>         u64                     vram_start;
>         u64                     vram_end;
>         /* FB region , it's same as local vram region in single GPU, in XGMI
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add comments to gmc structure

[Alex Deucher]

On Wed, Nov 6, 2019 at 12:21 PM Zeng, Oak <Oak.Zeng@amd.com> wrote:
>
> Explain fields like aper_base, agp_start etc. The definition
> of those fields are confusing as they are from different view
> (CPU or GPU). Add comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>

A few comments below, otherwise looks good to me.

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..8003201 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,43 @@ struct amdgpu_xgmi {
>  };
>
>  struct amdgpu_gmc {
> +       /* FB's physical address in MMIO space (for CPU to
> +        * map FB). This is different compared to the apg/

apg -> agp

> +        * gart/vram_start/end field as the later is from
> +        * GPU's view and aper_base is from CPU's view.
> +        */
>         resource_size_t         aper_size;
>         resource_size_t         aper_base;
>         /* for some chips with <= 32MB we need to lie
>          * about vram size near mc fb location */
>         u64                     mc_vram_size;
>         u64                     visible_vram_size;
> +       /* APG aperture start and end in MC address space

APG -> AGP

> +        * Driver find a hole in the MC address space
> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +        * Under VMID0, logical address == MC address
> +        * AGP aperture is used to simulate FB in ZFB case
> +        */

You may want to add a comment that the AGP aperture just maps to
physical bus or IOVA addresses on the platform.  It's also used for
page tables in system memory.

>         u64                     agp_size;
>         u64                     agp_start;
>         u64                     agp_end;
> +       /* GART aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +        * registers
> +        * Under VMID0, logical address inside GART aperture will
> +        * be translated through gpuvm gart page table to access
> +        * paged system memory
> +        */
>         u64                     gart_size;
>         u64                     gart_start;
>         u64                     gart_end;
> +       /* Frame buffer aperture of this GPU device. Different from
> +        * fb_start (see below), this only covers the local GPU device.
> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +        * and calculate vram_start of this local device by adding an
> +        * offset inside the XGMI hive.
> +        */
>         u64                     vram_start;
>         u64                     vram_end;
>         /* FB region , it's same as local vram region in single GPU, in XGMI
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: Add comments to gmc structure

[Zeng, Oak]

Thanks Alex. 

> AGP is also used for page tables in system memory.
I am not aware of this usage. I thought page table are all in frame buffer today. Was this a use case in old asics?

Oak

-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com> 
Sent: Wednesday, November 6, 2019 12:37 PM
To: Zeng, Oak <Oak.Zeng@amd.com>
Cc: amd-gfx@lists.freedesktop.org; Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Add comments to gmc structure

On Wed, Nov 6, 2019 at 12:21 PM Zeng, Oak <Oak.Zeng@amd.com> wrote:
>
> Explain fields like aper_base, agp_start etc. The definition of those 
> fields are confusing as they are from different view (CPU or GPU). Add 
> comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>

A few comments below, otherwise looks good to me.

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 25 
> +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..8003201 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,43 @@ struct amdgpu_xgmi {  };
>
>  struct amdgpu_gmc {
> +       /* FB's physical address in MMIO space (for CPU to
> +        * map FB). This is different compared to the apg/

apg -> agp

> +        * gart/vram_start/end field as the later is from
> +        * GPU's view and aper_base is from CPU's view.
> +        */
>         resource_size_t         aper_size;
>         resource_size_t         aper_base;
>         /* for some chips with <= 32MB we need to lie
>          * about vram size near mc fb location */
>         u64                     mc_vram_size;
>         u64                     visible_vram_size;
> +       /* APG aperture start and end in MC address space

APG -> AGP

> +        * Driver find a hole in the MC address space
> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +        * Under VMID0, logical address == MC address
> +        * AGP aperture is used to simulate FB in ZFB case
> +        */

You may want to add a comment that the AGP aperture just maps to physical bus or IOVA addresses on the platform.  It's also used for page tables in system memory.

>         u64                     agp_size;
>         u64                     agp_start;
>         u64                     agp_end;
> +       /* GART aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +        * registers
> +        * Under VMID0, logical address inside GART aperture will
> +        * be translated through gpuvm gart page table to access
> +        * paged system memory
> +        */
>         u64                     gart_size;
>         u64                     gart_start;
>         u64                     gart_end;
> +       /* Frame buffer aperture of this GPU device. Different from
> +        * fb_start (see below), this only covers the local GPU device.
> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +        * and calculate vram_start of this local device by adding an
> +        * offset inside the XGMI hive.
> +        */
>         u64                     vram_start;
>         u64                     vram_end;
>         /* FB region , it's same as local vram region in single GPU, 
> in XGMI
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: Add comments to gmc structure

[Zeng, Oak]

Thanks Alex. 

> AGP is also used for page tables in system memory.
I am not aware of this usage. I thought page table are all in frame buffer today. Was this a use case in old asics?

Oak

-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com> 
Sent: Wednesday, November 6, 2019 12:37 PM
To: Zeng, Oak <Oak.Zeng@amd.com>
Cc: amd-gfx@lists.freedesktop.org; Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Add comments to gmc structure

On Wed, Nov 6, 2019 at 12:21 PM Zeng, Oak <Oak.Zeng@amd.com> wrote:
>
> Explain fields like aper_base, agp_start etc. The definition of those 
> fields are confusing as they are from different view (CPU or GPU). Add 
> comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>

A few comments below, otherwise looks good to me.

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 25 
> +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..8003201 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,43 @@ struct amdgpu_xgmi {  };
>
>  struct amdgpu_gmc {
> +       /* FB's physical address in MMIO space (for CPU to
> +        * map FB). This is different compared to the apg/

apg -> agp

> +        * gart/vram_start/end field as the later is from
> +        * GPU's view and aper_base is from CPU's view.
> +        */
>         resource_size_t         aper_size;
>         resource_size_t         aper_base;
>         /* for some chips with <= 32MB we need to lie
>          * about vram size near mc fb location */
>         u64                     mc_vram_size;
>         u64                     visible_vram_size;
> +       /* APG aperture start and end in MC address space

APG -> AGP

> +        * Driver find a hole in the MC address space
> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +        * Under VMID0, logical address == MC address
> +        * AGP aperture is used to simulate FB in ZFB case
> +        */

You may want to add a comment that the AGP aperture just maps to physical bus or IOVA addresses on the platform.  It's also used for page tables in system memory.

>         u64                     agp_size;
>         u64                     agp_start;
>         u64                     agp_end;
> +       /* GART aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +        * registers
> +        * Under VMID0, logical address inside GART aperture will
> +        * be translated through gpuvm gart page table to access
> +        * paged system memory
> +        */
>         u64                     gart_size;
>         u64                     gart_start;
>         u64                     gart_end;
> +       /* Frame buffer aperture of this GPU device. Different from
> +        * fb_start (see below), this only covers the local GPU device.
> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +        * and calculate vram_start of this local device by adding an
> +        * offset inside the XGMI hive.
> +        */
>         u64                     vram_start;
>         u64                     vram_end;
>         /* FB region , it's same as local vram region in single GPU, 
> in XGMI
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add comments to gmc structure

[Koenig, Christian]

Am 06.11.19 um 21:05 schrieb Zeng, Oak:
> Thanks Alex.
>
>> AGP is also used for page tables in system memory.
> I am not aware of this usage. I thought page table are all in frame buffer today. Was this a use case in old asics?

No, that is pretty new and only works for Renoir. But we have disabled 
it currently because of bad interactions with IOMMU.

Christian.

>
> Oak
>
> -----Original Message-----
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Wednesday, November 6, 2019 12:37 PM
> To: Zeng, Oak <Oak.Zeng@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: Add comments to gmc structure
>
> On Wed, Nov 6, 2019 at 12:21 PM Zeng, Oak <Oak.Zeng@amd.com> wrote:
>> Explain fields like aper_base, agp_start etc. The definition of those
>> fields are confusing as they are from different view (CPU or GPU). Add
>> comments for easier understand.
>>
>> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
>> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> A few comments below, otherwise looks good to me.
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 25
>> +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> index 555d8e5..8003201 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> @@ -127,18 +127,43 @@ struct amdgpu_xgmi {  };
>>
>>   struct amdgpu_gmc {
>> +       /* FB's physical address in MMIO space (for CPU to
>> +        * map FB). This is different compared to the apg/
> apg -> agp
>
>> +        * gart/vram_start/end field as the later is from
>> +        * GPU's view and aper_base is from CPU's view.
>> +        */
>>          resource_size_t         aper_size;
>>          resource_size_t         aper_base;
>>          /* for some chips with <= 32MB we need to lie
>>           * about vram size near mc fb location */
>>          u64                     mc_vram_size;
>>          u64                     visible_vram_size;
>> +       /* APG aperture start and end in MC address space
> APG -> AGP
>
>> +        * Driver find a hole in the MC address space
>> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
>> +        * Under VMID0, logical address == MC address
>> +        * AGP aperture is used to simulate FB in ZFB case
>> +        */
> You may want to add a comment that the AGP aperture just maps to physical bus or IOVA addresses on the platform.  It's also used for page tables in system memory.
>
>>          u64                     agp_size;
>>          u64                     agp_start;
>>          u64                     agp_end;
>> +       /* GART aperture start and end in MC address space
>> +        * Driver find a hole in the MC address space
>> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
>> +        * registers
>> +        * Under VMID0, logical address inside GART aperture will
>> +        * be translated through gpuvm gart page table to access
>> +        * paged system memory
>> +        */
>>          u64                     gart_size;
>>          u64                     gart_start;
>>          u64                     gart_end;
>> +       /* Frame buffer aperture of this GPU device. Different from
>> +        * fb_start (see below), this only covers the local GPU device.
>> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
>> +        * and calculate vram_start of this local device by adding an
>> +        * offset inside the XGMI hive.
>> +        */
>>          u64                     vram_start;
>>          u64                     vram_end;
>>          /* FB region , it's same as local vram region in single GPU,
>> in XGMI
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add comments to gmc structure

[Koenig, Christian]

Am 06.11.19 um 21:05 schrieb Zeng, Oak:
> Thanks Alex.
>
>> AGP is also used for page tables in system memory.
> I am not aware of this usage. I thought page table are all in frame buffer today. Was this a use case in old asics?

No, that is pretty new and only works for Renoir. But we have disabled 
it currently because of bad interactions with IOMMU.

Christian.

>
> Oak
>
> -----Original Message-----
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Wednesday, November 6, 2019 12:37 PM
> To: Zeng, Oak <Oak.Zeng@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: Add comments to gmc structure
>
> On Wed, Nov 6, 2019 at 12:21 PM Zeng, Oak <Oak.Zeng@amd.com> wrote:
>> Explain fields like aper_base, agp_start etc. The definition of those
>> fields are confusing as they are from different view (CPU or GPU). Add
>> comments for easier understand.
>>
>> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
>> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> A few comments below, otherwise looks good to me.
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 25
>> +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> index 555d8e5..8003201 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> @@ -127,18 +127,43 @@ struct amdgpu_xgmi {  };
>>
>>   struct amdgpu_gmc {
>> +       /* FB's physical address in MMIO space (for CPU to
>> +        * map FB). This is different compared to the apg/
> apg -> agp
>
>> +        * gart/vram_start/end field as the later is from
>> +        * GPU's view and aper_base is from CPU's view.
>> +        */
>>          resource_size_t         aper_size;
>>          resource_size_t         aper_base;
>>          /* for some chips with <= 32MB we need to lie
>>           * about vram size near mc fb location */
>>          u64                     mc_vram_size;
>>          u64                     visible_vram_size;
>> +       /* APG aperture start and end in MC address space
> APG -> AGP
>
>> +        * Driver find a hole in the MC address space
>> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
>> +        * Under VMID0, logical address == MC address
>> +        * AGP aperture is used to simulate FB in ZFB case
>> +        */
> You may want to add a comment that the AGP aperture just maps to physical bus or IOVA addresses on the platform.  It's also used for page tables in system memory.
>
>>          u64                     agp_size;
>>          u64                     agp_start;
>>          u64                     agp_end;
>> +       /* GART aperture start and end in MC address space
>> +        * Driver find a hole in the MC address space
>> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
>> +        * registers
>> +        * Under VMID0, logical address inside GART aperture will
>> +        * be translated through gpuvm gart page table to access
>> +        * paged system memory
>> +        */
>>          u64                     gart_size;
>>          u64                     gart_start;
>>          u64                     gart_end;
>> +       /* Frame buffer aperture of this GPU device. Different from
>> +        * fb_start (see below), this only covers the local GPU device.
>> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
>> +        * and calculate vram_start of this local device by adding an
>> +        * offset inside the XGMI hive.
>> +        */
>>          u64                     vram_start;
>>          u64                     vram_end;
>>          /* FB region , it's same as local vram region in single GPU,
>> in XGMI
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: Add comments to gmc structure

[Zeng, Oak]

[-- Attachment #1.1: Type: text/plain, Size: 6324 bytes --]

Hi Yong,

That has been submitted.

MC address is the address sent to memory controller bus to address the physical memory. In many places it is also called physical address.

Logical address is the address before it is translated to physical address. The translation can be either linear or through page table. Under VMID0, logical address of some apertures (AGP, FB) are linearly mapped to MC address. (I think in our driver implementation, logical address == MC address). But Gart aperture is mapped through gpuvm (gart) page table - the mapped address can also be called virtual address.

Under VMID1~15, it is user space virtual address (can also be called logical address), mapped through gpuvm page tables.

So virtual address is one type of logical address. Virtual address is usually mapped through page table/vm.

Just my understanding.

Regards,
Oak

From: Zhao, Yong <Yong.Zhao-5C7GfCeVMHo@public.gmane.org>
Sent: Thursday, November 7, 2019 12:16 PM
To: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
Cc: Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH] drm/amdgpu: Add comments to gmc structure

If this is not submitted, I would like to see some comments regarding the explanation of MC address and logical address, which I prefer to mention as GPU physical/virtual address.

Regards,
Yong


________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org<mailto:alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>>
Sent: Thursday, November 7, 2019 9:02 AM
To: Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org<mailto:Oak.Zeng-5C7GfCeVMHo@public.gmane.org>>
Cc: Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org<mailto:Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>>; Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org<mailto:Christian.Koenig@amd.com>>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>>
Subject: Re: [PATCH] drm/amdgpu: Add comments to gmc structure

On Wed, Nov 6, 2019 at 12:27 PM Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org<mailto:Oak.Zeng@amd.com>> wrote:
>
> Explain fields like aper_base, agp_start etc. The definition
> of those fields are confusing as they are from different view
> (CPU or GPU). Add comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng-5C7GfCeVMHo@public.gmane.org<mailto:Oak.Zeng-5C7GfCeVMHo@public.gmane.org>>

Same comments as the previous version.  With those addressed,
Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org<mailto:alexander.deucher-5C7GfCeVMHo@public.gmane.org>>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..1356ff9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,44 @@ struct amdgpu_xgmi {
>  };
>
>  struct amdgpu_gmc {
> +       /* FB's physical address in MMIO space (for CPU to
> +        * map FB). This is different compared to the apg/
> +        * gart/vram_start/end field as the later is from
> +        * GPU's view and aper_base is from CPU's view.
> +        */
>         resource_size_t         aper_size;
>         resource_size_t         aper_base;
>         /* for some chips with <= 32MB we need to lie
>          * about vram size near mc fb location */
>         u64                     mc_vram_size;
>         u64                     visible_vram_size;
> +       /* APG aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +        * Under VMID0, logical address == MC address
> +        * AGP aperture is used to simulate FB in ZFB case
> +        */
>         u64                     agp_size;
>         u64                     agp_start;
>         u64                     agp_end;
> +       /* GART aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +        * registers
> +        * Under VMID0, logical address inside GART aperture will
> +        * be translated through gpuvm gart page table to access
> +        * paged system memory
> +        */
>         u64                     gart_size;
>         u64                     gart_start;
>         u64                     gart_end;
> +       /* Frame buffer aperture of this GPU device. Different from
> +        * fb_start (see below), this only covers the local GPU device.
> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +        * and calculate vram_start of this local device by adding an
> +        * offset inside the XGMI hive.
> +        * Under VMID0, logical address == MC address
> +        */
>         u64                     vram_start;
>         u64                     vram_end;
>         /* FB region , it's same as local vram region in single GPU, in XGMI
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 14300 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: Add comments to gmc structure

[Zeng, Oak]

[-- Attachment #1.1: Type: text/plain, Size: 5660 bytes --]

Hi Yong,

That has been submitted.

MC address is the address sent to memory controller bus to address the physical memory. In many places it is also called physical address.

Logical address is the address before it is translated to physical address. The translation can be either linear or through page table. Under VMID0, logical address of some apertures (AGP, FB) are linearly mapped to MC address. (I think in our driver implementation, logical address == MC address). But Gart aperture is mapped through gpuvm (gart) page table - the mapped address can also be called virtual address.

Under VMID1~15, it is user space virtual address (can also be called logical address), mapped through gpuvm page tables.

So virtual address is one type of logical address. Virtual address is usually mapped through page table/vm.

Just my understanding.

Regards,
Oak

From: Zhao, Yong <Yong.Zhao@amd.com>
Sent: Thursday, November 7, 2019 12:16 PM
To: Alex Deucher <alexdeucher@gmail.com>; Zeng, Oak <Oak.Zeng@amd.com>
Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Add comments to gmc structure

If this is not submitted, I would like to see some comments regarding the explanation of MC address and logical address, which I prefer to mention as GPU physical/virtual address.

Regards,
Yong


________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Alex Deucher <alexdeucher@gmail.com<mailto:alexdeucher@gmail.com>>
Sent: Thursday, November 7, 2019 9:02 AM
To: Zeng, Oak <Oak.Zeng@amd.com<mailto:Oak.Zeng@amd.com>>
Cc: Kuehling, Felix <Felix.Kuehling@amd.com<mailto:Felix.Kuehling@amd.com>>; Koenig, Christian <Christian.Koenig@amd.com<mailto:Christian.Koenig@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH] drm/amdgpu: Add comments to gmc structure

On Wed, Nov 6, 2019 at 12:27 PM Zeng, Oak <Oak.Zeng@amd.com<mailto:Oak.Zeng@amd.com>> wrote:
>
> Explain fields like aper_base, agp_start etc. The definition
> of those fields are confusing as they are from different view
> (CPU or GPU). Add comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com<mailto:Oak.Zeng@amd.com>>

Same comments as the previous version.  With those addressed,
Reviewed-by: Alex Deucher <alexander.deucher@amd.com<mailto:alexander.deucher@amd.com>>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..1356ff9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,44 @@ struct amdgpu_xgmi {
>  };
>
>  struct amdgpu_gmc {
> +       /* FB's physical address in MMIO space (for CPU to
> +        * map FB). This is different compared to the apg/
> +        * gart/vram_start/end field as the later is from
> +        * GPU's view and aper_base is from CPU's view.
> +        */
>         resource_size_t         aper_size;
>         resource_size_t         aper_base;
>         /* for some chips with <= 32MB we need to lie
>          * about vram size near mc fb location */
>         u64                     mc_vram_size;
>         u64                     visible_vram_size;
> +       /* APG aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +        * Under VMID0, logical address == MC address
> +        * AGP aperture is used to simulate FB in ZFB case
> +        */
>         u64                     agp_size;
>         u64                     agp_start;
>         u64                     agp_end;
> +       /* GART aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +        * registers
> +        * Under VMID0, logical address inside GART aperture will
> +        * be translated through gpuvm gart page table to access
> +        * paged system memory
> +        */
>         u64                     gart_size;
>         u64                     gart_start;
>         u64                     gart_end;
> +       /* Frame buffer aperture of this GPU device. Different from
> +        * fb_start (see below), this only covers the local GPU device.
> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +        * and calculate vram_start of this local device by adding an
> +        * offset inside the XGMI hive.
> +        * Under VMID0, logical address == MC address
> +        */
>         u64                     vram_start;
>         u64                     vram_end;
>         /* FB region , it's same as local vram region in single GPU, in XGMI
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 13635 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add comments to gmc structure

[Zhao, Yong]

[-- Attachment #1.1: Type: text/plain, Size: 4336 bytes --]

If this is not submitted, I would like to see some comments regarding the explanation of MC address and logical address, which I prefer to mention as GPU physical/virtual address.

Regards,
Yong


________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Thursday, November 7, 2019 9:02 AM
To: Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
Cc: Kuehling, Felix <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>; Koenig, Christian <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <amd-gfx-PD4FTy7X32lNgt0PjOBp93rCq3LdnpKM@public.gmane.orgrg>
Subject: Re: [PATCH] drm/amdgpu: Add comments to gmc structure

On Wed, Nov 6, 2019 at 12:27 PM Zeng, Oak <Oak.Zeng-5C7GfCeVMHo@public.gmane.org> wrote:
>
> Explain fields like aper_base, agp_start etc. The definition
> of those fields are confusing as they are from different view
> (CPU or GPU). Add comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>

Same comments as the previous version.  With those addressed,
Reviewed-by: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..1356ff9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,44 @@ struct amdgpu_xgmi {
>  };
>
>  struct amdgpu_gmc {
> +       /* FB's physical address in MMIO space (for CPU to
> +        * map FB). This is different compared to the apg/
> +        * gart/vram_start/end field as the later is from
> +        * GPU's view and aper_base is from CPU's view.
> +        */
>         resource_size_t         aper_size;
>         resource_size_t         aper_base;
>         /* for some chips with <= 32MB we need to lie
>          * about vram size near mc fb location */
>         u64                     mc_vram_size;
>         u64                     visible_vram_size;
> +       /* APG aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +        * Under VMID0, logical address == MC address
> +        * AGP aperture is used to simulate FB in ZFB case
> +        */
>         u64                     agp_size;
>         u64                     agp_start;
>         u64                     agp_end;
> +       /* GART aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +        * registers
> +        * Under VMID0, logical address inside GART aperture will
> +        * be translated through gpuvm gart page table to access
> +        * paged system memory
> +        */
>         u64                     gart_size;
>         u64                     gart_start;
>         u64                     gart_end;
> +       /* Frame buffer aperture of this GPU device. Different from
> +        * fb_start (see below), this only covers the local GPU device.
> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +        * and calculate vram_start of this local device by adding an
> +        * offset inside the XGMI hive.
> +        * Under VMID0, logical address == MC address
> +        */
>         u64                     vram_start;
>         u64                     vram_end;
>         /* FB region , it's same as local vram region in single GPU, in XGMI
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 9104 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add comments to gmc structure

[Zhao, Yong]

[-- Attachment #1.1: Type: text/plain, Size: 4038 bytes --]

If this is not submitted, I would like to see some comments regarding the explanation of MC address and logical address, which I prefer to mention as GPU physical/virtual address.

Regards,
Yong


________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Alex Deucher <alexdeucher@gmail.com>
Sent: Thursday, November 7, 2019 9:02 AM
To: Zeng, Oak <Oak.Zeng@amd.com>
Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amdgpu: Add comments to gmc structure

On Wed, Nov 6, 2019 at 12:27 PM Zeng, Oak <Oak.Zeng@amd.com> wrote:
>
> Explain fields like aper_base, agp_start etc. The definition
> of those fields are confusing as they are from different view
> (CPU or GPU). Add comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>

Same comments as the previous version.  With those addressed,
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..1356ff9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,44 @@ struct amdgpu_xgmi {
>  };
>
>  struct amdgpu_gmc {
> +       /* FB's physical address in MMIO space (for CPU to
> +        * map FB). This is different compared to the apg/
> +        * gart/vram_start/end field as the later is from
> +        * GPU's view and aper_base is from CPU's view.
> +        */
>         resource_size_t         aper_size;
>         resource_size_t         aper_base;
>         /* for some chips with <= 32MB we need to lie
>          * about vram size near mc fb location */
>         u64                     mc_vram_size;
>         u64                     visible_vram_size;
> +       /* APG aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +        * Under VMID0, logical address == MC address
> +        * AGP aperture is used to simulate FB in ZFB case
> +        */
>         u64                     agp_size;
>         u64                     agp_start;
>         u64                     agp_end;
> +       /* GART aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +        * registers
> +        * Under VMID0, logical address inside GART aperture will
> +        * be translated through gpuvm gart page table to access
> +        * paged system memory
> +        */
>         u64                     gart_size;
>         u64                     gart_start;
>         u64                     gart_end;
> +       /* Frame buffer aperture of this GPU device. Different from
> +        * fb_start (see below), this only covers the local GPU device.
> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +        * and calculate vram_start of this local device by adding an
> +        * offset inside the XGMI hive.
> +        * Under VMID0, logical address == MC address
> +        */
>         u64                     vram_start;
>         u64                     vram_end;
>         /* FB region , it's same as local vram region in single GPU, in XGMI
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 8838 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add comments to gmc structure

[Alex Deucher]

On Wed, Nov 6, 2019 at 12:27 PM Zeng, Oak <Oak.Zeng@amd.com> wrote:
>
> Explain fields like aper_base, agp_start etc. The definition
> of those fields are confusing as they are from different view
> (CPU or GPU). Add comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>

Same comments as the previous version.  With those addressed,
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..1356ff9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,44 @@ struct amdgpu_xgmi {
>  };
>
>  struct amdgpu_gmc {
> +       /* FB's physical address in MMIO space (for CPU to
> +        * map FB). This is different compared to the apg/
> +        * gart/vram_start/end field as the later is from
> +        * GPU's view and aper_base is from CPU's view.
> +        */
>         resource_size_t         aper_size;
>         resource_size_t         aper_base;
>         /* for some chips with <= 32MB we need to lie
>          * about vram size near mc fb location */
>         u64                     mc_vram_size;
>         u64                     visible_vram_size;
> +       /* APG aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +        * Under VMID0, logical address == MC address
> +        * AGP aperture is used to simulate FB in ZFB case
> +        */
>         u64                     agp_size;
>         u64                     agp_start;
>         u64                     agp_end;
> +       /* GART aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +        * registers
> +        * Under VMID0, logical address inside GART aperture will
> +        * be translated through gpuvm gart page table to access
> +        * paged system memory
> +        */
>         u64                     gart_size;
>         u64                     gart_start;
>         u64                     gart_end;
> +       /* Frame buffer aperture of this GPU device. Different from
> +        * fb_start (see below), this only covers the local GPU device.
> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +        * and calculate vram_start of this local device by adding an
> +        * offset inside the XGMI hive.
> +        * Under VMID0, logical address == MC address
> +        */
>         u64                     vram_start;
>         u64                     vram_end;
>         /* FB region , it's same as local vram region in single GPU, in XGMI
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add comments to gmc structure

[Alex Deucher]

On Wed, Nov 6, 2019 at 12:27 PM Zeng, Oak <Oak.Zeng@amd.com> wrote:
>
> Explain fields like aper_base, agp_start etc. The definition
> of those fields are confusing as they are from different view
> (CPU or GPU). Add comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>

Same comments as the previous version.  With those addressed,
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..1356ff9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,44 @@ struct amdgpu_xgmi {
>  };
>
>  struct amdgpu_gmc {
> +       /* FB's physical address in MMIO space (for CPU to
> +        * map FB). This is different compared to the apg/
> +        * gart/vram_start/end field as the later is from
> +        * GPU's view and aper_base is from CPU's view.
> +        */
>         resource_size_t         aper_size;
>         resource_size_t         aper_base;
>         /* for some chips with <= 32MB we need to lie
>          * about vram size near mc fb location */
>         u64                     mc_vram_size;
>         u64                     visible_vram_size;
> +       /* APG aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +        * Under VMID0, logical address == MC address
> +        * AGP aperture is used to simulate FB in ZFB case
> +        */
>         u64                     agp_size;
>         u64                     agp_start;
>         u64                     agp_end;
> +       /* GART aperture start and end in MC address space
> +        * Driver find a hole in the MC address space
> +        * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +        * registers
> +        * Under VMID0, logical address inside GART aperture will
> +        * be translated through gpuvm gart page table to access
> +        * paged system memory
> +        */
>         u64                     gart_size;
>         u64                     gart_start;
>         u64                     gart_end;
> +       /* Frame buffer aperture of this GPU device. Different from
> +        * fb_start (see below), this only covers the local GPU device.
> +        * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +        * and calculate vram_start of this local device by adding an
> +        * offset inside the XGMI hive.
> +        * Under VMID0, logical address == MC address
> +        */
>         u64                     vram_start;
>         u64                     vram_end;
>         /* FB region , it's same as local vram region in single GPU, in XGMI
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add comments to gmc structure

[Christian König]

Am 06.11.19 um 18:26 schrieb Zeng, Oak:
> Explain fields like aper_base, agp_start etc. The definition
> of those fields are confusing as they are from different view
> (CPU or GPU). Add comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..1356ff9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,44 @@ struct amdgpu_xgmi {
>   };
>   
>   struct amdgpu_gmc {
> +	/* FB's physical address in MMIO space (for CPU to
> +	 * map FB). This is different compared to the apg/
> +	 * gart/vram_start/end field as the later is from
> +	 * GPU's view and aper_base is from CPU's view.
> +	 */
>   	resource_size_t		aper_size;
>   	resource_size_t		aper_base;
>   	/* for some chips with <= 32MB we need to lie
>   	 * about vram size near mc fb location */
>   	u64			mc_vram_size;
>   	u64			visible_vram_size;
> +	/* APG aperture start and end in MC address space
> +	 * Driver find a hole in the MC address space
> +	 * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +	 * Under VMID0, logical address == MC address
> +	 * AGP aperture is used to simulate FB in ZFB case
> +	 */

Really nice improvement. Alex noted as well please mention the page 
table use case here as well.

Additional to that we also have a use case for display scanout, but that 
one is currently not implemented yet. Might be a good idea to mention 
that as well.

With that fixed the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

>   	u64			agp_size;
>   	u64			agp_start;
>   	u64			agp_end;
> +	/* GART aperture start and end in MC address space
> +	 * Driver find a hole in the MC address space
> +	 * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +	 * registers
> +	 * Under VMID0, logical address inside GART aperture will
> +	 * be translated through gpuvm gart page table to access
> +	 * paged system memory
> +	 */
>   	u64			gart_size;
>   	u64			gart_start;
>   	u64			gart_end;
> +	/* Frame buffer aperture of this GPU device. Different from
> +	 * fb_start (see below), this only covers the local GPU device.
> +	 * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +	 * and calculate vram_start of this local device by adding an
> +	 * offset inside the XGMI hive.
> +	 * Under VMID0, logical address == MC address
> +	 */
>   	u64			vram_start;
>   	u64			vram_end;
>   	/* FB region , it's same as local vram region in single GPU, in XGMI

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add comments to gmc structure

[Christian König]

Am 06.11.19 um 18:26 schrieb Zeng, Oak:
> Explain fields like aper_base, agp_start etc. The definition
> of those fields are confusing as they are from different view
> (CPU or GPU). Add comments for easier understand.
>
> Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 555d8e5..1356ff9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -127,18 +127,44 @@ struct amdgpu_xgmi {
>   };
>   
>   struct amdgpu_gmc {
> +	/* FB's physical address in MMIO space (for CPU to
> +	 * map FB). This is different compared to the apg/
> +	 * gart/vram_start/end field as the later is from
> +	 * GPU's view and aper_base is from CPU's view.
> +	 */
>   	resource_size_t		aper_size;
>   	resource_size_t		aper_base;
>   	/* for some chips with <= 32MB we need to lie
>   	 * about vram size near mc fb location */
>   	u64			mc_vram_size;
>   	u64			visible_vram_size;
> +	/* APG aperture start and end in MC address space
> +	 * Driver find a hole in the MC address space
> +	 * to place AGP by setting MC_VM_AGP_BOT/TOP registers
> +	 * Under VMID0, logical address == MC address
> +	 * AGP aperture is used to simulate FB in ZFB case
> +	 */

Really nice improvement. Alex noted as well please mention the page 
table use case here as well.

Additional to that we also have a use case for display scanout, but that 
one is currently not implemented yet. Might be a good idea to mention 
that as well.

With that fixed the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

>   	u64			agp_size;
>   	u64			agp_start;
>   	u64			agp_end;
> +	/* GART aperture start and end in MC address space
> +	 * Driver find a hole in the MC address space
> +	 * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
> +	 * registers
> +	 * Under VMID0, logical address inside GART aperture will
> +	 * be translated through gpuvm gart page table to access
> +	 * paged system memory
> +	 */
>   	u64			gart_size;
>   	u64			gart_start;
>   	u64			gart_end;
> +	/* Frame buffer aperture of this GPU device. Different from
> +	 * fb_start (see below), this only covers the local GPU device.
> +	 * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
> +	 * and calculate vram_start of this local device by adding an
> +	 * offset inside the XGMI hive.
> +	 * Under VMID0, logical address == MC address
> +	 */
>   	u64			vram_start;
>   	u64			vram_end;
>   	/* FB region , it's same as local vram region in single GPU, in XGMI

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: Add comments to gmc structure

[Zeng, Oak]

Explain fields like aper_base, agp_start etc. The definition
of those fields are confusing as they are from different view
(CPU or GPU). Add comments for easier understand.

Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index 555d8e5..1356ff9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -127,18 +127,44 @@ struct amdgpu_xgmi {
 };
 
 struct amdgpu_gmc {
+	/* FB's physical address in MMIO space (for CPU to
+	 * map FB). This is different compared to the apg/
+	 * gart/vram_start/end field as the later is from
+	 * GPU's view and aper_base is from CPU's view.
+	 */
 	resource_size_t		aper_size;
 	resource_size_t		aper_base;
 	/* for some chips with <= 32MB we need to lie
 	 * about vram size near mc fb location */
 	u64			mc_vram_size;
 	u64			visible_vram_size;
+	/* APG aperture start and end in MC address space
+	 * Driver find a hole in the MC address space
+	 * to place AGP by setting MC_VM_AGP_BOT/TOP registers
+	 * Under VMID0, logical address == MC address
+	 * AGP aperture is used to simulate FB in ZFB case
+	 */
 	u64			agp_size;
 	u64			agp_start;
 	u64			agp_end;
+	/* GART aperture start and end in MC address space
+	 * Driver find a hole in the MC address space
+	 * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
+	 * registers
+	 * Under VMID0, logical address inside GART aperture will
+	 * be translated through gpuvm gart page table to access
+	 * paged system memory
+	 */
 	u64			gart_size;
 	u64			gart_start;
 	u64			gart_end;
+	/* Frame buffer aperture of this GPU device. Different from
+	 * fb_start (see below), this only covers the local GPU device.
+	 * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
+	 * and calculate vram_start of this local device by adding an
+	 * offset inside the XGMI hive.
+	 * Under VMID0, logical address == MC address
+	 */
 	u64			vram_start;
 	u64			vram_end;
 	/* FB region , it's same as local vram region in single GPU, in XGMI
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: Add comments to gmc structure

[Zeng, Oak]

Explain fields like aper_base, agp_start etc. The definition
of those fields are confusing as they are from different view
(CPU or GPU). Add comments for easier understand.

Change-Id: I02c2a27cd0dbc205498eb86aafa722f2e0c25fe6
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index 555d8e5..1356ff9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -127,18 +127,44 @@ struct amdgpu_xgmi {
 };
 
 struct amdgpu_gmc {
+	/* FB's physical address in MMIO space (for CPU to
+	 * map FB). This is different compared to the apg/
+	 * gart/vram_start/end field as the later is from
+	 * GPU's view and aper_base is from CPU's view.
+	 */
 	resource_size_t		aper_size;
 	resource_size_t		aper_base;
 	/* for some chips with <= 32MB we need to lie
 	 * about vram size near mc fb location */
 	u64			mc_vram_size;
 	u64			visible_vram_size;
+	/* APG aperture start and end in MC address space
+	 * Driver find a hole in the MC address space
+	 * to place AGP by setting MC_VM_AGP_BOT/TOP registers
+	 * Under VMID0, logical address == MC address
+	 * AGP aperture is used to simulate FB in ZFB case
+	 */
 	u64			agp_size;
 	u64			agp_start;
 	u64			agp_end;
+	/* GART aperture start and end in MC address space
+	 * Driver find a hole in the MC address space
+	 * to place GART by setting VM_CONTEXT0_PAGE_TABLE_START/END_ADDR
+	 * registers
+	 * Under VMID0, logical address inside GART aperture will
+	 * be translated through gpuvm gart page table to access
+	 * paged system memory
+	 */
 	u64			gart_size;
 	u64			gart_start;
 	u64			gart_end;
+	/* Frame buffer aperture of this GPU device. Different from
+	 * fb_start (see below), this only covers the local GPU device.
+	 * Driver get fb_start from MC_VM_FB_LOCATION_BASE (set by vbios)
+	 * and calculate vram_start of this local device by adding an
+	 * offset inside the XGMI hive.
+	 * Under VMID0, logical address == MC address
+	 */
 	u64			vram_start;
 	u64			vram_end;
 	/* FB region , it's same as local vram region in single GPU, in XGMI
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx