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

[Hans de Goede]

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 at redhat.com <mailto:hdegoede at 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.

So since Jani said your patch-set was more or less ready I think it would
be best if you add my version of this to your patch-set and then post
a new version of your patch-set. But first let me do a v3 addressing
the remarks on doc text. Note I will wait a bit before sending out v3
to see if I get more feedback.

Regards,

Hans


> 
> Thanks & Best Regards,
> 
> Rajat
> 
>     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.
> 
>     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.
> 
>     Regards,
> 
>     Hans
> 
>     p.s.
> 
>     I plan to start working on the lcdshadow subsystem next. As discussed the
>     plan for this subsystem is to allow drivers outside of the DRM subsys, such
>     as for example the thinkpad_acpi driver, to register a lcdshadow device,
>     which DRM drivers can then get a reference to and use to implement these
>     properties.
>