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@xxxxxxxxxx> > > > > > > > > > > > > > > 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
