From a6a8bb848d5ca40bc0eb708ddeb23df2b0eca1fb Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 25 Jul 2014 17:47:18 +0200 Subject: drm: Move modeset_lock_all helpers to drm_modeset_lock.[hc] Somehow we've forgotten about this little bit of OCD. Reviewed-by: Dave Airlie Reviewed-by: Matt Roper Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_modeset_lock.c | 95 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) (limited to 'drivers/gpu/drm/drm_modeset_lock.c') diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index 0dc57d5ecd10..73e6534fd0aa 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -56,6 +56,101 @@ */ +/** + * drm_modeset_lock_all - take all modeset locks + * @dev: drm device + * + * This function takes all modeset locks, suitable where a more fine-grained + * scheme isn't (yet) implemented. Locks must be dropped with + * drm_modeset_unlock_all. + */ +void drm_modeset_lock_all(struct drm_device *dev) +{ + struct drm_mode_config *config = &dev->mode_config; + struct drm_modeset_acquire_ctx *ctx; + int ret; + + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); + if (WARN_ON(!ctx)) + return; + + mutex_lock(&config->mutex); + + drm_modeset_acquire_init(ctx, 0); + +retry: + ret = drm_modeset_lock(&config->connection_mutex, ctx); + if (ret) + goto fail; + ret = drm_modeset_lock_all_crtcs(dev, ctx); + if (ret) + goto fail; + + WARN_ON(config->acquire_ctx); + + /* now we hold the locks, so now that it is safe, stash the + * ctx for drm_modeset_unlock_all(): + */ + config->acquire_ctx = ctx; + + drm_warn_on_modeset_not_all_locked(dev); + + return; + +fail: + if (ret == -EDEADLK) { + drm_modeset_backoff(ctx); + goto retry; + } +} +EXPORT_SYMBOL(drm_modeset_lock_all); + +/** + * drm_modeset_unlock_all - drop all modeset locks + * @dev: device + * + * This function drop all modeset locks taken by drm_modeset_lock_all. + */ +void drm_modeset_unlock_all(struct drm_device *dev) +{ + struct drm_mode_config *config = &dev->mode_config; + struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx; + + if (WARN_ON(!ctx)) + return; + + config->acquire_ctx = NULL; + drm_modeset_drop_locks(ctx); + drm_modeset_acquire_fini(ctx); + + kfree(ctx); + + mutex_unlock(&dev->mode_config.mutex); +} +EXPORT_SYMBOL(drm_modeset_unlock_all); + +/** + * drm_warn_on_modeset_not_all_locked - check that all modeset locks are locked + * @dev: device + * + * Useful as a debug assert. + */ +void drm_warn_on_modeset_not_all_locked(struct drm_device *dev) +{ + struct drm_crtc *crtc; + + /* Locking is currently fubar in the panic handler. */ + if (oops_in_progress) + return; + + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) + WARN_ON(!drm_modeset_is_locked(&crtc->mutex)); + + WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); + WARN_ON(!mutex_is_locked(&dev->mode_config.mutex)); +} +EXPORT_SYMBOL(drm_warn_on_modeset_not_all_locked); + /** * drm_modeset_acquire_init - initialize acquire context * @ctx: the acquire context -- cgit v1.2.3 From d059f652e73c35678d28d4cd09ab2cec89696af9 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 25 Jul 2014 18:07:40 +0200 Subject: drm: Handle legacy per-crtc locking with full acquire ctx So drivers using the atomic interfaces expect that they can acquire additional locks internal to the driver as-needed. Examples would be locks to protect shared state like shared display PLLs. Unfortunately the legacy ioctls assume that all locking is fully done by the drm core. Now for those paths which grab all locks we already have to keep around an acquire context in dev->mode_config. Helper functions that implement legacy interfaces in terms of atomic support can therefore grab this acquire contexts and reuse it. The only interfaces left are the cursor and pageflip ioctls. So add functions to grab the crtc lock these need using an acquire context and preserve it for atomic drivers to reuse. v2: - Fixup comments&kerneldoc. - Drop the WARNING from modeset_lock_all_crtcs since that can be used in legacy paths with crtc locking. v3: Fix a type on the kerneldoc Dave spotted. Cc: Dave Airlie Reviewed-by: Dave Airlie Reviewed-by: Matt Roper Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 8 ++-- drivers/gpu/drm/drm_modeset_lock.c | 84 ++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 6 +++ include/drm/drm_modeset_lock.h | 5 +++ 4 files changed, 99 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/drm_modeset_lock.c') diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index caaa01f3b353..ab121b6d980c 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2801,7 +2801,7 @@ static int drm_mode_cursor_common(struct drm_device *dev, if (crtc->cursor) return drm_mode_cursor_universal(crtc, req, file_priv); - drm_modeset_lock(&crtc->mutex, NULL); + drm_modeset_lock_crtc(crtc); if (req->flags & DRM_MODE_CURSOR_BO) { if (!crtc->funcs->cursor_set && !crtc->funcs->cursor_set2) { ret = -ENXIO; @@ -2825,7 +2825,7 @@ static int drm_mode_cursor_common(struct drm_device *dev, } } out: - drm_modeset_unlock(&crtc->mutex); + drm_modeset_unlock_crtc(crtc); return ret; @@ -4561,7 +4561,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, if (!crtc) return -ENOENT; - drm_modeset_lock(&crtc->mutex, NULL); + drm_modeset_lock_crtc(crtc); if (crtc->primary->fb == NULL) { /* The framebuffer is currently unbound, presumably * due to a hotplug event, that userspace has not @@ -4645,7 +4645,7 @@ out: drm_framebuffer_unreference(fb); if (old_fb) drm_framebuffer_unreference(old_fb); - drm_modeset_unlock(&crtc->mutex); + drm_modeset_unlock_crtc(crtc); return ret; } diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index 73e6534fd0aa..4753c8bd5ab5 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -129,6 +129,90 @@ void drm_modeset_unlock_all(struct drm_device *dev) } EXPORT_SYMBOL(drm_modeset_unlock_all); +/** + * drm_modeset_lock_crtc - lock crtc with hidden acquire ctx + * @crtc: drm crtc + * + * This function locks the given crtc using a hidden acquire context. This is + * necessary so that drivers internally using the atomic interfaces can grab + * further locks with the lock acquire context. + */ +void drm_modeset_lock_crtc(struct drm_crtc *crtc) +{ + struct drm_modeset_acquire_ctx *ctx; + int ret; + + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); + if (WARN_ON(!ctx)) + return; + + drm_modeset_acquire_init(ctx, 0); + +retry: + ret = drm_modeset_lock(&crtc->mutex, ctx); + if (ret) + goto fail; + + WARN_ON(crtc->acquire_ctx); + + /* now we hold the locks, so now that it is safe, stash the + * ctx for drm_modeset_unlock_crtc(): + */ + crtc->acquire_ctx = ctx; + + return; + +fail: + if (ret == -EDEADLK) { + drm_modeset_backoff(ctx); + goto retry; + } +} +EXPORT_SYMBOL(drm_modeset_lock_crtc); + +/** + * drm_modeset_legacy_acquire_ctx - find acquire ctx for legacy ioctls + * crtc: drm crtc + * + * Legacy ioctl operations like cursor updates or page flips only have per-crtc + * locking, and store the acquire ctx in the corresponding crtc. All other + * legacy operations take all locks and use a global acquire context. This + * function grabs the right one. + */ +struct drm_modeset_acquire_ctx * +drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc) +{ + if (crtc->acquire_ctx) + return crtc->acquire_ctx; + + WARN_ON(!crtc->dev->mode_config.acquire_ctx); + + return crtc->dev->mode_config.acquire_ctx; +} +EXPORT_SYMBOL(drm_modeset_legacy_acquire_ctx); + +/** + * drm_modeset_unlock_crtc - drop crtc lock + * @crtc: drm crtc + * + * This drops the crtc lock acquire with drm_modeset_lock_crtc() and all other + * locks acquired through the hidden context. + */ +void drm_modeset_unlock_crtc(struct drm_crtc *crtc) +{ + struct drm_modeset_acquire_ctx *ctx = crtc->acquire_ctx; + + if (WARN_ON(!ctx)) + return; + + crtc->acquire_ctx = NULL; + drm_modeset_drop_locks(ctx); + drm_modeset_acquire_fini(ctx); + + kfree(ctx); +} +EXPORT_SYMBOL(drm_modeset_unlock_crtc); + /** * drm_warn_on_modeset_not_all_locked - check that all modeset locks are locked * @dev: device diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index a11d73422e7f..508817bae538 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -371,6 +371,12 @@ struct drm_crtc { void *helper_private; struct drm_object_properties properties; + + /* + * For legacy crtc ioctls so that atomic drivers can get at the locking + * acquire context. + */ + struct drm_modeset_acquire_ctx *acquire_ctx; }; diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h index cf61e857bc06..d38e1508f11a 100644 --- a/include/drm/drm_modeset_lock.h +++ b/include/drm/drm_modeset_lock.h @@ -120,10 +120,15 @@ int drm_modeset_lock_interruptible(struct drm_modeset_lock *lock, void drm_modeset_unlock(struct drm_modeset_lock *lock); struct drm_device; +struct drm_crtc; void drm_modeset_lock_all(struct drm_device *dev); void drm_modeset_unlock_all(struct drm_device *dev); +void drm_modeset_lock_crtc(struct drm_crtc *crtc); +void drm_modeset_unlock_crtc(struct drm_crtc *crtc); void drm_warn_on_modeset_not_all_locked(struct drm_device *dev); +struct drm_modeset_acquire_ctx * +drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc); int drm_modeset_lock_all_crtcs(struct drm_device *dev, struct drm_modeset_acquire_ctx *ctx); -- cgit v1.2.3 From cb597bb3a2fbfc871cc1c703fb330d247bd21394 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Sun, 27 Jul 2014 19:09:33 +0200 Subject: drm: trylock modest locking for fbdev panics In the fbdev code we want to do trylocks only to avoid deadlocks and other ugly issues. Thus far we've only grabbed the overall modeset lock, but that already failed to exclude a pile of potential concurrent operations. With proper atomic support this will be worse. So add a trylock mode to the modeset locking code which attempts all locks only with trylocks, if possible. We need to track this in the locking functions themselves and can't restrict this to drivers since driver-private w/w mutexes must be treated the same way. There's still the issue that other driver private locks aren't handled here at all, but well can't have everything. With this we will at least not regress, even once atomic allows lots of concurrent kms activity. Aside: We should move the acquire context to stack-based allocation in the callers to get rid of that awful WARN_ON(kmalloc_failed) control flow which just blows up when memory is short. But that's material for separate patches. v2: - Fix logic inversion fumble in the fb helper. - Add proper kerneldoc. Reviewed-by: Matt Roper Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_fb_helper.c | 10 +++---- drivers/gpu/drm/drm_modeset_lock.c | 56 ++++++++++++++++++++++++++++++-------- include/drm/drm_modeset_lock.h | 6 ++++ 3 files changed, 55 insertions(+), 17 deletions(-) (limited to 'drivers/gpu/drm/drm_modeset_lock.c') diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 3a6b6635e3f5..7b7b9565188f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -365,11 +365,11 @@ static bool drm_fb_helper_force_kernel_mode(void) if (dev->switch_power_state == DRM_SWITCH_POWER_OFF) continue; - /* NOTE: we use lockless flag below to avoid grabbing other - * modeset locks. So just trylock the underlying mutex - * directly: + /* + * NOTE: Use trylock mode to avoid deadlocks and sleeping in + * panic context. */ - if (!mutex_trylock(&dev->mode_config.mutex)) { + if (__drm_modeset_lock_all(dev, true) != 0) { error = true; continue; } @@ -378,7 +378,7 @@ static bool drm_fb_helper_force_kernel_mode(void) if (ret) error = true; - mutex_unlock(&dev->mode_config.mutex); + drm_modeset_unlock_all(dev); } return error; } diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index 4753c8bd5ab5..5280b64a0230 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -57,26 +57,37 @@ /** - * drm_modeset_lock_all - take all modeset locks - * @dev: drm device + * __drm_modeset_lock_all - internal helper to grab all modeset locks + * @dev: DRM device + * @trylock: trylock mode for atomic contexts * - * This function takes all modeset locks, suitable where a more fine-grained - * scheme isn't (yet) implemented. Locks must be dropped with - * drm_modeset_unlock_all. + * This is a special version of drm_modeset_lock_all() which can also be used in + * atomic contexts. Then @trylock must be set to true. + * + * Returns: + * 0 on success or negative error code on failure. */ -void drm_modeset_lock_all(struct drm_device *dev) +int __drm_modeset_lock_all(struct drm_device *dev, + bool trylock) { struct drm_mode_config *config = &dev->mode_config; struct drm_modeset_acquire_ctx *ctx; int ret; - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); - if (WARN_ON(!ctx)) - return; + ctx = kzalloc(sizeof(*ctx), + trylock ? GFP_ATOMIC : GFP_KERNEL); + if (!ctx) + return -ENOMEM; - mutex_lock(&config->mutex); + if (trylock) { + if (!mutex_trylock(&config->mutex)) + return -EBUSY; + } else { + mutex_lock(&config->mutex); + } drm_modeset_acquire_init(ctx, 0); + ctx->trylock_only = trylock; retry: ret = drm_modeset_lock(&config->connection_mutex, ctx); @@ -95,13 +106,29 @@ retry: drm_warn_on_modeset_not_all_locked(dev); - return; + return 0; fail: if (ret == -EDEADLK) { drm_modeset_backoff(ctx); goto retry; } + + return ret; +} +EXPORT_SYMBOL(__drm_modeset_lock_all); + +/** + * drm_modeset_lock_all - take all modeset locks + * @dev: drm device + * + * This function takes all modeset locks, suitable where a more fine-grained + * scheme isn't (yet) implemented. Locks must be dropped with + * drm_modeset_unlock_all. + */ +void drm_modeset_lock_all(struct drm_device *dev) +{ + WARN_ON(__drm_modeset_lock_all(dev, false) != 0); } EXPORT_SYMBOL(drm_modeset_lock_all); @@ -287,7 +314,12 @@ static inline int modeset_lock(struct drm_modeset_lock *lock, WARN_ON(ctx->contended); - if (interruptible && slow) { + if (ctx->trylock_only) { + if (!ww_mutex_trylock(&lock->mutex)) + return -EBUSY; + else + return 0; + } else if (interruptible && slow) { ret = ww_mutex_lock_slow_interruptible(&lock->mutex, &ctx->ww_ctx); } else if (interruptible) { ret = ww_mutex_lock_interruptible(&lock->mutex, &ctx->ww_ctx); diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h index d38e1508f11a..a3f736d24382 100644 --- a/include/drm/drm_modeset_lock.h +++ b/include/drm/drm_modeset_lock.h @@ -53,6 +53,11 @@ struct drm_modeset_acquire_ctx { * list of held locks (drm_modeset_lock) */ struct list_head locked; + + /** + * Trylock mode, use only for panic handlers! + */ + bool trylock_only; }; /** @@ -123,6 +128,7 @@ struct drm_device; struct drm_crtc; void drm_modeset_lock_all(struct drm_device *dev); +int __drm_modeset_lock_all(struct drm_device *dev, bool trylock); void drm_modeset_unlock_all(struct drm_device *dev); void drm_modeset_lock_crtc(struct drm_crtc *crtc); void drm_modeset_unlock_crtc(struct drm_crtc *crtc); -- cgit v1.2.3 From 295ee85316aedfe1878306d71b5e9c7d4498fb1b Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 30 Jul 2014 14:23:44 +0200 Subject: drm: Docbook fixes Bunch of small leftovers spotted by looking at the make htmldocs output. I've left out dp mst, there's too much amiss there. v2: Also add the missing parameter docbook in the dp mst code - Dave Airlie correctly pointed out that we don't actually want kerneldoc for the missing structure members in header files. Cc: Dave Airlie Reviewed-by: Matt Roper Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 5 +++-- drivers/gpu/drm/drm_dp_mst_topology.c | 1 + drivers/gpu/drm/drm_edid.c | 2 +- drivers/gpu/drm/drm_modeset_lock.c | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/drm_modeset_lock.c') diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 285e62a134b2..f09b75212081 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3512,9 +3512,10 @@ EXPORT_SYMBOL(drm_property_create_enum); * @flags: flags specifying the property type * @name: name of the property * @props: enumeration lists with property bitflags - * @num_values: number of pre-defined values + * @num_props: size of the @props array + * @supported_bits: bitmask of all supported enumeration values * - * This creates a new generic drm property which can then be attached to a drm + * This creates a new bitmask drm property which can then be attached to a drm * object with drm_object_attach_property. The returned property object must be * freed with drm_property_destroy. * diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index ac3c2738db94..352f5d6c763c 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2071,6 +2071,7 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr) * drm_dp_mst_hpd_irq() - MST hotplug IRQ notify * @mgr: manager to notify irq for. * @esi: 4 bytes from SINK_COUNT_ESI + * @handled: whether the hpd interrupt was consumed or not * * This should be called from the driver when it detects a short IRQ, * along with the value of the DEVICE_SERVICE_IRQ_VECTOR_ESI0. The diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1dbf3bc4c6a3..f905c63c0f68 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3433,10 +3433,10 @@ EXPORT_SYMBOL(drm_rgb_quant_range_selectable); /** * drm_assign_hdmi_deep_color_info - detect whether monitor supports * hdmi deep color modes and update drm_display_info if so. - * * @edid: monitor EDID information * @info: Updated with maximum supported deep color bpc and color format * if deep color supported. + * @connector: DRM connector, used only for debug output * * Parse the CEA extension according to CEA-861-B. * Return true if HDMI deep color supported, false if not or unknown. diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index 5280b64a0230..8749fc06570e 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -199,7 +199,7 @@ EXPORT_SYMBOL(drm_modeset_lock_crtc); /** * drm_modeset_legacy_acquire_ctx - find acquire ctx for legacy ioctls - * crtc: drm crtc + * @crtc: drm crtc * * Legacy ioctl operations like cursor updates or page flips only have per-crtc * locking, and store the acquire ctx in the corresponding crtc. All other -- cgit v1.2.3