Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
Hi Peter, [Paging John Stultz on this] On Tue, Sep 29, 2020 at 6:30 PM Peter Collingbourne wrote: > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > Can't Android FVP use drm-hwcomposer instead ? > > Not without kernel changes. See e.g. > https://www.spinics.net/lists/dri-devel/msg255883.html Has this gotten better in the last year+ so you can now use drm-hwcomposer? I.e. can we now delete the fbdev driver? > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > emulation, then let's do that. Not keep fbdev drivers on life support for > > longer than necessary. I wanted to fix this and started to look at it at one point, however the required Android userspace build was a bit intimidating to get going. I would be able to look into this if a prebuilt Android for the emulation model was available. Yours, Linus Walleij
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On 30/09/2020 11:43, Daniel Vetter wrote: > On Tue, Sep 29, 2020 at 01:51:36PM -0700, Peter Collingbourne wrote: >> On Tue, Sep 29, 2020 at 11:44 AM Daniel Vetter wrote: >>> >>> On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: On Tue, Sep 29, 2020 at 9:52 AM Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: >> On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: >>> On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > Hi, > > On 28/09/2020 22:08, Peter Collingbourne wrote: >> Also revert the follow-up change "drm: pl111: Absorb the external >> register header". >> >> This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 >> and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. >> >> The fbdev driver is used by Android's FVP configuration. Using the >> DRM driver together with DRM's fbdev emulation results in a failure >> to boot Android. The root cause is that Android's generic fbdev >> userspace driver relies on the ability to set the pixel format via >> FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > Can't Android FVP use drm-hwcomposer instead ? >>> >>> Not without kernel changes. See e.g. >>> https://www.spinics.net/lists/dri-devel/msg255883.html >> >> That discussion seems to have died down with no further action. >> >> I also was kinda under the assumption that with Android these buffers >> would be allocated directly from dma-buf heaps/ion, so this all seems not >> that well baked out. The disagreement about whether these allocations should be made via the render node or via dma-buf/ion is one reason why it was hard to make progress on this, unfortunately. >>> >>> Yeah, using dumb buffer create to allocate random buffers for shared >>> software rendering isn't super popular move. >>> >>> But aside from all this, why is this blocking the migration from fbdev >>> to drm? With fbdev you don't have buffer allocations, nor dma-buf >>> support, and somehow android can boot. But on drm you can't boot if >>> these things are not available. That sounds like the bar for drm is >>> maybe a tad too high for simple dumb kms drivers like pl which are >>> just there to get a picture on the screen/panel. >> >> I would not intend to use dumb buffer create to allocate >> non-framebuffer-destined buffers for software rendering. >> >> With the generic fbdev driver any allocations not destined for the >> framebuffer use ashmem, which is how the driver avoids making huge >> allocations in fbdev space. I would intend to do something similar for >> DRM where framebuffer-destined allocations use dumb buffer create and >> non-framebuffer-destined allocations use dma-buf. At the time I >> prototyped this approach on the Android side [1] and together with my >> render-nodes-everywhere patch and some other hopefully-uncontroversial >> changes it worked and let me boot to the home screen. I would be very >> happy if this approach were allowed on render nodes so that Android >> FVP would be unblocked from moving off fbdev, but at the point where >> we left the thread off last time you weren't sure whether it would be >> acceptable [2]. >> >> One thing that I might not have mentioned on the thread last time was >> that render nodes have the advantage that unlike primary nodes, >> opening them does not take master if it is not already taken. This >> means that on Android, using primary nodes instead of render nodes in >> the process responsible for creating frame buffers creates a startup >> race condition for master between the hwcomposer process (which >> normally opens the primary node and is responsible for flipping >> frames) and the surfaceflinger process (which normally opens the >> render node and is responsible for drawing onto the framebuffer and >> exporting its frames to the composer). > > I still don't get how this works with fbdev, how do applications render > directly into the fbdev memory (when that's the target)? How is that fbdev > memory shared with them? I suspect they are users of the DRM_FBDEV_LEAK_PHYS_SMEM option... > > Since if the memory shared there isn't really a functional regression, > even if we haven't yet solved the "how can clients allocate > scanout-capable memory for simple displays" issue. Neil > -Daniel > >> >> Peter >> >> [1] >> https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/2438955 >> [2] https://www.spinics.net/lists/dri-devel/msg256559.html >> >>> -Daniel >>> Also, if we need to add more random fbdev ioctls to the drm fbdev emulation, then let's do that. Not keep fbdev drivers on life support for longer than necessary. >>> >>> That
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 9:52 AM Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > > Hi, > > > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > > > > register header". > > > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > > > > DRM driver together with DRM's fbdev emulation results in a failure > > > > > > to boot Android. The root cause is that Android's generic fbdev > > > > > > userspace driver relies on the ability to set the pixel format via > > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > > > Not without kernel changes. See e.g. > > > https://www.spinics.net/lists/dri-devel/msg255883.html > > > > That discussion seems to have died down with no further action. > > > > I also was kinda under the assumption that with Android these buffers > > would be allocated directly from dma-buf heaps/ion, so this all seems not > > that well baked out. The disagreement about whether these allocations should be made via the render node or via dma-buf/ion is one reason why it was hard to make progress on this, unfortunately. > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > > > emulation, then let's do that. Not keep fbdev drivers on life support > > > > for > > > > longer than necessary. > > > > > > That should have been done *before* removing the old driver, which was > > > a userspace break that was introduced in 5.9rc1. We shouldn't leave > > > userspace broken for the period of time that it would take to develop > > > that change. Even if such a change were developed before 5.9 is > > > released, it probably wouldn't be considered a bug fix that would be > > > eligible for 5.9. > > > > > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > > > > There have been other less critical behavioral differences > > > > > > identified > > > > > > between the fbdev driver and the DRM driver with fbdev emulation. > > > > > > The > > > > > > DRM driver exposes different values for the panel's width, height > > > > > > and > > > > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > > > > > > with yres_virtual greater than the maximum supported value instead > > > > > > of letting the syscall succeed and setting yres_virtual based on > > > > > > yres. > > > > > > > > Also something that should be fixed I think in upstream, in the drm > > > > fbdev > > > > emulation. At least doesn't sound like it's something unfixable. > > > > > > Again, it should have been fixed before removing the old driver. > > > > Yeah, but we also don't have a whole lot people who seem to push for this. > > So the only way to find the people who still care is to remove the fbdev > > drivers. > > > > fbdev is dead. I'm totally fine reverting the driver to shut up the > > regression report, but it's not fixing anything long term. There's not > > going to be new drivers, or new hw support in existing drivers in fbdev. > > > > Also note that there's not spec nor test suite for fbdev, so "you guys > > should have known before removing drivers" doesn't work. > > btw revert itself is stuck somewhere, so I can't find it. And for the You should be able to download the commit like this (the commit message will need amending to remove most of the trailers at the bottom, though): git fetch https://linux.googlesource.com/linux/kernel/git/torvalds/linux refs/changes/52/2952/1 > revert can you pls just resurrect the files in drivers/video, without > touching anything in drivers/gpu? Tying down drm drivers with stuff in > fbdev doesn't make much sense to me. The touching of code in drm was a consequence of needing to revert a follow-up commit which moved code previously shared between fbdev and drm into the drm driver. Or do you think we should "manually" revert the first commit and as a consequence end up with the two copies of the code? Peter > > Thanks, Daniel > > > -Daniel > > > > > > > > Peter > > > > > > > -Daniel > > > > > > > > > > > > > > > > Signed-off-by: Peter Collingbourne > > > > > > --- > > > > > > View this change in Gerrit: > > > > > > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > > > > > > > > > > > MAINTAINERS | 5 + > > > > > > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > > > > > > drivers/gpu/drm/pl111/pl111_display.c | 1 + > > > > > > drivers/gpu/drm/pl111/pl111_dr
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 11:44 AM Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > > > > On Tue, Sep 29, 2020 at 9:52 AM Daniel Vetter wrote: > > > > > > On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > > > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > > > > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > > > > Hi, > > > > > > > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > > > > Also revert the follow-up change "drm: pl111: Absorb the > > > > > > > > external > > > > > > > > register header". > > > > > > > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using > > > > > > > > the > > > > > > > > DRM driver together with DRM's fbdev emulation results in a > > > > > > > > failure > > > > > > > > to boot Android. The root cause is that Android's generic fbdev > > > > > > > > userspace driver relies on the ability to set the pixel format > > > > > > > > via > > > > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > > > > > > > Not without kernel changes. See e.g. > > > > > https://www.spinics.net/lists/dri-devel/msg255883.html > > > > > > > > That discussion seems to have died down with no further action. > > > > > > > > I also was kinda under the assumption that with Android these buffers > > > > would be allocated directly from dma-buf heaps/ion, so this all seems > > > > not > > > > that well baked out. > > > > The disagreement about whether these allocations should be made via > > the render node or via dma-buf/ion is one reason why it was hard to > > make progress on this, unfortunately. > > Yeah, using dumb buffer create to allocate random buffers for shared > software rendering isn't super popular move. > > But aside from all this, why is this blocking the migration from fbdev > to drm? With fbdev you don't have buffer allocations, nor dma-buf > support, and somehow android can boot. But on drm you can't boot if > these things are not available. That sounds like the bar for drm is > maybe a tad too high for simple dumb kms drivers like pl which are > just there to get a picture on the screen/panel. I would not intend to use dumb buffer create to allocate non-framebuffer-destined buffers for software rendering. With the generic fbdev driver any allocations not destined for the framebuffer use ashmem, which is how the driver avoids making huge allocations in fbdev space. I would intend to do something similar for DRM where framebuffer-destined allocations use dumb buffer create and non-framebuffer-destined allocations use dma-buf. At the time I prototyped this approach on the Android side [1] and together with my render-nodes-everywhere patch and some other hopefully-uncontroversial changes it worked and let me boot to the home screen. I would be very happy if this approach were allowed on render nodes so that Android FVP would be unblocked from moving off fbdev, but at the point where we left the thread off last time you weren't sure whether it would be acceptable [2]. One thing that I might not have mentioned on the thread last time was that render nodes have the advantage that unlike primary nodes, opening them does not take master if it is not already taken. This means that on Android, using primary nodes instead of render nodes in the process responsible for creating frame buffers creates a startup race condition for master between the hwcomposer process (which normally opens the primary node and is responsible for flipping frames) and the surfaceflinger process (which normally opens the render node and is responsible for drawing onto the framebuffer and exporting its frames to the composer). Peter [1] https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/2438955 [2] https://www.spinics.net/lists/dri-devel/msg256559.html > -Daniel > > > > > > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > > > > > emulation, then let's do that. Not keep fbdev drivers on life > > > > > > support for > > > > > > longer than necessary. > > > > > > > > > > That should have been done *before* removing the old driver, which was > > > > > a userspace break that was introduced in 5.9rc1. We shouldn't leave > > > > > userspace broken for the period of time that it would take to develop > > > > > that change. Even if such a change were developed before 5.9 is > > > > > released, it probably wouldn't be considered a bug fix that would be > > > > > eligible for 5.9. > > > > > > > > > > > > > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > > > > > > > > > > There have
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > Hi, > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > register header". > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > DRM driver together with DRM's fbdev emulation results in a failure > > > to boot Android. The root cause is that Android's generic fbdev > > > userspace driver relies on the ability to set the pixel format via > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > Can't Android FVP use drm-hwcomposer instead ? Not without kernel changes. See e.g. https://www.spinics.net/lists/dri-devel/msg255883.html > Also, if we need to add more random fbdev ioctls to the drm fbdev > emulation, then let's do that. Not keep fbdev drivers on life support for > longer than necessary. That should have been done *before* removing the old driver, which was a userspace break that was introduced in 5.9rc1. We shouldn't leave userspace broken for the period of time that it would take to develop that change. Even if such a change were developed before 5.9 is released, it probably wouldn't be considered a bug fix that would be eligible for 5.9. > > > > > Neil > > > > > > > > There have been other less critical behavioral differences identified > > > between the fbdev driver and the DRM driver with fbdev emulation. The > > > DRM driver exposes different values for the panel's width, height and > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > > > with yres_virtual greater than the maximum supported value instead > > > of letting the syscall succeed and setting yres_virtual based on yres. > > Also something that should be fixed I think in upstream, in the drm fbdev > emulation. At least doesn't sound like it's something unfixable. Again, it should have been fixed before removing the old driver. Peter > -Daniel > > > > > > > Signed-off-by: Peter Collingbourne > > > --- > > > View this change in Gerrit: > > > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > > > > > MAINTAINERS | 5 + > > > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > > > drivers/gpu/drm/pl111/pl111_display.c | 1 + > > > drivers/gpu/drm/pl111/pl111_drm.h | 73 -- > > > drivers/gpu/drm/pl111/pl111_drv.c | 1 + > > > drivers/gpu/drm/pl111/pl111_versatile.c | 1 + > > > drivers/video/fbdev/Kconfig | 20 + > > > drivers/video/fbdev/Makefile| 1 + > > > drivers/video/fbdev/amba-clcd.c | 986 > > > include/linux/amba/clcd-regs.h | 87 +++ > > > include/linux/amba/clcd.h | 290 +++ > > > 11 files changed, 1393 insertions(+), 73 deletions(-) > > > create mode 100644 drivers/video/fbdev/amba-clcd.c > > > create mode 100644 include/linux/amba/clcd-regs.h > > > create mode 100644 include/linux/amba/clcd.h > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 190c7fa2ea01..671c1fa79e64 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -1460,6 +1460,11 @@ S: Odd Fixes > > > F: drivers/amba/ > > > F: include/linux/amba/bus.h > > > > > > +ARM PRIMECELL CLCD PL110 DRIVER > > > +M: Russell King > > > +S: Odd Fixes > > > +F: drivers/video/fbdev/amba-clcd.* > > > + > > > ARM PRIMECELL KMI PL050 DRIVER > > > M: Russell King > > > S: Odd Fixes > > > diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c > > > b/drivers/gpu/drm/pl111/pl111_debugfs.c > > > index 317f68abf18b..26ca8cdf3e60 100644 > > > --- a/drivers/gpu/drm/pl111/pl111_debugfs.c > > > +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c > > > @@ -3,6 +3,7 @@ > > > * Copyright © 2017 Broadcom > > > */ > > > > > > +#include > > > #include > > > > > > #include > > > diff --git a/drivers/gpu/drm/pl111/pl111_display.c > > > b/drivers/gpu/drm/pl111/pl111_display.c > > > index b3e8697cafcf..703ddc803c55 100644 > > > --- a/drivers/gpu/drm/pl111/pl111_display.c > > > +++ b/drivers/gpu/drm/pl111/pl111_display.c > > > @@ -9,6 +9,7 @@ > > > * Copyright (C) 2011 Texas Instruments > > > */ > > > > > > +#include > > > #include > > > #include > > > #include > > > diff --git a/drivers/gpu/drm/pl111/pl111_drm.h > > > b/drivers/gpu/drm/pl111/pl111_drm.h > > > index 2a46b5bd8576..ba399bcb792f 100644 > > > --- a/drivers/gpu/drm/pl111/pl111_drm.h > > > +++ b/drivers/gpu/drm/pl111/pl111_drm.h > > > @@ -23,79 +23,6 @@ > > > #include > > > #include > > > > > > -/* > > > - * CLCD Controller Internal Register addresses > > > - */ > > > -#define CLCD_TIM0 0x > > > -#define CLCD_TIM1 0x0004 > > > -#define CLCD_TIM2 0x0008 > > > -#define CLCD
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 01:51:36PM -0700, Peter Collingbourne wrote: > On Tue, Sep 29, 2020 at 11:44 AM Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > > > > > > On Tue, Sep 29, 2020 at 9:52 AM Daniel Vetter wrote: > > > > > > > > On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > > > > > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > > > > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter > > > > > > wrote: > > > > > > > > > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > > > > > Also revert the follow-up change "drm: pl111: Absorb the > > > > > > > > > external > > > > > > > > > register header". > > > > > > > > > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. > > > > > > > > > Using the > > > > > > > > > DRM driver together with DRM's fbdev emulation results in a > > > > > > > > > failure > > > > > > > > > to boot Android. The root cause is that Android's generic > > > > > > > > > fbdev > > > > > > > > > userspace driver relies on the ability to set the pixel > > > > > > > > > format via > > > > > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev > > > > > > > > > emulation. > > > > > > > > > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > > > > > > > > > Not without kernel changes. See e.g. > > > > > > https://www.spinics.net/lists/dri-devel/msg255883.html > > > > > > > > > > That discussion seems to have died down with no further action. > > > > > > > > > > I also was kinda under the assumption that with Android these buffers > > > > > would be allocated directly from dma-buf heaps/ion, so this all seems > > > > > not > > > > > that well baked out. > > > > > > The disagreement about whether these allocations should be made via > > > the render node or via dma-buf/ion is one reason why it was hard to > > > make progress on this, unfortunately. > > > > Yeah, using dumb buffer create to allocate random buffers for shared > > software rendering isn't super popular move. > > > > But aside from all this, why is this blocking the migration from fbdev > > to drm? With fbdev you don't have buffer allocations, nor dma-buf > > support, and somehow android can boot. But on drm you can't boot if > > these things are not available. That sounds like the bar for drm is > > maybe a tad too high for simple dumb kms drivers like pl which are > > just there to get a picture on the screen/panel. > > I would not intend to use dumb buffer create to allocate > non-framebuffer-destined buffers for software rendering. > > With the generic fbdev driver any allocations not destined for the > framebuffer use ashmem, which is how the driver avoids making huge > allocations in fbdev space. I would intend to do something similar for > DRM where framebuffer-destined allocations use dumb buffer create and > non-framebuffer-destined allocations use dma-buf. At the time I > prototyped this approach on the Android side [1] and together with my > render-nodes-everywhere patch and some other hopefully-uncontroversial > changes it worked and let me boot to the home screen. I would be very > happy if this approach were allowed on render nodes so that Android > FVP would be unblocked from moving off fbdev, but at the point where > we left the thread off last time you weren't sure whether it would be > acceptable [2]. > > One thing that I might not have mentioned on the thread last time was > that render nodes have the advantage that unlike primary nodes, > opening them does not take master if it is not already taken. This > means that on Android, using primary nodes instead of render nodes in > the process responsible for creating frame buffers creates a startup > race condition for master between the hwcomposer process (which > normally opens the primary node and is responsible for flipping > frames) and the surfaceflinger process (which normally opens the > render node and is responsible for drawing onto the framebuffer and > exporting its frames to the composer). I still don't get how this works with fbdev, how do applications render directly into the fbdev memory (when that's the target)? How is that fbdev memory shared with them? Since if the memory shared there isn't really a functional regression, even if we haven't yet solved the "how can clients allocate scanout-capable memory for simple displays" issue. -Daniel > > Peter > > [1] > https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/2438955 > [2] https://www.spinics.net/lists/dri-devel/msg256559.html > > > -Daniel > > > > > > > > > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > >
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 10:29:22PM +0200, Linus Walleij wrote: > On Tue, Sep 29, 2020 at 8:44 PM Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > > > But aside from all this, why is this blocking the migration from fbdev > > to drm? With fbdev you don't have buffer allocations, nor dma-buf > > support, and somehow android can boot. > > I do not know how Android does things these days but back in the > days it would request a virtual framebuffer twice the height of the > physical framebuffer and then pan that up/down between composing > frames, thus achieving a type of double-buffering from userspace. > > Given the type of bugs Peter is seeing this seems to still be the > case, Peter can you confirm? We have the overallocate option for the fbdev emulation to support this use case of flipping buffers. So this part should work I think, but maybe some other parts of our check_var/set_par implementation don't work in a way to make surface flinger happy. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 8:44 PM Daniel Vetter wrote: > On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > But aside from all this, why is this blocking the migration from fbdev > to drm? With fbdev you don't have buffer allocations, nor dma-buf > support, and somehow android can boot. I do not know how Android does things these days but back in the days it would request a virtual framebuffer twice the height of the physical framebuffer and then pan that up/down between composing frames, thus achieving a type of double-buffering from userspace. Given the type of bugs Peter is seeing this seems to still be the case, Peter can you confirm? Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > > On Tue, Sep 29, 2020 at 9:52 AM Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > > > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > > > Hi, > > > > > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > > > > > register header". > > > > > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > > > > > DRM driver together with DRM's fbdev emulation results in a > > > > > > > failure > > > > > > > to boot Android. The root cause is that Android's generic fbdev > > > > > > > userspace driver relies on the ability to set the pixel format via > > > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > > > > > Not without kernel changes. See e.g. > > > > https://www.spinics.net/lists/dri-devel/msg255883.html > > > > > > That discussion seems to have died down with no further action. > > > > > > I also was kinda under the assumption that with Android these buffers > > > would be allocated directly from dma-buf heaps/ion, so this all seems not > > > that well baked out. > > The disagreement about whether these allocations should be made via > the render node or via dma-buf/ion is one reason why it was hard to > make progress on this, unfortunately. Yeah, using dumb buffer create to allocate random buffers for shared software rendering isn't super popular move. But aside from all this, why is this blocking the migration from fbdev to drm? With fbdev you don't have buffer allocations, nor dma-buf support, and somehow android can boot. But on drm you can't boot if these things are not available. That sounds like the bar for drm is maybe a tad too high for simple dumb kms drivers like pl which are just there to get a picture on the screen/panel. -Daniel > > > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > > > > emulation, then let's do that. Not keep fbdev drivers on life support > > > > > for > > > > > longer than necessary. > > > > > > > > That should have been done *before* removing the old driver, which was > > > > a userspace break that was introduced in 5.9rc1. We shouldn't leave > > > > userspace broken for the period of time that it would take to develop > > > > that change. Even if such a change were developed before 5.9 is > > > > released, it probably wouldn't be considered a bug fix that would be > > > > eligible for 5.9. > > > > > > > > > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > > > > > > > There have been other less critical behavioral differences > > > > > > > identified > > > > > > > between the fbdev driver and the DRM driver with fbdev emulation. > > > > > > > The > > > > > > > DRM driver exposes different values for the panel's width, height > > > > > > > and > > > > > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO > > > > > > > syscall > > > > > > > with yres_virtual greater than the maximum supported value instead > > > > > > > of letting the syscall succeed and setting yres_virtual based on > > > > > > > yres. > > > > > > > > > > Also something that should be fixed I think in upstream, in the drm > > > > > fbdev > > > > > emulation. At least doesn't sound like it's something unfixable. > > > > > > > > Again, it should have been fixed before removing the old driver. > > > > > > Yeah, but we also don't have a whole lot people who seem to push for this. > > > So the only way to find the people who still care is to remove the fbdev > > > drivers. > > > > > > fbdev is dead. I'm totally fine reverting the driver to shut up the > > > regression report, but it's not fixing anything long term. There's not > > > going to be new drivers, or new hw support in existing drivers in fbdev. > > > > > > Also note that there's not spec nor test suite for fbdev, so "you guys > > > should have known before removing drivers" doesn't work. > > > > btw revert itself is stuck somewhere, so I can't find it. And for the > > You should be able to download the commit like this (the commit > message will need amending to remove most of the trailers at the > bottom, though): > > git fetch https://linux.googlesource.com/linux/kernel/git/torvalds/linux > refs/changes/52/2952/1 > > > revert can you pls just resurrect the files in drivers/video, without > > touching anything in drivers/gpu? Tying down drm drivers with stuff in > > fbdev doesn't make much sense to me. > > The touching of code in drm was a
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > > On Tue, Sep 29, 2020 at 9:52 AM Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > > > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > > > Hi, > > > > > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > > > > > register header". > > > > > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > > > > > DRM driver together with DRM's fbdev emulation results in a > > > > > > > failure > > > > > > > to boot Android. The root cause is that Android's generic fbdev > > > > > > > userspace driver relies on the ability to set the pixel format via > > > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > > > > > Not without kernel changes. See e.g. > > > > https://www.spinics.net/lists/dri-devel/msg255883.html > > > > > > That discussion seems to have died down with no further action. > > > > > > I also was kinda under the assumption that with Android these buffers > > > would be allocated directly from dma-buf heaps/ion, so this all seems not > > > that well baked out. > > The disagreement about whether these allocations should be made via > the render node or via dma-buf/ion is one reason why it was hard to > make progress on this, unfortunately. > > > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > > > > emulation, then let's do that. Not keep fbdev drivers on life support > > > > > for > > > > > longer than necessary. > > > > > > > > That should have been done *before* removing the old driver, which was > > > > a userspace break that was introduced in 5.9rc1. We shouldn't leave > > > > userspace broken for the period of time that it would take to develop > > > > that change. Even if such a change were developed before 5.9 is > > > > released, it probably wouldn't be considered a bug fix that would be > > > > eligible for 5.9. > > > > > > > > > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > > > > > > > There have been other less critical behavioral differences > > > > > > > identified > > > > > > > between the fbdev driver and the DRM driver with fbdev emulation. > > > > > > > The > > > > > > > DRM driver exposes different values for the panel's width, height > > > > > > > and > > > > > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO > > > > > > > syscall > > > > > > > with yres_virtual greater than the maximum supported value instead > > > > > > > of letting the syscall succeed and setting yres_virtual based on > > > > > > > yres. > > > > > > > > > > Also something that should be fixed I think in upstream, in the drm > > > > > fbdev > > > > > emulation. At least doesn't sound like it's something unfixable. > > > > > > > > Again, it should have been fixed before removing the old driver. > > > > > > Yeah, but we also don't have a whole lot people who seem to push for this. > > > So the only way to find the people who still care is to remove the fbdev > > > drivers. > > > > > > fbdev is dead. I'm totally fine reverting the driver to shut up the > > > regression report, but it's not fixing anything long term. There's not > > > going to be new drivers, or new hw support in existing drivers in fbdev. > > > > > > Also note that there's not spec nor test suite for fbdev, so "you guys > > > should have known before removing drivers" doesn't work. > > > > btw revert itself is stuck somewhere, so I can't find it. And for the > > You should be able to download the commit like this (the commit > message will need amending to remove most of the trailers at the > bottom, though): > > git fetch https://linux.googlesource.com/linux/kernel/git/torvalds/linux > refs/changes/52/2952/1 > > > revert can you pls just resurrect the files in drivers/video, without > > touching anything in drivers/gpu? Tying down drm drivers with stuff in > > fbdev doesn't make much sense to me. > > The touching of code in drm was a consequence of needing to revert a > follow-up commit which moved code previously shared between fbdev and > drm into the drm driver. > > Or do you think we should "manually" revert the first commit and as a > consequence end up with the two copies of the code? Yup. -Daniel > > Peter > > > > > Thanks, Daniel > > > > > -Daniel > > > > > > > > > > > Peter > > > > > > > > > -Daniel > > > > > > > > > > > > > > > > > > > Signed-off-by: Peter Collingbourne > > > > > > > --- > > > > > > > View this change in Gerrit:
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > Hi, > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > > > register header". > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > > > DRM driver together with DRM's fbdev emulation results in a failure > > > > > to boot Android. The root cause is that Android's generic fbdev > > > > > userspace driver relies on the ability to set the pixel format via > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > Not without kernel changes. See e.g. > > https://www.spinics.net/lists/dri-devel/msg255883.html > > That discussion seems to have died down with no further action. > > I also was kinda under the assumption that with Android these buffers > would be allocated directly from dma-buf heaps/ion, so this all seems not > that well baked out. > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > > emulation, then let's do that. Not keep fbdev drivers on life support for > > > longer than necessary. > > > > That should have been done *before* removing the old driver, which was > > a userspace break that was introduced in 5.9rc1. We shouldn't leave > > userspace broken for the period of time that it would take to develop > > that change. Even if such a change were developed before 5.9 is > > released, it probably wouldn't be considered a bug fix that would be > > eligible for 5.9. > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > There have been other less critical behavioral differences identified > > > > > between the fbdev driver and the DRM driver with fbdev emulation. The > > > > > DRM driver exposes different values for the panel's width, height and > > > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > > > > > with yres_virtual greater than the maximum supported value instead > > > > > of letting the syscall succeed and setting yres_virtual based on yres. > > > > > > Also something that should be fixed I think in upstream, in the drm fbdev > > > emulation. At least doesn't sound like it's something unfixable. > > > > Again, it should have been fixed before removing the old driver. > > Yeah, but we also don't have a whole lot people who seem to push for this. > So the only way to find the people who still care is to remove the fbdev > drivers. > > fbdev is dead. I'm totally fine reverting the driver to shut up the > regression report, but it's not fixing anything long term. There's not > going to be new drivers, or new hw support in existing drivers in fbdev. > > Also note that there's not spec nor test suite for fbdev, so "you guys > should have known before removing drivers" doesn't work. btw revert itself is stuck somewhere, so I can't find it. And for the revert can you pls just resurrect the files in drivers/video, without touching anything in drivers/gpu? Tying down drm drivers with stuff in fbdev doesn't make much sense to me. Thanks, Daniel > -Daniel > > > > > Peter > > > > > -Daniel > > > > > > > > > > > > > Signed-off-by: Peter Collingbourne > > > > > --- > > > > > View this change in Gerrit: > > > > > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > > > > > > > > > MAINTAINERS | 5 + > > > > > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > > > > > drivers/gpu/drm/pl111/pl111_display.c | 1 + > > > > > drivers/gpu/drm/pl111/pl111_drm.h | 73 -- > > > > > drivers/gpu/drm/pl111/pl111_drv.c | 1 + > > > > > drivers/gpu/drm/pl111/pl111_versatile.c | 1 + > > > > > drivers/video/fbdev/Kconfig | 20 + > > > > > drivers/video/fbdev/Makefile| 1 + > > > > > drivers/video/fbdev/amba-clcd.c | 986 > > > > > > > > > > include/linux/amba/clcd-regs.h | 87 +++ > > > > > include/linux/amba/clcd.h | 290 +++ > > > > > 11 files changed, 1393 insertions(+), 73 deletions(-) > > > > > create mode 100644 drivers/video/fbdev/amba-clcd.c > > > > > create mode 100644 include/linux/amba/clcd-regs.h > > > > > create mode 100644 include/linux/amba/clcd.h > > > > > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > > index 190c7fa2ea01..671c1fa79e64 100644 > > > > > --- a/MAINTAINERS > > > > > +++ b/MAINTAINERS > > > > > @@ -1460,6 +1460,11 @@ S: Odd Fixes > > > > > F: drivers/amba/ > > > > > F: include/linux/amba/bus.h > > > > > > >
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > Hi, > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > > register header". > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > > DRM driver together with DRM's fbdev emulation results in a failure > > > > to boot Android. The root cause is that Android's generic fbdev > > > > userspace driver relies on the ability to set the pixel format via > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > Not without kernel changes. See e.g. > https://www.spinics.net/lists/dri-devel/msg255883.html That discussion seems to have died down with no further action. I also was kinda under the assumption that with Android these buffers would be allocated directly from dma-buf heaps/ion, so this all seems not that well baked out. > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > emulation, then let's do that. Not keep fbdev drivers on life support for > > longer than necessary. > > That should have been done *before* removing the old driver, which was > a userspace break that was introduced in 5.9rc1. We shouldn't leave > userspace broken for the period of time that it would take to develop > that change. Even if such a change were developed before 5.9 is > released, it probably wouldn't be considered a bug fix that would be > eligible for 5.9. > > > > > > > > > Neil > > > > > > > > > > > There have been other less critical behavioral differences identified > > > > between the fbdev driver and the DRM driver with fbdev emulation. The > > > > DRM driver exposes different values for the panel's width, height and > > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > > > > with yres_virtual greater than the maximum supported value instead > > > > of letting the syscall succeed and setting yres_virtual based on yres. > > > > Also something that should be fixed I think in upstream, in the drm fbdev > > emulation. At least doesn't sound like it's something unfixable. > > Again, it should have been fixed before removing the old driver. Yeah, but we also don't have a whole lot people who seem to push for this. So the only way to find the people who still care is to remove the fbdev drivers. fbdev is dead. I'm totally fine reverting the driver to shut up the regression report, but it's not fixing anything long term. There's not going to be new drivers, or new hw support in existing drivers in fbdev. Also note that there's not spec nor test suite for fbdev, so "you guys should have known before removing drivers" doesn't work. -Daniel > > Peter > > > -Daniel > > > > > > > > > > Signed-off-by: Peter Collingbourne > > > > --- > > > > View this change in Gerrit: > > > > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > > > > > > > MAINTAINERS | 5 + > > > > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > > > > drivers/gpu/drm/pl111/pl111_display.c | 1 + > > > > drivers/gpu/drm/pl111/pl111_drm.h | 73 -- > > > > drivers/gpu/drm/pl111/pl111_drv.c | 1 + > > > > drivers/gpu/drm/pl111/pl111_versatile.c | 1 + > > > > drivers/video/fbdev/Kconfig | 20 + > > > > drivers/video/fbdev/Makefile| 1 + > > > > drivers/video/fbdev/amba-clcd.c | 986 > > > > include/linux/amba/clcd-regs.h | 87 +++ > > > > include/linux/amba/clcd.h | 290 +++ > > > > 11 files changed, 1393 insertions(+), 73 deletions(-) > > > > create mode 100644 drivers/video/fbdev/amba-clcd.c > > > > create mode 100644 include/linux/amba/clcd-regs.h > > > > create mode 100644 include/linux/amba/clcd.h > > > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > index 190c7fa2ea01..671c1fa79e64 100644 > > > > --- a/MAINTAINERS > > > > +++ b/MAINTAINERS > > > > @@ -1460,6 +1460,11 @@ S: Odd Fixes > > > > F: drivers/amba/ > > > > F: include/linux/amba/bus.h > > > > > > > > +ARM PRIMECELL CLCD PL110 DRIVER > > > > +M: Russell King > > > > +S: Odd Fixes > > > > +F: drivers/video/fbdev/amba-clcd.* > > > > + > > > > ARM PRIMECELL KMI PL050 DRIVER > > > > M: Russell King > > > > S: Odd Fixes > > > > diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c > > > > b/drivers/gpu/drm/pl111/pl111_debugfs.c > > > > index 317f68abf18b..26ca8cdf3e60 100644 > > > > --- a/drivers/gpu/drm/pl111/pl111_debugfs.c > > > > +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c > > > > @@ -3,6 +3,7 @@ > > > > * Copyright © 2017 Broadcom
[PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
Also revert the follow-up change "drm: pl111: Absorb the external register header". This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. The fbdev driver is used by Android's FVP configuration. Using the DRM driver together with DRM's fbdev emulation results in a failure to boot Android. The root cause is that Android's generic fbdev userspace driver relies on the ability to set the pixel format via FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. There have been other less critical behavioral differences identified between the fbdev driver and the DRM driver with fbdev emulation. The DRM driver exposes different values for the panel's width, height and refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall with yres_virtual greater than the maximum supported value instead of letting the syscall succeed and setting yres_virtual based on yres. Signed-off-by: Peter Collingbourne --- View this change in Gerrit: https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 MAINTAINERS | 5 + drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + drivers/gpu/drm/pl111/pl111_display.c | 1 + drivers/gpu/drm/pl111/pl111_drm.h | 73 -- drivers/gpu/drm/pl111/pl111_drv.c | 1 + drivers/gpu/drm/pl111/pl111_versatile.c | 1 + drivers/video/fbdev/Kconfig | 20 + drivers/video/fbdev/Makefile| 1 + drivers/video/fbdev/amba-clcd.c | 986 include/linux/amba/clcd-regs.h | 87 +++ include/linux/amba/clcd.h | 290 +++ 11 files changed, 1393 insertions(+), 73 deletions(-) create mode 100644 drivers/video/fbdev/amba-clcd.c create mode 100644 include/linux/amba/clcd-regs.h create mode 100644 include/linux/amba/clcd.h diff --git a/MAINTAINERS b/MAINTAINERS index 190c7fa2ea01..671c1fa79e64 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1460,6 +1460,11 @@ S: Odd Fixes F: drivers/amba/ F: include/linux/amba/bus.h +ARM PRIMECELL CLCD PL110 DRIVER +M: Russell King +S: Odd Fixes +F: drivers/video/fbdev/amba-clcd.* + ARM PRIMECELL KMI PL050 DRIVER M: Russell King S: Odd Fixes diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c b/drivers/gpu/drm/pl111/pl111_debugfs.c index 317f68abf18b..26ca8cdf3e60 100644 --- a/drivers/gpu/drm/pl111/pl111_debugfs.c +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c @@ -3,6 +3,7 @@ * Copyright © 2017 Broadcom */ +#include #include #include diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c index b3e8697cafcf..703ddc803c55 100644 --- a/drivers/gpu/drm/pl111/pl111_display.c +++ b/drivers/gpu/drm/pl111/pl111_display.c @@ -9,6 +9,7 @@ * Copyright (C) 2011 Texas Instruments */ +#include #include #include #include diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h index 2a46b5bd8576..ba399bcb792f 100644 --- a/drivers/gpu/drm/pl111/pl111_drm.h +++ b/drivers/gpu/drm/pl111/pl111_drm.h @@ -23,79 +23,6 @@ #include #include -/* - * CLCD Controller Internal Register addresses - */ -#define CLCD_TIM0 0x -#define CLCD_TIM1 0x0004 -#define CLCD_TIM2 0x0008 -#define CLCD_TIM3 0x000c -#define CLCD_UBAS 0x0010 -#define CLCD_LBAS 0x0014 - -#define CLCD_PL110_IENB0x0018 -#define CLCD_PL110_CNTL0x001c -#define CLCD_PL110_STAT0x0020 -#define CLCD_PL110_INTR0x0024 -#define CLCD_PL110_UCUR0x0028 -#define CLCD_PL110_LCUR0x002C - -#define CLCD_PL111_CNTL0x0018 -#define CLCD_PL111_IENB0x001c -#define CLCD_PL111_RIS 0x0020 -#define CLCD_PL111_MIS 0x0024 -#define CLCD_PL111_ICR 0x0028 -#define CLCD_PL111_UCUR0x002c -#define CLCD_PL111_LCUR0x0030 - -#define CLCD_PALL 0x0200 -#define CLCD_PALETTE 0x0200 - -#define TIM2_PCD_LO_MASK GENMASK(4, 0) -#define TIM2_PCD_LO_BITS 5 -#define TIM2_CLKSEL(1 << 5) -#define TIM2_ACB_MASK GENMASK(10, 6) -#define TIM2_IVS (1 << 11) -#define TIM2_IHS (1 << 12) -#define TIM2_IPC (1 << 13) -#define TIM2_IOE (1 << 14) -#define TIM2_BCD (1 << 26) -#define TIM2_PCD_HI_MASK GENMASK(31, 27) -#define TIM2_PCD_HI_BITS 5 -#define TIM2_PCD_HI_SHIFT 27 - -#define CNTL_LCDEN (1 << 0) -#define CNTL_LCDBPP1 (0 << 1) -#define CNTL_LCDBPP2 (1 << 1) -#define CNTL_LCDBPP4 (2 << 1) -#define CNTL_LCDBPP8 (3 << 1) -#define CNTL_LCDBPP16 (4 << 1) -#define CNTL_LCDBPP16_565 (6 << 1) -#define CN
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > Hi, > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > Also revert the follow-up change "drm: pl111: Absorb the external > > register header". > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > The fbdev driver is used by Android's FVP configuration. Using the > > DRM driver together with DRM's fbdev emulation results in a failure > > to boot Android. The root cause is that Android's generic fbdev > > userspace driver relies on the ability to set the pixel format via > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > Can't Android FVP use drm-hwcomposer instead ? Also, if we need to add more random fbdev ioctls to the drm fbdev emulation, then let's do that. Not keep fbdev drivers on life support for longer than necessary. > > Neil > > > > > There have been other less critical behavioral differences identified > > between the fbdev driver and the DRM driver with fbdev emulation. The > > DRM driver exposes different values for the panel's width, height and > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > > with yres_virtual greater than the maximum supported value instead > > of letting the syscall succeed and setting yres_virtual based on yres. Also something that should be fixed I think in upstream, in the drm fbdev emulation. At least doesn't sound like it's something unfixable. -Daniel > > > > Signed-off-by: Peter Collingbourne > > --- > > View this change in Gerrit: > > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > > > MAINTAINERS | 5 + > > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > > drivers/gpu/drm/pl111/pl111_display.c | 1 + > > drivers/gpu/drm/pl111/pl111_drm.h | 73 -- > > drivers/gpu/drm/pl111/pl111_drv.c | 1 + > > drivers/gpu/drm/pl111/pl111_versatile.c | 1 + > > drivers/video/fbdev/Kconfig | 20 + > > drivers/video/fbdev/Makefile| 1 + > > drivers/video/fbdev/amba-clcd.c | 986 > > include/linux/amba/clcd-regs.h | 87 +++ > > include/linux/amba/clcd.h | 290 +++ > > 11 files changed, 1393 insertions(+), 73 deletions(-) > > create mode 100644 drivers/video/fbdev/amba-clcd.c > > create mode 100644 include/linux/amba/clcd-regs.h > > create mode 100644 include/linux/amba/clcd.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 190c7fa2ea01..671c1fa79e64 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -1460,6 +1460,11 @@ S: Odd Fixes > > F: drivers/amba/ > > F: include/linux/amba/bus.h > > > > +ARM PRIMECELL CLCD PL110 DRIVER > > +M: Russell King > > +S: Odd Fixes > > +F: drivers/video/fbdev/amba-clcd.* > > + > > ARM PRIMECELL KMI PL050 DRIVER > > M: Russell King > > S: Odd Fixes > > diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c > > b/drivers/gpu/drm/pl111/pl111_debugfs.c > > index 317f68abf18b..26ca8cdf3e60 100644 > > --- a/drivers/gpu/drm/pl111/pl111_debugfs.c > > +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c > > @@ -3,6 +3,7 @@ > > * Copyright © 2017 Broadcom > > */ > > > > +#include > > #include > > > > #include > > diff --git a/drivers/gpu/drm/pl111/pl111_display.c > > b/drivers/gpu/drm/pl111/pl111_display.c > > index b3e8697cafcf..703ddc803c55 100644 > > --- a/drivers/gpu/drm/pl111/pl111_display.c > > +++ b/drivers/gpu/drm/pl111/pl111_display.c > > @@ -9,6 +9,7 @@ > > * Copyright (C) 2011 Texas Instruments > > */ > > > > +#include > > #include > > #include > > #include > > diff --git a/drivers/gpu/drm/pl111/pl111_drm.h > > b/drivers/gpu/drm/pl111/pl111_drm.h > > index 2a46b5bd8576..ba399bcb792f 100644 > > --- a/drivers/gpu/drm/pl111/pl111_drm.h > > +++ b/drivers/gpu/drm/pl111/pl111_drm.h > > @@ -23,79 +23,6 @@ > > #include > > #include > > > > -/* > > - * CLCD Controller Internal Register addresses > > - */ > > -#define CLCD_TIM0 0x > > -#define CLCD_TIM1 0x0004 > > -#define CLCD_TIM2 0x0008 > > -#define CLCD_TIM3 0x000c > > -#define CLCD_UBAS 0x0010 > > -#define CLCD_LBAS 0x0014 > > - > > -#define CLCD_PL110_IENB0x0018 > > -#define CLCD_PL110_CNTL0x001c > > -#define CLCD_PL110_STAT0x0020 > > -#define CLCD_PL110_INTR0x0024 > > -#define CLCD_PL110_UCUR0x0028 > > -#define CLCD_PL110_LCUR0x002C > > - > > -#define CLCD_PL111_CNTL0x0018 > > -#define CLCD_PL111_IENB0x001c > > -#define CLCD_PL111_RIS 0x0020 > > -#define CLCD_PL111_MIS 0x0024 > > -#define CLCD_PL111_ICR 0x0028 > > -#define CLCD_PL111_UCUR0x002c > > -#define CLCD_PL111_LCUR0x0030 > > - >
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
Hi, On 28/09/2020 22:08, Peter Collingbourne wrote: > Also revert the follow-up change "drm: pl111: Absorb the external > register header". > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > The fbdev driver is used by Android's FVP configuration. Using the > DRM driver together with DRM's fbdev emulation results in a failure > to boot Android. The root cause is that Android's generic fbdev > userspace driver relies on the ability to set the pixel format via > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. Can't Android FVP use drm-hwcomposer instead ? Neil > > There have been other less critical behavioral differences identified > between the fbdev driver and the DRM driver with fbdev emulation. The > DRM driver exposes different values for the panel's width, height and > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > with yres_virtual greater than the maximum supported value instead > of letting the syscall succeed and setting yres_virtual based on yres. > > Signed-off-by: Peter Collingbourne > --- > View this change in Gerrit: > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > MAINTAINERS | 5 + > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > drivers/gpu/drm/pl111/pl111_display.c | 1 + > drivers/gpu/drm/pl111/pl111_drm.h | 73 -- > drivers/gpu/drm/pl111/pl111_drv.c | 1 + > drivers/gpu/drm/pl111/pl111_versatile.c | 1 + > drivers/video/fbdev/Kconfig | 20 + > drivers/video/fbdev/Makefile| 1 + > drivers/video/fbdev/amba-clcd.c | 986 > include/linux/amba/clcd-regs.h | 87 +++ > include/linux/amba/clcd.h | 290 +++ > 11 files changed, 1393 insertions(+), 73 deletions(-) > create mode 100644 drivers/video/fbdev/amba-clcd.c > create mode 100644 include/linux/amba/clcd-regs.h > create mode 100644 include/linux/amba/clcd.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 190c7fa2ea01..671c1fa79e64 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1460,6 +1460,11 @@ S: Odd Fixes > F: drivers/amba/ > F: include/linux/amba/bus.h > > +ARM PRIMECELL CLCD PL110 DRIVER > +M: Russell King > +S: Odd Fixes > +F: drivers/video/fbdev/amba-clcd.* > + > ARM PRIMECELL KMI PL050 DRIVER > M: Russell King > S: Odd Fixes > diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c > b/drivers/gpu/drm/pl111/pl111_debugfs.c > index 317f68abf18b..26ca8cdf3e60 100644 > --- a/drivers/gpu/drm/pl111/pl111_debugfs.c > +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c > @@ -3,6 +3,7 @@ > * Copyright © 2017 Broadcom > */ > > +#include > #include > > #include > diff --git a/drivers/gpu/drm/pl111/pl111_display.c > b/drivers/gpu/drm/pl111/pl111_display.c > index b3e8697cafcf..703ddc803c55 100644 > --- a/drivers/gpu/drm/pl111/pl111_display.c > +++ b/drivers/gpu/drm/pl111/pl111_display.c > @@ -9,6 +9,7 @@ > * Copyright (C) 2011 Texas Instruments > */ > > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/pl111/pl111_drm.h > b/drivers/gpu/drm/pl111/pl111_drm.h > index 2a46b5bd8576..ba399bcb792f 100644 > --- a/drivers/gpu/drm/pl111/pl111_drm.h > +++ b/drivers/gpu/drm/pl111/pl111_drm.h > @@ -23,79 +23,6 @@ > #include > #include > > -/* > - * CLCD Controller Internal Register addresses > - */ > -#define CLCD_TIM00x > -#define CLCD_TIM10x0004 > -#define CLCD_TIM20x0008 > -#define CLCD_TIM30x000c > -#define CLCD_UBAS0x0010 > -#define CLCD_LBAS0x0014 > - > -#define CLCD_PL110_IENB 0x0018 > -#define CLCD_PL110_CNTL 0x001c > -#define CLCD_PL110_STAT 0x0020 > -#define CLCD_PL110_INTR 0x0024 > -#define CLCD_PL110_UCUR 0x0028 > -#define CLCD_PL110_LCUR 0x002C > - > -#define CLCD_PL111_CNTL 0x0018 > -#define CLCD_PL111_IENB 0x001c > -#define CLCD_PL111_RIS 0x0020 > -#define CLCD_PL111_MIS 0x0024 > -#define CLCD_PL111_ICR 0x0028 > -#define CLCD_PL111_UCUR 0x002c > -#define CLCD_PL111_LCUR 0x0030 > - > -#define CLCD_PALL0x0200 > -#define CLCD_PALETTE 0x0200 > - > -#define TIM2_PCD_LO_MASK GENMASK(4, 0) > -#define TIM2_PCD_LO_BITS 5 > -#define TIM2_CLKSEL (1 << 5) > -#define TIM2_ACB_MASKGENMASK(10, 6) > -#define TIM2_IVS (1 << 11) > -#define TIM2_IHS (1 << 12) > -#define TIM2_IPC (1 << 13) > -#define TIM2_IOE (1 << 14) > -#define TIM2_BCD (1 << 26) > -#define TIM2_PCD_HI_MASK GENMASK(31, 27) > -#define TIM2_PCD_HI_BITS 5 > -#define TIM2
