| Message ID | c5e48c59-6033-8fc0-d7e7-614b12931e73@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 17, 2016 at 02:20:10PM +0100, Maarten Lankhorst wrote: > With checks! This will allow safe access to the current state, > while ensuring that the correct locks are held. > > Changes since v1: > - Constify returned states. > - Require plane lock to return plane state, don't allow crtc_lock to > be sufficient. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> I wanted to add some more docs to drm_atomic_state and related code and realized this series here hasn't landed. What's the status? Anything pending from this long discussion, Ville? Or should I go through it and do some review&merging? -Daniel > --- > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index e527684dd394..9af5aba8c27c 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -334,6 +334,66 @@ __drm_atomic_get_current_plane_state(struct drm_atomic_state *state, > return plane->state; > } > > + > +/** > + * drm_atomic_get_current_plane_state - get current plane state > + * @plane: plane to grab > + * > + * This function returns the current plane state for the given plane, > + * with extra locking checks to make sure that the plane state can be > + * retrieved safely. > + * > + * Returns: > + * > + * Pointer to the current plane state. > + */ > +static inline const struct drm_plane_state * > +drm_atomic_get_current_plane_state(struct drm_plane *plane) > +{ > + drm_modeset_lock_assert_held(&plane->mutex); > + > + return plane->state; > +} > + > +/** > + * drm_atomic_get_current_crtc_state - get current crtc state > + * @crtc: crtc to grab > + * > + * This function returns the current crtc state for the given crtc, > + * with extra locking checks to make sure that the crtc state can be > + * retrieved safely. > + * > + * Returns: > + * > + * Pointer to the current crtc state. > + */ > +static inline const struct drm_crtc_state * > +drm_atomic_get_current_crtc_state(struct drm_crtc *crtc) > +{ > + drm_modeset_lock_assert_held(&crtc->mutex); > + > + return crtc->state; > +} > + > +/** > + * drm_atomic_get_current_connector_state - get current connector state > + * @connector: connector to grab > + * > + * This function returns the current connector state for the given connector, > + * with extra locking checks to make sure that the connector state can be > + * retrieved safely. > + * > + * Returns: > + * > + * Pointer to the current connector state. > + */ > +#define drm_atomic_get_current_connector_state(connector) \ > +({ /* Implemented as macro to avoid including drmP for struct drm_device */ \ > + struct drm_connector *con__ = (connector); \ > + drm_modeset_lock_assert_held(&con__->dev->mode_config.connection_mutex); \ > + (const struct drm_connector_state *)con__->state; \ > +}) > + > int __must_check > drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state, > struct drm_display_mode *mode); > diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h > index d918ce45ec2c..3ca4ee838b07 100644 > --- a/include/drm/drm_modeset_lock.h > +++ b/include/drm/drm_modeset_lock.h > @@ -109,6 +109,15 @@ static inline bool drm_modeset_is_locked(struct drm_modeset_lock *lock) > return ww_mutex_is_locked(&lock->mutex); > } > > +static inline void drm_modeset_lock_assert_held(struct drm_modeset_lock *lock) > +{ > +#ifdef CONFIG_LOCKDEP > + lockdep_assert_held(&lock->mutex.base); > +#else > + WARN_ON(!drm_modeset_is_locked(lock)); > +#endif > +} > + > int drm_modeset_lock(struct drm_modeset_lock *lock, > struct drm_modeset_acquire_ctx *ctx); > int drm_modeset_lock_interruptible(struct drm_modeset_lock *lock, >
On Thu, Nov 17, 2016 at 02:20:10PM +0100, Maarten Lankhorst wrote: > With checks! This will allow safe access to the current state, > while ensuring that the correct locks are held. > > Changes since v1: > - Constify returned states. > - Require plane lock to return plane state, don't allow crtc_lock to > be sufficient. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index e527684dd394..9af5aba8c27c 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -334,6 +334,66 @@ __drm_atomic_get_current_plane_state(struct drm_atomic_state *state, > return plane->state; > } > > + > +/** > + * drm_atomic_get_current_plane_state - get current plane state > + * @plane: plane to grab > + * > + * This function returns the current plane state for the given plane, > + * with extra locking checks to make sure that the plane state can be > + * retrieved safely. > + * > + * Returns: > + * > + * Pointer to the current plane state. > + */ > +static inline const struct drm_plane_state * > +drm_atomic_get_current_plane_state(struct drm_plane *plane) > +{ > + drm_modeset_lock_assert_held(&plane->mutex); > + > + return plane->state; > +} > + > +/** > + * drm_atomic_get_current_crtc_state - get current crtc state > + * @crtc: crtc to grab > + * > + * This function returns the current crtc state for the given crtc, > + * with extra locking checks to make sure that the crtc state can be > + * retrieved safely. > + * > + * Returns: > + * > + * Pointer to the current crtc state. > + */ > +static inline const struct drm_crtc_state * > +drm_atomic_get_current_crtc_state(struct drm_crtc *crtc) > +{ > + drm_modeset_lock_assert_held(&crtc->mutex); > + > + return crtc->state; > +} > + > +/** > + * drm_atomic_get_current_connector_state - get current connector state > + * @connector: connector to grab > + * > + * This function returns the current connector state for the given connector, > + * with extra locking checks to make sure that the connector state can be > + * retrieved safely. > + * > + * Returns: > + * > + * Pointer to the current connector state. > + */ > +#define drm_atomic_get_current_connector_state(connector) \ > +({ /* Implemented as macro to avoid including drmP for struct drm_device */ \ Imo that's not a terrible good reason. Eventually will split drm_device.h out from drmP.h, and then we can nuke the additional include. Can you pls respin? -Daniel > + struct drm_connector *con__ = (connector); \ > + drm_modeset_lock_assert_held(&con__->dev->mode_config.connection_mutex); \ > + (const struct drm_connector_state *)con__->state; \ > +}) > + > int __must_check > drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state, > struct drm_display_mode *mode); > diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h > index d918ce45ec2c..3ca4ee838b07 100644 > --- a/include/drm/drm_modeset_lock.h > +++ b/include/drm/drm_modeset_lock.h > @@ -109,6 +109,15 @@ static inline bool drm_modeset_is_locked(struct drm_modeset_lock *lock) > return ww_mutex_is_locked(&lock->mutex); > } > > +static inline void drm_modeset_lock_assert_held(struct drm_modeset_lock *lock) > +{ > +#ifdef CONFIG_LOCKDEP > + lockdep_assert_held(&lock->mutex.base); > +#else > + WARN_ON(!drm_modeset_is_locked(lock)); > +#endif > +} > + > int drm_modeset_lock(struct drm_modeset_lock *lock, > struct drm_modeset_acquire_ctx *ctx); > int drm_modeset_lock_interruptible(struct drm_modeset_lock *lock, >
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index e527684dd394..9af5aba8c27c 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -334,6 +334,66 @@ __drm_atomic_get_current_plane_state(struct drm_atomic_state *state, return plane->state; } + +/** + * drm_atomic_get_current_plane_state - get current plane state + * @plane: plane to grab + * + * This function returns the current plane state for the given plane, + * with extra locking checks to make sure that the plane state can be + * retrieved safely. + * + * Returns: + * + * Pointer to the current plane state. + */ +static inline const struct drm_plane_state * +drm_atomic_get_current_plane_state(struct drm_plane *plane) +{ + drm_modeset_lock_assert_held(&plane->mutex); + + return plane->state; +} + +/** + * drm_atomic_get_current_crtc_state - get current crtc state + * @crtc: crtc to grab + * + * This function returns the current crtc state for the given crtc, + * with extra locking checks to make sure that the crtc state can be + * retrieved safely. + * + * Returns: + * + * Pointer to the current crtc state. + */ +static inline const struct drm_crtc_state * +drm_atomic_get_current_crtc_state(struct drm_crtc *crtc) +{ + drm_modeset_lock_assert_held(&crtc->mutex); + + return crtc->state; +} + +/** + * drm_atomic_get_current_connector_state - get current connector state + * @connector: connector to grab + * + * This function returns the current connector state for the given connector, + * with extra locking checks to make sure that the connector state can be + * retrieved safely. + * + * Returns: + * + * Pointer to the current connector state. + */ +#define drm_atomic_get_current_connector_state(connector) \ +({ /* Implemented as macro to avoid including drmP for struct drm_device */ \ + struct drm_connector *con__ = (connector); \ + drm_modeset_lock_assert_held(&con__->dev->mode_config.connection_mutex); \ + (const struct drm_connector_state *)con__->state; \ +}) + int __must_check drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state, struct drm_display_mode *mode); diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h index d918ce45ec2c..3ca4ee838b07 100644 --- a/include/drm/drm_modeset_lock.h +++ b/include/drm/drm_modeset_lock.h @@ -109,6 +109,15 @@ static inline bool drm_modeset_is_locked(struct drm_modeset_lock *lock) return ww_mutex_is_locked(&lock->mutex); } +static inline void drm_modeset_lock_assert_held(struct drm_modeset_lock *lock) +{ +#ifdef CONFIG_LOCKDEP + lockdep_assert_held(&lock->mutex.base); +#else + WARN_ON(!drm_modeset_is_locked(lock)); +#endif +} + int drm_modeset_lock(struct drm_modeset_lock *lock, struct drm_modeset_acquire_ctx *ctx); int drm_modeset_lock_interruptible(struct drm_modeset_lock *lock,
With checks! This will allow safe access to the current state, while ensuring that the correct locks are held. Changes since v1: - Constify returned states. - Require plane lock to return plane state, don't allow crtc_lock to be sufficient. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> ---