[PATCH v2] drm/amdgpu: Read vram width from integrated system info table

[PATCH v2] drm/amdgpu: Read vram width from integrated system info table

[Harry Wentland]

On KB, KV, CZ we should read the vram width from integrated system
table, if we can. The NOOFCHAN in MC_SHARED_CHMAP is not accurate.

With this change we can enable two 4k displays on CZ again. This use
case was broken sometime in January when we started looking at
vram_width for bandwidth calculations instead of hardcoding this value.

v2:
  Return 0 if integrated system info table is not available.

Tested-by: Roman Li <roman.li@amd.com>
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 29 ++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h |  2 +
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c        | 87 +++++++++++++++-------------
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c        | 87 +++++++++++++++-------------
 4 files changed, 123 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
index f52b1bf3d3d9..ad4329922f79 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
@@ -754,6 +754,35 @@ union igp_info {
 	struct _ATOM_INTEGRATED_SYSTEM_INFO_V1_9 info_9;
 };
 
+/*
+ * Return vram width from integrated system info table, if available,
+ * or 0 if not.
+ */
+int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev)
+{
+	struct amdgpu_mode_info *mode_info = &adev->mode_info;
+	int index = GetIndexIntoMasterTable(DATA, IntegratedSystemInfo);
+	u16 data_offset, size;
+	union igp_info *igp_info;
+	u8 frev, crev;
+
+	/* get any igp specific overrides */
+	if (amdgpu_atom_parse_data_header(mode_info->atom_context, index, &size,
+				   &frev, &crev, &data_offset)) {
+		igp_info = (union igp_info *)
+			(mode_info->atom_context->bios + data_offset);
+		switch (crev) {
+		case 8:
+		case 9:
+			return igp_info->info_8.ucUMAChannelNumber * 64;
+		default:
+			return 0;
+		}
+	}
+
+	return 0;
+}
+
 static void amdgpu_atombios_get_igp_ss_overrides(struct amdgpu_device *adev,
 						 struct amdgpu_atom_ss *ss,
 						 int id)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
index 4e0f488487f3..38d0fe32e5cd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
@@ -148,6 +148,8 @@ int amdgpu_atombios_get_clock_info(struct amdgpu_device *adev);
 
 int amdgpu_atombios_get_gfx_info(struct amdgpu_device *adev);
 
+int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev);
+
 bool amdgpu_atombios_get_asic_ss_info(struct amdgpu_device *adev,
 				      struct amdgpu_atom_ss *ss,
 				      int id, u32 clock);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 0c0a6015cca5..78643a1baa5c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -37,6 +37,8 @@
 #include "oss/oss_2_0_d.h"
 #include "oss/oss_2_0_sh_mask.h"
 
+#include "amdgpu_atombios.h"
+
 static void gmc_v7_0_set_gart_funcs(struct amdgpu_device *adev);
 static void gmc_v7_0_set_irq_funcs(struct amdgpu_device *adev);
 static int gmc_v7_0_wait_for_idle(void *handle);
@@ -325,48 +327,51 @@ static void gmc_v7_0_mc_program(struct amdgpu_device *adev)
  */
 static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
 {
-	u32 tmp;
-	int chansize, numchan;
-
-	/* Get VRAM informations */
-	tmp = RREG32(mmMC_ARB_RAMCFG);
-	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
-		chansize = 64;
-	} else {
-		chansize = 32;
-	}
-	tmp = RREG32(mmMC_SHARED_CHMAP);
-	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
-	case 0:
-	default:
-		numchan = 1;
-		break;
-	case 1:
-		numchan = 2;
-		break;
-	case 2:
-		numchan = 4;
-		break;
-	case 3:
-		numchan = 8;
-		break;
-	case 4:
-		numchan = 3;
-		break;
-	case 5:
-		numchan = 6;
-		break;
-	case 6:
-		numchan = 10;
-		break;
-	case 7:
-		numchan = 12;
-		break;
-	case 8:
-		numchan = 16;
-		break;
+	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
+	if (!adev->mc.vram_width) {
+		u32 tmp;
+		int chansize, numchan;
+
+		/* Get VRAM informations */
+		tmp = RREG32(mmMC_ARB_RAMCFG);
+		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
+			chansize = 64;
+		} else {
+			chansize = 32;
+		}
+		tmp = RREG32(mmMC_SHARED_CHMAP);
+		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
+		case 0:
+		default:
+			numchan = 1;
+			break;
+		case 1:
+			numchan = 2;
+			break;
+		case 2:
+			numchan = 4;
+			break;
+		case 3:
+			numchan = 8;
+			break;
+		case 4:
+			numchan = 3;
+			break;
+		case 5:
+			numchan = 6;
+			break;
+		case 6:
+			numchan = 10;
+			break;
+		case 7:
+			numchan = 12;
+			break;
+		case 8:
+			numchan = 16;
+			break;
+		}
+		adev->mc.vram_width = numchan * chansize;
 	}
-	adev->mc.vram_width = numchan * chansize;
 	/* Could aper size report 0 ? */
 	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
 	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index d19d1c5e2847..42b2f357a799 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -38,6 +38,8 @@
 #include "vid.h"
 #include "vi.h"
 
+#include "amdgpu_atombios.h"
+
 
 static void gmc_v8_0_set_gart_funcs(struct amdgpu_device *adev);
 static void gmc_v8_0_set_irq_funcs(struct amdgpu_device *adev);
@@ -487,48 +489,51 @@ static void gmc_v8_0_mc_program(struct amdgpu_device *adev)
  */
 static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
 {
-	u32 tmp;
-	int chansize, numchan;
-
-	/* Get VRAM informations */
-	tmp = RREG32(mmMC_ARB_RAMCFG);
-	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
-		chansize = 64;
-	} else {
-		chansize = 32;
-	}
-	tmp = RREG32(mmMC_SHARED_CHMAP);
-	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
-	case 0:
-	default:
-		numchan = 1;
-		break;
-	case 1:
-		numchan = 2;
-		break;
-	case 2:
-		numchan = 4;
-		break;
-	case 3:
-		numchan = 8;
-		break;
-	case 4:
-		numchan = 3;
-		break;
-	case 5:
-		numchan = 6;
-		break;
-	case 6:
-		numchan = 10;
-		break;
-	case 7:
-		numchan = 12;
-		break;
-	case 8:
-		numchan = 16;
-		break;
+	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
+	if (!adev->mc.vram_width) {
+		u32 tmp;
+		int chansize, numchan;
+
+		/* Get VRAM informations */
+		tmp = RREG32(mmMC_ARB_RAMCFG);
+		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
+			chansize = 64;
+		} else {
+			chansize = 32;
+		}
+		tmp = RREG32(mmMC_SHARED_CHMAP);
+		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
+		case 0:
+		default:
+			numchan = 1;
+			break;
+		case 1:
+			numchan = 2;
+			break;
+		case 2:
+			numchan = 4;
+			break;
+		case 3:
+			numchan = 8;
+			break;
+		case 4:
+			numchan = 3;
+			break;
+		case 5:
+			numchan = 6;
+			break;
+		case 6:
+			numchan = 10;
+			break;
+		case 7:
+			numchan = 12;
+			break;
+		case 8:
+			numchan = 16;
+			break;
+		}
+		adev->mc.vram_width = numchan * chansize;
 	}
-	adev->mc.vram_width = numchan * chansize;
 	/* Could aper size report 0 ? */
 	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
 	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
-- 
2.11.0

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

Re: [PATCH v2] drm/amdgpu: Read vram width from integrated system info table

[Kai Wasserbäch]

[-- Attachment #1.1.1: Type: text/plain, Size: 8416 bytes --]

Dear Harry,
Harry Wentland wrote on 03.04.2017 17:01:
> On KB, KV, CZ we should read the vram width from integrated system
> table, if we can. The NOOFCHAN in MC_SHARED_CHMAP is not accurate.
> 
> With this change we can enable two 4k displays on CZ again. This use
> case was broken sometime in January when we started looking at
> vram_width for bandwidth calculations instead of hardcoding this value.
> 
> v2:
>   Return 0 if integrated system info table is not available.
> 
> Tested-by: Roman Li <roman.li-5C7GfCeVMHo@public.gmane.org>
> Signed-off-by: Harry Wentland <harry.wentland-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 29 ++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h |  2 +
>  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c        | 87 +++++++++++++++-------------
>  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c        | 87 +++++++++++++++-------------
>  4 files changed, 123 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> index f52b1bf3d3d9..ad4329922f79 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> @@ -754,6 +754,35 @@ union igp_info {
>  	struct _ATOM_INTEGRATED_SYSTEM_INFO_V1_9 info_9;
>  };
>  
> +/*
> + * Return vram width from integrated system info table, if available,
> + * or 0 if not.
> + */
> +int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev)
> +{
> +	struct amdgpu_mode_info *mode_info = &adev->mode_info;
> +	int index = GetIndexIntoMasterTable(DATA, IntegratedSystemInfo);
> +	u16 data_offset, size;
> +	union igp_info *igp_info;
> +	u8 frev, crev;
> +
> +	/* get any igp specific overrides */
> +	if (amdgpu_atom_parse_data_header(mode_info->atom_context, index, &size,
> +				   &frev, &crev, &data_offset)) {
> +		igp_info = (union igp_info *)
> +			(mode_info->atom_context->bios + data_offset);
> +		switch (crev) {
> +		case 8:
> +		case 9:
> +			return igp_info->info_8.ucUMAChannelNumber * 64;
> +		default:
> +			return 0;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static void amdgpu_atombios_get_igp_ss_overrides(struct amdgpu_device *adev,
>  						 struct amdgpu_atom_ss *ss,
>  						 int id)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> index 4e0f488487f3..38d0fe32e5cd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> @@ -148,6 +148,8 @@ int amdgpu_atombios_get_clock_info(struct amdgpu_device *adev);
>  
>  int amdgpu_atombios_get_gfx_info(struct amdgpu_device *adev);
>  
> +int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev);
> +
>  bool amdgpu_atombios_get_asic_ss_info(struct amdgpu_device *adev,
>  				      struct amdgpu_atom_ss *ss,
>  				      int id, u32 clock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 0c0a6015cca5..78643a1baa5c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -37,6 +37,8 @@
>  #include "oss/oss_2_0_d.h"
>  #include "oss/oss_2_0_sh_mask.h"
>  
> +#include "amdgpu_atombios.h"
> +
>  static void gmc_v7_0_set_gart_funcs(struct amdgpu_device *adev);
>  static void gmc_v7_0_set_irq_funcs(struct amdgpu_device *adev);
>  static int gmc_v7_0_wait_for_idle(void *handle);
> @@ -325,48 +327,51 @@ static void gmc_v7_0_mc_program(struct amdgpu_device *adev)
>   */
>  static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
>  {
> -	u32 tmp;
> -	int chansize, numchan;
> -
> -	/* Get VRAM informations */
> -	tmp = RREG32(mmMC_ARB_RAMCFG);
> -	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> -		chansize = 64;
> -	} else {
> -		chansize = 32;
> -	}
> -	tmp = RREG32(mmMC_SHARED_CHMAP);
> -	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
> -	case 0:
> -	default:
> -		numchan = 1;
> -		break;
> -	case 1:
> -		numchan = 2;
> -		break;
> -	case 2:
> -		numchan = 4;
> -		break;
> -	case 3:
> -		numchan = 8;
> -		break;
> -	case 4:
> -		numchan = 3;
> -		break;
> -	case 5:
> -		numchan = 6;
> -		break;
> -	case 6:
> -		numchan = 10;
> -		break;
> -	case 7:
> -		numchan = 12;
> -		break;
> -	case 8:
> -		numchan = 16;
> -		break;
> +	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
> +	if (!adev->mc.vram_width) {
> +		u32 tmp;
> +		int chansize, numchan;
> +
> +		/* Get VRAM informations */
> +		tmp = RREG32(mmMC_ARB_RAMCFG);
> +		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> +			chansize = 64;
> +		} else {
> +			chansize = 32;
> +		}
> +		tmp = RREG32(mmMC_SHARED_CHMAP);
> +		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
> +		case 0:
> +		default:
> +			numchan = 1;
> +			break;
> +		case 1:
> +			numchan = 2;
> +			break;
> +		case 2:
> +			numchan = 4;
> +			break;
> +		case 3:
> +			numchan = 8;
> +			break;
> +		case 4:
> +			numchan = 3;
> +			break;
> +		case 5:
> +			numchan = 6;
> +			break;
> +		case 6:
> +			numchan = 10;
> +			break;
> +		case 7:
> +			numchan = 12;
> +			break;
> +		case 8:
> +			numchan = 16;
> +			break;
> +		}
> +		adev->mc.vram_width = numchan * chansize;
>  	}
> -	adev->mc.vram_width = numchan * chansize;
>  	/* Could aper size report 0 ? */
>  	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>  	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index d19d1c5e2847..42b2f357a799 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -38,6 +38,8 @@
>  #include "vid.h"
>  #include "vi.h"
>  
> +#include "amdgpu_atombios.h"
> +
>  
>  static void gmc_v8_0_set_gart_funcs(struct amdgpu_device *adev);
>  static void gmc_v8_0_set_irq_funcs(struct amdgpu_device *adev);
> @@ -487,48 +489,51 @@ static void gmc_v8_0_mc_program(struct amdgpu_device *adev)
>   */
>  static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
>  {
> -	u32 tmp;
> -	int chansize, numchan;
> -
> -	/* Get VRAM informations */
> -	tmp = RREG32(mmMC_ARB_RAMCFG);
> -	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> -		chansize = 64;
> -	} else {
> -		chansize = 32;
> -	}
> -	tmp = RREG32(mmMC_SHARED_CHMAP);
> -	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
> -	case 0:
> -	default:
> -		numchan = 1;
> -		break;
> -	case 1:
> -		numchan = 2;
> -		break;
> -	case 2:
> -		numchan = 4;
> -		break;
> -	case 3:
> -		numchan = 8;
> -		break;
> -	case 4:
> -		numchan = 3;
> -		break;
> -	case 5:
> -		numchan = 6;
> -		break;
> -	case 6:
> -		numchan = 10;
> -		break;
> -	case 7:
> -		numchan = 12;
> -		break;
> -	case 8:
> -		numchan = 16;
> -		break;
> +	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
> +	if (!adev->mc.vram_width) {
> +		u32 tmp;
> +		int chansize, numchan;
> +
> +		/* Get VRAM informations */
> +		tmp = RREG32(mmMC_ARB_RAMCFG);
> +		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> +			chansize = 64;
> +		} else {
> +			chansize = 32;
> +		}
> +		tmp = RREG32(mmMC_SHARED_CHMAP);
> +		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
> +		case 0:
> +		default:
> +			numchan = 1;
> +			break;
> +		case 1:
> +			numchan = 2;
> +			break;
> +		case 2:
> +			numchan = 4;
> +			break;
> +		case 3:
> +			numchan = 8;
> +			break;
> +		case 4:
> +			numchan = 3;
> +			break;
> +		case 5:
> +			numchan = 6;
> +			break;
> +		case 6:
> +			numchan = 10;
> +			break;
> +		case 7:
> +			numchan = 12;
> +			break;
> +		case 8:
> +			numchan = 16;
> +			break;
> +		}
> +		adev->mc.vram_width = numchan * chansize;

I might miss something subtle here, but after reading this a couple of times,
I've convinced myself this is exactly the same switch statement as in
gmc_v7_0_mc_init() above, right? If so: why not move that part to common code as
well?

Cheers,
Kai


>  	}
> -	adev->mc.vram_width = numchan * chansize;
>  	/* Could aper size report 0 ? */
>  	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>  	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

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

Re: [PATCH v2] drm/amdgpu: Read vram width from integrated system info table

[Harry Wentland]

On 2017-04-03 11:20 AM, Kai Wasserbäch wrote:
> Dear Harry,
> Harry Wentland wrote on 03.04.2017 17:01:
>> On KB, KV, CZ we should read the vram width from integrated system
>> table, if we can. The NOOFCHAN in MC_SHARED_CHMAP is not accurate.
>>
>> With this change we can enable two 4k displays on CZ again. This use
>> case was broken sometime in January when we started looking at
>> vram_width for bandwidth calculations instead of hardcoding this value.
>>
>> v2:
>>   Return 0 if integrated system info table is not available.
>>
>> Tested-by: Roman Li <roman.li@amd.com>
>> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 29 ++++++++++
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h |  2 +
>>  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c        | 87 +++++++++++++++-------------
>>  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c        | 87 +++++++++++++++-------------
>>  4 files changed, 123 insertions(+), 82 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
>> index f52b1bf3d3d9..ad4329922f79 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
>> @@ -754,6 +754,35 @@ union igp_info {
>>  	struct _ATOM_INTEGRATED_SYSTEM_INFO_V1_9 info_9;
>>  };
>>
>> +/*
>> + * Return vram width from integrated system info table, if available,
>> + * or 0 if not.
>> + */
>> +int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev)
>> +{
>> +	struct amdgpu_mode_info *mode_info = &adev->mode_info;
>> +	int index = GetIndexIntoMasterTable(DATA, IntegratedSystemInfo);
>> +	u16 data_offset, size;
>> +	union igp_info *igp_info;
>> +	u8 frev, crev;
>> +
>> +	/* get any igp specific overrides */
>> +	if (amdgpu_atom_parse_data_header(mode_info->atom_context, index, &size,
>> +				   &frev, &crev, &data_offset)) {
>> +		igp_info = (union igp_info *)
>> +			(mode_info->atom_context->bios + data_offset);
>> +		switch (crev) {
>> +		case 8:
>> +		case 9:
>> +			return igp_info->info_8.ucUMAChannelNumber * 64;
>> +		default:
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static void amdgpu_atombios_get_igp_ss_overrides(struct amdgpu_device *adev,
>>  						 struct amdgpu_atom_ss *ss,
>>  						 int id)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
>> index 4e0f488487f3..38d0fe32e5cd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
>> @@ -148,6 +148,8 @@ int amdgpu_atombios_get_clock_info(struct amdgpu_device *adev);
>>
>>  int amdgpu_atombios_get_gfx_info(struct amdgpu_device *adev);
>>
>> +int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev);
>> +
>>  bool amdgpu_atombios_get_asic_ss_info(struct amdgpu_device *adev,
>>  				      struct amdgpu_atom_ss *ss,
>>  				      int id, u32 clock);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index 0c0a6015cca5..78643a1baa5c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -37,6 +37,8 @@
>>  #include "oss/oss_2_0_d.h"
>>  #include "oss/oss_2_0_sh_mask.h"
>>
>> +#include "amdgpu_atombios.h"
>> +
>>  static void gmc_v7_0_set_gart_funcs(struct amdgpu_device *adev);
>>  static void gmc_v7_0_set_irq_funcs(struct amdgpu_device *adev);
>>  static int gmc_v7_0_wait_for_idle(void *handle);
>> @@ -325,48 +327,51 @@ static void gmc_v7_0_mc_program(struct amdgpu_device *adev)
>>   */
>>  static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
>>  {
>> -	u32 tmp;
>> -	int chansize, numchan;
>> -
>> -	/* Get VRAM informations */
>> -	tmp = RREG32(mmMC_ARB_RAMCFG);
>> -	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
>> -		chansize = 64;
>> -	} else {
>> -		chansize = 32;
>> -	}
>> -	tmp = RREG32(mmMC_SHARED_CHMAP);
>> -	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
>> -	case 0:
>> -	default:
>> -		numchan = 1;
>> -		break;
>> -	case 1:
>> -		numchan = 2;
>> -		break;
>> -	case 2:
>> -		numchan = 4;
>> -		break;
>> -	case 3:
>> -		numchan = 8;
>> -		break;
>> -	case 4:
>> -		numchan = 3;
>> -		break;
>> -	case 5:
>> -		numchan = 6;
>> -		break;
>> -	case 6:
>> -		numchan = 10;
>> -		break;
>> -	case 7:
>> -		numchan = 12;
>> -		break;
>> -	case 8:
>> -		numchan = 16;
>> -		break;
>> +	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
>> +	if (!adev->mc.vram_width) {
>> +		u32 tmp;
>> +		int chansize, numchan;
>> +
>> +		/* Get VRAM informations */
>> +		tmp = RREG32(mmMC_ARB_RAMCFG);
>> +		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
>> +			chansize = 64;
>> +		} else {
>> +			chansize = 32;
>> +		}
>> +		tmp = RREG32(mmMC_SHARED_CHMAP);
>> +		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
>> +		case 0:
>> +		default:
>> +			numchan = 1;
>> +			break;
>> +		case 1:
>> +			numchan = 2;
>> +			break;
>> +		case 2:
>> +			numchan = 4;
>> +			break;
>> +		case 3:
>> +			numchan = 8;
>> +			break;
>> +		case 4:
>> +			numchan = 3;
>> +			break;
>> +		case 5:
>> +			numchan = 6;
>> +			break;
>> +		case 6:
>> +			numchan = 10;
>> +			break;
>> +		case 7:
>> +			numchan = 12;
>> +			break;
>> +		case 8:
>> +			numchan = 16;
>> +			break;
>> +		}
>> +		adev->mc.vram_width = numchan * chansize;
>>  	}
>> -	adev->mc.vram_width = numchan * chansize;
>>  	/* Could aper size report 0 ? */
>>  	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>  	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index d19d1c5e2847..42b2f357a799 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -38,6 +38,8 @@
>>  #include "vid.h"
>>  #include "vi.h"
>>
>> +#include "amdgpu_atombios.h"
>> +
>>
>>  static void gmc_v8_0_set_gart_funcs(struct amdgpu_device *adev);
>>  static void gmc_v8_0_set_irq_funcs(struct amdgpu_device *adev);
>> @@ -487,48 +489,51 @@ static void gmc_v8_0_mc_program(struct amdgpu_device *adev)
>>   */
>>  static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
>>  {
>> -	u32 tmp;
>> -	int chansize, numchan;
>> -
>> -	/* Get VRAM informations */
>> -	tmp = RREG32(mmMC_ARB_RAMCFG);
>> -	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
>> -		chansize = 64;
>> -	} else {
>> -		chansize = 32;
>> -	}
>> -	tmp = RREG32(mmMC_SHARED_CHMAP);
>> -	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
>> -	case 0:
>> -	default:
>> -		numchan = 1;
>> -		break;
>> -	case 1:
>> -		numchan = 2;
>> -		break;
>> -	case 2:
>> -		numchan = 4;
>> -		break;
>> -	case 3:
>> -		numchan = 8;
>> -		break;
>> -	case 4:
>> -		numchan = 3;
>> -		break;
>> -	case 5:
>> -		numchan = 6;
>> -		break;
>> -	case 6:
>> -		numchan = 10;
>> -		break;
>> -	case 7:
>> -		numchan = 12;
>> -		break;
>> -	case 8:
>> -		numchan = 16;
>> -		break;
>> +	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
>> +	if (!adev->mc.vram_width) {
>> +		u32 tmp;
>> +		int chansize, numchan;
>> +
>> +		/* Get VRAM informations */
>> +		tmp = RREG32(mmMC_ARB_RAMCFG);
>> +		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
>> +			chansize = 64;
>> +		} else {
>> +			chansize = 32;
>> +		}
>> +		tmp = RREG32(mmMC_SHARED_CHMAP);
>> +		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
>> +		case 0:
>> +		default:
>> +			numchan = 1;
>> +			break;
>> +		case 1:
>> +			numchan = 2;
>> +			break;
>> +		case 2:
>> +			numchan = 4;
>> +			break;
>> +		case 3:
>> +			numchan = 8;
>> +			break;
>> +		case 4:
>> +			numchan = 3;
>> +			break;
>> +		case 5:
>> +			numchan = 6;
>> +			break;
>> +		case 6:
>> +			numchan = 10;
>> +			break;
>> +		case 7:
>> +			numchan = 12;
>> +			break;
>> +		case 8:
>> +			numchan = 16;
>> +			break;
>> +		}
>> +		adev->mc.vram_width = numchan * chansize;
>
> I might miss something subtle here, but after reading this a couple of times,
> I've convinced myself this is exactly the same switch statement as in
> gmc_v7_0_mc_init() above, right? If so: why not move that part to common code as
> well?
>

My patch isn't adding (or removing) any duplication, just wrapping the 
whole thing in a call to atombios.

I'd rather not change the design of the GMC component with this patch. 
It could very well have been a design decisions to have this 
duplication, in part because of the register access. mmMC_ARB_RAMCFG 
could be defined differently for v7 and v8. The different defines are 
pulled in through different register header includes, gmc_7_1_d.h vs 
gmc/gmc_8_1_d.h.

Harry

> Cheers,
> Kai
>
>
>>  	}
>> -	adev->mc.vram_width = numchan * chansize;
>>  	/* Could aper size report 0 ? */
>>  	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>>  	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH v2] drm/amdgpu: Read vram width from integrated system info table

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Harry Wentland
> Sent: Monday, April 03, 2017 11:01 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Wentland, Harry
> Subject: [PATCH v2] drm/amdgpu: Read vram width from integrated system
> info table
> 
> On KB, KV, CZ we should read the vram width from integrated system
> table, if we can. The NOOFCHAN in MC_SHARED_CHMAP is not accurate.
> 
> With this change we can enable two 4k displays on CZ again. This use
> case was broken sometime in January when we started looking at
> vram_width for bandwidth calculations instead of hardcoding this value.
> 
> v2:
>   Return 0 if integrated system info table is not available.
> 
> Tested-by: Roman Li <roman.li@amd.com>
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 29 ++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h |  2 +
>  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c        | 87 +++++++++++++++-----
> --------
>  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c        | 87 +++++++++++++++-----
> --------
>  4 files changed, 123 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> index f52b1bf3d3d9..ad4329922f79 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> @@ -754,6 +754,35 @@ union igp_info {
>  	struct _ATOM_INTEGRATED_SYSTEM_INFO_V1_9 info_9;
>  };
> 
> +/*
> + * Return vram width from integrated system info table, if available,
> + * or 0 if not.
> + */
> +int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev)
> +{
> +	struct amdgpu_mode_info *mode_info = &adev->mode_info;
> +	int index = GetIndexIntoMasterTable(DATA, IntegratedSystemInfo);
> +	u16 data_offset, size;
> +	union igp_info *igp_info;
> +	u8 frev, crev;
> +
> +	/* get any igp specific overrides */
> +	if (amdgpu_atom_parse_data_header(mode_info->atom_context,
> index, &size,
> +				   &frev, &crev, &data_offset)) {
> +		igp_info = (union igp_info *)
> +			(mode_info->atom_context->bios + data_offset);
> +		switch (crev) {
> +		case 8:
> +		case 9:
> +			return igp_info->info_8.ucUMAChannelNumber *
> 64;
> +		default:
> +			return 0;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static void amdgpu_atombios_get_igp_ss_overrides(struct amdgpu_device
> *adev,
>  						 struct amdgpu_atom_ss *ss,
>  						 int id)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> index 4e0f488487f3..38d0fe32e5cd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> @@ -148,6 +148,8 @@ int amdgpu_atombios_get_clock_info(struct
> amdgpu_device *adev);
> 
>  int amdgpu_atombios_get_gfx_info(struct amdgpu_device *adev);
> 
> +int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev);
> +
>  bool amdgpu_atombios_get_asic_ss_info(struct amdgpu_device *adev,
>  				      struct amdgpu_atom_ss *ss,
>  				      int id, u32 clock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 0c0a6015cca5..78643a1baa5c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -37,6 +37,8 @@
>  #include "oss/oss_2_0_d.h"
>  #include "oss/oss_2_0_sh_mask.h"
> 
> +#include "amdgpu_atombios.h"
> +
>  static void gmc_v7_0_set_gart_funcs(struct amdgpu_device *adev);
>  static void gmc_v7_0_set_irq_funcs(struct amdgpu_device *adev);
>  static int gmc_v7_0_wait_for_idle(void *handle);
> @@ -325,48 +327,51 @@ static void gmc_v7_0_mc_program(struct
> amdgpu_device *adev)
>   */
>  static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
>  {
> -	u32 tmp;
> -	int chansize, numchan;
> -
> -	/* Get VRAM informations */
> -	tmp = RREG32(mmMC_ARB_RAMCFG);
> -	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> -		chansize = 64;
> -	} else {
> -		chansize = 32;
> -	}
> -	tmp = RREG32(mmMC_SHARED_CHMAP);
> -	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
> -	case 0:
> -	default:
> -		numchan = 1;
> -		break;
> -	case 1:
> -		numchan = 2;
> -		break;
> -	case 2:
> -		numchan = 4;
> -		break;
> -	case 3:
> -		numchan = 8;
> -		break;
> -	case 4:
> -		numchan = 3;
> -		break;
> -	case 5:
> -		numchan = 6;
> -		break;
> -	case 6:
> -		numchan = 10;
> -		break;
> -	case 7:
> -		numchan = 12;
> -		break;
> -	case 8:
> -		numchan = 16;
> -		break;
> +	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
> +	if (!adev->mc.vram_width) {
> +		u32 tmp;
> +		int chansize, numchan;
> +
> +		/* Get VRAM informations */
> +		tmp = RREG32(mmMC_ARB_RAMCFG);
> +		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> +			chansize = 64;
> +		} else {
> +			chansize = 32;
> +		}
> +		tmp = RREG32(mmMC_SHARED_CHMAP);
> +		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP,
> NOOFCHAN)) {
> +		case 0:
> +		default:
> +			numchan = 1;
> +			break;
> +		case 1:
> +			numchan = 2;
> +			break;
> +		case 2:
> +			numchan = 4;
> +			break;
> +		case 3:
> +			numchan = 8;
> +			break;
> +		case 4:
> +			numchan = 3;
> +			break;
> +		case 5:
> +			numchan = 6;
> +			break;
> +		case 6:
> +			numchan = 10;
> +			break;
> +		case 7:
> +			numchan = 12;
> +			break;
> +		case 8:
> +			numchan = 16;
> +			break;
> +		}
> +		adev->mc.vram_width = numchan * chansize;
>  	}
> -	adev->mc.vram_width = numchan * chansize;
>  	/* Could aper size report 0 ? */
>  	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>  	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index d19d1c5e2847..42b2f357a799 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -38,6 +38,8 @@
>  #include "vid.h"
>  #include "vi.h"
> 
> +#include "amdgpu_atombios.h"
> +
> 
>  static void gmc_v8_0_set_gart_funcs(struct amdgpu_device *adev);
>  static void gmc_v8_0_set_irq_funcs(struct amdgpu_device *adev);
> @@ -487,48 +489,51 @@ static void gmc_v8_0_mc_program(struct
> amdgpu_device *adev)
>   */
>  static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
>  {
> -	u32 tmp;
> -	int chansize, numchan;
> -
> -	/* Get VRAM informations */
> -	tmp = RREG32(mmMC_ARB_RAMCFG);
> -	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> -		chansize = 64;
> -	} else {
> -		chansize = 32;
> -	}
> -	tmp = RREG32(mmMC_SHARED_CHMAP);
> -	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
> -	case 0:
> -	default:
> -		numchan = 1;
> -		break;
> -	case 1:
> -		numchan = 2;
> -		break;
> -	case 2:
> -		numchan = 4;
> -		break;
> -	case 3:
> -		numchan = 8;
> -		break;
> -	case 4:
> -		numchan = 3;
> -		break;
> -	case 5:
> -		numchan = 6;
> -		break;
> -	case 6:
> -		numchan = 10;
> -		break;
> -	case 7:
> -		numchan = 12;
> -		break;
> -	case 8:
> -		numchan = 16;
> -		break;
> +	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
> +	if (!adev->mc.vram_width) {
> +		u32 tmp;
> +		int chansize, numchan;
> +
> +		/* Get VRAM informations */
> +		tmp = RREG32(mmMC_ARB_RAMCFG);
> +		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> +			chansize = 64;
> +		} else {
> +			chansize = 32;
> +		}
> +		tmp = RREG32(mmMC_SHARED_CHMAP);
> +		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP,
> NOOFCHAN)) {
> +		case 0:
> +		default:
> +			numchan = 1;
> +			break;
> +		case 1:
> +			numchan = 2;
> +			break;
> +		case 2:
> +			numchan = 4;
> +			break;
> +		case 3:
> +			numchan = 8;
> +			break;
> +		case 4:
> +			numchan = 3;
> +			break;
> +		case 5:
> +			numchan = 6;
> +			break;
> +		case 6:
> +			numchan = 10;
> +			break;
> +		case 7:
> +			numchan = 12;
> +			break;
> +		case 8:
> +			numchan = 16;
> +			break;
> +		}
> +		adev->mc.vram_width = numchan * chansize;
>  	}
> -	adev->mc.vram_width = numchan * chansize;
>  	/* Could aper size report 0 ? */
>  	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
>  	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> --
> 2.11.0
> 
> _______________________________________________
> 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 v2] drm/amdgpu: Read vram width from integrated system info table

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Kai Wasserbäch
> Sent: Monday, April 03, 2017 11:20 AM
> To: Wentland, Harry; amd-gfx list
> Subject: Re: [PATCH v2] drm/amdgpu: Read vram width from integrated
> system info table
> 
> Dear Harry,
> Harry Wentland wrote on 03.04.2017 17:01:
> > On KB, KV, CZ we should read the vram width from integrated system
> > table, if we can. The NOOFCHAN in MC_SHARED_CHMAP is not accurate.
> >
> > With this change we can enable two 4k displays on CZ again. This use
> > case was broken sometime in January when we started looking at
> > vram_width for bandwidth calculations instead of hardcoding this value.
> >
> > v2:
> >   Return 0 if integrated system info table is not available.
> >
> > Tested-by: Roman Li <roman.li@amd.com>
> > Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 29 ++++++++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h |  2 +
> >  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c        | 87 +++++++++++++++---
> ----------
> >  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c        | 87 +++++++++++++++---
> ----------
> >  4 files changed, 123 insertions(+), 82 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> > index f52b1bf3d3d9..ad4329922f79 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> > @@ -754,6 +754,35 @@ union igp_info {
> >  	struct _ATOM_INTEGRATED_SYSTEM_INFO_V1_9 info_9;
> >  };
> >
> > +/*
> > + * Return vram width from integrated system info table, if available,
> > + * or 0 if not.
> > + */
> > +int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev)
> > +{
> > +	struct amdgpu_mode_info *mode_info = &adev->mode_info;
> > +	int index = GetIndexIntoMasterTable(DATA, IntegratedSystemInfo);
> > +	u16 data_offset, size;
> > +	union igp_info *igp_info;
> > +	u8 frev, crev;
> > +
> > +	/* get any igp specific overrides */
> > +	if (amdgpu_atom_parse_data_header(mode_info->atom_context,
> index, &size,
> > +				   &frev, &crev, &data_offset)) {
> > +		igp_info = (union igp_info *)
> > +			(mode_info->atom_context->bios + data_offset);
> > +		switch (crev) {
> > +		case 8:
> > +		case 9:
> > +			return igp_info->info_8.ucUMAChannelNumber *
> 64;
> > +		default:
> > +			return 0;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static void amdgpu_atombios_get_igp_ss_overrides(struct
> amdgpu_device *adev,
> >  						 struct amdgpu_atom_ss *ss,
> >  						 int id)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> > index 4e0f488487f3..38d0fe32e5cd 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.h
> > @@ -148,6 +148,8 @@ int amdgpu_atombios_get_clock_info(struct
> amdgpu_device *adev);
> >
> >  int amdgpu_atombios_get_gfx_info(struct amdgpu_device *adev);
> >
> > +int amdgpu_atombios_get_vram_width(struct amdgpu_device *adev);
> > +
> >  bool amdgpu_atombios_get_asic_ss_info(struct amdgpu_device *adev,
> >  				      struct amdgpu_atom_ss *ss,
> >  				      int id, u32 clock);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > index 0c0a6015cca5..78643a1baa5c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > @@ -37,6 +37,8 @@
> >  #include "oss/oss_2_0_d.h"
> >  #include "oss/oss_2_0_sh_mask.h"
> >
> > +#include "amdgpu_atombios.h"
> > +
> >  static void gmc_v7_0_set_gart_funcs(struct amdgpu_device *adev);
> >  static void gmc_v7_0_set_irq_funcs(struct amdgpu_device *adev);
> >  static int gmc_v7_0_wait_for_idle(void *handle);
> > @@ -325,48 +327,51 @@ static void gmc_v7_0_mc_program(struct
> amdgpu_device *adev)
> >   */
> >  static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
> >  {
> > -	u32 tmp;
> > -	int chansize, numchan;
> > -
> > -	/* Get VRAM informations */
> > -	tmp = RREG32(mmMC_ARB_RAMCFG);
> > -	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> > -		chansize = 64;
> > -	} else {
> > -		chansize = 32;
> > -	}
> > -	tmp = RREG32(mmMC_SHARED_CHMAP);
> > -	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
> > -	case 0:
> > -	default:
> > -		numchan = 1;
> > -		break;
> > -	case 1:
> > -		numchan = 2;
> > -		break;
> > -	case 2:
> > -		numchan = 4;
> > -		break;
> > -	case 3:
> > -		numchan = 8;
> > -		break;
> > -	case 4:
> > -		numchan = 3;
> > -		break;
> > -	case 5:
> > -		numchan = 6;
> > -		break;
> > -	case 6:
> > -		numchan = 10;
> > -		break;
> > -	case 7:
> > -		numchan = 12;
> > -		break;
> > -	case 8:
> > -		numchan = 16;
> > -		break;
> > +	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
> > +	if (!adev->mc.vram_width) {
> > +		u32 tmp;
> > +		int chansize, numchan;
> > +
> > +		/* Get VRAM informations */
> > +		tmp = RREG32(mmMC_ARB_RAMCFG);
> > +		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> > +			chansize = 64;
> > +		} else {
> > +			chansize = 32;
> > +		}
> > +		tmp = RREG32(mmMC_SHARED_CHMAP);
> > +		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP,
> NOOFCHAN)) {
> > +		case 0:
> > +		default:
> > +			numchan = 1;
> > +			break;
> > +		case 1:
> > +			numchan = 2;
> > +			break;
> > +		case 2:
> > +			numchan = 4;
> > +			break;
> > +		case 3:
> > +			numchan = 8;
> > +			break;
> > +		case 4:
> > +			numchan = 3;
> > +			break;
> > +		case 5:
> > +			numchan = 6;
> > +			break;
> > +		case 6:
> > +			numchan = 10;
> > +			break;
> > +		case 7:
> > +			numchan = 12;
> > +			break;
> > +		case 8:
> > +			numchan = 16;
> > +			break;
> > +		}
> > +		adev->mc.vram_width = numchan * chansize;
> >  	}
> > -	adev->mc.vram_width = numchan * chansize;
> >  	/* Could aper size report 0 ? */
> >  	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >  	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > index d19d1c5e2847..42b2f357a799 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > @@ -38,6 +38,8 @@
> >  #include "vid.h"
> >  #include "vi.h"
> >
> > +#include "amdgpu_atombios.h"
> > +
> >
> >  static void gmc_v8_0_set_gart_funcs(struct amdgpu_device *adev);
> >  static void gmc_v8_0_set_irq_funcs(struct amdgpu_device *adev);
> > @@ -487,48 +489,51 @@ static void gmc_v8_0_mc_program(struct
> amdgpu_device *adev)
> >   */
> >  static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
> >  {
> > -	u32 tmp;
> > -	int chansize, numchan;
> > -
> > -	/* Get VRAM informations */
> > -	tmp = RREG32(mmMC_ARB_RAMCFG);
> > -	if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> > -		chansize = 64;
> > -	} else {
> > -		chansize = 32;
> > -	}
> > -	tmp = RREG32(mmMC_SHARED_CHMAP);
> > -	switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP, NOOFCHAN)) {
> > -	case 0:
> > -	default:
> > -		numchan = 1;
> > -		break;
> > -	case 1:
> > -		numchan = 2;
> > -		break;
> > -	case 2:
> > -		numchan = 4;
> > -		break;
> > -	case 3:
> > -		numchan = 8;
> > -		break;
> > -	case 4:
> > -		numchan = 3;
> > -		break;
> > -	case 5:
> > -		numchan = 6;
> > -		break;
> > -	case 6:
> > -		numchan = 10;
> > -		break;
> > -	case 7:
> > -		numchan = 12;
> > -		break;
> > -	case 8:
> > -		numchan = 16;
> > -		break;
> > +	adev->mc.vram_width = amdgpu_atombios_get_vram_width(adev);
> > +	if (!adev->mc.vram_width) {
> > +		u32 tmp;
> > +		int chansize, numchan;
> > +
> > +		/* Get VRAM informations */
> > +		tmp = RREG32(mmMC_ARB_RAMCFG);
> > +		if (REG_GET_FIELD(tmp, MC_ARB_RAMCFG, CHANSIZE)) {
> > +			chansize = 64;
> > +		} else {
> > +			chansize = 32;
> > +		}
> > +		tmp = RREG32(mmMC_SHARED_CHMAP);
> > +		switch (REG_GET_FIELD(tmp, MC_SHARED_CHMAP,
> NOOFCHAN)) {
> > +		case 0:
> > +		default:
> > +			numchan = 1;
> > +			break;
> > +		case 1:
> > +			numchan = 2;
> > +			break;
> > +		case 2:
> > +			numchan = 4;
> > +			break;
> > +		case 3:
> > +			numchan = 8;
> > +			break;
> > +		case 4:
> > +			numchan = 3;
> > +			break;
> > +		case 5:
> > +			numchan = 6;
> > +			break;
> > +		case 6:
> > +			numchan = 10;
> > +			break;
> > +		case 7:
> > +			numchan = 12;
> > +			break;
> > +		case 8:
> > +			numchan = 16;
> > +			break;
> > +		}
> > +		adev->mc.vram_width = numchan * chansize;
> 
> I might miss something subtle here, but after reading this a couple of times,
> I've convinced myself this is exactly the same switch statement as in
> gmc_v7_0_mc_init() above, right? If so: why not move that part to common
> code as
> well?

The register offsets and bitfields may change across different IP revisions.  The actual switch statement itself could be common I guess (minus the registers), but I'm not sure if it's worth the effort.

Alex

> 
> Cheers,
> Kai
> 
> 
> >  	}
> > -	adev->mc.vram_width = numchan * chansize;
> >  	/* Could aper size report 0 ? */
> >  	adev->mc.aper_base = pci_resource_start(adev->pdev, 0);
> >  	adev->mc.aper_size = pci_resource_len(adev->pdev, 0);
> 

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

Re: [PATCH v2] drm/amdgpu: Read vram width from integrated system info table

[Kai Wasserbäch]

[-- Attachment #1.1.1: Type: text/plain, Size: 1171 bytes --]

Deucher, Alexander wrote on 03.04.2017 17:43:
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] On Behalf
>> Of Kai Wasserbäch
>> Sent: Monday, April 03, 2017 11:20 AM
>> To: Wentland, Harry; amd-gfx list
>> Subject: Re: [PATCH v2] drm/amdgpu: Read vram width from integrated
>> system info table
>>
>> [...]
>>
>> I might miss something subtle here, but after reading this a couple of times,
>> I've convinced myself this is exactly the same switch statement as in
>> gmc_v7_0_mc_init() above, right? If so: why not move that part to common
>> code as
>> well?
> 
> The register offsets and bitfields may change across different IP revisions.  The actual switch statement itself could be common I guess (minus the registers), but I'm not sure if it's worth the effort.

Thanks to both of you for the explanation. Since everything was named exactly
the same I assumed the definitions for the registers matched as well. Still if
large chunks of code are common helpers might still be nice and reduce the
overall LOCs. Anyway, that's obviously your decision.

Cheers,
Kai


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

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