Patchwork [Maverick] drm/radeon/kms: add ioport register access

login
register
mail settings
Submitter Alberto Milone
Date July 8, 2010, 5:28 p.m.
Message ID <AANLkTin71V9NZf_Slddka9P7PjzAzh6-25knn7XdfYpH@mail.gmail.com>
Download mbox | patch
Permalink /patch/58266/
State Accepted
Headers show

Comments

Alberto Milone - July 8, 2010, 5:28 p.m.
On 8 July 2010 14:43, Tim Gardner <tim.gardner@canonical.com> wrote:
> On 07/08/2010 05:26 AM, Alberto Milone wrote:
>>
>> Hi all,
>>
>> The attached patch fixes an issue when trying to access NB_MISC
>> registers on rs780/rs880 devices. The main problem is that they are
>> only accessible via I/O, rather than MMIO. This makes HDMI work
>> flawlessly here.
>>
>> The patch is available in the drm-radeon-testing branch, therefore it
>> should make it into 2.6.36:
>>
>> http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=shortlog;h=refs/heads/drm-radeon-testing
>>
>> Please consider accepting this patch in Maverick.
>>
>> Thanks,
>>
>>
>
> What happens in radeon_device_fini() on an ATOM platform when
> rdev->rio_mem==NULL ? I think pci_iounmap() will crash. Shouldn't it be:
>
> if (rdev->rio_mem)
>        pci_iounmap(rdev->pdev, rdev->rio_mem);
>
> rtg
> --
> Tim Gardner tim.gardner@canonical.com
>

Good point, Tim, even though all radeons should have an io resource.
Better be safe than sorry.

Here's an updated version of the patch (courtesy of the "git commit
--amend" command) with that additional check.

Let me know if you prefer to split the fix into 2 separate patches
instead i.e. the previous patch + another patch with the 2 lines
change that you suggested (whatever works best for you).

Thanks,
Tim Gardner - July 9, 2010, 3:39 p.m.
On 07/08/2010 11:28 AM, Alberto Milone wrote:
> On 8 July 2010 14:43, Tim Gardner<tim.gardner@canonical.com>  wrote:
>> On 07/08/2010 05:26 AM, Alberto Milone wrote:
>>>
>>> Hi all,
>>>
>>> The attached patch fixes an issue when trying to access NB_MISC
>>> registers on rs780/rs880 devices. The main problem is that they are
>>> only accessible via I/O, rather than MMIO. This makes HDMI work
>>> flawlessly here.
>>>
>>> The patch is available in the drm-radeon-testing branch, therefore it
>>> should make it into 2.6.36:
>>>
>>> http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=shortlog;h=refs/heads/drm-radeon-testing
>>>
>>> Please consider accepting this patch in Maverick.
>>>
>>> Thanks,
>>>
>>>
>>
>> What happens in radeon_device_fini() on an ATOM platform when
>> rdev->rio_mem==NULL ? I think pci_iounmap() will crash. Shouldn't it be:
>>
>> if (rdev->rio_mem)
>>         pci_iounmap(rdev->pdev, rdev->rio_mem);
>>
>> rtg
>> --
>> Tim Gardner tim.gardner@canonical.com
>>
>
> Good point, Tim, even though all radeons should have an io resource.
> Better be safe than sorry.
>
> Here's an updated version of the patch (courtesy of the "git commit
> --amend" command) with that additional check.
>
> Let me know if you prefer to split the fix into 2 separate patches
> instead i.e. the previous patch + another patch with the 2 lines
> change that you suggested (whatever works best for you).
>
> Thanks,
>

Applied. Are you gonna bug the upstream guys about this?

Patch

From df3794895de6ca406fedd86161d832de5a9286c8 Mon Sep 17 00:00:00 2001
From: Alex Deucher <alexdeucher@gmail.com>
Date: Wed, 30 Jun 2010 11:52:50 -0400
Subject: [PATCH 1/1] drm/radeon/kms: add ioport register access (squashed)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is required for the NB_MISC regs on rs780/rs880 which
means HDMI/DVI/DP ports using PCIEPHY won't work without
it. It might also help with s/r (asic init) issues on other
atombios cards.

Fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=28774
and similar issues reported by Alberto Milone.

Squash io fix patch.

Signed-off-by: Alex Deucher <alexdeucher@gmail.com>
Tested-by: Rafał Miłecki <zajec5@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>

drm/radeon/kms: ioport fixes

Walk the pci resources and make sure to use the I/O resource.
Unfortunately, the pci I/O resource number varies between
asic families.  If we don't find an I/O resource fall back
the the previous behavior of using MMIO for ATOM IIO.

Signed-off-by: Alex Deucher <alexdeucher@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/radeon/atom.c          |    5 +--
 drivers/gpu/drm/radeon/atom.h          |    2 +
 drivers/gpu/drm/radeon/radeon.h        |   25 +++++++++++++++++++
 drivers/gpu/drm/radeon/radeon_device.c |   41 +++++++++++++++++++++++++++++++-
 4 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atom.c b/drivers/gpu/drm/radeon/atom.c
index 1d56983..b2e3314 100644
--- a/drivers/gpu/drm/radeon/atom.c
+++ b/drivers/gpu/drm/radeon/atom.c
@@ -108,12 +108,11 @@  static uint32_t atom_iio_execute(struct atom_context *ctx, int base,
 			base++;
 			break;
 		case ATOM_IIO_READ:
-			temp = ctx->card->reg_read(ctx->card, CU16(base + 1));
+			temp = ctx->card->ioreg_read(ctx->card, CU16(base + 1));
 			base += 3;
 			break;
 		case ATOM_IIO_WRITE:
-			(void)ctx->card->reg_read(ctx->card, CU16(base + 1));
-			ctx->card->reg_write(ctx->card, CU16(base + 1), temp);
+			ctx->card->ioreg_write(ctx->card, CU16(base + 1), temp);
 			base += 3;
 			break;
 		case ATOM_IIO_CLEAR:
diff --git a/drivers/gpu/drm/radeon/atom.h b/drivers/gpu/drm/radeon/atom.h
index cd1b64a..a589a55 100644
--- a/drivers/gpu/drm/radeon/atom.h
+++ b/drivers/gpu/drm/radeon/atom.h
@@ -113,6 +113,8 @@  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 */
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 8e1d44c..35f0800 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1030,6 +1030,9 @@  struct radeon_device {
 	uint32_t                        pcie_reg_mask;
 	radeon_rreg_t			pciep_rreg;
 	radeon_wreg_t			pciep_wreg;
+	/* io port */
+	void __iomem                    *rio_mem;
+	resource_size_t			rio_mem_size;
 	struct radeon_clock             clock;
 	struct radeon_mc		mc;
 	struct radeon_gart		gart;
@@ -1111,6 +1114,26 @@  static inline void r100_mm_wreg(struct radeon_device *rdev, uint32_t reg, uint32
 	}
 }
 
+static inline u32 r100_io_rreg(struct radeon_device *rdev, u32 reg)
+{
+	if (reg < rdev->rio_mem_size)
+		return ioread32(rdev->rio_mem + reg);
+	else {
+		iowrite32(reg, rdev->rio_mem + RADEON_MM_INDEX);
+		return ioread32(rdev->rio_mem + RADEON_MM_DATA);
+	}
+}
+
+static inline void r100_io_wreg(struct radeon_device *rdev, u32 reg, u32 v)
+{
+	if (reg < rdev->rio_mem_size)
+		iowrite32(v, rdev->rio_mem + reg);
+	else {
+		iowrite32(reg, rdev->rio_mem + RADEON_MM_INDEX);
+		iowrite32(v, rdev->rio_mem + RADEON_MM_DATA);
+	}
+}
+
 /*
  * Cast helper
  */
@@ -1149,6 +1172,8 @@  static inline void r100_mm_wreg(struct radeon_device *rdev, uint32_t reg, uint32
 		WREG32_PLL(reg, tmp_);				\
 	} while (0)
 #define DREG32_SYS(sqf, rdev, reg) seq_printf((sqf), #reg " : 0x%08X\n", r100_mm_rreg((rdev), (reg)))
+#define RREG32_IO(reg) r100_io_rreg(rdev, (reg))
+#define WREG32_IO(reg, v) r100_io_wreg(rdev, (reg), (v))
 
 /*
  * Indirect registers accessor
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index f10faed..c61df68 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -415,6 +415,22 @@  static uint32_t cail_reg_read(struct card_info *info, uint32_t reg)
 	return r;
 }
 
+static void cail_ioreg_write(struct card_info *info, uint32_t reg, uint32_t val)
+{
+	struct radeon_device *rdev = info->dev->dev_private;
+
+	WREG32_IO(reg*4, val);
+}
+
+static uint32_t cail_ioreg_read(struct card_info *info, uint32_t reg)
+{
+	struct radeon_device *rdev = info->dev->dev_private;
+	uint32_t r;
+
+	r = RREG32_IO(reg*4);
+	return r;
+}
+
 int radeon_atombios_init(struct radeon_device *rdev)
 {
 	struct card_info *atom_card_info =
@@ -427,6 +443,15 @@  int radeon_atombios_init(struct radeon_device *rdev)
 	atom_card_info->dev = rdev->ddev;
 	atom_card_info->reg_read = cail_reg_read;
 	atom_card_info->reg_write = cail_reg_write;
+	/* needed for iio ops */
+	if (rdev->rio_mem) {
+		atom_card_info->ioreg_read = cail_ioreg_read;
+		atom_card_info->ioreg_write = cail_ioreg_write;
+	} else {
+		DRM_ERROR("Unable to find PCI I/O BAR; using MMIO for ATOM IIO\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;
@@ -573,7 +598,7 @@  int radeon_device_init(struct radeon_device *rdev,
 		       struct pci_dev *pdev,
 		       uint32_t flags)
 {
-	int r;
+	int r, i;
 	int dma_bits;
 
 	rdev->shutdown = false;
@@ -659,6 +684,17 @@  int radeon_device_init(struct radeon_device *rdev,
 	DRM_INFO("register mmio base: 0x%08X\n", (uint32_t)rdev->rmmio_base);
 	DRM_INFO("register mmio size: %u\n", (unsigned)rdev->rmmio_size);
 
+	/* io port mapping */
+	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
+		if (pci_resource_flags(rdev->pdev, i) & IORESOURCE_IO) {
+			rdev->rio_mem_size = pci_resource_len(rdev->pdev, i);
+			rdev->rio_mem = pci_iomap(rdev->pdev, i, rdev->rio_mem_size);
+			break;
+		}
+	}
+	if (rdev->rio_mem == NULL)
+		DRM_ERROR("Unable to find PCI I/O BAR\n");
+
 	/* if we have > 1 VGA cards, then disable the radeon VGA resources */
 	/* this will fail for cards that aren't VGA class devices, just
 	 * ignore it */
@@ -701,6 +737,9 @@  void radeon_device_fini(struct radeon_device *rdev)
 	destroy_workqueue(rdev->wq);
 	vga_switcheroo_unregister_client(rdev->pdev);
 	vga_client_register(rdev->pdev, NULL, NULL, NULL);
+	if (rdev->rio_mem)
+		pci_iounmap(rdev->pdev, rdev->rio_mem);
+	rdev->rio_mem = NULL;
 	iounmap(rdev->rmmio);
 	rdev->rmmio = NULL;
 }
-- 
1.7.1