All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: drop legacy IO bar support
@ 2021-03-15 15:17 Alex Deucher
  2021-03-15 15:26 ` Christian König
  0 siblings, 1 reply; 3+ messages in thread
From: Alex Deucher @ 2021-03-15 15:17 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Nicholas Johnson

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.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
---
 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 */
-- 
2.30.2

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/amdgpu: drop legacy IO bar support
  2021-03-15 15:17 [PATCH] drm/amdgpu: drop legacy IO bar support Alex Deucher
@ 2021-03-15 15:26 ` Christian König
  2021-03-16  4:57   ` Nicholas Johnson
  0 siblings, 1 reply; 3+ messages in thread
From: Christian König @ 2021-03-15 15:26 UTC (permalink / raw)
  To: Alex Deucher, amd-gfx; +Cc: Nicholas Johnson

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.

Oh my, that stuff was still around?

And yes that is expected, there is probably no legacy IO space allocated 
to the thunderbolt hardware.

> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>

Reviewed-by: Christian König <christian.koenig@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 */

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/amdgpu: drop legacy IO bar support
  2021-03-15 15:26 ` Christian König
@ 2021-03-16  4:57   ` Nicholas Johnson
  0 siblings, 0 replies; 3+ messages in thread
From: Nicholas Johnson @ 2021-03-16  4:57 UTC (permalink / raw)
  To: Christian König; +Cc: Alex Deucher, amd-gfx

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@outlook.com.au>

Cheers!

Kind regards,
Nicholas Johnson
> 
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> 
> Reviewed-by: Christian König <christian.koenig@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 */
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-03-16  8:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 15:17 [PATCH] drm/amdgpu: drop legacy IO bar support Alex Deucher
2021-03-15 15:26 ` Christian König
2021-03-16  4:57   ` Nicholas Johnson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.