[PATCH 1/8] drm/amdgpu: change MMHUB register access from MMIO to RLCG

Felix Kuehling felix.kuehling at amd.com
Thu Apr 15 15:47:38 UTC 2021


Am 2021-04-15 um 3:25 a.m. schrieb Zhou, Peng Ju:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hi Felix
> Thanks for your proposal about " Given the number of call-sites being modified in this patch series"
> We discussed internally, and have following concern:
> 	1. expose our ranges to opensource.

Someone modifying our code will need to know which register access macro
to use. So this information needs to be available in some form. If you
try to hide it, people will break stuff by using the wrong register
access macro. If the information is not easily available, even AMD
engineers will break this.


> 	2. lost the orthogonality in our software stack.

Can you elaborate what you mean by "orthogonality"?

Thanks,
  Felix


> So we want to keep our original workaround.
>
> Do you agree?
>
>
> ---------------------------------------------------------------------- 
> BW
> Pengju Zhou
>
>
>
> -----Original Message-----
> From: Zhou, Peng Ju 
> Sent: Friday, April 9, 2021 11:36 AM
> To: Kuehling, Felix <Felix.Kuehling at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: RE: [PATCH 1/8] drm/amdgpu: change MMHUB register access from MMIO to RLCG
>
> [AMD Official Use Only - Internal Distribution Only]
>
> Hi Felix
> That is a great idea, I will try it.
>
>
> ---------------------------------------------------------------------- 
> BW
> Pengju Zhou
>
>
>
> -----Original Message-----
> From: Kuehling, Felix <Felix.Kuehling at amd.com> 
> Sent: Thursday, April 8, 2021 10:58 PM
> To: Zhou, Peng Ju <PengJu.Zhou at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 1/8] drm/amdgpu: change MMHUB register access from MMIO to RLCG
>
> Given the number of call-sites being modified in this patch series, would it be easier (and more maintainable) to change the behaviour or the regular register macros and add NO_RLC versions for the exceptions, similar to NO_KIQ?
>
> Regards,
>   Felix
>
> Am 2021-04-08 um 6:21 a.m. schrieb Peng Ju Zhou:
>> From: pengzhou <PengJu.Zhou at amd.com>
>>
>> In SRIOV environment, KMD should access MMHUB registers with RLCG if 
>> MMHUB indirect access bit enabled.
>>
>> Change MMHUB register access from MMIO to RLCG.
>>
>> Signed-off-by: pengzhou <PengJu.Zhou at amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 12 ++++++--  
>> drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c | 39 +++++++++++++------------
>>  2 files changed, 29 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> index 2bfd620576f2..42818c40d08c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> @@ -38,6 +38,7 @@
>>  #include "soc15.h"
>>  #include "soc15d.h"
>>  #include "soc15_common.h"
>> +#include "gc/gc_10_1_0_offset.h"
>>  
>>  #include "nbio_v2_3.h"
>>  
>> @@ -253,7 +254,10 @@ static void gmc_v10_0_flush_vm_hub(struct amdgpu_device *adev, uint32_t vmid,
>>  			DRM_ERROR("Timeout waiting for sem acquire in VM flush!\n");
>>  	}
>>  
>> -	WREG32_NO_KIQ(hub->vm_inv_eng0_req + hub->eng_distance * eng, inv_req);
>> +	if (vmhub == AMDGPU_MMHUB_0)
>> +		WREG32_RLC_NO_KIQ((hub->vm_inv_eng0_req + eng), inv_req);
>> +	else
>> +		WREG32_NO_KIQ(hub->vm_inv_eng0_req + eng, inv_req);
>>  
>>  	/*
>>  	 * Issue a dummy read to wait for the ACK register to be cleared @@ 
>> -280,8 +284,10 @@ static void gmc_v10_0_flush_vm_hub(struct amdgpu_device *adev, uint32_t vmid,
>>  		 * add semaphore release after invalidation,
>>  		 * write with 0 means semaphore release
>>  		 */
>> -		WREG32_NO_KIQ(hub->vm_inv_eng0_sem +
>> -			      hub->eng_distance * eng, 0);
>> +		if (vmhub == AMDGPU_MMHUB_0)
>> +			WREG32_RLC_NO_KIQ(hub->vm_inv_eng0_sem + eng, 0);
>> +		else
>> +			WREG32_NO_KIQ(hub->vm_inv_eng0_sem + eng, 0);
>>  
>>  	spin_unlock(&adev->gmc.invalidate_lock);
>>  
>> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
>> index da7edd1ed6b2..e8ecdf383192 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
>> @@ -29,6 +29,7 @@
>>  #include "mmhub/mmhub_2_0_0_default.h"
>>  #include "navi10_enum.h"
>>  
>> +#include "gc/gc_10_1_0_offset.h"
>>  #include "soc15_common.h"
>>  
>>  #define mmMM_ATC_L2_MISC_CG_Sienna_Cichlid                      0x064d
>> @@ -165,11 +166,11 @@ static void mmhub_v2_0_setup_vm_pt_regs(struct 
>> amdgpu_device *adev, uint32_t vmi  {
>>  	struct amdgpu_vmhub *hub = &adev->vmhub[AMDGPU_MMHUB_0];
>>  
>> -	WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32,
>> +	WREG32_SOC15_OFFSET_RLC(MMHUB, 0, 
>> +mmMMVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_LO32,
>>  			    hub->ctx_addr_distance * vmid,
>>  			    lower_32_bits(page_table_base));
>>  
>> -	WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32,
>> +	WREG32_SOC15_OFFSET_RLC(MMHUB, 0, 
>> +mmMMVM_CONTEXT0_PAGE_TABLE_BASE_ADDR_HI32,
>>  			    hub->ctx_addr_distance * vmid,
>>  			    upper_32_bits(page_table_base));  } @@ -180,14 +181,14 @@ 
>> static void mmhub_v2_0_init_gart_aperture_regs(struct amdgpu_device 
>> *adev)
>>  
>>  	mmhub_v2_0_setup_vm_pt_regs(adev, 0, pt_base);
>>  
>> -	WREG32_SOC15(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_START_ADDR_LO32,
>> +	WREG32_SOC15_RLC(MMHUB, 0, 
>> +mmMMVM_CONTEXT0_PAGE_TABLE_START_ADDR_LO32,
>>  		     (u32)(adev->gmc.gart_start >> 12));
>> -	WREG32_SOC15(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_START_ADDR_HI32,
>> +	WREG32_SOC15_RLC(MMHUB, 0, 
>> +mmMMVM_CONTEXT0_PAGE_TABLE_START_ADDR_HI32,
>>  		     (u32)(adev->gmc.gart_start >> 44));
>>  
>> -	WREG32_SOC15(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_END_ADDR_LO32,
>> +	WREG32_SOC15_RLC(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_END_ADDR_LO32,
>>  		     (u32)(adev->gmc.gart_end >> 12));
>> -	WREG32_SOC15(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_END_ADDR_HI32,
>> +	WREG32_SOC15_RLC(MMHUB, 0, mmMMVM_CONTEXT0_PAGE_TABLE_END_ADDR_HI32,
>>  		     (u32)(adev->gmc.gart_end >> 44));  }
>>  
>> @@ -197,9 +198,9 @@ static void mmhub_v2_0_init_system_aperture_regs(struct amdgpu_device *adev)
>>  	uint32_t tmp;
>>  
>>  	/* Program the AGP BAR */
>> -	WREG32_SOC15(MMHUB, 0, mmMMMC_VM_AGP_BASE, 0);
>> -	WREG32_SOC15(MMHUB, 0, mmMMMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
>> -	WREG32_SOC15(MMHUB, 0, mmMMMC_VM_AGP_TOP, adev->gmc.agp_end >> 24);
>> +	WREG32_SOC15_RLC(MMHUB, 0, mmMMMC_VM_AGP_BASE, 0);
>> +	WREG32_SOC15_RLC(MMHUB, 0, mmMMMC_VM_AGP_BOT, adev->gmc.agp_start >> 24);
>> +	WREG32_SOC15_RLC(MMHUB, 0, mmMMMC_VM_AGP_TOP, adev->gmc.agp_end >> 
>> +24);
>>  
>>  	if (!amdgpu_sriov_vf(adev)) {
>>  		/* Program the system aperture low logical page number. */ @@ 
>> -304,12 +305,12 @@ static void mmhub_v2_0_enable_system_domain(struct 
>> amdgpu_device *adev)  {
>>  	uint32_t tmp;
>>  
>> -	tmp = RREG32_SOC15(MMHUB, 0, mmMMVM_CONTEXT0_CNTL);
>> +	tmp = RREG32_SOC15_RLC(MMHUB, 0, mmMMVM_CONTEXT0_CNTL);
>>  	tmp = REG_SET_FIELD(tmp, MMVM_CONTEXT0_CNTL, ENABLE_CONTEXT, 1);
>>  	tmp = REG_SET_FIELD(tmp, MMVM_CONTEXT0_CNTL, PAGE_TABLE_DEPTH, 0);
>>  	tmp = REG_SET_FIELD(tmp, MMVM_CONTEXT0_CNTL,
>>  			    RETRY_PERMISSION_OR_INVALID_PAGE_FAULT, 0);
>> -	WREG32_SOC15(MMHUB, 0, mmMMVM_CONTEXT0_CNTL, tmp);
>> +	WREG32_SOC15_RLC(MMHUB, 0, mmMMVM_CONTEXT0_CNTL, tmp);
>>  }
>>  
>>  static void mmhub_v2_0_disable_identity_aperture(struct amdgpu_device 
>> *adev) @@ -371,16 +372,16 @@ static void mmhub_v2_0_setup_vmid_config(struct amdgpu_device *adev)
>>  		tmp = REG_SET_FIELD(tmp, MMVM_CONTEXT1_CNTL,
>>  				    RETRY_PERMISSION_OR_INVALID_PAGE_FAULT,
>>  				    !adev->gmc.noretry);
>> -		WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_CONTEXT1_CNTL,
>> +		WREG32_SOC15_OFFSET_RLC(MMHUB, 0, mmMMVM_CONTEXT1_CNTL,
>>  				    i * hub->ctx_distance, tmp);
>> -		WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_CONTEXT1_PAGE_TABLE_START_ADDR_LO32,
>> +		WREG32_SOC15_OFFSET_RLC(MMHUB, 0, 
>> +mmMMVM_CONTEXT1_PAGE_TABLE_START_ADDR_LO32,
>>  				    i * hub->ctx_addr_distance, 0);
>> -		WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_CONTEXT1_PAGE_TABLE_START_ADDR_HI32,
>> +		WREG32_SOC15_OFFSET_RLC(MMHUB, 0, 
>> +mmMMVM_CONTEXT1_PAGE_TABLE_START_ADDR_HI32,
>>  				    i * hub->ctx_addr_distance, 0);
>> -		WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_CONTEXT1_PAGE_TABLE_END_ADDR_LO32,
>> +		WREG32_SOC15_OFFSET_RLC(MMHUB, 0, 
>> +mmMMVM_CONTEXT1_PAGE_TABLE_END_ADDR_LO32,
>>  				    i * hub->ctx_addr_distance,
>>  				    lower_32_bits(adev->vm_manager.max_pfn - 1));
>> -		WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_CONTEXT1_PAGE_TABLE_END_ADDR_HI32,
>> +		WREG32_SOC15_OFFSET_RLC(MMHUB, 0, 
>> +mmMMVM_CONTEXT1_PAGE_TABLE_END_ADDR_HI32,
>>  				    i * hub->ctx_addr_distance,
>>  				    upper_32_bits(adev->vm_manager.max_pfn - 1));
>>  	}
>> @@ -392,9 +393,9 @@ static void mmhub_v2_0_program_invalidation(struct amdgpu_device *adev)
>>  	unsigned i;
>>  
>>  	for (i = 0; i < 18; ++i) {
>> -		WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_INVALIDATE_ENG0_ADDR_RANGE_LO32,
>> +		WREG32_SOC15_OFFSET_RLC(MMHUB, 0, 
>> +mmMMVM_INVALIDATE_ENG0_ADDR_RANGE_LO32,
>>  				    i * hub->eng_addr_distance, 0xffffffff);
>> -		WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_INVALIDATE_ENG0_ADDR_RANGE_HI32,
>> +		WREG32_SOC15_OFFSET_RLC(MMHUB, 0, 
>> +mmMMVM_INVALIDATE_ENG0_ADDR_RANGE_HI32,
>>  				    i * hub->eng_addr_distance, 0x1f);
>>  	}
>>  }
>> @@ -423,7 +424,7 @@ static void mmhub_v2_0_gart_disable(struct 
>> amdgpu_device *adev)
>>  
>>  	/* Disable all tables */
>>  	for (i = 0; i < AMDGPU_NUM_VMID; i++)
>> -		WREG32_SOC15_OFFSET(MMHUB, 0, mmMMVM_CONTEXT0_CNTL,
>> +		WREG32_SOC15_OFFSET_RLC(MMHUB, 0, mmMMVM_CONTEXT0_CNTL,
>>  				    i * hub->ctx_distance, 0);
>>  
>>  	/* Setup TLB control */


More information about the amd-gfx mailing list