| Message ID | 20261002-b4-framebuffer-stride-fix-v1-1-4a8498416b01@protonmail.com |
|---|---|
| State | New |
| Delegated to: | Anatolij Gustschin |
| Headers | show |
| Series | video: console: clear rows stride-aware in set_row | expand |
Hi Petr, On 2026-10-02T21:19:31, Petr Hodina via B4 Relay <devnull+petr.hodina.protonmail.com@kernel.org> wrote: > video: console: clear rows stride-aware in set_row > > console_set_row() and console_set_row_2() clear a row with a single > contiguous fill assuming line_length == xsize * pbytes. A stride-padded > framebuffer inherited from a prior boot stage (e.g. via > simple-framebuffer) breaks that assumption leaving stale pixels after > scroll. > > Step per scanline by line_length instead matching the glyph renderer. > > Signed-off-by: Petr Hodina <petr.hodina@protonmail.com> > > drivers/video/console_normal.c | 16 +++++++++------- > drivers/video/console_rotate.c | 16 +++++++++------- > 2 files changed, 18 insertions(+), 14 deletions(-) > diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c > @@ -29,11 +28,14 @@ static int console_set_row(struct udevice *dev, uint row, int clr) > + for (j = 0; j < fontdata->height; j++) { > + dst = line; > + for (i = 0; i < vid_priv->xsize; i++) > + fill_pixel_and_goto_next(&dst, clr, pbytes, pbytes); > + line += vid_priv->line_length; > + } The old loop writes height * xsize pixels contiguously, so with a padded stride it runs into the padding and stops short of the last scanlines of the row. BTW video_fill_part() already does a stride-aware fill and calls video_damage(), so this could become something like: return video_fill_part(dev->parent, 0, row * fontdata->height, vid_priv->xsize, (row + 1) * fontdata->height, clr); That avoids open-coding the same loop here and in console_set_row_2(). It returns -ENOSYS for an unsupported bpix, so the check_bpix_support() call could go too. What do you think? I am OK with the patch as it is, if you prefer. > diff --git a/drivers/video/console_rotate.c b/drivers/video/console_rotate.c > @@ -112,18 +112,20 @@ static int console_set_row_2(struct udevice *dev, uint row, int clr) > + void *start, *line, *dst; > + int i, j; > int pbytes = VNBYTES(vid_priv->bpix); > > start = vid_priv->fb + vid_priv->ysize * vid_priv->line_length - > (row + 1) * fontdata->height * vid_priv->line_length; > line = start; Since you are changing this, start is no longer needed. You could assign to line directly, as console_set_row() does. Reviewed-by: Simon Glass <sjg@chromium.org> Regards, Simon
diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c index 07db613ac53..f86cf24349d 100644 --- a/drivers/video/console_normal.c +++ b/drivers/video/console_normal.c @@ -18,10 +18,9 @@ static int console_set_row(struct udevice *dev, uint row, int clr) struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent); struct console_simple_priv *priv = dev_get_priv(dev); struct video_fontdata *fontdata = priv->fontdata; - void *line, *dst, *end; - int pixels = fontdata->height * vid_priv->xsize; + void *line, *dst; int ret; - int i; + int i, j; int pbytes; ret = check_bpix_support(vid_priv->bpix); @@ -29,11 +28,14 @@ static int console_set_row(struct udevice *dev, uint row, int clr) return ret; line = vid_priv->fb + row * fontdata->height * vid_priv->line_length; - dst = line; pbytes = VNBYTES(vid_priv->bpix); - for (i = 0; i < pixels; i++) - fill_pixel_and_goto_next(&dst, clr, pbytes, pbytes); - end = dst; + + for (j = 0; j < fontdata->height; j++) { + dst = line; + for (i = 0; i < vid_priv->xsize; i++) + fill_pixel_and_goto_next(&dst, clr, pbytes, pbytes); + line += vid_priv->line_length; + } video_damage(dev->parent, 0, diff --git a/drivers/video/console_rotate.c b/drivers/video/console_rotate.c index 886b25dcfaf..4423aded25a 100644 --- a/drivers/video/console_rotate.c +++ b/drivers/video/console_rotate.c @@ -112,18 +112,20 @@ static int console_set_row_2(struct udevice *dev, uint row, int clr) struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent); struct console_simple_priv *priv = dev_get_priv(dev); struct video_fontdata *fontdata = priv->fontdata; - void *start, *line, *dst, *end; - int pixels = fontdata->height * vid_priv->xsize; - int i; + void *start, *line, *dst; + int i, j; int pbytes = VNBYTES(vid_priv->bpix); start = vid_priv->fb + vid_priv->ysize * vid_priv->line_length - (row + 1) * fontdata->height * vid_priv->line_length; line = start; - dst = line; - for (i = 0; i < pixels; i++) - fill_pixel_and_goto_next(&dst, clr, pbytes, pbytes); - end = dst; + + for (j = 0; j < fontdata->height; j++) { + dst = line; + for (i = 0; i < vid_priv->xsize; i++) + fill_pixel_and_goto_next(&dst, clr, pbytes, pbytes); + line += vid_priv->line_length; + } video_damage(dev->parent, 0,