* [PATCH v2 01/10] fbcon: Avoid OOB font access if console rotation fails
From: Thomas Zimmermann @ 2026-04-07 9:23 UTC (permalink / raw)
To: deller, gregkh, jirislaby, geert, simona, sam
Cc: linux-fbdev, dri-devel, linux-kernel, linux-serial,
Thomas Zimmermann, stable
In-Reply-To: <20260407092555.58816-1-tzimmermann@suse.de>
Clear the font buffer if the reallocation during console rotation fails
in fbcon_rotate_font(). The putcs implementations for the rotated buffer
will return early in this case. See [1] for an example.
Currently, fbcon_rotate_font() keeps the old buffer, which is too small
for the rotated font. Printing to the rotated console with a high-enough
character code will overflow the font buffer.
v2:
- fix typos in commit message
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 6cc50e1c5b57 ("[PATCH] fbcon: Console Rotation - Add support to rotate font bitmap")
Cc: <stable@vger.kernel.org> # v2.6.15+
Link: https://elixir.bootlin.com/linux/v6.19/source/drivers/video/fbdev/core/fbcon_ccw.c#L144 # [1]
---
drivers/video/fbdev/core/fbcon_rotate.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
index 1562a8f20b4f..5348f6c6f57c 100644
--- a/drivers/video/fbdev/core/fbcon_rotate.c
+++ b/drivers/video/fbdev/core/fbcon_rotate.c
@@ -46,6 +46,10 @@ int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc)
info->fbops->fb_sync(info);
if (par->fd_size < d_cellsize * len) {
+ kfree(par->fontbuffer);
+ par->fontbuffer = NULL;
+ par->fd_size = 0;
+
dst = kmalloc_array(len, d_cellsize, GFP_KERNEL);
if (dst == NULL) {
@@ -54,7 +58,6 @@ int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc)
}
par->fd_size = d_cellsize * len;
- kfree(par->fontbuffer);
par->fontbuffer = dst;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 06/10] lib/fonts: Refactor glyph-pattern helpers
From: Thomas Zimmermann @ 2026-04-07 9:23 UTC (permalink / raw)
To: deller, gregkh, jirislaby, geert, simona, sam
Cc: linux-fbdev, dri-devel, linux-kernel, linux-serial,
Thomas Zimmermann
In-Reply-To: <20260407092555.58816-1-tzimmermann@suse.de>
Change the signatures of the pattern helpers to align them with other
font-glyph helpers: use the font_glyph_ prefix and pass the glyph
buffer first.
Calculating the position of the involved bit is somewhat obfuscated
in the original implementation. Move it into the new helper
__font_glyph_pos() and use the result as array index and bit position.
Note that these bit helpers use a bit pitch, while other code uses a
byte pitch. This is intentional and required here.
v2:
- fix typos in commit message
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
lib/fonts/font_rotate.c | 45 ++++++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/lib/fonts/font_rotate.c b/lib/fonts/font_rotate.c
index d107a8d0a2b0..d79ec5eef712 100644
--- a/lib/fonts/font_rotate.c
+++ b/lib/fonts/font_rotate.c
@@ -15,21 +15,33 @@
#include "font.h"
-static inline int pattern_test_bit(u32 x, u32 y, u32 pitch, const char *pat)
+static unsigned int __font_glyph_pos(unsigned int x, unsigned int y, unsigned int bit_pitch,
+ unsigned int *bit)
{
- u32 tmp = (y * pitch) + x, index = tmp / 8, bit = tmp % 8;
+ unsigned int off = y * bit_pitch + x;
+ unsigned int bit_shift = off % 8;
- pat += index;
- return (*pat) & (0x80 >> bit);
+ *bit = 0x80 >> bit_shift; /* MSB has position 0, LSB has position 7 */
+
+ return off / 8;
}
-static inline void pattern_set_bit(u32 x, u32 y, u32 pitch, char *pat)
+static bool font_glyph_test_bit(const unsigned char *glyph, unsigned int x, unsigned int y,
+ unsigned int bit_pitch)
{
- u32 tmp = (y * pitch) + x, index = tmp / 8, bit = tmp % 8;
+ unsigned int bit;
+ unsigned int i = __font_glyph_pos(x, y, bit_pitch, &bit);
+
+ return glyph[i] & bit;
+}
- pat += index;
+static void font_glyph_set_bit(unsigned char *glyph, unsigned int x, unsigned int y,
+ unsigned int bit_pitch)
+{
+ unsigned int bit;
+ unsigned int i = __font_glyph_pos(x, y, bit_pitch, &bit);
- (*pat) |= 0x80 >> bit;
+ glyph[i] |= bit;
}
static inline void rotate_cw(const char *in, char *out, u32 width, u32 height)
@@ -42,9 +54,8 @@ static inline void rotate_cw(const char *in, char *out, u32 width, u32 height)
for (i = 0; i < h; i++) {
for (j = 0; j < w; j++) {
- if (pattern_test_bit(j, i, width, in))
- pattern_set_bit(height - 1 - i - shift, j,
- height, out);
+ if (font_glyph_test_bit(in, j, i, width))
+ font_glyph_set_bit(out, height - 1 - i - shift, j, height);
}
}
}
@@ -81,10 +92,9 @@ static inline void rotate_ud(const char *in, char *out, u32 width, u32 height)
for (i = 0; i < height; i++) {
for (j = 0; j < width - shift; j++) {
- if (pattern_test_bit(j, i, width, in))
- pattern_set_bit(width - (1 + j + shift),
- height - (1 + i),
- width, out);
+ if (font_glyph_test_bit(in, j, i, width))
+ font_glyph_set_bit(out, width - (1 + j + shift),
+ height - (1 + i), width);
}
}
}
@@ -119,9 +129,8 @@ static inline void rotate_ccw(const char *in, char *out, u32 width, u32 height)
for (i = 0; i < h; i++) {
for (j = 0; j < w; j++) {
- if (pattern_test_bit(j, i, width, in))
- pattern_set_bit(i, width - 1 - j - shift,
- height, out);
+ if (font_glyph_test_bit(in, j, i, width))
+ font_glyph_set_bit(out, i, width - 1 - j - shift, height);
}
}
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 02/10] vt: Implement helpers for struct vc_font in source file
From: Thomas Zimmermann @ 2026-04-07 9:23 UTC (permalink / raw)
To: deller, gregkh, jirislaby, geert, simona, sam
Cc: linux-fbdev, dri-devel, linux-kernel, linux-serial,
Thomas Zimmermann
In-Reply-To: <20260407092555.58816-1-tzimmermann@suse.de>
Move the helpers vc_font_pitch() and vc_font_size() from the VT
header file into source file. They are not called very often, so
there's no benefit in keeping them in the headers. Also avoids
including <linux/math.h> from the header.
v2:
- fix typo in commit description
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/tty/vt/vt.c | 35 ++++++++++++++++++++++++++++++++++
include/linux/console_struct.h | 30 ++---------------------------
2 files changed, 37 insertions(+), 28 deletions(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index e2df99e3d458..3d89d30c9596 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -71,6 +71,7 @@
* by Adam Tla/lka <atlka@pg.gda.pl>, Aug 2006
*/
+#include <linux/math.h>
#include <linux/module.h>
#include <linux/types.h>
#include <linux/sched/signal.h>
@@ -230,6 +231,40 @@ enum {
blank_vesa_wait,
};
+/*
+ * struct vc_font
+ */
+
+/**
+ * vc_font_pitch - Calculates the number of bytes between two adjacent scanlines
+ * @font: The VC font
+ *
+ * Returns:
+ * The number of bytes between two adjacent scanlines in the font data
+ */
+unsigned int vc_font_pitch(const struct vc_font *font)
+{
+ return DIV_ROUND_UP(font->width, 8);
+}
+EXPORT_SYMBOL_GPL(vc_font_pitch);
+
+/**
+ * vc_font_size - Calculates the size of the font data in bytes
+ * @font: The VC font
+ *
+ * vc_font_size() calculates the number of bytes of font data in the
+ * font specified by @font. The function calculates the size from the
+ * font parameters.
+ *
+ * Returns:
+ * The size of the font data in bytes.
+ */
+unsigned int vc_font_size(const struct vc_font *font)
+{
+ return font->height * vc_font_pitch(font) * font->charcount;
+}
+EXPORT_SYMBOL_GPL(vc_font_size);
+
/*
* /sys/class/tty/tty0/
*
diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
index 6ce498b31855..fe915afdece5 100644
--- a/include/linux/console_struct.h
+++ b/include/linux/console_struct.h
@@ -13,7 +13,6 @@
#ifndef _LINUX_CONSOLE_STRUCT_H
#define _LINUX_CONSOLE_STRUCT_H
-#include <linux/math.h>
#include <linux/vt.h>
#include <linux/wait.h>
#include <linux/workqueue.h>
@@ -83,33 +82,8 @@ struct vc_font {
const unsigned char *data;
};
-/**
- * vc_font_pitch - Calculates the number of bytes between two adjacent scanlines
- * @font: The VC font
- *
- * Returns:
- * The number of bytes between two adjacent scanlines in the font data
- */
-static inline unsigned int vc_font_pitch(const struct vc_font *font)
-{
- return DIV_ROUND_UP(font->width, 8);
-}
-
-/**
- * vc_font_size - Calculates the size of the font data in bytes
- * @font: The VC font
- *
- * vc_font_size() calculates the number of bytes of font data in the
- * font specified by @font. The function calculates the size from the
- * font parameters.
- *
- * Returns:
- * The size of the font data in bytes.
- */
-static inline unsigned int vc_font_size(const struct vc_font *font)
-{
- return font->height * vc_font_pitch(font) * font->charcount;
-}
+unsigned int vc_font_pitch(const struct vc_font *font);
+unsigned int vc_font_size(const struct vc_font *font);
/*
* Example: vc_data of a console that was scrolled 3 lines down.
--
2.53.0
^ permalink raw reply related
* [PATCH v2 03/10] lib/fonts: Provide helpers for calculating glyph pitch and size
From: Thomas Zimmermann @ 2026-04-07 9:23 UTC (permalink / raw)
To: deller, gregkh, jirislaby, geert, simona, sam
Cc: linux-fbdev, dri-devel, linux-kernel, linux-serial,
Thomas Zimmermann
In-Reply-To: <20260407092555.58816-1-tzimmermann@suse.de>
Implement pitch and size calculation for a single font glyph in the
new helpers font_glyph_pitch() and font_glyph_size(). Replace the
instances where the calculations are open-coded.
Note that in the case of fbcon console rotation, the parameters for
a glyph's width and height might be reversed. This is intentional.
v2:
- fix typos in commit message
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/tty/vt/vt.c | 5 ++--
drivers/video/fbdev/core/fbcon_ccw.c | 11 +++----
drivers/video/fbdev/core/fbcon_cw.c | 11 +++----
drivers/video/fbdev/core/fbcon_rotate.c | 6 ++--
drivers/video/fbdev/core/fbcon_ud.c | 7 +++--
include/linux/font.h | 40 +++++++++++++++++++++++++
lib/fonts/fonts.c | 2 +-
7 files changed, 61 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 3d89d30c9596..23b9683b52a5 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -71,7 +71,6 @@
* by Adam Tla/lka <atlka@pg.gda.pl>, Aug 2006
*/
-#include <linux/math.h>
#include <linux/module.h>
#include <linux/types.h>
#include <linux/sched/signal.h>
@@ -244,7 +243,7 @@ enum {
*/
unsigned int vc_font_pitch(const struct vc_font *font)
{
- return DIV_ROUND_UP(font->width, 8);
+ return font_glyph_pitch(font->width);
}
EXPORT_SYMBOL_GPL(vc_font_pitch);
@@ -261,7 +260,7 @@ EXPORT_SYMBOL_GPL(vc_font_pitch);
*/
unsigned int vc_font_size(const struct vc_font *font)
{
- return font->height * vc_font_pitch(font) * font->charcount;
+ return font_glyph_size(font->width, font->height) * font->charcount;
}
EXPORT_SYMBOL_GPL(vc_font_size);
diff --git a/drivers/video/fbdev/core/fbcon_ccw.c b/drivers/video/fbdev/core/fbcon_ccw.c
index 2f394b5a17f7..96ef449ee6ac 100644
--- a/drivers/video/fbdev/core/fbcon_ccw.c
+++ b/drivers/video/fbdev/core/fbcon_ccw.c
@@ -26,7 +26,7 @@ static void ccw_update_attr(u8 *dst, u8 *src, int attribute,
struct vc_data *vc)
{
int i, j, offset = (vc->vc_font.height < 10) ? 1 : 2;
- int width = (vc->vc_font.height + 7) >> 3;
+ int width = font_glyph_pitch(vc->vc_font.height);
int mod = vc->vc_font.height % 8;
u8 c, msk = ~(0xff << offset), msk1 = 0;
@@ -101,7 +101,7 @@ static inline void ccw_putcs_aligned(struct vc_data *vc, struct fb_info *info,
{
struct fbcon_par *par = info->fbcon_par;
u16 charmask = vc->vc_hi_font_mask ? 0x1ff : 0xff;
- u32 idx = (vc->vc_font.height + 7) >> 3;
+ u32 idx = font_glyph_pitch(vc->vc_font.height);
u8 *src;
while (cnt--) {
@@ -131,7 +131,7 @@ static void ccw_putcs(struct vc_data *vc, struct fb_info *info,
{
struct fb_image image;
struct fbcon_par *par = info->fbcon_par;
- u32 width = (vc->vc_font.height + 7)/8;
+ u32 width = font_glyph_pitch(vc->vc_font.height);
u32 cellsize = width * vc->vc_font.width;
u32 maxcnt = info->pixmap.size/cellsize;
u32 scan_align = info->pixmap.scan_align - 1;
@@ -223,7 +223,8 @@ static void ccw_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
struct fb_cursor cursor;
struct fbcon_par *par = info->fbcon_par;
unsigned short charmask = vc->vc_hi_font_mask ? 0x1ff : 0xff;
- int w = (vc->vc_font.height + 7) >> 3, c;
+ int w = font_glyph_pitch(vc->vc_font.height);
+ int c;
int y = real_y(par->p, vc->state.y);
int attribute, use_sw = vc->vc_cursor_type & CUR_SW;
int err = 1, dx, dy;
@@ -297,7 +298,7 @@ static void ccw_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
char *tmp, *mask = kmalloc_array(w, vc->vc_font.width,
GFP_ATOMIC);
int cur_height, size, i = 0;
- int width = (vc->vc_font.width + 7)/8;
+ int width = font_glyph_pitch(vc->vc_font.width);
if (!mask)
return;
diff --git a/drivers/video/fbdev/core/fbcon_cw.c b/drivers/video/fbdev/core/fbcon_cw.c
index 3c3ad3471ec4..ea712654edae 100644
--- a/drivers/video/fbdev/core/fbcon_cw.c
+++ b/drivers/video/fbdev/core/fbcon_cw.c
@@ -26,7 +26,7 @@ static void cw_update_attr(u8 *dst, u8 *src, int attribute,
struct vc_data *vc)
{
int i, j, offset = (vc->vc_font.height < 10) ? 1 : 2;
- int width = (vc->vc_font.height + 7) >> 3;
+ int width = font_glyph_pitch(vc->vc_font.height);
u8 c, msk = ~(0xff >> offset);
for (i = 0; i < vc->vc_font.width; i++) {
@@ -86,7 +86,7 @@ static inline void cw_putcs_aligned(struct vc_data *vc, struct fb_info *info,
{
struct fbcon_par *par = info->fbcon_par;
u16 charmask = vc->vc_hi_font_mask ? 0x1ff : 0xff;
- u32 idx = (vc->vc_font.height + 7) >> 3;
+ u32 idx = font_glyph_pitch(vc->vc_font.height);
u8 *src;
while (cnt--) {
@@ -116,7 +116,7 @@ static void cw_putcs(struct vc_data *vc, struct fb_info *info,
{
struct fb_image image;
struct fbcon_par *par = info->fbcon_par;
- u32 width = (vc->vc_font.height + 7)/8;
+ u32 width = font_glyph_pitch(vc->vc_font.height);
u32 cellsize = width * vc->vc_font.width;
u32 maxcnt = info->pixmap.size/cellsize;
u32 scan_align = info->pixmap.scan_align - 1;
@@ -206,7 +206,8 @@ static void cw_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
struct fb_cursor cursor;
struct fbcon_par *par = info->fbcon_par;
unsigned short charmask = vc->vc_hi_font_mask ? 0x1ff : 0xff;
- int w = (vc->vc_font.height + 7) >> 3, c;
+ int w = font_glyph_pitch(vc->vc_font.height);
+ int c;
int y = real_y(par->p, vc->state.y);
int attribute, use_sw = vc->vc_cursor_type & CUR_SW;
int err = 1, dx, dy;
@@ -280,7 +281,7 @@ static void cw_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
char *tmp, *mask = kmalloc_array(w, vc->vc_font.width,
GFP_ATOMIC);
int cur_height, size, i = 0;
- int width = (vc->vc_font.width + 7)/8;
+ int width = font_glyph_pitch(vc->vc_font.width);
if (!mask)
return;
diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
index 5348f6c6f57c..18575c5182db 100644
--- a/drivers/video/fbdev/core/fbcon_rotate.c
+++ b/drivers/video/fbdev/core/fbcon_rotate.c
@@ -33,14 +33,12 @@ int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc)
src = par->fontdata = vc->vc_font.data;
par->cur_rotate = par->p->con_rotate;
len = vc->vc_font.charcount;
- s_cellsize = ((vc->vc_font.width + 7)/8) *
- vc->vc_font.height;
+ s_cellsize = font_glyph_size(vc->vc_font.width, vc->vc_font.height);
d_cellsize = s_cellsize;
if (par->rotate == FB_ROTATE_CW ||
par->rotate == FB_ROTATE_CCW)
- d_cellsize = ((vc->vc_font.height + 7)/8) *
- vc->vc_font.width;
+ d_cellsize = font_glyph_size(vc->vc_font.height, vc->vc_font.width);
if (info->fbops->fb_sync)
info->fbops->fb_sync(info);
diff --git a/drivers/video/fbdev/core/fbcon_ud.c b/drivers/video/fbdev/core/fbcon_ud.c
index 6fc30cad5b19..f7cd89c42b01 100644
--- a/drivers/video/fbdev/core/fbcon_ud.c
+++ b/drivers/video/fbdev/core/fbcon_ud.c
@@ -26,7 +26,7 @@ static void ud_update_attr(u8 *dst, u8 *src, int attribute,
struct vc_data *vc)
{
int i, offset = (vc->vc_font.height < 10) ? 1 : 2;
- int width = (vc->vc_font.width + 7) >> 3;
+ int width = font_glyph_pitch(vc->vc_font.width);
unsigned int cellsize = vc->vc_font.height * width;
u8 c;
@@ -153,7 +153,7 @@ static void ud_putcs(struct vc_data *vc, struct fb_info *info,
{
struct fb_image image;
struct fbcon_par *par = info->fbcon_par;
- u32 width = (vc->vc_font.width + 7)/8;
+ u32 width = font_glyph_pitch(vc->vc_font.width);
u32 cellsize = width * vc->vc_font.height;
u32 maxcnt = info->pixmap.size/cellsize;
u32 scan_align = info->pixmap.scan_align - 1;
@@ -253,7 +253,8 @@ static void ud_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
struct fb_cursor cursor;
struct fbcon_par *par = info->fbcon_par;
unsigned short charmask = vc->vc_hi_font_mask ? 0x1ff : 0xff;
- int w = (vc->vc_font.width + 7) >> 3, c;
+ int w = font_glyph_pitch(vc->vc_font.width);
+ int c;
int y = real_y(par->p, vc->state.y);
int attribute, use_sw = vc->vc_cursor_type & CUR_SW;
int err = 1, dx, dy;
diff --git a/include/linux/font.h b/include/linux/font.h
index 5401f07dd6ce..3bd49d914b22 100644
--- a/include/linux/font.h
+++ b/include/linux/font.h
@@ -11,10 +11,50 @@
#ifndef _VIDEO_FONT_H
#define _VIDEO_FONT_H
+#include <linux/math.h>
#include <linux/types.h>
struct console_font;
+/*
+ * Glyphs
+ */
+
+/**
+ * font_glyph_pitch - Calculates the number of bytes per scanline
+ * @width: The glyph width in bits per scanline
+ *
+ * A glyph's pitch is the number of bytes in a single scanline, rounded
+ * up to the next full byte. The parameter @width receives the number
+ * of visible bits per scanline. For example, if width is 14 bytes per
+ * scanline, the pitch is 2 bytes per scanline. If width is 8 bits per
+ * scanline, the pitch is 1 byte per scanline.
+ *
+ * Returns:
+ * The number of bytes in a single scanline of the glyph
+ */
+static inline unsigned int font_glyph_pitch(unsigned int width)
+{
+ return DIV_ROUND_UP(width, 8);
+}
+
+/**
+ * font_glyph_size - Calculates the number of bytes per glyph
+ * @width: The glyph width in bits per scanline
+ * @vpitch: The number of scanlines in the glyph
+ *
+ * The number of bytes in a glyph depends on the pitch and the number
+ * of scanlines. font_glyph_size automatically calculates the pitch
+ * from the given width. The parameter @vpitch gives the number of
+ * scanlines, which is usually the glyph's height in scanlines. Fonts
+ * coming from user space can sometimes have a different vertical pitch
+ * with empty scanlines between two adjacent glyphs.
+ */
+static inline unsigned int font_glyph_size(unsigned int width, unsigned int vpitch)
+{
+ return font_glyph_pitch(width) * vpitch;
+}
+
/*
* font_data_t and helpers
*/
diff --git a/lib/fonts/fonts.c b/lib/fonts/fonts.c
index 5938f542906b..f5d5333450a0 100644
--- a/lib/fonts/fonts.c
+++ b/lib/fonts/fonts.c
@@ -26,7 +26,7 @@
#include "font.h"
-#define console_font_pitch(font) DIV_ROUND_UP((font)->width, 8)
+#define console_font_pitch(font) font_glyph_pitch((font)->width)
/*
* Helpers for font_data_t
--
2.53.0
^ permalink raw reply related
* [PATCH v2 08/10] lib/fonts: Implement font rotation
From: Thomas Zimmermann @ 2026-04-07 9:23 UTC (permalink / raw)
To: deller, gregkh, jirislaby, geert, simona, sam
Cc: linux-fbdev, dri-devel, linux-kernel, linux-serial,
Thomas Zimmermann
In-Reply-To: <20260407092555.58816-1-tzimmermann@suse.de>
Move the core of fbcon's font-rotation code to the font library as
the new helper font_data_rotate(). The code can rotate in steps of
90°. For completeness, it also copies the glyph data for multiples
of 360°.
Bring back the memset optimization. A memset to 0 again clears the
whole glyph output buffer. Then use the internal rotation helpers on
the cleared output. Fbcon's original implementation worked like this,
but lost it during refactoring.
Replace fbcon's font-rotation code with the new implementations.
All that's left to do for fbcon is to maintain its internal fbcon
state.
v2:
- fix typos
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/video/fbdev/core/fbcon.h | 2 +-
drivers/video/fbdev/core/fbcon_rotate.c | 78 +++++-------------
include/linux/font.h | 3 +
lib/fonts/font_rotate.c | 103 ++++++++++++++++++++++++
4 files changed, 126 insertions(+), 60 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index 1e3c1ef84762..1793f34a6c84 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -86,7 +86,7 @@ struct fbcon_par {
const u8 *fontdata;
u8 *cursor_src;
u32 cursor_size;
- u32 fd_size;
+ size_t fd_size;
const struct fbcon_bitops *bitops;
};
diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
index 588dc9d6758a..74206f5a6e98 100644
--- a/drivers/video/fbdev/core/fbcon_rotate.c
+++ b/drivers/video/fbdev/core/fbcon_rotate.c
@@ -8,84 +8,44 @@
* more details.
*/
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/string.h>
+#include <linux/errno.h>
#include <linux/fb.h>
#include <linux/font.h>
-#include <linux/vt_kern.h>
-#include <linux/console.h>
-#include <asm/types.h>
+
#include "fbcon.h"
#include "fbcon_rotate.h"
int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc)
{
struct fbcon_par *par = info->fbcon_par;
- int len, err = 0;
- int s_cellsize, d_cellsize, i;
- const u8 *src;
- u8 *dst;
+ unsigned char *fontbuffer;
+ int ret;
if (vc->vc_font.data == par->fontdata &&
par->p->con_rotate == par->cur_rotate)
- goto finished;
+ return 0;
- src = par->fontdata = vc->vc_font.data;
+ par->fontdata = vc->vc_font.data;
par->cur_rotate = par->p->con_rotate;
- len = vc->vc_font.charcount;
- s_cellsize = font_glyph_size(vc->vc_font.width, vc->vc_font.height);
- d_cellsize = s_cellsize;
-
- if (par->rotate == FB_ROTATE_CW ||
- par->rotate == FB_ROTATE_CCW)
- d_cellsize = font_glyph_size(vc->vc_font.height, vc->vc_font.width);
if (info->fbops->fb_sync)
info->fbops->fb_sync(info);
- if (par->fd_size < d_cellsize * len) {
- kfree(par->fontbuffer);
- par->fontbuffer = NULL;
- par->fd_size = 0;
-
- dst = kmalloc_array(len, d_cellsize, GFP_KERNEL);
-
- if (dst == NULL) {
- err = -ENOMEM;
- goto finished;
- }
-
- par->fd_size = d_cellsize * len;
- par->fontbuffer = dst;
+ fontbuffer = font_data_rotate(par->p->fontdata, vc->vc_font.width,
+ vc->vc_font.height, vc->vc_font.charcount,
+ par->rotate, par->fontbuffer, &par->fd_size);
+ if (IS_ERR(fontbuffer)) {
+ ret = PTR_ERR(fontbuffer);
+ goto err_kfree;
}
- dst = par->fontbuffer;
+ par->fontbuffer = fontbuffer;
- switch (par->rotate) {
- case FB_ROTATE_UD:
- for (i = len; i--; ) {
- font_glyph_rotate_180(src, vc->vc_font.width, vc->vc_font.height, dst);
- src += s_cellsize;
- dst += d_cellsize;
- }
- break;
- case FB_ROTATE_CW:
- for (i = len; i--; ) {
- font_glyph_rotate_90(src, vc->vc_font.width, vc->vc_font.height, dst);
- src += s_cellsize;
- dst += d_cellsize;
- }
- break;
- case FB_ROTATE_CCW:
- for (i = len; i--; ) {
- font_glyph_rotate_270(src, vc->vc_font.width, vc->vc_font.height, dst);
- src += s_cellsize;
- dst += d_cellsize;
- }
- break;
- }
+ return 0;
+
+err_kfree:
+ kfree(par->fontbuffer);
+ par->fontbuffer = NULL; /* clear here to avoid output */
-finished:
- return err;
+ return ret;
}
diff --git a/include/linux/font.h b/include/linux/font.h
index 0a240dd70422..6845f02d739a 100644
--- a/include/linux/font.h
+++ b/include/linux/font.h
@@ -111,6 +111,9 @@ void font_glyph_rotate_180(const unsigned char *glyph, unsigned int width, unsig
unsigned char *out);
void font_glyph_rotate_270(const unsigned char *glyph, unsigned int width, unsigned int height,
unsigned char *out);
+unsigned char *font_data_rotate(font_data_t *fd, unsigned int width, unsigned int height,
+ unsigned int charcount, unsigned int steps,
+ unsigned char *buf, size_t *bufsize);
/*
* Font description
diff --git a/lib/fonts/font_rotate.c b/lib/fonts/font_rotate.c
index 09f6218e036f..065e0fc0667b 100644
--- a/lib/fonts/font_rotate.c
+++ b/lib/fonts/font_rotate.c
@@ -9,8 +9,11 @@
* more details.
*/
+#include <linux/errno.h>
#include <linux/export.h>
#include <linux/math.h>
+#include <linux/overflow.h>
+#include <linux/slab.h>
#include <linux/string.h>
#include "font.h"
@@ -170,3 +173,103 @@ void font_glyph_rotate_270(const unsigned char *glyph, unsigned int width, unsig
__font_glyph_rotate_270(glyph, width, height, out);
}
EXPORT_SYMBOL_GPL(font_glyph_rotate_270);
+
+/**
+ * font_data_rotate - Rotate font data by multiples of 90°
+ * @fd: The font data to rotate
+ * @width: The glyph width in bits per scanline
+ * @height: The number of scanlines in the glyph
+ * @charcount: The number of glyphs in the font
+ * @steps: Number of rotation steps of 90°
+ * @buf: Preallocated output buffer; can be NULL
+ * @bufsize: The size of @buf in bytes; can be NULL
+ *
+ * The parameters @width and @height refer to the visible number of pixels
+ * and scanlines in a single glyph. The number of glyphs is given in @charcount.
+ * Rotation happens in steps of 90°. The @steps parameter can have any value,
+ * but only 0 to 3 produce distinct results. With 4 or higher, a full rotation
+ * has been performed. You can pass any value for @steps and the helper will
+ * perform the appropriate rotation. Note that the returned buffer is not
+ * compatible with font_data_t. It only contains glyph data in the same format
+ * as returned by font_data_buf(). Callers are responsible to free the returned
+ * buffer with kfree(). Font rotation typically happens when displays get
+ * re-oriented. To avoid unnecessary re-allocation of the memory buffer, the
+ * caller can pass in an earlier result buffer in @buf for reuse. The old and
+ * new buffer sizes are given and retrieved by the caller in @bufsize. The
+ * allocation semantics are compatible with krealloc().
+ *
+ * Returns:
+ * A buffer with rotated glyphs on success, or an error pointer otherwise
+ */
+unsigned char *font_data_rotate(font_data_t *fd, unsigned int width, unsigned int height,
+ unsigned int charcount, unsigned int steps,
+ unsigned char *buf, size_t *bufsize)
+{
+ const unsigned char *src = font_data_buf(fd);
+ unsigned int s_cellsize = font_glyph_size(width, height);
+ unsigned int d_cellsize, i;
+ unsigned char *dst;
+ size_t size;
+
+ steps %= 4;
+
+ switch (steps) {
+ case 0:
+ case 2:
+ d_cellsize = s_cellsize;
+ break;
+ case 1:
+ case 3:
+ d_cellsize = font_glyph_size(height, width); /* flip width/height */
+ break;
+ }
+
+ if (check_mul_overflow(charcount, d_cellsize, &size))
+ return ERR_PTR(-EINVAL);
+
+ if (!buf || !bufsize || size > *bufsize) {
+ dst = kmalloc_array(charcount, d_cellsize, GFP_KERNEL);
+ if (!dst)
+ return ERR_PTR(-ENOMEM);
+
+ kfree(buf);
+ buf = dst;
+ if (bufsize)
+ *bufsize = size;
+ } else {
+ dst = buf;
+ }
+
+ switch (steps) {
+ case 0:
+ memcpy(dst, src, size);
+ break;
+ case 1:
+ memset(dst, 0, size);
+ for (i = 0; i < charcount; ++i) {
+ __font_glyph_rotate_90(src, width, height, dst);
+ src += s_cellsize;
+ dst += d_cellsize;
+ }
+ break;
+ case 2:
+ memset(dst, 0, size);
+ for (i = 0; i < charcount; ++i) {
+ __font_glyph_rotate_180(src, width, height, dst);
+ src += s_cellsize;
+ dst += d_cellsize;
+ }
+ break;
+ case 3:
+ memset(dst, 0, size);
+ for (i = 0; i < charcount; ++i) {
+ __font_glyph_rotate_270(src, width, height, dst);
+ src += s_cellsize;
+ dst += d_cellsize;
+ }
+ break;
+ }
+
+ return buf;
+}
+EXPORT_SYMBOL_GPL(font_data_rotate);
--
2.53.0
^ permalink raw reply related
* [PATCH v2 09/10] fbcon: Fill cursor mask in helper function
From: Thomas Zimmermann @ 2026-04-07 9:23 UTC (permalink / raw)
To: deller, gregkh, jirislaby, geert, simona, sam
Cc: linux-fbdev, dri-devel, linux-kernel, linux-serial,
Thomas Zimmermann
In-Reply-To: <20260407092555.58816-1-tzimmermann@suse.de>
Fbcon creates a cursor shape on the fly from the user-configured
settings. The logic to create a glyph with the cursor's bitmap mask
is duplicated in four places. In the cases that involve console
rotation, the implementation further rotates the cursor glyph for
displaying.
Consolidate all cursor-mask creation in a single helper. Update the
callers accordingly. For console rotation, use the glyph helpers to
rotate the created cursor glyph to the correct orientation.
v2:
- fix sparse truncated-bits warning
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/video/fbdev/core/bitblit.c | 35 ++-----------------
drivers/video/fbdev/core/fbcon.c | 40 ++++++++++++++++++++++
drivers/video/fbdev/core/fbcon.h | 2 ++
drivers/video/fbdev/core/fbcon_ccw.c | 51 ++++++----------------------
drivers/video/fbdev/core/fbcon_cw.c | 51 ++++++----------------------
drivers/video/fbdev/core/fbcon_ud.c | 50 +++++++--------------------
6 files changed, 78 insertions(+), 151 deletions(-)
diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c
index 7478accea8ec..65681dcc5930 100644
--- a/drivers/video/fbdev/core/bitblit.c
+++ b/drivers/video/fbdev/core/bitblit.c
@@ -329,46 +329,17 @@ static void bit_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
vc->vc_cursor_type != par->p->cursor_shape ||
par->cursor_state.mask == NULL ||
par->cursor_reset) {
- char *mask = kmalloc_array(w, vc->vc_font.height, GFP_ATOMIC);
- int cur_height, size, i = 0;
- u8 msk = 0xff;
+ unsigned char *mask = kmalloc_array(vc->vc_font.height, w, GFP_ATOMIC);
if (!mask)
return;
+ fbcon_fill_cursor_mask(par, vc, mask);
kfree(par->cursor_state.mask);
- par->cursor_state.mask = mask;
+ par->cursor_state.mask = (const char *)mask;
par->p->cursor_shape = vc->vc_cursor_type;
cursor.set |= FB_CUR_SETSHAPE;
-
- switch (CUR_SIZE(par->p->cursor_shape)) {
- case CUR_NONE:
- cur_height = 0;
- break;
- case CUR_UNDERLINE:
- cur_height = (vc->vc_font.height < 10) ? 1 : 2;
- break;
- case CUR_LOWER_THIRD:
- cur_height = vc->vc_font.height/3;
- break;
- case CUR_LOWER_HALF:
- cur_height = vc->vc_font.height >> 1;
- break;
- case CUR_TWO_THIRDS:
- cur_height = (vc->vc_font.height << 1)/3;
- break;
- case CUR_BLOCK:
- default:
- cur_height = vc->vc_font.height;
- break;
- }
- size = (vc->vc_font.height - cur_height) * w;
- while (size--)
- mask[i++] = ~msk;
- size = cur_height * w;
- while (size--)
- mask[i++] = msk;
}
par->cursor_state.enable = enable && !use_sw;
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 8641b0b3edc4..ff4c69e971f8 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -446,6 +446,46 @@ static void fbcon_del_cursor_work(struct fb_info *info)
cancel_delayed_work_sync(&par->cursor_work);
}
+void fbcon_fill_cursor_mask(struct fbcon_par *par, struct vc_data *vc, unsigned char *mask)
+{
+ static const unsigned int pattern = 0xffffffff;
+ unsigned int pitch = vc_font_pitch(&vc->vc_font);
+ unsigned int cur_height, size;
+
+ switch (CUR_SIZE(vc->vc_cursor_type)) {
+ case CUR_NONE:
+ cur_height = 0;
+ break;
+ case CUR_UNDERLINE:
+ if (vc->vc_font.height < 10)
+ cur_height = 1;
+ else
+ cur_height = 2;
+ break;
+ case CUR_LOWER_THIRD:
+ cur_height = vc->vc_font.height / 3;
+ break;
+ case CUR_LOWER_HALF:
+ cur_height = vc->vc_font.height / 2;
+ break;
+ case CUR_TWO_THIRDS:
+ cur_height = (vc->vc_font.height * 2) / 3;
+ break;
+ case CUR_BLOCK:
+ default:
+ cur_height = vc->vc_font.height;
+ break;
+ }
+
+ size = (vc->vc_font.height - cur_height) * pitch;
+ while (size--)
+ *mask++ = (unsigned char)~pattern;
+
+ size = cur_height * pitch;
+ while (size--)
+ *mask++ = (unsigned char)pattern;
+}
+
#ifndef MODULE
static int __init fb_console_setup(char *this_opt)
{
diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index 1793f34a6c84..bb0727b70631 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -192,6 +192,8 @@ extern void fbcon_set_tileops(struct vc_data *vc, struct fb_info *info);
extern void fbcon_set_bitops_ur(struct fbcon_par *par);
extern int soft_cursor(struct fb_info *info, struct fb_cursor *cursor);
+void fbcon_fill_cursor_mask(struct fbcon_par *par, struct vc_data *vc, unsigned char *mask);
+
#define FBCON_ATTRIBUTE_UNDERLINE 1
#define FBCON_ATTRIBUTE_REVERSE 2
#define FBCON_ATTRIBUTE_BOLD 4
diff --git a/drivers/video/fbdev/core/fbcon_ccw.c b/drivers/video/fbdev/core/fbcon_ccw.c
index 72453a2aaca8..723d9a33067f 100644
--- a/drivers/video/fbdev/core/fbcon_ccw.c
+++ b/drivers/video/fbdev/core/fbcon_ccw.c
@@ -296,57 +296,26 @@ static void ccw_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
vc->vc_cursor_type != par->p->cursor_shape ||
par->cursor_state.mask == NULL ||
par->cursor_reset) {
- char *tmp, *mask = kmalloc_array(w, vc->vc_font.width,
- GFP_ATOMIC);
- int cur_height, size, i = 0;
- int width = font_glyph_pitch(vc->vc_font.width);
+ unsigned char *tmp, *mask;
- if (!mask)
+ tmp = kmalloc_array(vc->vc_font.height, vc_font_pitch(&vc->vc_font), GFP_ATOMIC);
+ if (!tmp)
return;
+ fbcon_fill_cursor_mask(par, vc, tmp);
- tmp = kmalloc_array(width, vc->vc_font.height, GFP_ATOMIC);
-
- if (!tmp) {
- kfree(mask);
+ mask = kmalloc_array(vc->vc_font.width, w, GFP_ATOMIC);
+ if (!mask) {
+ kfree(tmp);
return;
}
+ font_glyph_rotate_270(tmp, vc->vc_font.width, vc->vc_font.height, mask);
+ kfree(tmp);
kfree(par->cursor_state.mask);
- par->cursor_state.mask = mask;
+ par->cursor_state.mask = (const char *)mask;
par->p->cursor_shape = vc->vc_cursor_type;
cursor.set |= FB_CUR_SETSHAPE;
-
- switch (CUR_SIZE(par->p->cursor_shape)) {
- case CUR_NONE:
- cur_height = 0;
- break;
- case CUR_UNDERLINE:
- cur_height = (vc->vc_font.height < 10) ? 1 : 2;
- break;
- case CUR_LOWER_THIRD:
- cur_height = vc->vc_font.height/3;
- break;
- case CUR_LOWER_HALF:
- cur_height = vc->vc_font.height >> 1;
- break;
- case CUR_TWO_THIRDS:
- cur_height = (vc->vc_font.height << 1)/3;
- break;
- case CUR_BLOCK:
- default:
- cur_height = vc->vc_font.height;
- break;
- }
-
- size = (vc->vc_font.height - cur_height) * width;
- while (size--)
- tmp[i++] = 0;
- size = cur_height * width;
- while (size--)
- tmp[i++] = 0xff;
- font_glyph_rotate_270(tmp, vc->vc_font.width, vc->vc_font.height, mask);
- kfree(tmp);
}
par->cursor_state.enable = enable && !use_sw;
diff --git a/drivers/video/fbdev/core/fbcon_cw.c b/drivers/video/fbdev/core/fbcon_cw.c
index 5690fc1d7854..732d093d462f 100644
--- a/drivers/video/fbdev/core/fbcon_cw.c
+++ b/drivers/video/fbdev/core/fbcon_cw.c
@@ -279,57 +279,26 @@ static void cw_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
vc->vc_cursor_type != par->p->cursor_shape ||
par->cursor_state.mask == NULL ||
par->cursor_reset) {
- char *tmp, *mask = kmalloc_array(w, vc->vc_font.width,
- GFP_ATOMIC);
- int cur_height, size, i = 0;
- int width = font_glyph_pitch(vc->vc_font.width);
+ unsigned char *tmp, *mask;
- if (!mask)
+ tmp = kmalloc_array(vc->vc_font.height, vc_font_pitch(&vc->vc_font), GFP_ATOMIC);
+ if (!tmp)
return;
+ fbcon_fill_cursor_mask(par, vc, tmp);
- tmp = kmalloc_array(width, vc->vc_font.height, GFP_ATOMIC);
-
- if (!tmp) {
- kfree(mask);
+ mask = kmalloc_array(vc->vc_font.width, w, GFP_ATOMIC);
+ if (!mask) {
+ kfree(tmp);
return;
}
+ font_glyph_rotate_90(tmp, vc->vc_font.width, vc->vc_font.height, mask);
+ kfree(tmp);
kfree(par->cursor_state.mask);
- par->cursor_state.mask = mask;
+ par->cursor_state.mask = (const char *)mask;
par->p->cursor_shape = vc->vc_cursor_type;
cursor.set |= FB_CUR_SETSHAPE;
-
- switch (CUR_SIZE(par->p->cursor_shape)) {
- case CUR_NONE:
- cur_height = 0;
- break;
- case CUR_UNDERLINE:
- cur_height = (vc->vc_font.height < 10) ? 1 : 2;
- break;
- case CUR_LOWER_THIRD:
- cur_height = vc->vc_font.height/3;
- break;
- case CUR_LOWER_HALF:
- cur_height = vc->vc_font.height >> 1;
- break;
- case CUR_TWO_THIRDS:
- cur_height = (vc->vc_font.height << 1)/3;
- break;
- case CUR_BLOCK:
- default:
- cur_height = vc->vc_font.height;
- break;
- }
-
- size = (vc->vc_font.height - cur_height) * width;
- while (size--)
- tmp[i++] = 0;
- size = cur_height * width;
- while (size--)
- tmp[i++] = 0xff;
- font_glyph_rotate_90(tmp, vc->vc_font.width, vc->vc_font.height, mask);
- kfree(tmp);
}
par->cursor_state.enable = enable && !use_sw;
diff --git a/drivers/video/fbdev/core/fbcon_ud.c b/drivers/video/fbdev/core/fbcon_ud.c
index f7cd89c42b01..a1981fa4701a 100644
--- a/drivers/video/fbdev/core/fbcon_ud.c
+++ b/drivers/video/fbdev/core/fbcon_ud.c
@@ -326,50 +326,26 @@ static void ud_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
vc->vc_cursor_type != par->p->cursor_shape ||
par->cursor_state.mask == NULL ||
par->cursor_reset) {
- char *mask = kmalloc_array(w, vc->vc_font.height, GFP_ATOMIC);
- int cur_height, size, i = 0;
- u8 msk = 0xff;
+ unsigned char *tmp, *mask;
- if (!mask)
+ tmp = kmalloc_array(vc->vc_font.height, w, GFP_ATOMIC);
+ if (!tmp)
return;
+ fbcon_fill_cursor_mask(par, vc, tmp);
+
+ mask = kmalloc_array(vc->vc_font.height, w, GFP_ATOMIC);
+ if (!mask) {
+ kfree(tmp);
+ return;
+ }
+ font_glyph_rotate_180(tmp, vc->vc_font.width, vc->vc_font.height, mask);
+ kfree(tmp);
kfree(par->cursor_state.mask);
- par->cursor_state.mask = mask;
+ par->cursor_state.mask = (const char *)mask;
par->p->cursor_shape = vc->vc_cursor_type;
cursor.set |= FB_CUR_SETSHAPE;
-
- switch (CUR_SIZE(par->p->cursor_shape)) {
- case CUR_NONE:
- cur_height = 0;
- break;
- case CUR_UNDERLINE:
- cur_height = (vc->vc_font.height < 10) ? 1 : 2;
- break;
- case CUR_LOWER_THIRD:
- cur_height = vc->vc_font.height/3;
- break;
- case CUR_LOWER_HALF:
- cur_height = vc->vc_font.height >> 1;
- break;
- case CUR_TWO_THIRDS:
- cur_height = (vc->vc_font.height << 1)/3;
- break;
- case CUR_BLOCK:
- default:
- cur_height = vc->vc_font.height;
- break;
- }
-
- size = cur_height * w;
-
- while (size--)
- mask[i++] = msk;
-
- size = (vc->vc_font.height - cur_height) * w;
-
- while (size--)
- mask[i++] = ~msk;
}
par->cursor_state.enable = enable && !use_sw;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 04/10] lib/fonts: Clean up Makefile
From: Thomas Zimmermann @ 2026-04-07 9:23 UTC (permalink / raw)
To: deller, gregkh, jirislaby, geert, simona, sam
Cc: linux-fbdev, dri-devel, linux-kernel, linux-serial,
Thomas Zimmermann
In-Reply-To: <20260407092555.58816-1-tzimmermann@suse.de>
Simplify the Makefile. Drop font-obj-y and sort the fonts by dictionary
order. Done in preparation for supporting optional font rotation.
v2:
- sort Makefile font entries by Family/Size in ascending order (Geert, Jiri)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
lib/fonts/Makefile | 35 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/lib/fonts/Makefile b/lib/fonts/Makefile
index 30a85a4292fa..b176af53d53e 100644
--- a/lib/fonts/Makefile
+++ b/lib/fonts/Makefile
@@ -1,23 +1,22 @@
# SPDX-License-Identifier: GPL-2.0
# Font handling
-font-objs := fonts.o
+font-y := fonts.o
-font-objs-$(CONFIG_FONT_SUN8x16) += font_sun8x16.o
-font-objs-$(CONFIG_FONT_SUN12x22) += font_sun12x22.o
-font-objs-$(CONFIG_FONT_8x8) += font_8x8.o
-font-objs-$(CONFIG_FONT_8x16) += font_8x16.o
-font-objs-$(CONFIG_FONT_6x11) += font_6x11.o
-font-objs-$(CONFIG_FONT_7x14) += font_7x14.o
-font-objs-$(CONFIG_FONT_10x18) += font_10x18.o
-font-objs-$(CONFIG_FONT_PEARL_8x8) += font_pearl_8x8.o
-font-objs-$(CONFIG_FONT_ACORN_8x8) += font_acorn_8x8.o
-font-objs-$(CONFIG_FONT_MINI_4x6) += font_mini_4x6.o
-font-objs-$(CONFIG_FONT_6x10) += font_6x10.o
-font-objs-$(CONFIG_FONT_TER10x18) += font_ter10x18.o
-font-objs-$(CONFIG_FONT_TER16x32) += font_ter16x32.o
-font-objs-$(CONFIG_FONT_6x8) += font_6x8.o
+# Built-in fonts; sorted by Family-Size in ascending order
+font-$(CONFIG_FONT_6x8) += font_6x8.o
+font-$(CONFIG_FONT_6x10) += font_6x10.o
+font-$(CONFIG_FONT_6x11) += font_6x11.o
+font-$(CONFIG_FONT_7x14) += font_7x14.o
+font-$(CONFIG_FONT_8x8) += font_8x8.o
+font-$(CONFIG_FONT_8x16) += font_8x16.o
+font-$(CONFIG_FONT_10x18) += font_10x18.o
+font-$(CONFIG_FONT_ACORN_8x8) += font_acorn_8x8.o
+font-$(CONFIG_FONT_MINI_4x6) += font_mini_4x6.o
+font-$(CONFIG_FONT_PEARL_8x8) += font_pearl_8x8.o
+font-$(CONFIG_FONT_SUN8x16) += font_sun8x16.o
+font-$(CONFIG_FONT_SUN12x22) += font_sun12x22.o
+font-$(CONFIG_FONT_TER10x18) += font_ter10x18.o
+font-$(CONFIG_FONT_TER16x32) += font_ter16x32.o
-font-objs += $(font-objs-y)
-
-obj-$(CONFIG_FONT_SUPPORT) += font.o
+obj-$(CONFIG_FONT_SUPPORT) += font.o
--
2.53.0
^ permalink raw reply related
* [PATCH v2 07/10] lib/fonts: Refactor glyph-rotation helpers
From: Thomas Zimmermann @ 2026-04-07 9:23 UTC (permalink / raw)
To: deller, gregkh, jirislaby, geert, simona, sam
Cc: linux-fbdev, dri-devel, linux-kernel, linux-serial,
Thomas Zimmermann
In-Reply-To: <20260407092555.58816-1-tzimmermann@suse.de>
Change the signatures of the glyph-rotation helpers to match their
public interfaces. Drop the inline qualifier.
Rename several variables to better match their meaning. Especially
rename variables to bit_pitch (or a variant thereof) if they store
a pitch value in bits per scanline. The original code is fairly
confusing about this. Move the calculation of the bit pitch into the
new helper font_glyph_bit_pitch().
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
lib/fonts/font_rotate.c | 85 ++++++++++++++++++++++++-----------------
1 file changed, 49 insertions(+), 36 deletions(-)
diff --git a/lib/fonts/font_rotate.c b/lib/fonts/font_rotate.c
index d79ec5eef712..09f6218e036f 100644
--- a/lib/fonts/font_rotate.c
+++ b/lib/fonts/font_rotate.c
@@ -15,6 +15,12 @@
#include "font.h"
+/* number of bits per line */
+static unsigned int font_glyph_bit_pitch(unsigned int width)
+{
+ return round_up(width, 8);
+}
+
static unsigned int __font_glyph_pos(unsigned int x, unsigned int y, unsigned int bit_pitch,
unsigned int *bit)
{
@@ -44,18 +50,21 @@ static void font_glyph_set_bit(unsigned char *glyph, unsigned int x, unsigned in
glyph[i] |= bit;
}
-static inline void rotate_cw(const char *in, char *out, u32 width, u32 height)
+static void __font_glyph_rotate_90(const unsigned char *glyph,
+ unsigned int width, unsigned int height,
+ unsigned char *out)
{
- int i, j, h = height, w = width;
- int shift = (8 - (height % 8)) & 7;
-
- width = (width + 7) & ~7;
- height = (height + 7) & ~7;
-
- for (i = 0; i < h; i++) {
- for (j = 0; j < w; j++) {
- if (font_glyph_test_bit(in, j, i, width))
- font_glyph_set_bit(out, height - 1 - i - shift, j, height);
+ unsigned int x, y;
+ unsigned int shift = (8 - (height % 8)) & 7;
+ unsigned int bit_pitch = font_glyph_bit_pitch(width);
+ unsigned int out_bit_pitch = font_glyph_bit_pitch(height);
+
+ for (y = 0; y < height; y++) {
+ for (x = 0; x < width; x++) {
+ if (font_glyph_test_bit(glyph, x, y, bit_pitch)) {
+ font_glyph_set_bit(out, out_bit_pitch - 1 - y - shift, x,
+ out_bit_pitch);
+ }
}
}
}
@@ -79,22 +88,24 @@ void font_glyph_rotate_90(const unsigned char *glyph, unsigned int width, unsign
{
memset(out, 0, font_glyph_size(height, width)); /* flip width/height */
- rotate_cw(glyph, out, width, height);
+ __font_glyph_rotate_90(glyph, width, height, out);
}
EXPORT_SYMBOL_GPL(font_glyph_rotate_90);
-static inline void rotate_ud(const char *in, char *out, u32 width, u32 height)
+static void __font_glyph_rotate_180(const unsigned char *glyph,
+ unsigned int width, unsigned int height,
+ unsigned char *out)
{
- int i, j;
- int shift = (8 - (width % 8)) & 7;
-
- width = (width + 7) & ~7;
-
- for (i = 0; i < height; i++) {
- for (j = 0; j < width - shift; j++) {
- if (font_glyph_test_bit(in, j, i, width))
- font_glyph_set_bit(out, width - (1 + j + shift),
- height - (1 + i), width);
+ unsigned int x, y;
+ unsigned int shift = (8 - (width % 8)) & 7;
+ unsigned int bit_pitch = font_glyph_bit_pitch(width);
+
+ for (y = 0; y < height; y++) {
+ for (x = 0; x < width; x++) {
+ if (font_glyph_test_bit(glyph, x, y, bit_pitch)) {
+ font_glyph_set_bit(out, width - (1 + x + shift), height - (1 + y),
+ bit_pitch);
+ }
}
}
}
@@ -115,22 +126,24 @@ void font_glyph_rotate_180(const unsigned char *glyph, unsigned int width, unsig
{
memset(out, 0, font_glyph_size(width, height));
- rotate_ud(glyph, out, width, height);
+ __font_glyph_rotate_180(glyph, width, height, out);
}
EXPORT_SYMBOL_GPL(font_glyph_rotate_180);
-static inline void rotate_ccw(const char *in, char *out, u32 width, u32 height)
+static void __font_glyph_rotate_270(const unsigned char *glyph,
+ unsigned int width, unsigned int height,
+ unsigned char *out)
{
- int i, j, h = height, w = width;
- int shift = (8 - (width % 8)) & 7;
-
- width = (width + 7) & ~7;
- height = (height + 7) & ~7;
-
- for (i = 0; i < h; i++) {
- for (j = 0; j < w; j++) {
- if (font_glyph_test_bit(in, j, i, width))
- font_glyph_set_bit(out, i, width - 1 - j - shift, height);
+ unsigned int x, y;
+ unsigned int shift = (8 - (width % 8)) & 7;
+ unsigned int bit_pitch = font_glyph_bit_pitch(width);
+ unsigned int out_bit_pitch = font_glyph_bit_pitch(height);
+
+ for (y = 0; y < height; y++) {
+ for (x = 0; x < width; x++) {
+ if (font_glyph_test_bit(glyph, x, y, bit_pitch))
+ font_glyph_set_bit(out, y, bit_pitch - 1 - x - shift,
+ out_bit_pitch);
}
}
}
@@ -154,6 +167,6 @@ void font_glyph_rotate_270(const unsigned char *glyph, unsigned int width, unsig
{
memset(out, 0, font_glyph_size(height, width)); /* flip width/height */
- rotate_ccw(glyph, out, width, height);
+ __font_glyph_rotate_270(glyph, width, height, out);
}
EXPORT_SYMBOL_GPL(font_glyph_rotate_270);
--
2.53.0
^ permalink raw reply related
* [PATCH v2 10/10] fbcon: Put font-rotation state into separate struct
From: Thomas Zimmermann @ 2026-04-07 9:23 UTC (permalink / raw)
To: deller, gregkh, jirislaby, geert, simona, sam
Cc: linux-fbdev, dri-devel, linux-kernel, linux-serial,
Thomas Zimmermann
In-Reply-To: <20260407092555.58816-1-tzimmermann@suse.de>
Move all temporary state of the font-rotation code into the struct
rotated in struct fbcon_par. Protect it with the Kconfig symbol
CONFIG_FRAMEBUFFER_CONSOLE_ROTATION. Avoids mixing it up with fbcon's
regular state.
v2:
- fix typos
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/video/fbdev/core/fbcon.c | 8 ++++++--
drivers/video/fbdev/core/fbcon.h | 12 +++++++----
drivers/video/fbdev/core/fbcon_ccw.c | 8 ++++----
drivers/video/fbdev/core/fbcon_cw.c | 8 ++++----
drivers/video/fbdev/core/fbcon_rotate.c | 27 +++++++++++++------------
drivers/video/fbdev/core/fbcon_ud.c | 10 ++++-----
6 files changed, 41 insertions(+), 32 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index ff4c69e971f8..b0e3e765360d 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -787,7 +787,9 @@ static void fbcon_release(struct fb_info *info)
kfree(par->cursor_state.mask);
kfree(par->cursor_data);
kfree(par->cursor_src);
- kfree(par->fontbuffer);
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
+ kfree(par->rotated.buf);
+#endif
kfree(info->fbcon_par);
info->fbcon_par = NULL;
}
@@ -1040,7 +1042,9 @@ static const char *fbcon_startup(void)
par = info->fbcon_par;
par->currcon = -1;
par->graphics = 1;
- par->cur_rotate = -1;
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
+ par->rotated.buf_rotate = -1;
+#endif
p->con_rotate = initial_rotation;
if (p->con_rotate == -1)
diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index bb0727b70631..321cc7f44baa 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -80,13 +80,17 @@ struct fbcon_par {
int graphics;
bool initialized;
int rotate;
- int cur_rotate;
char *cursor_data;
- u8 *fontbuffer;
- const u8 *fontdata;
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
+ struct {
+ font_data_t *fontdata; /* source font */
+ u8 *buf; /* rotated glyphs */
+ size_t bufsize;
+ int buf_rotate; /* rotation of buf */
+ } rotated;
+#endif
u8 *cursor_src;
u32 cursor_size;
- size_t fd_size;
const struct fbcon_bitops *bitops;
};
diff --git a/drivers/video/fbdev/core/fbcon_ccw.c b/drivers/video/fbdev/core/fbcon_ccw.c
index 723d9a33067f..33f02d579e02 100644
--- a/drivers/video/fbdev/core/fbcon_ccw.c
+++ b/drivers/video/fbdev/core/fbcon_ccw.c
@@ -106,7 +106,7 @@ static inline void ccw_putcs_aligned(struct vc_data *vc, struct fb_info *info,
u8 *src;
while (cnt--) {
- src = par->fontbuffer + (scr_readw(s--) & charmask) * cellsize;
+ src = par->rotated.buf + (scr_readw(s--) & charmask) * cellsize;
if (attr) {
ccw_update_attr(buf, src, attr, vc);
@@ -142,7 +142,7 @@ static void ccw_putcs(struct vc_data *vc, struct fb_info *info,
u8 *dst, *buf = NULL;
u32 vyres = GETVYRES(par->p, info);
- if (!par->fontbuffer)
+ if (!par->rotated.buf)
return;
image.fg_color = fg;
@@ -232,14 +232,14 @@ static void ccw_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
char *src;
u32 vyres = GETVYRES(par->p, info);
- if (!par->fontbuffer)
+ if (!par->rotated.buf)
return;
cursor.set = 0;
c = scr_readw((u16 *) vc->vc_pos);
attribute = get_attribute(info, c);
- src = par->fontbuffer + ((c & charmask) * (w * vc->vc_font.width));
+ src = par->rotated.buf + ((c & charmask) * (w * vc->vc_font.width));
if (par->cursor_state.image.data != src ||
par->cursor_reset) {
diff --git a/drivers/video/fbdev/core/fbcon_cw.c b/drivers/video/fbdev/core/fbcon_cw.c
index 732d093d462f..bde820967eb9 100644
--- a/drivers/video/fbdev/core/fbcon_cw.c
+++ b/drivers/video/fbdev/core/fbcon_cw.c
@@ -91,7 +91,7 @@ static inline void cw_putcs_aligned(struct vc_data *vc, struct fb_info *info,
u8 *src;
while (cnt--) {
- src = par->fontbuffer + (scr_readw(s++) & charmask) * cellsize;
+ src = par->rotated.buf + (scr_readw(s++) & charmask) * cellsize;
if (attr) {
cw_update_attr(buf, src, attr, vc);
@@ -127,7 +127,7 @@ static void cw_putcs(struct vc_data *vc, struct fb_info *info,
u8 *dst, *buf = NULL;
u32 vxres = GETVXRES(par->p, info);
- if (!par->fontbuffer)
+ if (!par->rotated.buf)
return;
image.fg_color = fg;
@@ -215,14 +215,14 @@ static void cw_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
char *src;
u32 vxres = GETVXRES(par->p, info);
- if (!par->fontbuffer)
+ if (!par->rotated.buf)
return;
cursor.set = 0;
c = scr_readw((u16 *) vc->vc_pos);
attribute = get_attribute(info, c);
- src = par->fontbuffer + ((c & charmask) * (w * vc->vc_font.width));
+ src = par->rotated.buf + ((c & charmask) * (w * vc->vc_font.width));
if (par->cursor_state.image.data != src ||
par->cursor_reset) {
diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c
index 74206f5a6e98..6cdbc96eeca6 100644
--- a/drivers/video/fbdev/core/fbcon_rotate.c
+++ b/drivers/video/fbdev/core/fbcon_rotate.c
@@ -18,34 +18,35 @@
int fbcon_rotate_font(struct fb_info *info, struct vc_data *vc)
{
struct fbcon_par *par = info->fbcon_par;
- unsigned char *fontbuffer;
+ unsigned char *buf;
int ret;
- if (vc->vc_font.data == par->fontdata &&
- par->p->con_rotate == par->cur_rotate)
+ if (par->p->fontdata == par->rotated.fontdata && par->rotate == par->rotated.buf_rotate)
return 0;
- par->fontdata = vc->vc_font.data;
- par->cur_rotate = par->p->con_rotate;
+ par->rotated.fontdata = par->p->fontdata;
+ par->rotated.buf_rotate = par->rotate;
if (info->fbops->fb_sync)
info->fbops->fb_sync(info);
- fontbuffer = font_data_rotate(par->p->fontdata, vc->vc_font.width,
- vc->vc_font.height, vc->vc_font.charcount,
- par->rotate, par->fontbuffer, &par->fd_size);
- if (IS_ERR(fontbuffer)) {
- ret = PTR_ERR(fontbuffer);
+ buf = font_data_rotate(par->rotated.fontdata, vc->vc_font.width,
+ vc->vc_font.height, vc->vc_font.charcount,
+ par->rotated.buf_rotate, par->rotated.buf,
+ &par->rotated.bufsize);
+ if (IS_ERR(buf)) {
+ ret = PTR_ERR(buf);
goto err_kfree;
}
- par->fontbuffer = fontbuffer;
+ par->rotated.buf = buf;
return 0;
err_kfree:
- kfree(par->fontbuffer);
- par->fontbuffer = NULL; /* clear here to avoid output */
+ kfree(par->rotated.buf);
+ par->rotated.buf = NULL; /* clear here to avoid output */
+ par->rotated.bufsize = 0;
return ret;
}
diff --git a/drivers/video/fbdev/core/fbcon_ud.c b/drivers/video/fbdev/core/fbcon_ud.c
index a1981fa4701a..eaf08999e249 100644
--- a/drivers/video/fbdev/core/fbcon_ud.c
+++ b/drivers/video/fbdev/core/fbcon_ud.c
@@ -92,7 +92,7 @@ static inline void ud_putcs_aligned(struct vc_data *vc, struct fb_info *info,
u8 *src;
while (cnt--) {
- src = par->fontbuffer + (scr_readw(s--) & charmask) * cellsize;
+ src = par->rotated.buf + (scr_readw(s--) & charmask) * cellsize;
if (attr) {
ud_update_attr(buf, src, attr, vc);
@@ -127,7 +127,7 @@ static inline void ud_putcs_unaligned(struct vc_data *vc,
u8 *src;
while (cnt--) {
- src = par->fontbuffer + (scr_readw(s--) & charmask) * cellsize;
+ src = par->rotated.buf + (scr_readw(s--) & charmask) * cellsize;
if (attr) {
ud_update_attr(buf, src, attr, vc);
@@ -164,7 +164,7 @@ static void ud_putcs(struct vc_data *vc, struct fb_info *info,
u32 vyres = GETVYRES(par->p, info);
u32 vxres = GETVXRES(par->p, info);
- if (!par->fontbuffer)
+ if (!par->rotated.buf)
return;
image.fg_color = fg;
@@ -262,14 +262,14 @@ static void ud_cursor(struct vc_data *vc, struct fb_info *info, bool enable,
u32 vyres = GETVYRES(par->p, info);
u32 vxres = GETVXRES(par->p, info);
- if (!par->fontbuffer)
+ if (!par->rotated.buf)
return;
cursor.set = 0;
c = scr_readw((u16 *) vc->vc_pos);
attribute = get_attribute(info, c);
- src = par->fontbuffer + ((c & charmask) * (w * vc->vc_font.height));
+ src = par->rotated.buf + ((c & charmask) * (w * vc->vc_font.height));
if (par->cursor_state.image.data != src ||
par->cursor_reset) {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Eliot Courtney @ 2026-04-07 13:42 UTC (permalink / raw)
To: Joel Fernandes, Eliot Courtney, linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
rust-for-linux, Nikola Djukic, 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,
Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, joel, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <5db2aab1-4b65-486e-ad9b-27a108bdb0d6@nvidia.com>
On Tue Apr 7, 2026 at 6:55 AM JST, Joel Fernandes wrote:
>>> + /// Compute upper bound on page table pages needed for `num_virt_pages`.
>>> + ///
>>> + /// Walks from PTE level up through PDE levels, accumulating the tree.
>>> + pub(crate) fn pt_pages_upper_bound(&self, num_virt_pages: usize) -> usize {
>>> + let mut total = 0;
>>> +
>>> + // PTE pages at the leaf level.
>>> + let pte_epp = self.entries_per_page(self.pte_level());
>>> + let mut pages_at_level = num_virt_pages.div_ceil(pte_epp);
>>> + total += pages_at_level;
>>> +
>>> + // Walk PDE levels bottom-up (reverse of pde_levels()).
>>> + for &level in self.pde_levels().iter().rev() {
>>> + let epp = self.entries_per_page(level);
>>> +
>>> + // How many pages at this level do we need to point to
>>> + // the previous pages_at_level?
>>> + pages_at_level = pages_at_level.div_ceil(epp);
>>> + total += pages_at_level;
>>> + }
>>> +
>>> + total
>>> + }
>>> +}
>>> +
>>
>> We have a lot of matches on the MMU version here (and below in Pte, Pde,
>> DualPde). What about making MmuVersion into a trait (e.g. Mmu) with
>> associated types for Pte, Pde, DualPde which can implement traits
>> defining their common operations too?
>
> I coded this up and it did not look pretty, there's not much LOC savings and the
> code becomes harder to read because of parametrization of several functions. Also:
Thanks for looking into it. Sorry to be a bother, but would you have a
branch around with the code? I'm curious what didn't look good about it.
>> Then you can parameterise Vmm/PtWalk on this type.
>
> The match still to be done somewhere, so you end up matching on chipset to call
> the correct parametrized functions versus just passing in the parameter or
> chipset down, in some cases.
>
> For now I am inclined to leave it as is. Also there's a Rust pitfall we all
> learnt during the turing and other patch reviews, sometimes doing a bunch of
> matches is good especially if the number of variants are expected to be fixed
> (in the mm case, version 2 and version 3). Traits have some disadvantages too,
> example dyn traits have to heap-allocated, parametrizing can increase code size
> (due to monomorphization) etc.
Yeah, it's just this is a lot of matches in a lot of places. And we have
ver2 / ver3 specific code leaking into the general pagetable.rs file. So
it would be really nice if we could find a way to improve this specific
aspect. We can reduce the match to happening in just one file. You can
avoid heap allocation if you would like by making Vmm an enum,
for example, and doing the match based dispatch there at the top of the
API tree, rather than at the bottom where it fans out into a lot more
locations.
>
> thanks,
>
> --
> Joel Fernandes
^ permalink raw reply
* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Joel Fernandes @ 2026-04-07 13:59 UTC (permalink / raw)
To: Eliot Courtney, linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
rust-for-linux, Nikola Djukic, 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,
Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, joel, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <DHMYSTLVHIFJ.A2BDMPVNZNLS@nvidia.com>
Hi Eliot,
On 4/7/2026 9:42 AM, Eliot Courtney wrote:
> On Tue Apr 7, 2026 at 6:55 AM JST, Joel Fernandes wrote:
>>>> + /// Compute upper bound on page table pages needed for `num_virt_pages`.
>>>> + ///
>>>> + /// Walks from PTE level up through PDE levels, accumulating the tree.
>>>> + pub(crate) fn pt_pages_upper_bound(&self, num_virt_pages: usize) -> usize {
>>>> + let mut total = 0;
>>>> +
>>>> + // PTE pages at the leaf level.
>>>> + let pte_epp = self.entries_per_page(self.pte_level());
>>>> + let mut pages_at_level = num_virt_pages.div_ceil(pte_epp);
>>>> + total += pages_at_level;
>>>> +
>>>> + // Walk PDE levels bottom-up (reverse of pde_levels()).
>>>> + for &level in self.pde_levels().iter().rev() {
>>>> + let epp = self.entries_per_page(level);
>>>> +
>>>> + // How many pages at this level do we need to point to
>>>> + // the previous pages_at_level?
>>>> + pages_at_level = pages_at_level.div_ceil(epp);
>>>> + total += pages_at_level;
>>>> + }
>>>> +
>>>> + total
>>>> + }
>>>> +}
>>>> +
>>>
>>> We have a lot of matches on the MMU version here (and below in Pte, Pde,
>>> DualPde). What about making MmuVersion into a trait (e.g. Mmu) with
>>> associated types for Pte, Pde, DualPde which can implement traits
>>> defining their common operations too?
>>
>> I coded this up and it did not look pretty, there's not much LOC savings and the
>> code becomes harder to read because of parametrization of several functions. Also:
>
> Thanks for looking into it. Sorry to be a bother, but would you have a
> branch around with the code? I'm curious what didn't look good about it.
Sorry but I already mentioned that above, the parameterizing of dozens of
function call sites, 3-4 new traits (because each struct like
Pte/Pde/DualPde etc each need their own trait which different MMU versions
implement) etc. The code because hard to read and readability is the top
critical criteria for me - I am personally strictly against "Lets use shiny
features in language at the cost of making code unreadable". Because that
translates into bugs and nightmare for maintainability.
I don't have the code at the moment, but if you still want to spend on time
on this direction, feel free to share a tree. I am happy to take a look.
>>> Then you can parameterise Vmm/PtWalk on this type.
>>
>> The match still to be done somewhere, so you end up matching on chipset to call
>> the correct parametrized functions versus just passing in the parameter or
>> chipset down, in some cases.
>>
>> For now I am inclined to leave it as is. Also there's a Rust pitfall we all
>> learnt during the turing and other patch reviews, sometimes doing a bunch of
>> matches is good especially if the number of variants are expected to be fixed
>> (in the mm case, version 2 and version 3). Traits have some disadvantages too,
>> example dyn traits have to heap-allocated, parametrizing can increase code size
>> (due to monomorphization) etc.
>
> Yeah, it's just this is a lot of matches in a lot of places. And we have
> ver2 / ver3 specific code leaking into the general pagetable.rs file. So
That's not a leak, that's by design. pagetable.rs is where the matches are
centralized, most of the code changes here on out should happen outside of
this file.
31 out of 42 matches in the mm code are in pagetable.rs, so it is already
centralized.
> it would be really nice if we could find a way to improve this specific
> aspect. We can reduce the match to happening in just one file.
Assuming we know what we're improving. ;-)
> You can> avoid heap allocation if you would like by making Vmm an enum,
> for example, and doing the match based dispatch there at the top of the
> API tree, rather than at the bottom where it fans out into a lot more
> locations.
heap allocation is not always free, this code sensitive to dynamic
allocations in the kernel, due to MM reclaim and locking. I would like to
keep it simple.
thanks,
--
Joel Fernandes
^ permalink raw reply
* [PATCH v8 1/4] dt-bindings: backlight: Add max25014 support
From: Maud Spierings via B4 Relay @ 2026-04-07 14:41 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown, Frank Li
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel, Maud Spierings
In-Reply-To: <20260407-max25014-v8-0-14eac7ed673a@gocontroll.com>
From: Maud Spierings <maudspierings@gocontroll.com>
The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
with integrated boost controller.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
In the current implementation the control registers for channel 1,
control all channels. So only one led subnode with led-sources is
supported right now. If at some point the driver functionality is
expanded the bindings can be easily extended with it.
---
.../bindings/leds/backlight/maxim,max25014.yaml | 83 ++++++++++++++++++++++
MAINTAINERS | 5 ++
2 files changed, 88 insertions(+)
diff --git a/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
new file mode 100644
index 0000000000000..d00be2e081938
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim max25014 backlight controller
+
+maintainers:
+ - Maud Spierings <maudspierings@gocontroll.com>
+
+properties:
+ compatible:
+ enum:
+ - maxim,max25014
+
+ reg:
+ maxItems: 1
+
+ default-brightness:
+ minimum: 0
+ maximum: 100
+ default: 50
+
+ enable-gpios:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ power-supply:
+ description: Regulator which controls the boost converter input rail.
+
+ pwms:
+ maxItems: 1
+
+ maxim,iset:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 15
+ default: 11
+ description:
+ Value of the ISET field in the ISET register. This controls the current
+ scale of the outputs, a higher number means more current.
+
+ maxim,strings:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description:
+ A 4-bit bitfield that describes which led strings to turn on.
+ minItems: 4
+ maxItems: 4
+ items:
+ maximum: 1
+ default:
+ [1 1 1 1]
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ backlight@6f {
+ compatible = "maxim,max25014";
+ reg = <0x6f>;
+ default-brightness = <50>;
+ enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+ power-supply = <®_backlight>;
+ pwms = <&pwm1>;
+ maxim,iset = <7>;
+ maxim,strings = <1 1 1 0>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 10d12b51b1f6f..7e3ad236537fe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15688,6 +15688,11 @@ F: Documentation/userspace-api/media/drivers/max2175.rst
F: drivers/media/i2c/max2175*
F: include/uapi/linux/max2175.h
+MAX25014 BACKLIGHT DRIVER
+M: Maud Spierings <maudspierings@gocontroll.com>
+S: Maintained
+F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
+
MAX31335 RTC DRIVER
M: Antoniu Miclaus <antoniu.miclaus@analog.com>
L: linux-rtc@vger.kernel.org
--
2.53.0
^ permalink raw reply related
* [PATCH v8 2/4] backlight: add max25014atg backlight
From: Maud Spierings via B4 Relay @ 2026-04-07 14:41 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown, Frank Li
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel, Maud Spierings
In-Reply-To: <20260407-max25014-v8-0-14eac7ed673a@gocontroll.com>
From: Maud Spierings <maudspierings@gocontroll.com>
The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC
with integrated boost controller.
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
MAINTAINERS | 1 +
drivers/video/backlight/Kconfig | 7 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/max25014.c | 377 +++++++++++++++++++++++++++++++++++++
4 files changed, 386 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 7e3ad236537fe..1f1a5326a6aab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15692,6 +15692,7 @@ MAX25014 BACKLIGHT DRIVER
M: Maud Spierings <maudspierings@gocontroll.com>
S: Maintained
F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml
+F: drivers/video/backlight/max25014.c
MAX31335 RTC DRIVER
M: Antoniu Miclaus <antoniu.miclaus@analog.com>
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index a7a3fbaf7c29e..f4e99542ffe8f 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -282,6 +282,13 @@ config BACKLIGHT_DA9052
help
Enable the Backlight Driver for DA9052-BC and DA9053-AA/Bx PMICs.
+config BACKLIGHT_MAX25014
+ tristate "Backlight driver for Maxim MAX25014"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ If you are using a MAX25014 chip as a backlight driver say Y to enable it.
+
config BACKLIGHT_MAX8925
tristate "Backlight driver for MAX8925"
depends on MFD_MAX8925
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 794820a98ed49..21c8313cfb121 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o
obj-$(CONFIG_BACKLIGHT_LP855X) += lp855x_bl.o
obj-$(CONFIG_BACKLIGHT_LP8788) += lp8788_bl.o
obj-$(CONFIG_BACKLIGHT_LV5207LP) += lv5207lp.o
+obj-$(CONFIG_BACKLIGHT_MAX25014) += max25014.o
obj-$(CONFIG_BACKLIGHT_MAX8925) += max8925_bl.o
obj-$(CONFIG_BACKLIGHT_MP3309C) += mp3309c.o
obj-$(CONFIG_BACKLIGHT_MT6370) += mt6370-backlight.o
diff --git a/drivers/video/backlight/max25014.c b/drivers/video/backlight/max25014.c
new file mode 100644
index 0000000000000..3ee45617261f3
--- /dev/null
+++ b/drivers/video/backlight/max25014.c
@@ -0,0 +1,377 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Backlight driver for Maxim MAX25014
+ *
+ * Copyright (C) 2025 GOcontroll B.V.
+ * Author: Maud Spierings <maudspierings@gocontroll.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define MAX25014_ISET_DEFAULT_100 11
+#define MAX_BRIGHTNESS 100
+#define MIN_BRIGHTNESS 0
+#define TON_MAX 130720 /* @153Hz */
+#define TON_STEP 1307 /* @153Hz */
+#define TON_MIN 0
+
+#define MAX25014_DEV_ID 0x00
+#define MAX25014_REV_ID 0x01
+#define MAX25014_ISET 0x02
+#define MAX25014_IMODE 0x03
+#define MAX25014_TON1H 0x04
+#define MAX25014_TON1L 0x05
+#define MAX25014_TON2H 0x06
+#define MAX25014_TON2L 0x07
+#define MAX25014_TON3H 0x08
+#define MAX25014_TON3L 0x09
+#define MAX25014_TON4H 0x0A
+#define MAX25014_TON4L 0x0B
+#define MAX25014_TON_1_4_LSB 0x0C
+#define MAX25014_SETTING 0x12
+#define MAX25014_DISABLE 0x13
+#define MAX25014_BSTMON 0x14
+#define MAX25014_IOUT1 0x15
+#define MAX25014_IOUT2 0x16
+#define MAX25014_IOUT3 0x17
+#define MAX25014_IOUT4 0x18
+#define MAX25014_OPEN 0x1B
+#define MAX25014_SHORTGND 0x1C
+#define MAX25014_SHORTED_LED 0x1D
+#define MAX25014_MASK 0x1E
+#define MAX25014_DIAG 0x1F
+
+#define MAX25014_ISET_ENA BIT(5)
+#define MAX25014_ISET_PSEN BIT(4)
+#define MAX25014_IMODE_HDIM BIT(2)
+#define MAX25014_SETTING_FPWM GENMASK(6, 4)
+#define MAX25014_DISABLE_DIS_MASK GENMASK(3, 0)
+#define MAX25014_DIAG_OT BIT(0)
+#define MAX25014_DIAG_OTW BIT(1)
+#define MAX25014_DIAG_HW_RST BIT(2)
+#define MAX25014_DIAG_BSTOV BIT(3)
+#define MAX25014_DIAG_BSTUV BIT(4)
+#define MAX25014_DIAG_IREFOOR BIT(5)
+
+struct max25014 {
+ struct i2c_client *client;
+ struct backlight_device *bl;
+ struct regmap *regmap;
+ struct gpio_desc *enable;
+ uint32_t iset;
+ uint8_t strings_mask;
+};
+
+static const struct regmap_config max25014_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MAX25014_DIAG,
+};
+
+static int max25014_initial_power_state(struct max25014 *maxim)
+{
+ uint32_t val;
+ int ret;
+
+ ret = regmap_read(maxim->regmap, MAX25014_ISET, &val);
+ if (ret)
+ return ret;
+
+ return val & MAX25014_ISET_ENA ? BACKLIGHT_POWER_ON : BACKLIGHT_POWER_OFF;
+}
+
+static int max25014_check_errors(struct max25014 *maxim)
+{
+ uint32_t val;
+ uint8_t i;
+ int ret;
+
+ ret = regmap_read(maxim->regmap, MAX25014_OPEN, &val);
+ if (ret)
+ return ret;
+ if (val) {
+ dev_err(&maxim->client->dev, "Open led strings detected on:\n");
+ for (i = 0; i < 4; i++) {
+ if (val & 1 << i)
+ dev_err(&maxim->client->dev, "string %d\n", i + 1);
+ }
+ return -EIO;
+ }
+
+ ret = regmap_read(maxim->regmap, MAX25014_SHORTGND, &val);
+ if (ret)
+ return ret;
+ if (val) {
+ dev_err(&maxim->client->dev, "Short to ground detected on:\n");
+ for (i = 0; i < 4; i++) {
+ if (val & 1 << i)
+ dev_err(&maxim->client->dev, "string %d\n", i + 1);
+ }
+ return -EIO;
+ }
+
+ ret = regmap_read(maxim->regmap, MAX25014_SHORTED_LED, &val);
+ if (ret)
+ return ret;
+ if (val) {
+ dev_err(&maxim->client->dev, "Shorted led detected on:\n");
+ for (i = 0; i < 4; i++) {
+ if (val & 1 << i)
+ dev_err(&maxim->client->dev, "string %d\n", i + 1);
+ }
+ return -EIO;
+ }
+
+ ret = regmap_read(maxim->regmap, MAX25014_DIAG, &val);
+ if (ret)
+ return ret;
+ /*
+ * The HW_RST bit always starts at 1 after power up.
+ * It is reset on first read, does not indicate an error.
+ */
+ if (val && val != MAX25014_DIAG_HW_RST) {
+ if (val & MAX25014_DIAG_OT)
+ dev_err(&maxim->client->dev,
+ "Overtemperature shutdown\n");
+ if (val & MAX25014_DIAG_OTW)
+ dev_err(&maxim->client->dev,
+ "Chip is getting too hot (>125C)\n");
+ if (val & MAX25014_DIAG_BSTOV)
+ dev_err(&maxim->client->dev,
+ "Boost converter overvoltage\n");
+ if (val & MAX25014_DIAG_BSTUV)
+ dev_err(&maxim->client->dev,
+ "Boost converter undervoltage\n");
+ if (val & MAX25014_DIAG_IREFOOR)
+ dev_err(&maxim->client->dev, "IREF out of range\n");
+ return -EIO;
+ }
+ return 0;
+}
+
+/*
+ * 1. disable unused strings
+ * 2. set dim mode
+ * 3. set setting register
+ * 4. enable the backlight
+ */
+static int max25014_configure(struct max25014 *maxim, int initial_state)
+{
+ uint32_t val;
+ int ret;
+
+ ret = regmap_read(maxim->regmap, MAX25014_DISABLE, &val);
+ if (ret)
+ return ret;
+
+ /*
+ * Strings can only be disabled when MAX25014_ISET_ENA == 0, check if
+ * it needs to be changed at all to prevent the backlight flashing when
+ * it is configured correctly by the bootloader
+ */
+ if (!((val & MAX25014_DISABLE_DIS_MASK) == maxim->strings_mask)) {
+ if (initial_state == BACKLIGHT_POWER_ON) {
+ ret = regmap_write(maxim->regmap, MAX25014_ISET, 0);
+ if (ret)
+ return ret;
+ }
+ ret = regmap_write(maxim->regmap, MAX25014_DISABLE, maxim->strings_mask);
+ if (ret)
+ return ret;
+ }
+
+ ret = regmap_write(maxim->regmap, MAX25014_IMODE, MAX25014_IMODE_HDIM);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(maxim->regmap, MAX25014_SETTING, &val);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(maxim->regmap, MAX25014_SETTING,
+ val & ~MAX25014_SETTING_FPWM);
+ if (ret)
+ return ret;
+
+ return regmap_write(maxim->regmap, MAX25014_ISET,
+ maxim->iset | MAX25014_ISET_ENA |
+ MAX25014_ISET_PSEN);
+}
+
+static int max25014_update_status(struct backlight_device *bl_dev)
+{
+ struct max25014 *maxim = bl_get_data(bl_dev);
+ uint32_t reg;
+ int ret;
+
+ reg = TON_STEP * backlight_get_brightness(bl_dev);
+
+ /*
+ * 18 bit number lowest, 2 bits in first register,
+ * next lowest 8 in the L register, next 8 in the H register
+ * Seemingly setting the strength of only one string controls all of
+ * them, individual settings don't affect the outcome.
+ */
+ ret = regmap_write(maxim->regmap, MAX25014_TON_1_4_LSB, reg & 0b00000011);
+ if (ret != 0)
+ return ret;
+ ret = regmap_write(maxim->regmap, MAX25014_TON1L, (reg >> 2) & 0b11111111);
+ if (ret != 0)
+ return ret;
+ return regmap_write(maxim->regmap, MAX25014_TON1H, (reg >> 10) & 0b11111111);
+}
+
+static const struct backlight_ops max25014_bl_ops = {
+ .options = BL_CORE_SUSPENDRESUME,
+ .update_status = max25014_update_status,
+};
+
+static int max25014_parse_dt(struct max25014 *maxim,
+ uint32_t *initial_brightness)
+{
+ struct device *dev = &maxim->client->dev;
+ struct device_node *node = dev->of_node;
+ uint32_t strings[4];
+ int res, i;
+
+ res = of_property_count_u32_elems(node, "maxim,strings");
+ if (res == 4) {
+ of_property_read_u32_array(node, "maxim,strings", strings, 4);
+ for (i = 0; i < 4; i++) {
+ if (strings[i] == 0)
+ maxim->strings_mask |= 1 << i;
+ }
+ } else {
+ maxim->strings_mask = 0;
+ }
+
+ *initial_brightness = 50U;
+ of_property_read_u32(node, "default-brightness", initial_brightness);
+
+ maxim->iset = MAX25014_ISET_DEFAULT_100;
+ of_property_read_u32(node, "maxim,iset", &maxim->iset);
+
+ if (maxim->iset > 15)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid iset, should be a value from 0-15, entered was %d\n",
+ maxim->iset);
+
+ if (*initial_brightness > 100)
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid initial brightness, should be a value from 0-100, entered was %d\n",
+ *initial_brightness);
+
+ return 0;
+}
+
+static int max25014_probe(struct i2c_client *cl)
+{
+ const struct i2c_device_id *id = i2c_client_get_device_id(cl);
+ struct backlight_properties props;
+ uint32_t initial_brightness = 50;
+ struct backlight_device *bl;
+ struct max25014 *maxim;
+ int ret;
+
+ maxim = devm_kzalloc(&cl->dev, sizeof(struct max25014), GFP_KERNEL);
+ if (!maxim)
+ return -ENOMEM;
+
+ maxim->client = cl;
+
+ ret = max25014_parse_dt(maxim, &initial_brightness);
+ if (ret)
+ return ret;
+
+ ret = devm_regulator_get_enable(&maxim->client->dev, "power");
+ if (ret)
+ return dev_err_probe(&maxim->client->dev, ret,
+ "failed to get power-supply");
+
+ maxim->enable = devm_gpiod_get_optional(&maxim->client->dev, "enable",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(maxim->enable))
+ return dev_err_probe(&maxim->client->dev, PTR_ERR(maxim->enable),
+ "failed to get enable gpio\n");
+
+ /* Datasheet Electrical Characteristics tSTARTUP 2ms */
+ fsleep(2000);
+
+ maxim->regmap = devm_regmap_init_i2c(cl, &max25014_regmap_config);
+ if (IS_ERR(maxim->regmap))
+ return dev_err_probe(&maxim->client->dev, PTR_ERR(maxim->regmap),
+ "failed to initialize the i2c regmap\n");
+
+ i2c_set_clientdata(cl, maxim);
+
+ ret = max25014_check_errors(maxim);
+ if (ret) /* error is already reported in the above function */
+ return ret;
+
+ ret = max25014_initial_power_state(maxim);
+ if (ret < 0)
+ return dev_err_probe(&maxim->client->dev, ret, "Could not get enabled state\n");
+
+ memset(&props, 0, sizeof(struct backlight_properties));
+ props.type = BACKLIGHT_PLATFORM;
+ props.max_brightness = MAX_BRIGHTNESS;
+ props.brightness = initial_brightness;
+ props.scale = BACKLIGHT_SCALE_LINEAR;
+ props.power = ret;
+
+ ret = max25014_configure(maxim, ret);
+ if (ret)
+ return dev_err_probe(&maxim->client->dev, ret, "device config error");
+
+ bl = devm_backlight_device_register(&maxim->client->dev, id->name,
+ &maxim->client->dev, maxim,
+ &max25014_bl_ops, &props);
+ if (IS_ERR(bl))
+ return dev_err_probe(&maxim->client->dev, PTR_ERR(bl),
+ "failed to register backlight\n");
+
+ maxim->bl = bl;
+
+ backlight_update_status(maxim->bl);
+
+ return 0;
+}
+
+static void max25014_remove(struct i2c_client *cl)
+{
+ struct max25014 *maxim = i2c_get_clientdata(cl);
+
+ backlight_device_set_brightness(maxim->bl, 0);
+ gpiod_set_value_cansleep(maxim->enable, 0);
+}
+
+static const struct of_device_id max25014_dt_ids[] = {
+ { .compatible = "maxim,max25014", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max25014_dt_ids);
+
+static const struct i2c_device_id max25014_ids[] = {
+ { "max25014" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max25014_ids);
+
+static struct i2c_driver max25014_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = of_match_ptr(max25014_dt_ids),
+ },
+ .probe = max25014_probe,
+ .remove = max25014_remove,
+ .id_table = max25014_ids,
+};
+module_i2c_driver(max25014_driver);
+
+MODULE_DESCRIPTION("Maxim MAX25014 backlight driver");
+MODULE_AUTHOR("Maud Spierings <maudspierings@gocontroll.com>");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related
* [PATCH v8 0/4] backlight: add new max25014 backlight driver
From: Maud Spierings via B4 Relay @ 2026-04-07 14:41 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown, Frank Li
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel, Maud Spierings
The Maxim MAX25014 is an automotive grade backlight driver IC. Its
datasheet can be found at [1].
With its integrated boost controller, it can power 4 channels (led
strings) and has a number of different modes using pwm and or i2c.
Currently implemented is only i2c control.
link: https://www.analog.com/media/en/technical-documentation/data-sheets/MAX25014.pdf [1]
Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
Changes in v8:
- Clean up some leftovers from the led subnodes, address/size-cells (Rob)
- Add Robs rb tag
- Add Daniels rb tag
- Bring the Kconfig option in line text wise with other drivers
- Rebase on the latest next
- Link to v7: https://lore.kernel.org/r/20260123-max25014-v7-0-15e504b9acc7@gocontroll.com
Changes in v7:
- remove the led subnodes
- always enable the regulator by using devm_regulator_get_enable()
- remove the no longer required gotos and simplify early returns
- fix the name of the SHORTED_LED error field
- fix the name of the SHORTGND error field
- use the proper backlight helper functions for setting/getting
brightness
- Link to v6: https://lore.kernel.org/r/20251201-max25014-v6-0-88e3ac8112ff@gocontroll.com
Changes in v6:
- fixup changes in v4 where default brightness handling was changed but
not noted
- remove leftover comment about initializing brightness
- use BIT definitions for fields in the DIAG register
- apply reverse christmas tree initialization of local variables
- remove !=0 from checks, just check if (ret)
- remove > 0 from checks, just check if (val)
- use dev_err_probe() more
- set enable gpio high in the get() instead of seperately calling set()
- change usleep_range() to fsleep()
- remove null checks when setting gpio value
- get regular regulator, not optional to avoid further NULL checks in
case none is provided
- introduce max25014_initial_power_state() to check if the bootloader
has already initialized the backlight and to correctly set props.power
- squash max25014_register_control() into max25014_update_status()
- in max25014_configure() perform extra checking on the DISABLE register
now that the state from the bootloader is taken into account
- Link to v5: https://lore.kernel.org/r/20251107-max25014-v5-0-9a6aa57306bf@gocontroll.com
Changes in v5:
- moved comment about current functions of the driver to the actual
comment section of the commit
- fixed the led@0 property, regex patternProperty is not needed as of
now
- added extra clarification about the ISET field/register
- moved #address-cells and #size-cells to the correct location
- remove leftover default-brightness in backlight nodes
- Link to v4: https://lore.kernel.org/r/20251009-max25014-v4-0-6adb2a0aa35f@gocontroll.com
Changes in v4:
- remove setting default brightness, let backlight core take care of it
- use a led node to describe the backlight
- use led-sources to enable specific channels
- also wait 2ms when there is a supply but no enable
- change dev_warn() to dev_err() in error path in max25014_check_errors()
- set backlight_properties.scale to BACKLIGHT_SCALE_LINEAR
- rebase latest next
- add address-cells and size-cells to i2c4 in av101hdt-a10.dtso
- Link to v3: https://lore.kernel.org/r/20250911-max25014-v3-0-d03f4eba375e@gocontroll.com
Changes in v3:
- fixed commit message type intgrated -> integrated
- added maximum and description to maxim,iset-property
- dropped unused labels and pinctrl in bindings example
- put the compatible first in the bindings example and dts
- removed brackets around defines
- removed the leftover pdata struct field
- removed the initial_brightness struct field
- Link to v2: https://lore.kernel.org/r/20250819-max25014-v2-0-5fd7aeb141ea@gocontroll.com
Changes in v2:
- Remove leftover unused property from the bindings example
- Complete the bindings example with all properties
- Remove some double info from the maxim,iset property
- Remove platform_data header, fold its data into the max25014 struct
- Don't force defines to be unsigned
- Remove stray struct max25014 declaration
- Remove chipname and device from the max25014 struct
- Inline the max25014_backlight_register() and strings_mask() functions
- Remove CONFIG_OF ifdef
- Link to v1: https://lore.kernel.org/r/20250725-max25014-v1-0-0e8cce92078e@gocontroll.com
---
Maud Spierings (4):
dt-bindings: backlight: Add max25014 support
backlight: add max25014atg backlight
arm64: dts: freescale: moduline-display-av101hdt-a10: add backlight
arm64: dts: freescale: moduline-display-av123z7m-n17: add backlight
.../bindings/leds/backlight/maxim,max25014.yaml | 83 +++++
MAINTAINERS | 6 +
...x8p-ml81-moduline-display-106-av101hdt-a10.dtso | 24 ++
...x8p-ml81-moduline-display-106-av123z7m-n17.dtso | 19 +-
drivers/video/backlight/Kconfig | 7 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/max25014.c | 377 +++++++++++++++++++++
7 files changed, 516 insertions(+), 1 deletion(-)
---
base-commit: f3e6330d7fe42b204af05a2dbc68b379e0ad179e
change-id: 20250626-max25014-4207591e1af5
Best regards,
--
Maud Spierings <maudspierings@gocontroll.com>
^ permalink raw reply
* [PATCH v8 3/4] arm64: dts: freescale: moduline-display-av101hdt-a10: add backlight
From: Maud Spierings via B4 Relay @ 2026-04-07 14:41 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown, Frank Li
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel, Maud Spierings
In-Reply-To: <20260407-max25014-v8-0-14eac7ed673a@gocontroll.com>
From: Maud Spierings <maudspierings@gocontroll.com>
Add the missing backlight driver.
Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
...x8p-ml81-moduline-display-106-av101hdt-a10.dtso | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso
index c6fc5d5b1e5fa..23e4bf64b28b5 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso
+++ b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso
@@ -17,6 +17,7 @@
panel {
compatible = "boe,av101hdt-a10";
+ backlight = <&backlight>;
enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
pinctrl-0 = <&pinctrl_panel>;
pinctrl-names = "default";
@@ -40,7 +41,30 @@ reg_vbus: regulator-vbus {
};
};
+&i2c4 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ backlight: backlight@6f {
+ compatible = "maxim,max25014";
+ reg = <0x6f>;
+ default-brightness = <50>;
+ enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_backlight>;
+ maxim,iset = <7>;
+ maxim,strings = <1 1 1 0>;
+ };
+};
+
&iomuxc {
+ pinctrl_backlight: backlightgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_GPIO1_IO04__GPIO1_IO04
+ (MX8MP_PULL_UP | MX8MP_PULL_ENABLE)
+ >;
+ };
+
pinctrl_panel: panelgrp {
fsl,pins = <
MX8MP_IOMUXC_GPIO1_IO07__GPIO1_IO07
--
2.53.0
^ permalink raw reply related
* [PATCH v8 4/4] arm64: dts: freescale: moduline-display-av123z7m-n17: add backlight
From: Maud Spierings via B4 Relay @ 2026-04-07 14:41 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown, Frank Li
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel, Maud Spierings
In-Reply-To: <20260407-max25014-v8-0-14eac7ed673a@gocontroll.com>
From: Maud Spierings <maudspierings@gocontroll.com>
Add the missing backlight.
Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
...p-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso
index 3eb665ce9d5d2..0b969c8c04db1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso
+++ b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso
@@ -16,6 +16,7 @@
panel {
compatible = "boe,av123z7m-n17";
+ backlight = <&backlight>;
enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
pinctrl-0 = <&pinctrl_panel>;
pinctrl-names = "default";
@@ -91,10 +92,26 @@ lvds1_out: endpoint {
};
};
- /* max25014 @ 0x6f */
+ backlight: backlight@6f {
+ compatible = "maxim,max25014";
+ reg = <0x6f>;
+ default-brightness = <50>;
+ enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_backlight>;
+ maxim,iset = <7>;
+ maxim,strings = <1 1 1 1>;
+ };
};
&iomuxc {
+ pinctrl_backlight: backlightgrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_GPIO1_IO04__GPIO1_IO04
+ (MX8MP_PULL_UP | MX8MP_PULL_ENABLE)
+ >;
+ };
+
pinctrl_lvds_bridge: lvdsbridgegrp {
fsl,pins = <
MX8MP_IOMUXC_SAI1_TXD2__GPIO4_IO14
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2 05/10] lib/fonts: Implement glyph rotation
From: Helge Deller @ 2026-04-07 15:48 UTC (permalink / raw)
To: Thomas Zimmermann, gregkh, jirislaby, geert, simona, sam
Cc: linux-fbdev, dri-devel, linux-kernel, linux-serial
In-Reply-To: <20260407092555.58816-6-tzimmermann@suse.de>
Hi Thomas,
On 4/7/26 11:23, Thomas Zimmermann wrote:
> Move the glyph rotation helpers from fbcon to the font library. Wrap them
> behind clean interfaces. Also clear the output memory to zero. Previously,
> the implementation relied on the caller to do that.
>
> Go through the fbcon code and callers of the glyph-rotation helpers. In
> addition to the font rotation, there's also the cursor code, which uses
> the rotation helpers.
>
> The font-rotation relied on a single memset to zero for the whole font.
> This is now multiple memsets on each glyph. This will be sorted out when
> the font library also implements font rotation.
>
> Building glyph rotation in the font library still depends on
> CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y. If we get more users of the code,
> we can still add a dedicated Kconfig symbol to the font library.
>
> No changes have been made to the actual implementation of the rotate_*()
> and pattern_*() functions. These will be refactored as separate changes.
>
> v2:
> - fix typos
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/video/fbdev/core/fbcon_ccw.c | 4 +-
> drivers/video/fbdev/core/fbcon_cw.c | 4 +-
> drivers/video/fbdev/core/fbcon_rotate.c | 12 +-
> drivers/video/fbdev/core/fbcon_rotate.h | 71 -----------
> include/linux/font.h | 8 ++
> lib/fonts/Makefile | 1 +
> lib/fonts/font_rotate.c | 150 ++++++++++++++++++++++++
Patch is Ok.
But since you move/add the file lib/fonts/font_rotate.c,
it should be reflected in MAINTAINERS file.
Do you mind sending a follow-up patch which adds /lib/fonts/* to the
FRAMEBUFFER LAYER and FRAMEBUFFER CORE entries in the MAINTAINERS file?
Other than that, I've now added this series to the fbdev git tree.
Thanks!
Helge
^ permalink raw reply
* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Alexandre Courbot @ 2026-04-08 7:03 UTC (permalink / raw)
To: Joel Fernandes
Cc: Eliot Courtney, linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
Bjorn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Dave Airlie, Daniel Almeida,
Koen Koning, dri-devel, rust-for-linux, Nikola Djukic,
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, Alex Gaynor, Boqun Feng,
John Hubbard, Alistair Popple, Timur Tabi, Edwin Peer,
Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
Philipp Stanner, Elle Rhumsaa, alexeyi, joel, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <537a8c5a-3885-4c47-99f6-963b48ddf87d@nvidia.com>
On Tue Apr 7, 2026 at 10:59 PM JST, Joel Fernandes wrote:
> Hi Eliot,
>
> On 4/7/2026 9:42 AM, Eliot Courtney wrote:
>> On Tue Apr 7, 2026 at 6:55 AM JST, Joel Fernandes wrote:
>>>>> + /// Compute upper bound on page table pages needed for `num_virt_pages`.
>>>>> + ///
>>>>> + /// Walks from PTE level up through PDE levels, accumulating the tree.
>>>>> + pub(crate) fn pt_pages_upper_bound(&self, num_virt_pages: usize) -> usize {
>>>>> + let mut total = 0;
>>>>> +
>>>>> + // PTE pages at the leaf level.
>>>>> + let pte_epp = self.entries_per_page(self.pte_level());
>>>>> + let mut pages_at_level = num_virt_pages.div_ceil(pte_epp);
>>>>> + total += pages_at_level;
>>>>> +
>>>>> + // Walk PDE levels bottom-up (reverse of pde_levels()).
>>>>> + for &level in self.pde_levels().iter().rev() {
>>>>> + let epp = self.entries_per_page(level);
>>>>> +
>>>>> + // How many pages at this level do we need to point to
>>>>> + // the previous pages_at_level?
>>>>> + pages_at_level = pages_at_level.div_ceil(epp);
>>>>> + total += pages_at_level;
>>>>> + }
>>>>> +
>>>>> + total
>>>>> + }
>>>>> +}
>>>>> +
>>>>
>>>> We have a lot of matches on the MMU version here (and below in Pte, Pde,
>>>> DualPde). What about making MmuVersion into a trait (e.g. Mmu) with
>>>> associated types for Pte, Pde, DualPde which can implement traits
>>>> defining their common operations too?
>>>
>>> I coded this up and it did not look pretty, there's not much LOC savings and the
>>> code becomes harder to read because of parametrization of several functions. Also:
>>
>> Thanks for looking into it. Sorry to be a bother, but would you have a
>> branch around with the code? I'm curious what didn't look good about it.
>
> Sorry but I already mentioned that above, the parameterizing of dozens of
> function call sites, 3-4 new traits (because each struct like
> Pte/Pde/DualPde etc each need their own trait which different MMU versions
> implement) etc. The code because hard to read and readability is the top
> critical criteria for me - I am personally strictly against "Lets use shiny
> features in language at the cost of making code unreadable". Because that
> translates into bugs and nightmare for maintainability.
After a quick look I'd say that having a trait here would actually be
*good* for correctness and maintainability.
The current design implies that every operation on a page table (most
likely using the walker) goes through a branching point. Just looking at
`PtWalk::read_pte_at_level`, there are already at least 6
`if version == 2 { } else { }` branches that all resolve to the same
result. Include walking down the PDEs and you have at least a dozen of
these just to resolve a virtual address. I know CPUs are fast, but this
is still wasted cycles for no good reason.
If you use a trait here, and make `PtWalk` generic against it, you can
optimize this away. We had a similar situation when we introduced Turing
support and the v2 ucode header, and tried both approaches: the
trait-based one was slightly shorter, and arguably more readable.
But the main argument to use a trait here IMO is that it enables
associated types and constants. That's particularly critical since some
equivalent fields have different lengths between v2 and v3. An
associated `Bounded` type for these would force the caller to validate
the length of these fields before calling a non-fallible operation,
which is exactly the level of caution that we want when dealing with
page tables.
In order to fully benefit from it, we will need the bitfield macro from
the `kernel` crate so the PDE/PTE fields can be `Bounded`, I will try to
make it available quickly in a patch that you can depend on.
But long story short, and although I need to dive deeper into the code,
this looks like a good candidate for using a trait and associated types.
^ permalink raw reply
* Re: [PATCH v10 02/21] gpu: nova-core: gsp: Extract usable FB region from GSP
From: Alexandre Courbot @ 2026-04-08 7:33 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Dave Airlie, Daniel Almeida, Koen Koning,
dri-devel, rust-for-linux, Nikola Djukic, 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, Alex Gaynor, Boqun Feng, John Hubbard,
Alistair Popple, Timur Tabi, Edwin Peer, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, Eliot Courtney, joel, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260331212048.2229260-3-joelagnelf@nvidia.com>
On Wed Apr 1, 2026 at 6:20 AM JST, Joel Fernandes wrote:
> Add first_usable_fb_region() to GspStaticConfigInfo to extract the first
> usable FB region from GSP's fbRegionInfoParams. Usable regions are those
> that are not reserved or protected.
>
> The extracted region is stored in GetGspStaticInfoReply and exposed as
> usable_fb_region field for use by the memory subsystem.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
> drivers/gpu/nova-core/gsp/commands.rs | 11 ++++--
> drivers/gpu/nova-core/gsp/fw/commands.rs | 44 +++++++++++++++++++++++-
> 2 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
> index c89c7b57a751..41742c1633c8 100644
> --- a/drivers/gpu/nova-core/gsp/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/commands.rs
> @@ -4,6 +4,7 @@
> array,
> convert::Infallible,
> ffi::FromBytesUntilNulError,
> + ops::Range,
> str::Utf8Error, //
> };
>
> @@ -189,22 +190,28 @@ fn init(&self) -> impl Init<Self::Command, Self::InitError> {
> }
> }
>
> -/// The reply from the GSP to the [`GetGspInfo`] command.
> +/// The reply from the GSP to the [`GetGspStaticInfo`] command.
> pub(crate) struct GetGspStaticInfoReply {
> gpu_name: [u8; 64],
> + /// Usable FB (VRAM) region for driver memory allocation.
> + #[expect(dead_code)]
> + pub(crate) usable_fb_region: Range<u64>,
Let's print the region when creating the GPU (using `dev_info` or
`dev_dbg`) - this can be useful to the user, and lets us remove this
`dead_code`.
^ permalink raw reply
* Re: [PATCH v10 01/21] gpu: nova-core: gsp: Return GspStaticInfo from boot()
From: Alexandre Courbot @ 2026-04-08 7:34 UTC (permalink / raw)
To: Joel Fernandes
Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Dave Airlie, Daniel Almeida, Koen Koning,
dri-devel, rust-for-linux, Nikola Djukic, 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, Alex Gaynor, Boqun Feng, John Hubbard,
Alistair Popple, Timur Tabi, Edwin Peer, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, Eliot Courtney, joel, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <20260331212048.2229260-2-joelagnelf@nvidia.com>
On Wed Apr 1, 2026 at 6:20 AM JST, Joel Fernandes wrote:
> Refactor the GSP boot function to return only the GspStaticInfo,
> removing the FbLayout from the return tuple.
We are not returning the `FbLayout` - that bit was introduced from an
earlier revision of this series and is not in the original code.
^ permalink raw reply
* Re: [PATCH v10 07/21] gpu: nova-core: mm: Add TLB flush support
From: Alexandre Courbot @ 2026-04-08 7:40 UTC (permalink / raw)
To: Matthew Brost
Cc: Joel Fernandes, linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
Bjorn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Dave Airlie, Daniel Almeida,
Koen Koning, dri-devel, rust-for-linux, Nikola Djukic,
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, Lucas De Marchi, Thomas Hellstrom,
Helge Deller, Alex Gaynor, Boqun Feng, John Hubbard,
Alistair Popple, Timur Tabi, Edwin Peer, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, Eliot Courtney, joel, linux-doc, amd-gfx,
intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <adSSrZp6a551xNTu@gsse-cloud1.jf.intel.com>
On Tue Apr 7, 2026 at 2:14 PM JST, Matthew Brost wrote:
> On Mon, Apr 06, 2026 at 06:10:07PM -0400, Joel Fernandes wrote:
>>
>>
>> On 4/6/2026 5:24 PM, Joel Fernandes wrote:
>> >
>> >
>> > On 4/2/2026 1:59 AM, Matthew Brost wrote:
>> >> On Tue, Mar 31, 2026 at 05:20:34PM -0400, Joel Fernandes wrote:
>> >>> Add TLB (Translation Lookaside Buffer) flush support for GPU MMU.
>> >>>
>> >>> After modifying page table entries, the GPU's TLB must be invalidated
>> >>> to ensure the new mappings take effect. The Tlb struct provides flush
>> >>> functionality through BAR0 registers.
>> >>>
>> >>> The flush operation writes the page directory base address and triggers
>> >>> an invalidation, polling for completion with a 2 second timeout matching
>> >>> the Nouveau driver.
>> >>>
>> >>> Cc: Nikola Djukic <ndjukic@nvidia.com>
>> >>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> >>> ---
>> >>> drivers/gpu/nova-core/mm.rs | 1 +
>> >>> drivers/gpu/nova-core/mm/tlb.rs | 95 +++++++++++++++++++++++++++++++++
>> >>> drivers/gpu/nova-core/regs.rs | 42 +++++++++++++++
>> >>> 3 files changed, 138 insertions(+)
>> >>> create mode 100644 drivers/gpu/nova-core/mm/tlb.rs
>> >>>
>> >>> diff --git a/drivers/gpu/nova-core/mm.rs b/drivers/gpu/nova-core/mm.rs
>> >>> index 8f3089a5fa88..cfe9cbe11d57 100644
>> >>> --- a/drivers/gpu/nova-core/mm.rs
>> >>> +++ b/drivers/gpu/nova-core/mm.rs
>> >>> @@ -5,6 +5,7 @@
>> >>> #![expect(dead_code)]
>> >>>
>> >>> pub(crate) mod pramin;
>> >>> +pub(crate) mod tlb;
>> >>>
>> >>> use kernel::sizes::SZ_4K;
>> >>>
>> >>> diff --git a/drivers/gpu/nova-core/mm/tlb.rs b/drivers/gpu/nova-core/mm/tlb.rs
>> >>> new file mode 100644
>> >>> index 000000000000..cd3cbcf4c739
>> >>> --- /dev/null
>> >>> +++ b/drivers/gpu/nova-core/mm/tlb.rs
>> >>> @@ -0,0 +1,95 @@
>> >>> +// SPDX-License-Identifier: GPL-2.0
>> >>> +
>> >>> +//! TLB (Translation Lookaside Buffer) flush support for GPU MMU.
>> >>> +//!
>> >>> +//! After modifying page table entries, the GPU's TLB must be flushed to
>> >>> +//! ensure the new mappings take effect. This module provides TLB flush
>> >>> +//! functionality for virtual memory managers.
>> >>> +//!
>> >>> +//! # Example
>> >>> +//!
>> >>> +//! ```ignore
>> >>> +//! use crate::mm::tlb::Tlb;
>> >>> +//!
>> >>> +//! fn page_table_update(tlb: &Tlb, pdb_addr: VramAddress) -> Result<()> {
>> >>> +//! // ... modify page tables ...
>> >>> +//!
>> >>> +//! // Flush TLB to make changes visible (polls for completion).
>> >>> +//! tlb.flush(pdb_addr)?;
>> >>> +//!
>> >>> +//! Ok(())
>> >>> +//! }
>> >>> +//! ```
>> >>> +
>> >>> +use kernel::{
>> >>> + devres::Devres,
>> >>> + io::poll::read_poll_timeout,
>> >>> + io::Io,
>> >>> + new_mutex,
>> >>> + prelude::*,
>> >>> + sync::{
>> >>> + Arc,
>> >>> + Mutex, //
>> >>> + },
>> >>> + time::Delta, //
>> >>> +};
>> >>> +
>> >>> +use crate::{
>> >>> + driver::Bar0,
>> >>> + mm::VramAddress,
>> >>> + regs, //
>> >>> +};
>> >>> +
>> >>> +/// TLB manager for GPU translation buffer operations.
>> >>> +#[pin_data]
>> >>> +pub(crate) struct Tlb {
>> >>> + bar: Arc<Devres<Bar0>>,
>> >>> + /// TLB flush serialization lock: This lock is acquired during the
>> >>> + /// DMA fence signalling critical path. It must NEVER be held across any
>> >>> + /// reclaimable CPU memory allocations because the memory reclaim path can
>> >>> + /// call `dma_fence_wait()`, which would deadlock with this lock held.
>> >>> + #[pin]
>> >>> + lock: Mutex<()>,
>> >>> +}
>> >>> +
>> >>> +impl Tlb {
>> >>> + /// Create a new TLB manager.
>> >>> + pub(super) fn new(bar: Arc<Devres<Bar0>>) -> impl PinInit<Self> {
>> >>> + pin_init!(Self {
>> >>> + bar,
>> >>> + lock <- new_mutex!((), "tlb_flush"),
>> >>> + })
>> >>> + }
>> >>> +
>> >>> + /// Flush the GPU TLB for a specific page directory base.
>> >>> + ///
>> >>> + /// This invalidates all TLB entries associated with the given PDB address.
>> >>> + /// Must be called after modifying page table entries to ensure the GPU sees
>> >>> + /// the updated mappings.
>> >>> + pub(crate) fn flush(&self, pdb_addr: VramAddress) -> Result {
>> >>
>> >> This landed on my list randomly, so I took a look.
>> >>
>> >> Wouldn’t you want to virtualize the invalidation based on your device?
>> >> For example, what if you need to register interface changes on future hardware?
>> >
>> > Good point, for future hardware it indeed makes sense. I will do that.
>> Actually, at least in the future as far as I can see, the register definitions
>> are the same for TLB invalidation are the same, so we are good and I will not be
>> making any change in this regard.
>>
>> But, thanks for raising the point and forcing me to double check!
>>
>
> Not my driver, but this looks like a classic “works now” change that may
> not hold up later, which is why I replied to something that isn’t really
> my business.
>
> Again, not my area, but I’ve been through this before. Generally,
> getting the abstractions right up front pays off.
Our policy so far has been to introduce abstractions when there is a
justifiable hardware difference within the (reduced) set of GPUs that we
support. Adding HALs imply using virtual calls, which are harder to
trace, or monomorphization using generic types, which complicate the
code (and are hard to justify when there is only one implementation).
`Tlb` is already an abstraction of sorts; the register accesses are
well-contained within this leaf module, so if the need arises to support
a different scheme this can be done transparently to callers, which is
what matter imho.
^ permalink raw reply
* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Eliot Courtney @ 2026-04-08 13:26 UTC (permalink / raw)
To: Joel Fernandes, Eliot Courtney, linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
rust-for-linux, Nikola Djukic, 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,
Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, joel, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <537a8c5a-3885-4c47-99f6-963b48ddf87d@nvidia.com>
On Tue Apr 7, 2026 at 10:59 PM JST, Joel Fernandes wrote:
> Hi Eliot,
>
> On 4/7/2026 9:42 AM, Eliot Courtney wrote:
>> On Tue Apr 7, 2026 at 6:55 AM JST, Joel Fernandes wrote:
>>>>> + /// Compute upper bound on page table pages needed for `num_virt_pages`.
>>>>> + ///
>>>>> + /// Walks from PTE level up through PDE levels, accumulating the tree.
>>>>> + pub(crate) fn pt_pages_upper_bound(&self, num_virt_pages: usize) -> usize {
>>>>> + let mut total = 0;
>>>>> +
>>>>> + // PTE pages at the leaf level.
>>>>> + let pte_epp = self.entries_per_page(self.pte_level());
>>>>> + let mut pages_at_level = num_virt_pages.div_ceil(pte_epp);
>>>>> + total += pages_at_level;
>>>>> +
>>>>> + // Walk PDE levels bottom-up (reverse of pde_levels()).
>>>>> + for &level in self.pde_levels().iter().rev() {
>>>>> + let epp = self.entries_per_page(level);
>>>>> +
>>>>> + // How many pages at this level do we need to point to
>>>>> + // the previous pages_at_level?
>>>>> + pages_at_level = pages_at_level.div_ceil(epp);
>>>>> + total += pages_at_level;
>>>>> + }
>>>>> +
>>>>> + total
>>>>> + }
>>>>> +}
>>>>> +
>>>>
>>>> We have a lot of matches on the MMU version here (and below in Pte, Pde,
>>>> DualPde). What about making MmuVersion into a trait (e.g. Mmu) with
>>>> associated types for Pte, Pde, DualPde which can implement traits
>>>> defining their common operations too?
>>>
>>> I coded this up and it did not look pretty, there's not much LOC savings and the
>>> code becomes harder to read because of parametrization of several functions. Also:
>>
>> Thanks for looking into it. Sorry to be a bother, but would you have a
>> branch around with the code? I'm curious what didn't look good about it.
>
> Sorry but I already mentioned that above, the parameterizing of dozens of
> function call sites, 3-4 new traits (because each struct like
> Pte/Pde/DualPde etc each need their own trait which different MMU versions
> implement) etc. The code because hard to read and readability is the top
> critical criteria for me - I am personally strictly against "Lets use shiny
> features in language at the cost of making code unreadable". Because that
> translates into bugs and nightmare for maintainability.
>
> I don't have the code at the moment, but if you still want to spend on time
> on this direction, feel free to share a tree. I am happy to take a look.
I had a go at this, you can see the branch here [1] - it might not be
perfect, but I think the shape is directionally good. It's structured so
the HEAD commit has the diff from the current approach to the
parametrised approach. The main decision is where to do the type
erasure, I chose in `Vmm` since it looks like the main top level API for
this code, but could do `BarUser` instead. I think it's overall better.
I also think Alex's point about associated types making it easier to use
the appropriate Bounded type is a good one.
[1]: https://github.com/Edgeworth/linux/commits/review/nova-mm-v10/
>>>> Then you can parameterise Vmm/PtWalk on this type.
>>>
>>> The match still to be done somewhere, so you end up matching on chipset to call
>>> the correct parametrized functions versus just passing in the parameter or
>>> chipset down, in some cases.
>>>
>>> For now I am inclined to leave it as is. Also there's a Rust pitfall we all
>>> learnt during the turing and other patch reviews, sometimes doing a bunch of
>>> matches is good especially if the number of variants are expected to be fixed
>>> (in the mm case, version 2 and version 3). Traits have some disadvantages too,
>>> example dyn traits have to heap-allocated, parametrizing can increase code size
>>> (due to monomorphization) etc.
>>
>> Yeah, it's just this is a lot of matches in a lot of places. And we have
>> ver2 / ver3 specific code leaking into the general pagetable.rs file. So
>
> That's not a leak, that's by design. pagetable.rs is where the matches are
> centralized, most of the code changes here on out should happen outside of
> this file.
>
> 31 out of 42 matches in the mm code are in pagetable.rs, so it is already
> centralized.
>
>> it would be really nice if we could find a way to improve this specific
>> aspect. We can reduce the match to happening in just one file.
>
> Assuming we know what we're improving. ;-)
>
>> You can> avoid heap allocation if you would like by making Vmm an enum,
>> for example, and doing the match based dispatch there at the top of the
>> API tree, rather than at the bottom where it fans out into a lot more
>> locations.
>
> heap allocation is not always free, this code sensitive to dynamic
> allocations in the kernel, due to MM reclaim and locking. I would like to
> keep it simple.
If you do the type erasure via enum in Vmm, you won't need to allocate
it on the heap. The branch I posted above has an example on how to do
this, although there might be a better way.
>
> thanks,
>
> --
> Joel Fernandes
^ permalink raw reply
* Re: [PATCH 2/8] firmware: efi: Never declare sysfb_primary_display on x86
From: Ard Biesheuvel @ 2026-04-08 13:45 UTC (permalink / raw)
To: Thomas Zimmermann, Javier Martinez Canillas, Arnd Bergmann,
Ilias Apalodimas, Huacai Chen, WANG Xuerui, maarten.lankhorst,
mripard, David Airlie, Simona Vetter, kys, haiyangz, Wei Liu,
decui, Long Li, Helge Deller
Cc: linux-arm-kernel, loongarch, linux-efi, linux-riscv, dri-devel,
linux-hyperv, linux-fbdev, kernel test robot
In-Reply-To: <20260402092305.208728-3-tzimmermann@suse.de>
Hi Thomas,
On Thu, 2 Apr 2026, at 11:09, Thomas Zimmermann wrote:
> The x86 architecture comes with its own instance of the global
> state variable sysfb_primary_display. Never declare it in the EFI
> subsystem. Fix the test for CONFIG_FIRMWARE_EDID accordingly.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: e65ca1646311 ("efi: export sysfb_primary_display for EDID")
> Cc: kernel test robot <lkp@intel.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: linux-efi@vger.kernel.org
> ---
> drivers/firmware/efi/efi-init.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Should this be sent out as a fix?
^ permalink raw reply
* Re: [PATCH 2/8] firmware: efi: Never declare sysfb_primary_display on x86
From: Thomas Zimmermann @ 2026-04-08 14:07 UTC (permalink / raw)
To: Ard Biesheuvel, Javier Martinez Canillas, Arnd Bergmann,
Ilias Apalodimas, Huacai Chen, WANG Xuerui, maarten.lankhorst,
mripard, David Airlie, Simona Vetter, kys, haiyangz, Wei Liu,
decui, Long Li, Helge Deller
Cc: linux-arm-kernel, loongarch, linux-efi, linux-riscv, dri-devel,
linux-hyperv, linux-fbdev, kernel test robot
In-Reply-To: <d0624a61-b96b-4b2f-89c2-029e8671039d@app.fastmail.com>
Hi
Am 08.04.26 um 15:45 schrieb Ard Biesheuvel:
> Hi Thomas,
>
> On Thu, 2 Apr 2026, at 11:09, Thomas Zimmermann wrote:
>> The x86 architecture comes with its own instance of the global
>> state variable sysfb_primary_display. Never declare it in the EFI
>> subsystem. Fix the test for CONFIG_FIRMWARE_EDID accordingly.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Fixes: e65ca1646311 ("efi: export sysfb_primary_display for EDID")
>> Cc: kernel test robot <lkp@intel.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Ard Biesheuvel <ardb@kernel.org>
>> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> Cc: linux-efi@vger.kernel.org
>> ---
>> drivers/firmware/efi/efi-init.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
> Should this be sent out as a fix?
Yes, please.
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Joel Fernandes @ 2026-04-08 16:58 UTC (permalink / raw)
To: Eliot Courtney, linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
rust-for-linux, Nikola Djukic, 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,
Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
Elle Rhumsaa, alexeyi, joel, linux-doc, amd-gfx, intel-gfx,
intel-xe, linux-fbdev
In-Reply-To: <DHNT32C2Q5HN.LLME0RV17Z8V@nvidia.com>
Hi Eliot,
On 4/8/2026 9:26 AM, Eliot Courtney wrote:
> On Tue Apr 7, 2026 at 10:59 PM JST, Joel Fernandes wrote:
>> Hi Eliot,
>>
>> On 4/7/2026 9:42 AM, Eliot Courtney wrote:
>>> On Tue Apr 7, 2026 at 6:55 AM JST, Joel Fernandes wrote:
>>>>>> + /// Compute upper bound on page table pages needed for `num_virt_pages`.
>>>>>> + ///
>>>>>> + /// Walks from PTE level up through PDE levels, accumulating the tree.
>>>>>> + pub(crate) fn pt_pages_upper_bound(&self, num_virt_pages: usize) -> usize {
>>>>>> + let mut total = 0;
>>>>>> +
>>>>>> + // PTE pages at the leaf level.
>>>>>> + let pte_epp = self.entries_per_page(self.pte_level());
>>>>>> + let mut pages_at_level = num_virt_pages.div_ceil(pte_epp);
>>>>>> + total += pages_at_level;
>>>>>> +
>>>>>> + // Walk PDE levels bottom-up (reverse of pde_levels()).
>>>>>> + for &level in self.pde_levels().iter().rev() {
>>>>>> + let epp = self.entries_per_page(level);
>>>>>> +
>>>>>> + // How many pages at this level do we need to point to
>>>>>> + // the previous pages_at_level?
>>>>>> + pages_at_level = pages_at_level.div_ceil(epp);
>>>>>> + total += pages_at_level;
>>>>>> + }
>>>>>> +
>>>>>> + total
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>
>>>>> We have a lot of matches on the MMU version here (and below in Pte, Pde,
>>>>> DualPde). What about making MmuVersion into a trait (e.g. Mmu) with
>>>>> associated types for Pte, Pde, DualPde which can implement traits
>>>>> defining their common operations too?
>>>>
>>>> I coded this up and it did not look pretty, there's not much LOC savings and the
>>>> code becomes harder to read because of parametrization of several functions. Also:
>>>
>>> Thanks for looking into it. Sorry to be a bother, but would you have a
>>> branch around with the code? I'm curious what didn't look good about it.
>>
>> Sorry but I already mentioned that above, the parameterizing of dozens of
>> function call sites, 3-4 new traits (because each struct like
>> Pte/Pde/DualPde etc each need their own trait which different MMU versions
>> implement) etc. The code because hard to read and readability is the top
>> critical criteria for me - I am personally strictly against "Lets use shiny
>> features in language at the cost of making code unreadable". Because that
>> translates into bugs and nightmare for maintainability.
>>
>> I don't have the code at the moment, but if you still want to spend on time
>> on this direction, feel free to share a tree. I am happy to take a look.
>
> I had a go at this, you can see the branch here [1] - it might not be
> perfect, but I think the shape is directionally good. It's structured so
> the HEAD commit has the diff from the current approach to the
> parametrised approach. The main decision is where to do the type
> erasure, I chose in `Vmm` since it looks like the main top level API for
> this code, but could do `BarUser` instead. I think it's overall better.
> I also think Alex's point about associated types making it easier to use
> the appropriate Bounded type is a good one.
>
> [1]: https://github.com/Edgeworth/linux/commits/review/nova-mm-v10/
First, thanks for the effort. I looked through this, its pretty much what I
had before when I used traits. I don't think it is better to be honest. In
fact your version is worse, it adds many new types and things like the
following which I did not need before.
To put it mildly, the following suggestion should not be anywhere near my code:
/// Type-erased MMU-specific [`Vmm`] implementations.
enum VmmInner {
/// `Vmm` implementation for MMU v2.
V2(VmmImpl<MmuV2>),
/// `Vmm` implementation for MMU v3.
V3(VmmImpl<MmuV3>),
}
/// MMU-specific [`Vmm`] implementation.
struct VmmImpl<M: Mmu> {
Seriously, I have to pass on this. :-)
And, you unfortunately seem to have ignored my point about requiring 4 NEW
traits (Mmu, PteOps, PdeOps, DualPdeOps etc), which I did not need before.
So you're making the code much much worse than before actually. We don't
new traits and types pointlessly.
The only positive thing I could take away from your diff is the following
(I thought I had already done that, but I'll double check).
- fn level_index(&self, level: u64) -> u64 {
+ fn level_index(&self, level: PageTableLevel) -> u64 {
Also you're parametrizing VirtualAddress as well which I did not have before:
- let va = VirtualAddress::from(vfn);
+ let va = M::va(VirtualAddress::from(vfn));
This is another step back.
> I also think Alex's point about associated types making it easier to use
> the appropriate Bounded type is a good one.
I will reply to Alex thread, separately. I have some good data that should
hopefully convince you and Alex that my approach in this patch is better
(Version struct based dispatch than monomorphization). I would emphasize,
as we all know, that we should make optimizations and changes based on real
data and proper technical arguments so in the spirit of that, I have
collected data with both approaches and I will reply to Alex's email with
all that in there.
Also, the bounded types usage is orthogonal to version-parameterization.
That can be done regardless, we already use bitfield macro in this code and
can use bounded types within that if needed to restrict type creation. So I
don't think we should mix the 2 concepts "bounded types" and
"parameterization".
thanks,
--
Joel Fernandes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox