On Wed, May 27, 2020 at 12:25:00PM +0200, Mark Kettenis wrote:
> > Date: Wed, 27 May 2020 19:39:07 +1000
> > From: Jonathan Gray <j...@jsg.id.au>
> > 
> > When testing the row and column increase for efifb on a 1920x1080
> > display I noticed the top part of the screen continues to contain part
> > of a white on blue line from earlier in the dmesg even after the machine
> > has finished booting.
> > 
> > RI_CENTER changes the ri_bits offset, doing RI_CLEARMARGINS in cnremap
> > clears the fragment of a line caused by using RI_CENTER.
> 
> ok kettenis@

I had a different fix for this issue, which prevents the margins to
appear in the first place.

This is why the margins appear:

In rasops(9), if 'ri_emuwidth' is larger than 'ri_width', it is set to
'ri_width', and similarly for 'ri_emuheight' relative to 'ri_height'.

In efifb_cnattach_common(), we call rasops_init() with EFIFB_HEIGHT
and EFIFB_WIDTH as parameters, so on smaller screens or with larger
fonts, or when increasing the EFIFB_HEIGHT and EFIFB_WIDTH values,
'ri_emu{width,height}' becomes equals to 'ri_{width,height}' and no
centering happens.

Then in efifb_cnremap() and efifb_attach(), efifb_std_descr.nrows and
efifb_std_descr.ncols are used instead, and in this case 'ri_emuwidth'
and 'ri_emuheight' can now be a few pixels smaller than 'ri_width' and
'ri_height' and the text area is centered where it previously wasn't,
causing the content previously displayed in what are now margins to
remain there.

Here is a rebased version of the diff:

Index: sys/arch/amd64/amd64/efifb.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/efifb.c,v
retrieving revision 1.31
diff -u -p -r1.31 efifb.c
--- sys/arch/amd64/amd64/efifb.c        27 May 2020 07:48:02 -0000      1.31
+++ sys/arch/amd64/amd64/efifb.c        28 May 2020 07:17:49 -0000
@@ -222,7 +222,7 @@ efifb_attach(struct device *parent, stru
                ri->ri_flg &= ~RI_CLEAR;
                ri->ri_flg |= RI_VCONS | RI_WRONLY;
 
-               rasops_init(ri, efifb_std_descr.nrows, efifb_std_descr.ncols);
+               rasops_init(ri, EFIFB_HEIGHT, EFIFB_WIDTH);
 
                ri->ri_ops.pack_attr(ri->ri_active, 0, 0, 0, &defattr);
                wsdisplay_cnattach(&efifb_std_descr, ri->ri_active, ccol, crow,
@@ -480,7 +480,7 @@ efifb_cnremap(void)
        ri->ri_flg &= ~RI_CLEAR;
        ri->ri_flg |= RI_CENTER | RI_WRONLY;
 
-       rasops_init(ri, efifb_std_descr.nrows, efifb_std_descr.ncols);
+       rasops_init(ri, EFIFB_HEIGHT, EFIFB_WIDTH);
 
        efifb_early_cleanup();
 }

Because the efifb resolution doesn't change, this ensures 'ri_emuwidth'
and 'ri_emuheight' will always get the same value when we remap and
later when we attach, so the text area is always displayed at the same
position.

I verified it still works, and it solves the issue for me on a 1920x1080
screen.

It was commited [1] and reverted [2] at the time we tried to increase
EFIFB_HEIGHT / EFIFB_WIDTH. I reverted it to be on the safe side as we
were nearing a release and it wasn't useful on its own after reverting
the other changes.

[1] 
https://cvsweb.openbsd.org/src/sys/arch/amd64/amd64/efifb.c?rev=1.20&content-type=text/x-cvsweb-markup
[2] 
https://cvsweb.openbsd.org/src/sys/arch/amd64/amd64/efifb.c?rev=1.23&content-type=text/x-cvsweb-markup

Reply via email to