[PATCH v1 0/4] staging: fbtft: reduce stack usage by avoiding large write_reg() varargs

[PATCH v1 0/4] staging: fbtft: reduce stack usage by avoiding large write_reg() varargs

[Sun Jian]

Hi,

This series fixes clang `-Wframe-larger-than=1024` warnings in the fbtft
staging drivers.

The warnings are triggered by very large `write_reg()`/`write_register()`
varargs calls, which result in excessive stack usage.

Switch the affected paths to send a u8 command byte followed by the u8
payload using `fbtft_write_buf_dc()`. The register values and ordering are
kept unchanged; only the transfer method is updated.

Patches:
  1/4 staging: fbtft: core: avoid large stack usage in DT init parsing
  2/4 staging: fbtft: ssd1351: send gamma table via fbtft_write_buf_dc()
  3/4 staging: fbtft: ssd1331: send gamma table via fbtft_write_buf_dc()
  4/4 staging: fbtft: hx8353d: send LUT via buffer to reduce stack usage

Thanks,
Sun Jian

--
Sun Jian (4):
  staging: fbtft: core: avoid large stack usage in DT init parsing
  staging: fbtft: ssd1351: send gamma table via fbtft_write_buf_dc()
  staging: fbtft: ssd1331: send gamma table via fbtft_write_buf_dc()
  staging: fbtft: hx8353d: send LUT via buffer to reduce stack usage

 drivers/staging/fbtft/fb_hx8353d.c | 38 ++++++++++++++++++++++--------
 drivers/staging/fbtft/fb_ssd1331.c | 29 ++++++++++++-----------
 drivers/staging/fbtft/fb_ssd1351.c | 35 ++++++++++++---------------
 drivers/staging/fbtft/fbtft-core.c | 32 ++++++++++---------------
 4 files changed, 71 insertions(+), 63 deletions(-)

-- 
2.43.0

[PATCH v1 1/4] staging: fbtft: core: avoid large stack usage in DT init parsing

[Sun Jian]

Clang reports a large stack frame for fbtft_init_display_from_property()
(-Wframe-larger-than=1024) when the init sequence is emitted through a
fixed 64-argument write_register() call.

write_reg()/write_register() relies on NUMARGS((int[]){...}) and large
varargs which inflates stack usage. Switch the DT "init" path to send the
command byte and the payload via fbtft_write_buf_dc() instead.

No functional change intended: the same register values are sent in the
same order, only the transport is changed.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 drivers/staging/fbtft/fbtft-core.c | 32 ++++++++++++------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 8a5ccc8ae0a1..127d0de87e03 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -846,7 +846,8 @@ EXPORT_SYMBOL(fbtft_unregister_framebuffer);
 static int fbtft_init_display_from_property(struct fbtft_par *par)
 {
 	struct device *dev = par->info->device;
-	int buf[64], count, index, i, j, ret;
+	u8 buf[64];
+	int count, index, i, j, ret;
 	u32 *values;
 	u32 val;
 
@@ -881,7 +882,7 @@ static int fbtft_init_display_from_property(struct fbtft_par *par)
 					ret = -EINVAL;
 					goto out_free;
 				}
-				buf[i++] = val;
+				buf[i++] = val & 0xFF;
 				val = values[++index];
 			}
 			/* make debug message */
@@ -891,23 +892,16 @@ static int fbtft_init_display_from_property(struct fbtft_par *par)
 				fbtft_par_dbg(DEBUG_INIT_DISPLAY, par,
 					      "buf[%d] = %02X\n", j, buf[j]);
 
-			par->fbtftops.write_register(par, i,
-				buf[0], buf[1], buf[2], buf[3],
-				buf[4], buf[5], buf[6], buf[7],
-				buf[8], buf[9], buf[10], buf[11],
-				buf[12], buf[13], buf[14], buf[15],
-				buf[16], buf[17], buf[18], buf[19],
-				buf[20], buf[21], buf[22], buf[23],
-				buf[24], buf[25], buf[26], buf[27],
-				buf[28], buf[29], buf[30], buf[31],
-				buf[32], buf[33], buf[34], buf[35],
-				buf[36], buf[37], buf[38], buf[39],
-				buf[40], buf[41], buf[42], buf[43],
-				buf[44], buf[45], buf[46], buf[47],
-				buf[48], buf[49], buf[50], buf[51],
-				buf[52], buf[53], buf[54], buf[55],
-				buf[56], buf[57], buf[58], buf[59],
-				buf[60], buf[61], buf[62], buf[63]);
+			/* buf[0] is command, buf[1..i-1] is data */
+			ret = fbtft_write_buf_dc(par, &buf[0], 1, 0);
+			if (ret < 0)
+				goto out_free;
+
+			if (i > 1) {
+				ret = fbtft_write_buf_dc(par, &buf[1], i - 1, 1);
+				if (ret < 0)
+					goto out_free;
+			}
 		} else if (val & FBTFT_OF_INIT_DELAY) {
 			fbtft_par_dbg(DEBUG_INIT_DISPLAY, par,
 				      "init: msleep(%u)\n", val & 0xFFFF);
-- 
2.43.0

Re: [PATCH v1 1/4] staging: fbtft: core: avoid large stack usage in DT init parsing

[Andy Shevchenko]

On Sun, Jan 04, 2026 at 07:06:35PM +0800, Sun Jian wrote:
> Clang reports a large stack frame for fbtft_init_display_from_property()
> (-Wframe-larger-than=1024) when the init sequence is emitted through a
> fixed 64-argument write_register() call.
> 
> write_reg()/write_register() relies on NUMARGS((int[]){...}) and large
> varargs which inflates stack usage. Switch the DT "init" path to send the
> command byte and the payload via fbtft_write_buf_dc() instead.
> 
> No functional change intended: the same register values are sent in the
> same order, only the transport is changed.

How did you test this?

...

>  	struct device *dev = par->info->device;
> -	int buf[64], count, index, i, j, ret;
> +	u8 buf[64];
> +	int count, index, i, j, ret;

Please, try to preserve reversed xmas tree order.

>  	u32 *values;
>  	u32 val;
>  

...

> -				buf[i++] = val;
> +				buf[i++] = val & 0xFF;

Unneeded change, I suppose.

...

> -			par->fbtftops.write_register(par, i,
> -				buf[0], buf[1], buf[2], buf[3],
> -				buf[4], buf[5], buf[6], buf[7],
> -				buf[8], buf[9], buf[10], buf[11],
> -				buf[12], buf[13], buf[14], buf[15],
> -				buf[16], buf[17], buf[18], buf[19],
> -				buf[20], buf[21], buf[22], buf[23],
> -				buf[24], buf[25], buf[26], buf[27],
> -				buf[28], buf[29], buf[30], buf[31],
> -				buf[32], buf[33], buf[34], buf[35],
> -				buf[36], buf[37], buf[38], buf[39],
> -				buf[40], buf[41], buf[42], buf[43],
> -				buf[44], buf[45], buf[46], buf[47],
> -				buf[48], buf[49], buf[50], buf[51],
> -				buf[52], buf[53], buf[54], buf[55],
> -				buf[56], buf[57], buf[58], buf[59],
> -				buf[60], buf[61], buf[62], buf[63]);
> +			/* buf[0] is command, buf[1..i-1] is data */
> +			ret = fbtft_write_buf_dc(par, &buf[0], 1, 0);
> +			if (ret < 0)
> +				goto out_free;
> +
> +			if (i > 1) {
> +				ret = fbtft_write_buf_dc(par, &buf[1], i - 1, 1);
> +				if (ret < 0)
> +					goto out_free;
> +			}

I believe this is incorrect change and has not to be applied. write !=
write_register. Without any evidence of testing, definite NAK to it.
Otherwise, please provide detailed testing pattern and which devices were
tested.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/4] staging: fbtft: core: avoid large stack usage in DT init parsing

[sun jian]

Hi Andy,

Thanks for the feedback.

You are right: changing the DT init path from write_register() to
fbtft_write_buf_dc() implicitly assumes "cmd byte + payload bytes" and
does not preserve the generic write_register() semantics (e.g. regwidth /
bus-specific handling).I only have clang/arm64 build coverage (no
access to the actual panels),
so I can’t provide runtime validation yet. For the remaining 3 driver-local
patches, all affected drivers have .regwidth = 8 and the sequences are
“1-byte command + N bytes data” (gamma/LUT). The intent was to avoid the
huge write_reg() varargs call that triggers -Wframe-larger-than=1024.

Given the lack of hardware, would you prefer one of the following?

1. Drop the driver changes and instead bump -Wframe-larger-than for these
   specific objects in the Makefile as an exception; or

2. Keep the driver changes but I should provide a detailed test pattern /
   list of tested devices — if so, what level of detail would be acceptable
   (exact panel model + wiring/bus type + expected output), and is “build-only”
   ever sufficient for warning-only changes in fbtft?

Happy to follow the approach you think is appropriate for this staging driver.

Best regards,
Sun Jian

On Tue, Jan 6, 2026 at 12:28 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Sun, Jan 04, 2026 at 07:06:35PM +0800, Sun Jian wrote:
> > Clang reports a large stack frame for fbtft_init_display_from_property()
> > (-Wframe-larger-than=1024) when the init sequence is emitted through a
> > fixed 64-argument write_register() call.
> >
> > write_reg()/write_register() relies on NUMARGS((int[]){...}) and large
> > varargs which inflates stack usage. Switch the DT "init" path to send the
> > command byte and the payload via fbtft_write_buf_dc() instead.
> >
> > No functional change intended: the same register values are sent in the
> > same order, only the transport is changed.
>
> How did you test this?
>
> ...
>
> >       struct device *dev = par->info->device;
> > -     int buf[64], count, index, i, j, ret;
> > +     u8 buf[64];
> > +     int count, index, i, j, ret;
>
> Please, try to preserve reversed xmas tree order.
>
> >       u32 *values;
> >       u32 val;
> >
>
> ...
>
> > -                             buf[i++] = val;
> > +                             buf[i++] = val & 0xFF;
>
> Unneeded change, I suppose.
>
> ...
>
> > -                     par->fbtftops.write_register(par, i,
> > -                             buf[0], buf[1], buf[2], buf[3],
> > -                             buf[4], buf[5], buf[6], buf[7],
> > -                             buf[8], buf[9], buf[10], buf[11],
> > -                             buf[12], buf[13], buf[14], buf[15],
> > -                             buf[16], buf[17], buf[18], buf[19],
> > -                             buf[20], buf[21], buf[22], buf[23],
> > -                             buf[24], buf[25], buf[26], buf[27],
> > -                             buf[28], buf[29], buf[30], buf[31],
> > -                             buf[32], buf[33], buf[34], buf[35],
> > -                             buf[36], buf[37], buf[38], buf[39],
> > -                             buf[40], buf[41], buf[42], buf[43],
> > -                             buf[44], buf[45], buf[46], buf[47],
> > -                             buf[48], buf[49], buf[50], buf[51],
> > -                             buf[52], buf[53], buf[54], buf[55],
> > -                             buf[56], buf[57], buf[58], buf[59],
> > -                             buf[60], buf[61], buf[62], buf[63]);
> > +                     /* buf[0] is command, buf[1..i-1] is data */
> > +                     ret = fbtft_write_buf_dc(par, &buf[0], 1, 0);
> > +                     if (ret < 0)
> > +                             goto out_free;
> > +
> > +                     if (i > 1) {
> > +                             ret = fbtft_write_buf_dc(par, &buf[1], i - 1, 1);
> > +                             if (ret < 0)
> > +                                     goto out_free;
> > +                     }
>
> I believe this is incorrect change and has not to be applied. write !=
> write_register. Without any evidence of testing, definite NAK to it.
> Otherwise, please provide detailed testing pattern and which devices were
> tested.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v1 1/4] staging: fbtft: core: avoid large stack usage in DT init parsing

[Andy Shevchenko]

On Tue, Jan 06, 2026 at 01:00:33AM +0800, sun jian wrote:

> Thanks for the feedback.

You're welcome, but please, do not top-post!

> You are right: changing the DT init path from write_register() to
> fbtft_write_buf_dc() implicitly assumes "cmd byte + payload bytes" and
> does not preserve the generic write_register() semantics (e.g. regwidth /
> bus-specific handling).I only have clang/arm64 build coverage (no
> access to the actual panels),
> so I can’t provide runtime validation yet. For the remaining 3 driver-local
> patches, all affected drivers have .regwidth = 8 and the sequences are
> “1-byte command + N bytes data” (gamma/LUT). The intent was to avoid the
> huge write_reg() varargs call that triggers -Wframe-larger-than=1024.
> 
> Given the lack of hardware, would you prefer one of the following?

How can you test without hardware at hand?

> 1. Drop the driver changes and instead bump -Wframe-larger-than for these
>    specific objects in the Makefile as an exception; or
> 
> 2. Keep the driver changes but I should provide a detailed test pattern /
>    list of tested devices — if so, what level of detail would be acceptable
>    (exact panel model + wiring/bus type + expected output), and is “build-only”
>    ever sufficient for warning-only changes in fbtft?
> 
> Happy to follow the approach you think is appropriate for this staging driver.

I already explained in the response to the cover letter. Please, read it.

> On Tue, Jan 6, 2026 at 12:28 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > On Sun, Jan 04, 2026 at 07:06:35PM +0800, Sun Jian wrote:
> > > Clang reports a large stack frame for fbtft_init_display_from_property()
> > > (-Wframe-larger-than=1024) when the init sequence is emitted through a
> > > fixed 64-argument write_register() call.
> > >
> > > write_reg()/write_register() relies on NUMARGS((int[]){...}) and large
> > > varargs which inflates stack usage. Switch the DT "init" path to send the
> > > command byte and the payload via fbtft_write_buf_dc() instead.
> > >
> > > No functional change intended: the same register values are sent in the
> > > same order, only the transport is changed.
> >
> > How did you test this?

...

> > > -                     par->fbtftops.write_register(par, i,
> > > -                             buf[0], buf[1], buf[2], buf[3],
> > > -                             buf[4], buf[5], buf[6], buf[7],
> > > -                             buf[8], buf[9], buf[10], buf[11],
> > > -                             buf[12], buf[13], buf[14], buf[15],
> > > -                             buf[16], buf[17], buf[18], buf[19],
> > > -                             buf[20], buf[21], buf[22], buf[23],
> > > -                             buf[24], buf[25], buf[26], buf[27],
> > > -                             buf[28], buf[29], buf[30], buf[31],
> > > -                             buf[32], buf[33], buf[34], buf[35],
> > > -                             buf[36], buf[37], buf[38], buf[39],
> > > -                             buf[40], buf[41], buf[42], buf[43],
> > > -                             buf[44], buf[45], buf[46], buf[47],
> > > -                             buf[48], buf[49], buf[50], buf[51],
> > > -                             buf[52], buf[53], buf[54], buf[55],
> > > -                             buf[56], buf[57], buf[58], buf[59],
> > > -                             buf[60], buf[61], buf[62], buf[63]);
> > > +                     /* buf[0] is command, buf[1..i-1] is data */
> > > +                     ret = fbtft_write_buf_dc(par, &buf[0], 1, 0);
> > > +                     if (ret < 0)
> > > +                             goto out_free;
> > > +
> > > +                     if (i > 1) {
> > > +                             ret = fbtft_write_buf_dc(par, &buf[1], i - 1, 1);
> > > +                             if (ret < 0)
> > > +                                     goto out_free;
> > > +                     }
> >
> > I believe this is incorrect change and has not to be applied. write !=
> > write_register. Without any evidence of testing, definite NAK to it.
> > Otherwise, please provide detailed testing pattern and which devices were
> > tested.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/4] staging: fbtft: core: avoid large stack usage in DT init parsing

[sun jian]

On Tue, Jan 6, 2026 at 2:15 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, Jan 06, 2026 at 01:00:33AM +0800, sun jian wrote:
>
> > Thanks for the feedback.
>
> You're welcome, but please, do not top-post!
Sorry about that - I'll use inline replies.
>

> How can you test without hardware at hand?
>
>
> I already explained in the response to the cover letter. Please, read it.
>
Given that， I will drop all the changes.

Thanks,
Sun Jian

[PATCH v1 2/4] staging: fbtft: ssd1351: send gamma table via fbtft_write_buf_dc()

[Sun Jian]

Clang reports a large stack frame in set_gamma() (-Wframe-larger-than=1024)
due to the large write_reg() call emitting 63 gamma bytes via varargs.

Send the command byte (0xB8) and the gamma payload using
fbtft_write_buf_dc() to avoid the varargs/NUMARGS stack blow-up.

No functional change intended.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 drivers/staging/fbtft/fb_ssd1351.c | 35 +++++++++++++-----------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/fbtft/fb_ssd1351.c b/drivers/staging/fbtft/fb_ssd1351.c
index 6736b09b2f45..b4ab2c81e528 100644
--- a/drivers/staging/fbtft/fb_ssd1351.c
+++ b/drivers/staging/fbtft/fb_ssd1351.c
@@ -119,43 +119,38 @@ static int set_var(struct fbtft_par *par)
  */
 static int set_gamma(struct fbtft_par *par, u32 *curves)
 {
-	unsigned long tmp[GAMMA_NUM * GAMMA_LEN];
+	u8 data[GAMMA_LEN];
+	u8 cmd = 0xB8;
 	int i, acc = 0;
+	int ret;
 
-	for (i = 0; i < 63; i++) {
+	for (i = 0; i < GAMMA_LEN; i++) {
 		if (i > 0 && curves[i] < 2) {
 			dev_err(par->info->device,
 				"Illegal value in Grayscale Lookup Table at index %d : %d. Must be greater than 1\n",
 				i, curves[i]);
 			return -EINVAL;
 		}
+
 		acc += curves[i];
-		tmp[i] = acc;
+
 		if (acc > 180) {
 			dev_err(par->info->device,
 				"Illegal value(s) in Grayscale Lookup Table. At index=%d : %d, the accumulated value has exceeded 180\n",
 				i, acc);
 			return -EINVAL;
 		}
+
+		data[i] = acc;
 	}
 
-	write_reg(par, 0xB8,
-		  tmp[0],  tmp[1],  tmp[2],  tmp[3],
-		  tmp[4],  tmp[5],  tmp[6],  tmp[7],
-		  tmp[8],  tmp[9],  tmp[10], tmp[11],
-		  tmp[12], tmp[13], tmp[14], tmp[15],
-		  tmp[16], tmp[17], tmp[18], tmp[19],
-		  tmp[20], tmp[21], tmp[22], tmp[23],
-		  tmp[24], tmp[25], tmp[26], tmp[27],
-		  tmp[28], tmp[29], tmp[30], tmp[31],
-		  tmp[32], tmp[33], tmp[34], tmp[35],
-		  tmp[36], tmp[37], tmp[38], tmp[39],
-		  tmp[40], tmp[41], tmp[42], tmp[43],
-		  tmp[44], tmp[45], tmp[46], tmp[47],
-		  tmp[48], tmp[49], tmp[50], tmp[51],
-		  tmp[52], tmp[53], tmp[54], tmp[55],
-		  tmp[56], tmp[57], tmp[58], tmp[59],
-		  tmp[60], tmp[61], tmp[62]);
+	ret = fbtft_write_buf_dc(par, &cmd, 1, 0);
+	if (ret < 0)
+		return ret;
+
+	ret = fbtft_write_buf_dc(par, data, sizeof(data), 1);
+	if (ret < 0)
+		return ret;
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH v1 2/4] staging: fbtft: ssd1351: send gamma table via fbtft_write_buf_dc()

[Dan Carpenter]

On Sun, Jan 04, 2026 at 07:06:36PM +0800, Sun Jian wrote:
> Clang reports a large stack frame in set_gamma() (-Wframe-larger-than=1024)
> due to the large write_reg() call emitting 63 gamma bytes via varargs.
> 
> Send the command byte (0xB8) and the gamma payload using
> fbtft_write_buf_dc() to avoid the varargs/NUMARGS stack blow-up.
> 
> No functional change intended.
> 
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>  drivers/staging/fbtft/fb_ssd1351.c | 35 +++++++++++++-----------------
>  1 file changed, 15 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/staging/fbtft/fb_ssd1351.c b/drivers/staging/fbtft/fb_ssd1351.c
> index 6736b09b2f45..b4ab2c81e528 100644
> --- a/drivers/staging/fbtft/fb_ssd1351.c
> +++ b/drivers/staging/fbtft/fb_ssd1351.c
> @@ -119,43 +119,38 @@ static int set_var(struct fbtft_par *par)
>   */
>  static int set_gamma(struct fbtft_par *par, u32 *curves)
>  {
> -	unsigned long tmp[GAMMA_NUM * GAMMA_LEN];
> +	u8 data[GAMMA_LEN];

Ugh...  GAMMA_NUM is 1 so this is an annoying calculation.  So what
this does is it changes the type from unsigned long to u8 and renames
the variable.  I am fine with renaming the variable it's unrelated and
makes the review harder.

> +	u8 cmd = 0xB8;
>  	int i, acc = 0;
> +	int ret;
>  
> -	for (i = 0; i < 63; i++) {
> +	for (i = 0; i < GAMMA_LEN; i++) {

GAMMA_LEN is 63.  So this looks like a change, but it's an unrelated
cleanup.

>  		if (i > 0 && curves[i] < 2) {
>  			dev_err(par->info->device,
>  				"Illegal value in Grayscale Lookup Table at index %d : %d. Must be greater than 1\n",
>  				i, curves[i]);
>  			return -EINVAL;
>  		}
> +

This is an unrelated white space change.

>  		acc += curves[i];
> -		tmp[i] = acc;
> +
>  		if (acc > 180) {
>  			dev_err(par->info->device,
>  				"Illegal value(s) in Grayscale Lookup Table. At index=%d : %d, the accumulated value has exceeded 180\n",
>  				i, acc);
>  			return -EINVAL;
>  		}
> +
> +		data[i] = acc;

Here we move the acc assignment after the sanity check, but it's just
an unrelated cleanup.

>  	}
>  
> -	write_reg(par, 0xB8,
> -		  tmp[0],  tmp[1],  tmp[2],  tmp[3],
> -		  tmp[4],  tmp[5],  tmp[6],  tmp[7],
> -		  tmp[8],  tmp[9],  tmp[10], tmp[11],
> -		  tmp[12], tmp[13], tmp[14], tmp[15],
> -		  tmp[16], tmp[17], tmp[18], tmp[19],
> -		  tmp[20], tmp[21], tmp[22], tmp[23],
> -		  tmp[24], tmp[25], tmp[26], tmp[27],
> -		  tmp[28], tmp[29], tmp[30], tmp[31],
> -		  tmp[32], tmp[33], tmp[34], tmp[35],
> -		  tmp[36], tmp[37], tmp[38], tmp[39],
> -		  tmp[40], tmp[41], tmp[42], tmp[43],
> -		  tmp[44], tmp[45], tmp[46], tmp[47],
> -		  tmp[48], tmp[49], tmp[50], tmp[51],
> -		  tmp[52], tmp[53], tmp[54], tmp[55],
> -		  tmp[56], tmp[57], tmp[58], tmp[59],
> -		  tmp[60], tmp[61], tmp[62]);
> +	ret = fbtft_write_buf_dc(par, &cmd, 1, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = fbtft_write_buf_dc(par, data, sizeof(data), 1);
> +	if (ret < 0)
> +		return ret;

These are good changes.  Just change the type from unsigned long to u8
and use fbtft_write_buf_dc() instead of write_reg().  Then do the other
changes in a separate patch.

Same for the other patches.

regards,
dan carpenter

>  
>  	return 0;
>  }
> -- 
> 2.43.0
> 

Re: [PATCH v1 2/4] staging: fbtft: ssd1351: send gamma table via fbtft_write_buf_dc()

[sun jian]

Hi Dan,

Thanks. Agreed.

For v2 I’ll keep this patch minimal: only switch from write_reg() varargs
to fbtft_write_buf_dc() and reduce the stack usage, without other changes.

Any follow-up cleanups (if needed) will be sent as a separate patch.

Thanks,
Sun Jian

On Mon, Jan 5, 2026 at 10:39 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Sun, Jan 04, 2026 at 07:06:36PM +0800, Sun Jian wrote:
> > Clang reports a large stack frame in set_gamma() (-Wframe-larger-than=1024)
> > due to the large write_reg() call emitting 63 gamma bytes via varargs.
> >
> > Send the command byte (0xB8) and the gamma payload using
> > fbtft_write_buf_dc() to avoid the varargs/NUMARGS stack blow-up.
> >
> > No functional change intended.
> >
> > Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> > ---
> >  drivers/staging/fbtft/fb_ssd1351.c | 35 +++++++++++++-----------------
> >  1 file changed, 15 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/staging/fbtft/fb_ssd1351.c b/drivers/staging/fbtft/fb_ssd1351.c
> > index 6736b09b2f45..b4ab2c81e528 100644
> > --- a/drivers/staging/fbtft/fb_ssd1351.c
> > +++ b/drivers/staging/fbtft/fb_ssd1351.c
> > @@ -119,43 +119,38 @@ static int set_var(struct fbtft_par *par)
> >   */
> >  static int set_gamma(struct fbtft_par *par, u32 *curves)
> >  {
> > -     unsigned long tmp[GAMMA_NUM * GAMMA_LEN];
> > +     u8 data[GAMMA_LEN];
>
> Ugh...  GAMMA_NUM is 1 so this is an annoying calculation.  So what
> this does is it changes the type from unsigned long to u8 and renames
> the variable.  I am fine with renaming the variable it's unrelated and
> makes the review harder.
>
> > +     u8 cmd = 0xB8;
> >       int i, acc = 0;
> > +     int ret;
> >
> > -     for (i = 0; i < 63; i++) {
> > +     for (i = 0; i < GAMMA_LEN; i++) {
>
> GAMMA_LEN is 63.  So this looks like a change, but it's an unrelated
> cleanup.
>
> >               if (i > 0 && curves[i] < 2) {
> >                       dev_err(par->info->device,
> >                               "Illegal value in Grayscale Lookup Table at index %d : %d. Must be greater than 1\n",
> >                               i, curves[i]);
> >                       return -EINVAL;
> >               }
> > +
>
> This is an unrelated white space change.
>
> >               acc += curves[i];
> > -             tmp[i] = acc;
> > +
> >               if (acc > 180) {
> >                       dev_err(par->info->device,
> >                               "Illegal value(s) in Grayscale Lookup Table. At index=%d : %d, the accumulated value has exceeded 180\n",
> >                               i, acc);
> >                       return -EINVAL;
> >               }
> > +
> > +             data[i] = acc;
>
> Here we move the acc assignment after the sanity check, but it's just
> an unrelated cleanup.
>
> >       }
> >
> > -     write_reg(par, 0xB8,
> > -               tmp[0],  tmp[1],  tmp[2],  tmp[3],
> > -               tmp[4],  tmp[5],  tmp[6],  tmp[7],
> > -               tmp[8],  tmp[9],  tmp[10], tmp[11],
> > -               tmp[12], tmp[13], tmp[14], tmp[15],
> > -               tmp[16], tmp[17], tmp[18], tmp[19],
> > -               tmp[20], tmp[21], tmp[22], tmp[23],
> > -               tmp[24], tmp[25], tmp[26], tmp[27],
> > -               tmp[28], tmp[29], tmp[30], tmp[31],
> > -               tmp[32], tmp[33], tmp[34], tmp[35],
> > -               tmp[36], tmp[37], tmp[38], tmp[39],
> > -               tmp[40], tmp[41], tmp[42], tmp[43],
> > -               tmp[44], tmp[45], tmp[46], tmp[47],
> > -               tmp[48], tmp[49], tmp[50], tmp[51],
> > -               tmp[52], tmp[53], tmp[54], tmp[55],
> > -               tmp[56], tmp[57], tmp[58], tmp[59],
> > -               tmp[60], tmp[61], tmp[62]);
> > +     ret = fbtft_write_buf_dc(par, &cmd, 1, 0);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     ret = fbtft_write_buf_dc(par, data, sizeof(data), 1);
> > +     if (ret < 0)
> > +             return ret;
>
> These are good changes.  Just change the type from unsigned long to u8
> and use fbtft_write_buf_dc() instead of write_reg().  Then do the other
> changes in a separate patch.
>
> Same for the other patches.
>
> regards,
> dan carpenter
>
> >
> >       return 0;
> >  }
> > --
> > 2.43.0
> >

[PATCH v1 3/4] staging: fbtft: ssd1331: send gamma table via fbtft_write_buf_dc()

[Sun Jian]

Clang reports a large stack frame in set_gamma() (-Wframe-larger-than=1024)
because write_reg() expands into a large varargs call and triggers
NUMARGS((int[]){...}) stack allocation.

Use fbtft_write_buf_dc() to send the command byte (0xB8) and the gamma
payload as a buffer.

No functional change intended.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 drivers/staging/fbtft/fb_ssd1331.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/fbtft/fb_ssd1331.c b/drivers/staging/fbtft/fb_ssd1331.c
index 06b7056d6c71..cbe10f191f5b 100644
--- a/drivers/staging/fbtft/fb_ssd1331.c
+++ b/drivers/staging/fbtft/fb_ssd1331.c
@@ -130,37 +130,38 @@ static void write_reg8_bus8(struct fbtft_par *par, int len, ...)
  */
 static int set_gamma(struct fbtft_par *par, u32 *curves)
 {
-	unsigned long tmp[GAMMA_NUM * GAMMA_LEN];
+	u8 data[GAMMA_LEN];
+	u8 cmd = 0xB8;
 	int i, acc = 0;
+	int ret;
 
-	for (i = 0; i < 63; i++) {
+	for (i = 0; i < GAMMA_LEN; i++) {
 		if (i > 0 && curves[i] < 2) {
 			dev_err(par->info->device,
 				"Illegal value in Grayscale Lookup Table at index %d. Must be greater than 1\n",
 				i);
 			return -EINVAL;
 		}
+
 		acc += curves[i];
-		tmp[i] = acc;
+
 		if (acc > 180) {
 			dev_err(par->info->device,
 				"Illegal value(s) in Grayscale Lookup Table. At index=%d, the accumulated value has exceeded 180\n",
 				i);
 			return -EINVAL;
 		}
+
+		data[i] = acc;
 	}
 
-	write_reg(par, 0xB8,
-		  tmp[0], tmp[1], tmp[2], tmp[3], tmp[4], tmp[5], tmp[6],
-		  tmp[7], tmp[8], tmp[9], tmp[10], tmp[11], tmp[12], tmp[13],
-		  tmp[14], tmp[15], tmp[16], tmp[17], tmp[18], tmp[19], tmp[20],
-		  tmp[21], tmp[22], tmp[23], tmp[24], tmp[25], tmp[26],	tmp[27],
-		  tmp[28], tmp[29], tmp[30], tmp[31], tmp[32], tmp[33], tmp[34],
-		  tmp[35], tmp[36], tmp[37], tmp[38], tmp[39], tmp[40], tmp[41],
-		  tmp[42], tmp[43], tmp[44], tmp[45], tmp[46], tmp[47], tmp[48],
-		  tmp[49], tmp[50], tmp[51], tmp[52], tmp[53], tmp[54], tmp[55],
-		  tmp[56], tmp[57], tmp[58], tmp[59], tmp[60], tmp[61],
-		  tmp[62]);
+	ret = fbtft_write_buf_dc(par, &cmd, 1, 0);
+	if (ret < 0)
+		return ret;
+
+	ret = fbtft_write_buf_dc(par, data, sizeof(data), 1);
+	if (ret < 0)
+		return ret;
 
 	return 0;
 }
-- 
2.43.0

[PATCH v1 4/4] staging: fbtft: hx8353d: send LUT via buffer to reduce stack usage

[Sun Jian]

Clang reports a large stack frame in init_display()
 (-Wframe-larger-than=1024) due to the very large
 write_reg(MIPI_DCS_WRITE_LUT, ...) call.

Send MIPI_DCS_WRITE_LUT followed by the LUT payload using
fbtft_write_buf_dc() to avoid the varargs/NUMARGS stack blow-up.

No functional change intended.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 drivers/staging/fbtft/fb_hx8353d.c | 38 ++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/fbtft/fb_hx8353d.c b/drivers/staging/fbtft/fb_hx8353d.c
index 3e73b69b6a27..7e667d60792f 100644
--- a/drivers/staging/fbtft/fb_hx8353d.c
+++ b/drivers/staging/fbtft/fb_hx8353d.c
@@ -17,6 +17,21 @@
 #define DRVNAME "fb_hx8353d"
 #define DEFAULT_GAMMA "50 77 40 08 BF 00 03 0F 00 01 73 00 72 03 B0 0F 08 00 0F"
 
+static const u8 lut[] = {
+			  0,  2,  4,  6,  8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
+			 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62,
+			  0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
+			 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
+			 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
+			 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
+			  0,  2,  4,  6,  8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
+			 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62,
+			  0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
+			 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
+			 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
+			 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
+		};
+
 static int init_display(struct fbtft_par *par)
 {
 	par->fbtftops.reset(par);
@@ -48,18 +63,21 @@ static int init_display(struct fbtft_par *par)
 	write_reg(par, MIPI_DCS_SET_DISPLAY_ON);
 
 	/* RGBSET */
-	write_reg(par, MIPI_DCS_WRITE_LUT,
-		  0,  2,  4,  6,  8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
-		32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62,
-		 0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
-		16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
-		32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
-		48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
-		 0,  2,  4,  6,  8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
-		32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62);
+	{
+		u8 cmd = MIPI_DCS_WRITE_LUT;
+		int ret;
+
+		ret = fbtft_write_buf_dc(par, &cmd, 1, 0);
+		if (ret < 0)
+			return ret;
+
+		ret = fbtft_write_buf_dc(par, (void *)lut, sizeof(lut), 1);
+		if (ret < 0)
+			return ret;
+	}
 
 	return 0;
-};
+}
 
 static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
 {
-- 
2.43.0

Re: [PATCH v1 4/4] staging: fbtft: hx8353d: send LUT via buffer to reduce stack usage

[Andy Shevchenko]

On Sun, Jan 04, 2026 at 07:06:38PM +0800, Sun Jian wrote:
> Clang reports a large stack frame in init_display()
>  (-Wframe-larger-than=1024) due to the very large
>  write_reg(MIPI_DCS_WRITE_LUT, ...) call.
> 
> Send MIPI_DCS_WRITE_LUT followed by the LUT payload using
> fbtft_write_buf_dc() to avoid the varargs/NUMARGS stack blow-up.
> 
> No functional change intended.

...

> +static const u8 lut[] = {
> +			  0,  2,  4,  6,  8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
> +			 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62,
> +			  0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
> +			 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
> +			 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
> +			 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
> +			  0,  2,  4,  6,  8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30,
> +			 32, 34, 36, 38, 40, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62,
> +			  0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
> +			 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
> +			 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
> +			 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,

Two tabs too many on each line.

> +		};
> +

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/4] staging: fbtft: reduce stack usage by avoiding large write_reg() varargs

[Andy Shevchenko]

On Sun, Jan 04, 2026 at 07:06:34PM +0800, Sun Jian wrote:

> This series fixes clang `-Wframe-larger-than=1024` warnings in the fbtft
> staging drivers.
> 
> The warnings are triggered by very large `write_reg()`/`write_register()`
> varargs calls, which result in excessive stack usage.
> 
> Switch the affected paths to send a u8 command byte followed by the u8
> payload using `fbtft_write_buf_dc()`. The register values and ordering are
> kept unchanged; only the transfer method is updated.

Looking at the patches I think this is wrong. W.o. detailed test pattern
provided and the list of the devices, NAK.

If you want to address a warning without HW being accessible, perhaps you just
need a simple bump in the Makefile as an exception, however it's also doubtful
as it will hide a potential issue with the stack in the future.

-- 
With Best Regards,
Andy Shevchenko