The vboxvideo driver is missing a call to remove conflicting framebuffers.
Surprisingly, when using legacy BIOS booting this does not really cause any issues. But when using UEFI to boot the VM then plymouth will draw on both the efifb /dev/fb0 and /dev/drm/card0 (which has registered /dev/fb1 as fbdev emulation).
VirtualBox will actual display the output of both devices (I guess it is showing whatever was drawn last), this causes weird artifacts because of pitch issues in the efifb when the VM window is not sized at 1024x768 (the window will resize to its last size once the vboxvideo driver loads, changing the pitch).
Adding the missing drm_fb_helper_remove_conflicting_pci_framebuffers() call fixes this.
Cc: stable@vger.kernel.org Fixes: 2695eae1f6d3 ("drm/vboxvideo: Switch to generic fbdev emulation") Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/gpu/drm/vboxvideo/vbox_drv.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index 8512d970a09f..261255085918 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -76,6 +76,10 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_mode_fini;
+ ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "vboxvideodrmfb"); + if (ret) + goto err_irq_fini; + ret = drm_fbdev_generic_setup(&vbox->ddev, 32); if (ret) goto err_irq_fini;
On Wed, Mar 25, 2020 at 03:43:10PM +0100, Hans de Goede wrote:
The vboxvideo driver is missing a call to remove conflicting framebuffers.
Surprisingly, when using legacy BIOS booting this does not really cause any issues. But when using UEFI to boot the VM then plymouth will draw on both the efifb /dev/fb0 and /dev/drm/card0 (which has registered /dev/fb1 as fbdev emulation).
VirtualBox will actual display the output of both devices (I guess it is showing whatever was drawn last), this causes weird artifacts because of pitch issues in the efifb when the VM window is not sized at 1024x768 (the window will resize to its last size once the vboxvideo driver loads, changing the pitch).
Adding the missing drm_fb_helper_remove_conflicting_pci_framebuffers() call fixes this.
Cc: stable@vger.kernel.org Fixes: 2695eae1f6d3 ("drm/vboxvideo: Switch to generic fbdev emulation") Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/gpu/drm/vboxvideo/vbox_drv.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index 8512d970a09f..261255085918 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -76,6 +76,10 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_mode_fini;
- ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "vboxvideodrmfb");
- if (ret)
goto err_irq_fini;
To avoid transient issues this should be done as early as possible, definitely before the drm driver starts to touch the "hw". With that
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
I do wonder though why the automatic removal of conflicting framebuffers doesn't work, fbdev should already do that from register_framebuffer(), which is called somewhere in drm_fbdev_generic_setup (after a few layers).
Did you check why the two framebuffers don't conflict, and why the uefi one doesn't get thrown out? -Daniel
- ret = drm_fbdev_generic_setup(&vbox->ddev, 32); if (ret) goto err_irq_fini;
-- 2.26.0.rc2
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi,
On 3/26/20 12:29 PM, Daniel Vetter wrote:
On Wed, Mar 25, 2020 at 03:43:10PM +0100, Hans de Goede wrote:
The vboxvideo driver is missing a call to remove conflicting framebuffers.
Surprisingly, when using legacy BIOS booting this does not really cause any issues. But when using UEFI to boot the VM then plymouth will draw on both the efifb /dev/fb0 and /dev/drm/card0 (which has registered /dev/fb1 as fbdev emulation).
VirtualBox will actual display the output of both devices (I guess it is showing whatever was drawn last), this causes weird artifacts because of pitch issues in the efifb when the VM window is not sized at 1024x768 (the window will resize to its last size once the vboxvideo driver loads, changing the pitch).
Adding the missing drm_fb_helper_remove_conflicting_pci_framebuffers() call fixes this.
Cc: stable@vger.kernel.org Fixes: 2695eae1f6d3 ("drm/vboxvideo: Switch to generic fbdev emulation") Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/gpu/drm/vboxvideo/vbox_drv.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index 8512d970a09f..261255085918 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -76,6 +76,10 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_mode_fini;
- ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "vboxvideodrmfb");
- if (ret)
goto err_irq_fini;To avoid transient issues this should be done as early as possible, definitely before the drm driver starts to touch the "hw".>
Ok will fix and then push this to drm-misc-next-fixes, thank you.
With that
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
I do wonder though why the automatic removal of conflicting framebuffers doesn't work, fbdev should already do that from register_framebuffer(), which is called somewhere in drm_fbdev_generic_setup (after a few layers).
Did you check why the two framebuffers don't conflict, and why the uefi one doesn't get thrown out?
I did not check, I was not aware that this check was already happening in register_framebuffer(). So I just checked and the reason why this is not working is because the fbdev emulation done by drm_fbdev_generic_setup does not fill in fb_info.apertures->ranges[0] .
So fb_info.apertures->ranges[0].base is left as 0 which does not match with the registered efifb's aperture.
We could try to fix this, but that is not entirely trivial, we would need to pass the pci_dev pointer down into drm_fb_helper_alloc_fbi() then and then like remove_conflicting_pci_framebuffers() does add apertures for all IORESOURCE_MEM PCI bars, but that would conflict with drivers which do rely on drm_fb_helper_alloc_fbi() currently allocating one empty aperture and then actually fill that itself...
Regards,
Hans
On Thu, Mar 26, 2020 at 2:18 PM Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 3/26/20 12:29 PM, Daniel Vetter wrote:
On Wed, Mar 25, 2020 at 03:43:10PM +0100, Hans de Goede wrote:
The vboxvideo driver is missing a call to remove conflicting framebuffers.
Surprisingly, when using legacy BIOS booting this does not really cause any issues. But when using UEFI to boot the VM then plymouth will draw on both the efifb /dev/fb0 and /dev/drm/card0 (which has registered /dev/fb1 as fbdev emulation).
VirtualBox will actual display the output of both devices (I guess it is showing whatever was drawn last), this causes weird artifacts because of pitch issues in the efifb when the VM window is not sized at 1024x768 (the window will resize to its last size once the vboxvideo driver loads, changing the pitch).
Adding the missing drm_fb_helper_remove_conflicting_pci_framebuffers() call fixes this.
Cc: stable@vger.kernel.org Fixes: 2695eae1f6d3 ("drm/vboxvideo: Switch to generic fbdev emulation") Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/gpu/drm/vboxvideo/vbox_drv.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index 8512d970a09f..261255085918 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -76,6 +76,10 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_mode_fini;
- ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "vboxvideodrmfb");
- if (ret)
goto err_irq_fini;To avoid transient issues this should be done as early as possible, definitely before the drm driver starts to touch the "hw".>
Ok will fix and then push this to drm-misc-next-fixes, thank you.
With that
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
I do wonder though why the automatic removal of conflicting framebuffers doesn't work, fbdev should already do that from register_framebuffer(), which is called somewhere in drm_fbdev_generic_setup (after a few layers).
Did you check why the two framebuffers don't conflict, and why the uefi one doesn't get thrown out?
I did not check, I was not aware that this check was already happening in register_framebuffer(). So I just checked and the reason why this is not working is because the fbdev emulation done by drm_fbdev_generic_setup does not fill in fb_info.apertures->ranges[0] .
So fb_info.apertures->ranges[0].base is left as 0 which does not match with the registered efifb's aperture.
We could try to fix this, but that is not entirely trivial, we would need to pass the pci_dev pointer down into drm_fb_helper_alloc_fbi() then and then like remove_conflicting_pci_framebuffers() does add apertures for all IORESOURCE_MEM PCI bars, but that would conflict with drivers which do rely on drm_fb_helper_alloc_fbi() currently allocating one empty aperture and then actually fill that itself...
You don't need the pci device, because resources are attached to the struct device directly. So you could just go through all IORESOURCE_MEM ranges, and add them. And the struct device we always have through drm_device->dev. This idea just crossed my mind since you brought up IORESOURCE_MEM, might be good to try that out instead of having to litter all drivers with explicit removal calls. The explicit removal is really only for races (vga hw tends to blow up if 2 drivers touch it, stuff like that), or when there's resources conflicts. Can you try to look into that?
This generic solution will still not be enough for shmem/cma based drivers on SoC, but at least it should take care of anything that puts the framebuffer into some kind of bar or stolen memory region. What drivers do explicitly for those is just put in the framebuffer base address. But iirc fbdev core does that already unconditionally. -Daniel
Hi,
On 3/26/20 2:39 PM, Daniel Vetter wrote:
On Thu, Mar 26, 2020 at 2:18 PM Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 3/26/20 12:29 PM, Daniel Vetter wrote:
On Wed, Mar 25, 2020 at 03:43:10PM +0100, Hans de Goede wrote:
The vboxvideo driver is missing a call to remove conflicting framebuffers.
Surprisingly, when using legacy BIOS booting this does not really cause any issues. But when using UEFI to boot the VM then plymouth will draw on both the efifb /dev/fb0 and /dev/drm/card0 (which has registered /dev/fb1 as fbdev emulation).
VirtualBox will actual display the output of both devices (I guess it is showing whatever was drawn last), this causes weird artifacts because of pitch issues in the efifb when the VM window is not sized at 1024x768 (the window will resize to its last size once the vboxvideo driver loads, changing the pitch).
Adding the missing drm_fb_helper_remove_conflicting_pci_framebuffers() call fixes this.
Cc: stable@vger.kernel.org Fixes: 2695eae1f6d3 ("drm/vboxvideo: Switch to generic fbdev emulation") Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/gpu/drm/vboxvideo/vbox_drv.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index 8512d970a09f..261255085918 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -76,6 +76,10 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_mode_fini;
- ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "vboxvideodrmfb");
- if (ret)
goto err_irq_fini;To avoid transient issues this should be done as early as possible, definitely before the drm driver starts to touch the "hw".>
Ok will fix and then push this to drm-misc-next-fixes, thank you.
With that
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
I do wonder though why the automatic removal of conflicting framebuffers doesn't work, fbdev should already do that from register_framebuffer(), which is called somewhere in drm_fbdev_generic_setup (after a few layers).
Did you check why the two framebuffers don't conflict, and why the uefi one doesn't get thrown out?
I did not check, I was not aware that this check was already happening in register_framebuffer(). So I just checked and the reason why this is not working is because the fbdev emulation done by drm_fbdev_generic_setup does not fill in fb_info.apertures->ranges[0] .
So fb_info.apertures->ranges[0].base is left as 0 which does not match with the registered efifb's aperture.
We could try to fix this, but that is not entirely trivial, we would need to pass the pci_dev pointer down into drm_fb_helper_alloc_fbi() then and then like remove_conflicting_pci_framebuffers() does add apertures for all IORESOURCE_MEM PCI bars, but that would conflict with drivers which do rely on drm_fb_helper_alloc_fbi() currently allocating one empty aperture and then actually fill that itself...
You don't need the pci device, because resources are attached to the struct device directly. So you could just go through all IORESOURCE_MEM ranges, and add them. And the struct device we always have through drm_device->dev. This idea just crossed my mind since you brought up IORESOURCE_MEM, might be good to try that out instead of having to litter all drivers with explicit removal calls. The explicit removal is really only for races (vga hw tends to blow up if 2 drivers touch it, stuff like that), or when there's resources conflicts. Can you try to look into that?
Interesting idea, but I'm afraid that my plate is quite full with a lot of more urgent things atm, so I really do not have time for this, sorry.
Regards,
Hans
On Thu, Mar 26, 2020 at 3:40 PM Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 3/26/20 2:39 PM, Daniel Vetter wrote:
On Thu, Mar 26, 2020 at 2:18 PM Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 3/26/20 12:29 PM, Daniel Vetter wrote:
On Wed, Mar 25, 2020 at 03:43:10PM +0100, Hans de Goede wrote:
The vboxvideo driver is missing a call to remove conflicting framebuffers.
Surprisingly, when using legacy BIOS booting this does not really cause any issues. But when using UEFI to boot the VM then plymouth will draw on both the efifb /dev/fb0 and /dev/drm/card0 (which has registered /dev/fb1 as fbdev emulation).
VirtualBox will actual display the output of both devices (I guess it is showing whatever was drawn last), this causes weird artifacts because of pitch issues in the efifb when the VM window is not sized at 1024x768 (the window will resize to its last size once the vboxvideo driver loads, changing the pitch).
Adding the missing drm_fb_helper_remove_conflicting_pci_framebuffers() call fixes this.
Cc: stable@vger.kernel.org Fixes: 2695eae1f6d3 ("drm/vboxvideo: Switch to generic fbdev emulation") Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/gpu/drm/vboxvideo/vbox_drv.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index 8512d970a09f..261255085918 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -76,6 +76,10 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_mode_fini;
- ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "vboxvideodrmfb");
- if (ret)
goto err_irq_fini;To avoid transient issues this should be done as early as possible, definitely before the drm driver starts to touch the "hw".>
Ok will fix and then push this to drm-misc-next-fixes, thank you.
With that
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
I do wonder though why the automatic removal of conflicting framebuffers doesn't work, fbdev should already do that from register_framebuffer(), which is called somewhere in drm_fbdev_generic_setup (after a few layers).
Did you check why the two framebuffers don't conflict, and why the uefi one doesn't get thrown out?
I did not check, I was not aware that this check was already happening in register_framebuffer(). So I just checked and the reason why this is not working is because the fbdev emulation done by drm_fbdev_generic_setup does not fill in fb_info.apertures->ranges[0] .
So fb_info.apertures->ranges[0].base is left as 0 which does not match with the registered efifb's aperture.
We could try to fix this, but that is not entirely trivial, we would need to pass the pci_dev pointer down into drm_fb_helper_alloc_fbi() then and then like remove_conflicting_pci_framebuffers() does add apertures for all IORESOURCE_MEM PCI bars, but that would conflict with drivers which do rely on drm_fb_helper_alloc_fbi() currently allocating one empty aperture and then actually fill that itself...
You don't need the pci device, because resources are attached to the struct device directly. So you could just go through all IORESOURCE_MEM ranges, and add them. And the struct device we always have through drm_device->dev. This idea just crossed my mind since you brought up IORESOURCE_MEM, might be good to try that out instead of having to litter all drivers with explicit removal calls. The explicit removal is really only for races (vga hw tends to blow up if 2 drivers touch it, stuff like that), or when there's resources conflicts. Can you try to look into that?
Interesting idea, but I'm afraid that my plate is quite full with a lot of more urgent things atm, so I really do not have time for this, sorry.
Hm can you capture this idea in a todo then instead? I don't really like the idea of everyone having to add bogus remove_conflicting_framebuffer calls just because the generic fbdev helper falls short of expectations. Kinda not happy to land the vboxvideo patch since it's just a hack to work around a helper bug. -Daniel
Hi,
On 3/26/20 3:47 PM, Daniel Vetter wrote:
On Thu, Mar 26, 2020 at 3:40 PM Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 3/26/20 2:39 PM, Daniel Vetter wrote:
On Thu, Mar 26, 2020 at 2:18 PM Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 3/26/20 12:29 PM, Daniel Vetter wrote:
On Wed, Mar 25, 2020 at 03:43:10PM +0100, Hans de Goede wrote:
The vboxvideo driver is missing a call to remove conflicting framebuffers.
Surprisingly, when using legacy BIOS booting this does not really cause any issues. But when using UEFI to boot the VM then plymouth will draw on both the efifb /dev/fb0 and /dev/drm/card0 (which has registered /dev/fb1 as fbdev emulation).
VirtualBox will actual display the output of both devices (I guess it is showing whatever was drawn last), this causes weird artifacts because of pitch issues in the efifb when the VM window is not sized at 1024x768 (the window will resize to its last size once the vboxvideo driver loads, changing the pitch).
Adding the missing drm_fb_helper_remove_conflicting_pci_framebuffers() call fixes this.
Cc: stable@vger.kernel.org Fixes: 2695eae1f6d3 ("drm/vboxvideo: Switch to generic fbdev emulation") Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/gpu/drm/vboxvideo/vbox_drv.c | 4 ++++ 1 file changed, 4 insertions(+)diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c index 8512d970a09f..261255085918 100644 --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c @@ -76,6 +76,10 @@ static int vbox_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) if (ret) goto err_mode_fini;
- ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "vboxvideodrmfb");
- if (ret)
goto err_irq_fini;To avoid transient issues this should be done as early as possible, definitely before the drm driver starts to touch the "hw".>
Ok will fix and then push this to drm-misc-next-fixes, thank you.
With that
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
I do wonder though why the automatic removal of conflicting framebuffers doesn't work, fbdev should already do that from register_framebuffer(), which is called somewhere in drm_fbdev_generic_setup (after a few layers).
Did you check why the two framebuffers don't conflict, and why the uefi one doesn't get thrown out?
I did not check, I was not aware that this check was already happening in register_framebuffer(). So I just checked and the reason why this is not working is because the fbdev emulation done by drm_fbdev_generic_setup does not fill in fb_info.apertures->ranges[0] .
So fb_info.apertures->ranges[0].base is left as 0 which does not match with the registered efifb's aperture.
We could try to fix this, but that is not entirely trivial, we would need to pass the pci_dev pointer down into drm_fb_helper_alloc_fbi() then and then like remove_conflicting_pci_framebuffers() does add apertures for all IORESOURCE_MEM PCI bars, but that would conflict with drivers which do rely on drm_fb_helper_alloc_fbi() currently allocating one empty aperture and then actually fill that itself...
You don't need the pci device, because resources are attached to the struct device directly. So you could just go through all IORESOURCE_MEM ranges, and add them. And the struct device we always have through drm_device->dev. This idea just crossed my mind since you brought up IORESOURCE_MEM, might be good to try that out instead of having to litter all drivers with explicit removal calls. The explicit removal is really only for races (vga hw tends to blow up if 2 drivers touch it, stuff like that), or when there's resources conflicts. Can you try to look into that?
Interesting idea, but I'm afraid that my plate is quite full with a lot of more urgent things atm, so I really do not have time for this, sorry.
Hm can you capture this idea in a todo then instead? I don't really like the idea of everyone having to add bogus remove_conflicting_framebuffer calls just because the generic fbdev helper falls short of expectations. Kinda not happy to land the vboxvideo patch since it's just a hack to work around a helper bug.
Sure I can add a TODO for this. Note part of the problem is that some drivers already manually set up the single aperture currently allocated by drm_fb_helper_alloc_fbi() and the question is if we can just drop that code when we switch to automatically adding apertures without this having bad side effects.
Regards,
Hans
linux-stable-mirror@lists.linaro.org
-
Daniel Vetter -
Hans de Goede