* [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
* [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
* 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
* 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 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 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 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: 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 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] 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 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 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 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
* [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] 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
* 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] fbdev: au1100fb: Add missing check for clk_enable
From: Helge Deller @ 2026-01-28 13:29 UTC (permalink / raw)
To: Chen Ni, u.kleine-koenig, elfring; +Cc: linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <20260128091004.2747011-1-nichen@iscas.ac.cn>
On 1/28/26 10:10, Chen Ni wrote:
> Add check for the return value of clk_enable() andreturn the error
> if it fails in order to catch the error.
Why?
If it really fails, the screen and everything stay blank,
so it's even worse....
It seems nobody is actually checking the return value, most likely
because if it fails, the system will sleep forever anyway.
Or am I missing something?
For now I reject this patch.
Helge
> 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);
^ permalink raw reply
* [PATCH v4] fbtft: Improve damage_range to mark only changed rows 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.
From: ChanSoo Shin @ 2026-01-28 14:08 UTC (permalink / raw)
To: andy; +Cc: gregkh, dri-devel, linux-fbdev, linux-staging, ChanSoo Shin
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 v4] fbtft: Improve damage_range to mark only changed rows 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.
From: Greg KH @ 2026-01-28 14:44 UTC (permalink / raw)
To: ChanSoo Shin; +Cc: andy, dri-devel, linux-fbdev, linux-staging
In-Reply-To: <20260128140844.949262-1-csshin9928@gmail.com>
On Wed, Jan 28, 2026 at 11:08:44PM +0900, ChanSoo Shin wrote:
> Signed-off-by: ChanSoo Shin <csshin9928@gmail.com>
> ---
Something went wrong with your subject line :(
^ permalink raw reply
* Re: [PATCH v1 v1 0/4] [RUST] Framebuffer driver support
From: Alexandre Courbot @ 2026-01-28 14:55 UTC (permalink / raw)
To: pengfuyuan
Cc: Thomas Zimmermann, Danilo Krummrich, Alice Ryhl, Daniel Almeida,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Greg Kroah-Hartman,
Rafael J . Wysocki, David Airlie, Simona Vetter, Helge Deller,
Hans de Goede, Lee Jones, Sam Ravnborg, Zsolt Kajtar,
Ville Syrjälä, rust-for-linux, linux-kernel, dri-devel,
linux-fbdev
In-Reply-To: <20260127080419.GA965382@peng>
On Tue Jan 27, 2026 at 5:04 PM JST, pengfuyuan wrote:
> On Mon, Jan 26, 2026 at 07:28:21PM +0900, Alexandre Courbot wrote:
>> On Mon Jan 26, 2026 at 7:01 PM JST, Thomas Zimmermann wrote:
>> > Hi
>> >
>> > Am 26.01.26 um 09:17 schrieb pengfuyuan:
>> >> This patch series adds Rust bindings and safe abstractions for the Linux
>> >> framebuffer subsystem, enabling framebuffer drivers to be implemented in Rust.
>> >
>> > The framebuffer subsystem is obsolete and has been deprecated for a
>> > decade. No new drivers accepted. Anything that really wants fbdev
>> > already has a driver. Can we please let it die?
>>
>> This, and the patchset is also obviously AI-generated.
>
> Hi,
> Thank you for the feedback.
> I’d like to be clear about how I used AI in this work:
>
> 1.Cover letter – Yes, I used AI to help summarize and phrase the cover letter.
> 2.Comments in the code – Some comments were written or refined with AI assistance.
> 3.Learning the codebase – When reading and understanding existing Rust-for-Linux code (including DRM and other abstractions), I used AI as a helper to analyze and explain structure and patterns.
> 4.Writing the code – The implementation was not fully generated by AI. I wrote the code myself and used AI mainly to look up existing abstractions, traits, and APIs (e.g. “how does X work? ”, “what’s the right trait for Y?”) while I was coding.
>
> So: AI was used for summaries, comments, learning, and looking things up; the logic and structure of the code are mine, and I take responsibility for them.
> If you have concerns about specific parts (e.g. wording, style, or design), I’m happy to rework those patches or to adjust how I describe tool use in future submissions.
Appreciate the clarification. One piece of feedback if I may.
The cover letter is the first thing reviewers see of your patchset. If
it reads like impersonal, mechanically exhaustive generic AI slop full
of bullet points that doesn't follow kernel conventions, reviewers will
assume the rest of the patchset is of the same caliber and discard it as
something entirely generated by a bot.
A patch is not only the code. Comments and commit logs are as important
and should be given the same care. This patchset is a great illustration
of how AI completely misses the point: it will write 5 dreadful
paragraphs explaining *what* it did, but what reviewers want is one
sentence that explains *why*.
If your use of AI for the commit logs and comments is motivated by a
lack of confidence in your English, how about this: write them in your
native language and ask AI to translate it for you. That's something
LLMs actually shine at (that, and using them to learn about a codebase),
and the translated result will still carry the flow you intended, as
well as your own touch.
In other words, make sure that the AI assists you, as opposed to you
assisting the AI.
Hope this helps. Looking forward to your future submissions.
^ permalink raw reply
* Re: [PATCH] fbdev: au1100fb: Add missing check for clk_enable
From: Uwe Kleine-König @ 2026-01-28 14:55 UTC (permalink / raw)
To: Helge Deller; +Cc: Chen Ni, elfring, linux-fbdev, dri-devel, linux-kernel
In-Reply-To: <9ca6e1cc-7469-450b-a58f-9279412fb9de@gmx.de>
[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]
On Wed, Jan 28, 2026 at 02:29:37PM +0100, Helge Deller wrote:
> On 1/28/26 10:10, Chen Ni wrote:
> > Add check for the return value of clk_enable() andreturn the error
> > if it fails in order to catch the error.
>
> Why?
> If it really fails, the screen and everything stay blank,
> so it's even worse....
> It seems nobody is actually checking the return value, most likely
> because if it fails, the system will sleep forever anyway.
> Or am I missing something?
>
> For now I reject this patch.
I think it's the right approach to check the return value. If
au1100fb_drv_resume() fails, the system as a whole will still be
up and it will know that the fb failed to resume.
With the status quo it will assume it's properly resumed despite it
didn't.
Still there are some things so criticise:
- s/Add missing check for clk_enable/Check return value of clk_enable() in .resume().
- s/ / /
While looking at the driver I spotted another issue: There is a single
static struct au1100fb_regs fbregs;
to store the register values. This only works if there are <= 1 such
devices (and wastes memory if there is no such device).
Usually there is at most one such device, but it's cheap to move the
struct into struct au1100fb_device, and if it's only to make the driver
a better template for new drivers.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH RFC v6 05/26] nova-core: mm: Add support to use PRAMIN windows to write to VRAM
From: Joel Fernandes @ 2026-01-28 15:27 UTC (permalink / raw)
To: Danilo Krummrich
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: <DG07HZN0PL87.X5MKDCVVYIRE@kernel.org>
On 1/28/2026 7:04 AM, Danilo Krummrich wrote:
> 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.
I will go over these concerns, just to clarify - do you mean forbidding
*any* lock or do you mean only forbidding non-atomic locks? I believe we
can avoid non-atomic locks completely - actually I just wrote a patch
before I read this email to do just. If we are to forbid any locking at
all, that might require some careful redesign to handle the above race
afaics.
>
> 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.
Thanks for listing all the concerns below, this is very valuable. I will
go over all these and all cases before posting the v7 now that I have this.
--
Joel Fernandes
> 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] fbtft: Improve damage_range to mark only changed rows
From: Nam Cao @ 2026-01-28 17:05 UTC (permalink / raw)
To: ChanSoo Shin, andy
Cc: gregkh, dri-devel, linux-fbdev, linux-staging, ChanSoo Shin
In-Reply-To: <20260128130503.868466-1-csshin9928@gmail.com>
ChanSoo Shin <csshin9928@gmail.com> writes:
> 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);
This doesn't look right: mkdirty() takes start row and number of rows,
not start row and end row. Don't be fooled by how mkdirty() is declared,
look at how it is implemented.
Nam
^ permalink raw reply
* [PATCH v5] fbtft: limit dirty rows based on damage range
From: ChanSoo Shin @ 2026-01-28 20:39 UTC (permalink / raw)
To: andy; +Cc: gregkh, dri-devel, linux-fbdev, linux-staging, ChanSoo Shin
Instead of marking the entire display as dirty, calculate the start
and end rows based on the damage offset and length and only mark the
affected rows dirty. This reduces unnecessary full framebuffer updates
for partial writes.
Signed-off-by: ChanSoo Shin <csshin9928@gmail.com>
---
drivers/staging/fbtft/fbtft-core.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 8a5ccc8ae0a1..1d5cb45199d0 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -415,8 +415,12 @@ 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;
+ __u32 height = end_row - start_row + 1;
+
+ par->fbtftops.mkdirty(info, start_row, height);
}
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] staging: fbtft: use guard() to simplify code
From: Paul Retourné @ 2026-01-28 21:26 UTC (permalink / raw)
To: andy, gregkh
Cc: Paul Retourné, dri-devel, linux-fbdev, linux-staging,
linux-kernel
Use guard() to simplify mutex locking. No functional change.
Signed-off-by: Paul Retourné <paul.retourne@orange.fr>
---
drivers/staging/fbtft/fb_ssd1305.c | 4 ++--
drivers/staging/fbtft/fb_ssd1306.c | 4 ++--
drivers/staging/fbtft/fbtft-sysfs.c | 8 ++++----
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/fbtft/fb_ssd1305.c b/drivers/staging/fbtft/fb_ssd1305.c
index 020fe48fed0be..d9215cb35e8f8 100644
--- a/drivers/staging/fbtft/fb_ssd1305.c
+++ b/drivers/staging/fbtft/fb_ssd1305.c
@@ -10,6 +10,7 @@
#include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/delay.h>
+#include <linux/cleanup.h>
#include "fbtft.h"
@@ -35,12 +36,11 @@ static int init_display(struct fbtft_par *par)
par->fbtftops.reset(par);
if (par->gamma.curves[0] == 0) {
- mutex_lock(&par->gamma.lock);
+ guard(mutex)(&par->gamma.lock);
if (par->info->var.yres == 64)
par->gamma.curves[0] = 0xCF;
else
par->gamma.curves[0] = 0x8F;
- mutex_unlock(&par->gamma.lock);
}
/* Set Display OFF */
diff --git a/drivers/staging/fbtft/fb_ssd1306.c b/drivers/staging/fbtft/fb_ssd1306.c
index 478d710469b91..c230d599ff928 100644
--- a/drivers/staging/fbtft/fb_ssd1306.c
+++ b/drivers/staging/fbtft/fb_ssd1306.c
@@ -10,6 +10,7 @@
#include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/delay.h>
+#include <linux/cleanup.h>
#include "fbtft.h"
@@ -34,12 +35,11 @@ static int init_display(struct fbtft_par *par)
par->fbtftops.reset(par);
if (par->gamma.curves[0] == 0) {
- mutex_lock(&par->gamma.lock);
+ guard(mutex)(&par->gamma.lock);
if (par->info->var.yres == 64)
par->gamma.curves[0] = 0xCF;
else
par->gamma.curves[0] = 0x8F;
- mutex_unlock(&par->gamma.lock);
}
/* Set Display OFF */
diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c
index e45c90a03a903..72d6cf899336e 100644
--- a/drivers/staging/fbtft/fbtft-sysfs.c
+++ b/drivers/staging/fbtft/fbtft-sysfs.c
@@ -1,4 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
+#include <linux/cleanup.h>
+
#include "fbtft.h"
#include "internal.h"
@@ -95,14 +97,13 @@ sprintf_gamma(struct fbtft_par *par, u32 *curves, char *buf)
ssize_t len = 0;
unsigned int i, j;
- mutex_lock(&par->gamma.lock);
+ guard(mutex)(&par->gamma.lock);
for (i = 0; i < par->gamma.num_curves; i++) {
for (j = 0; j < par->gamma.num_values; j++)
len += scnprintf(&buf[len], PAGE_SIZE,
"%04x ", curves[i * par->gamma.num_values + j]);
buf[len - 1] = '\n';
}
- mutex_unlock(&par->gamma.lock);
return len;
}
@@ -124,11 +125,10 @@ static ssize_t store_gamma_curve(struct device *device,
if (ret)
return ret;
- mutex_lock(&par->gamma.lock);
+ guard(mutex)(&par->gamma.lock);
memcpy(par->gamma.curves, tmp_curves,
par->gamma.num_curves * par->gamma.num_values *
sizeof(tmp_curves[0]));
- mutex_unlock(&par->gamma.lock);
return count;
}
--
2.52.0
^ 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