[PATCH 06/10] drm/amdgpu: add amdgpu_gmc_agp_location

[Christian König]

Am 27.08.2018 um 20:50 schrieb Felix Kuehling:
> On 2018-08-27 12:53 PM, Christian König wrote:
>> Helper to figure out the location of the AGP BAR.
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 42 +++++++++++++++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++
>>   2 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> index 4331a0e25cdc..eed5352f3136 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>> @@ -141,3 +141,45 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>>   	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
>>   			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
>>   }
>> +
>> +/**
>> + * amdgpu_gmc_agp_location - try to find AGP location
>> + * @adev: amdgpu device structure holding all necessary informations
>> + * @mc: memory controller structure holding memory informations
>> + *
>> + * Function will place try to fina a place for the AGP BAR in the MC address
> s/fina/find
>
>> + * space.
>> + *
>> + * AGP BAR will be assigned the largest available hole in the address space.
> I'd add a comment that this function must be called after
> amdgpu_gmc_vram_location and amdgpu_gmc_gart_location.
>
>> + */
>> +void amdgpu_gmc_agp_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc)
>> +{
>> +	const uint64_t sixteen_gb = 1ULL << 34;
>> +	u64 size_af, size_bf;
>> +
>> +	if (mc->vram_start > mc->gart_start) {
>> +		size_bf = mc->vram_start - mc->gart_end + 1;
>> +		size_af = mc->mc_mask - mc->vram_end;
>> +	} else {
>> +		size_bf = mc->vram_start;
>> +		size_af = mc->mc_mask - mc->gart_end;
>> +	}
>> +
>> +	size_bf &= ~(sixteen_gb - 1);
>> +	size_af &= ~(sixteen_gb - 1);
> This is not correct. E.g. vram_end  = 12GB, gart_start = 28GB, size =
> 16GB. agp_start will be rounded up to 16GB and AGP will run into the
> GART aperture.
>
> You need to align the addresses before calculating the sizes.

Ah, crap you are right. Going to fix this.

I'm really wondering if we shouldn't use "struct resource" for that 
handling?

This will only get more complicated in the future when even more 
apertures are enabled.

Alex what do you think?

Regards,
Christian.

>
> Regards,
>    Felix
>
>> +
>> +	if (size_bf > size_af) {
>> +		mc->agp_start = mc->vram_start > mc->gart_start ?
>> +			mc->gart_start : 0;
>> +		mc->agp_size = size_bf;
>> +	} else {
>> +		mc->agp_start = (mc->vram_start > mc->gart_start ?
>> +			mc->vram_end : mc->gart_end) + 1,
>> +		mc->agp_size = size_af;
>> +	}
>> +
>> +	mc->agp_start = ALIGN(mc->agp_start, sixteen_gb);
>> +	mc->agp_end = mc->agp_start + mc->agp_size - 1;
>> +	dev_info(adev->dev, "AGP: %lluM 0x%016llX - 0x%016llX\n",
>> +			mc->agp_size >> 20, mc->agp_start, mc->agp_end);
>> +}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> index 72fcc9338f5e..163110fe375d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> @@ -81,6 +81,9 @@ struct amdgpu_gmc {
>>   	 * about vram size near mc fb location */
>>   	u64			mc_vram_size;
>>   	u64			visible_vram_size;
>> +	u64			agp_size;
>> +	u64			agp_start;
>> +	u64			agp_end;
>>   	u64			gart_size;
>>   	u64			gart_start;
>>   	u64			gart_end;
>> @@ -138,5 +141,7 @@ void amdgpu_gmc_vram_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
>>   			      u64 base);
>>   void amdgpu_gmc_gart_location(struct amdgpu_device *adev,
>>   			      struct amdgpu_gmc *mc);
>> +void amdgpu_gmc_agp_location(struct amdgpu_device *adev,
>> +			     struct amdgpu_gmc *mc);
>>   
>>   #endif
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx