| Message ID | 20240713160353.62410-1-me@samjakob.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/display/bcm2835_fb: fix fb_use_offsets condition | expand |
On Sat, 13 Jul 2024 at 17:04, SamJakob <me@samjakob.com> wrote: > > It is common practice when implementing double-buffering > on VideoCore to do so by multiplying the height of the > virtual buffer by the number of virtual screens desired > (i.e., two - in the case of double-bufferring). > > At present, this won't work in QEMU because the logic in > fb_use_offsets require that both the virtual width and > height exceed their physical counterparts. > > This appears to be unintentional/a typo and indeed the > comment states; "Experimentally, the hardware seems to > do this only if the viewport size is larger than the > physical screen". The viewport/virtual size would be > larger than the physical size if either virtual dimension > were larger than their physical counterparts and not > necessarily both. > > Signed-off-by: SamJakob <me@samjakob.com> Thanks for this bugfix; I've applied it to my target-arm.next queue and it should get upstream within a week or so. -- PMM
On 16/7/24 12:35, Peter Maydell wrote: > On Sat, 13 Jul 2024 at 17:04, SamJakob <me@samjakob.com> wrote: >> >> It is common practice when implementing double-buffering >> on VideoCore to do so by multiplying the height of the >> virtual buffer by the number of virtual screens desired >> (i.e., two - in the case of double-bufferring). >> >> At present, this won't work in QEMU because the logic in >> fb_use_offsets require that both the virtual width and >> height exceed their physical counterparts. >> >> This appears to be unintentional/a typo and indeed the >> comment states; "Experimentally, the hardware seems to >> do this only if the viewport size is larger than the >> physical screen". The viewport/virtual size would be >> larger than the physical size if either virtual dimension >> were larger than their physical counterparts and not >> necessarily both. >> >> Signed-off-by: SamJakob <me@samjakob.com> > > Thanks for this bugfix; I've applied it to my target-arm.next > queue and it should get upstream within a week or so. Since I'm seeing 2 times the same patch, adding R-b again on this one: Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> (BTW Peter the commit description is mis-aligned, if you don't mind, correcting it while applying would be appreciated!) Thanks, Phil.
On Tue, 16 Jul 2024 at 15:37, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > On 16/7/24 12:35, Peter Maydell wrote: > > On Sat, 13 Jul 2024 at 17:04, SamJakob <me@samjakob.com> wrote: > >> > >> It is common practice when implementing double-buffering > >> on VideoCore to do so by multiplying the height of the > >> virtual buffer by the number of virtual screens desired > >> (i.e., two - in the case of double-bufferring). > >> > >> At present, this won't work in QEMU because the logic in > >> fb_use_offsets require that both the virtual width and > >> height exceed their physical counterparts. > >> > >> This appears to be unintentional/a typo and indeed the > >> comment states; "Experimentally, the hardware seems to > >> do this only if the viewport size is larger than the > >> physical screen". The viewport/virtual size would be > >> larger than the physical size if either virtual dimension > >> were larger than their physical counterparts and not > >> necessarily both. > >> > >> Signed-off-by: SamJakob <me@samjakob.com> > > > > Thanks for this bugfix; I've applied it to my target-arm.next > > queue and it should get upstream within a week or so. > > Since I'm seeing 2 times the same patch, adding R-b again on > this one: > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > (BTW Peter the commit description is mis-aligned, if you > don't mind, correcting it while applying would be appreciated!) Yep, I already picked up your r-by and rewrapped the commit message. -- PMM
Thank you! My apologies for the duplicates and alignment - it’s my first time using a mailing list! > On Jul 16, 2024, at 3:44 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 16 Jul 2024 at 15:37, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: >> >>> On 16/7/24 12:35, Peter Maydell wrote: >>> On Sat, 13 Jul 2024 at 17:04, SamJakob <me@samjakob.com> wrote: >>>> >>>> It is common practice when implementing double-buffering >>>> on VideoCore to do so by multiplying the height of the >>>> virtual buffer by the number of virtual screens desired >>>> (i.e., two - in the case of double-bufferring). >>>> >>>> At present, this won't work in QEMU because the logic in >>>> fb_use_offsets require that both the virtual width and >>>> height exceed their physical counterparts. >>>> >>>> This appears to be unintentional/a typo and indeed the >>>> comment states; "Experimentally, the hardware seems to >>>> do this only if the viewport size is larger than the >>>> physical screen". The viewport/virtual size would be >>>> larger than the physical size if either virtual dimension >>>> were larger than their physical counterparts and not >>>> necessarily both. >>>> >>>> Signed-off-by: SamJakob <me@samjakob.com> >>> >>> Thanks for this bugfix; I've applied it to my target-arm.next >>> queue and it should get upstream within a week or so. >> >> Since I'm seeing 2 times the same patch, adding R-b again on >> this one: >> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> >> (BTW Peter the commit description is mis-aligned, if you >> don't mind, correcting it while applying would be appreciated!) > > Yep, I already picked up your r-by and rewrapped the commit > message. > > -- PMM
diff --git a/hw/display/bcm2835_fb.c b/hw/display/bcm2835_fb.c index e40ed2d2e1..650db3da82 100644 --- a/hw/display/bcm2835_fb.c +++ b/hw/display/bcm2835_fb.c @@ -145,7 +145,7 @@ static bool fb_use_offsets(BCM2835FBConfig *config) * viewport size is larger than the physical screen. (It doesn't * prevent the guest setting this silly viewport setting, though...) */ - return config->xres_virtual > config->xres && + return config->xres_virtual > config->xres || config->yres_virtual > config->yres; }
It is common practice when implementing double-buffering on VideoCore to do so by multiplying the height of the virtual buffer by the number of virtual screens desired (i.e., two - in the case of double-bufferring). At present, this won't work in QEMU because the logic in fb_use_offsets require that both the virtual width and height exceed their physical counterparts. This appears to be unintentional/a typo and indeed the comment states; "Experimentally, the hardware seems to do this only if the viewport size is larger than the physical screen". The viewport/virtual size would be larger than the physical size if either virtual dimension were larger than their physical counterparts and not necessarily both. Signed-off-by: SamJakob <me@samjakob.com> --- hw/display/bcm2835_fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)