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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 11, 2020 at 10:47 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> From: Rajat Jain <rajatja@xxxxxxxxxx>
>
> Add support for generic electronic privacy screen properties, that
> can be added by systems that have an integrated EPS.
>
> Changes in v2 (Hans de Goede)
> - Create 2 properties, "privacy-screen sw-state" and
>   "privacy-screen hw-state", to deal with devices where the OS might be
>   locked out of making state changes
> - Write kerneldoc explaining how the 2 properties work together, what
>   happens when changes to the state are made outside of the DRM code's
>   control, etc.
>
> Signed-off-by: Rajat Jain <rajatja@xxxxxxxxxx>
> Co-authored-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>


Ack, Thanks for doing this.

>
> ---
>  Documentation/gpu/drm-kms.rst     |   2 +
>  drivers/gpu/drm/drm_atomic_uapi.c |   6 ++
>  drivers/gpu/drm/drm_connector.c   | 100 ++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h       |  50 +++++++++++++++
>  4 files changed, 158 insertions(+)
>
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 906771e03103..b72b1e0db343 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -445,6 +445,8 @@ Property Types and Blob Property Support
>  .. kernel-doc:: drivers/gpu/drm/drm_property.c
>     :export:
>
> +.. _standard_connector_properties:
> +
>  Standard Connector Properties
>  -----------------------------
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index a1e5e262bae2..e56a11208515 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -766,6 +766,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>                                                    fence_ptr);
>         } else if (property == connector->max_bpc_property) {
>                 state->max_requested_bpc = val;
> +       } else if (property == connector->privacy_screen_sw_state_property) {
> +               state->privacy_screen_sw_state = val;
>         } else if (connector->funcs->atomic_set_property) {
>                 return connector->funcs->atomic_set_property(connector,
>                                 state, property, val);
> @@ -842,6 +844,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>                 *val = 0;
>         } else if (property == connector->max_bpc_property) {
>                 *val = state->max_requested_bpc;
> +       } else if (property == connector->privacy_screen_sw_state_property) {
> +               *val = state->privacy_screen_sw_state;
> +       } else if (property == connector->privacy_screen_hw_state_property) {
> +               *val = state->privacy_screen_hw_state;
>         } else if (connector->funcs->atomic_get_property) {
>                 return connector->funcs->atomic_get_property(connector,
>                                 state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 644f0ad10671..01360edc2376 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1186,6 +1186,45 @@ static const struct drm_prop_enum_list dp_colorspaces[] = {
>   *     can also expose this property to external outputs, in which case they
>   *     must support "None", which should be the default (since external screens
>   *     have a built-in scaler).
> + *
> + * privacy-screen sw-state, privacy-screen hw-state:
> + *     These 2 optional properties can be used to query the state of the
> + *     electronic privacy screen that is available on some displays; and in
> + *     some cases also control the state. If a driver implements these
> + *     properties then both properties must be present.
> + *
> + *     "privacy-screen hw-state" is read-only and reflects the actual state
> + *     of the privacy-screen, possible values: "Enabled", "Disabled,
> + *     "Enabled, locked", "Disabled, locked". The locked states indicate
> + *     that the state cannot be changed through the DRM API. E.g. there
> + *     might be devices where the firmware-setup options, or a hardware
> + *     slider-switch, offer always on / off modes.


May be add: "This is what the userspace tools must use to query and
report the actual status at the moment, if needed"

>
> + *
> + *     "privacy-screen sw-state" can be set to change the privacy-screen state
> + *     when not locked. In this case the driver must update the hw-state
> + *     property to reflect the new state on completion of the commit of the
> + *     sw-state property. Setting the sw-state property when the hw-state is
> + *     locked must be interpreted by the driver as a request to change the
> + *     state to the set state when the hw-state becomes unlocked. E.g. if
> + *     "privacy-screen hw-state" is "Enabled, locked" and the sw-state
> + *     gets set to "Disabled" followed by the user unlocking the state by
> + *     changing the slider-switch position, then the driver must set the
> + *     state to "Disabled" upon receiving the unlock event.
> + *
> + *     In some cases the privacy-screen state might change outside of control
> + *     of the DRM code. E.g. there might be a firmware handled hotkey which
> + *     toggles the state, or the state might be changed through another
> + *     userspace API such as writing /proc/acpi/ibm/lcdshadow. In this case
> + *     the driver must update both the hw-state and the sw-state to reflect
> + *     the new value, overwriting any pending state requests in the sw-state.


May be add: "The pending state requests are thus 'discarded'".

>
> + *
> + *     Note that the ability for the state to change outside of control of
> + *     the DRM master process means that userspace must not cache the value
> + *     of the sw-state. Ccaching the sw-state value and including it in later
> + *     atomic commits may lead to overriding a state change done through e.g.
> + *     a firmware handled hotkey. Therefor userspace must not include the
> + *     privacy-screen sw-state in an atomic commit unless it wants to change
> + *     its value.
>   */
>
>  int drm_connector_create_standard_properties(struct drm_device *dev)
> @@ -2152,6 +2191,67 @@ int drm_connector_set_panel_orientation_with_quirk(
>  }
>  EXPORT_SYMBOL(drm_connector_set_panel_orientation_with_quirk);
>
> +static const struct drm_prop_enum_list privacy_screen_enum[] = {
> +       { PRIVACY_SCREEN_DISABLED,              "Disabled" },
> +       { PRIVACY_SCREEN_ENABLED,               "Enabled" },
> +       { PRIVACY_SCREEN_DISABLED_LOCKED,       "Disabled, locked" },
> +       { PRIVACY_SCREEN_ENABLED_LOCKED,        "Enabled, locked" },
> +};
> +
> +/**
> + * drm_connector_create_privacy_screen_properties -
> + *     create the drm connecter's privacy-screen properties.
> + * @connector: connector for which to create the privacy-screen properties
> + *
> + * This function creates the "privacy-screen sw-state" and "privacy-screen
> + * hw-state" properties for the connector. They are not attached.
> + */
> +void
> +drm_connector_create_privacy_screen_properties(struct drm_connector *connector)
> +{
> +       if (connector->privacy_screen_sw_state_property)
> +               return;
> +
> +       /* Note sw-state only supports the first 2 values of the enum */
> +       connector->privacy_screen_sw_state_property =
> +               drm_property_create_enum(connector->dev, DRM_MODE_PROP_ENUM,
> +                               "privacy-screen sw-state",
> +                               privacy_screen_enum, 2);
> +
> +       connector->privacy_screen_hw_state_property =
> +               drm_property_create_enum(connector->dev,
> +                               DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_ENUM,
> +                               "privacy-screen hw-state",
> +                               privacy_screen_enum,
> +                               ARRAY_SIZE(privacy_screen_enum));
> +}
> +EXPORT_SYMBOL(drm_connector_create_privacy_screen_properties);
> +
> +/**
> + * drm_connector_attach_privacy_screen_properties -
> + *     attach the drm connecter's privacy-screen properties.
> + * @connector: connector on which to attach the privacy-screen properties
> + *
> + * This function attaches the "privacy-screen sw-state" and "privacy-screen
> + * hw-state" properties to the connector. The initial state of both is set
> + * to "Disabled".
> + */
> +void
> +drm_connector_attach_privacy_screen_properties(struct drm_connector *connector)
> +{
> +       if (!connector->privacy_screen_sw_state_property)
> +               return;
> +
> +       drm_object_attach_property(&connector->base,
> +                                  connector->privacy_screen_sw_state_property,
> +                                  PRIVACY_SCREEN_DISABLED);
> +
> +       drm_object_attach_property(&connector->base,
> +                                  connector->privacy_screen_hw_state_property,
> +                                  PRIVACY_SCREEN_DISABLED);
> +}
> +EXPORT_SYMBOL(drm_connector_attach_privacy_screen_properties);
> +
>  int drm_connector_set_obj_prop(struct drm_mode_object *obj,
>                                     struct drm_property *property,
>                                     uint64_t value)
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 19ae6bb5c85b..a8844f4c6ae9 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -271,6 +271,30 @@ struct drm_monitor_range_info {
>         u8 max_vfreq;
>  };
>
> +/**
> + * enum drm_privacy_screen_status - privacy screen status
> + *
> + * This enum is used to track and control the state of the integrated privacy
> + * screen present on some display panels, via the "privacy-screen sw-state"
> + * and "privacy-screen hw-state" properties. Note the _LOCKED enum values
> + * are only valid for the "privacy-screen hw-state" property.
> + *
> + * @PRIVACY_SCREEN_DISABLED:
> + *  The privacy-screen on the panel is disabled
> + * @PRIVACY_SCREEN_ENABLED:
> + *  The privacy-screen on the panel is enabled
> + * @PRIVACY_SCREEN_DISABLED_LOCKED:
> + *  The privacy-screen on the panel is disabled and locked (cannot be changed)
> + * @PRIVACY_SCREEN_ENABLED_LOCKED:
> + *  The privacy-screen on the panel is enabled and locked (cannot be changed)
> + */
> +enum drm_privacy_screen_status {
> +       PRIVACY_SCREEN_DISABLED = 0,
> +       PRIVACY_SCREEN_ENABLED,
> +       PRIVACY_SCREEN_DISABLED_LOCKED,
> +       PRIVACY_SCREEN_ENABLED_LOCKED,
> +};
> +
>  /*
>   * This is a consolidated colorimetry list supported by HDMI and
>   * DP protocol standard. The respective connectors will register
> @@ -686,6 +710,18 @@ struct drm_connector_state {
>          */
>         u8 max_bpc;
>
> +       /**
> +        * @privacy_screen_sw_state: See :ref:`Standard Connector
> +        * Properties<standard_connector_properties>`
> +        */
> +       enum drm_privacy_screen_status privacy_screen_sw_state;
> +
> +       /**
> +        * @privacy_screen_hw_state: See :ref:`Standard Connector
> +        * Properties<standard_connector_properties>`
> +        */
> +       enum drm_privacy_screen_status privacy_screen_hw_state;
> +
>         /**
>          * @hdr_output_metadata:
>          * DRM blob property for HDR output metadata
> @@ -1285,6 +1321,18 @@ struct drm_connector {
>          */
>         struct drm_property *max_bpc_property;
>
> +       /**
> +        * @privacy_screen_sw_state_property: Optional atomic property for the
> +        * connector to control the integrated privacy screen.
> +        */
> +       struct drm_property *privacy_screen_sw_state_property;
> +
> +       /**
> +        * @privacy_screen_hw_state_property: Optional atomic property for the
> +        * connector to report the actual integrated privacy screen state.
> +        */
> +       struct drm_property *privacy_screen_hw_state_property;
> +
>  #define DRM_CONNECTOR_POLL_HPD (1 << 0)
>  #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
>  #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
> @@ -1598,6 +1646,8 @@ int drm_connector_set_panel_orientation_with_quirk(
>         int width, int height);
>  int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>                                           int min, int max);
> +void drm_connector_create_privacy_screen_properties(struct drm_connector *conn);
> +void drm_connector_attach_privacy_screen_properties(struct drm_connector *conn);
>
>  /**
>   * struct drm_tile_group - Tile group metadata
> --
> 2.26.0
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux