Re: RFC: Drm-connector properties managed by another driver / privacy screen support

From: Hans de Goede <hdegoede@redhat.com>
To: Jani Nikula <jani.nikula@linux.intel.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>
Cc: Benjamin Berg <bberg@redhat.com>,
	Christian Kellner <ckellner@redhat.com>,
	Javier Martinez Canillas <javierm@redhat.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	Nitin Joshi1 <njoshi1@lenovo.com>,
	Rajat Jain <rajatja@google.com>,
	Mark Pearson <mpearson@lenovo.com>
Subject: Re: RFC: Drm-connector properties managed by another driver / privacy screen support
Date: Wed, 15 Apr 2020 17:40:46 +0200	[thread overview]
Message-ID: <d47ba6ef-efd0-9f28-1ae4-b971b95a8f8b@redhat.com> (raw)
In-Reply-To: <87tv1k4vl6.fsf@intel.com>

Hi,

On 4/15/20 5:28 PM, Jani Nikula wrote:
> On Wed, 15 Apr 2020, Hans de Goede <hdegoede@redhat.com> wrote:
>> ii. Currently the "privacy-screen" property added by Rajat's
>> patch-set is an enum with 2 possible values:
>> "Enabled"
>> "Disabled"
>>
>> We could add a third value "Not Available", which would be the
>> default and then for internal panels always add the property
>> so that we avoid the problem that detecting if the laptop has
>> an internal privacy screen needs to be done before the connector
>> is registered. Then we can add some hooks which allow an
>> lcdshadow-driver to register itself against a connector later
>> (which is non trivial wrt probe order, but lets ignore that for now).
> 
> I regret dropping the ball on Rajat's series (sorry!).
> 
> I do think having the connector property for this is the way to go.

I 100% agree.

> Even
> if we couldn't necessarily figure out all the details on the kernel
> internal connections, can we settle on the property though, so we could
> move forward with Rajat's series?

Yes please, this will also allow us to move forward with userspace
support even if for testing that we do some hacks for the kernel's
internal connections for now.

> Moreover, do we actually need two properties, one which could indicate
> userspace's desire for the property, and another that tells the hardware
> state?

No I do not think so. I would expect there to just be one property,
I guess that if the state is (partly) firmware controlled then there
might be a race, but we will need a notification mechanism (*) for
firmware triggered state changes anyways, so shortly after loosing
the race userspace will process the notification and it will know
about it.

One thing which might be useful is a way to signal that the property
is read-only in case we ever hit hw where that is the case.

> I'd so very much like to have no in-kernel/in-firmware shortcuts
> to enable/disable the privacy screen, and instead have any hardware
> buttons just be events that the userspace could react to. However I
> don't think that'll be the case unfortunately.

In my experience with keyboard-backlight support, we will (unfortunately)
see a mix and in some case we will get a notification that the firmware
has adjusted the state, rather then just getting a keypress and
dealing with that ourselves.  In some cases we may even be able to
choose, so the fw will deal with it by default but we can ask it
to just send a key-press.  But I do believe that we can *not* expect
that we will always just get a keypress for userspace to deal with.

Regards,

Hans

*) Some udev event I guess, I sorta assume there already is a
notification mechanism for property change notifications ?

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