On Mon, 2021-09-06 at 09:35 +0200, Hans de Goede wrote:
> Add support for privacy-screen consumers to register a notifier to
> be notified of external (e.g. done by the hw itself on a hotkey press)
> state changes.
>
> Reviewed-by: Emil Velikov <emil.l.velikov@xxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_privacy_screen.c | 67 +++++++++++++++++++++++
> include/drm/drm_privacy_screen_consumer.h | 15 +++++
> include/drm/drm_privacy_screen_driver.h | 4 ++
> 3 files changed, 86 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_privacy_screen.c
> b/drivers/gpu/drm/drm_privacy_screen.c
> index 294a09194bfb..7a5f878c3171 100644
> --- a/drivers/gpu/drm/drm_privacy_screen.c
> +++ b/drivers/gpu/drm/drm_privacy_screen.c
> @@ -255,6 +255,49 @@ void drm_privacy_screen_get_state(struct
> drm_privacy_screen *priv,
> }
> EXPORT_SYMBOL(drm_privacy_screen_get_state);
>
> +/**
> + * drm_privacy_screen_register_notifier - register a notifier
> + * @priv: Privacy screen to register the notifier with
> + * @nb: Notifier-block for the notifier to register
> + *
> + * Register a notifier with the privacy-screen to be notified of changes
> made
> + * to the privacy-screen state from outside of the privacy-screen class.
> + * E.g. the state may be changed by the hardware itself in response to a
> + * hotkey press.
> + *
> + * The notifier is called with no locks held. The new hw_state and sw_state
> + * can be retrieved using the drm_privacy_screen_get_state() function.
> + * A pointer to the drm_privacy_screen's struct is passed as the void *data
> + * argument of the notifier_block's notifier_call.
> + *
> + * The notifier will NOT be called when changes are made through
> + * drm_privacy_screen_set_sw_state(). It is only called for external
> changes.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int drm_privacy_screen_register_notifier(struct drm_privacy_screen *priv,
> + struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_register(&priv->notifier_head, nb);
> +}
> +EXPORT_SYMBOL(drm_privacy_screen_register_notifier);
> +
> +/**
> + * drm_privacy_screen_unregister_notifier - unregister a notifier
> + * @priv: Privacy screen to register the notifier with
> + * @nb: Notifier-block for the notifier to register
> + *
> + * Unregister a notifier registered with
> drm_privacy_screen_register_notifier().
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int drm_privacy_screen_unregister_notifier(struct drm_privacy_screen *priv,
> + struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_unregister(&priv->notifier_head, nb);
> +}
> +EXPORT_SYMBOL(drm_privacy_screen_unregister_notifier);
> +
> /*** drm_privacy_screen_driver.h functions ***/
>
> static ssize_t sw_state_show(struct device *dev,
> @@ -352,6 +395,7 @@ struct drm_privacy_screen *drm_privacy_screen_register(
> return ERR_PTR(-ENOMEM);
>
> mutex_init(&priv->lock);
> + BLOCKING_INIT_NOTIFIER_HEAD(&priv->notifier_head);
>
> priv->dev.class = drm_class;
> priv->dev.type = &drm_privacy_screen_type;
> @@ -399,3 +443,26 @@ void drm_privacy_screen_unregister(struct
> drm_privacy_screen *priv)
> device_unregister(&priv->dev);
> }
> EXPORT_SYMBOL(drm_privacy_screen_unregister);
> +
> +/**
> + * drm_privacy_screen_call_notifier_chain - notify consumers of state
> change
> + * @priv: Privacy screen to register the notifier with
> + *
> + * A privacy-screen provider driver can call this functions upon external
> + * changes to the privacy-screen state. E.g. the state may be changed by
> the
> + * hardware itself in response to a hotkey press.
> + * This function must be called without holding the privacy-screen lock.
> + * the driver must update sw_state and hw_state to reflect the new state
> before
> + * calling this function.
> + * The expected behavior from the driver upon receiving an external state
> + * change event is: 1. Take the lock; 2. Update sw_state and hw_state;
> + * 3. Release the lock. 4. Call drm_privacy_screen_call_notifier_chain().
> + */
> +void drm_privacy_screen_call_notifier_chain(struct drm_privacy_screen
> *priv)
> +{
> + if (WARN_ON(mutex_is_locked(&priv->lock)))
> + return;
Are we sure about this check? mutex_is_locked() checks whether a mutex is
locked by anyone, not just us. So this seems like it would cause us to
WARN_ON() and abort if anyone else (not just ourselves) is holding the lock to
read the privacy screen state.
> +
> + blocking_notifier_call_chain(&priv->notifier_head, 0, priv);
> +}
> +EXPORT_SYMBOL(drm_privacy_screen_call_notifier_chain);
> diff --git a/include/drm/drm_privacy_screen_consumer.h
> b/include/drm/drm_privacy_screen_consumer.h
> index 0cbd23b0453d..7f66a90d15b7 100644
> --- a/include/drm/drm_privacy_screen_consumer.h
> +++ b/include/drm/drm_privacy_screen_consumer.h
> @@ -24,6 +24,11 @@ int drm_privacy_screen_set_sw_state(struct
> drm_privacy_screen *priv,
> void drm_privacy_screen_get_state(struct drm_privacy_screen *priv,
> enum drm_privacy_screen_status
> *sw_state_ret,
> enum drm_privacy_screen_status
> *hw_state_ret);
> +
> +int drm_privacy_screen_register_notifier(struct drm_privacy_screen *priv,
> + struct notifier_block *nb);
> +int drm_privacy_screen_unregister_notifier(struct drm_privacy_screen *priv,
> + struct notifier_block *nb);
> #else
> static inline struct drm_privacy_screen *drm_privacy_screen_get(struct
> device *dev,
> const char
> *con_id)
> @@ -45,6 +50,16 @@ static inline void drm_privacy_screen_get_state(struct
> drm_privacy_screen *priv,
> *sw_state_ret = PRIVACY_SCREEN_DISABLED;
> *hw_state_ret = PRIVACY_SCREEN_DISABLED;
> }
> +static inline int drm_privacy_screen_register_notifier(struct
> drm_privacy_screen *priv,
> + struct notifier_block
> *nb)
> +{
> + return -ENODEV;
> +}
> +static inline int drm_privacy_screen_unregister_notifier(struct
> drm_privacy_screen *priv,
> + struct
> notifier_block *nb)
> +{
> + return -ENODEV;
> +}
> #endif
>
> #endif
> diff --git a/include/drm/drm_privacy_screen_driver.h
> b/include/drm/drm_privacy_screen_driver.h
> index 5187ae52eb03..24591b607675 100644
> --- a/include/drm/drm_privacy_screen_driver.h
> +++ b/include/drm/drm_privacy_screen_driver.h
> @@ -54,6 +54,8 @@ struct drm_privacy_screen {
> struct mutex lock;
> /** @list: privacy-screen devices list list-entry. */
> struct list_head list;
> + /** @notifier_head: privacy-screen notifier head. */
> + struct blocking_notifier_head notifier_head;
> /**
> * @ops: &struct drm_privacy_screen_ops for this privacy-screen.
> * This is NULL if the driver has unregistered the privacy-screen.
> @@ -77,4 +79,6 @@ struct drm_privacy_screen *drm_privacy_screen_register(
> struct device *parent, const struct drm_privacy_screen_ops *ops);
> void drm_privacy_screen_unregister(struct drm_privacy_screen *priv);
>
> +void drm_privacy_screen_call_notifier_chain(struct drm_privacy_screen
> *priv);
> +
> #endif
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
