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

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

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?

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

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