From owner-dev-commits-src-main@freebsd.org Mon Jan 18 07:17:52 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id AD0424E33E9 for ; Mon, 18 Jan 2021 07:17:52 +0000 (UTC) (envelope-from tsoome@me.com) Received: from pv50p00im-ztdg10012101.me.com (pv50p00im-ztdg10012101.me.com [17.58.6.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4DK35p4zZXz3JtB for ; Mon, 18 Jan 2021 07:17:50 +0000 (UTC) (envelope-from tsoome@me.com) Received: from nazgul.lan (148-52-235-80.sta.estpak.ee [80.235.52.148]) by pv50p00im-ztdg10012101.me.com (Postfix) with ESMTPSA id 4977E84016F; Mon, 18 Jan 2021 07:17:47 +0000 (UTC) From: Toomas Soome Message-Id: <4CC2ED16-9C4A-4821-BAB0-F1B01C525EFD@me.com> Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.40.0.2.32\)) Subject: Re: git: 1caed70c6264 - main - loader: update gfx module Date: Mon, 18 Jan 2021 09:17:44 +0200 In-Reply-To: Cc: Toomas Soome , "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" To: Oliver Pinter References: <202101172216.10HMGH1D076854@gitrepo.freebsd.org> X-Mailer: Apple Mail (2.3654.40.0.2.32) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.343, 18.0.737 definitions=2021-01-18_07:2021-01-15, 2021-01-18 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-2006250000 definitions=main-2101180042 X-Rspamd-Queue-Id: 4DK35p4zZXz3JtB X-Spamd-Bar: --- X-Spamd-Result: default: False [-3.50 / 15.00]; TO_DN_EQ_ADDR_SOME(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; TO_DN_SOME(0.00)[]; R_SPF_ALLOW(-0.20)[+ip4:17.58.0.0/16:c]; FREEMAIL_FROM(0.00)[me.com]; MV_CASE(0.50)[]; RCPT_COUNT_FIVE(0.00)[5]; DKIM_TRACE(0.00)[me.com:+]; DMARC_POLICY_ALLOW(-0.50)[me.com,quarantine]; NEURAL_HAM_SHORT(-1.00)[-1.000]; FREEMAIL_TO(0.00)[gmail.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+,1:+,2:~]; FREEMAIL_ENVFROM(0.00)[me.com]; ASN(0.00)[asn:714, ipnet:17.58.0.0/20, country:US]; MID_RHS_MATCH_FROM(0.00)[]; RBL_DBL_DONT_QUERY_IPS(0.00)[17.58.6.49:from]; DWL_DNSWL_NONE(0.00)[me.com:dkim]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; R_DKIM_ALLOW(-0.20)[me.com:s=1a1hai]; FREEFALL_USER(0.00)[tsoome]; FROM_HAS_DN(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; TAGGED_RCPT(0.00)[]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; SPAMHAUS_ZRD(0.00)[17.58.6.49:from:127.0.2.255]; RECEIVED_SPAMHAUS_PBL(0.00)[80.235.52.148:received]; TO_MATCH_ENVRCPT_SOME(0.00)[]; RCVD_IN_DNSWL_NONE(0.00)[17.58.6.49:from]; RWL_MAILSPIKE_POSSIBLE(0.00)[17.58.6.49:from]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; MAILMAN_DEST(0.00)[dev-commits-src-main] Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.34 X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jan 2021 07:17:52 -0000 > On 18. Jan 2021, at 02:39, Oliver Pinter = wrote: >=20 >=20 >=20 > On Sunday, January 17, 2021, Toomas Soome > wrote: > The branch main has been updated by tsoome: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3D1caed70c62646a5978bbeb4562bc3ae2= dabc874a = >=20 > commit 1caed70c62646a5978bbeb4562bc3ae2dabc874a > Author: Toomas Soome > AuthorDate: 2021-01-17 22:09:18 +0000 > Commit: Toomas Soome > 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 > #include > #include > @@ -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 = mailing list > https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main = > To unsubscribe, send any mail to = "dev-commits-src-main-unsubscribe@freebsd.org = "