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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 at amd.com>
>> Signed-off-by: Harry Wentland <harry.wentland at 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);
>
>


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux