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?id9741
Suggested-by: Lennart Sorensen <redacted>
---
drivers/video/fbdev/aty/radeon_base.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
@@ -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)+returnret;+/* 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_PPCgotoerr_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_PPCgotoerr_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_PPCerr_release_pci0:pci_release_region(pdev,0);err_release_fb:framebuffer_release(info);+#endiferr_disable:err_out:returnret;
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.
@@ -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)+returnret;+/* 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_PPCgotoerr_release_fb;+#endif
If this is not a problem on PPC, the kernel shouldn't print an error either.
quoted hunk
} 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
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
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?id9741
Suggested-by: Lennart Sorensen <redacted>
---
Please always have a changelog in patch new revisions. For individual
patches, you can insert the changes here.
@@ -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)+returnret;+/* 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_PPCgotoerr_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
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
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?id9741
Suggested-by: Lennart Sorensen <redacted>
---
Please always have a changelog in patch new revisions. For individual
patches, you can insert the changes here.
@@ -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)+returnret;+/* 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_PPCgotoerr_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
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
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?id9741
Suggested-by: Lennart Sorensen <redacted>
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since the conflicts in region is due to OFfb explicitly refers to it.
drivers/video/fbdev/aty/radeon_base.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
@@ -2312,19 +2328,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)+returnret;+/* request the mem regions */ret=pci_request_region(pdev,0,"radeonfb framebuffer");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 0.\n",pci_name(rinfo->pdev));gotoerr_release_fb;+#endif}ret=pci_request_region(pdev,2,"radeonfb mmio");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 2.\n",pci_name(rinfo->pdev));gotoerr_release_pci0;+#endif}/* map the regions */
@@ -2509,10 +2533,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,iounmap(rinfo->mmio_base);err_release_pci2:pci_release_region(pdev,2);+#ifndef CONFIG_FB_OFerr_release_pci0:pci_release_region(pdev,0);err_release_fb:framebuffer_release(info);+#endiferr_disable:err_out:returnret;
On Thursday, December 21, 2017 11:07:56 PM 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.
This still needs to be explained more, from my last mail:
"The last put_fb_info() on fb_info should call ->fb_destroy
(offb_destroy in our case) and remove_conflicting_framebuffers()
is calling put_fb_info() so there is some extra reference on
fb_info somewhere preventing it from going away.
Please look into fixing this."
Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id9741
Suggested-by: Lennart Sorensen <redacted>
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since the conflicts in region is due to OFfb explicitly refers to it.
It seems that there may still be configurations when this is
incorrect -> when offb drives primary (non-radeon) card and radeonfb
drives secondary (radeon) card..
@@ -2312,19 +2328,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)+returnret;+/* request the mem regions */ret=pci_request_region(pdev,0,"radeonfb framebuffer");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 0.\n",pci_name(rinfo->pdev));gotoerr_release_fb;+#endif}ret=pci_request_region(pdev,2,"radeonfb mmio");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 2.\n",pci_name(rinfo->pdev));gotoerr_release_pci0;+#endif}/* map the regions */
@@ -2509,10 +2533,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,iounmap(rinfo->mmio_base);err_release_pci2:pci_release_region(pdev,2);+#ifndef CONFIG_FB_OFerr_release_pci0:pci_release_region(pdev,0);err_release_fb:framebuffer_release(info);+#endiferr_disable:err_out:returnret;
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
From: Lennart Sorensen <hidden> Date: 2018-01-03 15:31:37
On Wed, Jan 03, 2018 at 03:47:35PM +0100, Bartlomiej Zolnierkiewicz wrote:
On Thursday, December 21, 2017 11:07:56 PM 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.
This still needs to be explained more, from my last mail:
"The last put_fb_info() on fb_info should call ->fb_destroy
(offb_destroy in our case) and remove_conflicting_framebuffers()
is calling put_fb_info() so there is some extra reference on
fb_info somewhere preventing it from going away.
Please look into fixing this."
My impression of that problem is that the offb driver is in use because
it is running the console, and until the radeonfb driver is loaded and
ready to take over, you can't stop using the offb driver, but you can't
currently load the radeonfb because offb is holding resources it wants.
Maybe I have misunderstood what the code is doing, but that seems to be
the problem.
On an x86 PC you could drop one fb and go to text mode then start another
fb driver. PPC doesn't have that option since there is no text mode.
--
Len Sorensen
Hi Bartlomiej,
On Wed, Jan 3, 2018 at 4:23 PM, Lennart Sorensen
[off-list ref] wrote:
On Wed, Jan 03, 2018 at 03:47:35PM +0100, Bartlomiej Zolnierkiewicz wrote:
quoted
On Thursday, December 21, 2017 11:07:56 PM 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.
This still needs to be explained more, from my last mail:
"The last put_fb_info() on fb_info should call ->fb_destroy
(offb_destroy in our case) and remove_conflicting_framebuffers()
is calling put_fb_info() so there is some extra reference on
fb_info somewhere preventing it from going away.
Please look into fixing this."
My impression of that problem is that the offb driver is in use because
it is running the console, and until the radeonfb driver is loaded and
ready to take over, you can't stop using the offb driver, but you can't
currently load the radeonfb because offb is holding resources it wants.
Maybe I have misunderstood what the code is doing, but that seems to be
the problem.
On an x86 PC you could drop one fb and go to text mode then start another
fb driver. PPC doesn't have that option since there is no text mode.
Bartlomiej,
On Wed, Jan 3, 2018 at 3:47 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
On Thursday, December 21, 2017 11:07:56 PM 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.
This still needs to be explained more, from my last mail:
"The last put_fb_info() on fb_info should call ->fb_destroy
(offb_destroy in our case) and remove_conflicting_framebuffers()
is calling put_fb_info() so there is some extra reference on
fb_info somewhere preventing it from going away.
Please look into fixing this."
I am not familiar with the fb stuff internals but here is what I see:
# modprobe radeonfb
leads to:
[ 52.058546] bus: 'pci': add driver radeonfb
[ 52.058588] bus: 'pci': driver_probe_device: matched device
0000:00:10.0 with driver radeonfb
[ 52.058595] bus: 'pci': really_probe: probing driver radeonfb with
device 0000:00:10.0
[ 52.058608] devices_kset: Moving 0000:00:10.0 to end of list
[ 52.058613] radeonfb_pci_register BEGIN
[ 52.058634] radeonfb 0000:00:10.0: enabling device (0006 -> 0007)
<at this point radeon_kick_out_firmware_fb is called>
[ 52.058666] checking generic (9c008000 96000) vs hw (98000000 8000000)
[ 52.058667] fb: switching to radeonfb from OFfb ATY,RockHo
[ 52.058844] Console: switching to colour dummy device 80x25
[ 52.058860] device: 'fb0': device_unregister
[ 52.058956] PM: Removing info for No Bus:fb0
[ 52.059014] device: 'fb0': device_create_release
<a call to do_unregister_framebuffer is done>
<put_fb_info is done with a count=2 and dev=NULL>
[ 52.059048] device: 'vtcon1': device_unregister
[ 52.059076] PM: Removing info for No Bus:vtcon1
[ 52.059091] device: 'vtcon1': device_create_release
[ 52.059107] radeonfb 0000:00:10.0: BAR 0: can't reserve [mem
0x98000000-0x9fffffff pref]
[ 52.256151] aper_base: 98000000 MC_FB_LOC to: 9bff9800, MC_AGP_LOC
to: ffffa000
[ 52.256157] radeonfb (0000:00:10.0): Found 32768k of DDR 64 bits
wide videoram
I can confirm that offb_destroy is never called (not sure exactly
why), but in any case the call to radeon_kick_out_firmware_fb happen
much earlier, at least before the put_fb_info.
Could you describe a bit more the chain of calls you were thinking of ?
quoted
Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id9741
Suggested-by: Lennart Sorensen <redacted>
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since the conflicts in region is due to OFfb explicitly refers to it.
It seems that there may still be configurations when this is
incorrect -> when offb drives primary (non-radeon) card and radeonfb
drives secondary (radeon) card..
@@ -2312,19 +2328,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)+returnret;+/* request the mem regions */ret=pci_request_region(pdev,0,"radeonfb framebuffer");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 0.\n",pci_name(rinfo->pdev));gotoerr_release_fb;+#endif}ret=pci_request_region(pdev,2,"radeonfb mmio");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 2.\n",pci_name(rinfo->pdev));gotoerr_release_pci0;+#endif}/* map the regions */
@@ -2509,10 +2533,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,iounmap(rinfo->mmio_base);err_release_pci2:pci_release_region(pdev,2);+#ifndef CONFIG_FB_OFerr_release_pci0:pci_release_region(pdev,0);err_release_fb:framebuffer_release(info);+#endiferr_disable:err_out:returnret;
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
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, see commit a56f7428d753
("drm/radeon: Add early unregister of firmware fb's").
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?id9741
Suggested-by: Lennart Sorensen <redacted>
Cc: Bartlomiej Zolnierkiewicz <redacted>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Tomi Valkeinen <redacted>
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since the conflicts in region is due to OFfb explicitly refers to it.
v4: Use drm_fb_helper_remove_conflicting_framebuffers from drm_fb_helper
drivers/video/fbdev/aty/radeon_base.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
@@ -2312,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)+returnret;+/* request the mem regions */ret=pci_request_region(pdev,0,"radeonfb framebuffer");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 0.\n",pci_name(rinfo->pdev));gotoerr_release_fb;+#endif}ret=pci_request_region(pdev,2,"radeonfb mmio");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 2.\n",pci_name(rinfo->pdev));gotoerr_release_pci0;+#endif}/* map the regions */
@@ -2509,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_FB_OFerr_release_pci0:pci_release_region(pdev,0);err_release_fb:framebuffer_release(info);+#endiferr_disable:err_out:returnret;
On Tuesday, January 30, 2018 02:14:10 PM Mathieu Malaterre wrote:
Bartlomiej,
On Wed, Jan 3, 2018 at 3:47 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Thursday, December 21, 2017 11:07:56 PM 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.
This still needs to be explained more, from my last mail:
"The last put_fb_info() on fb_info should call ->fb_destroy
(offb_destroy in our case) and remove_conflicting_framebuffers()
is calling put_fb_info() so there is some extra reference on
fb_info somewhere preventing it from going away.
Please look into fixing this."
I am not familiar with the fb stuff internals but here is what I see:
# modprobe radeonfb
leads to:
[ 52.058546] bus: 'pci': add driver radeonfb
[ 52.058588] bus: 'pci': driver_probe_device: matched device
0000:00:10.0 with driver radeonfb
[ 52.058595] bus: 'pci': really_probe: probing driver radeonfb with
device 0000:00:10.0
[ 52.058608] devices_kset: Moving 0000:00:10.0 to end of list
[ 52.058613] radeonfb_pci_register BEGIN
[ 52.058634] radeonfb 0000:00:10.0: enabling device (0006 -> 0007)
<at this point radeon_kick_out_firmware_fb is called>
[ 52.058666] checking generic (9c008000 96000) vs hw (98000000 8000000)
[ 52.058667] fb: switching to radeonfb from OFfb ATY,RockHo
[ 52.058844] Console: switching to colour dummy device 80x25
[ 52.058860] device: 'fb0': device_unregister
[ 52.058956] PM: Removing info for No Bus:fb0
[ 52.059014] device: 'fb0': device_create_release
<a call to do_unregister_framebuffer is done>
<put_fb_info is done with a count=2 and dev=NULL>
[ 52.059048] device: 'vtcon1': device_unregister
[ 52.059076] PM: Removing info for No Bus:vtcon1
[ 52.059091] device: 'vtcon1': device_create_release
[ 52.059107] radeonfb 0000:00:10.0: BAR 0: can't reserve [mem
0x98000000-0x9fffffff pref]
[ 52.256151] aper_base: 98000000 MC_FB_LOC to: 9bff9800, MC_AGP_LOC
to: ffffa000
[ 52.256157] radeonfb (0000:00:10.0): Found 32768k of DDR 64 bits
wide videoram
I can confirm that offb_destroy is never called (not sure exactly
why), but in any case the call to radeon_kick_out_firmware_fb happen
much earlier, at least before the put_fb_info.
It is okay, put_fb_info() is called indirectly by radeon_kick_out_firmware_fb()
radeon_kick_out_firmware_fb()
remove_conflicting_framebuffers()
do_remove_conflicting_framebuffers()
do_unregister_framebuffer()
put_fb_info()
offb_destroy() is not called because there is an extra reference on old
fb_info (->count = 2):
static void put_fb_info(struct fb_info *fb_info)
{
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
The question is why there is an extra reference, probably user-space
is still holding the fb_info reference obtained in fb_open() call and
fb_release() is never called. Besides not calling fbops->fb_destroy()
this also causes missing call of fbops->fb_release() (in fb_release())
which some fb drivers are implementing (but not offb.c).
Could you describe a bit more the chain of calls you were thinking of ?
Please add WARN_ON(1) to get_fb_info() and put_fb_info() so we can check
from the stacktrace if it is actually fb_open() that holds the extra
old fb_info reference.
drivers/video/fbdev/core/fbmem.c:
static struct fb_info *get_fb_info(unsigned int idx)
{
struct fb_info *fb_info;
if (idx >= FB_MAX)
return ERR_PTR(-ENODEV);
mutex_lock(®istration_lock);
fb_info = registered_fb[idx];
if (fb_info)
atomic_inc(&fb_info->count);
if (fb_info)
WARN_ON(1);
mutex_unlock(®istration_lock);
return fb_info;
}
static void put_fb_info(struct fb_info *fb_info)
{
WARN_ON(1);
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
quoted
quoted
Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id9741
Suggested-by: Lennart Sorensen <redacted>
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since the conflicts in region is due to OFfb explicitly refers to it.
It seems that there may still be configurations when this is
incorrect -> when offb drives primary (non-radeon) card and radeonfb
drives secondary (radeon) card..
@@ -2312,19 +2328,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)+returnret;+/* request the mem regions */ret=pci_request_region(pdev,0,"radeonfb framebuffer");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 0.\n",pci_name(rinfo->pdev));gotoerr_release_fb;+#endif}ret=pci_request_region(pdev,2,"radeonfb mmio");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 2.\n",pci_name(rinfo->pdev));gotoerr_release_pci0;+#endif}/* map the regions */
@@ -2509,10 +2533,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,iounmap(rinfo->mmio_base);err_release_pci2:pci_release_region(pdev,2);+#ifndef CONFIG_FB_OFerr_release_pci0:pci_release_region(pdev,0);err_release_fb:framebuffer_release(info);+#endiferr_disable:err_out:returnret;
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Thanks,
-M
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Bartlomiej,
On Wed, Jan 31, 2018 at 12:57 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
On Tuesday, January 30, 2018 02:14:10 PM Mathieu Malaterre wrote:
quoted
Bartlomiej,
On Wed, Jan 3, 2018 at 3:47 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Thursday, December 21, 2017 11:07:56 PM 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.
This still needs to be explained more, from my last mail:
"The last put_fb_info() on fb_info should call ->fb_destroy
(offb_destroy in our case) and remove_conflicting_framebuffers()
is calling put_fb_info() so there is some extra reference on
fb_info somewhere preventing it from going away.
Please look into fixing this."
I am not familiar with the fb stuff internals but here is what I see:
# modprobe radeonfb
leads to:
[ 52.058546] bus: 'pci': add driver radeonfb
[ 52.058588] bus: 'pci': driver_probe_device: matched device
0000:00:10.0 with driver radeonfb
[ 52.058595] bus: 'pci': really_probe: probing driver radeonfb with
device 0000:00:10.0
[ 52.058608] devices_kset: Moving 0000:00:10.0 to end of list
[ 52.058613] radeonfb_pci_register BEGIN
[ 52.058634] radeonfb 0000:00:10.0: enabling device (0006 -> 0007)
<at this point radeon_kick_out_firmware_fb is called>
[ 52.058666] checking generic (9c008000 96000) vs hw (98000000 8000000)
[ 52.058667] fb: switching to radeonfb from OFfb ATY,RockHo
[ 52.058844] Console: switching to colour dummy device 80x25
[ 52.058860] device: 'fb0': device_unregister
[ 52.058956] PM: Removing info for No Bus:fb0
[ 52.059014] device: 'fb0': device_create_release
<a call to do_unregister_framebuffer is done>
<put_fb_info is done with a count=2 and dev=NULL>
[ 52.059048] device: 'vtcon1': device_unregister
[ 52.059076] PM: Removing info for No Bus:vtcon1
[ 52.059091] device: 'vtcon1': device_create_release
[ 52.059107] radeonfb 0000:00:10.0: BAR 0: can't reserve [mem
0x98000000-0x9fffffff pref]
[ 52.256151] aper_base: 98000000 MC_FB_LOC to: 9bff9800, MC_AGP_LOC
to: ffffa000
[ 52.256157] radeonfb (0000:00:10.0): Found 32768k of DDR 64 bits
wide videoram
I can confirm that offb_destroy is never called (not sure exactly
why), but in any case the call to radeon_kick_out_firmware_fb happen
much earlier, at least before the put_fb_info.
It is okay, put_fb_info() is called indirectly by radeon_kick_out_firmware_fb()
radeon_kick_out_firmware_fb()
remove_conflicting_framebuffers()
do_remove_conflicting_framebuffers()
do_unregister_framebuffer()
put_fb_info()
offb_destroy() is not called because there is an extra reference on old
fb_info (->count = 2):
static void put_fb_info(struct fb_info *fb_info)
{
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
The question is why there is an extra reference, probably user-space
is still holding the fb_info reference obtained in fb_open() call and
fb_release() is never called. Besides not calling fbops->fb_destroy()
this also causes missing call of fbops->fb_release() (in fb_release())
which some fb drivers are implementing (but not offb.c).
quoted
Could you describe a bit more the chain of calls you were thinking of ?
Please add WARN_ON(1) to get_fb_info() and put_fb_info() so we can check
from the stacktrace if it is actually fb_open() that holds the extra
old fb_info reference.
drivers/video/fbdev/core/fbmem.c:
static struct fb_info *get_fb_info(unsigned int idx)
{
struct fb_info *fb_info;
if (idx >= FB_MAX)
return ERR_PTR(-ENODEV);
mutex_lock(®istration_lock);
fb_info = registered_fb[idx];
if (fb_info)
atomic_inc(&fb_info->count);
if (fb_info)
WARN_ON(1);
mutex_unlock(®istration_lock);
return fb_info;
}
static void put_fb_info(struct fb_info *fb_info)
{
WARN_ON(1);
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id9741
Suggested-by: Lennart Sorensen <redacted>
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since the conflicts in region is due to OFfb explicitly refers to it.
It seems that there may still be configurations when this is
incorrect -> when offb drives primary (non-radeon) card and radeonfb
drives secondary (radeon) card..
@@ -2312,19 +2328,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)+returnret;+/* request the mem regions */ret=pci_request_region(pdev,0,"radeonfb framebuffer");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 0.\n",pci_name(rinfo->pdev));gotoerr_release_fb;+#endif}ret=pci_request_region(pdev,2,"radeonfb mmio");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 2.\n",pci_name(rinfo->pdev));gotoerr_release_pci0;+#endif}/* map the regions */
@@ -2509,10 +2533,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,iounmap(rinfo->mmio_base);err_release_pci2:pci_release_region(pdev,2);+#ifndef CONFIG_FB_OFerr_release_pci0:pci_release_region(pdev,0);err_release_fb:framebuffer_release(info);+#endiferr_disable:err_out:returnret;
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Thanks,
-M
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
From: kbuild test robot <hidden> Date: 2018-02-03 02:40:20
Hi Mathieu,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.15 next-20180202]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Mathieu-Malaterre/Fix-loading-of-module-radeonfb-on-PowerMac/20180203-085907
config: x86_64-randconfig-x009-201804 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from drivers/video/fbdev/aty/radeon_base.c:91:0:
On Wednesday, January 31, 2018 08:51:23 PM Mathieu Malaterre wrote:
Bartlomiej,
On Wed, Jan 31, 2018 at 12:57 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Tuesday, January 30, 2018 02:14:10 PM Mathieu Malaterre wrote:
quoted
Bartlomiej,
On Wed, Jan 3, 2018 at 3:47 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Thursday, December 21, 2017 11:07:56 PM 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.
This still needs to be explained more, from my last mail:
"The last put_fb_info() on fb_info should call ->fb_destroy
(offb_destroy in our case) and remove_conflicting_framebuffers()
is calling put_fb_info() so there is some extra reference on
fb_info somewhere preventing it from going away.
Please look into fixing this."
I am not familiar with the fb stuff internals but here is what I see:
# modprobe radeonfb
leads to:
[ 52.058546] bus: 'pci': add driver radeonfb
[ 52.058588] bus: 'pci': driver_probe_device: matched device
0000:00:10.0 with driver radeonfb
[ 52.058595] bus: 'pci': really_probe: probing driver radeonfb with
device 0000:00:10.0
[ 52.058608] devices_kset: Moving 0000:00:10.0 to end of list
[ 52.058613] radeonfb_pci_register BEGIN
[ 52.058634] radeonfb 0000:00:10.0: enabling device (0006 -> 0007)
<at this point radeon_kick_out_firmware_fb is called>
[ 52.058666] checking generic (9c008000 96000) vs hw (98000000 8000000)
[ 52.058667] fb: switching to radeonfb from OFfb ATY,RockHo
[ 52.058844] Console: switching to colour dummy device 80x25
[ 52.058860] device: 'fb0': device_unregister
[ 52.058956] PM: Removing info for No Bus:fb0
[ 52.059014] device: 'fb0': device_create_release
<a call to do_unregister_framebuffer is done>
<put_fb_info is done with a count=2 and dev=NULL>
[ 52.059048] device: 'vtcon1': device_unregister
[ 52.059076] PM: Removing info for No Bus:vtcon1
[ 52.059091] device: 'vtcon1': device_create_release
[ 52.059107] radeonfb 0000:00:10.0: BAR 0: can't reserve [mem
0x98000000-0x9fffffff pref]
[ 52.256151] aper_base: 98000000 MC_FB_LOC to: 9bff9800, MC_AGP_LOC
to: ffffa000
[ 52.256157] radeonfb (0000:00:10.0): Found 32768k of DDR 64 bits
wide videoram
I can confirm that offb_destroy is never called (not sure exactly
why), but in any case the call to radeon_kick_out_firmware_fb happen
much earlier, at least before the put_fb_info.
It is okay, put_fb_info() is called indirectly by radeon_kick_out_firmware_fb()
radeon_kick_out_firmware_fb()
remove_conflicting_framebuffers()
do_remove_conflicting_framebuffers()
do_unregister_framebuffer()
put_fb_info()
offb_destroy() is not called because there is an extra reference on old
fb_info (->count = 2):
static void put_fb_info(struct fb_info *fb_info)
{
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
The question is why there is an extra reference, probably user-space
is still holding the fb_info reference obtained in fb_open() call and
fb_release() is never called. Besides not calling fbops->fb_destroy()
this also causes missing call of fbops->fb_release() (in fb_release())
which some fb drivers are implementing (but not offb.c).
quoted
Could you describe a bit more the chain of calls you were thinking of ?
Please add WARN_ON(1) to get_fb_info() and put_fb_info() so we can check
from the stacktrace if it is actually fb_open() that holds the extra
old fb_info reference.
drivers/video/fbdev/core/fbmem.c:
static struct fb_info *get_fb_info(unsigned int idx)
{
struct fb_info *fb_info;
if (idx >= FB_MAX)
return ERR_PTR(-ENODEV);
mutex_lock(®istration_lock);
fb_info = registered_fb[idx];
if (fb_info)
atomic_inc(&fb_info->count);
if (fb_info)
WARN_ON(1);
mutex_unlock(®istration_lock);
return fb_info;
}
static void put_fb_info(struct fb_info *fb_info)
{
WARN_ON(1);
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
Thank you, it helps.
User-space is holding reference on the /dev/fb0 and old fb_info
(from offb) while offb is being replaced by radeonfb (this is
why ->fb_destroy is never called). You may try checking with
lsof command to see what is holding /dev/fb0 open..
Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id9741
Suggested-by: Lennart Sorensen <redacted>
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since the conflicts in region is due to OFfb explicitly refers to it.
It seems that there may still be configurations when this is
incorrect -> when offb drives primary (non-radeon) card and radeonfb
drives secondary (radeon) card..
@@ -2312,19 +2328,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)+returnret;+/* request the mem regions */ret=pci_request_region(pdev,0,"radeonfb framebuffer");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 0.\n",pci_name(rinfo->pdev));gotoerr_release_fb;+#endif}ret=pci_request_region(pdev,2,"radeonfb mmio");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 2.\n",pci_name(rinfo->pdev));gotoerr_release_pci0;+#endif}/* map the regions */
@@ -2509,10 +2533,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,iounmap(rinfo->mmio_base);err_release_pci2:pci_release_region(pdev,2);+#ifndef CONFIG_FB_OFerr_release_pci0:pci_release_region(pdev,0);err_release_fb:framebuffer_release(info);+#endiferr_disable:err_out:returnret;
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Hi,
On Thu, Feb 8, 2018 at 2:28 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
On Wednesday, January 31, 2018 08:51:23 PM Mathieu Malaterre wrote:
quoted
Bartlomiej,
On Wed, Jan 31, 2018 at 12:57 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Tuesday, January 30, 2018 02:14:10 PM Mathieu Malaterre wrote:
quoted
Bartlomiej,
On Wed, Jan 3, 2018 at 3:47 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Thursday, December 21, 2017 11:07:56 PM 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.
This still needs to be explained more, from my last mail:
"The last put_fb_info() on fb_info should call ->fb_destroy
(offb_destroy in our case) and remove_conflicting_framebuffers()
is calling put_fb_info() so there is some extra reference on
fb_info somewhere preventing it from going away.
Please look into fixing this."
I am not familiar with the fb stuff internals but here is what I see:
# modprobe radeonfb
leads to:
[ 52.058546] bus: 'pci': add driver radeonfb
[ 52.058588] bus: 'pci': driver_probe_device: matched device
0000:00:10.0 with driver radeonfb
[ 52.058595] bus: 'pci': really_probe: probing driver radeonfb with
device 0000:00:10.0
[ 52.058608] devices_kset: Moving 0000:00:10.0 to end of list
[ 52.058613] radeonfb_pci_register BEGIN
[ 52.058634] radeonfb 0000:00:10.0: enabling device (0006 -> 0007)
<at this point radeon_kick_out_firmware_fb is called>
[ 52.058666] checking generic (9c008000 96000) vs hw (98000000 8000000)
[ 52.058667] fb: switching to radeonfb from OFfb ATY,RockHo
[ 52.058844] Console: switching to colour dummy device 80x25
[ 52.058860] device: 'fb0': device_unregister
[ 52.058956] PM: Removing info for No Bus:fb0
[ 52.059014] device: 'fb0': device_create_release
<a call to do_unregister_framebuffer is done>
<put_fb_info is done with a count=2 and dev=NULL>
[ 52.059048] device: 'vtcon1': device_unregister
[ 52.059076] PM: Removing info for No Bus:vtcon1
[ 52.059091] device: 'vtcon1': device_create_release
[ 52.059107] radeonfb 0000:00:10.0: BAR 0: can't reserve [mem
0x98000000-0x9fffffff pref]
[ 52.256151] aper_base: 98000000 MC_FB_LOC to: 9bff9800, MC_AGP_LOC
to: ffffa000
[ 52.256157] radeonfb (0000:00:10.0): Found 32768k of DDR 64 bits
wide videoram
I can confirm that offb_destroy is never called (not sure exactly
why), but in any case the call to radeon_kick_out_firmware_fb happen
much earlier, at least before the put_fb_info.
It is okay, put_fb_info() is called indirectly by radeon_kick_out_firmware_fb()
radeon_kick_out_firmware_fb()
remove_conflicting_framebuffers()
do_remove_conflicting_framebuffers()
do_unregister_framebuffer()
put_fb_info()
offb_destroy() is not called because there is an extra reference on old
fb_info (->count = 2):
static void put_fb_info(struct fb_info *fb_info)
{
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
The question is why there is an extra reference, probably user-space
is still holding the fb_info reference obtained in fb_open() call and
fb_release() is never called. Besides not calling fbops->fb_destroy()
this also causes missing call of fbops->fb_release() (in fb_release())
which some fb drivers are implementing (but not offb.c).
quoted
Could you describe a bit more the chain of calls you were thinking of ?
Please add WARN_ON(1) to get_fb_info() and put_fb_info() so we can check
from the stacktrace if it is actually fb_open() that holds the extra
old fb_info reference.
drivers/video/fbdev/core/fbmem.c:
static struct fb_info *get_fb_info(unsigned int idx)
{
struct fb_info *fb_info;
if (idx >= FB_MAX)
return ERR_PTR(-ENODEV);
mutex_lock(®istration_lock);
fb_info = registered_fb[idx];
if (fb_info)
atomic_inc(&fb_info->count);
if (fb_info)
WARN_ON(1);
mutex_unlock(®istration_lock);
return fb_info;
}
static void put_fb_info(struct fb_info *fb_info)
{
WARN_ON(1);
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
Thank you, it helps.
User-space is holding reference on the /dev/fb0 and old fb_info
(from offb) while offb is being replaced by radeonfb (this is
why ->fb_destroy is never called). You may try checking with
lsof command to see what is holding /dev/fb0 open..
Right, I totally missed that X was running:
$ sudo lsof /dev/fb0
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
Xorg 469 root mem CHR 29,0 6500 /dev/fb0
Xorg 469 root 12u CHR 29,0 0t0 6500 /dev/fb0
so I simply:
$ dmesg > before_xdm_stop
$ sudo service xdm stop
$ dmesg > after_xdm_stop
$ sudo lsof /dev/fb0
-> nothing
And I can verify in dmesg the call to put_fb_info:
$ diff -u before_xdm_stop after_xdm_stop
@@ -1589,3 +1589,31 @@ [ 19.650088] gem 0002:20:0f.0 eth0: Link is up at 100 Mbps, full-duplex [ 19.650211] gem 0002:20:0f.0 eth0: Pause is disabled [ 19.650245] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready+[ 38.545478] WARNING: CPU: 0 PID: 468 at
drivers/video/fbdev/core/fbmem.c:77 put_fb_info+0x18/0x68
+[ 38.545772] Modules linked in: uinput arc4 b43 bcma mac80211
snd_aoa_codec_toonie snd_aoa_fabric_layout snd_aoa sha256_generic
cfg80211 evdev sg snd_aoa_i2sbus snd_aoa_soundbus snd_pcm snd_timer
snd soundcore ssb usb_storage autofs4 ext4 crc16 mbcache jbd2 fscrypto
usbhid ohci_pci ehci_pci ohci_hcd ehci_hcd sungem firewire_ohci
sungem_phy firewire_core crc_itu_t sr_mod usbcore cdrom sd_mod
nls_base usb_common
+[ 38.546894] CPU: 0 PID: 468 Comm: Xorg Tainted: G W
4.15.0+ #31
+[ 38.547103] NIP: c03083ec LR: c0308bb0 CTR: c0308b48
+[ 38.547252] REGS: de661dc0 TRAP: 0700 Tainted: G W
(4.15.0+)
+[ 38.547459] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 22002222 XER: 20000000
+[ 38.547661]
+ GPR00: c0308bb0 de661e70 ddd108c0 dee02c00 de0febe0
de0febe8 00000001 ddd108c0
+ GPR08: 00000000 c07b0a40 00000000 df501e10 22002428
007e0ff4 00000000 00000000
+ GPR16: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
+ GPR24: ddd108c0 de0febe8 dede1c10 df30bbb0 df501e10
dede1c10 dee02c10 dee02c00
+[ 38.548684] NIP [c03083ec] put_fb_info+0x18/0x68
+[ 38.548821] LR [c0308bb0] fb_release+0x68/0x80
+[ 38.548951] Call Trace:
+[ 38.549026] [de661e70] [dede1c10] 0xdede1c10 (unreliable)
+[ 38.549186] [de661e80] [c0308bb0] fb_release+0x68/0x80
+[ 38.549344] [de661e90] [c01c1198] __fput+0xac/0x20c
+[ 38.553889] [de661ec0] [c0062140] task_work_run+0xc0/0xe8
+[ 38.558437] [de661ee0] [c0048918] do_exit+0x268/0x964
+[ 38.562960] [de661f20] [c00490b4] do_group_exit+0x4c/0xb0
+[ 38.567400] [de661f30] [c0049138] __wake_up_parent+0x0/0x3c
+[ 38.571740] [de661f40] [c00151bc] ret_from_syscall+0x0/0x38
+[ 38.575965] --- interrupt: c01 at 0xb794a778
+ LR = 0xb794a748
+[ 38.584039] Instruction dump:
+[ 38.587878] 80010014 38210010 7c0803a6 4e800020 38600000 4e800020
7c0802a6 90010004
+[ 38.595537] 60000000 9421fff0 7c0802a6 90010014 <0fe00000>
7d401828 314affff 7d40192d
+[ 38.603316] ---[ end trace ee2e036160cab00c ]---
Now with your WARN_ON patch and xdm service stopped, here is what I get:
https://people.debian.org/~malat/full_xdm_stop_offb_without.log
You'll see that modprobe to radeonfb still fails.
If you now compare with the patch (v4) applied (+ WARN_ON and xdm
service stopped):
https://people.debian.org/~malat/full_xdm_stop_offb_with.log
You'll see a printk INFO for the call to offb_destroy:
...
[ 48.025983] MM calling offb_destroy
...
offb_destroy is called too late, which explains the failure for
loading radeonfb without the patch.
-M
Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id9741
Suggested-by: Lennart Sorensen <redacted>
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since the conflicts in region is due to OFfb explicitly refers to it.
It seems that there may still be configurations when this is
incorrect -> when offb drives primary (non-radeon) card and radeonfb
drives secondary (radeon) card..
@@ -2312,19 +2328,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)+returnret;+/* request the mem regions */ret=pci_request_region(pdev,0,"radeonfb framebuffer");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 0.\n",pci_name(rinfo->pdev));gotoerr_release_fb;+#endif}ret=pci_request_region(pdev,2,"radeonfb mmio");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 2.\n",pci_name(rinfo->pdev));gotoerr_release_pci0;+#endif}/* map the regions */
@@ -2509,10 +2533,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,iounmap(rinfo->mmio_base);err_release_pci2:pci_release_region(pdev,2);+#ifndef CONFIG_FB_OFerr_release_pci0:pci_release_region(pdev,0);err_release_fb:framebuffer_release(info);+#endiferr_disable:err_out:returnret;
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
On Saturday, February 10, 2018 01:48:55 PM Mathieu Malaterre wrote:
quoted hunk
Hi,
On Thu, Feb 8, 2018 at 2:28 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Wednesday, January 31, 2018 08:51:23 PM Mathieu Malaterre wrote:
quoted
Bartlomiej,
On Wed, Jan 31, 2018 at 12:57 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Tuesday, January 30, 2018 02:14:10 PM Mathieu Malaterre wrote:
quoted
Bartlomiej,
On Wed, Jan 3, 2018 at 3:47 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Thursday, December 21, 2017 11:07:56 PM 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.
This still needs to be explained more, from my last mail:
"The last put_fb_info() on fb_info should call ->fb_destroy
(offb_destroy in our case) and remove_conflicting_framebuffers()
is calling put_fb_info() so there is some extra reference on
fb_info somewhere preventing it from going away.
Please look into fixing this."
I am not familiar with the fb stuff internals but here is what I see:
# modprobe radeonfb
leads to:
[ 52.058546] bus: 'pci': add driver radeonfb
[ 52.058588] bus: 'pci': driver_probe_device: matched device
0000:00:10.0 with driver radeonfb
[ 52.058595] bus: 'pci': really_probe: probing driver radeonfb with
device 0000:00:10.0
[ 52.058608] devices_kset: Moving 0000:00:10.0 to end of list
[ 52.058613] radeonfb_pci_register BEGIN
[ 52.058634] radeonfb 0000:00:10.0: enabling device (0006 -> 0007)
<at this point radeon_kick_out_firmware_fb is called>
[ 52.058666] checking generic (9c008000 96000) vs hw (98000000 8000000)
[ 52.058667] fb: switching to radeonfb from OFfb ATY,RockHo
[ 52.058844] Console: switching to colour dummy device 80x25
[ 52.058860] device: 'fb0': device_unregister
[ 52.058956] PM: Removing info for No Bus:fb0
[ 52.059014] device: 'fb0': device_create_release
<a call to do_unregister_framebuffer is done>
<put_fb_info is done with a count=2 and dev=NULL>
[ 52.059048] device: 'vtcon1': device_unregister
[ 52.059076] PM: Removing info for No Bus:vtcon1
[ 52.059091] device: 'vtcon1': device_create_release
[ 52.059107] radeonfb 0000:00:10.0: BAR 0: can't reserve [mem
0x98000000-0x9fffffff pref]
[ 52.256151] aper_base: 98000000 MC_FB_LOC to: 9bff9800, MC_AGP_LOC
to: ffffa000
[ 52.256157] radeonfb (0000:00:10.0): Found 32768k of DDR 64 bits
wide videoram
I can confirm that offb_destroy is never called (not sure exactly
why), but in any case the call to radeon_kick_out_firmware_fb happen
much earlier, at least before the put_fb_info.
It is okay, put_fb_info() is called indirectly by radeon_kick_out_firmware_fb()
radeon_kick_out_firmware_fb()
remove_conflicting_framebuffers()
do_remove_conflicting_framebuffers()
do_unregister_framebuffer()
put_fb_info()
offb_destroy() is not called because there is an extra reference on old
fb_info (->count = 2):
static void put_fb_info(struct fb_info *fb_info)
{
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
The question is why there is an extra reference, probably user-space
is still holding the fb_info reference obtained in fb_open() call and
fb_release() is never called. Besides not calling fbops->fb_destroy()
this also causes missing call of fbops->fb_release() (in fb_release())
which some fb drivers are implementing (but not offb.c).
quoted
Could you describe a bit more the chain of calls you were thinking of ?
Please add WARN_ON(1) to get_fb_info() and put_fb_info() so we can check
from the stacktrace if it is actually fb_open() that holds the extra
old fb_info reference.
drivers/video/fbdev/core/fbmem.c:
static struct fb_info *get_fb_info(unsigned int idx)
{
struct fb_info *fb_info;
if (idx >= FB_MAX)
return ERR_PTR(-ENODEV);
mutex_lock(®istration_lock);
fb_info = registered_fb[idx];
if (fb_info)
atomic_inc(&fb_info->count);
if (fb_info)
WARN_ON(1);
mutex_unlock(®istration_lock);
return fb_info;
}
static void put_fb_info(struct fb_info *fb_info)
{
WARN_ON(1);
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
Thank you, it helps.
User-space is holding reference on the /dev/fb0 and old fb_info
(from offb) while offb is being replaced by radeonfb (this is
why ->fb_destroy is never called). You may try checking with
lsof command to see what is holding /dev/fb0 open..
Right, I totally missed that X was running:
$ sudo lsof /dev/fb0
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
Xorg 469 root mem CHR 29,0 6500 /dev/fb0
Xorg 469 root 12u CHR 29,0 0t0 6500 /dev/fb0
so I simply:
$ dmesg > before_xdm_stop
$ sudo service xdm stop
$ dmesg > after_xdm_stop
$ sudo lsof /dev/fb0
-> nothing
And I can verify in dmesg the call to put_fb_info:
$ diff -u before_xdm_stop after_xdm_stop
@@ -1589,3 +1589,31 @@ [ 19.650088] gem 0002:20:0f.0 eth0: Link is up at 100 Mbps, full-duplex [ 19.650211] gem 0002:20:0f.0 eth0: Pause is disabled [ 19.650245] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready+[ 38.545478] WARNING: CPU: 0 PID: 468 at
drivers/video/fbdev/core/fbmem.c:77 put_fb_info+0x18/0x68
+[ 38.545772] Modules linked in: uinput arc4 b43 bcma mac80211
snd_aoa_codec_toonie snd_aoa_fabric_layout snd_aoa sha256_generic
cfg80211 evdev sg snd_aoa_i2sbus snd_aoa_soundbus snd_pcm snd_timer
snd soundcore ssb usb_storage autofs4 ext4 crc16 mbcache jbd2 fscrypto
usbhid ohci_pci ehci_pci ohci_hcd ehci_hcd sungem firewire_ohci
sungem_phy firewire_core crc_itu_t sr_mod usbcore cdrom sd_mod
nls_base usb_common
+[ 38.546894] CPU: 0 PID: 468 Comm: Xorg Tainted: G W
4.15.0+ #31
+[ 38.547103] NIP: c03083ec LR: c0308bb0 CTR: c0308b48
+[ 38.547252] REGS: de661dc0 TRAP: 0700 Tainted: G W
(4.15.0+)
+[ 38.547459] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 22002222 XER: 20000000
+[ 38.547661]
+ GPR00: c0308bb0 de661e70 ddd108c0 dee02c00 de0febe0
de0febe8 00000001 ddd108c0
+ GPR08: 00000000 c07b0a40 00000000 df501e10 22002428
007e0ff4 00000000 00000000
+ GPR16: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
+ GPR24: ddd108c0 de0febe8 dede1c10 df30bbb0 df501e10
dede1c10 dee02c10 dee02c00
+[ 38.548684] NIP [c03083ec] put_fb_info+0x18/0x68
+[ 38.548821] LR [c0308bb0] fb_release+0x68/0x80
+[ 38.548951] Call Trace:
+[ 38.549026] [de661e70] [dede1c10] 0xdede1c10 (unreliable)
+[ 38.549186] [de661e80] [c0308bb0] fb_release+0x68/0x80
+[ 38.549344] [de661e90] [c01c1198] __fput+0xac/0x20c
+[ 38.553889] [de661ec0] [c0062140] task_work_run+0xc0/0xe8
+[ 38.558437] [de661ee0] [c0048918] do_exit+0x268/0x964
+[ 38.562960] [de661f20] [c00490b4] do_group_exit+0x4c/0xb0
+[ 38.567400] [de661f30] [c0049138] __wake_up_parent+0x0/0x3c
+[ 38.571740] [de661f40] [c00151bc] ret_from_syscall+0x0/0x38
+[ 38.575965] --- interrupt: c01 at 0xb794a778
+ LR = 0xb794a748
+[ 38.584039] Instruction dump:
+[ 38.587878] 80010014 38210010 7c0803a6 4e800020 38600000 4e800020
7c0802a6 90010004
+[ 38.595537] 60000000 9421fff0 7c0802a6 90010014 <0fe00000>
7d401828 314affff 7d40192d
+[ 38.603316] ---[ end trace ee2e036160cab00c ]---
Now with your WARN_ON patch and xdm service stopped, here is what I get:
https://people.debian.org/~malat/full_xdm_stop_offb_without.log
You'll see that modprobe to radeonfb still fails.
If you now compare with the patch (v4) applied (+ WARN_ON and xdm
service stopped):
https://people.debian.org/~malat/full_xdm_stop_offb_with.log
You'll see a printk INFO for the call to offb_destroy:
...
[ 48.025983] MM calling offb_destroy
...
offb_destroy is called too late, which explains the failure for
loading radeonfb without the patch.
offb_destroy() seems to be called in the right place but we still
need to kick out offb. IOW your patch is needed but with xdm stop
sequence it should be possible to drop ifdef hacks.
PS switch to drm_fb_helper_remove_conflicting_framebuffers() in v4
should be reverted to just use remove_conflicting_framebuffers() as
the former one is for DRM subsystem only.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id9741
Suggested-by: Lennart Sorensen <redacted>
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since the conflicts in region is due to OFfb explicitly refers to it.
It seems that there may still be configurations when this is
incorrect -> when offb drives primary (non-radeon) card and radeonfb
drives secondary (radeon) card..
@@ -2312,19 +2328,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)+returnret;+/* request the mem regions */ret=pci_request_region(pdev,0,"radeonfb framebuffer");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 0.\n",pci_name(rinfo->pdev));gotoerr_release_fb;+#endif}ret=pci_request_region(pdev,2,"radeonfb mmio");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 2.\n",pci_name(rinfo->pdev));gotoerr_release_pci0;+#endif}/* map the regions */
@@ -2509,10 +2533,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,iounmap(rinfo->mmio_base);err_release_pci2:pci_release_region(pdev,2);+#ifndef CONFIG_FB_OFerr_release_pci0:pci_release_region(pdev,0);err_release_fb:framebuffer_release(info);+#endiferr_disable:err_out:returnret;
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
On Tue, Feb 13, 2018 at 1:05 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
On Saturday, February 10, 2018 01:48:55 PM Mathieu Malaterre wrote:
quoted
Hi,
On Thu, Feb 8, 2018 at 2:28 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Wednesday, January 31, 2018 08:51:23 PM Mathieu Malaterre wrote:
quoted
Bartlomiej,
On Wed, Jan 31, 2018 at 12:57 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Tuesday, January 30, 2018 02:14:10 PM Mathieu Malaterre wrote:
quoted
Bartlomiej,
On Wed, Jan 3, 2018 at 3:47 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:
quoted
On Thursday, December 21, 2017 11:07:56 PM 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.
This still needs to be explained more, from my last mail:
"The last put_fb_info() on fb_info should call ->fb_destroy
(offb_destroy in our case) and remove_conflicting_framebuffers()
is calling put_fb_info() so there is some extra reference on
fb_info somewhere preventing it from going away.
Please look into fixing this."
I am not familiar with the fb stuff internals but here is what I see:
# modprobe radeonfb
leads to:
[ 52.058546] bus: 'pci': add driver radeonfb
[ 52.058588] bus: 'pci': driver_probe_device: matched device
0000:00:10.0 with driver radeonfb
[ 52.058595] bus: 'pci': really_probe: probing driver radeonfb with
device 0000:00:10.0
[ 52.058608] devices_kset: Moving 0000:00:10.0 to end of list
[ 52.058613] radeonfb_pci_register BEGIN
[ 52.058634] radeonfb 0000:00:10.0: enabling device (0006 -> 0007)
<at this point radeon_kick_out_firmware_fb is called>
[ 52.058666] checking generic (9c008000 96000) vs hw (98000000 8000000)
[ 52.058667] fb: switching to radeonfb from OFfb ATY,RockHo
[ 52.058844] Console: switching to colour dummy device 80x25
[ 52.058860] device: 'fb0': device_unregister
[ 52.058956] PM: Removing info for No Bus:fb0
[ 52.059014] device: 'fb0': device_create_release
<a call to do_unregister_framebuffer is done>
<put_fb_info is done with a count=2 and dev=NULL>
[ 52.059048] device: 'vtcon1': device_unregister
[ 52.059076] PM: Removing info for No Bus:vtcon1
[ 52.059091] device: 'vtcon1': device_create_release
[ 52.059107] radeonfb 0000:00:10.0: BAR 0: can't reserve [mem
0x98000000-0x9fffffff pref]
[ 52.256151] aper_base: 98000000 MC_FB_LOC to: 9bff9800, MC_AGP_LOC
to: ffffa000
[ 52.256157] radeonfb (0000:00:10.0): Found 32768k of DDR 64 bits
wide videoram
I can confirm that offb_destroy is never called (not sure exactly
why), but in any case the call to radeon_kick_out_firmware_fb happen
much earlier, at least before the put_fb_info.
It is okay, put_fb_info() is called indirectly by radeon_kick_out_firmware_fb()
radeon_kick_out_firmware_fb()
remove_conflicting_framebuffers()
do_remove_conflicting_framebuffers()
do_unregister_framebuffer()
put_fb_info()
offb_destroy() is not called because there is an extra reference on old
fb_info (->count = 2):
static void put_fb_info(struct fb_info *fb_info)
{
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
The question is why there is an extra reference, probably user-space
is still holding the fb_info reference obtained in fb_open() call and
fb_release() is never called. Besides not calling fbops->fb_destroy()
this also causes missing call of fbops->fb_release() (in fb_release())
which some fb drivers are implementing (but not offb.c).
quoted
Could you describe a bit more the chain of calls you were thinking of ?
Please add WARN_ON(1) to get_fb_info() and put_fb_info() so we can check
from the stacktrace if it is actually fb_open() that holds the extra
old fb_info reference.
drivers/video/fbdev/core/fbmem.c:
static struct fb_info *get_fb_info(unsigned int idx)
{
struct fb_info *fb_info;
if (idx >= FB_MAX)
return ERR_PTR(-ENODEV);
mutex_lock(®istration_lock);
fb_info = registered_fb[idx];
if (fb_info)
atomic_inc(&fb_info->count);
if (fb_info)
WARN_ON(1);
mutex_unlock(®istration_lock);
return fb_info;
}
static void put_fb_info(struct fb_info *fb_info)
{
WARN_ON(1);
if (!atomic_dec_and_test(&fb_info->count))
return;
if (fb_info->fbops->fb_destroy)
fb_info->fbops->fb_destroy(fb_info);
}
Thank you, it helps.
User-space is holding reference on the /dev/fb0 and old fb_info
(from offb) while offb is being replaced by radeonfb (this is
why ->fb_destroy is never called). You may try checking with
lsof command to see what is holding /dev/fb0 open..
Right, I totally missed that X was running:
$ sudo lsof /dev/fb0
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
Xorg 469 root mem CHR 29,0 6500 /dev/fb0
Xorg 469 root 12u CHR 29,0 0t0 6500 /dev/fb0
so I simply:
$ dmesg > before_xdm_stop
$ sudo service xdm stop
$ dmesg > after_xdm_stop
$ sudo lsof /dev/fb0
-> nothing
And I can verify in dmesg the call to put_fb_info:
$ diff -u before_xdm_stop after_xdm_stop
@@ -1589,3 +1589,31 @@ [ 19.650088] gem 0002:20:0f.0 eth0: Link is up at 100 Mbps, full-duplex [ 19.650211] gem 0002:20:0f.0 eth0: Pause is disabled [ 19.650245] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready+[ 38.545478] WARNING: CPU: 0 PID: 468 at
drivers/video/fbdev/core/fbmem.c:77 put_fb_info+0x18/0x68
+[ 38.545772] Modules linked in: uinput arc4 b43 bcma mac80211
snd_aoa_codec_toonie snd_aoa_fabric_layout snd_aoa sha256_generic
cfg80211 evdev sg snd_aoa_i2sbus snd_aoa_soundbus snd_pcm snd_timer
snd soundcore ssb usb_storage autofs4 ext4 crc16 mbcache jbd2 fscrypto
usbhid ohci_pci ehci_pci ohci_hcd ehci_hcd sungem firewire_ohci
sungem_phy firewire_core crc_itu_t sr_mod usbcore cdrom sd_mod
nls_base usb_common
+[ 38.546894] CPU: 0 PID: 468 Comm: Xorg Tainted: G W
4.15.0+ #31
+[ 38.547103] NIP: c03083ec LR: c0308bb0 CTR: c0308b48
+[ 38.547252] REGS: de661dc0 TRAP: 0700 Tainted: G W
(4.15.0+)
+[ 38.547459] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 22002222 XER: 20000000
+[ 38.547661]
+ GPR00: c0308bb0 de661e70 ddd108c0 dee02c00 de0febe0
de0febe8 00000001 ddd108c0
+ GPR08: 00000000 c07b0a40 00000000 df501e10 22002428
007e0ff4 00000000 00000000
+ GPR16: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
+ GPR24: ddd108c0 de0febe8 dede1c10 df30bbb0 df501e10
dede1c10 dee02c10 dee02c00
+[ 38.548684] NIP [c03083ec] put_fb_info+0x18/0x68
+[ 38.548821] LR [c0308bb0] fb_release+0x68/0x80
+[ 38.548951] Call Trace:
+[ 38.549026] [de661e70] [dede1c10] 0xdede1c10 (unreliable)
+[ 38.549186] [de661e80] [c0308bb0] fb_release+0x68/0x80
+[ 38.549344] [de661e90] [c01c1198] __fput+0xac/0x20c
+[ 38.553889] [de661ec0] [c0062140] task_work_run+0xc0/0xe8
+[ 38.558437] [de661ee0] [c0048918] do_exit+0x268/0x964
+[ 38.562960] [de661f20] [c00490b4] do_group_exit+0x4c/0xb0
+[ 38.567400] [de661f30] [c0049138] __wake_up_parent+0x0/0x3c
+[ 38.571740] [de661f40] [c00151bc] ret_from_syscall+0x0/0x38
+[ 38.575965] --- interrupt: c01 at 0xb794a778
+ LR = 0xb794a748
+[ 38.584039] Instruction dump:
+[ 38.587878] 80010014 38210010 7c0803a6 4e800020 38600000 4e800020
7c0802a6 90010004
+[ 38.595537] 60000000 9421fff0 7c0802a6 90010014 <0fe00000>
7d401828 314affff 7d40192d
+[ 38.603316] ---[ end trace ee2e036160cab00c ]---
Now with your WARN_ON patch and xdm service stopped, here is what I get:
https://people.debian.org/~malat/full_xdm_stop_offb_without.log
You'll see that modprobe to radeonfb still fails.
If you now compare with the patch (v4) applied (+ WARN_ON and xdm
service stopped):
https://people.debian.org/~malat/full_xdm_stop_offb_with.log
You'll see a printk INFO for the call to offb_destroy:
...
[ 48.025983] MM calling offb_destroy
...
offb_destroy is called too late, which explains the failure for
loading radeonfb without the patch.
offb_destroy() seems to be called in the right place but we still
need to kick out offb. IOW your patch is needed but with xdm stop
sequence it should be possible to drop ifdef hacks.
/facepalm/ I can't believe the solution was in front of me from day one.
PS switch to drm_fb_helper_remove_conflicting_framebuffers() in v4
should be reverted to just use remove_conflicting_framebuffers() as
the former one is for DRM subsystem only.
Ah, great ! Thanks for the clarification. v5 sent.
Thanks much !
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Signed-off-by: Mathieu Malaterre <redacted>
Link: https://bugs.debian.org/826629#57
Link: https://bugzilla.kernel.org/show_bug.cgi?id9741
Suggested-by: Lennart Sorensen <redacted>
---
v2: Only fails when CONFIG_PCC is not set
v3: Only fails when CONFIG_FB_OF is not set, CONFIG_PCC was too broad. Since the conflicts in region is due to OFfb explicitly refers to it.
It seems that there may still be configurations when this is
incorrect -> when offb drives primary (non-radeon) card and radeonfb
drives secondary (radeon) card..
@@ -2312,19 +2328,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)+returnret;+/* request the mem regions */ret=pci_request_region(pdev,0,"radeonfb framebuffer");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 0.\n",pci_name(rinfo->pdev));gotoerr_release_fb;+#endif}ret=pci_request_region(pdev,2,"radeonfb mmio");if(ret<0){+#ifndef CONFIG_FB_OFprintk(KERN_ERR"radeonfb (%s): cannot request region 2.\n",pci_name(rinfo->pdev));gotoerr_release_pci0;+#endif}/* map the regions */
@@ -2509,10 +2533,12 @@ static int radeonfb_pci_register(struct pci_dev *pdev,iounmap(rinfo->mmio_base);err_release_pci2:pci_release_region(pdev,2);+#ifndef CONFIG_FB_OFerr_release_pci0:pci_release_region(pdev,0);err_release_fb:framebuffer_release(info);+#endiferr_disable:err_out:returnret;
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics