[PATCH 2/4] video: fbdev: amba-clcd: Retire elder CLCD driver

Tue Sep 22 04:10:57 UTC 2020

On Mon, Sep 21, 2020 at 2:32 PM Linus Walleij <linus.walleij at linaro.org> wrote:
>
> On Tue, Sep 15, 2020 at 3:10 AM Peter Collingbourne <pcc at google.com> wrote:
> > On Tue, Jun 9, 2020 at 1:08 PM Linus Walleij <linus.walleij at linaro.org> wrote:
> > >
> > > All the functionality in this driver has been reimplemented
> > > in the new DRM driver in drivers/gpu/drm/pl111/* and all
> > > the boards using it have been migrated to use the DRM driver
> > > with all configuration coming from the device tree.
> >
> > Android's FVP configuration still uses FBDEV.
>
> But all DRM drivers support frame buffer emulation so I don't
> see how this can be a problem? The new DRM driver provides
> a framebuffer.

Okay, I was unaware of that feature and it is disabled in Android
kernels. I am now testing the DRM driver by applying this diff to
FVP's kernel config:

diff --git a/fvp.fragment b/fvp.fragment
index b12be37278a4..481767c06ac9 100644
--- a/fvp.fragment
+++ b/fvp.fragment
@@ -1,5 +1,6 @@
-CONFIG_FB=m
-CONFIG_FB_ARMCLCD=m
+CONFIG_DRM_PANEL_SIMPLE=m
+CONFIG_DRM_PL111=m
+CONFIG_DRM_FBDEV_EMULATION=y
 CONFIG_MOUSE_PS2=m
 CONFIG_SERIO_AMBAKMI=m
 CONFIG_MMC_ARMMMCI=m

> I'm also confused about how this can be in use.
> fvp-base-revc.dts includes rtsm_ve-motherboard.dtsi
> where the PL111 is defined.
>
> commit f1fe12c8bf33241e04c57a0fc5b25b16ba77ba2d
> "ARM: dts: Modernize the Vexpress PL111 integration"
> changes the DTS for the FVP such that only the new DRM
> driver really works with it: it removes the panel-dpi encoded
> panel and defines the panel
> "arm,rtsm-display" for FVP and that is *only* supported
> by drivers/gpu/drm/panel/panel-simple.c from DRM.

That commit predates commit fa083b99eb284186ae65193ae856ef2801a1646d
which added the DTS for FVP, along with a panel-dpi compatible panel
which is still present:
https://github.com/torvalds/linux/blob/98477740630f270aecf648f1d6a9dbc6027d4ff1/arch/arm64/boot/dts/arm/fvp-base-revc.dts#L189

> The upstream FVP has been using the new DRM driver
> by default since
> commit 37ad688497785c9ad1471236e28efda1e2f39741
> "arm64: defconfig: Switch to PL11x DRM driver"
> in january 2019.

That only affected defconfig though which Android doesn't use.

> Are you using old or outoftree devicetrees with a newer
> kernel?

No, I'm using an up-to-date device tree (pretty sure about that
otherwise I wouldn't have hit problems like
https://android-review.googlesource.com/c/device/generic/goldfish/+/1394347
).

> > While it would be great
> > to see it migrated to DRM, this first requires resolving a design
> > incompatibility between Android's composer and DRM devices that only
> > support software rendering. I proposed a patch that would help resolve
> > this [1], but there was disagreement about the approach and I haven't
> > had time to get back to this.
> >
> > Can this please be reverted until FVP on Android can be migrated to DRM?
>
> We need a clear technical reason, I understand that using DRM
> directly might be a problem, but DRM supports full framebuffer
> emulation and from a userspace point of view, what the new
> driver provides should be equivalent.
>
> I can think of problems like Android seeing the new fancy
> DRM userspace ABI but surely it can be instructed to
> ignore that and just use the framebuffer emulation instead?
>
> If there is anything else making DRMs framebuffer emulation
> substandard I am sure the DRM developers would like to know,
> especially if it makes Android unhappy.

Okay, I found at least two issues so far. The first is that the driver
does not come up in 32bpp mode, seemingly because of this line of code
here:
https://github.com/torvalds/linux/blob/98477740630f270aecf648f1d6a9dbc6027d4ff1/drivers/gpu/drm/pl111/pl111_versatile.c#L368

The second problem seems to be that Android's calls to
ioctl(FBIOPUT_VSCREENINFO) fail. I thought that this might be related
to the declared lack of 32bpp support, but even with the above line
changed to .fb_bpp = 32 (probably not the right fix given the comment
above it would probably break any users of the "real" Versatile
Express board; the driver probably instead needs to select the bpp
based on the max-memory-bandwidth like the old fbdev driver did) I
still get this logcat output:

09-22 01:31:08.807   272   272 W gralloc : FBIOPUT_VSCREENINFO failed,
page flipping not supported
09-22 01:31:08.807   272   272 W gralloc : page flipping not supported
(yres_virtual=768, requested=1536)
09-22 01:31:08.807   272   272 I gralloc : using (fd=7)
09-22 01:31:08.807   272   272 I gralloc : id           = pl111drmfb
09-22 01:31:08.807   272   272 I gralloc : xres         = 1024 px
09-22 01:31:08.807   272   272 I gralloc : yres         = 768 px
09-22 01:31:08.807   272   272 I gralloc : xres_virtual = 1024 px
09-22 01:31:08.807   272   272 I gralloc : yres_virtual = 768 px
09-22 01:31:08.807   272   272 I gralloc : bpp          = 32
09-22 01:31:08.807   272   272 I gralloc : r            = 16:8
09-22 01:31:08.807   272   272 I gralloc : g            =  8:8
09-22 01:31:08.807   272   272 I gralloc : b            =  0:8
09-22 01:31:08.807   272   272 I gralloc : width        = 400 mm (65.024002 dpi)
09-22 01:31:08.807   272   272 I gralloc : height       = 300 mm (65.023994 dpi)
09-22 01:31:08.807   272   272 I gralloc : refresh rate = 60.00 Hz

which is coming from the code starting here:
https://cs.android.com/android/platform/superproject/+/master:hardware/libhardware/modules/gralloc/framebuffer.cpp;l=185
(you can ignore the part of the log message that talks about page
flipping; in this context the main purpose of the ioctl is to give
effect to the code on lines 166-172 that sets the pixel format, and as
can be seen from the subsequent output the format remains BGRX888
rather than RGBX8888).

The output from a successful boot with the old fbdev driver looks like this:

09-22 00:54:57.750   272   272 W gralloc : page flipping not supported
(yres_virtual=768, requested=1536)
09-22 00:54:57.750   272   272 I gralloc : using (fd=7)
09-22 00:54:57.750   272   272 I gralloc : id           = CLCD FB
09-22 00:54:57.750   272   272 I gralloc : xres         = 1024 px
09-22 00:54:57.750   272   272 I gralloc : yres         = 768 px
09-22 00:54:57.750   272   272 I gralloc : xres_virtual = 1024 px
09-22 00:54:57.750   272   272 I gralloc : yres_virtual = 768 px
09-22 00:54:57.750   272   272 I gralloc : bpp          = 32
09-22 00:54:57.750   272   272 I gralloc : r            =  0:8
09-22 00:54:57.750   272   272 I gralloc : g            =  8:8
09-22 00:54:57.750   272   272 I gralloc : b            = 16:8
09-22 00:54:57.750   272   272 I gralloc : width        = 163 mm
(159.568100 dpi)
09-22 00:54:57.750   272   272 I gralloc : height       = 122 mm
(159.895081 dpi)
09-22 00:54:57.750   272   272 I gralloc : refresh rate = 65.34 Hz

which aside from the pixel format indicates some differences with the
width, height and refresh rate which may be significant.

As a result of FBIOPUT_VSCREENINFO failing, the FVP window fails to
resize so I don't see any graphical output (this ioctl in the fbdev
driver set the magic registers that caused FVP to resize its window;
my investigation into DRM earlier this year revealed the equivalent
register-setting operation in DRM to be DRM_IOCTL_MODE_ATOMIC).

I'll try to look at this closer tomorrow to see whether Android is
doing something wrong, but it seems plausible that DRM's FBDEV
emulation is missing a critical feature, at least for pl111.

Peter
