| Message ID | 5283e2811498034cc2de77f10eb16b9cd67a0698.camel@kernel.crashing.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] hw/misc/bcm2835_property: Fix framebuffer with recent RPi kernels | expand |
Hi Benjamin, On 10/17/21 09:48, Benjamin Herrenschmidt wrote: > The framebuffer driver fails to initialize with recent Raspberry Pi > kernels, such as the ones shipped in the current RaspiOS images > (with the out of tree bcm2708_fb.c driver) Which particular version? > > The reason is that this driver uses a new firmware call to query the > number of displays, and the fallback when this call fails is broken. > > So implement the call and claim we have exactly one display > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > hw/misc/bcm2835_property.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c > index 73941bdae9..b958fa6a5c 100644 > --- a/hw/misc/bcm2835_property.c > +++ b/hw/misc/bcm2835_property.c > @@ -269,6 +269,10 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) > stl_le_phys(&s->dma_as, value + 12, 0); > resplen = 4; > break; > + case 0x00040013: /* Get num displays */ > + stl_le_phys(&s->dma_as, value + 12, 1); > + resplen = 4; > + break; > > case 0x00060001: /* Get DMA channels */ > /* channels 2-5 */ > I carry (among others) this patch for the raspi4: -- >8 -- diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c index 3b3f5a804d9..8bd149211af 100644 --- a/hw/misc/bcm2835_property.c +++ b/hw/misc/bcm2835_property.c @@ -36,6 +36,7 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) */ BCM2835FBConfig fbconfig = s->fbdev->config; bool fbconfig_updated = false; + int fbconfig_idx = 0; value &= ~0xf; @@ -290,6 +291,25 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) resplen = 4; break; + case 0x00040013: /* Get number of displays */ + stl_le_phys(&s->dma_as, value + 12, 0 /* old fw: unique display */); + resplen = 4; + break; + + case 0x00048013: /* Select display */ + fbconfig_idx = ldl_le_phys(&s->dma_as, value + 12); + if (fbconfig_idx) { + qemu_log_mask(LOG_GUEST_ERROR, + "bcm2835_property: Only one display implemented," + " requested display #%d\n", fbconfig_idx); + } + resplen = 4; + break; + + case 0x00040014: /* Get display settings */ + resplen = 0; /* as old fw we don't support that */ + break; + case 0x00060001: /* Get DMA channels */ /* channels 2-5 */ stl_le_phys(&s->dma_as, value + 12, 0x003C); --- FYI I plan to respin Alex's recent series with these patches: https://lore.kernel.org/qemu-devel/20211004134742.2044280-1-alex.bennee@linaro.org/ as soon as I get some time...
On Sun, 2021-10-17 at 17:08 +0200, Philippe Mathieu-Daudé wrote: > Hi Benjamin, > > On 10/17/21 09:48, Benjamin Herrenschmidt wrote: > > The framebuffer driver fails to initialize with recent Raspberry Pi > > kernels, such as the ones shipped in the current RaspiOS images > > (with the out of tree bcm2708_fb.c driver) > > Which particular version? The one I dug out of 2021-05-07-raspios-buster-arm64-lite.img (note that this then fails to boot some time after the fb is setup, even after the fix, in the vchip driver init (before serial is up even). That said, the same fb problem happens with 5.10.60-v8+ from raspbian. I'm not sure your fix will work on these, see below: > + case 0x00040013: /* Get number of displays */ > + stl_le_phys(&s->dma_as, value + 12, 0 /* old fw: unique display */); > + resplen = 4; > + break; > This should have been equivalen to not having the property. However, the failure path in the driver looks like this (note the mismatch between the comment and the code.. this is rpi 5.10.73 from the rpi repo : ret = rpi_firmware_property(fw, RPI_FIRMWARE_FRAMEBUFFER_GET_NUM_DISPLAYS, &num_displays, sizeof(u32)); /* If we fail to get the number of displays, or it returns 0, then * assume old firmware that doesn't have the mailbox call, so just * set one display */ if (ret || num_displays == 0) { dev_err(&dev->dev, "Unable to determine number of FBs. Disabling driver.\n"); return -ENOENT; } else { fbdev->firmware_supports_multifb = 1; } So it appears that the intention at some stage was to set only one display but the code as written will fail to initialize the drive if the properly is absent *or* if it returns 0. I've just checked the rpi-5.15.y branch and it's the same. Cheers, Ben.
On 10/18/21 02:41, Benjamin Herrenschmidt wrote: > On Sun, 2021-10-17 at 17:08 +0200, Philippe Mathieu-Daudé wrote: >> Hi Benjamin, >> >> On 10/17/21 09:48, Benjamin Herrenschmidt wrote: >>> The framebuffer driver fails to initialize with recent Raspberry Pi >>> kernels, such as the ones shipped in the current RaspiOS images >>> (with the out of tree bcm2708_fb.c driver) >> >> Which particular version? > > The one I dug out of 2021-05-07-raspios-buster-arm64-lite.img (note > that this then fails to boot some time after the fb is setup, even > after the fix, in the vchip driver init (before serial is up even). > > That said, the same fb problem happens with 5.10.60-v8+ from raspbian. > > I'm not sure your fix will work on these, see below: > >> + case 0x00040013: /* Get number of displays */ >> + stl_le_phys(&s->dma_as, value + 12, 0 /* old fw: unique display */); >> + resplen = 4; >> + break; >> > This should have been equivalen to not having the property. However, > the failure path in the driver looks like this (note the mismatch > between the comment and the code.. this is rpi 5.10.73 from the rpi > repo : > > ret = rpi_firmware_property(fw, > RPI_FIRMWARE_FRAMEBUFFER_GET_NUM_DISPLAYS, > &num_displays, sizeof(u32)); > > /* If we fail to get the number of displays, or it returns 0, then > * assume old firmware that doesn't have the mailbox call, so just > * set one display > */ > if (ret || num_displays == 0) { > dev_err(&dev->dev, > "Unable to determine number of FBs. Disabling driver.\n"); > return -ENOENT; > } else { > fbdev->firmware_supports_multifb = 1; > } > > So it appears that the intention at some stage was to set only one display but > the code as written will fail to initialize the drive if the properly is absent > *or* if it returns 0. > > I've just checked the rpi-5.15.y branch and it's the same. Indeed. I stopped testing recent kernels because they use too many features QEMU don't implement. Our model should generate the DTB blob of devices implemented, instead of giving false expectations to the guest by passing an unmodified dtb. This is on my TODO, I might give it a try next WE.
On Mon, 2021-10-18 at 11:47 +0200, Philippe Mathieu-Daudé wrote: > > > I've just checked the rpi-5.15.y branch and it's the same. > > Indeed. I stopped testing recent kernels because they use too many > features QEMU don't implement. > > Our model should generate the DTB blob of devices implemented, instead > of giving false expectations to the guest by passing an unmodified dtb. > > This is on my TODO, I might give it a try next WE. Indeed. That said, we do implement the fb, so we probably want that fix. The fix for the virtual gpios is probably unnecessary however if we do what you want. That being said, with those two fixes, I can boot the latest 5.10 I get from raspbian. Cheers, Ben.
On 10/18/21 12:27, Benjamin Herrenschmidt wrote: > On Mon, 2021-10-18 at 11:47 +0200, Philippe Mathieu-Daudé wrote: >> >>> I've just checked the rpi-5.15.y branch and it's the same. >> >> Indeed. I stopped testing recent kernels because they use too many >> features QEMU don't implement. >> >> Our model should generate the DTB blob of devices implemented, instead >> of giving false expectations to the guest by passing an unmodified dtb. >> >> This is on my TODO, I might give it a try next WE. > > Indeed. That said, we do implement the fb, so we probably want that > fix. The fix for the virtual gpios is probably unnecessary however if > we do what you want. > > That being said, with those two fixes, I can boot the latest 5.10 I get > from raspbian. Great. This test should pass with your 5.10 kernel then: https://lore.kernel.org/qemu-devel/20200215192216.4899-9-f4bug@amsat.org/ Do you mind providing the equivalent 'deb_url' / 'deb_hash' you used, so I can adapt this test to cover a Raspbian 5.10 kernel in our CI?
On 10/17/21 09:48, Benjamin Herrenschmidt wrote: > The framebuffer driver fails to initialize with recent Raspberry Pi > kernels, such as the ones shipped in the current RaspiOS images > (with the out of tree bcm2708_fb.c driver) > > The reason is that this driver uses a new firmware call to query the > number of displays, and the fallback when this call fails is broken. > > So implement the call and claim we have exactly one display > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > hw/misc/bcm2835_property.c | 4 ++++ > 1 file changed, 4 insertions(+) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Hi Peter, Could you take this patch via your ARM tree? Thanks! On Thu, Nov 18, 2021 at 4:05 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > On 10/17/21 09:48, Benjamin Herrenschmidt wrote: > > The framebuffer driver fails to initialize with recent Raspberry Pi > > kernels, such as the ones shipped in the current RaspiOS images > > (with the out of tree bcm2708_fb.c driver) > > > > The reason is that this driver uses a new firmware call to query the > > number of displays, and the fallback when this call fails is broken. > > > > So implement the call and claim we have exactly one display > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > --- > > hw/misc/bcm2835_property.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c index 73941bdae9..b958fa6a5c 100644 --- a/hw/misc/bcm2835_property.c +++ b/hw/misc/bcm2835_property.c @@ -269,6 +269,10 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) stl_le_phys(&s->dma_as, value + 12, 0); resplen = 4; break; + case 0x00040013: /* Get num displays */ + stl_le_phys(&s->dma_as, value + 12, 1); + resplen = 4; + break; case 0x00060001: /* Get DMA channels */ /* channels 2-5 */
The framebuffer driver fails to initialize with recent Raspberry Pi kernels, such as the ones shipped in the current RaspiOS images (with the out of tree bcm2708_fb.c driver) The reason is that this driver uses a new firmware call to query the number of displays, and the fallback when this call fails is broken. So implement the call and claim we have exactly one display Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- hw/misc/bcm2835_property.c | 4 ++++ 1 file changed, 4 insertions(+)