On Wed, 2022-01-19 at 16:50 +0100, Thomas Zimmermann wrote:
Hi
Am 19.01.22 um 16:12 schrieb Zack Rusin:
On Wed, 2022-01-19 at 15:00 +0100, Thomas Zimmermann wrote:
Hi Zack
Am 19.01.22 um 10:13 schrieb Thomas Zimmermann:
Hi
Am 19.01.22 um 03:15 schrieb Zack Rusin:
On Tue, 2022-01-18 at 20:00 +0100, Javier Martinez Canillas wrote:
Hello Zack,
On 1/17/22 19:03, Zack Rusin wrote: > From: Zack Rusin zackr@vmware.com > > When sysfb_simple is enabled loading vmwgfx fails because > the > regions > are held by the platform. In that case > remove_conflicting*_framebuffers > only removes the simplefb but not the regions held by > sysfb. >
Indeed, that's an issue. I wonder if we should drop the IORESOURCE_BUSY flag from the memory resource added to the "simple- framebuffer" device ?
I think this is one of those cases where it depends on what we plan to do after that. Sementically it makes sense to have it in there - the framebuffer memory is claimed by the "simple-framebuffer" and it's busy, it's just that it creates issues for drivers after unloading. I think removing it, while making things easier for drivers, would be confusing for people reading the code later, unless there's some kind of cleanup that would happen with it (e.g. removing IORESOURCE_BUSY altogether and making the drm drivers properly request their resources). At least by itself it doesn't seem to be much better solution than having the drm drivers not call pci_request_region[s], which apart from hyperv and cirrus (iirc bochs does it for resources other than fb which wouldn't have been claimed by "simple- framebuffer") is already the case.
I do think we should do one of them to make the codebase coherent: either remove IORESOURCE_BUSY from "simple-framebuffer" or remove pci_request_region[s] from hyperv and cirrus.
I just discussed this a bit with Javier. It's a problem with the simple-framebuffer code, rather then vmwgfx.
IMHO the best solution is to drop IORESOURCE_BUSY from sysfb and have drivers register/release the range with _BUSY. That would signal the memory belongs to the sysfb device but is not busy unless a driver has been bound. After simplefb released the range, it should be 'non- busy' again and available for vmwgfx. Simpledrm does a hot-unplug of the sysfb device, so the memory range gets released entirely. If you want, I'll prepare some patches for this scenario.
Attached is a patch that implements this. Doing
cat /proc/iomem ... e0000000-efffffff : 0000:00:02.0
e0000000-e07e8fff : BOOTFB
e0000000-e07e8fff : simplefb
...
shows the memory. 'BOOTFB' is the simple-framebuffer device and 'simplefb' is the driver. Only the latter uses _BUSY. Same for and the memory canbe acquired by vmwgfx.
Zack, please test this patch. If it works, I'll send out the real patchset.
Hmm, the patch looks good but it doesn't work. After boot: /proc/iomem 50000000-7fffffff : pcie@0x40000000 78000000-7fffffff : 0000:00:0f.0 78000000-782fffff : BOOTFB
and vmwgfx fails on pci_request_regions:
kernel: fb0: switching to vmwgfx from simple kernel: Console: switching to colour dummy device 80x25 kernel: vmwgfx 0000:00:0f.0: BAR 2: can't reserve [mem 0x78000000- 0x7fffffff 64bit pref] kernel: vmwgfx: probe of 0000:00:0f.0 failed with error -16
leaving the system without a fb driver.
OK, I suspect that it would work if you use simpledrm instead of into the kernel binary.
Yes, simpledrm works fine. BTW, is there any remaining work before distros can enable it by default?
If that works, would you consider protecting pci_request_region() with #if not defined(CONFIG_FB_SIMPLE) #endif
with a FIXME comment?
Yes, I think that's a good compromise. I'll respin the patch with that.
z