On Wed, Jun 24, 2015 at 08:57:23AM +0100, Chris Wilson wrote:
> 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.vet...@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)
Write access holds both locks, which means for read access it's ok to just
hold one. But since that's caused confusions I fully agree that extracting
this into a static inline and commenting properly is a good idea.
-Daniel
> }
>
> 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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
