* Re: [PATCH] fbtft: Improve damage_range to mark only changed rows
From: Dan Carpenter @ 2026-01-28 13:28 UTC (permalink / raw)
To: ChanSoo Shin; +Cc: andy, gregkh, dri-devel, linux-fbdev, linux-staging
In-Reply-To: <20260128130503.868466-1-csshin9928@gmail.com>
On Wed, Jan 28, 2026 at 10:05:03PM +0900, ChanSoo Shin wrote:
> Instead of marking the entire display as dirty, calculate
> start_row and end_row based on off/len and mark only those rows.
> This improves performance for partial framebuffer updates.
>
> Signed-off-by: ChanSoo Shin <csshin9928@gmail.com>
> ---
This is v3 but it's not marked or described.
Have you tested this patch? What was the speed up?
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] fbtft: Improve damage_range to mark only changed rows
From: Dan Carpenter @ 2026-01-28 13:26 UTC (permalink / raw)
To: Waffle0823; +Cc: andy, gregkh, dri-devel, linux-fbdev, linux-staging
In-Reply-To: <20260128085720.862399-1-csshin9928@gmail.com>
On Wed, Jan 28, 2026 at 05:57:20PM +0900, Waffle0823 wrote:
> Instead of marking the entire display as dirty, calculate
> start_row and end_row based on off/len and mark only those rows.
> This improves performance for partial framebuffer updates.
>
> Signed-off-by: Waffle0283 csshin9928@gmail.com
Please use your real name that you use to sign legal documents. The
email address is in the wrong format.
Have you tested this patch? What was the speedup?
regards,
dan carpenter
^ permalink raw reply
* [PATCH] fbtft: Improve damage_range to mark only changed rows
From: ChanSoo Shin @ 2026-01-28 13:05 UTC (permalink / raw)
To: andy; +Cc: gregkh, dri-devel, linux-fbdev, linux-staging, ChanSoo Shin
Instead of marking the entire display as dirty, calculate
start_row and end_row based on off/len and mark only those rows.
This improves performance for partial framebuffer updates.
Signed-off-by: ChanSoo Shin <csshin9928@gmail.com>
---
drivers/staging/fbtft/fbtft-core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 8a5ccc8ae0a1..0fbdfdaaa94d 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -415,8 +415,11 @@ static void fbtft_ops_damage_range(struct fb_info *info, off_t off, size_t len)
{
struct fbtft_par *par = info->par;
- /* TODO: only mark changed area update all for now */
- par->fbtftops.mkdirty(info, -1, 0);
+ __u32 width = info->var.xres;
+ __u32 start_row = off / width;
+ __u32 end_row = (off + len - 1) / width;
+
+ par->fbtftops.mkdirty(info, start_row, end_row);
}
static void fbtft_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height)
--
2.52.0
^ permalink raw reply related
* Re: [PATCH RFC v6 00/26] nova-core: Memory management infrastructure (v6)
From: Joel Fernandes @ 2026-01-28 12:44 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Joel Fernandes, linux-kernel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
Vivi Rodrigo, Tvrtko Ursulin, Rui Huang, Matthew Auld,
Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
Alice Ryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Bjorn Roy Baron, Benno Lossin, Andreas Hindborg, Trevor Gross,
John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
Alexandre Courbot, Andrea Righi, Andy Ritger, Zhi Wang,
Alexey Ivanov, Balbir Singh, Philipp Stanner, Elle Rhumsaa,
Daniel Almeida, nouveau, dri-devel, rust-for-linux, linux-doc,
amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DG06XUWOJLO5.1ESB8ES6A6081@kernel.org>
On Jan 28, 2026, at 6:38 AM, Danilo Krummrich <dakr@kernel.org> wrote:
> On Tue Jan 20, 2026 at 9:42 PM CET, Joel Fernandes wrote:
>> This series is rebased on drm-rust-kernel/drm-rust-next and provides memory
>> management infrastructure for the nova-core GPU driver. It combines several
>> previous series and provides a foundation for nova GPU memory management
>> including page tables, virtual memory management, and BAR mapping. All these
>> are critical nova-core features.
>
> Thanks for this work, I will go through the series soon. (Although it would also
> be nice to have what I mention below addressed first.)
Thanks, I appreciate that.
> I'm not overly happy with this version history. I understand that you are
> building things on top of each other, but going back and forth with adding and
> removing features from a series is confusing and makes it hard to keep track of
> things.
>
> (In the worst case it may even result in reviewers skipping over it leaving you
> with no progress eventually.)
>
> [...]
>
> Hence, please separate the features from each other in separate patch series,
> with their own proper version history and changelog. In order to account for the
> dependencies, you can just mention them in the cover letter and add a link to
> the other related patch series, which should be sufficient for people interested
> in the full picture.
>
> I think the most clean approach would probably be a split with CList, DRM buddy
> and Nova MM stuff.
>
> And just to clarify, in the end I do not care too much about whether it's all in
> a single series or split up, but going back and forth with combining things that
> once have been separate and have a separate history doesn't work out well.
I understand the concern, and I appreciate you taking the time to explain. Let
me provide some context on how we ended up here, as it may help clarify the
situation.
1. This is a multi-month undertaking with many interdependencies. It is
difficult to predict which patches will come to exist, the optimal order, how to split, which series
first, or what pieces are missing. This is similar to the evolution of nova
itself - complex interdependencies make it hard to predict what will be
needed. Rather than waiting months for a perfect plan before posting
anything, I chose to iterate publicly.
2. The decision to move GPU buddy out of DRM came later in the process [1].
This significantly changed the scope, requiring a much larger patch to
handle the buddy infrastructure that everything else depends on.
3. The decision to separate buddy from the CList series came from wanting to
make progress on CList independently [2]. That effort alone took almost a
month with several rewrites based on feedback from others.
4. There was some back and forth on whether to post code with users or code
that could potentially be used. This influenced the decision to combine
things into the same series to demonstrate working functionality.
5. The memory management code only became functional around v3. Page table
walking turned out to be tricky, and I did not have a proper user at that
time. Eventually I realized BAR1 is a strong use case for page table
translation, so I added support for that.
Regarding splitting the series: that makes sense, I will split into CList, GPU
buddy, and Nova MM as you suggest. You make a fair point about the versioning
too - labeling new patches (even though most are old) as v6 is confusing. One question: what version
numbers should each split series use? CList was at v3 before being combined,
and similar story for GPU buddy and Nova MM. Should I continue from the last
version number they were posted with, or continue from v6?
[1] https://lore.kernel.org/all/20251124234432.1988476-1-joelagnelf@nvidia.com/
[2] https://lore.kernel.org/all/20251129213056.4021375-1-joelagnelf@nvidia.com/
--
Joel Fernandes
^ permalink raw reply
* Re: [PATCH RFC v6 05/26] nova-core: mm: Add support to use PRAMIN windows to write to VRAM
From: Danilo Krummrich @ 2026-01-28 12:04 UTC (permalink / raw)
To: Joel Fernandes
Cc: Zhi Wang, linux-kernel, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
Alice Ryhl, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Bjorn Roy Baron, Benno Lossin, Andreas Hindborg, Trevor Gross,
John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
Alexandre Courbot, Andrea Righi, Andy Ritger, Alexey Ivanov,
Balbir Singh, Philipp Stanner, Elle Rhumsaa, Daniel Almeida,
nouveau, dri-devel, rust-for-linux, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <DS0PR12MB6486717785F6DD14EE1F1C46A397A@DS0PR12MB6486.namprd12.prod.outlook.com>
On Fri Jan 23, 2026 at 12:16 AM CET, Joel Fernandes wrote:
> My plan is to make TLB and PRAMIN use immutable references in their function
> calls and then implement internal locking. I've already done this for the GPU
> buddy functions, so it should be doable, and we'll keep it consistent. As a
> result, we will have finer-grain locking on the memory management objects
> instead of requiring to globally lock a common GpuMm object. I'll plan on
> doing this for v7.
>
> Also, the PTE allocation race you mentioned is already handled by PRAMIN
> serialization. Since threads must hold the PRAMIN lock to write page table
> entries, concurrent writers are not possible:
>
> Thread A: acquire PRAMIN lock
> Thread A: read PDE (via PRAMIN) -> NULL
> Thread A: alloc PT page, write PDE
> Thread A: release PRAMIN lock
>
> Thread B: acquire PRAMIN lock
> Thread B: read PDE (via PRAMIN) -> sees A's pointer
> Thread B: uses existing PT page, no allocation needed
This won't work unfortunately.
We have to separate allocations and modifications of the page tabe. Or in other
words, we must not allocate new PDEs or PTEs while holding the lock protecting
the page table from modifications.
Once we have VM_BIND in nova-drm, we will have the situation that userspace
passes jobs to modify the GPUs virtual address space and hence the page tables.
Such a jobs has mainly three stages.
(1) The submit stage.
This is where the job is initialized, dependencies are set up and the
driver has to pre-allocate all kinds of structures that are required
throughout the subsequent stages of the job.
(2) The run stage.
This is the stage where the job is staged for execution and its DMA fence
has been made public (i.e. it is accessible by userspace).
This is the stage where we are in the DMA fence signalling critical
section, hence we can't do any non-atomic allocations, since otherwise we
could deadlock in MMU notifier callbacks for instance.
This is the stage where the page table is actually modified. Hence, we
can't acquire any locks that might be held elsewhere while doing
non-atomic allocations. Also note that this is transitive, e.g. if you
take lock A and somewhere else a lock B is taked while A is already held
and we do non-atomic allocations while holding B, then A can't be held in
the DMA fence signalling critical path either.
It is also worth noting that this is the stage where we know the exact
operations we have to execute based on the VM_BIND request from userspace.
For instance, in the submit stage we may only know that userspace wants
that we map a BO with a certain offset in the GPUs virtual address space
at [0x0, 0x1000000]. What we don't know is what exact operations this does
require, i.e. "What do we have to unmap first?", "Are there any
overlapping mappings that we have to truncate?", etc.
So, we have to consider this when we pre-allocate in the submit stage.
(3) The cleanup stage.
This is where the job has been signaled and hence left the DMA fence
signalling critical section.
In this stage the job is cleaned up, which includes freeing data that is
not required anymore, such as PTEs and PDEs.
^ permalink raw reply
* Re: [PATCH v2] staging: sm750fb: replace magic number with jiffies timeout
From: Dan Carpenter @ 2026-01-28 11:55 UTC (permalink / raw)
To: Madhumitha Sundar
Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260127154056.74855-1-madhuananda18@gmail.com>
On Tue, Jan 27, 2026 at 03:40:56PM +0000, Madhumitha Sundar wrote:
> The hardware wait loop in hw_sm750_de_wait used a hardcoded loop
> counter (0x10000000), which depends on CPU speed and is unreliable.
>
> Replace the loop counter with a jiffies-based timeout (1 second)
> using time_before(). This ensures consistent delays across architectures.
>
> Signed-off-by: Madhumitha Sundar <madhuananda18@gmail.com>
> ---
This feels like an AI patch? AI patches need to be disclosed.
Anyway, we couldn't merge something like this without testing.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] staging: sm750fb: replace magic number with defined constant
From: Dan Carpenter @ 2026-01-28 11:46 UTC (permalink / raw)
To: Madhumitha Sundar
Cc: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, linux-staging,
linux-kernel
In-Reply-To: <20260127132758.49650-1-madhuananda18@gmail.com>
On Tue, Jan 27, 2026 at 01:27:58PM +0000, Madhumitha Sundar wrote:
> The hardware wait loop in hw_sm750_de_wait uses a hardcoded magic
> number (0x10000000) for the timeout counter.
>
> Define a constant SM750_MAX_LOOP in sm750.h and use it to improve
> code readability and maintainability.
Timeout counters have no special meaning. They aren't intended to be
re-used in multiple places.
There is a kind of bug where people think we exit with the counter set
to zero but actually we exit with it set the -1. Normally, when I see
this sort of bug, I change the timer from cnt-- to --cnt which changes
loop from iterating 100 times to iterating 99 times. It's fine. No
one cares if about the exact number, just the approximate range.
The original was more clear.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH RFC v6 00/26] nova-core: Memory management infrastructure (v6)
From: Danilo Krummrich @ 2026-01-28 11:37 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Jonathan Corbet, Alex Deucher,
Christian König, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Huang Rui, Matthew Auld, Matthew Brost,
Lucas De Marchi, Thomas Hellström, Helge Deller, Alice Ryhl,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, John Hubbard, Alistair Popple, Timur Tabi,
Edwin Peer, Alexandre Courbot, Andrea Righi, Andy Ritger,
Zhi Wang, Alexey Ivanov, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, Daniel Almeida, joel, nouveau, dri-devel,
rust-for-linux, linux-doc, amd-gfx, intel-gfx, intel-xe,
linux-fbdev
In-Reply-To: <20260120204303.3229303-1-joelagnelf@nvidia.com>
On Tue Jan 20, 2026 at 9:42 PM CET, Joel Fernandes wrote:
> This series is rebased on drm-rust-kernel/drm-rust-next and provides memory
> management infrastructure for the nova-core GPU driver. It combines several
> previous series and provides a foundation for nova GPU memory management
> including page tables, virtual memory management, and BAR mapping. All these
> are critical nova-core features.
Thanks for this work, I will go through the series soon. (Although it would also
be nice to have what I mention below addressed first.)
> The series includes:
> - A Rust module (CList) to interface with C circular linked lists, required
> for iterating over buddy allocator blocks.
> - Movement of the DRM buddy allocator up to drivers/gpu/ level, renamed to GPU buddy.
> - Rust bindings for the GPU buddy allocator.
> - PRAMIN aperture support for direct VRAM access.
> - Page table types for MMU v2 and v3 formats.
> - Virtual Memory Manager (VMM) for GPU virtual address space management.
> - BAR1 user interface for mapping access GPU via virtual memory.
> - Selftests for PRAMIN and BAR1 user interface (disabled by default).
>
> Changes from v5 to v6:
> - Rebased on drm-rust-kernel/drm-rust-next
> - Added page table types and page table walker infrastructure
> - Added Virtual Memory Manager (VMM)
> - Added BAR1 user interface
> - Added TLB flush support
> - Added GpuMm memory manager
> - Extended to 26 patches from 6 (full mm infrastructure now included)
>
> The git tree with all patches can be found at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git (tag: nova-mm-v6-20260120)
>
> Link to v5: https://lore.kernel.org/all/20251219203805.1246586-1-joelagnelf@nvidia.com/
>
> Previous series that are combined:
> - v4 (clist + buddy): https://lore.kernel.org/all/20251204215129.2357292-1-joelagnelf@nvidia.com/
> - v3 (clist only): https://lore.kernel.org/all/20251129213056.4021375-1-joelagnelf@nvidia.com/
> - v2 (clist only): https://lore.kernel.org/all/20251111171315.2196103-4-joelagnelf@nvidia.com/
> - clist RFC (original with buddy): https://lore.kernel.org/all/20251030190613.1224287-1-joelagnelf@nvidia.com/
> - DRM buddy move: https://lore.kernel.org/all/20251124234432.1988476-1-joelagnelf@nvidia.com/
> - PRAMIN series: https://lore.kernel.org/all/20251020185539.49986-1-joelagnelf@nvidia.com/
I'm not overly happy with this version history. I understand that you are
building things on top of each other, but going back and forth with adding and
removing features from a series is confusing and makes it hard to keep track of
things.
(In the worst case it may even result in reviewers skipping over it leaving you
with no progress eventually.)
I.e. you stared with a CList and DRM buddy RFC, then DRM buddy disappeared for a
few versions and came back eventually. Then, in the next version, the PRAMIN
stuff came back in, which also had a predecessor series already and now you
added lots of MM stuff on top of it.
The whole version history is about what features and patches were added and
removed to/from the series, rather than about what actually changed design wise
and code wise between the iterations (which is the important part for reviewers
and maintainers).
I also think it is confusing that a lot of the patches in this series have never
been posted before, yet they are labeled as v6 of this RFC.
Hence, please separate the features from each other in separate patch series,
with their own proper version history and changelog. In order to account for the
dependencies, you can just mention them in the cover letter and add a link to
the other related patch series, which should be sufficient for people interested
in the full picture.
I think the most clean approach would probably be a split with CList, DRM buddy
and Nova MM stuff.
And just to clarify, in the end I do not care too much about whether it's all in
a single series or split up, but going back and forth with combining things that
once have been separate and have a separate history doesn't work out well.
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
From: Daniel Thompson @ 2026-01-28 11:20 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: tessolveupstream, lee, danielt, jingoohan1, deller, pavel, robh,
krzk+dt, conor+dt, dri-devel, linux-fbdev, linux-leds, devicetree,
linux-kernel
In-Reply-To: <500b603d-5abc-4c45-8d56-bbc88fc85b83@kernel.org>
On Wed, Jan 28, 2026 at 11:11:33AM +0100, Krzysztof Kozlowski wrote:
> On 23/01/2026 12:11, tessolveupstream@gmail.com wrote:
> >
> >
> > On 20-01-2026 20:01, Krzysztof Kozlowski wrote:
> >> On 20/01/2026 13:50, Sudarshan Shetty wrote:
> >>> Update the gpio-backlight binding to support configurations that require
> >>> more than one GPIO for enabling/disabling the backlight.
> >>
> >>
> >> Why? Which devices need it? How a backlight would have three enable
> >> GPIOs? I really do not believe, so you need to write proper hardware
> >> justification.
> >>
> >
> > To clarify our hardware setup:
> > the panel requires one GPIO for the backlight enable signal, and it
> > also has a PWM input. Since the QCS615 does not provide a PWM controller
> > for this use case, the PWM input is connected to a GPIO that is driven
> > high to provide a constant 100% duty cycle, as explained in the link
> > below.
> > https://lore.kernel.org/all/20251028061636.724667-1-tessolveupstream@gmail.com/T/#m93ca4e5c7bf055715ed13316d91f0cd544244cf5
>
> That's not an enable gpio, but PWM.
>
> You write bindings for this device, not for something else - like your
> board.
Sudarshan: I believe at one point the intent was to model this hardware
as a pwm-backlight (using enables GPIOs to drive the enable pin)
attached to a pwm-gpio (to drive the PWM pin). Did this approach work?
Daniel.
^ permalink raw reply
* Re: [PATCH v2 2/2] backlight: gpio: add support for multiple GPIOs for backlight control
From: Daniel Thompson @ 2026-01-28 10:57 UTC (permalink / raw)
To: Sudarshan Shetty
Cc: lee, danielt, jingoohan1, deller, pavel, robh, krzk+dt, conor+dt,
dri-devel, linux-fbdev, linux-leds, devicetree, linux-kernel
In-Reply-To: <20260120125036.2203995-3-tessolveupstream@gmail.com>
On Tue, Jan 20, 2026 at 06:20:36PM +0530, Sudarshan Shetty wrote:
> The gpio-backlight driver currently supports only a single GPIO to
> enable or disable a backlight device. Some panels require multiple
> enable GPIOs to be asserted together.
>
> Extend the driver to support an array of GPIOs for a single backlight
> instance. All configured GPIOs are toggled together based on the
> backlight state.
>
> Existing single-GPIO Device Tree users remain unaffected.
>
> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
> ---
> drivers/video/backlight/gpio_backlight.c | 66 ++++++++++++++++--------
> 1 file changed, 45 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
> index 728a546904b0..11d21de82cf5 100644
> --- a/drivers/video/backlight/gpio_backlight.c
> +++ b/drivers/video/backlight/gpio_backlight.c
> @@ -14,17 +14,29 @@
> #include <linux/platform_device.h>
> #include <linux/property.h>
> #include <linux/slab.h>
> +#include <linux/bitmap.h>
>
> struct gpio_backlight {
> struct device *dev;
> - struct gpio_desc *gpiod;
> + struct gpio_descs *gpiods;
> + unsigned long *bitmap;
> };
>
> static int gpio_backlight_update_status(struct backlight_device *bl)
> {
> struct gpio_backlight *gbl = bl_get_data(bl);
> + unsigned int n = gbl->gpiods->ndescs;
> + int br = backlight_get_brightness(bl);
>
> - gpiod_set_value_cansleep(gbl->gpiod, backlight_get_brightness(bl));
> + if (br)
> + bitmap_fill(gbl->bitmap, n);
> + else
> + bitmap_zero(gbl->bitmap, n);
> +
> + gpiod_set_array_value_cansleep(n,
> + gbl->gpiods->desc,
> + gbl->gpiods->info,
> + gbl->bitmap);
>
> return 0;
> }
> @@ -48,26 +60,43 @@ static int gpio_backlight_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev);
> struct device_node *of_node = dev->of_node;
> - struct backlight_properties props;
> + struct backlight_properties props = { };
This change is unrelated to the patch description. Do not "hide"
changes like this. It you want to replace the memset() it's better to
send a separate patch.
> struct backlight_device *bl;
> struct gpio_backlight *gbl;
> - int ret, init_brightness, def_value;
> + bool def_value;
> + enum gpiod_flags flags;
> + unsigned int n;
> + int words;
>
> - gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
> - if (gbl == NULL)
> + gbl = devm_kcalloc(dev, 1, sizeof(*gbl), GFP_KERNEL);
> + if (!gbl)
Again, this change is unrelated to the patch description. Do not include
changes that are not described in the patch description.
> return -ENOMEM;
>
> if (pdata)
> gbl->dev = pdata->dev;
>
> def_value = device_property_read_bool(dev, "default-on");
> + flags = def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
> +
> + gbl->gpiods = devm_gpiod_get_array(dev, NULL, flags);
How is it safe to transition from GPIOD_ASIS to GPIOD_OUT_LOW
here?
Forcing the backlight off if the default-on property is not present
will prevent the backlight state being properly inherited from the
bootloader.
> + if (IS_ERR(gbl->gpiods)) {
> + if (PTR_ERR(gbl->gpiods) == -ENODEV)
> + return dev_err_probe(dev, -EINVAL,
> + "The gpios parameter is missing or invalid\n");
> + return PTR_ERR(gbl->gpiods);
> + }
>
> - gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
> - if (IS_ERR(gbl->gpiod))
> - return dev_err_probe(dev, PTR_ERR(gbl->gpiod),
> - "The gpios parameter is missing or invalid\n");
> + n = gbl->gpiods->ndescs;
> + if (!n)
> + return dev_err_probe(dev, -EINVAL,
> + "No GPIOs provided\n");
> +
> + words = BITS_TO_LONGS(n);
> + gbl->bitmap = devm_kcalloc(dev, words, sizeof(unsigned long),
> + GFP_KERNEL);
> + if (!gbl->bitmap)
> + return -ENOMEM;
>
> - memset(&props, 0, sizeof(props));
> props.type = BACKLIGHT_RAW;
> props.max_brightness = 1;
> bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl,
> @@ -81,21 +110,16 @@ static int gpio_backlight_probe(struct platform_device *pdev)
> if (!of_node || !of_node->phandle)
> /* Not booted with device tree or no phandle link to the node */
> bl->props.power = def_value ? BACKLIGHT_POWER_ON
> - : BACKLIGHT_POWER_OFF;
> - else if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
> + : BACKLIGHT_POWER_OFF;
> + else if (gpiod_get_value_cansleep(gbl->gpiods->desc[0]) == 0)
This logic is broken. This code path needs to be taken is *any* GPIO is
low (and, as mentioned, the initial GPIO state must be GPIOD_ASIS).
> bl->props.power = BACKLIGHT_POWER_OFF;
> else
> bl->props.power = BACKLIGHT_POWER_ON;
>
> - bl->props.brightness = 1;
> -
> - init_brightness = backlight_get_brightness(bl);
> - ret = gpiod_direction_output(gbl->gpiod, init_brightness);
> - if (ret) {
> - dev_err(dev, "failed to set initial brightness\n");
> - return ret;
> - }
> + bl->props.brightness = def_value ? 1 : 0;
>
> + gpio_backlight_update_status(bl);
> +
> platform_set_drvdata(pdev, bl);
> return 0;
> }
Daniel.
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
From: Krzysztof Kozlowski @ 2026-01-28 10:14 UTC (permalink / raw)
To: tessolveupstream, lee, danielt, jingoohan1
Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev,
linux-leds, devicetree, linux-kernel
In-Reply-To: <5f78fbe8-288d-4b0a-af57-e834bd1186ba@gmail.com>
On 27/01/2026 13:46, tessolveupstream@gmail.com wrote:
>
>
> On 23-01-2026 16:41, tessolveupstream@gmail.com wrote:
>>
>>
>> On 20-01-2026 20:01, Krzysztof Kozlowski wrote:
>>> On 20/01/2026 13:50, Sudarshan Shetty wrote:
>>>> Update the gpio-backlight binding to support configurations that require
>>>> more than one GPIO for enabling/disabling the backlight.
>>>
>>>
>>> Why? Which devices need it? How a backlight would have three enable
>>> GPIOs? I really do not believe, so you need to write proper hardware
>>> justification.
>>>
>>
>> To clarify our hardware setup:
>> the panel requires one GPIO for the backlight enable signal, and it
>> also has a PWM input. Since the QCS615 does not provide a PWM controller
>> for this use case, the PWM input is connected to a GPIO that is driven
>> high to provide a constant 100% duty cycle, as explained in the link
>> below.
>> https://lore.kernel.org/all/20251028061636.724667-1-tessolveupstream@gmail.com/T/#m93ca4e5c7bf055715ed13316d91f0cd544244cf5
>>
>>>>
>>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
>>>> ---
>>>> .../leds/backlight/gpio-backlight.yaml | 24 +++++++++++++++++--
>>>> 1 file changed, 22 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>>> index 584030b6b0b9..4e4a856cbcd7 100644
>>>> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>>> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>>> @@ -16,8 +16,18 @@ properties:
>>>> const: gpio-backlight
>>>>
>>>> gpios:
>>>> - description: The gpio that is used for enabling/disabling the backlight.
>>>> - maxItems: 1
>>>> + description: |
>>>> + The gpio that is used for enabling/disabling the backlight.
>>>> + Multiple GPIOs can be specified for panels that require several
>>>> + enable signals. All GPIOs are controlled together.
>>>> + type: array
>>>
>>> There is no such syntax in the bindings, from where did you get it? Type
>>> is already defined.
>>>
>>> items:
>>> minItems: 1
>>> maxItems: 3
>>>
>>>
>>>> + minItems: 1
>>>> + items:
>>>> + type: array
>>>> + minItems: 3
>>>> + maxItems: 3
>>>> + items:
>>>> + type: integer
>>>
>>> All this is some odd stuff - just to be clear, don't send us LLM output.
>>> I don't want to waste my time to review microslop.
>>>
>>> Was it done with help of Microslop?
>>>
>>
>> I understand now that the schema changes I proposed were not correct,
>> and I will address this in the next patch series. My intention was to
>> check whether the gpio-backlight binding could support more than one
>> enable-type GPIO.
>> Could you please advise what would be an appropriate maximum number of
>> GPIOs for gpio-backlight in such a scenario? For example, would allowing
>> 2 GPIOs be acceptable, or should this case be handled in a different way?
>>
>
> In line with Daniel’s suggestion, I am planning to adopt a fixed upper
> limit for the number of backlight GPIOs. The current hardware only
> requires two GPIOs, so the maxItems can be set to 2.
>
> If future platforms or customers require support for a higher number
> of GPIOs, this limit can be increased and the driver can be
> updated accordingly.
>
> Kindly advise if this solution aligns with your expectations, or if
> you prefer an alternative maximum value.
You have entire commit msg to explain the hardware and explain WHY you
are doing this. In a concise and readable way. I will not be going
through 2 different email threads with 20 messages to figure that out.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: backlight: gpio-backlight: allow multiple GPIOs
From: Krzysztof Kozlowski @ 2026-01-28 10:11 UTC (permalink / raw)
To: tessolveupstream, lee, danielt, jingoohan1
Cc: deller, pavel, robh, krzk+dt, conor+dt, dri-devel, linux-fbdev,
linux-leds, devicetree, linux-kernel
In-Reply-To: <54d156ba-e177-4059-a808-2505983b4e2e@gmail.com>
On 23/01/2026 12:11, tessolveupstream@gmail.com wrote:
>
>
> On 20-01-2026 20:01, Krzysztof Kozlowski wrote:
>> On 20/01/2026 13:50, Sudarshan Shetty wrote:
>>> Update the gpio-backlight binding to support configurations that require
>>> more than one GPIO for enabling/disabling the backlight.
>>
>>
>> Why? Which devices need it? How a backlight would have three enable
>> GPIOs? I really do not believe, so you need to write proper hardware
>> justification.
>>
>
> To clarify our hardware setup:
> the panel requires one GPIO for the backlight enable signal, and it
> also has a PWM input. Since the QCS615 does not provide a PWM controller
> for this use case, the PWM input is connected to a GPIO that is driven
> high to provide a constant 100% duty cycle, as explained in the link
> below.
> https://lore.kernel.org/all/20251028061636.724667-1-tessolveupstream@gmail.com/T/#m93ca4e5c7bf055715ed13316d91f0cd544244cf5
That's not an enable gpio, but PWM.
You write bindings for this device, not for something else - like your
board.
>
>>>
>>> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
>>> ---
>>> .../leds/backlight/gpio-backlight.yaml | 24 +++++++++++++++++--
>>> 1 file changed, 22 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>> index 584030b6b0b9..4e4a856cbcd7 100644
>>> --- a/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>> +++ b/Documentation/devicetree/bindings/leds/backlight/gpio-backlight.yaml
>>> @@ -16,8 +16,18 @@ properties:
>>> const: gpio-backlight
>>>
>>> gpios:
>>> - description: The gpio that is used for enabling/disabling the backlight.
>>> - maxItems: 1
>>> + description: |
>>> + The gpio that is used for enabling/disabling the backlight.
>>> + Multiple GPIOs can be specified for panels that require several
>>> + enable signals. All GPIOs are controlled together.
>>> + type: array
>>
>> There is no such syntax in the bindings, from where did you get it? Type
>> is already defined.
>>
>> items:
>> minItems: 1
>> maxItems: 3
>>
>>
>>> + minItems: 1
>>> + items:
>>> + type: array
>>> + minItems: 3
>>> + maxItems: 3
>>> + items:
>>> + type: integer
>>
>> All this is some odd stuff - just to be clear, don't send us LLM output.
>> I don't want to waste my time to review microslop.
>>
>> Was it done with help of Microslop?
>>
>
> I understand now that the schema changes I proposed were not correct,
How such code could be even created... Just in case, do you understand
that Microslop and LLM is waste of our time?
> and I will address this in the next patch series. My intention was to
> check whether the gpio-backlight binding could support more than one
> enable-type GPIO.
> Could you please advise what would be an appropriate maximum number of
> GPIOs for gpio-backlight in such a scenario? For example, would allowing
> 2 GPIOs be acceptable, or should this case be handled in a different way?
We have plenty of examples for this, but anyway you won't need it
because this is not an enable GPIO.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] fbtft: Improve damage_range to mark only changed rows
From: Greg KH @ 2026-01-28 9:57 UTC (permalink / raw)
To: Waffle0823; +Cc: andy, dri-devel, linux-fbdev, linux-staging
In-Reply-To: <20260128092210.864021-1-csshin9928@gmail.com>
On Wed, Jan 28, 2026 at 06:22:10PM +0900, Waffle0823 wrote:
> Instead of marking the entire display as dirty, calculate
> start_row and end_row based on off/len and mark only those rows.
> This improves performance for partial framebuffer updates.
>
> Signed-off-by: Waffle0823 <csshin9928@gmail.com>
> ---
> drivers/staging/fbtft/fbtft-core.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
> index 8a5ccc8ae0a1..0fbdfdaaa94d 100644
> --- a/drivers/staging/fbtft/fbtft-core.c
> +++ b/drivers/staging/fbtft/fbtft-core.c
> @@ -415,8 +415,11 @@ static void fbtft_ops_damage_range(struct fb_info *info, off_t off, size_t len)
> {
> struct fbtft_par *par = info->par;
>
> - /* TODO: only mark changed area update all for now */
> - par->fbtftops.mkdirty(info, -1, 0);
> + __u32 width = info->var.xres;
> + __u32 start_row = off / width;
> + __u32 end_row = (off + len - 1) / width;
> +
> + par->fbtftops.mkdirty(info, start_row, end_row);
> }
>
> static void fbtft_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height)
> --
> 2.52.0
>
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- It looks like you did not use your "real" name for the patch on either
the Signed-off-by: line, or the From: line (both of which have to
match). Please read the kernel file,
Documentation/process/submitting-patches.rst for how to do this
correctly.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* Re: [PATCH] fbdev: au1100fb: Add missing check for clk_enable
From: Markus Elfring @ 2026-01-28 9:40 UTC (permalink / raw)
To: Chen Ni, linux-fbdev, dri-devel, Helge Deller,
Uwe Kleine-König; +Cc: LKML
In-Reply-To: <20260128091004.2747011-1-nichen@iscas.ac.cn>
> Add check for the return value of clk_enable() andreturn the error
and return?
> if it fails in order to catch the error.
How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.19-rc7#n145
Regards,
Markus
^ permalink raw reply
* [PATCH] fbtft: Improve damage_range to mark only changed rows
From: Waffle0823 @ 2026-01-28 9:22 UTC (permalink / raw)
To: andy; +Cc: gregkh, dri-devel, linux-fbdev, linux-staging, Waffle0823
Instead of marking the entire display as dirty, calculate
start_row and end_row based on off/len and mark only those rows.
This improves performance for partial framebuffer updates.
Signed-off-by: Waffle0823 <csshin9928@gmail.com>
---
drivers/staging/fbtft/fbtft-core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 8a5ccc8ae0a1..0fbdfdaaa94d 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -415,8 +415,11 @@ static void fbtft_ops_damage_range(struct fb_info *info, off_t off, size_t len)
{
struct fbtft_par *par = info->par;
- /* TODO: only mark changed area update all for now */
- par->fbtftops.mkdirty(info, -1, 0);
+ __u32 width = info->var.xres;
+ __u32 start_row = off / width;
+ __u32 end_row = (off + len - 1) / width;
+
+ par->fbtftops.mkdirty(info, start_row, end_row);
}
static void fbtft_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height)
--
2.52.0
^ permalink raw reply related
* [PATCH] fbdev: au1100fb: Add missing check for clk_enable
From: Chen Ni @ 2026-01-28 9:10 UTC (permalink / raw)
To: deller, u.kleine-koenig, elfring
Cc: linux-fbdev, dri-devel, linux-kernel, Chen Ni
Add check for the return value of clk_enable() andreturn the error
if it fails in order to catch the error.
Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
---
drivers/video/fbdev/au1100fb.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
index 6251a6b07b3a..5e7a8f018b7b 100644
--- a/drivers/video/fbdev/au1100fb.c
+++ b/drivers/video/fbdev/au1100fb.c
@@ -567,13 +567,16 @@ int au1100fb_drv_suspend(struct platform_device *dev, pm_message_t state)
int au1100fb_drv_resume(struct platform_device *dev)
{
struct au1100fb_device *fbdev = platform_get_drvdata(dev);
+ int ret;
if (!fbdev)
return 0;
memcpy(fbdev->regs, &fbregs, sizeof(struct au1100fb_regs));
- clk_enable(fbdev->lcdclk);
+ ret = clk_enable(fbdev->lcdclk);
+ if (ret)
+ return ret;
/* Unblank the LCD */
au1100fb_fb_blank(VESA_NO_BLANKING, &fbdev->info);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] fbtft: Improve damage_range to mark only changed rows
From: Greg KH @ 2026-01-28 9:03 UTC (permalink / raw)
To: Waffle0823; +Cc: andy, dri-devel, linux-fbdev, linux-staging
In-Reply-To: <20260128085720.862399-1-csshin9928@gmail.com>
On Wed, Jan 28, 2026 at 05:57:20PM +0900, Waffle0823 wrote:
> Instead of marking the entire display as dirty, calculate
> start_row and end_row based on off/len and mark only those rows.
> This improves performance for partial framebuffer updates.
>
> Signed-off-by: Waffle0283 csshin9928@gmail.com
> ---
> drivers/staging/fbtft/fbtft-core.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
> index 8a5ccc8ae0a1..0fbdfdaaa94d 100644
> --- a/drivers/staging/fbtft/fbtft-core.c
> +++ b/drivers/staging/fbtft/fbtft-core.c
> @@ -415,8 +415,11 @@ static void fbtft_ops_damage_range(struct fb_info *info, off_t off, size_t len)
> {
> struct fbtft_par *par = info->par;
>
> - /* TODO: only mark changed area update all for now */
> - par->fbtftops.mkdirty(info, -1, 0);
> + __u32 width = info->var.xres;
> + __u32 start_row = off / width;
> + __u32 end_row = (off + len - 1) / width;
> +
> + par->fbtftops.mkdirty(info, start_row, end_row);
> }
>
> static void fbtft_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height)
> --
> 2.52.0
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- It looks like you did not use your "real" name for the patch on either
the Signed-off-by: line, or the From: line (both of which have to
match). Please read the kernel file,
Documentation/process/submitting-patches.rst for how to do this
correctly.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* Re: [PATCH] printk, vt, fbcon: Remove console_conditional_schedule()
From: Petr Mladek @ 2026-01-28 9:02 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: linux-kernel, linux-serial, linux-fbdev, dri-devel,
linux-rt-devel, Steven Rostedt, John Ogness, Sergey Senozhatsky,
Greg Kroah-Hartman, Jiri Slaby, Simona Vetter, Helge Deller
In-Reply-To: <20260126180836.SNCdMW2f@linutronix.de>
On Mon 2026-01-26 19:08:36, Sebastian Andrzej Siewior wrote:
> do_con_write(), fbcon_redraw.*() invoke console_conditional_schedule()
> which is a conditional scheduling point based on printk's internal
> variables console_may_schedule. It may only be used if the console lock
> is acquired for instance via console_lock() or console_trylock().
>
> Prinkt sets the internal variable to 1 (and allows to schedule)
> if the console lock has been acquired via console_lock(). The trylock
> does not allow it.
>
> The console_conditional_schedule() invocation in do_con_write() is
> invoked shortly before console_unlock().
> The console_conditional_schedule() invocation in fbcon_redraw.*()
> original from fbcon_scroll() / vt's con_scroll() which originate from a
> line feed.
>
> In console_unlock() the variable is set to 0 (forbids to schedule) and
> it tries to schedule while making progress printing. This is brand new
> compared to when console_conditional_schedule() was added in v2.4.9.11.
>
> In v2.6.38-rc3, console_unlock() (started its existence) iterated over
> all consoles and flushed them with disabled interrupts. A scheduling
> attempt here was not possible, it relied that a long print scheduled
> before console_unlock().
>
> Since commit 8d91f8b15361d ("printk: do cond_resched() between lines
> while outputting to consoles"), which appeared in v4.5-rc1,
> console_unlock() attempts to schedule if it was allowed to schedule
> while during console_lock(). Each record is idealy one line so after
> every line feed.
>
> This console_conditional_schedule() is also only relevant on
> PREEMPT_NONE and PREEMPT_VOLUNTARY builds. In other configurations
> cond_resched() becomes a nop and has no impact.
>
> I'm bringing this all up just proof that it is not required anymore. It
> becomes a problem on a PREEMPT_RT build with debug code enabled because
> that might_sleep() in cond_resched() remains and triggers a warnings.
> This is due to
>
> legacy_kthread_func-> console_flush_one_record -> vt_console_print-> lf
> -> con_scroll -> fbcon_scroll
>
> and vt_console_print() acquires a spinlock_t which does not allow a
> voluntary schedule. There is no need to fb_scroll() to schedule since
> console_flush_one_record() attempts to schedule after each line.
> !PREEMPT_RT is not affected because the legacy printing thread is only
> enabled on PREEMPT_RT builds.
>
> Therefore I suggest to remove console_conditional_schedule().
>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Helge Deller <deller@gmx.de>
> Cc: linux-fbdev@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Fixes: 5f53ca3ff83b4 ("printk: Implement legacy printer kthread for PREEMPT_RT")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Just for record. This change looks OK from printk() POV.
printk() does console_trylock() and calls console_unlock()
with preemption disabled anyway, see vprintk_emit().
VT code still synchronizes some operations using console_lock().
It is possible that some non-printk related operations rely
on this. But it is hard to say. It might actually be a good
idea to find it out.
Also I have seen many printk-related softlockups. But they
were always caused by slow serial consoles. I can't remember
any in VT code.
Feel free to use:
Acked-by: Petr Mladek <pmladek@suse.com> # from printk() POV
Best Regards,
Petr
^ permalink raw reply
* [PATCH] fbtft: Improve damage_range to mark only changed rows
From: Waffle0823 @ 2026-01-28 8:57 UTC (permalink / raw)
To: andy; +Cc: gregkh, dri-devel, linux-fbdev, linux-staging, Waffle0823
Instead of marking the entire display as dirty, calculate
start_row and end_row based on off/len and mark only those rows.
This improves performance for partial framebuffer updates.
Signed-off-by: Waffle0283 csshin9928@gmail.com
---
drivers/staging/fbtft/fbtft-core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 8a5ccc8ae0a1..0fbdfdaaa94d 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -415,8 +415,11 @@ static void fbtft_ops_damage_range(struct fb_info *info, off_t off, size_t len)
{
struct fbtft_par *par = info->par;
- /* TODO: only mark changed area update all for now */
- par->fbtftops.mkdirty(info, -1, 0);
+ __u32 width = info->var.xres;
+ __u32 start_row = off / width;
+ __u32 end_row = (off + len - 1) / width;
+
+ par->fbtftops.mkdirty(info, start_row, end_row);
}
static void fbtft_ops_damage_area(struct fb_info *info, u32 x, u32 y, u32 width, u32 height)
--
2.52.0
^ permalink raw reply related
* Re: [PATCH] fbdev: fix fb_pad_unaligned_buffer mask
From: Helge Deller @ 2026-01-27 20:47 UTC (permalink / raw)
To: Osama Abdelkader, Simona Vetter, Thomas Zimmermann, Lee Jones,
Murad Masimov, Yongzhen Zhang, Quanmin Yan, linux-fbdev,
dri-devel, linux-kernel
In-Reply-To: <20260127193101.12343-1-osama.abdelkader@gmail.com>
On 1/27/26 20:30, Osama Abdelkader wrote:
> mask is u8, so it should use 0xff instead of 0xfff
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
> drivers/video/fbdev/core/fbmem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
applied.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH v2] fbdev: avoid out-of-bounds read in fb_pad_unaligned_buffer()
From: Osama Abdelkader @ 2026-01-27 19:40 UTC (permalink / raw)
To: Helge Deller
Cc: Simona Vetter, Thomas Zimmermann, Lee Jones,
Daniel Thompson (RISCstar), Murad Masimov, Quanmin Yan,
Yongzhen Zhang, linux-fbdev, dri-devel, linux-kernel,
syzbot+55e03490a0175b8dd81d
In-Reply-To: <889fd11b-80ea-4c23-b47f-4e6b17536b0f@gmx.de>
On Tue, Jan 27, 2026 at 06:57:32PM +0100, Helge Deller wrote:
> On 1/24/26 17:46, Osama Abdelkader wrote:
> > fb_pad_unaligned_buffer() unconditionally reads and advances the source
> > pointer for the final byte of each row, even when no bits from that byte
> > are actually consumed.
> >
> > When shift_high >= mod, the remaining bits do not cross a byte boundary,
> > but the code still accesses the next source byte. This can lead to
> > out-of-bounds reads under malformed geometry, as reported by syzbot.
> >
> > Fix this by only accessing and consuming the final source byte when it
> > contributes bits (shift_high < mod).
> >
> > This fixes the KASAN slab-out-of-bounds read reported by syzkaller:
> > https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
> >
> > Reported-by: syzbot+55e03490a0175b8dd81d@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
> > Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> > ---
> > v2: address the real issue (shift_high >= mod) condition.
> > ---
> > drivers/video/fbdev/core/fbmem.c | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> > index eff757ebbed1..d125c3db37a1 100644
> > --- a/drivers/video/fbdev/core/fbmem.c
> > +++ b/drivers/video/fbdev/core/fbmem.c
> > @@ -100,7 +100,7 @@ EXPORT_SYMBOL(fb_pad_aligned_buffer);
> > void fb_pad_unaligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 idx, u32 height,
> > u32 shift_high, u32 shift_low, u32 mod)
> > {
> > - u8 mask = (u8) (0xfff << shift_high), tmp;
> > + u8 mask = (u8) (0xff << shift_high), tmp;
>
> This part is correct, but shouldn't be part of this patch.
I just sent a seperate patch for that, and going to remove it in next version of this one.
>
>
> > int i, j;
> > for (i = height; i--; ) {
> > @@ -113,15 +113,18 @@ void fb_pad_unaligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 idx, u32 height,
> > dst[j+1] = tmp;
> > src++;
> > }
> > - tmp = dst[idx];
> > - tmp &= mask;
> > - tmp |= *src >> shift_low;
> > - dst[idx] = tmp;
> > +
> > + /* Only consume another source byte if it contributes bits */
> > if (shift_high < mod) {
> > + tmp = dst[idx];
> > + tmp &= mask;
> > + tmp |= *src >> shift_low;
> > + dst[idx] = tmp;
> > tmp = *src << shift_high;
> > dst[idx+1] = tmp;
> > + src++;
> > }
> > - src++;
>
> Above you moved the src pointer inside the if(), so every line
> processed may miss a ptr increment. This means the source would need to
> be different too, but it hasn't changed, as it's still used from
> bit_putcs_unaligned() which prints a char from the character fonts.
>
> So, I believe this part at least is wrong.
> Did you test it?
>
I couldn't find syzbot's ReproC, so I did minimal one, I will re-test it and write you.
> Helge
BR,
Osama
^ permalink raw reply
* [PATCH] fbdev: fix fb_pad_unaligned_buffer mask
From: Osama Abdelkader @ 2026-01-27 19:30 UTC (permalink / raw)
To: Simona Vetter, Helge Deller, Thomas Zimmermann, Lee Jones,
Murad Masimov, Osama Abdelkader, Yongzhen Zhang, Quanmin Yan,
linux-fbdev, dri-devel, linux-kernel
mask is u8, so it should use 0xff instead of 0xfff
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
drivers/video/fbdev/core/fbmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index eff757ebbed1..cf199038f069 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -100,7 +100,7 @@ EXPORT_SYMBOL(fb_pad_aligned_buffer);
void fb_pad_unaligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 idx, u32 height,
u32 shift_high, u32 shift_low, u32 mod)
{
- u8 mask = (u8) (0xfff << shift_high), tmp;
+ u8 mask = (u8) (0xff << shift_high), tmp;
int i, j;
for (i = height; i--; ) {
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] printk, vt, fbcon: Remove console_conditional_schedule()
From: Helge Deller @ 2026-01-27 18:05 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sebastian Andrzej Siewior
Cc: linux-kernel, linux-serial, linux-fbdev, dri-devel,
linux-rt-devel, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Jiri Slaby, Simona Vetter
In-Reply-To: <2026012757-voting-griminess-ca35@gregkh>
On 1/27/26 15:24, Greg Kroah-Hartman wrote:
> On Mon, Jan 26, 2026 at 07:08:36PM +0100, Sebastian Andrzej Siewior wrote:
>> do_con_write(), fbcon_redraw.*() invoke console_conditional_schedule()
>> which is a conditional scheduling point based on printk's internal
>> variables console_may_schedule. It may only be used if the console lock
>> is acquired for instance via console_lock() or console_trylock().
>>
>> Prinkt sets the internal variable to 1 (and allows to schedule)
>> if the console lock has been acquired via console_lock(). The trylock
>> does not allow it.
>>
>> The console_conditional_schedule() invocation in do_con_write() is
>> invoked shortly before console_unlock().
>> The console_conditional_schedule() invocation in fbcon_redraw.*()
>> original from fbcon_scroll() / vt's con_scroll() which originate from a
>> line feed.
>>
>> In console_unlock() the variable is set to 0 (forbids to schedule) and
>> it tries to schedule while making progress printing. This is brand new
>> compared to when console_conditional_schedule() was added in v2.4.9.11.
>>
>> In v2.6.38-rc3, console_unlock() (started its existence) iterated over
>> all consoles and flushed them with disabled interrupts. A scheduling
>> attempt here was not possible, it relied that a long print scheduled
>> before console_unlock().
>>
>> Since commit 8d91f8b15361d ("printk: do cond_resched() between lines
>> while outputting to consoles"), which appeared in v4.5-rc1,
>> console_unlock() attempts to schedule if it was allowed to schedule
>> while during console_lock(). Each record is idealy one line so after
>> every line feed.
>>
>> This console_conditional_schedule() is also only relevant on
>> PREEMPT_NONE and PREEMPT_VOLUNTARY builds. In other configurations
>> cond_resched() becomes a nop and has no impact.
>>
>> I'm bringing this all up just proof that it is not required anymore. It
>> becomes a problem on a PREEMPT_RT build with debug code enabled because
>> that might_sleep() in cond_resched() remains and triggers a warnings.
>> This is due to
>>
>> legacy_kthread_func-> console_flush_one_record -> vt_console_print-> lf
>> -> con_scroll -> fbcon_scroll
>>
>> and vt_console_print() acquires a spinlock_t which does not allow a
>> voluntary schedule. There is no need to fb_scroll() to schedule since
>> console_flush_one_record() attempts to schedule after each line.
>> !PREEMPT_RT is not affected because the legacy printing thread is only
>> enabled on PREEMPT_RT builds.
>>
>> Therefore I suggest to remove console_conditional_schedule().
>>
>> Cc: Simona Vetter <simona@ffwll.ch>
>> Cc: Helge Deller <deller@gmx.de>
>> Cc: linux-fbdev@vger.kernel.org
>> Cc: dri-devel@lists.freedesktop.org
>> Fixes: 5f53ca3ff83b4 ("printk: Implement legacy printer kthread for PREEMPT_RT")
>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> ---
>>
>> A follow-up to
>> https://lore.kernel.org/all/20260114145955.d924Z-zu@linutronix.de/
>>
>> drivers/tty/vt/vt.c | 1 -
>> drivers/video/fbdev/core/fbcon.c | 6 ------
>> include/linux/console.h | 1 -
>> kernel/printk/printk.c | 16 ----------------
>> 4 files changed, 24 deletions(-)
>>
>> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
>> [....]
>
> No objection from me about removing this if it's not needed anymore!
>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
I've added it to the fbdev git tree to get some testing....
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH v2] fbdev: avoid out-of-bounds read in fb_pad_unaligned_buffer()
From: Helge Deller @ 2026-01-27 17:57 UTC (permalink / raw)
To: Osama Abdelkader, Simona Vetter, Thomas Zimmermann, Lee Jones,
Daniel Thompson (RISCstar), Murad Masimov, Quanmin Yan,
Yongzhen Zhang, linux-fbdev, dri-devel, linux-kernel
Cc: syzbot+55e03490a0175b8dd81d
In-Reply-To: <20260124164633.20444-1-osama.abdelkader@gmail.com>
On 1/24/26 17:46, Osama Abdelkader wrote:
> fb_pad_unaligned_buffer() unconditionally reads and advances the source
> pointer for the final byte of each row, even when no bits from that byte
> are actually consumed.
>
> When shift_high >= mod, the remaining bits do not cross a byte boundary,
> but the code still accesses the next source byte. This can lead to
> out-of-bounds reads under malformed geometry, as reported by syzbot.
>
> Fix this by only accessing and consuming the final source byte when it
> contributes bits (shift_high < mod).
>
> This fixes the KASAN slab-out-of-bounds read reported by syzkaller:
> https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
>
> Reported-by: syzbot+55e03490a0175b8dd81d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=55e03490a0175b8dd81d
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
> v2: address the real issue (shift_high >= mod) condition.
> ---
> drivers/video/fbdev/core/fbmem.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index eff757ebbed1..d125c3db37a1 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -100,7 +100,7 @@ EXPORT_SYMBOL(fb_pad_aligned_buffer);
> void fb_pad_unaligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 idx, u32 height,
> u32 shift_high, u32 shift_low, u32 mod)
> {
> - u8 mask = (u8) (0xfff << shift_high), tmp;
> + u8 mask = (u8) (0xff << shift_high), tmp;
This part is correct, but shouldn't be part of this patch.
> int i, j;
>
> for (i = height; i--; ) {
> @@ -113,15 +113,18 @@ void fb_pad_unaligned_buffer(u8 *dst, u32 d_pitch, u8 *src, u32 idx, u32 height,
> dst[j+1] = tmp;
> src++;
> }
> - tmp = dst[idx];
> - tmp &= mask;
> - tmp |= *src >> shift_low;
> - dst[idx] = tmp;
> +
> + /* Only consume another source byte if it contributes bits */
> if (shift_high < mod) {
> + tmp = dst[idx];
> + tmp &= mask;
> + tmp |= *src >> shift_low;
> + dst[idx] = tmp;
> tmp = *src << shift_high;
> dst[idx+1] = tmp;
> + src++;
> }
> - src++;
Above you moved the src pointer inside the if(), so every line
processed may miss a ptr increment. This means the source would need to
be different too, but it hasn't changed, as it's still used from
bit_putcs_unaligned() which prints a char from the character fonts.
So, I believe this part at least is wrong.
Did you test it?
Helge
^ permalink raw reply
* Re: [PATCH] video: of_display_timing: fix refcount leak in of_get_display_timings()
From: Helge Deller @ 2026-01-27 17:32 UTC (permalink / raw)
To: Weigang He; +Cc: linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20260116095751.177055-1-geoffreyhe2@gmail.com>
On 1/16/26 10:57, Weigang He wrote:
> of_parse_phandle() returns a device_node with refcount incremented,
> which is stored in 'entry' and then copied to 'native_mode'. When the
> error paths at lines 184 or 192 jump to 'entryfail', native_mode's
> refcount is not decremented, causing a refcount leak.
>
> Fix this by changing the goto target from 'entryfail' to 'timingfail',
> which properly calls of_node_put(native_mode) before cleanup.
>
> Fixes: cc3f414cf2e4 ("video: add of helper for display timings/videomode")
> Cc: stable@vger.kernel.org
> Signed-off-by: Weigang He <geoffreyhe2@gmail.com>
> ---
> drivers/video/of_display_timing.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
applied.
Thanks!
Helge
^ permalink raw reply
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