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

[Huang Rui <ray.huang@amd.com>]

On Wed, Aug 19, 2020 at 11:28:01PM +0800, Kuehling, Felix wrote:
> 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.
> 

Thanks for the information.

> 
> >
> >> 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.
> 

Sure, it's not for carrizo, I will modify the commit message. Let me find a
Fiji/Polaris card to double confirm.

Thanks,
Ray

> 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@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);
> >>>  
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx