Hi Am 05.04.23 um 09:49 schrieb Thomas Zimmermann:
Hi Am 04.04.23 um 22:18 schrieb Daniel Vetter:This one nukes all framebuffers, which is a bit much. In reality gma500 is igpu and never shipped with anything discrete, so there should not be any difference. v2: Unfortunately the framebuffer sits outside of the pci bars for gma500, and so only using the pci helpers won't be enough. Otoh if we only use non-pci helper, then we don't get the vga handling, and subsequent refactoring to untangle these special cases won't work. It's not pretty, but the simplest fix (since gma500 really is the only quirky pci driver like this we have) is to just have both calls. Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> Cc: Patrik Jakobsson <patrik.r.jakobsson@xxxxxxxxx> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> Cc: Javier Martinez Canillas <javierm@xxxxxxxxxx> --- drivers/gpu/drm/gma500/psb_drv.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.cindex 2ce96b1b9c74..f1e0eed8fea4 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c@@ -422,12 +422,17 @@ static int psb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)/** We cannot yet easily find the framebuffer's location in memory. So- * remove all framebuffers here.+ * remove all framebuffers here. Note that we still want the pci special+ * handling to kick out vgacon. * * TODO: Refactor psb_driver_load() to map vdc_reg earlier. Then we* might be able to read the framebuffer range from the device.*/ - ret = drm_aperture_remove_framebuffers(true, &driver); + ret = drm_aperture_remove_framebuffers(false, &driver); + if (ret) + return ret; ++ ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &driver);This simply isn't it. If you have to work around your own API, it's time to rethink the API.
Here's a proposal:1) As you're changing aperture_remove_conflicting_devices() anyway, rename it to aperture_remove_conflicting_devices_at(), so it's clear that it takes a memory range.
2) Introduce aperture_remove_conflicting_pci_devices_at(), which takes a PCI device and a memory range. It should do the is_primary and vgacon stuff, but kick out the range.
3) Call aperture_remove_conflicting_pci_devices_at() from gma500 with the full range. Even if we can restructure gma500 to detect the firmware framebuffer from its registers (there's this TODO item), we'd still need aperture_remove_conflicting_pci_devices_at() to do something useful with it.
4) With that, aperture_remove_conflicting_devices_at() can drop the primary argument.
Of course, the DRM-related interface should be adapted as well. There's a bit of overlap in the implementation of both PCI aperture helpers, but the overall interface is clear.
Best regards Thomas
Best regards Thomasif (ret) return ret;
-- 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
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
