[PATCH v2] Fix loading of module radeonfb on PowerMac

Subsystems: framebuffer layer, radeon framebuffer display driver, the rest

STALE3110d

4 messages, 3 authors, 2016-11-17 · open the first message on its own page

[PATCH v2] Fix loading of module radeonfb on PowerMac

From: Mathieu Malaterre <hidden>
Date: 2016-11-14 19:59:40 

When the linux kernel is build with (typical kernel ship with Debian
installer):

CONFIG_FB_OF=y
CONFIG_VT_HW_CONSOLE_BINDING=y
CONFIG_FB_RADEON=m

The offb driver takes precedence over module radeonfb. It is then
impossible to load the module, error reported is:

[   96.551486] radeonfb 0000:00:10.0: enabling device (0006 -> 0007)
[   96.551526] radeonfb 0000:00:10.0: BAR 0: can't reserve [mem 0x98000000-0x9fffffff pref]
[   96.551531] radeonfb (0000:00:10.0): cannot request region 0.
[   96.551545] radeonfb: probe of 0000:00:10.0 failed with error -16

This patch reproduce the behavior of the module radeon, so as to make it
possible to load radeonfb when offb is first loaded.

It should be noticed that `offb_destroy` is never called which explain the
need to skip error detection on the radeon side.

Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id=119741
Suggested-by: Lennart Sorensen <redacted>
---
 drivers/video/fbdev/aty/radeon_base.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index 218339a..84d634b 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -2259,6 +2259,22 @@ static struct bin_attribute edid2_attr = {
 	.read	= radeon_show_edid2,
 };
 
+static int radeon_kick_out_firmware_fb(struct pci_dev *pdev)
+{
+	struct apertures_struct *ap;
+
+	ap = alloc_apertures(1);
+	if (!ap)
+		return -ENOMEM;
+
+	ap->ranges[0].base = pci_resource_start(pdev, 0);
+	ap->ranges[0].size = pci_resource_len(pdev, 0);
+
+	remove_conflicting_framebuffers(ap, KBUILD_MODNAME, false);
+	kfree(ap);
+
+	return 0;
+}
 
 static int radeonfb_pci_register(struct pci_dev *pdev,
 				 const struct pci_device_id *ent)
@@ -2314,19 +2330,27 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
 	rinfo->fb_base_phys = pci_resource_start (pdev, 0);
 	rinfo->mmio_base_phys = pci_resource_start (pdev, 2);
 
+	ret = radeon_kick_out_firmware_fb(pdev);
+	if (ret)
+		return ret;
+
 	/* request the mem regions */
 	ret = pci_request_region(pdev, 0, "radeonfb framebuffer");
 	if (ret < 0) {
 		printk( KERN_ERR "radeonfb (%s): cannot request region 0.\n",
 			pci_name(rinfo->pdev));
+#ifndef CONFIG_PPC
 		goto err_release_fb;
+#endif
 	}
 
 	ret = pci_request_region(pdev, 2, "radeonfb mmio");
 	if (ret < 0) {
 		printk( KERN_ERR "radeonfb (%s): cannot request region 2.\n",
 			pci_name(rinfo->pdev));
+#ifndef CONFIG_PPC
 		goto err_release_pci0;
+#endif
 	}
 
 	/* map the regions */
@@ -2511,10 +2535,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
 	iounmap(rinfo->mmio_base);
 err_release_pci2:
 	pci_release_region(pdev, 2);
+#ifndef CONFIG_PPC
 err_release_pci0:
 	pci_release_region(pdev, 0);
 err_release_fb:
         framebuffer_release(info);
+#endif
 err_disable:
 err_out:
 	return ret;
-- 
2.1.4

Re: [PATCH v2] Fix loading of module radeonfb on PowerMac

From: Tomi Valkeinen <hidden>
Date: 2016-11-15 11:46:35 

Hi,

On 14/11/16 21:59, Mathieu Malaterre wrote:
When the linux kernel is build with (typical kernel ship with Debian
installer):

CONFIG_FB_OF=y
CONFIG_VT_HW_CONSOLE_BINDING=y
CONFIG_FB_RADEON=m

The offb driver takes precedence over module radeonfb. It is then
impossible to load the module, error reported is:

[   96.551486] radeonfb 0000:00:10.0: enabling device (0006 -> 0007)
[   96.551526] radeonfb 0000:00:10.0: BAR 0: can't reserve [mem 0x98000000-0x9fffffff pref]
[   96.551531] radeonfb (0000:00:10.0): cannot request region 0.
[   96.551545] radeonfb: probe of 0000:00:10.0 failed with error -16

This patch reproduce the behavior of the module radeon, so as to make it
possible to load radeonfb when offb is first loaded.

It should be noticed that `offb_destroy` is never called which explain the
need to skip error detection on the radeon side.

Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id=119741
Suggested-by: Lennart Sorensen <redacted>
---
Please always have a changelog in patch new revisions. For individual
patches, you can insert the changes here.
quoted hunk
 drivers/video/fbdev/aty/radeon_base.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index 218339a..84d634b 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -2259,6 +2259,22 @@ static struct bin_attribute edid2_attr = {
 	.read	= radeon_show_edid2,
 };
 
+static int radeon_kick_out_firmware_fb(struct pci_dev *pdev)
+{
+	struct apertures_struct *ap;
+
+	ap = alloc_apertures(1);
+	if (!ap)
+		return -ENOMEM;
+
+	ap->ranges[0].base = pci_resource_start(pdev, 0);
+	ap->ranges[0].size = pci_resource_len(pdev, 0);
+
+	remove_conflicting_framebuffers(ap, KBUILD_MODNAME, false);
+	kfree(ap);
+
+	return 0;
+}
 
 static int radeonfb_pci_register(struct pci_dev *pdev,
 				 const struct pci_device_id *ent)
@@ -2314,19 +2330,27 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
 	rinfo->fb_base_phys = pci_resource_start (pdev, 0);
 	rinfo->mmio_base_phys = pci_resource_start (pdev, 2);
 
+	ret = radeon_kick_out_firmware_fb(pdev);
+	if (ret)
+		return ret;
+
 	/* request the mem regions */
 	ret = pci_request_region(pdev, 0, "radeonfb framebuffer");
 	if (ret < 0) {
 		printk( KERN_ERR "radeonfb (%s): cannot request region 0.\n",
 			pci_name(rinfo->pdev));
+#ifndef CONFIG_PPC
 		goto err_release_fb;
+#endif
If this is not a problem on PPC, the kernel shouldn't print an error either.

 	}
 
 	ret = pci_request_region(pdev, 2, "radeonfb mmio");
 	if (ret < 0) {
 		printk( KERN_ERR "radeonfb (%s): cannot request region 2.\n",
 			pci_name(rinfo->pdev));
+#ifndef CONFIG_PPC
 		goto err_release_pci0;
+#endif
Same here.
quoted hunk
 	}
 
 	/* map the regions */
@@ -2511,10 +2535,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
 	iounmap(rinfo->mmio_base);
 err_release_pci2:
 	pci_release_region(pdev, 2);
+#ifndef CONFIG_PPC
 err_release_pci0:
 	pci_release_region(pdev, 0);
 err_release_fb:
         framebuffer_release(info);
+#endif
 err_disable:
 err_out:
 	return ret;
So I don't quite follow what's going on here. Why the CONFIG_PPC
conditionals? Is this problem only with OFFB and only with PPC?

And I think the code itself should have comments on this rather strange
behavior: the driver fails to get HW resources, but decides to ignore
the failure on PPC.

 Tomi

Re: [PATCH v2] Fix loading of module radeonfb on PowerMac

From: Lennart Sorensen <hidden>
Date: 2016-11-15 16:26:10 

On Tue, Nov 15, 2016 at 01:46:10PM +0200, Tomi Valkeinen wrote:
Hi,

On 14/11/16 21:59, Mathieu Malaterre wrote:
quoted
When the linux kernel is build with (typical kernel ship with Debian
installer):

CONFIG_FB_OF=y
CONFIG_VT_HW_CONSOLE_BINDING=y
CONFIG_FB_RADEON=m

The offb driver takes precedence over module radeonfb. It is then
impossible to load the module, error reported is:

[   96.551486] radeonfb 0000:00:10.0: enabling device (0006 -> 0007)
[   96.551526] radeonfb 0000:00:10.0: BAR 0: can't reserve [mem 0x98000000-0x9fffffff pref]
[   96.551531] radeonfb (0000:00:10.0): cannot request region 0.
[   96.551545] radeonfb: probe of 0000:00:10.0 failed with error -16

This patch reproduce the behavior of the module radeon, so as to make it
possible to load radeonfb when offb is first loaded.

It should be noticed that `offb_destroy` is never called which explain the
need to skip error detection on the radeon side.

Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id=119741
Suggested-by: Lennart Sorensen <redacted>
---
Please always have a changelog in patch new revisions. For individual
patches, you can insert the changes here.
quoted
 drivers/video/fbdev/aty/radeon_base.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index 218339a..84d634b 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -2259,6 +2259,22 @@ static struct bin_attribute edid2_attr = {
 	.read	= radeon_show_edid2,
 };
 
+static int radeon_kick_out_firmware_fb(struct pci_dev *pdev)
+{
+	struct apertures_struct *ap;
+
+	ap = alloc_apertures(1);
+	if (!ap)
+		return -ENOMEM;
+
+	ap->ranges[0].base = pci_resource_start(pdev, 0);
+	ap->ranges[0].size = pci_resource_len(pdev, 0);
+
+	remove_conflicting_framebuffers(ap, KBUILD_MODNAME, false);
+	kfree(ap);
+
+	return 0;
+}
 
 static int radeonfb_pci_register(struct pci_dev *pdev,
 				 const struct pci_device_id *ent)
@@ -2314,19 +2330,27 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
 	rinfo->fb_base_phys = pci_resource_start (pdev, 0);
 	rinfo->mmio_base_phys = pci_resource_start (pdev, 2);
 
+	ret = radeon_kick_out_firmware_fb(pdev);
+	if (ret)
+		return ret;
+
 	/* request the mem regions */
 	ret = pci_request_region(pdev, 0, "radeonfb framebuffer");
 	if (ret < 0) {
 		printk( KERN_ERR "radeonfb (%s): cannot request region 0.\n",
 			pci_name(rinfo->pdev));
+#ifndef CONFIG_PPC
 		goto err_release_fb;
+#endif
If this is not a problem on PPC, the kernel shouldn't print an error either.
quoted
 	}
 
 	ret = pci_request_region(pdev, 2, "radeonfb mmio");
 	if (ret < 0) {
 		printk( KERN_ERR "radeonfb (%s): cannot request region 2.\n",
 			pci_name(rinfo->pdev));
+#ifndef CONFIG_PPC
 		goto err_release_pci0;
+#endif
Same here.
quoted
 	}
 
 	/* map the regions */
@@ -2511,10 +2535,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
 	iounmap(rinfo->mmio_base);
 err_release_pci2:
 	pci_release_region(pdev, 2);
+#ifndef CONFIG_PPC
 err_release_pci0:
 	pci_release_region(pdev, 0);
 err_release_fb:
         framebuffer_release(info);
+#endif
 err_disable:
 err_out:
 	return ret;
So I don't quite follow what's going on here. Why the CONFIG_PPC
conditionals? Is this problem only with OFFB and only with PPC?
I don't think so.  I am no convinced the pci_request_region even serves
a purpose.  Certainly a number of the other drivers don't even bother
doing it.

The problem is that offb does do it, and radeon_fb tries to do it,
and since one is trying to take over from the other, it can't do that
because the area is already reserved.

And I think the code itself should have comments on this rather strange
behavior: the driver fails to get HW resources, but decides to ignore
the failure on PPC.
It seems the simpler answer is to just not do it at all.

Now if some architectures require you to do it (no idea), then that
could be an issue.

Looking at all the other FB drivers, none of the ones that support
kicking out another driver to takeover call pci_request_region.
The ones that do call it all appear to be older drivers, likely not
updated much lately.

-- 
Len Sorensen

Re: [PATCH v2] Fix loading of module radeonfb on PowerMac

From: Mathieu Malaterre <hidden>
Date: 2016-11-17 07:32:37 

Tomi,

On Tue, Nov 15, 2016 at 12:46 PM, Tomi Valkeinen [off-list ref] wrote:
Hi,

On 14/11/16 21:59, Mathieu Malaterre wrote:
quoted
When the linux kernel is build with (typical kernel ship with Debian
installer):

CONFIG_FB_OF=y
CONFIG_VT_HW_CONSOLE_BINDING=y
CONFIG_FB_RADEON=m

The offb driver takes precedence over module radeonfb. It is then
impossible to load the module, error reported is:

[   96.551486] radeonfb 0000:00:10.0: enabling device (0006 -> 0007)
[   96.551526] radeonfb 0000:00:10.0: BAR 0: can't reserve [mem 0x98000000-0x9fffffff pref]
[   96.551531] radeonfb (0000:00:10.0): cannot request region 0.
[   96.551545] radeonfb: probe of 0000:00:10.0 failed with error -16

This patch reproduce the behavior of the module radeon, so as to make it
possible to load radeonfb when offb is first loaded.

It should be noticed that `offb_destroy` is never called which explain the
need to skip error detection on the radeon side.

Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id=119741
Suggested-by: Lennart Sorensen <redacted>
---
Please always have a changelog in patch new revisions. For individual
patches, you can insert the changes here.
quoted
 drivers/video/fbdev/aty/radeon_base.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index 218339a..84d634b 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -2259,6 +2259,22 @@ static struct bin_attribute edid2_attr = {
      .read   = radeon_show_edid2,
 };

+static int radeon_kick_out_firmware_fb(struct pci_dev *pdev)
+{
+     struct apertures_struct *ap;
+
+     ap = alloc_apertures(1);
+     if (!ap)
+             return -ENOMEM;
+
+     ap->ranges[0].base = pci_resource_start(pdev, 0);
+     ap->ranges[0].size = pci_resource_len(pdev, 0);
+
+     remove_conflicting_framebuffers(ap, KBUILD_MODNAME, false);
+     kfree(ap);
+
+     return 0;
+}

 static int radeonfb_pci_register(struct pci_dev *pdev,
                               const struct pci_device_id *ent)
@@ -2314,19 +2330,27 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
      rinfo->fb_base_phys = pci_resource_start (pdev, 0);
      rinfo->mmio_base_phys = pci_resource_start (pdev, 2);

+     ret = radeon_kick_out_firmware_fb(pdev);
+     if (ret)
+             return ret;
+
      /* request the mem regions */
      ret = pci_request_region(pdev, 0, "radeonfb framebuffer");
      if (ret < 0) {
              printk( KERN_ERR "radeonfb (%s): cannot request region 0.\n",
                      pci_name(rinfo->pdev));
+#ifndef CONFIG_PPC
              goto err_release_fb;
+#endif
If this is not a problem on PPC, the kernel shouldn't print an error either.
quoted
      }

      ret = pci_request_region(pdev, 2, "radeonfb mmio");
      if (ret < 0) {
              printk( KERN_ERR "radeonfb (%s): cannot request region 2.\n",
                      pci_name(rinfo->pdev));
+#ifndef CONFIG_PPC
              goto err_release_pci0;
+#endif
Same here.
quoted
      }

      /* map the regions */
@@ -2511,10 +2535,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
      iounmap(rinfo->mmio_base);
 err_release_pci2:
      pci_release_region(pdev, 2);
+#ifndef CONFIG_PPC
 err_release_pci0:
      pci_release_region(pdev, 0);
 err_release_fb:
         framebuffer_release(info);
+#endif
 err_disable:
 err_out:
      return ret;
So I don't quite follow what's going on here. Why the CONFIG_PPC
conditionals? Is this problem only with OFFB and only with PPC?
The radeonfb fails to load only (conflict with offb):
- when on PPC (PMAC actually)
- when compiled as module (CONFIG_FB_RADEON=y is OK)

And I think the code itself should have comments on this rather strange
behavior: the driver fails to get HW resources, but decides to ignore
the failure on PPC.
Actually you are right, I missed configuration such as Pegasos[0]
machine (no openfirmware-framebuffer implemented in their firmware at
all). So I cannot simply assume that offb already allocated stuff on
all PPC out there.

Will resent a v3 with proper changelog ASAP.

[0] https://en.wikipedia.org/wiki/Pegasos
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help