Re: clear margins when remapping efifb

2020-05-28 Thread Jonathan Gray
On Thu, May 28, 2020 at 12:23:26PM +0200, Frederic Cambus wrote:
> 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 
> > > 
> > > 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 -  1.31
> +++ sys/arch/amd64/amd64/efifb.c  28 May 2020 07:17:49 -
> @@ -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

Thanks I missed that commit in the log.

The result of your diff is the area the text starts at is higher on the
panel matching amdgpu and there are no glitches when booting via efi
with amdgpu disabled.

ok jsg@



Re: clear margins when remapping efifb

2020-05-28 Thread Frederic Cambus
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 
> > 
> > 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.c27 May 2020 07:48:02 -  1.31
+++ sys/arch/amd64/amd64/efifb.c28 May 2020 07:17:49 -
@@ -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



Re: clear margins when remapping efifb

2020-05-27 Thread Mark Kettenis
> Date: Wed, 27 May 2020 19:39:07 +1000
> From: Jonathan Gray 
> 
> 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@

> Index: efifb.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/efifb.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 efifb.c
> --- efifb.c   27 May 2020 07:48:02 -  1.31
> +++ efifb.c   27 May 2020 09:27:50 -
> @@ -219,7 +219,7 @@ efifb_attach(struct device *parent, stru
>   crow = ri->ri_crow;
>  
>   efifb_rasops_preinit(fb);
> - ri->ri_flg &= ~RI_CLEAR;
> + ri->ri_flg &= ~(RI_CLEAR | RI_CLEARMARGINS);
>   ri->ri_flg |= RI_VCONS | RI_WRONLY;
>  
>   rasops_init(ri, efifb_std_descr.nrows, efifb_std_descr.ncols);
> @@ -478,7 +478,7 @@ efifb_cnremap(void)
>  
>   efifb_rasops_preinit(fb);
>   ri->ri_flg &= ~RI_CLEAR;
> - ri->ri_flg |= RI_CENTER | RI_WRONLY;
> + ri->ri_flg |= RI_CENTER | RI_WRONLY | RI_CLEARMARGINS;
>  
>   rasops_init(ri, efifb_std_descr.nrows, efifb_std_descr.ncols);
>  
> 
> 



clear margins when remapping efifb

2020-05-27 Thread Jonathan Gray
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.

Index: efifb.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/efifb.c,v
retrieving revision 1.31
diff -u -p -r1.31 efifb.c
--- efifb.c 27 May 2020 07:48:02 -  1.31
+++ efifb.c 27 May 2020 09:27:50 -
@@ -219,7 +219,7 @@ efifb_attach(struct device *parent, stru
crow = ri->ri_crow;
 
efifb_rasops_preinit(fb);
-   ri->ri_flg &= ~RI_CLEAR;
+   ri->ri_flg &= ~(RI_CLEAR | RI_CLEARMARGINS);
ri->ri_flg |= RI_VCONS | RI_WRONLY;
 
rasops_init(ri, efifb_std_descr.nrows, efifb_std_descr.ncols);
@@ -478,7 +478,7 @@ efifb_cnremap(void)
 
efifb_rasops_preinit(fb);
ri->ri_flg &= ~RI_CLEAR;
-   ri->ri_flg |= RI_CENTER | RI_WRONLY;
+   ri->ri_flg |= RI_CENTER | RI_WRONLY | RI_CLEARMARGINS;
 
rasops_init(ri, efifb_std_descr.nrows, efifb_std_descr.ncols);