op 20-11-13 21:48, Rob Clark schreef: > At the moment, this doesn't do anything. But for atomic we will have an > ww_acquire_ctx associated with the state, to simplify the locking and > avoid potential deadlock when we cannot control the locking order. Nack. :-) Please don't split this out. ww_mutex may be backed by a mutex, but that's an implementation detail you must not rely on. > --- > drivers/gpu/drm/drm_crtc.c | 20 +++++++++++--------- > drivers/gpu/drm/i915/intel_display.c | 16 ++++++++-------- > drivers/gpu/drm/omapdrm/omap_crtc.c | 10 +++++----- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 ++++++++---- > include/drm/drm_crtc.h | 3 ++- > 5 files changed, 34 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 81ac351..55f37db 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -52,7 +52,7 @@ void drm_modeset_lock_all(struct drm_device *dev) > mutex_lock(&dev->mode_config.mutex); > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) > - mutex_lock_nest_lock(&crtc->mutex, &dev->mode_config.mutex); > + mutex_lock_nest_lock(&crtc->mutex.base, &dev->mode_config.mutex); > } This breaks ww_mutex semantics, for example. What if someone holding a ww_ctx acquires has one mutex, and tries to acquire a second crtc mutex? If lockdep was smart it would notice that this lock is nested in different locks, but I don't think lockdep is that smart. > EXPORT_SYMBOL(drm_modeset_lock_all); > > @@ -65,7 +65,7 @@ void drm_modeset_unlock_all(struct drm_device *dev) > struct drm_crtc *crtc; > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) > - mutex_unlock(&crtc->mutex); > + ww_mutex_unlock(&crtc->mutex); > > mutex_unlock(&dev->mode_config.mutex); > } > @@ -84,7 +84,7 @@ void drm_warn_on_modeset_not_all_locked(struct drm_device *dev) > return; > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) > - WARN_ON(!mutex_is_locked(&crtc->mutex)); > + WARN_ON(!ww_mutex_is_locked(&crtc->mutex)); > > WARN_ON(!mutex_is_locked(&dev->mode_config.mutex)); > } > @@ -613,6 +613,8 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb) > } > EXPORT_SYMBOL(drm_framebuffer_remove); > > +static DEFINE_WW_CLASS(crtc_ww_class); > + > /** > * drm_crtc_init - Initialise a new CRTC object > * @dev: DRM device > @@ -634,8 +636,8 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > crtc->invert_dimensions = false; > > drm_modeset_lock_all(dev); > - mutex_init(&crtc->mutex); > - mutex_lock_nest_lock(&crtc->mutex, &dev->mode_config.mutex); > + ww_mutex_init(&crtc->mutex, &crtc_ww_class); > + mutex_lock_nest_lock(&crtc->mutex.base, &dev->mode_config.mutex); In a later patch you keep this snippet, please make this a trylock instead. It removes the assumption that ww_mutex has mutex as base. ~Maarten _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
