| Message ID | 20240627173530.460615-1-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/fbdev-generic: Fix framebuffer on big endian devices | expand |
Thomas Huth <thuth@redhat.com> writes: Hello Thomas, > Starting with kernel 6.7, the framebuffer text console is not working > anymore with the virtio-gpu device on s390x hosts. Such big endian fb > devices are usinga different pixel ordering than little endian devices, > e.g. DRM_FORMAT_BGRX8888 instead of DRM_FORMAT_XRGB8888. > > This used to work fine as long as drm_client_buffer_addfb() was still > calling drm_mode_addfb() which called drm_driver_legacy_fb_format() > internally to get the right format. But drm_client_buffer_addfb() has > recently been reworked to call drm_mode_addfb2() instead with the > format value that has been passed to it as a parameter (see commit > 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()"). > > That format parameter is determined in drm_fbdev_generic_helper_fb_probe() > via the drm_mode_legacy_fb_format() function - which only generates > formats suitable for little endian devices. So to fix this issue > switch to drm_driver_legacy_fb_format() here instead to take the > device endianness into consideration. > > Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()") > Closes: https://issues.redhat.com/browse/RHEL-45158 > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > drivers/gpu/drm/drm_fbdev_generic.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c > index 97e579c33d84..1e200d815e1a 100644 > --- a/drivers/gpu/drm/drm_fbdev_generic.c > +++ b/drivers/gpu/drm/drm_fbdev_generic.c > @@ -84,7 +84,8 @@ static int drm_fbdev_generic_helper_fb_probe(struct drm_fb_helper *fb_helper, > sizes->surface_width, sizes->surface_height, > sizes->surface_bpp); > > - format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth); > + format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp, > + sizes->surface_depth); > buffer = drm_client_framebuffer_create(client, sizes->surface_width, > sizes->surface_height, format); > if (IS_ERR(buffer)) Indeed. Thanks a lot for the detailed explanation and the patch. Your fix makes sense to me. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Hi Am 27.06.24 um 19:35 schrieb Thomas Huth: > Starting with kernel 6.7, the framebuffer text console is not working > anymore with the virtio-gpu device on s390x hosts. Such big endian fb > devices are usinga different pixel ordering than little endian devices, > e.g. DRM_FORMAT_BGRX8888 instead of DRM_FORMAT_XRGB8888. > > This used to work fine as long as drm_client_buffer_addfb() was still > calling drm_mode_addfb() which called drm_driver_legacy_fb_format() > internally to get the right format. But drm_client_buffer_addfb() has > recently been reworked to call drm_mode_addfb2() instead with the > format value that has been passed to it as a parameter (see commit > 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()"). > > That format parameter is determined in drm_fbdev_generic_helper_fb_probe() > via the drm_mode_legacy_fb_format() function - which only generates > formats suitable for little endian devices. So to fix this issue > switch to drm_driver_legacy_fb_format() here instead to take the > device endianness into consideration. > > Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()") > Closes: https://issues.redhat.com/browse/RHEL-45158 > Signed-off-by: Thomas Huth <thuth@redhat.com> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/drm_fbdev_generic.c | 3 ++- This file is now called drm_fbdev_ttm.c in drm-misc-next. And a similar patch might be necessary for drm_fbdev_dma.c. The code in drm_fbdev_shmem.c apparently has it already. Best regards Thomas > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c > index 97e579c33d84..1e200d815e1a 100644 > --- a/drivers/gpu/drm/drm_fbdev_generic.c > +++ b/drivers/gpu/drm/drm_fbdev_generic.c > @@ -84,7 +84,8 @@ static int drm_fbdev_generic_helper_fb_probe(struct drm_fb_helper *fb_helper, > sizes->surface_width, sizes->surface_height, > sizes->surface_bpp); > > - format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth); > + format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp, > + sizes->surface_depth); > buffer = drm_client_framebuffer_create(client, sizes->surface_width, > sizes->surface_height, format); > if (IS_ERR(buffer))
Hi Thomas, CC Christian On Thu, Jun 27, 2024 at 7:35 PM Thomas Huth <thuth@redhat.com> wrote: > Starting with kernel 6.7, the framebuffer text console is not working > anymore with the virtio-gpu device on s390x hosts. Such big endian fb > devices are usinga different pixel ordering than little endian devices, > e.g. DRM_FORMAT_BGRX8888 instead of DRM_FORMAT_XRGB8888. > > This used to work fine as long as drm_client_buffer_addfb() was still > calling drm_mode_addfb() which called drm_driver_legacy_fb_format() > internally to get the right format. But drm_client_buffer_addfb() has > recently been reworked to call drm_mode_addfb2() instead with the > format value that has been passed to it as a parameter (see commit > 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()"). > > That format parameter is determined in drm_fbdev_generic_helper_fb_probe() > via the drm_mode_legacy_fb_format() function - which only generates > formats suitable for little endian devices. So to fix this issue > switch to drm_driver_legacy_fb_format() here instead to take the > device endianness into consideration. > > Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()") > Closes: https://issues.redhat.com/browse/RHEL-45158 > Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> works fine on m68k-virt, so: Tested-by: Geert Uytterhoeven <geert@linux-m68k.org> Christian had reported a similar issue before[1]. I submitted a different solution fixing virtio[2] instead, but that caused issues with virtio-mouse-pci cursor... > --- a/drivers/gpu/drm/drm_fbdev_generic.c > +++ b/drivers/gpu/drm/drm_fbdev_generic.c > @@ -84,7 +84,8 @@ static int drm_fbdev_generic_helper_fb_probe(struct drm_fb_helper *fb_helper, > sizes->surface_width, sizes->surface_height, > sizes->surface_bpp); > > - format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth); > + format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp, > + sizes->surface_depth); > buffer = drm_client_framebuffer_create(client, sizes->surface_width, > sizes->surface_height, format); > if (IS_ERR(buffer)) [1] https://lore.kernel.org/6530cea3-4507-454e-bc36-a6970c8e7578@xenosoft.de/ [2] "[PATCH v2] drm/virtio: Add suppport for non-native buffer formats" https://lore.kernel.org/47a81d2e0e47b1715718779b6978a8b595cc7c5d.1700140609.git.geert@linux-m68k.org Gr{oetje,eeting}s, Geert
Hi Thomas, On Fri, Jun 28, 2024 at 8:07 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > Am 27.06.24 um 19:35 schrieb Thomas Huth: > > Starting with kernel 6.7, the framebuffer text console is not working > > anymore with the virtio-gpu device on s390x hosts. Such big endian fb > > devices are usinga different pixel ordering than little endian devices, > > e.g. DRM_FORMAT_BGRX8888 instead of DRM_FORMAT_XRGB8888. > > > > This used to work fine as long as drm_client_buffer_addfb() was still > > calling drm_mode_addfb() which called drm_driver_legacy_fb_format() > > internally to get the right format. But drm_client_buffer_addfb() has > > recently been reworked to call drm_mode_addfb2() instead with the > > format value that has been passed to it as a parameter (see commit > > 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()"). > > > > That format parameter is determined in drm_fbdev_generic_helper_fb_probe() > > via the drm_mode_legacy_fb_format() function - which only generates > > formats suitable for little endian devices. So to fix this issue > > switch to drm_driver_legacy_fb_format() here instead to take the > > device endianness into consideration. > > > > Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()") > > Closes: https://issues.redhat.com/browse/RHEL-45158 > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > > > --- > > drivers/gpu/drm/drm_fbdev_generic.c | 3 ++- > > This file is now called drm_fbdev_ttm.c in drm-misc-next. And a similar > patch might be necessary for drm_fbdev_dma.c. The code in > drm_fbdev_shmem.c apparently has it already. We are getting too many copies of this logic... (yup, had to fix them all up in my WIP support for R1 ;-) Gr{oetje,eeting}s, Geert
On 28/06/2024 08.07, Thomas Zimmermann wrote: > Hi > > Am 27.06.24 um 19:35 schrieb Thomas Huth: >> Starting with kernel 6.7, the framebuffer text console is not working >> anymore with the virtio-gpu device on s390x hosts. Such big endian fb >> devices are usinga different pixel ordering than little endian devices, >> e.g. DRM_FORMAT_BGRX8888 instead of DRM_FORMAT_XRGB8888. >> >> This used to work fine as long as drm_client_buffer_addfb() was still >> calling drm_mode_addfb() which called drm_driver_legacy_fb_format() >> internally to get the right format. But drm_client_buffer_addfb() has >> recently been reworked to call drm_mode_addfb2() instead with the >> format value that has been passed to it as a parameter (see commit >> 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to >> drm_mode_addfb2()"). >> >> That format parameter is determined in drm_fbdev_generic_helper_fb_probe() >> via the drm_mode_legacy_fb_format() function - which only generates >> formats suitable for little endian devices. So to fix this issue >> switch to drm_driver_legacy_fb_format() here instead to take the >> device endianness into consideration. >> >> Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to >> drm_mode_addfb2()") >> Closes: https://issues.redhat.com/browse/RHEL-45158 >> Signed-off-by: Thomas Huth <thuth@redhat.com> > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > >> --- >> drivers/gpu/drm/drm_fbdev_generic.c | 3 ++- > > This file is now called drm_fbdev_ttm.c in drm-misc-next. Oh, ok, shall I send a v2 that is adjusted to that change, or can it be fixed while applying my patch? > And a similar patch might be necessary for drm_fbdev_dma.c. Looks similar, indeed. Shall I send a patch for that one, too? ... I currently don't have a setup for testing that, though... Thomas > The code in drm_fbdev_shmem.c > apparently has it already. > > Best regards > Thomas > >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_fbdev_generic.c >> b/drivers/gpu/drm/drm_fbdev_generic.c >> index 97e579c33d84..1e200d815e1a 100644 >> --- a/drivers/gpu/drm/drm_fbdev_generic.c >> +++ b/drivers/gpu/drm/drm_fbdev_generic.c >> @@ -84,7 +84,8 @@ static int drm_fbdev_generic_helper_fb_probe(struct >> drm_fb_helper *fb_helper, >> sizes->surface_width, sizes->surface_height, >> sizes->surface_bpp); >> - format = drm_mode_legacy_fb_format(sizes->surface_bpp, >> sizes->surface_depth); >> + format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp, >> + sizes->surface_depth); >> buffer = drm_client_framebuffer_create(client, sizes->surface_width, >> sizes->surface_height, format); >> if (IS_ERR(buffer)) >
Hi Thomas, On Mon, Jul 1, 2024 at 10:42 AM Thomas Huth <thuth@redhat.com> wrote: > On 28/06/2024 08.07, Thomas Zimmermann wrote: > > Am 27.06.24 um 19:35 schrieb Thomas Huth: > >> Starting with kernel 6.7, the framebuffer text console is not working > >> anymore with the virtio-gpu device on s390x hosts. Such big endian fb > >> devices are usinga different pixel ordering than little endian devices, > >> e.g. DRM_FORMAT_BGRX8888 instead of DRM_FORMAT_XRGB8888. > >> > >> This used to work fine as long as drm_client_buffer_addfb() was still > >> calling drm_mode_addfb() which called drm_driver_legacy_fb_format() > >> internally to get the right format. But drm_client_buffer_addfb() has > >> recently been reworked to call drm_mode_addfb2() instead with the > >> format value that has been passed to it as a parameter (see commit > >> 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to > >> drm_mode_addfb2()"). > >> > >> That format parameter is determined in drm_fbdev_generic_helper_fb_probe() > >> via the drm_mode_legacy_fb_format() function - which only generates > >> formats suitable for little endian devices. So to fix this issue > >> switch to drm_driver_legacy_fb_format() here instead to take the > >> device endianness into consideration. > >> > >> Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to > >> drm_mode_addfb2()") > >> Closes: https://issues.redhat.com/browse/RHEL-45158 > >> Signed-off-by: Thomas Huth <thuth@redhat.com> > > > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > > > > >> --- > >> drivers/gpu/drm/drm_fbdev_generic.c | 3 ++- > > > > This file is now called drm_fbdev_ttm.c in drm-misc-next. > > Oh, ok, shall I send a v2 that is adjusted to that change, or can it be > fixed while applying my patch? As this is a regression in mainline, which needs to be backported, too, it's best to apply your fix to v6.10-rc6, which does not have drm_fbdev_ttm.c yet. > > And a similar patch might be necessary for drm_fbdev_dma.c. > > Looks similar, indeed. Shall I send a patch for that one, too? ... I > currently don't have a setup for testing that, though... Obviously these need to be fixed, too. Gr{oetje,eeting}s, Geert
Hi Am 01.07.24 um 10:54 schrieb Geert Uytterhoeven: > Hi Thomas, > > On Mon, Jul 1, 2024 at 10:42 AM Thomas Huth <thuth@redhat.com> wrote: >> On 28/06/2024 08.07, Thomas Zimmermann wrote: >>> Am 27.06.24 um 19:35 schrieb Thomas Huth: >>>> Starting with kernel 6.7, the framebuffer text console is not working >>>> anymore with the virtio-gpu device on s390x hosts. Such big endian fb >>>> devices are usinga different pixel ordering than little endian devices, >>>> e.g. DRM_FORMAT_BGRX8888 instead of DRM_FORMAT_XRGB8888. >>>> >>>> This used to work fine as long as drm_client_buffer_addfb() was still >>>> calling drm_mode_addfb() which called drm_driver_legacy_fb_format() >>>> internally to get the right format. But drm_client_buffer_addfb() has >>>> recently been reworked to call drm_mode_addfb2() instead with the >>>> format value that has been passed to it as a parameter (see commit >>>> 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to >>>> drm_mode_addfb2()"). >>>> >>>> That format parameter is determined in drm_fbdev_generic_helper_fb_probe() >>>> via the drm_mode_legacy_fb_format() function - which only generates >>>> formats suitable for little endian devices. So to fix this issue >>>> switch to drm_driver_legacy_fb_format() here instead to take the >>>> device endianness into consideration. >>>> >>>> Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to >>>> drm_mode_addfb2()") >>>> Closes: https://issues.redhat.com/browse/RHEL-45158 >>>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> >>> >>> >>>> --- >>>> drivers/gpu/drm/drm_fbdev_generic.c | 3 ++- >>> This file is now called drm_fbdev_ttm.c in drm-misc-next. >> Oh, ok, shall I send a v2 that is adjusted to that change, or can it be >> fixed while applying my patch? > As this is a regression in mainline, which needs to be backported, > too, it's best to apply your fix to v6.10-rc6, which does not have > drm_fbdev_ttm.c yet. I guess we can put the current fix into drm-misc-fixes. > >>> And a similar patch might be necessary for drm_fbdev_dma.c. >> Looks similar, indeed. Shall I send a patch for that one, too? ... I >> currently don't have a setup for testing that, though... It's up to you. You'd need drm-misc-next from https://gitlab.freedesktop.org/drm/misc/kernel Best regards Thomas > Obviously these need to be fixed, too. > > Gr{oetje,eeting}s, > > Geert >
Merged into drm-misc-fixes. Am 27.06.24 um 19:35 schrieb Thomas Huth: > Starting with kernel 6.7, the framebuffer text console is not working > anymore with the virtio-gpu device on s390x hosts. Such big endian fb > devices are usinga different pixel ordering than little endian devices, > e.g. DRM_FORMAT_BGRX8888 instead of DRM_FORMAT_XRGB8888. > > This used to work fine as long as drm_client_buffer_addfb() was still > calling drm_mode_addfb() which called drm_driver_legacy_fb_format() > internally to get the right format. But drm_client_buffer_addfb() has > recently been reworked to call drm_mode_addfb2() instead with the > format value that has been passed to it as a parameter (see commit > 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()"). > > That format parameter is determined in drm_fbdev_generic_helper_fb_probe() > via the drm_mode_legacy_fb_format() function - which only generates > formats suitable for little endian devices. So to fix this issue > switch to drm_driver_legacy_fb_format() here instead to take the > device endianness into consideration. > > Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()") > Closes: https://issues.redhat.com/browse/RHEL-45158 > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > drivers/gpu/drm/drm_fbdev_generic.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c > index 97e579c33d84..1e200d815e1a 100644 > --- a/drivers/gpu/drm/drm_fbdev_generic.c > +++ b/drivers/gpu/drm/drm_fbdev_generic.c > @@ -84,7 +84,8 @@ static int drm_fbdev_generic_helper_fb_probe(struct drm_fb_helper *fb_helper, > sizes->surface_width, sizes->surface_height, > sizes->surface_bpp); > > - format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth); > + format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp, > + sizes->surface_depth); > buffer = drm_client_framebuffer_create(client, sizes->surface_width, > sizes->surface_height, format); > if (IS_ERR(buffer))
diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index 97e579c33d84..1e200d815e1a 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -84,7 +84,8 @@ static int drm_fbdev_generic_helper_fb_probe(struct drm_fb_helper *fb_helper, sizes->surface_width, sizes->surface_height, sizes->surface_bpp); - format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth); + format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp, + sizes->surface_depth); buffer = drm_client_framebuffer_create(client, sizes->surface_width, sizes->surface_height, format); if (IS_ERR(buffer))
Starting with kernel 6.7, the framebuffer text console is not working anymore with the virtio-gpu device on s390x hosts. Such big endian fb devices are usinga different pixel ordering than little endian devices, e.g. DRM_FORMAT_BGRX8888 instead of DRM_FORMAT_XRGB8888. This used to work fine as long as drm_client_buffer_addfb() was still calling drm_mode_addfb() which called drm_driver_legacy_fb_format() internally to get the right format. But drm_client_buffer_addfb() has recently been reworked to call drm_mode_addfb2() instead with the format value that has been passed to it as a parameter (see commit 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()"). That format parameter is determined in drm_fbdev_generic_helper_fb_probe() via the drm_mode_legacy_fb_format() function - which only generates formats suitable for little endian devices. So to fix this issue switch to drm_driver_legacy_fb_format() here instead to take the device endianness into consideration. Fixes: 6ae2ff23aa43 ("drm/client: Convert drm_client_buffer_addfb() to drm_mode_addfb2()") Closes: https://issues.redhat.com/browse/RHEL-45158 Signed-off-by: Thomas Huth <thuth@redhat.com> --- drivers/gpu/drm/drm_fbdev_generic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)