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

[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 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.

Best regards
Thomas

> 
> 
> 
>>> Like the other drm drivers we need to stop requesting all the pci
>>> regions
>>> to let the driver load with platform code enabled.
>>> This allows vmwgfx to load correctly on systems with sysfb_simple
>>> enabled.
>>>
>>
>> I read this very interesting thread from two years ago:
>>
>> https://lkml.org/lkml/2020/11/5/248
>>
>> Maybe is worth mentioning in the commit message what Daniel said there,
>> that is that only a few DRM drivers request explicitly the PCI regions
>> and the only reliable approach is for bus drivers to claim these.
> 
> Ah, great point. I'll update the commit log with that.
> 
>>> Signed-off-by: Zack Rusin <zackr at vmware.com>
>>> Fixes: 523375c943e5 ("drm/vmwgfx: Port vmwgfx to arm64")
>>> Cc: dri-devel at lists.freedesktop.org
>>> Cc: <stable at vger.kernel.org>
>>> Reviewed-by: Martin Krastev <krastevm at vmware.com>
>>> ---
>>
>> The patch looks good to me, thanks a lot for fixing this:
>>
>> Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>
> 
> Thanks for taking a look at this, I appreciate it!
> 
> z

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20220119/1e0d91d4/attachment.sig>