Messages in this thread Patch in this message | | | Date | Wed, 8 Jun 2022 09:30:53 +0200 | Subject | Re: [PATCH 10/36] tty/vt: consolemap: introduce UNI_*() macros | From | Jiri Slaby <> |
| |
On 08. 06. 22, 8:59, Jiri Slaby wrote: > On 07. 06. 22, 15:47, Ilpo Järvinen wrote: >> On Tue, 7 Jun 2022, Jiri Slaby wrote: >> >>> The code currently does shift, OR, and AND logic directly in the code. >>> It is not much obvious what happens there. Therefore define four macros >>> for that purpose and use them in the code. We use GENMASK() so that it >>> is clear which bits serve what purpose: >>> - UNI_GLYPH: bits 0.. 5 >>> - UNI_ROW: bits 6..10 >>> - UNI_DIR: bits 11..31 >>> >>> Signed-off-by: Jiri Slaby <jslaby@suse.cz> >>> --- >>> drivers/tty/vt/consolemap.c | 21 +++++++++++++-------- >>> 1 file changed, 13 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c >>> index 016c1a0b4290..e5fd225e87bd 100644 >>> --- a/drivers/tty/vt/consolemap.c >>> +++ b/drivers/tty/vt/consolemap.c >>> @@ -190,6 +190,11 @@ static int inv_translate[MAX_NR_CONSOLES]; >>> #define UNI_DIR_ROWS 32U >>> #define UNI_ROW_GLYPHS 64U >>> +#define UNI_DIR(uni) ( (uni) >> 11) >>> +#define UNI_ROW(uni) (((uni) & GENMASK(10, 6)) >> 6) >> >> This is opencoding what FIELD_GET() does. Maybe just define these as >> masks and use FIELD_GET in the code below. > > Ah, great -- I was thinking there should be something for that purpose > already, but didn't find this. But let's define these UNI_* macros using > appropriate FIELD_GET(). (And not using FIELD_GET() in the code.) > >>> +#define UNI_GLYPH(uni) ( (uni) & GENMASK( 5, 0)) > thanks,
JFYI, I ended up with this diff to the original approach: --- a/drivers/tty/vt/consolemap.c +++ b/drivers/tty/vt/consolemap.c @@ -23,6 +23,8 @@ * stack overflow. */
+#include <linux/bitfield.h> +#include <linux/bits.h> #include <linux/module.h> #include <linux/kd.h> #include <linux/errno.h> @@ -190,10 +192,17 @@ static int inv_translate[MAX_NR_CONSOLES]; #define UNI_DIR_ROWS 32U #define UNI_ROW_GLYPHS 64U
-#define UNI_DIR(uni) ( (uni) >> 11) -#define UNI_ROW(uni) (((uni) & GENMASK(10, 6)) >> 6) -#define UNI_GLYPH(uni) ( (uni) & GENMASK( 5, 0)) -#define UNI(dir, row, glyph) (((dir) << 11) | ((row) << 6) | (glyph)) +#define UNI_DIR_BITS(max) GENMASK((max), 11) +#define UNI_ROW_BITS GENMASK(10, 6) +#define UNI_GLYPH_BITS GENMASK( 5, 0) + +#define UNI_DIR(uni) FIELD_GET(UNI_DIR_BITS(sizeof(uni) * 8 - 1), (uni)) +#define UNI_ROW(uni) FIELD_GET(UNI_ROW_BITS, (uni)) +#define UNI_GLYPH(uni) FIELD_GET(UNI_GLYPH_BITS, (uni)) + +#define UNI(dir, row, glyph) (FIELD_PREP(UNI_DIR_BITS(31), (dir)) | \ + FIELD_PREP(UNI_ROW_BITS, (row)) | \ + FIELD_PREP(UNI_GLYPH_BITS, (glyph)))
/** * struct uni_pagedict -- unicode directory ======================================================= More text, but easier to follow, I think. except the UNI_DIR_BITS() has to have a parameter, otherwise compilation raises a too-big value warning with use of UNI_DIR() in con_insert_unipair() where uni is only of ushort type. Alternatively, we can cast uni to u32, but that produces worse assembly (extensions to u32 here and there).
thanks, -- js suse labs
| |