Re: [9front] displayport thinkpad x230 external monitor black screen

[unobe@cpan.org]

Quoth qwx@sciops.net:
> On Sun Apr  9 08:58:34 +0200 2023, unobe@cpan.org wrote:
> > I figured out the problem--the DisplayPort link training needed to be
> > implemented more fully.  I didn't know really much at all about
> > DisplayPort; that has changed.  I've attached a patch that implements
> > pattern 1 & 2 training for HBR, along with fixing a couple other minor
> > things I came across.[1]
> > 
> > I'd be curious if the patch works for you, igor and qwx.  I don't see
> > why it wouldn't, but who knows.  Reader be warned: I'm not a C
> > programmer, so I'm sure there's much that can be improved upon.
> 
> Alright, sorry for taking so long.  I'm going to test the patch on a
> w500 and a w520 (resp.  gm965 and sandybridge, iirc) as soon as I'm
> able.

Not at all! I appreciate the feedback. 

> I don't actually have any remarks on what you did itself, looks
> good; I only have one comment on style: please be consistent with the
> rest of the code, mostly add spacing around operators (for example the
> shifts), and remove useless parenthesis (less important).

I attempted to, but apparently failed.  Admittedly, it was a manual
check on my part.  C programming is not yet native to me, and I'm
still trying to learn the formatting style.  Is style(6) still the
standard?  Or is there a C beautifier that can be used so that this
(valid) sort of criticism is then a simple matter of just having the
contributor run a script over the code?  That is, if I run cb(1) using
the '-s' flag, will that suffice?  Gofmt was a smart move.  Perl::Tidy
(which I used in my day job) helps with this sort of thing as well,
and allows a comment to "turn off" beautifying for a section of code.
That sort of thing is helpful when the formatting itself is
non-standard yet provides clarity to the code.  I didn't see the same
ability with cb(1), other than it not reformatting structure
initializers.

> The only other thing:
> 
> > +	while(--i){
> > +		if(memcmp(tmp+i, magic, 8) == 0){
> > +			trace("magic begins at index %d, shifting\n", i);
> > +			memcpy(buf, tmp+i, 256-i);
> > +			memcpy(buf+(256-i), tmp, i);
> > +			i = 1;
> > +		}
> > +	}
> 
> Why not just a `break;' there?  Anyway, not important.

I vacillated on this exact thing, and apparently chose the road less
traveled by, and that has made all the difference.

> > To test my patch, I defined this test function to ensure I could get
> > my working display back.  I didn't need to add anything to /lib/vgadb.
> > 
> > fn lg {
> > 	@ {
> > 		rfork n; aux/realemu; cd /sys/src/cmd/aux/vga; mk; /sys/src/cmd/aux/vga/6.out -v -V -m igfx -l 2560x1080; sleep 5; aux/vga -m igfx -l 1366x768
> > 	}
> > }
> 
> You don't need to run aux/realemu unless you will use vesa, which you
> don't.

Thanks; I'll need to explore when I need to use this in my day-to-day
usage, because I recall having to load it for something like looking
for valid modes.  This reminds me of another confusing
point--realemu(8) refers to /dev/realmode, yet arch(3) refers to
/dev/realmodemem, the latter of which is actually provided.  I have
assumed the docs for realemu(8) are wrong and need to be updated.

  Also, in practice, do you still have to run aux/vga twice, or
> is that just for testing?

I use it for testing because otherwise I won't have a screen.  The
mouse control for the window loses focus, so I can't just keep typing
'lcd' to restore, thus the 'sleep 5;'. Perhaps it's more of a problem
with aux/vga not failing and reverting to the Last Known Good like it
should, but the 'sleep' method works for me now.

> Do you need this function at all now, that
> is, does it not work by just having `monitor=auto' in plan9.ini?

I have not tried that.  It's an external monitor and not always
connected.  I don't see 'monitor=auto' described in plan9.ini(8).  I'd
have to look at the implementation to see exactly what it does, and if
it's "smart" enough to switch to the built-in LCD screen when the
external isn't connected.

Sincerely,
Romano