On Thu, 2021-09-16 at 11:06 +0200, Hans de Goede wrote:
Hi,
On 9/15/21 10:26 PM, Lyude Paul wrote:
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@gmail.com Signed-off-by: Hans de Goede hdegoede@redhat.com
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;
- 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.
Thank you for catching this, yes this check indeed is wrong. AFAIK there is no way to check that the mutex has been locked by us, so this extra sanity check simply needs to be removed.
I'll drop the check before pushing this to drm-misc-next (more on that in a reply to the cover letter), if that is ok with you.
Sounds fine to me!
Or do you want me to do a new version addressing this?
Regards,
Hans
+ 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