On Mon, Oct 7, 2019 at 7:09 AM Sean Paul <seanpaul@xxxxxxxxxxxx> wrote: > > On Thu, Oct 3, 2019 at 3:57 PM Mat King <mathewk@xxxxxxxxxx> wrote: > > > > On Thu, Oct 3, 2019 at 2:59 AM Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: > > > > > > On Wed, 02 Oct 2019, Mat King <mathewk@xxxxxxxxxx> wrote: > > > > On Wed, Oct 2, 2019 at 4:46 AM Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: > > > >> > > > >> On Wed, 02 Oct 2019, Daniel Thompson <daniel.thompson@xxxxxxxxxx> wrote: > > > >> > On Wed, Oct 02, 2019 at 12:30:05PM +0300, Jani Nikula wrote: > > > >> >> On Tue, 01 Oct 2019, Mat King <mathewk@xxxxxxxxxx> wrote: > > > >> >> > Resending in plain text mode > > /snip > > > > > So my proposal would now be to add a new standard property to > > drm_connector called "privacy_screen" this property would be an enum > > which can take one of three values. > > > > PRIVACY_UNSUPPORTED - Privacy is not available for this connector > > PRIVACY_DISABLED - Privacy is available but turned off > > PRIVACY_ENABLED - Privacy is available and turned on > > Agree with Jani, use the property presence to determine if it's supported That makes sense; just to confirm can a property be added or removed after the connector is registered? > > > > > When the connector is initized the privacy screen property is set to > > PRIVACY_UNSUPPORTED and cannot be changed unless a drm_privacy_screen > > is registered to the connector. drm_privacy_screen will look something > > like > > > > struct drm_privacy_screen_ops { > > int (*get_privacy_state)(struct drm_privacy_screen *); > > int (*set_privacy_state)(struct drm_privacy_screen *, int); > > } > > > > struct drm_privacy_screen { > > /* The privacy screen device */ > > struct device *dev; > > > > /* The connector that the privacy screen is attached */ > > struct drm_connector *connector; > > > > /* Ops to get and set the privacy screen state */ > > struct drm_privacy_screen_ops *ops; > > > > /* The current state of the privacy screen */ > > int state; > > } > > > > Privacy screen device drivers will call a function to register the > > privacy screen with the connector. > > Do we actually need dedicated drivers for privacy screen? It seems > like something that is panel-specific hardware, so I'd suggest just > using the panel driver. The privacy screen is physically part of the display but the control interface, at least in all current use cases, is ACPI. Is there a way to control an ACPI device with the panel driver? > > Sean > > > > > int drm_privacy_screen_register(struct drm_privacy_screen_ops *ops, > > struct device *dev, struct drm_connector *); > > > > Calling this will set a new field on the connector "struct > > drm_privacy_screen *privacy_screen" and change the value of the > > property to ops->get_privacy_state(). When > > drm_mode_connector_set_obj_prop() is called with the > > privacy_screen_proptery if a privacy_screen is registered to the > > connector the ops->set_privacy_state() will be called with the new > > value. > > > > Setting of this property (and all drm properties) is done in user > > space using ioctrl. > > > > Registering the privacy screen with a connector may be tricky because > > the driver for the privacy screen will need to be able to identify > > which connector it belongs to and we will have to deal with connectors > > being added both before and after the privacy screen device is added > > by it's driver. > > > > How does that sound? I will work on a patch if that all sounds about right. > > > > One question I still have is there a way to not accept a value that is > > passed to drm_mode_connector_set_obj_prop()? In this case if a privacy > > screen is not registered the property must stay PRIVACY_UNSUPPORTED > > and if a privacy screen is registered then PRIVACY_UNSUPPORTED must > > never be set. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel