Date: Mon, 18 Jan 2021 09:17:44 +0200 From: Toomas Soome <tsoome@me.com> To: Oliver Pinter <oliver.pntr@gmail.com> Cc: Toomas Soome <tsoome@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org> Subject: Re: git: 1caed70c6264 - main - loader: update gfx module Message-ID: <4CC2ED16-9C4A-4821-BAB0-F1B01C525EFD@me.com> In-Reply-To: <CAPjTQNHxuRRYfD3FsDL8N1i=1n5_JNm6zhRm=D-%2BasNSUt4GJA@mail.gmail.com> References: <202101172216.10HMGH1D076854@gitrepo.freebsd.org> <CAPjTQNHxuRRYfD3FsDL8N1i=1n5_JNm6zhRm=D-%2BasNSUt4GJA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> On 18. Jan 2021, at 02:39, Oliver Pinter <oliver.pntr@gmail.com> = wrote: >=20 >=20 >=20 > On Sunday, January 17, 2021, Toomas Soome <tsoome@freebsd.org = <mailto:tsoome@freebsd.org>> wrote: > The branch main has been updated by tsoome: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3D1caed70c62646a5978bbeb4562bc3ae2= dabc874a = <https://cgit.freebsd.org/src/commit/?id=3D1caed70c62646a5978bbeb4562bc3ae= 2dabc874a> >=20 > commit 1caed70c62646a5978bbeb4562bc3ae2dabc874a > Author: Toomas Soome <tsoome@FreeBSD.org> > AuthorDate: 2021-01-17 22:09:18 +0000 > Commit: Toomas Soome <tsoome@FreeBSD.org> > CommitDate: 2021-01-17 22:15:36 +0000 >=20 > loader: update gfx module >=20 > Update from illumos review process. > Add more comments, drop memory buffer from blt functions. > --- > stand/common/gfx_fb.c | 118 = ++++++++++++++++++++++++++++++++------------------ > 1 file changed, 76 insertions(+), 42 deletions(-) >=20 > diff --git a/stand/common/gfx_fb.c b/stand/common/gfx_fb.c > index 9342521fd0cf..d98e6d1bae4b 100644 > --- a/stand/common/gfx_fb.c > +++ b/stand/common/gfx_fb.c > @@ -29,6 +29,61 @@ > * $FreeBSD$ > */ >=20 > +/* > + * The workhorse here is gfxfb_blt(). It is implemented to mimic UEFI > + * GOP Blt, and allows us to fill the rectangle on screen, copy > + * rectangle from video to buffer and buffer to video and video to = video. > + * Such implementation does allow us to have almost identical = implementation > + * for both BIOS VBE and UEFI. > + * > + * ALL pixel data is assumed to be 32-bit BGRA (byte order Blue, = Green, Red, > + * Alpha) format, this allows us to only handle RGB data and not to = worry > + * about mixing RGB with indexed colors. > + * Data exchange between memory buffer and video will translate BGRA > + * and native format as following: > + * > + * 32-bit to/from 32-bit is trivial case. > + * 32-bit to/from 24-bit is also simple - we just drop the alpha = channel. > + * 32-bit to/from 16-bit is more complicated, because we nee to = handle > + * data loss from 32-bit to 16-bit. While reading/writing from/to = video, we > + * need to apply masks of 16-bit color components. This will preserve > + * colors for terminal text. For 32-bit truecolor PMG images, we need = to > + * translate 32-bit colors to 15/16 bit colors and this means data = loss. > + * There are different algorithms how to perform such color space = reduction, > + * we are currently using bitwise right shift to reduce color space = and so far > + * this technique seems to be sufficient (see also gfx_fb_putimage(), = the > + * end of for loop). > + * 32-bit to/from 8-bit is the most troublesome because 8-bit colors = are > + * indexed. =46rom video, we do get color indexes, and we do = translate > + * color index values to RGB. To write to video, we again need to = translate > + * RGB to color index. Additionally, we need to translate between VGA = and > + * console colors. > + * > + * Our internal color data is represented using BGRA format. But the = hardware > + * used indexed colors for 8-bit colors (0-255) and for this mode we = do > + * need to perform translation to/from BGRA and index values. > + * > + * - paletteentry RGB <-> index - > + * BGRA BUFFER <----/ \ - VIDEO > + * \ / > + * - RGB (16/24/32) - > + * > + * To perform index to RGB translation, we use palette table = generated > + * from when we set up 8-bit mode video. We cannot read palette data = from > + * the hardware, because not all hardware supports reading it. > + * > + * BGRA to index is implemented in rgb_to_color_index() by searching > + * palette array for closest match of RBG values. > + * > + * Note: In 8-bit mode, We do store first 16 colors to palette = registers > + * in VGA color order, this serves two purposes; firstly, > + * if palette update is not supported, we still have correct 16 = colors. > + * Secondly, the kernel does get correct 16 colors when some other = boot > + * loader is used. However, the palette map for 8-bit colors is using > + * console color ordering - this does allow us to skip translation > + * from VGA colors to console colors, while we are reading RGB data. > + */ > + > #include <sys/cdefs.h> > #include <sys/param.h> > #include <stand.h> > @@ -257,7 +312,7 @@ rgb_to_color_index(uint8_t r, uint8_t g, uint8_t = b) > int diff; >=20 > color =3D 0; > - best =3D NCMAP * NCMAP * NCMAP; > + best =3D 255 * 255 * 255; >=20 > This change was intended to change from a named constant to magic = number?=20 yes, one thing is that NCMAP is wrong named constant to be used in this = context. We want to start with large enough value > 255^2 + 255^2 + = 255^2, where 255 is max value of RGB component. In that sense even = UINT32_MAX is ok, but I think, 255 * 255 * 255 is visually better = (better than UINT8_MAX * UINT8_MAX * UINT8_MAX). rgds, toomas > =20 > for (k =3D 0; k < NCMAP; k++) { > diff =3D r - pe8[k].Red; > dist =3D diff * diff; > @@ -337,7 +392,6 @@ gfx_mem_wr4(uint8_t *base, size_t size, uint32_t = o, uint32_t v) > *(uint32_t *)(base + o) =3D v; > } >=20 > -/* Our GFX Block transfer toolkit. */ > static int gfxfb_blt_fill(void *BltBuffer, > uint32_t DestinationX, uint32_t DestinationY, > uint32_t Width, uint32_t Height) > @@ -409,6 +463,8 @@ static int gfxfb_blt_fill(void *BltBuffer, > case 4: > gfx_mem_wr4(destination, size, off, = data); > break; > + default: > + return (EINVAL); > } > off +=3D bpp; > } > @@ -430,7 +486,7 @@ gfxfb_blt_video_to_buffer(void *BltBuffer, = uint32_t SourceX, uint32_t SourceY, > uint32_t x, sy, dy; > uint32_t bpp, pitch, copybytes; > off_t off; > - uint8_t *source, *destination, *buffer, *sb; > + uint8_t *source, *destination, *sb; > uint8_t rm, rp, gm, gp, bm, bp; > bool bgra; >=20 > @@ -468,36 +524,21 @@ gfxfb_blt_video_to_buffer(void *BltBuffer, = uint32_t SourceX, uint32_t SourceY, > ffs(gm) - 1 =3D=3D 8 && gp =3D=3D 8 && > ffs(bm) - 1 =3D=3D 8 && bp =3D=3D 0; >=20 > - if (bgra) { > - buffer =3D NULL; > - } else { > - buffer =3D malloc(copybytes); > - if (buffer =3D=3D NULL) > - return (ENOMEM); > - } > - > for (sy =3D SourceY, dy =3D DestinationY; dy < Height + = DestinationY; > sy++, dy++) { > off =3D sy * pitch + SourceX * bpp; > source =3D gfx_get_fb_address() + off; > + destination =3D (uint8_t *)BltBuffer + dy * Delta + > + DestinationX * sizeof (*p); >=20 > if (bgra) { > - destination =3D (uint8_t *)BltBuffer + dy * = Delta + > - DestinationX * sizeof (*p); > + bcopy(source, destination, copybytes); > } else { > - destination =3D buffer; > - } > - > - bcopy(source, destination, copybytes); > - > - if (!bgra) { > for (x =3D 0; x < Width; x++) { > uint32_t c =3D 0; >=20 > - p =3D (void *)((uint8_t *)BltBuffer + > - dy * Delta + > - (DestinationX + x) * sizeof (*p)); > - sb =3D buffer + x * bpp; > + p =3D (void *)(destination + x * = sizeof (*p)); > + sb =3D source + x * bpp; > switch (bpp) { > case 1: > c =3D *sb; > @@ -511,6 +552,8 @@ gfxfb_blt_video_to_buffer(void *BltBuffer, = uint32_t SourceX, uint32_t SourceY, > case 4: > c =3D *(uint32_t *)sb; > break; > + default: > + return (EINVAL); > } >=20 > if (bpp =3D=3D 1) { > @@ -527,7 +570,6 @@ gfxfb_blt_video_to_buffer(void *BltBuffer, = uint32_t SourceX, uint32_t SourceY, > } > } >=20 > - free(buffer); > return (0); > } >=20 > @@ -544,7 +586,7 @@ gfxfb_blt_buffer_to_video(void *BltBuffer, = uint32_t SourceX, uint32_t SourceY, > uint32_t x, sy, dy; > uint32_t bpp, pitch, copybytes; > off_t off; > - uint8_t *source, *destination, *buffer; > + uint8_t *source, *destination; > uint8_t rm, rp, gm, gp, bm, bp; > bool bgra; >=20 > @@ -582,13 +624,6 @@ gfxfb_blt_buffer_to_video(void *BltBuffer, = uint32_t SourceX, uint32_t SourceY, > ffs(gm) - 1 =3D=3D 8 && gp =3D=3D 8 && > ffs(bm) - 1 =3D=3D 8 && bp =3D=3D 0; >=20 > - if (bgra) { > - buffer =3D NULL; > - } else { > - buffer =3D malloc(copybytes); > - if (buffer =3D=3D NULL) > - return (ENOMEM); > - } > for (sy =3D SourceY, dy =3D DestinationY; sy < Height + = SourceY; > sy++, dy++) { > off =3D dy * pitch + DestinationX * bpp; > @@ -597,6 +632,7 @@ gfxfb_blt_buffer_to_video(void *BltBuffer, = uint32_t SourceX, uint32_t SourceY, > if (bgra) { > source =3D (uint8_t *)BltBuffer + sy * Delta + > SourceX * sizeof (*p); > + bcopy(source, destination, copybytes); > } else { > for (x =3D 0; x < Width; x++) { > uint32_t c; > @@ -615,35 +651,33 @@ gfxfb_blt_buffer_to_video(void *BltBuffer, = uint32_t SourceX, uint32_t SourceY, > off =3D x * bpp; > switch (bpp) { > case 1: > - gfx_mem_wr1(buffer, copybytes, > + gfx_mem_wr1(destination, = copybytes, > off, (c < 16) ? > cons_to_vga_colors[c] : = c); > break; > case 2: > - gfx_mem_wr2(buffer, copybytes, > + gfx_mem_wr2(destination, = copybytes, > off, c); > break; > case 3: > - gfx_mem_wr1(buffer, copybytes, > + gfx_mem_wr1(destination, = copybytes, > off, (c >> 16) & 0xff); > - gfx_mem_wr1(buffer, copybytes, > + gfx_mem_wr1(destination, = copybytes, > off + 1, (c >> 8) & 0xff); > - gfx_mem_wr1(buffer, copybytes, > + gfx_mem_wr1(destination, = copybytes, > off + 2, c & 0xff); > break; > case 4: > - gfx_mem_wr4(buffer, copybytes, > + gfx_mem_wr4(destination, = copybytes, > x * bpp, c); > break; > + default: > + return (EINVAL); > } > } > - source =3D buffer; > } > - > - bcopy(source, destination, copybytes); > } >=20 > - free(buffer); > return (0); > } >=20 > _______________________________________________ > dev-commits-src-main@freebsd.org = <mailto:dev-commits-src-main@freebsd.org> mailing list > https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main = <https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main>; > To unsubscribe, send any mail to = "dev-commits-src-main-unsubscribe@freebsd.org = <mailto:dev-commits-src-main-unsubscribe@freebsd.org>"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4CC2ED16-9C4A-4821-BAB0-F1B01C525EFD>