[PATCH] drm/vmwgfx: Stop requesting the pci regions

[Zack Rusin]

On Wed, 2022-01-19 at 10:13 +0100, Thomas Zimmermann wrote:
> 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 at 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.
> 
> If this doesn't work, pushing all request/release pairs into drivers 
> would be my next option.
> 
> If none of this is feasible, we can still remove pci_request_region()
> from vmwgfx.


I think that's orthogonal to the fix because having pci_request_region
makes vmwgfx behave differently from majority of DRM drivers, e.g. on
systems with sysfb enabled with 5.15 vmwgfx fails to boot and leaves
the system broken without any fb driver (because while we have
*remove_conflicting*_framebuffers we don't have drm_restore_system_fb
or such to load back the boot fb after drm driver load fails) but since
it's one of the few drivers which does request regions it took a bit
for us to notice.

So in this case I'd much rather be like the other drivers rather than
correct because it lowers the odds of vmwgfx breaking in the future.

z