[PATCH] drm/amdgpu: remap hdp coherency registers for vi on carrizo

[Felix Kuehling]

Am 2020-08-19 um 11:01 a.m. schrieb Huang Rui:
> On Wed, Aug 19, 2020 at 10:36:05PM +0800, Kuehling, Felix wrote:
>> Just for Carrizo, HDP flushing doesn't make a lot of sense because we
>> don't use HDP to access the framebuffer.
> OK, so soc15 and later need use HDP to access the framebuffer from user
> space. May I know why?

On all dGPUs we access framebuffer through HDP. On APUs we access the
carve-out directly in system memory and don't go through the PCIe BAR
and HDP.

The MMIO-remapping is done to allow user mode to initiate HDP flushes. I
don't remember why we only enable this on SOC15. Maybe we didn't know
this was available on older ASICs, or maybe the user mode code to use
this capability wasn't implemented on GFXv8.


>
>> The code you're changing doesn't look Carrizo-specific, but VI-specific.
>> So it would affect Fiji and Polaris as well. We already support Fiji and
>> Polaris dGPUs with KFD, apparently without remapping HDP registers. Why
>> is this change now needed for Carrizo?
> Because I found thunk will allocate the memory with flag
> KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP once I set ignore_crat. Then we failed
> to allocate the buffer.

I think the same would happen on Fiji and Polaris. Can you confirm this?

I'm not opposed to this change, but the justification should not be
Carrizo, but a more general GFXv8 statement.

Regards,
  Felix


>
> Thanks,
> Ray
>
>> Regards,
>>   Felix
>>
>> Am 2020-08-19 um 10:28 a.m. schrieb Huang Rui:
>>> Carrzio also needs remap HDP_MEM_COHERENCY_FLUSH_CNTL and
>>> HDP_REG_COHERENCY_FLUSH_CNTL to the empty page in mmio space. Then user
>>> mode is able to do flush hdp as well. It will used for force dgpu path
>>> on carrizo.
>>>
>>> Signed-off-by: Huang Rui <ray.huang at amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/vi.c | 18 +++++++++++++++---
>>>  1 file changed, 15 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
>>> index af8986a55354..a540fe7cffc7 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
>>> @@ -78,6 +78,7 @@
>>>  #include "dce_virtual.h"
>>>  #include "mxgpu_vi.h"
>>>  #include "amdgpu_dm.h"
>>> +#include <uapi/linux/kfd_ioctl.h>
>>>  
>>>  /*
>>>   * Indirect registers accessor
>>> @@ -944,10 +945,10 @@ static uint32_t vi_get_rev_id(struct amdgpu_device *adev)
>>>  static void vi_flush_hdp(struct amdgpu_device *adev, struct amdgpu_ring *ring)
>>>  {
>>>  	if (!ring || !ring->funcs->emit_wreg) {
>>> -		WREG32(mmHDP_MEM_COHERENCY_FLUSH_CNTL, 1);
>>> -		RREG32(mmHDP_MEM_COHERENCY_FLUSH_CNTL);
>>> +		WREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 1);
>>> +		RREG32((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2);
>>>  	} else {
>>> -		amdgpu_ring_emit_wreg(ring, mmHDP_MEM_COHERENCY_FLUSH_CNTL, 1);
>>> +		amdgpu_ring_emit_wreg(ring, (adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 1);
>>>  	}
>>>  }
>>>  
>>> @@ -1085,8 +1086,11 @@ static const struct amdgpu_asic_funcs vi_asic_funcs =
>>>  
>>>  static int vi_common_early_init(void *handle)
>>>  {
>>> +#define MMIO_REG_HOLE_OFFSET (0x80000 - PAGE_SIZE)
>>>  	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>>  
>>> +	adev->rmmio_remap.reg_offset = MMIO_REG_HOLE_OFFSET;
>>> +	adev->rmmio_remap.bus_addr = adev->rmmio_base + MMIO_REG_HOLE_OFFSET;
>>>  	if (adev->flags & AMD_IS_APU) {
>>>  		adev->smc_rreg = &cz_smc_rreg;
>>>  		adev->smc_wreg = &cz_smc_wreg;
>>> @@ -1332,6 +1336,12 @@ static int vi_common_sw_fini(void *handle)
>>>  	return 0;
>>>  }
>>>  
>>> +static void vi_remap_hdp_registers(struct amdgpu_device *adev)
>>> +{
>>> +	WREG32(mmREMAP_HDP_MEM_FLUSH_CNTL, adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL);
>>> +	WREG32(mmREMAP_HDP_REG_FLUSH_CNTL, adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL);
>>> +}
>>> +
>>>  static int vi_common_hw_init(void *handle)
>>>  {
>>>  	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>> @@ -1342,6 +1352,8 @@ static int vi_common_hw_init(void *handle)
>>>  	vi_pcie_gen3_enable(adev);
>>>  	/* enable aspm */
>>>  	vi_program_aspm(adev);
>>> +	/* remap hdp registers */
>>> +	vi_remap_hdp_registers(adev);
>>>  	/* enable the doorbell aperture */
>>>  	vi_enable_doorbell_aperture(adev, true);
>>>