Re: [RFC v2 0/1] drm/connector: Add support for privacy-screen properties

[Rajat Jain <rajatja@google.com>]

On Wed, May 13, 2020 at 12:49 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
>
> On Tue, 12 May 2020 18:09:15 +0200
> Hans de Goede <hdegoede@redhat.com> wrote:
>
> > Hi,
> >
> > On 5/12/20 4:20 PM, Pekka Paalanen wrote:
> > > On Tue, 12 May 2020 10:18:31 +0200
> > > Hans de Goede <hdegoede@redhat.com> wrote:
> > >
> > >> Hi,
> > >>
> > >> On 5/11/20 9:55 PM, Rajat Jain wrote:
> > >>> Hi Hans,
> > >>>
> > >>> On Mon, May 11, 2020 at 10:47 AM Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
> > >>>
> > >>>      Hi All,
> > >>>
> > >>>      This RFC takes Rajat's earlier patch for adding privacy-screen properties
> > >>>      infra to drm_connector.c and then adds the results of the discussion from
> > >>>      the "RFC: Drm-connector properties managed by another driver / privacy
> > >>>      screen support" mail thread on top, hence the v2.
> > >>>
> > >>>
> > >>> Thank you so much for doing this. I was following the said discussion and eventually it became quite complex for me to understand and follow :-)
> > >>
> > >> I hope the new doc text makes things clear again?
> > >>
> > >>
> > >>>      The most important thing here is big kernel-doc comment which gets added in
> > >>>      the first patch-chunk modifying drm_connector.c, this summarizes, or at
> > >>>      least tries to summarize, the conclusions of our previous discussion on
> > >>>      the userspace API and lays down the ground rules for how the 2 new
> > >>>      "privacy-screen sw-state" and  "privacy-screen hw-state" properties are
> > >>>      to be used both from the driver side as well as from the userspace side.
> > >>>
> > >>>      Other then that this modifies Rajat's patch to add 2 properties instead
> > >>>      of one, without much other changes.
> > >>>
> > >>>      Rajat, perhaps you can do a new version of your patch-set integration /
> > >>>      using this version of the properties and then if everyone is ok with
> > >>>      the proposed userspace API Jani can hopefully merge the whole set
> > >>>      through the i915 tree sometime during the 5.9 cycle.
> > >>>
> > >>>
> > >>> SGTM. I have actually moved to working on something else now, so I will most likely wait for this patch to get merged, before rebasing my other / remaining patches on top of that.
> > >>
> > >> We have the rule that code like this will not be merged until it has at least
> > >> one in kernel user. I plan to eventually use this too, but that is still
> > >> a while away as I first need to write a lcdshadow subsystem which the
> > >> thinkpad_acpi code can then use to register a lcdshadow device; and when
> > >> that all is in place, then I can hook it up on the drm code.
> > >
> > > Hi,
> > >
> > > I believe this falls under "new UAPI" rules, because this is adding new
> > > KMS properties. Hence an in-kernel user is not enough:
> > >
> > > https://www.kernel.org/doc/html/latest/gpu/drm-uapi.html#open-source-userspace-requirements
> >
> > Hmm, I believe that that mostly applies to new DRI (ioclt) interfaces for
> > submitting rendering commands to new GPUs and other complex new APIs and
> > not necessarily to introducing new properties.    Also note that all
> > properties are exported under X through Xrandr, at least reading them,
> > not sure about setting them.
>
> Please check with Daniel Vetter.
>
> My belief is that all new KMS properties that were never exposed by any
> driver before are new UAPI.
>
> My personal opinion is that Xorg/RandR exposing a KMS property does
> *not* count as real userspace *alone*. Simply plumbing a KMS property
> through RandR and then nothing actually using it does not prove
> anything about the property's design or usability.
>
> IMO, if you use Xorg/RandR as your userspace, you also need something
> that uses RandR and really pokes at the new property to prove it's
> viable.
>
> But that's just me.
>
> > Anyways I do plan to write some mutter code to test my lcdshadow subsys <->
> > DRM driver integration when that is all more then just vaporware. But I
> > would prefer for Rajat's series to land before that so that I can build
> > on top of it.
>
> The DRM maintainers make that call.
>
>
> On Tue, 12 May 2020 10:38:11 -0700
> Rajat Jain <rajatja@google.com> wrote:
>
> > The chrome browser currently uses the API exposed by my (previous)
> > patchset to control privacy screen.
> > https://source.chromium.org/chromium/chromium/src/+/master:ui/ozone/platform/drm/common/drm_util.cc;l=180?q=%22privacy-screen%22%20-f:third_party%2Fkernel%2Fv&originalUrl=https:%2F%2Fcs.chromium.org%2F
> >
> > I know this doesn't help directly, but just to say that there are
> > users waiting to use an API that we release. If these changes are
> > accepted, I expect to see the change in browser again, to match the
> > new API,  although that will be not until we decide to uprev our
> > kernel again.
>
> Chromium counts as userspace, I think many new features have landed
> with it as the userspace.
>
> Is that from some development branch, not actually merged or released
> yet? If yes, very good.

No, it's released (in Chromium for chromeOS platforms).

> When you submit kernel patches with new UAPI,
> it would be nice to point to the userspace review discussion where the
> userspace patches have been reviewed and accepted but not merged.

I doubt if that would happen - because they won't do it unless a
feature is available in the kernel they are using. I can definitely
create a public bug about what they need to do though.

Thanks,

Rajat

>
>
> Thanks,
> pq
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel