Hi Geert,
On Tue, Jul 20, 2021 at 09:43:22AM +0200, Geert Uytterhoeven wrote:
Hi Sam,
On Mon, Jul 19, 2021 at 9:05 PM Sam Ravnborg sam@ravnborg.org wrote:
On Wed, Jul 14, 2021 at 04:58:01PM +0200, Geert Uytterhoeven wrote:
Simplify the nested loops to handle conversion from linear frame buffer to ssd1307 page layout:
- Move last page handling one level up, as the value of "m" is the same inside a page,
- array->data[] is filled linearly, so there is no need to recalculate array_idx over and over again; a simple increment is sufficient.
Signed-off-by: Geert Uytterhoeven geert@linux-m68k.org
drivers/video/fbdev/ssd1307fb.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c index e6b6263e3bef847f..6d7bd025bca1a175 100644 --- a/drivers/video/fbdev/ssd1307fb.c +++ b/drivers/video/fbdev/ssd1307fb.c @@ -158,6 +158,7 @@ static int ssd1307fb_update_display(struct ssd1307fb_par *par) u8 *vmem = par->info->screen_buffer; unsigned int line_length = par->info->fix.line_length; unsigned int pages = DIV_ROUND_UP(par->height, 8);
u32 array_idx = 0; int ret, i, j, k; array = ssd1307fb_alloc_array(par->width * pages, SSD1307FB_DATA);@@ -194,19 +195,21 @@ static int ssd1307fb_update_display(struct ssd1307fb_par *par) */
for (i = 0; i < pages; i++) {
int m = 8;/* Last page may be partial */if (i + 1 == pages && par->height % 8)m = par->height % 8; for (j = 0; j < par->width; j++) {
int m = 8;u32 array_idx = i * par->width + j;array->data[array_idx] = 0;/* Last page may be partial */if (i + 1 == pages && par->height % 8)m = par->height % 8;
u8 data = 0;for (k = 0; k < m; k++) {If the last page is partial then m will be less than 8 for all bytes in j = 0..par-width - but m should only be less than 8 for the last iteration of the loop.
Do I miss something or is the code buggy?
"the loop" is the j-loop? If m is less than 8 for the last page, it should be less than 8 for all iterations of j, as all last bytes in each "line" (visible row) are partial, cfr. the comments above the code, explaining the representation of the screen.
OK, then the code works as intended. I had not read the comments and just assume it was only the last byte that was in need of a special treatment. So code is fine: Acked-by: Sam Ravnborg sam@ravnborg.org
u8 byte = vmem[(8 * i + k) * line_length + j / 8]; u8 bit = (byte >> (j % 8)) & 1;
array->data[array_idx] |= bit << k;
data |= bit << k; }array->data[array_idx++] = data; } }Gr{oetje,eeting}s,
Geert-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds