* Re: [PATCH RESEND 0/2] Refactoring the fbcon packed pixel drawing routines
From: Helge Deller @ 2025-03-19 18:04 UTC (permalink / raw)
To: Zsolt Kajtar, linux-fbdev, dri-devel, Thomas Zimmermann
In-Reply-To: <20250309184716.13732-1-soci@c64.rulez.org>
On 3/9/25 19:47, Zsolt Kajtar wrote:
> This is the same patch as before just updated to latest fbdev
> master and with better description. And hopefully sent intact this
> time.
>
> Zsolt Kajtar (2):
> Refactoring the fbcon packed pixel drawing routines
> Adding contact info for packed pixel drawing
This patch series has now been in fbdev for-next git tree for 10 days
without any major issues reported so far.
I think it's a good and necessary cleanup, although it's of
course quite big (but mostly copy&paste).
I'd like to get some feedback if there are any major
objections that this goes upstream during the next merge window?
(Thomas: I know you were not very enthusiastic about
the previous patch set. I think this one is better.)
Helge
> MAINTAINERS | 16 +
> drivers/video/fbdev/core/Kconfig | 10 +-
> drivers/video/fbdev/core/cfbcopyarea.c | 428 +-------------------
> drivers/video/fbdev/core/cfbfillrect.c | 362 +----------------
> drivers/video/fbdev/core/cfbimgblt.c | 357 +----------------
> drivers/video/fbdev/core/cfbmem.h | 43 ++
> drivers/video/fbdev/core/fb_copyarea.h | 405 +++++++++++++++++++
> drivers/video/fbdev/core/fb_draw.h | 274 ++++++-------
> drivers/video/fbdev/core/fb_fillrect.h | 280 ++++++++++++++
> drivers/video/fbdev/core/fb_imageblit.h | 495 ++++++++++++++++++++++++
> drivers/video/fbdev/core/syscopyarea.c | 369 +-----------------
> drivers/video/fbdev/core/sysfillrect.c | 324 +---------------
> drivers/video/fbdev/core/sysimgblt.c | 333 +---------------
> drivers/video/fbdev/core/sysmem.h | 39 ++
> 14 files changed, 1480 insertions(+), 2255 deletions(-)
> create mode 100644 drivers/video/fbdev/core/cfbmem.h
> create mode 100644 drivers/video/fbdev/core/fb_copyarea.h
> create mode 100644 drivers/video/fbdev/core/fb_fillrect.h
> create mode 100644 drivers/video/fbdev/core/fb_imageblit.h
> create mode 100644 drivers/video/fbdev/core/sysmem.h
>
^ permalink raw reply
* RE: fbdev deferred I/O broken in some scenarios
From: Michael Kelley @ 2025-03-19 20:29 UTC (permalink / raw)
To: Helge Deller, linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <303572c2-4839-4dae-a249-9967fcc9cf03@gmx.de>
From: Helge Deller <deller@gmx.de> Sent: Tuesday, March 18, 2025 1:16 AM
> Hi Michael,
>
> On 3/18/25 03:05, Michael Kelley wrote:
> > I've been trying to get mmap() working with the hyperv_fb.c fbdev driver, which
> > is for Linux guests running on Microsoft's Hyper-V hypervisor. The hyperv_fb driver
> > uses fbdev deferred I/O for performance reasons. But it looks to me like fbdev
> > deferred I/O is fundamentally broken when the underlying framebuffer memory
> > is allocated from kernel memory (alloc_pages or dma_alloc_coherent).
> >
> > The hyperv_fb.c driver may allocate the framebuffer memory in several ways,
> > depending on the size of the framebuffer specified by the Hyper-V host and the VM
> > "Generation". For a Generation 2 VM, the framebuffer memory is allocated by the
> > Hyper-V host and is assigned to guest MMIO space. The hyperv_fb driver does a
> > vmalloc() allocation for deferred I/O to work against. This combination handles mmap()
> > of /dev/fb<n> correctly and the performance benefits of deferred I/O are substantial.
> >
> > But for a Generation 1 VM, the hyperv_fb driver allocates the framebuffer memory in
> > contiguous guest physical memory using alloc_pages() or dma_alloc_coherent(), and
> > informs the Hyper-V host of the location. In this case, mmap() with deferred I/O does
> > not work. The mmap() succeeds, and user space updates to the mmap'ed memory are
> > correctly reflected to the framebuffer. But when the user space program does munmap()
> > or terminates, the Linux kernel free lists become scrambled and the kernel eventually
> > panics. The problem is that when munmap() is done, the PTEs in the VMA are cleaned
> > up, and the corresponding struct page refcounts are decremented. If the refcount goes
> > to zero (which it typically will), the page is immediately freed. In this way, some or all
> > of the framebuffer memory gets erroneously freed. From what I see, the VMA should
> > be marked VM_PFNMAP when allocated memory kernel is being used as the
> > framebuffer with deferred I/O, but that's not happening. The handling of deferred I/O
> > page faults would also need updating to make this work.
> >
> > The fbdev deferred I/O support was originally added to the hyperv_fb driver in the
> > 5.6 kernel, and based on my recent experiments, it has never worked correctly when
> > the framebuffer is allocated from kernel memory. fbdev deferred I/O support for using
> > kernel memory as the framebuffer was originally added in commit 37b4837959cb9
> > back in 2008 in Linux 2.6.29. But I don't see how it ever worked properly, unless
> > changes in generic memory management somehow broke it in the intervening years.
> >
> > I think I know how to fix all this. But before working on a patch, I wanted to check
> > with the fbdev community to see if this might be a known issue and whether there
> > is any additional insight someone might offer. Thanks for any comments or help.
>
> I haven't heard of any major deferred-i/o issues since I've jumped into fbdev
> maintenance. But you might be right, as I haven't looked much into it yet and
> there are just a few drivers using it.
>
Thanks for the input. In the fbdev directory, there are 9 drivers using deferred I/O.
Of those, 6 use vmalloc() to allocate the framebuffer, and that path works just fine.
The other 3 use alloc_pages(), dma_alloc_coherent(), or __get_free_pages(), all of
which manifest the underlying problem when munmap()'ed. Those 3 drivers are:
* hyperv_fb.c, which I'm working with
* sh_mobile_lcdcfb.c
* ssd1307fb.c
Do you have any ownership or status information about the last two? Neither is
listed in MAINTAINERS, so maybe they are for old devices and now effectively
abandoned. Before I make code changes to fb_defio.c, I wanted to make sure
I have all the context I can get, such as why this problem hasn't surfaced outside
of the hyperv_fb.c driver. Part of the reason is that evidently the vmalloc() based
approach is more common, and it works fine. With only two other drivers using
contiguous kernel memory allocations, and with those two perhaps being for old
and now mostly unused devices, that might explain things. The hyperv_fb.c
config that uses contiguous kernel memory is also somewhat rare, and I only
stumbled across it when debugging other problems.
In any case, I'm pretty convinced this is an issue with fb_defio.c and not
something specific to hyperv_fb.c, so I'll get to work on a fix and how it goes
from there.
Michael
^ permalink raw reply
* RE: fbdev deferred I/O broken in some scenarios
From: Michael Kelley @ 2025-03-19 20:38 UTC (permalink / raw)
To: Thomas Zimmermann, linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <a4fa1dbb-4989-4fc2-acbc-055f786e9b48@suse.de>
From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Tuesday, March 18, 2025 1:26 AM
>
> Am 18.03.25 um 03:05 schrieb Michael Kelley:
> > I've been trying to get mmap() working with the hyperv_fb.c fbdev driver, which
> > is for Linux guests running on Microsoft's Hyper-V hypervisor. The hyperv_fb driver
> > uses fbdev deferred I/O for performance reasons. But it looks to me like fbdev
> > deferred I/O is fundamentally broken when the underlying framebuffer memory
> > is allocated from kernel memory (alloc_pages or dma_alloc_coherent).
> >
> > The hyperv_fb.c driver may allocate the framebuffer memory in several ways,
> > depending on the size of the framebuffer specified by the Hyper-V host and the VM
> > "Generation". For a Generation 2 VM, the framebuffer memory is allocated by the
> > Hyper-V host and is assigned to guest MMIO space. The hyperv_fb driver does a
> > vmalloc() allocation for deferred I/O to work against. This combination handles mmap()
> > of /dev/fb<n> correctly and the performance benefits of deferred I/O are substantial.
> >
> > But for a Generation 1 VM, the hyperv_fb driver allocates the framebuffer memory in
> > contiguous guest physical memory using alloc_pages() or dma_alloc_coherent(), and
> > informs the Hyper-V host of the location. In this case, mmap() with deferred I/O does
> > not work. The mmap() succeeds, and user space updates to the mmap'ed memory are
> > correctly reflected to the framebuffer. But when the user space program does munmap()
> > or terminates, the Linux kernel free lists become scrambled and the kernel eventually
> > panics. The problem is that when munmap() is done, the PTEs in the VMA are cleaned
> > up, and the corresponding struct page refcounts are decremented. If the refcount goes
> > to zero (which it typically will), the page is immediately freed. In this way, some or all
> > of the framebuffer memory gets erroneously freed. From what I see, the VMA should
> > be marked VM_PFNMAP when allocated memory kernel is being used as the
> > framebuffer with deferred I/O, but that's not happening. The handling of deferred I/O
> > page faults would also need updating to make this work.
>
> I cannot help much with HyperV, but there's a get_page callback in
> struct fb_deferred_io. [1] It'll allow you to provide a custom page on
> each page fault. We use it in DRM to mmap SHMEM-backed pages. [2] Maybe
> this helps with hyperv_fb as well.
>
Thanks for your input. See also my reply to Helge.
Unfortunately, using a custom get_page() callback doesn't help. In the problematic
case, the standard deferred I/O get_page() function works correctly for getting the
struct page. My current thinking is that the problem is in fb_deferred_io_mmap()
where the vma needs to have the VM_PFNMAP flag set when the framebuffer
memory is a direct kernel allocation and not through vmalloc(). And there may be
some implications on the mkwrite function as well, but I'll need to sort that out
once I start coding.
For the DRM code using SHMEM-backed pages, do you know where the shared
memory comes from? Is that ultimately a kernel vmalloc() allocation?
Michael
^ permalink raw reply
* Re: [PATCH] video: fbdev: sm501fb: Add some geometry checks.
From: Helge Deller @ 2025-03-19 21:38 UTC (permalink / raw)
To: Danila Chernetsov; +Cc: linux-fbdev, dri-devel, linux-kernel, lvc-project
In-Reply-To: <20250319013011.467530-1-listdansp@mail.ru>
On 3/19/25 02:30, Danila Chernetsov wrote:
> Added checks for xoffset, yoffset settings.
> Incorrect settings of these parameters can lead to errors
> in sm501fb_pan_ functions.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 5fc404e47bdf ("[PATCH] fb: SM501 framebuffer driver")
> Signed-off-by: Danila Chernetsov <listdansp@mail.ru>
> ---
> drivers/video/fbdev/sm501fb.c | 7 +++++++
> 1 file changed, 7 insertions(+)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH v2] fbcon: Use static attribute groups for sysfs entries
From: Helge Deller @ 2025-03-19 21:49 UTC (permalink / raw)
To: Thomas Zimmermann, oushixiong1025, Simona Vetter
Cc: Samuel Thibault, Zsolt Kajtar, Thomas Weißschuh, linux-fbdev,
dri-devel, linux-kernel, Shixiong Ou
In-Reply-To: <9aec4ef1-7428-4630-a4af-d7448a023a60@suse.de>
On 3/14/25 09:16, Thomas Zimmermann wrote:
> Am 14.03.25 um 07:09 schrieb oushixiong1025@163.com:
>> From: Shixiong Ou <oushixiong@kylinos.cn>
>>
>> Using device_create_with_groups() to simplify creation and removal.
>> Same as commit 1083a7be4504 ("tty: Use static attribute groups for
>> sysfs entries").
>>
>> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> with minor comments below
>
>> ---
>> drivers/video/fbdev/core/fbcon.c | 69 +++++++++-----------------------
>> 1 file changed, 19 insertions(+), 50 deletions(-)
series applied with changes as suggested by Thomas.
Thanks!
Helge
^ permalink raw reply
* Re: fbdev deferred I/O broken in some scenarios
From: Thomas Zimmermann @ 2025-03-20 7:45 UTC (permalink / raw)
To: Michael Kelley, linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <BN7PR02MB4148864DDB065271B79FD3E4D4D92@BN7PR02MB4148.namprd02.prod.outlook.com>
Hi
Am 19.03.25 um 21:38 schrieb Michael Kelley:
> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Tuesday, March 18, 2025 1:26 AM
>> Am 18.03.25 um 03:05 schrieb Michael Kelley:
>>> I've been trying to get mmap() working with the hyperv_fb.c fbdev driver, which
>>> is for Linux guests running on Microsoft's Hyper-V hypervisor. The hyperv_fb driver
>>> uses fbdev deferred I/O for performance reasons. But it looks to me like fbdev
>>> deferred I/O is fundamentally broken when the underlying framebuffer memory
>>> is allocated from kernel memory (alloc_pages or dma_alloc_coherent).
>>>
>>> The hyperv_fb.c driver may allocate the framebuffer memory in several ways,
>>> depending on the size of the framebuffer specified by the Hyper-V host and the VM
>>> "Generation". For a Generation 2 VM, the framebuffer memory is allocated by the
>>> Hyper-V host and is assigned to guest MMIO space. The hyperv_fb driver does a
>>> vmalloc() allocation for deferred I/O to work against. This combination handles mmap()
>>> of /dev/fb<n> correctly and the performance benefits of deferred I/O are substantial.
>>>
>>> But for a Generation 1 VM, the hyperv_fb driver allocates the framebuffer memory in
>>> contiguous guest physical memory using alloc_pages() or dma_alloc_coherent(), and
>>> informs the Hyper-V host of the location. In this case, mmap() with deferred I/O does
>>> not work. The mmap() succeeds, and user space updates to the mmap'ed memory are
>>> correctly reflected to the framebuffer. But when the user space program does munmap()
>>> or terminates, the Linux kernel free lists become scrambled and the kernel eventually
>>> panics. The problem is that when munmap() is done, the PTEs in the VMA are cleaned
>>> up, and the corresponding struct page refcounts are decremented. If the refcount goes
>>> to zero (which it typically will), the page is immediately freed. In this way, some or all
>>> of the framebuffer memory gets erroneously freed. From what I see, the VMA should
>>> be marked VM_PFNMAP when allocated memory kernel is being used as the
>>> framebuffer with deferred I/O, but that's not happening. The handling of deferred I/O
>>> page faults would also need updating to make this work.
>> I cannot help much with HyperV, but there's a get_page callback in
>> struct fb_deferred_io. [1] It'll allow you to provide a custom page on
>> each page fault. We use it in DRM to mmap SHMEM-backed pages. [2] Maybe
>> this helps with hyperv_fb as well.
>>
> Thanks for your input. See also my reply to Helge.
>
> Unfortunately, using a custom get_page() callback doesn't help. In the problematic
> case, the standard deferred I/O get_page() function works correctly for getting the
> struct page. My current thinking is that the problem is in fb_deferred_io_mmap()
> where the vma needs to have the VM_PFNMAP flag set when the framebuffer
> memory is a direct kernel allocation and not through vmalloc(). And there may be
> some implications on the mkwrite function as well, but I'll need to sort that out
> once I start coding.
>
> For the DRM code using SHMEM-backed pages, do you know where the shared
> memory comes from? Is that ultimately a kernel vmalloc() allocation?
I think it's something special, as the regular vmalloc'ed-pages would be
handled by fb_defio automatically. In DRM we sometimes also use a
separate vmalloc'ed shadow buffer that serves as the fbdev framebuffer.
We then sync internally with the physical framebuffer memory. See [1]
for the related code. Udlfb does the as well IIRC.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v6.13.7/source/drivers/gpu/drm/drm_fbdev_ttm.c#L201
>
> Michael
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* Re: fbdev deferred I/O broken in some scenarios
From: Geert Uytterhoeven @ 2025-03-20 10:46 UTC (permalink / raw)
To: Michael Kelley
Cc: Helge Deller, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <BN7PR02MB4148157E5307E306FD9D093ED4D92@BN7PR02MB4148.namprd02.prod.outlook.com>
Hi Michael,
On Wed, 19 Mar 2025 at 21:29, Michael Kelley <mhklinux@outlook.com> wrote:
> From: Helge Deller <deller@gmx.de> Sent: Tuesday, March 18, 2025 1:16 AM
> > On 3/18/25 03:05, Michael Kelley wrote:
> > > I've been trying to get mmap() working with the hyperv_fb.c fbdev driver, which
> > > is for Linux guests running on Microsoft's Hyper-V hypervisor. The hyperv_fb driver
> > > uses fbdev deferred I/O for performance reasons. But it looks to me like fbdev
> > > deferred I/O is fundamentally broken when the underlying framebuffer memory
> > > is allocated from kernel memory (alloc_pages or dma_alloc_coherent).
> > >
> > > The hyperv_fb.c driver may allocate the framebuffer memory in several ways,
> > > depending on the size of the framebuffer specified by the Hyper-V host and the VM
> > > "Generation". For a Generation 2 VM, the framebuffer memory is allocated by the
> > > Hyper-V host and is assigned to guest MMIO space. The hyperv_fb driver does a
> > > vmalloc() allocation for deferred I/O to work against. This combination handles mmap()
> > > of /dev/fb<n> correctly and the performance benefits of deferred I/O are substantial.
> > >
> > > But for a Generation 1 VM, the hyperv_fb driver allocates the framebuffer memory in
> > > contiguous guest physical memory using alloc_pages() or dma_alloc_coherent(), and
> > > informs the Hyper-V host of the location. In this case, mmap() with deferred I/O does
> > > not work. The mmap() succeeds, and user space updates to the mmap'ed memory are
> > > correctly reflected to the framebuffer. But when the user space program does munmap()
> > > or terminates, the Linux kernel free lists become scrambled and the kernel eventually
> > > panics. The problem is that when munmap() is done, the PTEs in the VMA are cleaned
> > > up, and the corresponding struct page refcounts are decremented. If the refcount goes
> > > to zero (which it typically will), the page is immediately freed. In this way, some or all
> > > of the framebuffer memory gets erroneously freed. From what I see, the VMA should
> > > be marked VM_PFNMAP when allocated memory kernel is being used as the
> > > framebuffer with deferred I/O, but that's not happening. The handling of deferred I/O
> > > page faults would also need updating to make this work.
I assume this is triggered by running any fbdev userspace that uses
mmap(), e.g. fbtest?
> > > The fbdev deferred I/O support was originally added to the hyperv_fb driver in the
> > > 5.6 kernel, and based on my recent experiments, it has never worked correctly when
> > > the framebuffer is allocated from kernel memory. fbdev deferred I/O support for using
> > > kernel memory as the framebuffer was originally added in commit 37b4837959cb9
> > > back in 2008 in Linux 2.6.29. But I don't see how it ever worked properly, unless
> > > changes in generic memory management somehow broke it in the intervening years.
> > >
> > > I think I know how to fix all this. But before working on a patch, I wanted to check
> > > with the fbdev community to see if this might be a known issue and whether there
> > > is any additional insight someone might offer. Thanks for any comments or help.
> >
> > I haven't heard of any major deferred-i/o issues since I've jumped into fbdev
> > maintenance. But you might be right, as I haven't looked much into it yet and
> > there are just a few drivers using it.
>
> Thanks for the input. In the fbdev directory, there are 9 drivers using deferred I/O.
> Of those, 6 use vmalloc() to allocate the framebuffer, and that path works just fine.
> The other 3 use alloc_pages(), dma_alloc_coherent(), or __get_free_pages(), all of
> which manifest the underlying problem when munmap()'ed. Those 3 drivers are:
>
> * hyperv_fb.c, which I'm working with
> * sh_mobile_lcdcfb.c
> * ssd1307fb.c
Nowadays sh_mobile_lcdcfb is used only on various SuperH boards
(I have no hardware to test).
sh_mobile_lcdcfb was used on ARM-based SH/R-Mobile SoCs until DT
support was added to the DRM driver for the corresponding hardware.
The platform using it was migrated to DRM in commit 138588e9fa237f97
("ARM: dts: renesas: r8a7740: Add LCDC nodes") in v6.8). At the time
of the conversion, fbtest worked fine with sh_mobile_lcdcfb.
Deferred I/O is also used in DRM drivers for displays that are connected
using I2C or SPI. Last time I tried the st7735r driver, it worked fine
with fbtest. That was also on arm32, though.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] [PATCH v2] staging: sm750fb: Make g_fbmode truly constant
From: Greg KH @ 2025-03-20 14:09 UTC (permalink / raw)
To: Madhur Kumar
Cc: sudipm.mukherjee, teddy.wang, linux-fbdev, linux-staging,
linux-kernel, Dan Carpenter
In-Reply-To: <20250222201514.15730-1-madhurkumar004@gmail.com>
On Sun, Feb 23, 2025 at 01:45:14AM +0530, Madhur Kumar wrote:
> Declare g_fbmode as a pointer to constant data. This ensures that both
> array and its element are immutable.
>
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Madhur Kumar <madhurkumar004@gmail.com>
> ---
> Changes in v2:
> - Added commit message
> ---
> drivers/staging/sm750fb/sm750.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index 04c1b32a2..aa154032f 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -33,7 +33,7 @@
> static int g_hwcursor = 1;
> static int g_noaccel;
> static int g_nomtrr;
> -static const char *g_fbmode[] = {NULL, NULL};
> +static const char * const g_fbmode[] = {NULL, NULL};
How did you test this? It totally breaks the build :(
^ permalink raw reply
* Re: [PATCH v2] staging: sm750fb: Remove unused enum dpms
From: Greg Kroah-Hartman @ 2025-03-20 14:12 UTC (permalink / raw)
To: Gabriel Lima Luz
Cc: linux-fbdev, linux-staging, linux-kernel, Sudip Mukherjee,
Teddy Wang, ~lkcamp/patches
In-Reply-To: <20250314173114.4476-1-lima.gabriel.luz@gmail.com>
On Fri, Mar 14, 2025 at 02:31:09PM -0300, Gabriel Lima Luz wrote:
> remove unused enum and replace its usage with
> a unsigned int.
> add comment to ddk750_set_dpms function for
> documenting state values
>
> Signed-off-by: Gabriel Lima Luz <lima.gabriel.luz@gmail.com>
> ---
> drivers/staging/sm750fb/ddk750_power.c | 10 ++++++++--
> drivers/staging/sm750fb/ddk750_power.h | 9 +--------
> 2 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_power.c b/drivers/staging/sm750fb/ddk750_power.c
> index 12834f78eef7..547a96ccba9b 100644
> --- a/drivers/staging/sm750fb/ddk750_power.c
> +++ b/drivers/staging/sm750fb/ddk750_power.c
> @@ -3,8 +3,14 @@
> #include "ddk750_reg.h"
> #include "ddk750_power.h"
>
> -void ddk750_set_dpms(enum dpms state)
> -{
> +void ddk750_set_dpms(unsigned int state)
> +{ /*
> + * state values documentation
> + * crt_DPMS_ON = 0x0,
> + * crt_DPMS_STANDBY = 0x1,
> + * crt_DPMS_SUSPEND = 0x2,
> + * crt_DPMS_OFF = 0x3, unsigned int value;
> + */
> unsigned int value;
Why not just make this "enum dpms" instead?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: Replace architecture-specific defines with CONFIG_X86
From: Greg KH @ 2025-03-20 14:12 UTC (permalink / raw)
To: Madhur Kumar
Cc: sudipm.mukherjee, teddy.wang, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20250222193242.14302-1-madhurkumar004@gmail.com>
On Sun, Feb 23, 2025 at 01:02:42AM +0530, Madhur Kumar wrote:
> Replace the use of __i386__ and __x86_64__ with CONFIG_X86 to adhere to
> kernel coding guideline, make the code more portable and integrates
> better with the Kconfig system.
>
> Signed-off-by: Madhur Kumar <madhurkumar004@gmail.com>
> ---
> drivers/staging/sm750fb/ddk750_chip.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
> index 02860d3ec..67a2f6044 100644
> --- a/drivers/staging/sm750fb/ddk750_chip.c
> +++ b/drivers/staging/sm750fb/ddk750_chip.c
> @@ -229,7 +229,7 @@ int ddk750_init_hw(struct initchip_param *p_init_param)
> reg |= (VGA_CONFIGURATION_PLL | VGA_CONFIGURATION_MODE);
> poke32(VGA_CONFIGURATION, reg);
> } else {
> -#if defined(__i386__) || defined(__x86_64__)
> +#ifdef CONFIG_X86
> /* set graphic mode via IO method */
> outb_p(0x88, 0x3d4);
> outb_p(0x06, 0x3d5);
> --
> 2.48.1
>
>
Does not apply to my tree at all :(
^ permalink raw reply
* RE: fbdev deferred I/O broken in some scenarios
From: Michael Kelley @ 2025-03-20 20:15 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Helge Deller, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <CAMuHMdVvuPr454jcVExGpHX_94_=y0GfVPJu1YLO1-H0OkBTdQ@mail.gmail.com>
From: Geert Uytterhoeven <geert@linux-m68k.org> Sent: Thursday, March 20, 2025 3:46 AM
>
> Hi Michael,
>
> On Wed, 19 Mar 2025 at 21:29, Michael Kelley <mhklinux@outlook.com> wrote:
> > From: Helge Deller <deller@gmx.de> Sent: Tuesday, March 18, 2025 1:16 AM
> > > On 3/18/25 03:05, Michael Kelley wrote:
> > > > I've been trying to get mmap() working with the hyperv_fb.c fbdev driver, which
> > > > is for Linux guests running on Microsoft's Hyper-V hypervisor. The hyperv_fb driver
> > > > uses fbdev deferred I/O for performance reasons. But it looks to me like fbdev
> > > > deferred I/O is fundamentally broken when the underlying framebuffer memory
> > > > is allocated from kernel memory (alloc_pages or dma_alloc_coherent).
> > > >
> > > > The hyperv_fb.c driver may allocate the framebuffer memory in several ways,
> > > > depending on the size of the framebuffer specified by the Hyper-V host and the VM
> > > > "Generation". For a Generation 2 VM, the framebuffer memory is allocated by the
> > > > Hyper-V host and is assigned to guest MMIO space. The hyperv_fb driver does a
> > > > vmalloc() allocation for deferred I/O to work against. This combination handles mmap()
> > > > of /dev/fb<n> correctly and the performance benefits of deferred I/O are substantial.
> > > >
> > > > But for a Generation 1 VM, the hyperv_fb driver allocates the framebuffer memory in
> > > > contiguous guest physical memory using alloc_pages() or dma_alloc_coherent(), and
> > > > informs the Hyper-V host of the location. In this case, mmap() with deferred I/O does
> > > > not work. The mmap() succeeds, and user space updates to the mmap'ed memory are
> > > > correctly reflected to the framebuffer. But when the user space program does munmap()
> > > > or terminates, the Linux kernel free lists become scrambled and the kernel eventually
> > > > panics. The problem is that when munmap() is done, the PTEs in the VMA are cleaned
> > > > up, and the corresponding struct page refcounts are decremented. If the refcount goes
> > > > to zero (which it typically will), the page is immediately freed. In this way, some or all
> > > > of the framebuffer memory gets erroneously freed. From what I see, the VMA should
> > > > be marked VM_PFNMAP when allocated memory kernel is being used as the
> > > > framebuffer with deferred I/O, but that's not happening. The handling of deferred I/O
> > > > page faults would also need updating to make this work.
>
> I assume this is triggered by running any fbdev userspace that uses
> mmap(), e.g. fbtest?
Yes. I have my own little program, but it is similar to what I see fbtest does.
>
> > > > The fbdev deferred I/O support was originally added to the hyperv_fb driver in the
> > > > 5.6 kernel, and based on my recent experiments, it has never worked correctly when
> > > > the framebuffer is allocated from kernel memory. fbdev deferred I/O support for using
> > > > kernel memory as the framebuffer was originally added in commit 37b4837959cb9
> > > > back in 2008 in Linux 2.6.29. But I don't see how it ever worked properly, unless
> > > > changes in generic memory management somehow broke it in the intervening years.
> > > >
> > > > I think I know how to fix all this. But before working on a patch, I wanted to check
> > > > with the fbdev community to see if this might be a known issue and whether there
> > > > is any additional insight someone might offer. Thanks for any comments or help.
> > >
> > > I haven't heard of any major deferred-i/o issues since I've jumped into fbdev
> > > maintenance. But you might be right, as I haven't looked much into it yet and
> > > there are just a few drivers using it.
> >
> > Thanks for the input. In the fbdev directory, there are 9 drivers using deferred I/O.
> > Of those, 6 use vmalloc() to allocate the framebuffer, and that path works just fine.
> > The other 3 use alloc_pages(), dma_alloc_coherent(), or __get_free_pages(), all of
> > which manifest the underlying problem when munmap()'ed. Those 3 drivers are:
> >
> > * hyperv_fb.c, which I'm working with
> > * sh_mobile_lcdcfb.c
> > * ssd1307fb.c
>
> Nowadays sh_mobile_lcdcfb is used only on various SuperH boards
> (I have no hardware to test).
>
> sh_mobile_lcdcfb was used on ARM-based SH/R-Mobile SoCs until DT
> support was added to the DRM driver for the corresponding hardware.
> The platform using it was migrated to DRM in commit 138588e9fa237f97
> ("ARM: dts: renesas: r8a7740: Add LCDC nodes") in v6.8). At the time
> of the conversion, fbtest worked fine with sh_mobile_lcdcfb.
OK, good to know. sh_mobile_lcdcfb gets its framebuffer using
dma_alloc_coherent(). Do you recall how big the framebuffer was?
If over 4 MiB, dma_alloc_coherent() would have allocated from CMA,
and that works OK.
>
> Deferred I/O is also used in DRM drivers for displays that are connected
> using I2C or SPI. Last time I tried the st7735r driver, it worked fine
> with fbtest. That was also on arm32, though.
The st7735r driver appears to use fbtft-core.c, and that does a vmalloc()
for the screen buffer, so it won't have the problem I'm seeing.
Thanks for the help ....
Michael
^ permalink raw reply
* Re: [PATCH v3 06/11] backlight: Replace fb events with a dedicated function call
From: Thomas Zimmermann @ 2025-03-21 8:13 UTC (permalink / raw)
To: Daniel Thompson
Cc: lee, pavel, jingoohan1, deller, simona, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <Z9k7nAXNGDaQMnMO@aspen.lan>
Hi
Am 18.03.25 um 10:23 schrieb Daniel Thompson:
> On Thu, Mar 06, 2025 at 03:05:48PM +0100, Thomas Zimmermann wrote:
>> Remove support for fb events from backlight subsystem. Provide the
>> helper backlight_notify_blank_all() instead. Also export the existing
>> helper backlight_notify_blank() to update a single backlight device.
>>
>> In fbdev, call either helper to inform the backlight subsystem of
>> changes to a display's blank state. If the framebuffer device has a
>> specific backlight, only update this one; otherwise update all.
>>
>> v3:
>> - declare empty fb_bl_notify_blank() as static inline (kernel test robot)
> Looks like there are still configs where we get build failure.
>
>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Other than the build issues, generally this looks great. Just a couple
> of small issues below.
>
>
>> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
>> index 1c43f579396f..9dc93c5e480b 100644
>> --- a/drivers/video/backlight/backlight.c
>> +++ b/drivers/video/backlight/backlight.c
>> @@ -78,11 +77,8 @@ static const char *const backlight_scale_types[] = {
>> [BACKLIGHT_SCALE_NON_LINEAR] = "non-linear",
>> };
>>
>> -#if defined(CONFIG_FB_CORE) || (defined(CONFIG_FB_CORE_MODULE) && \
>> - defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
>> -static void backlight_notify_blank(struct backlight_device *bd,
>> - struct device *display_dev,
>> - bool fb_on, bool prev_fb_on)
>> +void backlight_notify_blank(struct backlight_device *bd, struct device *display_dev,
>> + bool fb_on, bool prev_fb_on)
>> {
>> guard(mutex)(&bd->ops_lock);
>>
>> @@ -103,68 +99,18 @@ static void backlight_notify_blank(struct backlight_device *bd,
>> }
>> }
>> }
>> +EXPORT_SYMBOL(backlight_notify_blank);
> Should this be EXPORT_SYMBOL_GPL()?
The other symbols in this source file are exported with EXPORT_SYMBOL().
Best regards
Thomas
>
>
>> -/*
>> - * fb_notifier_callback
>> - *
>> - * This callback gets called when something important happens inside a
>> - * framebuffer driver. The backlight core only cares about FB_BLANK_UNBLANK
>> - * which is reported to the driver using backlight_update_status()
>> - * as a state change.
>> - *
>> - * There may be several fbdev's connected to the backlight device,
>> - * in which case they are kept track of. A state change is only reported
>> - * if there is a change in backlight for the specified fbdev.
>> - */
>> -static int fb_notifier_callback(struct notifier_block *self,
>> - unsigned long event, void *data)
>> +void backlight_notify_blank_all(struct device *display_dev, bool fb_on, bool prev_fb_on)
>> {
>> struct backlight_device *bd;
>> - struct fb_event *evdata = data;
>> - struct fb_info *info = evdata->info;
>> - const int *fb_blank = evdata->data;
>> - struct backlight_device *fb_bd = fb_bl_device(info);
>> - bool fb_on, prev_fb_on;
>> -
>> - /* If we aren't interested in this event, skip it immediately ... */
>> - if (event != FB_EVENT_BLANK)
>> - return 0;
>> -
>> - bd = container_of(self, struct backlight_device, fb_notif);
>> -
>> - if (fb_bd && fb_bd != bd)
>> - return 0;
>> -
>> - fb_on = fb_blank[0] == FB_BLANK_UNBLANK;
>> - prev_fb_on = fb_blank[1] == FB_BLANK_UNBLANK;
>> -
>> - backlight_notify_blank(bd, info->device, fb_on, prev_fb_on);
>> -
>> - return 0;
>> -}
>> -
>> -static int backlight_register_fb(struct backlight_device *bd)
>> -{
>> - memset(&bd->fb_notif, 0, sizeof(bd->fb_notif));
>> - bd->fb_notif.notifier_call = fb_notifier_callback;
>>
>> - return fb_register_client(&bd->fb_notif);
>> -}
>> + guard(mutex)(&backlight_dev_list_mutex);
>>
>> -static void backlight_unregister_fb(struct backlight_device *bd)
>> -{
>> - fb_unregister_client(&bd->fb_notif);
>> -}
>> -#else
>> -static inline int backlight_register_fb(struct backlight_device *bd)
>> -{
>> - return 0;
>> + list_for_each_entry(bd, &backlight_dev_list, entry)
>> + backlight_notify_blank(bd, display_dev, fb_on, prev_fb_on);
>> }
>> -
>> -static inline void backlight_unregister_fb(struct backlight_device *bd)
>> -{
>> -}
>> -#endif /* CONFIG_FB_CORE */
>> +EXPORT_SYMBOL(backlight_notify_blank_all);
> Same here.
>
>
> Daniel.
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* Re: [PATCH v3 08/11] backlight: lcd: Replace fb events with a dedicated function call
From: Thomas Zimmermann @ 2025-03-21 8:16 UTC (permalink / raw)
To: Daniel Thompson
Cc: lee, pavel, jingoohan1, deller, simona, linux-leds, dri-devel,
linux-fbdev
In-Reply-To: <Z9k_qy-Kh3-v5tKg@aspen.lan>
Hi
Am 18.03.25 um 10:40 schrieb Daniel Thompson:
> On Thu, Mar 06, 2025 at 03:05:50PM +0100, Thomas Zimmermann wrote:
>> Remove support for fb events from the lcd subsystem. Provide the
>> helper lcd_notify_blank_all() instead. In fbdev, call
>> lcd_notify_blank_all() to inform the lcd subsystem of changes
>> to a display's blank state.
>>
>> Fbdev maintains a list of all installed notifiers. Instead of fbdev
>> notifiers, maintain an internal list of lcd devices.
> I don't love the LCD devices list, however I can see the list of notifiers
> had the same semantic effect (only less explicit) so I can live with
> it ;-).
>
>> v3:
>> - export lcd_notify_mode_change_all() (kernel test robot)
>> v2:
>> - maintain global list of lcd devices
>> - avoid IS_REACHABLE() in source file
>> - use lock guards
>> - initialize lcd list and list mutex
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Happy with these changes, but have the same EXPORT_SYMBOL_GPL()
> questions I did with the backlight code.
As in backlight.c, all existing symbols are exported with
EXPORT_SYMBOL(). I simply used what was already there.
Best regards
Thomas
>
>
> Daniel.
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* [PATCH v4 00/11] backlight, lcd, led: Remove fbdev dependencies
From: Thomas Zimmermann @ 2025-03-21 9:53 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann
This series removes the remaining dependencies on fbdev from the
backlight, lcd and led subsystems. Each depends on fbdev events to
track display state. Make fbdev inform each subsystem via a dedicated
interface instead.
Patches 1 to 3 make fbdev track blank state for each display, so that
backlight code doesn't have to.
Patches 4 to 6 remove fbdev event handling from backlight code. Patches
7 and 8 remove fbdev event handling from lcd code and patches 9 and 10
do the same for led's backlight trigger.
The final patch removes the event constants from fbdev.
With the series applied, the three subsystems do no longer depend on
fbdev. It's also a clean up for fbdev. Fbdev used to send out a large
number of events. That mechanism has been deprecated for some time and
converted call to dedicated functions instead.
Testing is very welcome, as I don't have the hardware to test this
series.
v4:
- protect backlight declarations with IS_REACHABLE()
v3:
- export several symbols
- static-inline declare empty placeholders
v2:
- avoid IS_REACHABLE() in source file (Lee)
- simplify several interfaces and helpers
- use lock guards
- initialize global lists and mutices
Thomas Zimmermann (11):
fbdev: Rework fb_blank()
fbdev: Track display blanking state
fbdev: Send old blank state in FB_EVENT_BLANK
backlight: Implement fbdev tracking with blank state from event
backlight: Move blank-state handling into helper
backlight: Replace fb events with a dedicated function call
backlight: lcd: Move event handling into helpers
backlight: lcd: Replace fb events with a dedicated function call
leds: backlight trigger: Move blank-state handling into helper
leds: backlight trigger: Replace fb events with a dedicated function
call
fbdev: Remove constants of unused events
drivers/leds/trigger/ledtrig-backlight.c | 48 +++++-----
drivers/video/backlight/backlight.c | 93 +++++--------------
drivers/video/backlight/lcd.c | 108 +++++++++--------------
drivers/video/fbdev/core/fb_backlight.c | 12 +++
drivers/video/fbdev/core/fb_info.c | 1 +
drivers/video/fbdev/core/fbmem.c | 82 ++++++++++++++---
drivers/video/fbdev/core/fbsysfs.c | 8 +-
include/linux/backlight.h | 32 +++----
include/linux/fb.h | 12 +--
include/linux/lcd.h | 21 ++++-
include/linux/leds.h | 6 ++
11 files changed, 215 insertions(+), 208 deletions(-)
--
2.48.1
^ permalink raw reply
* [PATCH v4 02/11] fbdev: Track display blanking state
From: Thomas Zimmermann @ 2025-03-21 9:53 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann,
Simona Vetter
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
Store the display's blank status in struct fb_info.blank and track
it in fb_blank(). As an extra, the status is now available from the
sysfs blank attribute.
Support for blanking is optional. Therefore framebuffer_alloc()
initializes the state to FB_BLANK_UNBLANK (i.e., the display is
on). If the fb_blank callback has been set, register_framebuffer()
sets the state to FB_BLANK_POWERDOWN. On the first modeset, the
call to fb_blank() will update it to _UNBLANK. This is important,
as listeners to FB_EVENT_BLANK will now see the display being
switched on.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Simona Vetter <simona.vetter@ffwll.ch>
---
drivers/video/fbdev/core/fb_info.c | 1 +
drivers/video/fbdev/core/fbmem.c | 17 ++++++++++++++++-
drivers/video/fbdev/core/fbsysfs.c | 8 ++++----
include/linux/fb.h | 2 ++
4 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/core/fb_info.c b/drivers/video/fbdev/core/fb_info.c
index 4847ebe50d7d..52f9bd2c5417 100644
--- a/drivers/video/fbdev/core/fb_info.c
+++ b/drivers/video/fbdev/core/fb_info.c
@@ -42,6 +42,7 @@ struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
info->device = dev;
info->fbcon_rotate_hint = -1;
+ info->blank = FB_BLANK_UNBLANK;
#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
mutex_init(&info->bl_curve_mutex);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 39e2b81473ad..5d1529d300b7 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -341,6 +341,7 @@ EXPORT_SYMBOL(fb_set_var);
int fb_blank(struct fb_info *info, int blank)
{
+ int old_blank = info->blank;
struct fb_event event;
int ret;
@@ -353,13 +354,19 @@ int fb_blank(struct fb_info *info, int blank)
event.info = info;
event.data = ␣
+ info->blank = blank;
+
ret = info->fbops->fb_blank(blank, info);
if (ret)
- return ret;
+ goto err;
fb_notifier_call_chain(FB_EVENT_BLANK, &event);
return 0;
+
+err:
+ info->blank = old_blank;
+ return ret;
}
EXPORT_SYMBOL(fb_blank);
@@ -408,6 +415,14 @@ static int do_register_framebuffer(struct fb_info *fb_info)
mutex_init(&fb_info->lock);
mutex_init(&fb_info->mm_lock);
+ /*
+ * With an fb_blank callback present, we assume that the
+ * display is blank, so that fb_blank() enables it on the
+ * first modeset.
+ */
+ if (fb_info->fbops->fb_blank)
+ fb_info->blank = FB_BLANK_POWERDOWN;
+
fb_device_create(fb_info);
if (fb_info->pixmap.addr == NULL) {
diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
index 1b3c9958ef5c..e337660bce46 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -242,11 +242,11 @@ static ssize_t store_blank(struct device *device,
return count;
}
-static ssize_t show_blank(struct device *device,
- struct device_attribute *attr, char *buf)
+static ssize_t show_blank(struct device *device, struct device_attribute *attr, char *buf)
{
-// struct fb_info *fb_info = dev_get_drvdata(device);
- return 0;
+ struct fb_info *fb_info = dev_get_drvdata(device);
+
+ return sysfs_emit(buf, "%d\n", fb_info->blank);
}
static ssize_t store_console(struct device *device,
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 5ba187e08cf7..f41d3334ac23 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -471,6 +471,8 @@ struct fb_info {
struct list_head modelist; /* mode list */
struct fb_videomode *mode; /* current mode */
+ int blank; /* current blanking; see FB_BLANK_ constants */
+
#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
/* assigned backlight device */
/* set before framebuffer registration,
--
2.48.1
^ permalink raw reply related
* [PATCH v4 01/11] fbdev: Rework fb_blank()
From: Thomas Zimmermann @ 2025-03-21 9:53 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann,
Simona Vetter
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
Reimplement fb_blank() to return early on errors. No functional
changes. Prepares the helper for tracking the blanking state in
struct fb_info.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Simona Vetter <simona.vetter@ffwll.ch>
---
drivers/video/fbdev/core/fbmem.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 3c568cff2913..39e2b81473ad 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -339,11 +339,13 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
}
EXPORT_SYMBOL(fb_set_var);
-int
-fb_blank(struct fb_info *info, int blank)
+int fb_blank(struct fb_info *info, int blank)
{
struct fb_event event;
- int ret = -EINVAL;
+ int ret;
+
+ if (!info->fbops->fb_blank)
+ return -EINVAL;
if (blank > FB_BLANK_POWERDOWN)
blank = FB_BLANK_POWERDOWN;
@@ -351,13 +353,13 @@ fb_blank(struct fb_info *info, int blank)
event.info = info;
event.data = ␣
- if (info->fbops->fb_blank)
- ret = info->fbops->fb_blank(blank, info);
+ ret = info->fbops->fb_blank(blank, info);
+ if (ret)
+ return ret;
- if (!ret)
- fb_notifier_call_chain(FB_EVENT_BLANK, &event);
+ fb_notifier_call_chain(FB_EVENT_BLANK, &event);
- return ret;
+ return 0;
}
EXPORT_SYMBOL(fb_blank);
--
2.48.1
^ permalink raw reply related
* [PATCH v4 03/11] fbdev: Send old blank state in FB_EVENT_BLANK
From: Thomas Zimmermann @ 2025-03-21 9:53 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann,
Simona Vetter
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
The event FB_EVENT_BLANK sends the new blank state in the event's
data field. Also send the old state. It's an additional field in the
data array; existing receivers won't notice the difference.
The backlight subsystem currently tracks blank state per display per
backlight. That is not optimal as it ties backlight code to fbdev. A
subsystem should not track internal state of another subsystem. With
both, new and old, blank state in FB_EVENT_BLANK, the backlight code
will not require its own state tracker any longer.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Simona Vetter <simona.vetter@ffwll.ch>
---
drivers/video/fbdev/core/fbmem.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 5d1529d300b7..9650b641d8e8 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -343,6 +343,7 @@ int fb_blank(struct fb_info *info, int blank)
{
int old_blank = info->blank;
struct fb_event event;
+ int data[2];
int ret;
if (!info->fbops->fb_blank)
@@ -351,8 +352,10 @@ int fb_blank(struct fb_info *info, int blank)
if (blank > FB_BLANK_POWERDOWN)
blank = FB_BLANK_POWERDOWN;
+ data[0] = blank;
+ data[1] = old_blank;
event.info = info;
- event.data = ␣
+ event.data = data;
info->blank = blank;
--
2.48.1
^ permalink raw reply related
* [PATCH v4 04/11] backlight: Implement fbdev tracking with blank state from event
From: Thomas Zimmermann @ 2025-03-21 9:53 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann,
Simona Vetter
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
Look at the blank state provided by FB_EVENT_BLANK to determine
whether to enable or disable a backlight. Remove the tracking fields
from struct backlight_device.
Tracking requires three variables, fb_on, prev_fb_on and the
backlight's use_count. If fb_on is true, the display has been
unblanked. The backlight needs to be enabled if the display was
blanked before (i.e., prev_fb_on is false) or if use_count is still
at 0. If fb_on is false, the display has been blanked. In this case,
the backlight has to be disabled was unblanked before and the
backlight's use_count is greater than 0.
This change removes fbdev state tracking from blacklight. All the
backlight requires it its own use counter and information about
changes to the display. Removing fbdev internals makes backlight
drivers easier to integrate into other display drivers, such as DRM.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Acked-by: Simona Vetter <simona.vetter@ffwll.ch>
---
drivers/video/backlight/backlight.c | 14 +++++++-------
include/linux/backlight.h | 10 +---------
2 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index f699e5827ccb..bb01f57c4683 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -98,9 +98,9 @@ static int fb_notifier_callback(struct notifier_block *self,
struct backlight_device *bd;
struct fb_event *evdata = data;
struct fb_info *info = evdata->info;
+ const int *fb_blank = evdata->data;
struct backlight_device *fb_bd = fb_bl_device(info);
- int node = info->node;
- int fb_blank = 0;
+ bool fb_on, prev_fb_on;
/* If we aren't interested in this event, skip it immediately ... */
if (event != FB_EVENT_BLANK)
@@ -116,15 +116,15 @@ static int fb_notifier_callback(struct notifier_block *self,
if (fb_bd && fb_bd != bd)
goto out;
- fb_blank = *(int *)evdata->data;
- if (fb_blank == FB_BLANK_UNBLANK && !bd->fb_bl_on[node]) {
- bd->fb_bl_on[node] = true;
+ fb_on = fb_blank[0] == FB_BLANK_UNBLANK;
+ prev_fb_on = fb_blank[1] == FB_BLANK_UNBLANK;
+
+ if (fb_on && (!prev_fb_on || !bd->use_count)) {
if (!bd->use_count++) {
bd->props.state &= ~BL_CORE_FBBLANK;
backlight_update_status(bd);
}
- } else if (fb_blank != FB_BLANK_UNBLANK && bd->fb_bl_on[node]) {
- bd->fb_bl_on[node] = false;
+ } else if (!fb_on && prev_fb_on && bd->use_count) {
if (!(--bd->use_count)) {
bd->props.state |= BL_CORE_FBBLANK;
backlight_update_status(bd);
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index f5652e5a9060..03723a5478f8 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -294,15 +294,7 @@ struct backlight_device {
struct device dev;
/**
- * @fb_bl_on: The state of individual fbdev's.
- *
- * Multiple fbdev's may share one backlight device. The fb_bl_on
- * records the state of the individual fbdev.
- */
- bool fb_bl_on[FB_MAX];
-
- /**
- * @use_count: The number of uses of fb_bl_on.
+ * @use_count: The number of unblanked displays.
*/
int use_count;
};
--
2.48.1
^ permalink raw reply related
* [PATCH v4 09/11] leds: backlight trigger: Move blank-state handling into helper
From: Thomas Zimmermann @ 2025-03-21 9:54 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann,
Simona Vetter
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
Move the handling of blank-state updates into a separate helper,
so that is can be called without the fbdev event. No functional
changes.
v2:
- rename helper to avoid renaming in a later patch (Lee)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Simona Vetter <simona.vetter@ffwll.ch>
---
drivers/leds/trigger/ledtrig-backlight.c | 30 ++++++++++++++----------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
index 487577d22cfc..8e66d55a6c82 100644
--- a/drivers/leds/trigger/ledtrig-backlight.c
+++ b/drivers/leds/trigger/ledtrig-backlight.c
@@ -25,12 +25,28 @@ struct bl_trig_notifier {
unsigned invert;
};
+static void ledtrig_backlight_notify_blank(struct bl_trig_notifier *n, int new_status)
+{
+ struct led_classdev *led = n->led;
+
+ if (new_status == n->old_status)
+ return;
+
+ if ((n->old_status == UNBLANK) ^ n->invert) {
+ n->brightness = led->brightness;
+ led_set_brightness_nosleep(led, LED_OFF);
+ } else {
+ led_set_brightness_nosleep(led, n->brightness);
+ }
+
+ n->old_status = new_status;
+}
+
static int fb_notifier_callback(struct notifier_block *p,
unsigned long event, void *data)
{
struct bl_trig_notifier *n = container_of(p,
struct bl_trig_notifier, notifier);
- struct led_classdev *led = n->led;
struct fb_event *fb_event = data;
int *blank;
int new_status;
@@ -42,17 +58,7 @@ static int fb_notifier_callback(struct notifier_block *p,
blank = fb_event->data;
new_status = *blank ? BLANK : UNBLANK;
- if (new_status == n->old_status)
- return 0;
-
- if ((n->old_status == UNBLANK) ^ n->invert) {
- n->brightness = led->brightness;
- led_set_brightness_nosleep(led, LED_OFF);
- } else {
- led_set_brightness_nosleep(led, n->brightness);
- }
-
- n->old_status = new_status;
+ ledtrig_backlight_notify_blank(n, new_status);
return 0;
}
--
2.48.1
^ permalink raw reply related
* [PATCH v4 05/11] backlight: Move blank-state handling into helper
From: Thomas Zimmermann @ 2025-03-21 9:53 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann,
Simona Vetter
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
Move the handling of blank-state updates into a separate helper,
so that is can be called without the fbdev event. No functional
changes.
As a minor improvement over the original code, the update replaces
manual locking with a guard.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Acked-by: Simona Vetter <simona.vetter@ffwll.ch>
---
drivers/video/backlight/backlight.c | 46 +++++++++++++++++------------
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index bb01f57c4683..1c43f579396f 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -80,6 +80,30 @@ static const char *const backlight_scale_types[] = {
#if defined(CONFIG_FB_CORE) || (defined(CONFIG_FB_CORE_MODULE) && \
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
+static void backlight_notify_blank(struct backlight_device *bd,
+ struct device *display_dev,
+ bool fb_on, bool prev_fb_on)
+{
+ guard(mutex)(&bd->ops_lock);
+
+ if (!bd->ops)
+ return;
+ if (bd->ops->controls_device && !bd->ops->controls_device(bd, display_dev))
+ return;
+
+ if (fb_on && (!prev_fb_on || !bd->use_count)) {
+ if (!bd->use_count++) {
+ bd->props.state &= ~BL_CORE_FBBLANK;
+ backlight_update_status(bd);
+ }
+ } else if (!fb_on && prev_fb_on && bd->use_count) {
+ if (!(--bd->use_count)) {
+ bd->props.state |= BL_CORE_FBBLANK;
+ backlight_update_status(bd);
+ }
+ }
+}
+
/*
* fb_notifier_callback
*
@@ -107,31 +131,15 @@ static int fb_notifier_callback(struct notifier_block *self,
return 0;
bd = container_of(self, struct backlight_device, fb_notif);
- mutex_lock(&bd->ops_lock);
- if (!bd->ops)
- goto out;
- if (bd->ops->controls_device && !bd->ops->controls_device(bd, info->device))
- goto out;
if (fb_bd && fb_bd != bd)
- goto out;
+ return 0;
fb_on = fb_blank[0] == FB_BLANK_UNBLANK;
prev_fb_on = fb_blank[1] == FB_BLANK_UNBLANK;
- if (fb_on && (!prev_fb_on || !bd->use_count)) {
- if (!bd->use_count++) {
- bd->props.state &= ~BL_CORE_FBBLANK;
- backlight_update_status(bd);
- }
- } else if (!fb_on && prev_fb_on && bd->use_count) {
- if (!(--bd->use_count)) {
- bd->props.state |= BL_CORE_FBBLANK;
- backlight_update_status(bd);
- }
- }
-out:
- mutex_unlock(&bd->ops_lock);
+ backlight_notify_blank(bd, info->device, fb_on, prev_fb_on);
+
return 0;
}
--
2.48.1
^ permalink raw reply related
* [PATCH v4 11/11] fbdev: Remove constants of unused events
From: Thomas Zimmermann @ 2025-03-21 9:54 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann,
Simona Vetter
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
The constants FB_EVENT_MODE_CHANGE and FB_EVENT_BLANK are unused.
Remove them from the header file.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Simona Vetter <simona.vetter@ffwll.ch>
---
include/linux/fb.h | 6 ------
1 file changed, 6 deletions(-)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index d45bd220cb8f..2497321e30bb 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -129,18 +129,12 @@ struct fb_cursor_user {
* Register/unregister for framebuffer events
*/
-/* The resolution of the passed in fb_info about to change */
-#define FB_EVENT_MODE_CHANGE 0x01
-
#ifdef CONFIG_GUMSTIX_AM200EPD
/* only used by mach-pxa/am200epd.c */
#define FB_EVENT_FB_REGISTERED 0x05
#define FB_EVENT_FB_UNREGISTERED 0x06
#endif
-/* A display blank is requested */
-#define FB_EVENT_BLANK 0x09
-
struct fb_event {
struct fb_info *info;
void *data;
--
2.48.1
^ permalink raw reply related
* [PATCH v4 06/11] backlight: Replace fb events with a dedicated function call
From: Thomas Zimmermann @ 2025-03-21 9:53 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann,
Simona Vetter
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
Remove support for fb events from backlight subsystem. Provide the
helper backlight_notify_blank_all() instead. Also export the existing
helper backlight_notify_blank() to update a single backlight device.
In fbdev, call either helper to inform the backlight subsystem of
changes to a display's blank state. If the framebuffer device has a
specific backlight, only update this one; otherwise update all.
v4:
- protect blacklight declarations with IS_REACHABLE() (kernel test robot)
v3:
- declare empty fb_bl_notify_blank() as static inline (kernel test robot)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Simona Vetter <simona.vetter@ffwll.ch>
---
drivers/video/backlight/backlight.c | 85 ++++---------------------
drivers/video/fbdev/core/fb_backlight.c | 12 ++++
drivers/video/fbdev/core/fbmem.c | 2 +
include/linux/backlight.h | 22 +++++--
include/linux/fb.h | 4 ++
5 files changed, 46 insertions(+), 79 deletions(-)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 1c43f579396f..9dc93c5e480b 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -15,7 +15,6 @@
#include <linux/notifier.h>
#include <linux/ctype.h>
#include <linux/err.h>
-#include <linux/fb.h>
#include <linux/slab.h>
#ifdef CONFIG_PMAC_BACKLIGHT
@@ -57,10 +56,10 @@
* a hot-key to adjust backlight, the driver must notify the backlight
* core that brightness has changed using backlight_force_update().
*
- * The backlight driver core receives notifications from fbdev and
- * if the event is FB_EVENT_BLANK and if the value of blank, from the
- * FBIOBLANK ioctrl, results in a change in the backlight state the
- * update_status() operation is called.
+ * Display drives can control the backlight device's status using
+ * backlight_notify_blank() and backlight_notify_blank_all(). If this
+ * results in a change in the backlight state the functions call the
+ * update_status() operation.
*/
static struct list_head backlight_dev_list;
@@ -78,11 +77,8 @@ static const char *const backlight_scale_types[] = {
[BACKLIGHT_SCALE_NON_LINEAR] = "non-linear",
};
-#if defined(CONFIG_FB_CORE) || (defined(CONFIG_FB_CORE_MODULE) && \
- defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
-static void backlight_notify_blank(struct backlight_device *bd,
- struct device *display_dev,
- bool fb_on, bool prev_fb_on)
+void backlight_notify_blank(struct backlight_device *bd, struct device *display_dev,
+ bool fb_on, bool prev_fb_on)
{
guard(mutex)(&bd->ops_lock);
@@ -103,68 +99,18 @@ static void backlight_notify_blank(struct backlight_device *bd,
}
}
}
+EXPORT_SYMBOL(backlight_notify_blank);
-/*
- * fb_notifier_callback
- *
- * This callback gets called when something important happens inside a
- * framebuffer driver. The backlight core only cares about FB_BLANK_UNBLANK
- * which is reported to the driver using backlight_update_status()
- * as a state change.
- *
- * There may be several fbdev's connected to the backlight device,
- * in which case they are kept track of. A state change is only reported
- * if there is a change in backlight for the specified fbdev.
- */
-static int fb_notifier_callback(struct notifier_block *self,
- unsigned long event, void *data)
+void backlight_notify_blank_all(struct device *display_dev, bool fb_on, bool prev_fb_on)
{
struct backlight_device *bd;
- struct fb_event *evdata = data;
- struct fb_info *info = evdata->info;
- const int *fb_blank = evdata->data;
- struct backlight_device *fb_bd = fb_bl_device(info);
- bool fb_on, prev_fb_on;
-
- /* If we aren't interested in this event, skip it immediately ... */
- if (event != FB_EVENT_BLANK)
- return 0;
-
- bd = container_of(self, struct backlight_device, fb_notif);
-
- if (fb_bd && fb_bd != bd)
- return 0;
-
- fb_on = fb_blank[0] == FB_BLANK_UNBLANK;
- prev_fb_on = fb_blank[1] == FB_BLANK_UNBLANK;
-
- backlight_notify_blank(bd, info->device, fb_on, prev_fb_on);
-
- return 0;
-}
-
-static int backlight_register_fb(struct backlight_device *bd)
-{
- memset(&bd->fb_notif, 0, sizeof(bd->fb_notif));
- bd->fb_notif.notifier_call = fb_notifier_callback;
- return fb_register_client(&bd->fb_notif);
-}
+ guard(mutex)(&backlight_dev_list_mutex);
-static void backlight_unregister_fb(struct backlight_device *bd)
-{
- fb_unregister_client(&bd->fb_notif);
-}
-#else
-static inline int backlight_register_fb(struct backlight_device *bd)
-{
- return 0;
+ list_for_each_entry(bd, &backlight_dev_list, entry)
+ backlight_notify_blank(bd, display_dev, fb_on, prev_fb_on);
}
-
-static inline void backlight_unregister_fb(struct backlight_device *bd)
-{
-}
-#endif /* CONFIG_FB_CORE */
+EXPORT_SYMBOL(backlight_notify_blank_all);
static void backlight_generate_event(struct backlight_device *bd,
enum backlight_update_reason reason)
@@ -455,12 +401,6 @@ struct backlight_device *backlight_device_register(const char *name,
return ERR_PTR(rc);
}
- rc = backlight_register_fb(new_bd);
- if (rc) {
- device_unregister(&new_bd->dev);
- return ERR_PTR(rc);
- }
-
new_bd->ops = ops;
#ifdef CONFIG_PMAC_BACKLIGHT
@@ -547,7 +487,6 @@ void backlight_device_unregister(struct backlight_device *bd)
bd->ops = NULL;
mutex_unlock(&bd->ops_lock);
- backlight_unregister_fb(bd);
device_unregister(&bd->dev);
}
EXPORT_SYMBOL(backlight_device_unregister);
diff --git a/drivers/video/fbdev/core/fb_backlight.c b/drivers/video/fbdev/core/fb_backlight.c
index 6fdaa9f81be9..dbed9696f4c5 100644
--- a/drivers/video/fbdev/core/fb_backlight.c
+++ b/drivers/video/fbdev/core/fb_backlight.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#include <linux/backlight.h>
#include <linux/export.h>
#include <linux/fb.h>
#include <linux/mutex.h>
@@ -36,4 +37,15 @@ struct backlight_device *fb_bl_device(struct fb_info *info)
return info->bl_dev;
}
EXPORT_SYMBOL(fb_bl_device);
+
+void fb_bl_notify_blank(struct fb_info *info, int old_blank)
+{
+ bool on = info->blank == FB_BLANK_UNBLANK;
+ bool prev_on = old_blank == FB_BLANK_UNBLANK;
+
+ if (info->bl_dev)
+ backlight_notify_blank(info->bl_dev, info->device, on, prev_on);
+ else
+ backlight_notify_blank_all(info->device, on, prev_on);
+}
#endif
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 9650b641d8e8..c931f270ac34 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -363,6 +363,8 @@ int fb_blank(struct fb_info *info, int blank)
if (ret)
goto err;
+ fb_bl_notify_blank(info, old_blank);
+
fb_notifier_call_chain(FB_EVENT_BLANK, &event);
return 0;
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 03723a5478f8..10e626db7eee 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -12,7 +12,6 @@
#include <linux/device.h>
#include <linux/fb.h>
#include <linux/mutex.h>
-#include <linux/notifier.h>
#include <linux/types.h>
/**
@@ -278,11 +277,6 @@ struct backlight_device {
*/
const struct backlight_ops *ops;
- /**
- * @fb_notif: The framebuffer notifier block
- */
- struct notifier_block fb_notif;
-
/**
* @entry: List entry of all registered backlight devices
*/
@@ -400,6 +394,22 @@ struct backlight_device *backlight_device_get_by_type(enum backlight_type type);
int backlight_device_set_brightness(struct backlight_device *bd,
unsigned long brightness);
+#if IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
+void backlight_notify_blank(struct backlight_device *bd,
+ struct device *display_dev,
+ bool fb_on, bool prev_fb_on);
+void backlight_notify_blank_all(struct device *display_dev,
+ bool fb_on, bool prev_fb_on);
+#else
+static inline void backlight_notify_blank(struct backlight_device *bd,
+ struct device *display_dev,
+ bool fb_on, bool prev_fb_on)
+{ }
+static inline void backlight_notify_blank_all(struct device *display_dev,
+ bool fb_on, bool prev_fb_on)
+{ }
+#endif
+
#define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
/**
diff --git a/include/linux/fb.h b/include/linux/fb.h
index f41d3334ac23..d45bd220cb8f 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -757,11 +757,15 @@ extern void fb_bl_default_curve(struct fb_info *fb_info, u8 off, u8 min, u8 max)
#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
struct backlight_device *fb_bl_device(struct fb_info *info);
+void fb_bl_notify_blank(struct fb_info *info, int old_blank);
#else
static inline struct backlight_device *fb_bl_device(struct fb_info *info)
{
return NULL;
}
+
+static inline void fb_bl_notify_blank(struct fb_info *info, int old_blank)
+{ }
#endif
static inline struct lcd_device *fb_lcd_device(struct fb_info *info)
--
2.48.1
^ permalink raw reply related
* [PATCH v4 07/11] backlight: lcd: Move event handling into helpers
From: Thomas Zimmermann @ 2025-03-21 9:54 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann,
Simona Vetter
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
Move the handling of display updates to separate helper functions.
There is code for handling fbdev blank events and fbdev mode changes.
The code currently runs from fbdev event notifiers, which will be
replaced.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Acked-by: Simona Vetter <simona.vetter@ffwll.ch>
---
drivers/video/backlight/lcd.c | 38 ++++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index 3267acf8dc5b..f57ff8bcc2fa 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -18,6 +18,32 @@
#include <linux/fb.h>
#include <linux/slab.h>
+static void lcd_notify_blank(struct lcd_device *ld, struct device *display_dev,
+ int power)
+{
+ guard(mutex)(&ld->ops_lock);
+
+ if (!ld->ops || !ld->ops->set_power)
+ return;
+ if (ld->ops->controls_device && !ld->ops->controls_device(ld, display_dev))
+ return;
+
+ ld->ops->set_power(ld, power);
+}
+
+static void lcd_notify_mode_change(struct lcd_device *ld, struct device *display_dev,
+ unsigned int width, unsigned int height)
+{
+ guard(mutex)(&ld->ops_lock);
+
+ if (!ld->ops || !ld->ops->set_mode)
+ return;
+ if (ld->ops->controls_device && !ld->ops->controls_device(ld, display_dev))
+ return;
+
+ ld->ops->set_mode(ld, width, height);
+}
+
#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
defined(CONFIG_LCD_CLASS_DEVICE_MODULE))
static int to_lcd_power(int fb_blank)
@@ -50,25 +76,17 @@ static int fb_notifier_callback(struct notifier_block *self,
struct fb_info *info = evdata->info;
struct lcd_device *fb_lcd = fb_lcd_device(info);
- guard(mutex)(&ld->ops_lock);
-
- if (!ld->ops)
- return 0;
- if (ld->ops->controls_device && !ld->ops->controls_device(ld, info->device))
- return 0;
if (fb_lcd && fb_lcd != ld)
return 0;
if (event == FB_EVENT_BLANK) {
int power = to_lcd_power(*(int *)evdata->data);
- if (ld->ops->set_power)
- ld->ops->set_power(ld, power);
+ lcd_notify_blank(ld, info->device, power);
} else {
const struct fb_videomode *videomode = evdata->data;
- if (ld->ops->set_mode)
- ld->ops->set_mode(ld, videomode->xres, videomode->yres);
+ lcd_notify_mode_change(ld, info->device, videomode->xres, videomode->yres);
}
return 0;
--
2.48.1
^ permalink raw reply related
* [PATCH v4 08/11] backlight: lcd: Replace fb events with a dedicated function call
From: Thomas Zimmermann @ 2025-03-21 9:54 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann,
Simona Vetter
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
Remove support for fb events from the lcd subsystem. Provide the
helper lcd_notify_blank_all() instead. In fbdev, call
lcd_notify_blank_all() to inform the lcd subsystem of changes
to a display's blank state.
Fbdev maintains a list of all installed notifiers. Instead of fbdev
notifiers, maintain an internal list of lcd devices.
v3:
- export lcd_notify_mode_change_all() (kernel test robot)
v2:
- maintain global list of lcd devices
- avoid IS_REACHABLE() in source file
- use lock guards
- initialize lcd list and list mutex
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Simona Vetter <simona.vetter@ffwll.ch>
---
drivers/video/backlight/lcd.c | 98 +++++++++-----------------------
drivers/video/fbdev/core/fbmem.c | 39 +++++++++++--
include/linux/lcd.h | 21 ++++++-
3 files changed, 79 insertions(+), 79 deletions(-)
diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index f57ff8bcc2fa..affe5c52471a 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -15,9 +15,11 @@
#include <linux/notifier.h>
#include <linux/ctype.h>
#include <linux/err.h>
-#include <linux/fb.h>
#include <linux/slab.h>
+static DEFINE_MUTEX(lcd_dev_list_mutex);
+static LIST_HEAD(lcd_dev_list);
+
static void lcd_notify_blank(struct lcd_device *ld, struct device *display_dev,
int power)
{
@@ -31,6 +33,17 @@ static void lcd_notify_blank(struct lcd_device *ld, struct device *display_dev,
ld->ops->set_power(ld, power);
}
+void lcd_notify_blank_all(struct device *display_dev, int power)
+{
+ struct lcd_device *ld;
+
+ guard(mutex)(&lcd_dev_list_mutex);
+
+ list_for_each_entry(ld, &lcd_dev_list, entry)
+ lcd_notify_blank(ld, display_dev, power);
+}
+EXPORT_SYMBOL(lcd_notify_blank_all);
+
static void lcd_notify_mode_change(struct lcd_device *ld, struct device *display_dev,
unsigned int width, unsigned int height)
{
@@ -44,75 +57,17 @@ static void lcd_notify_mode_change(struct lcd_device *ld, struct device *display
ld->ops->set_mode(ld, width, height);
}
-#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
- defined(CONFIG_LCD_CLASS_DEVICE_MODULE))
-static int to_lcd_power(int fb_blank)
+void lcd_notify_mode_change_all(struct device *display_dev,
+ unsigned int width, unsigned int height)
{
- switch (fb_blank) {
- case FB_BLANK_UNBLANK:
- return LCD_POWER_ON;
- /* deprecated; TODO: should become 'off' */
- case FB_BLANK_NORMAL:
- return LCD_POWER_REDUCED;
- case FB_BLANK_VSYNC_SUSPEND:
- return LCD_POWER_REDUCED_VSYNC_SUSPEND;
- /* 'off' */
- case FB_BLANK_HSYNC_SUSPEND:
- case FB_BLANK_POWERDOWN:
- default:
- return LCD_POWER_OFF;
- }
-}
+ struct lcd_device *ld;
-/* This callback gets called when something important happens inside a
- * framebuffer driver. We're looking if that important event is blanking,
- * and if it is, we're switching lcd power as well ...
- */
-static int fb_notifier_callback(struct notifier_block *self,
- unsigned long event, void *data)
-{
- struct lcd_device *ld = container_of(self, struct lcd_device, fb_notif);
- struct fb_event *evdata = data;
- struct fb_info *info = evdata->info;
- struct lcd_device *fb_lcd = fb_lcd_device(info);
-
- if (fb_lcd && fb_lcd != ld)
- return 0;
-
- if (event == FB_EVENT_BLANK) {
- int power = to_lcd_power(*(int *)evdata->data);
-
- lcd_notify_blank(ld, info->device, power);
- } else {
- const struct fb_videomode *videomode = evdata->data;
-
- lcd_notify_mode_change(ld, info->device, videomode->xres, videomode->yres);
- }
+ guard(mutex)(&lcd_dev_list_mutex);
- return 0;
+ list_for_each_entry(ld, &lcd_dev_list, entry)
+ lcd_notify_mode_change(ld, display_dev, width, height);
}
-
-static int lcd_register_fb(struct lcd_device *ld)
-{
- memset(&ld->fb_notif, 0, sizeof(ld->fb_notif));
- ld->fb_notif.notifier_call = fb_notifier_callback;
- return fb_register_client(&ld->fb_notif);
-}
-
-static void lcd_unregister_fb(struct lcd_device *ld)
-{
- fb_unregister_client(&ld->fb_notif);
-}
-#else
-static int lcd_register_fb(struct lcd_device *ld)
-{
- return 0;
-}
-
-static inline void lcd_unregister_fb(struct lcd_device *ld)
-{
-}
-#endif /* CONFIG_FB */
+EXPORT_SYMBOL(lcd_notify_mode_change_all);
static ssize_t lcd_power_show(struct device *dev, struct device_attribute *attr,
char *buf)
@@ -263,11 +218,8 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
return ERR_PTR(rc);
}
- rc = lcd_register_fb(new_ld);
- if (rc) {
- device_unregister(&new_ld->dev);
- return ERR_PTR(rc);
- }
+ guard(mutex)(&lcd_dev_list_mutex);
+ list_add(&new_ld->entry, &lcd_dev_list);
return new_ld;
}
@@ -284,10 +236,12 @@ void lcd_device_unregister(struct lcd_device *ld)
if (!ld)
return;
+ guard(mutex)(&lcd_dev_list_mutex);
+ list_del(&ld->entry);
+
mutex_lock(&ld->ops_lock);
ld->ops = NULL;
mutex_unlock(&ld->ops_lock);
- lcd_unregister_fb(ld);
device_unregister(&ld->dev);
}
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index c931f270ac34..001662c606d7 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -15,6 +15,7 @@
#include <linux/export.h>
#include <linux/fb.h>
#include <linux/fbcon.h>
+#include <linux/lcd.h>
#include <video/nomodeset.h>
@@ -220,6 +221,12 @@ static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
return err;
}
+static void fb_lcd_notify_mode_change(struct fb_info *info,
+ struct fb_videomode *mode)
+{
+ lcd_notify_mode_change_all(info->device, mode->xres, mode->yres);
+}
+
int
fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
{
@@ -227,7 +234,6 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
u32 activate;
struct fb_var_screeninfo old_var;
struct fb_videomode mode;
- struct fb_event event;
u32 unused;
if (var->activate & FB_ACTIVATE_INV_MODE) {
@@ -331,14 +337,38 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
if (ret)
return ret;
- event.info = info;
- event.data = &mode;
- fb_notifier_call_chain(FB_EVENT_MODE_CHANGE, &event);
+ fb_lcd_notify_mode_change(info, &mode);
return 0;
}
EXPORT_SYMBOL(fb_set_var);
+static void fb_lcd_notify_blank(struct fb_info *info)
+{
+ int power;
+
+ switch (info->blank) {
+ case FB_BLANK_UNBLANK:
+ power = LCD_POWER_ON;
+ break;
+ /* deprecated; TODO: should become 'off' */
+ case FB_BLANK_NORMAL:
+ power = LCD_POWER_REDUCED;
+ break;
+ case FB_BLANK_VSYNC_SUSPEND:
+ power = LCD_POWER_REDUCED_VSYNC_SUSPEND;
+ break;
+ /* 'off' */
+ case FB_BLANK_HSYNC_SUSPEND:
+ case FB_BLANK_POWERDOWN:
+ default:
+ power = LCD_POWER_OFF;
+ break;
+ }
+
+ lcd_notify_blank_all(info->device, power);
+}
+
int fb_blank(struct fb_info *info, int blank)
{
int old_blank = info->blank;
@@ -364,6 +394,7 @@ int fb_blank(struct fb_info *info, int blank)
goto err;
fb_bl_notify_blank(info, old_blank);
+ fb_lcd_notify_blank(info);
fb_notifier_call_chain(FB_EVENT_BLANK, &event);
diff --git a/include/linux/lcd.h b/include/linux/lcd.h
index c3ccdff4519a..d4fa03722b72 100644
--- a/include/linux/lcd.h
+++ b/include/linux/lcd.h
@@ -11,7 +11,6 @@
#include <linux/device.h>
#include <linux/mutex.h>
-#include <linux/notifier.h>
#define LCD_POWER_ON (0)
#define LCD_POWER_REDUCED (1) // deprecated; don't use in new code
@@ -79,8 +78,11 @@ struct lcd_device {
const struct lcd_ops *ops;
/* Serialise access to set_power method */
struct mutex update_lock;
- /* The framebuffer notifier block */
- struct notifier_block fb_notif;
+
+ /**
+ * @entry: List entry of all registered lcd devices
+ */
+ struct list_head entry;
struct device dev;
};
@@ -125,6 +127,19 @@ extern void lcd_device_unregister(struct lcd_device *ld);
extern void devm_lcd_device_unregister(struct device *dev,
struct lcd_device *ld);
+#if IS_REACHABLE(CONFIG_LCD_CLASS_DEVICE)
+void lcd_notify_blank_all(struct device *display_dev, int power);
+void lcd_notify_mode_change_all(struct device *display_dev,
+ unsigned int width, unsigned int height);
+#else
+static inline void lcd_notify_blank_all(struct device *display_dev, int power)
+{}
+
+static inline void lcd_notify_mode_change_all(struct device *display_dev,
+ unsigned int width, unsigned int height)
+{}
+#endif
+
#define to_lcd_device(obj) container_of(obj, struct lcd_device, dev)
static inline void * lcd_get_data(struct lcd_device *ld_dev)
--
2.48.1
^ permalink raw reply related
* [PATCH v4 10/11] leds: backlight trigger: Replace fb events with a dedicated function call
From: Thomas Zimmermann @ 2025-03-21 9:54 UTC (permalink / raw)
To: lee, pavel, danielt, jingoohan1, deller, simona
Cc: linux-leds, dri-devel, linux-fbdev, Thomas Zimmermann,
Simona Vetter
In-Reply-To: <20250321095517.313713-1-tzimmermann@suse.de>
Remove support for fb events from the led backlight trigger. Provide
the helper ledtrig_backlight_blank() instead. Call it from fbdev to
inform the trigger of changes to a display's blank state.
Fbdev maintains a list of all installed notifiers. Instead of the fbdev
notifiers, maintain an internal list of led backlight triggers.
v3:
- export ledtrig_backlight_blank()
v2:
- maintain global list of led backlight triggers (Lee)
- avoid IS_REACHABLE() in source file (Lee)
- notify on changes to blank state instead of display state
- use lock guards
- initialize led list and list mutex
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Simona Vetter <simona.vetter@ffwll.ch>
---
drivers/leds/trigger/ledtrig-backlight.c | 42 ++++++++++--------------
drivers/video/fbdev/core/fbmem.c | 19 ++++++-----
include/linux/leds.h | 6 ++++
3 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c
index 8e66d55a6c82..c1f0f5becaee 100644
--- a/drivers/leds/trigger/ledtrig-backlight.c
+++ b/drivers/leds/trigger/ledtrig-backlight.c
@@ -10,7 +10,6 @@
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/init.h>
-#include <linux/fb.h>
#include <linux/leds.h>
#include "../leds.h"
@@ -21,10 +20,14 @@ struct bl_trig_notifier {
struct led_classdev *led;
int brightness;
int old_status;
- struct notifier_block notifier;
unsigned invert;
+
+ struct list_head entry;
};
+static DEFINE_MUTEX(ledtrig_backlight_list_mutex);
+static LIST_HEAD(ledtrig_backlight_list);
+
static void ledtrig_backlight_notify_blank(struct bl_trig_notifier *n, int new_status)
{
struct led_classdev *led = n->led;
@@ -42,26 +45,17 @@ static void ledtrig_backlight_notify_blank(struct bl_trig_notifier *n, int new_s
n->old_status = new_status;
}
-static int fb_notifier_callback(struct notifier_block *p,
- unsigned long event, void *data)
+void ledtrig_backlight_blank(bool blank)
{
- struct bl_trig_notifier *n = container_of(p,
- struct bl_trig_notifier, notifier);
- struct fb_event *fb_event = data;
- int *blank;
- int new_status;
-
- /* If we aren't interested in this event, skip it immediately ... */
- if (event != FB_EVENT_BLANK)
- return 0;
-
- blank = fb_event->data;
- new_status = *blank ? BLANK : UNBLANK;
+ struct bl_trig_notifier *n;
+ int new_status = blank ? BLANK : UNBLANK;
- ledtrig_backlight_notify_blank(n, new_status);
+ guard(mutex)(&ledtrig_backlight_list_mutex);
- return 0;
+ list_for_each_entry(n, &ledtrig_backlight_list, entry)
+ ledtrig_backlight_notify_blank(n, new_status);
}
+EXPORT_SYMBOL(ledtrig_backlight_blank);
static ssize_t bl_trig_invert_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -106,8 +100,6 @@ ATTRIBUTE_GROUPS(bl_trig);
static int bl_trig_activate(struct led_classdev *led)
{
- int ret;
-
struct bl_trig_notifier *n;
n = kzalloc(sizeof(struct bl_trig_notifier), GFP_KERNEL);
@@ -118,11 +110,9 @@ static int bl_trig_activate(struct led_classdev *led)
n->led = led;
n->brightness = led->brightness;
n->old_status = UNBLANK;
- n->notifier.notifier_call = fb_notifier_callback;
- ret = fb_register_client(&n->notifier);
- if (ret)
- dev_err(led->dev, "unable to register backlight trigger\n");
+ guard(mutex)(&ledtrig_backlight_list_mutex);
+ list_add(&n->entry, &ledtrig_backlight_list);
return 0;
}
@@ -131,7 +121,9 @@ static void bl_trig_deactivate(struct led_classdev *led)
{
struct bl_trig_notifier *n = led_get_trigger_data(led);
- fb_unregister_client(&n->notifier);
+ guard(mutex)(&ledtrig_backlight_list_mutex);
+ list_del(&n->entry);
+
kfree(n);
}
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 001662c606d7..f089f72ec75a 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -16,6 +16,7 @@
#include <linux/fb.h>
#include <linux/fbcon.h>
#include <linux/lcd.h>
+#include <linux/leds.h>
#include <video/nomodeset.h>
@@ -369,11 +370,17 @@ static void fb_lcd_notify_blank(struct fb_info *info)
lcd_notify_blank_all(info->device, power);
}
+static void fb_ledtrig_backlight_notify_blank(struct fb_info *info)
+{
+ if (info->blank == FB_BLANK_UNBLANK)
+ ledtrig_backlight_blank(false);
+ else
+ ledtrig_backlight_blank(true);
+}
+
int fb_blank(struct fb_info *info, int blank)
{
int old_blank = info->blank;
- struct fb_event event;
- int data[2];
int ret;
if (!info->fbops->fb_blank)
@@ -382,11 +389,6 @@ int fb_blank(struct fb_info *info, int blank)
if (blank > FB_BLANK_POWERDOWN)
blank = FB_BLANK_POWERDOWN;
- data[0] = blank;
- data[1] = old_blank;
- event.info = info;
- event.data = data;
-
info->blank = blank;
ret = info->fbops->fb_blank(blank, info);
@@ -395,8 +397,7 @@ int fb_blank(struct fb_info *info, int blank)
fb_bl_notify_blank(info, old_blank);
fb_lcd_notify_blank(info);
-
- fb_notifier_call_chain(FB_EVENT_BLANK, &event);
+ fb_ledtrig_backlight_notify_blank(info);
return 0;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 98f9719c924c..b3f0aa081064 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -640,6 +640,12 @@ static inline void ledtrig_flash_ctrl(bool on) {}
static inline void ledtrig_torch_ctrl(bool on) {}
#endif
+#if IS_REACHABLE(CONFIG_LEDS_TRIGGER_BACKLIGHT)
+void ledtrig_backlight_blank(bool blank);
+#else
+static inline void ledtrig_backlight_blank(bool blank) {}
+#endif
+
/*
* Generic LED platform data for describing LED names and default triggers.
*/
--
2.48.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox