[Intel-gfx] [PATCH 09/11] drm: Amend connector/encoder list locking rules

[Chris Wilson]

On Tue, Jun 23, 2015 at 10:46:00PM +0200, Daniel Vetter wrote:
> Now that dp mst hotplug takes all locks we can amend the locking rules
> for the iterators. This is needed before we can roll these out in the
> atomic code to avoid getting burried in WARNINGs.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  include/drm/drm_crtc.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 3646c47b43de..c1c4f16f01ca 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1588,14 +1588,16 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
>  	list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
>  
>  #define drm_for_each_connector(connector, dev) \
> -	for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex)),		\
> +	for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex) &&		\
> +		     !drm_modeset_is_locked(&(dev)->mode_config.connection_mutex)), \
>  	     connector = list_first_entry(&(dev)->mode_config.connector_list,	\
>  					  struct drm_connector, head);		\
>  	     &connector->head != (&(dev)->mode_config.connector_list);		\
>  	     connector = list_next_entry(connector, head))
>  
>  #define drm_for_each_encoder(encoder, dev) \
> -	for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex)),		\
> +	for (WARN_ON(!mutex_is_locked(&(dev)->mode_config.mutex) &&		\
> +		     !drm_modeset_is_locked(&(dev)->mode_config.connection_mutex)), \
>  	     encoder = list_first_entry(&(dev)->mode_config.encoder_list,	\
>  					  struct drm_encoder, head);		\
>  	     &encoder->head != (&(dev)->mode_config.encoder_list);		\

Maybe something like:

	// not sure how specifc each lock will end up being
	static inline void assert_drm_mode_list_locked(dev)
	{
		/* lockdep for accuracy and verbose debug reportts,
		 * WARN_ON for warnings everywhere.
		 */
		lockdep_assert_held(&dev->mode_config.mutex);
		WARN_ON(!(mutex_is_locked(&dev->mode_config.mutex) &&
			  drm_modeset_is_locked(&dev->mode_config.connection_mutex)));

		// Daniel, it is not clear from context whether you mean
		// !a && !b, or !(a && b)
	}

	static inline struct drm_connector *drm_first_connector(dev)
	{
		assert_drm_mode_list_locked(dev);
		return list_first_entry(&dev->mode_config.connector_list,
					struct drm_connector, head);
	}

	#define drm_for_each_connector(connector, dev) \
		for (connector = drm_first_connector(dev); \
		     &connector->head != (&(dev)->mode_config.connector_list);		\
		     connector = list_next_entry(connector, head))

	static inline struct drm_encoder *drm_first_encoder(dev)
	{
		assert_drm_mode_list_locked(dev);
		return list_first_entry(&dev->mode_config.encoder_list,
					struct drm_encoder, head);
	}

	#define drm_for_each_encoder(encoder, dev) \
		for (encoder = drm_first_encoder(dev); \
		     &encoder->head != (&(dev)->mode_config.encoder_list);		\
		     encoder = list_next_entry(encoder, head))

Usual arguments of reusing code for clarity.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre