[Intel-gfx] [PATCH v7 09/11] drm: uevent for connector status change

Tue May 21 09:01:29 UTC 2019

On Tue, 21 May 2019 09:52:50 +0200
Daniel Vetter <daniel at ffwll.ch> wrote:

> On Tue, May 21, 2019 at 8:55 AM Pekka Paalanen <ppaalanen at gmail.com> wrote:
> >
> > On Mon, 20 May 2019 18:11:07 +0200
> > Daniel Vetter <daniel at ffwll.ch> wrote:
> >  
> > > On Fri, May 17, 2019 at 01:08:24PM +0300, Pekka Paalanen wrote:  
> > > > On Thu, 16 May 2019 14:24:55 +0200
> > > > Daniel Vetter <daniel at ffwll.ch> wrote:
> > > >  
> > > > > On Thu, May 16, 2019 at 11:22:11AM +0300, Pekka Paalanen wrote:  

...

> > > > > > No, my concern is not an issue with netlink reliability. It is a
> > > > > > potential issue when userspace chooses to not use netlink, and uses
> > > > > > something else instead. I'm not sure what that else is but Paul says
> > > > > > there is code in libudev and that is completely outside the control of
> > > > > > KMS apps like display servers.  
> > > > >
> > > > > afaik this other path only exists because it's the older one, for uapi
> > > > > backwards compatibility with older userspace. Shouldn't be used for
> > > > > anything.  
> > > >
> > > > "Shouldn't be used" and someone screaming "kernel regression"... are you
> > > > sure that path won't matter?
> > > >
> > > > Like some home-brewn distribution that happens to configure their
> > > > libudev and kernel to use the old method, uses already new userspace,
> > > > and then upgrades the kernel that starts sending fine-grained hotplug
> > > > events, resulting the display server randomly handling hotplug wrong.
> > > >
> > > > Reading Airlie's recent rant about kernel regression handling make this
> > > > a scary scenario where you would have no other choice than to rip all
> > > > the fine-grained uevents out again.
> > > >
> > > > Is there any difference in the kernel code between the old method and
> > > > the netlink method? Would it be possible to send fine-grained hotplug
> > > > events only through netlink, and fall back to the old 'HOTPLUG=1' for
> > > > the old method?  
> > >
> > > There's a lot of grey in kernel regressions, and for fringe setups used by
> > > few people I wouldn't worry about this. If they expect their shit to keep
> > > working when using new stuff and crappy old interfaces, they get to keep
> > > all the pieces.  
> >
> > It didn't sound gray at all, reading Dave Airlie's email about it. If
> > someone updates the kernel, and something works worse after that, then
> > it is by definition a kernel regression. Period. And the earliest
> > regression wins, i.e. if a revert breaks other things, the revert will
> > be done regardless.
> >  
> > > Dave's recent rant was a bit special, since userspace is clearly smoking
> > > some strong stuff (-modesetting's atomic is seriously not using atomic
> > > correctly), but it was also affecting too many people, and changing the
> > > boot setup meant you'd get a black screen on boot-up already. Instead of
> > > just on the first modeset with more than 1 screen.  
> >
> > Then I think I missed the context of Dave's email. Reading it again, I
> > still do not see that context.
> >
> > Btw. how do you determine "not using atomic correctly"? Has some uAPI
> > specification for atomic appeared? I wasn't aware there was any uAPI
> > specs, so there is no "incorrect use" if it happened to work once.  
> 
> -modesetting atomic blows up with more than one screen (even if you
> just move that screen between crtc). The breakage was that with the
> fastboot changes we've put the one screen onto crtc 1, but by default
> modesetting wants it on crtc 0, and it couldn't do that switch
> anymore.

Hi Daniel,

what says the assumption of the only monitor being driven by CRTC 0
was a bad one? :-p

It's probably not obvious that userspace needs to explicitly try to
avoid invalid configuration combinations by inspecting the current full
configuration and not just the one CRTC/connector it wants to use.

> All current atomic in -modesetting can do is pageflip and dpms off/on
> on the first screen on the first crtc. Anything more fancy goes boom,
> like where you change the connector/crtc links.
> 
> It's _really_ broken :-)

But it worked exactly that much, until a kernel change broke it, right?
Yes, I totally see the sillyness, but if it worked and we have these
no-regression rules...

> > I don't personally really like these rules, but if these are the rules,
> > then so be it. In my opinion it would be a huge step forward to get and
> > require uAPI specifications, that people could verify both kernel and
> > userspace against. Verifying against kernel code with no spec is what
> > leads to the -modesetting issue by the sounds of it.
> >
> > Documenting kernel internal interfaces is not it. People reading
> > DRM internal interface docs would need to know how DRM works internally
> > before they could map that information into uAPI, which makes it less
> > useful if not even useless for userspace developers.  
> 
> vgem is the idea here for validation, but if people ship atomic code
> that was never tested except for "boots on my laptop", then nothing is
> going to help.

A testing pattern library with vkms would be awesome indeed.

> And yes we have a huge gap with uapi documentation. btw for properties
> those section are meant to be useful for userspace people too:
> 
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#standard-connector-properties
> 
> and all subsequent chapters. I guess it's a bit burried, but this part
> is meant to be the uapi spec for properties. Is that also failing your
> expectations?

Yes: it is hard to find (it is in Driver Developer's Guide, buried
several chapters in), it is interleaved with lots of DRM internal
details, makes references to DRM internal functions, and probably
relies on DRM internals behaviour through the references by not
repeating what they do.

It is useful once you find it, but I don't think it's enough for making
good use in userspace for someone who hasn't been a DRM kernel
developer.

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20190521/0f69fd20/attachment.sig>