[PATCH] drm/amdgpu: drop legacy IO bar support

Nicholas Johnson nicholas.johnson-opensource at outlook.com.au
Tue Mar 16 04:57:41 UTC 2021


On Mon, Mar 15, 2021 at 04:26:49PM +0100, Christian König wrote:
> Am 15.03.21 um 16:17 schrieb Alex Deucher:
> > It was leftover from radeon where it was required for some
> > specific old hardware.  It hasn't been required for ages
> > and the driver already falls back to MMIO when legacy IO
> > is not available.  Legacy IO also seems to be problematic on
> > on some thunderbolt devices.  Drop it.
Nitpick, "Thunderbolt" pronoun
> 
> Oh my, that stuff was still around?
> 
> And yes that is expected, there is probably no legacy IO space allocated to
> the thunderbolt hardware.
Sometimes there is legacy IO space allocated with Thunderbolt; that is 
when the issue occurs. It is possible that there is a Thunderbolt bug 
and the legacy IOs are black-holed or something. When no legacy IO space 
is allocated by Thunderbolt, amdgpu.ko uses MMIO instead, and everything 
works.

This patch to remove legacy IO support does not apply cleanly against 
v5.12-rc3, but patch -p1 managed to adapt it. I tested it. It solves my 
issue (instant amdgpu init when eGPU attached) and does not seem to have 
broken anything. Now I wish I had brought this issue up ages ago!

Tested-by: Nicholas Johnson <nicholas.johnson-opensource at outlook.com.au>

Cheers!

Kind regards,
Nicholas Johnson
> 
> > Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> > Cc: Nicholas Johnson <nicholas.johnson-opensource at outlook.com.au>
> 
> Reviewed-by: Christian König <christian.koenig at amd.com>
> 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu.h          |  7 ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 43 ---------------
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 57 --------------------
> >   drivers/gpu/drm/amd/amdgpu/atom.c            |  4 +-
> >   drivers/gpu/drm/amd/amdgpu/atom.h            |  2 -
> >   5 files changed, 2 insertions(+), 111 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index f69e6389bdc6..e738ebbe738a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -876,8 +876,6 @@ struct amdgpu_device {
> >   	spinlock_t audio_endpt_idx_lock;
> >   	amdgpu_block_rreg_t		audio_endpt_rreg;
> >   	amdgpu_block_wreg_t		audio_endpt_wreg;
> > -	void __iomem                    *rio_mem;
> > -	resource_size_t			rio_mem_size;
> >   	struct amdgpu_doorbell		doorbell;
> >   	/* clock/pll info */
> > @@ -1109,9 +1107,6 @@ void amdgpu_mm_wreg_mmio_rlc(struct amdgpu_device *adev,
> >   void amdgpu_mm_wreg8(struct amdgpu_device *adev, uint32_t offset, uint8_t value);
> >   uint8_t amdgpu_mm_rreg8(struct amdgpu_device *adev, uint32_t offset);
> > -u32 amdgpu_io_rreg(struct amdgpu_device *adev, u32 reg);
> > -void amdgpu_io_wreg(struct amdgpu_device *adev, u32 reg, u32 v);
> > -
> >   u32 amdgpu_device_indirect_rreg(struct amdgpu_device *adev,
> >   				u32 pcie_index, u32 pcie_data,
> >   				u32 reg_addr);
> > @@ -1202,8 +1197,6 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
> >   	} while (0)
> >   #define DREG32_SYS(sqf, adev, reg) seq_printf((sqf), #reg " : 0x%08X\n", amdgpu_device_rreg((adev), (reg), false))
> > -#define RREG32_IO(reg) amdgpu_io_rreg(adev, (reg))
> > -#define WREG32_IO(reg, v) amdgpu_io_wreg(adev, (reg), (v))
> >   #define REG_FIELD_SHIFT(reg, field) reg##__##field##__SHIFT
> >   #define REG_FIELD_MASK(reg, field) reg##__##field##_MASK
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> > index 86add0f4ea4d..e05648a8a145 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> > @@ -1905,40 +1905,6 @@ static uint32_t cail_reg_read(struct card_info *info, uint32_t reg)
> >   	return r;
> >   }
> > -/**
> > - * cail_ioreg_write - write IO register
> > - *
> > - * @info: atom card_info pointer
> > - * @reg: IO register offset
> > - * @val: value to write to the pll register
> > - *
> > - * Provides a IO register accessor for the atom interpreter (r4xx+).
> > - */
> > -static void cail_ioreg_write(struct card_info *info, uint32_t reg, uint32_t val)
> > -{
> > -	struct amdgpu_device *adev = drm_to_adev(info->dev);
> > -
> > -	WREG32_IO(reg, val);
> > -}
> > -
> > -/**
> > - * cail_ioreg_read - read IO register
> > - *
> > - * @info: atom card_info pointer
> > - * @reg: IO register offset
> > - *
> > - * Provides an IO register accessor for the atom interpreter (r4xx+).
> > - * Returns the value of the IO register.
> > - */
> > -static uint32_t cail_ioreg_read(struct card_info *info, uint32_t reg)
> > -{
> > -	struct amdgpu_device *adev = drm_to_adev(info->dev);
> > -	uint32_t r;
> > -
> > -	r = RREG32_IO(reg);
> > -	return r;
> > -}
> > -
> >   static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev,
> >   						 struct device_attribute *attr,
> >   						 char *buf)
> > @@ -1998,15 +1964,6 @@ int amdgpu_atombios_init(struct amdgpu_device *adev)
> >   	atom_card_info->dev = adev_to_drm(adev);
> >   	atom_card_info->reg_read = cail_reg_read;
> >   	atom_card_info->reg_write = cail_reg_write;
> > -	/* needed for iio ops */
> > -	if (adev->rio_mem) {
> > -		atom_card_info->ioreg_read = cail_ioreg_read;
> > -		atom_card_info->ioreg_write = cail_ioreg_write;
> > -	} else {
> > -		DRM_DEBUG("PCI I/O BAR is not found. Using MMIO to access ATOM BIOS\n");
> > -		atom_card_info->ioreg_read = cail_reg_read;
> > -		atom_card_info->ioreg_write = cail_reg_write;
> > -	}
> >   	atom_card_info->mc_read = cail_mc_read;
> >   	atom_card_info->mc_write = cail_mc_write;
> >   	atom_card_info->pll_read = cail_pll_read;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index e04ec3c83485..112749549c00 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -466,49 +466,6 @@ void amdgpu_mm_wreg_mmio_rlc(struct amdgpu_device *adev,
> >   	}
> >   }
> > -/**
> > - * amdgpu_io_rreg - read an IO register
> > - *
> > - * @adev: amdgpu_device pointer
> > - * @reg: dword aligned register offset
> > - *
> > - * Returns the 32 bit value from the offset specified.
> > - */
> > -u32 amdgpu_io_rreg(struct amdgpu_device *adev, u32 reg)
> > -{
> > -	if (adev->in_pci_err_recovery)
> > -		return 0;
> > -
> > -	if ((reg * 4) < adev->rio_mem_size)
> > -		return ioread32(adev->rio_mem + (reg * 4));
> > -	else {
> > -		iowrite32((reg * 4), adev->rio_mem + (mmMM_INDEX * 4));
> > -		return ioread32(adev->rio_mem + (mmMM_DATA * 4));
> > -	}
> > -}
> > -
> > -/**
> > - * amdgpu_io_wreg - write to an IO register
> > - *
> > - * @adev: amdgpu_device pointer
> > - * @reg: dword aligned register offset
> > - * @v: 32 bit value to write to the register
> > - *
> > - * Writes the value specified to the offset specified.
> > - */
> > -void amdgpu_io_wreg(struct amdgpu_device *adev, u32 reg, u32 v)
> > -{
> > -	if (adev->in_pci_err_recovery)
> > -		return;
> > -
> > -	if ((reg * 4) < adev->rio_mem_size)
> > -		iowrite32(v, adev->rio_mem + (reg * 4));
> > -	else {
> > -		iowrite32((reg * 4), adev->rio_mem + (mmMM_INDEX * 4));
> > -		iowrite32(v, adev->rio_mem + (mmMM_DATA * 4));
> > -	}
> > -}
> > -
> >   /**
> >    * amdgpu_mm_rdoorbell - read a doorbell dword
> >    *
> > @@ -3361,17 +3318,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> >   	DRM_INFO("register mmio base: 0x%08X\n", (uint32_t)adev->rmmio_base);
> >   	DRM_INFO("register mmio size: %u\n", (unsigned)adev->rmmio_size);
> > -	/* io port mapping */
> > -	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> > -		if (pci_resource_flags(adev->pdev, i) & IORESOURCE_IO) {
> > -			adev->rio_mem_size = pci_resource_len(adev->pdev, i);
> > -			adev->rio_mem = pci_iomap(adev->pdev, i, adev->rio_mem_size);
> > -			break;
> > -		}
> > -	}
> > -	if (adev->rio_mem == NULL)
> > -		DRM_INFO("PCI I/O BAR is not found.\n");
> > -
> >   	/* enable PCIE atomic ops */
> >   	r = pci_enable_atomic_ops_to_root(adev->pdev,
> >   					  PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
> > @@ -3693,9 +3639,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
> >   	}
> >   	if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
> >   		vga_client_register(adev->pdev, NULL, NULL, NULL);
> > -	if (adev->rio_mem)
> > -		pci_iounmap(adev->pdev, adev->rio_mem);
> > -	adev->rio_mem = NULL;
> >   	iounmap(adev->rmmio);
> >   	adev->rmmio = NULL;
> >   	amdgpu_device_doorbell_fini(adev);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
> > index 515890f4f5a0..3dcb8b32f48b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/atom.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
> > @@ -114,11 +114,11 @@ static uint32_t atom_iio_execute(struct atom_context *ctx, int base,
> >   			base++;
> >   			break;
> >   		case ATOM_IIO_READ:
> > -			temp = ctx->card->ioreg_read(ctx->card, CU16(base + 1));
> > +			temp = ctx->card->reg_read(ctx->card, CU16(base + 1));
> >   			base += 3;
> >   			break;
> >   		case ATOM_IIO_WRITE:
> > -			ctx->card->ioreg_write(ctx->card, CU16(base + 1), temp);
> > +			ctx->card->reg_write(ctx->card, CU16(base + 1), temp);
> >   			base += 3;
> >   			break;
> >   		case ATOM_IIO_CLEAR:
> > diff --git a/drivers/gpu/drm/amd/amdgpu/atom.h b/drivers/gpu/drm/amd/amdgpu/atom.h
> > index 4205bbe5d8d7..d279759cab47 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/atom.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/atom.h
> > @@ -116,8 +116,6 @@ struct card_info {
> >   	struct drm_device *dev;
> >   	void (* reg_write)(struct card_info *, uint32_t, uint32_t);   /*  filled by driver */
> >   	uint32_t (* reg_read)(struct card_info *, uint32_t);          /*  filled by driver */
> > -	void (* ioreg_write)(struct card_info *, uint32_t, uint32_t);   /*  filled by driver */
> > -	uint32_t (* ioreg_read)(struct card_info *, uint32_t);          /*  filled by driver */
> >   	void (* mc_write)(struct card_info *, uint32_t, uint32_t);   /*  filled by driver */
> >   	uint32_t (* mc_read)(struct card_info *, uint32_t);          /*  filled by driver */
> >   	void (* pll_write)(struct card_info *, uint32_t, uint32_t);   /*  filled by driver */
> 


More information about the amd-gfx mailing list