From patchwork Mon Aug 12 11:50:45 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 2843048 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 12F949F294 for ; Mon, 12 Aug 2013 11:51:08 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 19F9A20221 for ; Mon, 12 Aug 2013 11:51:03 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 3FAC02018C for ; Mon, 12 Aug 2013 11:51:01 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1494EE5D19 for ; Mon, 12 Aug 2013 04:51:01 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from youngberry.canonical.com (youngberry.canonical.com [91.189.89.112]) by gabe.freedesktop.org (Postfix) with ESMTP id 781CEE5D19; Mon, 12 Aug 2013 04:50:46 -0700 (PDT) Received: from 5ed49945.cm-7-5c.dynamic.ziggo.nl ([94.212.153.69] helo=[192.168.1.128]) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1V8qe5-0001iF-QL; Mon, 12 Aug 2013 11:50:45 +0000 Message-ID: <5208CC15.8080403@canonical.com> Date: Mon, 12 Aug 2013 13:50:45 +0200 From: Maarten Lankhorst User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130623 Thunderbird/17.0.7 MIME-Version: 1.0 To: "nouveau@lists.freedesktop.org" Subject: [PATCH] drm/nouveau: fix vblank deadlock Cc: "dri-devel@lists.freedesktop.org" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This fixes a deadlock inversion when vblank is enabled/disabled by drm. &dev->vblank_time_lock is always taken when the vblank state is toggled, which caused a deadlock when &event->lock was also taken during event_get/put. Solve the race by requiring that lock to change enable/disable state, and always keeping vblank on the event list. Core drm ignores unwanted vblanks, so extra calls to drm_handle_vblank are harmless. Signed-off-by: Maarten Lankhorst diff --git a/drivers/gpu/drm/nouveau/core/core/event.c b/drivers/gpu/drm/nouveau/core/core/event.c index 7eb81c1..78bff7c 100644 --- a/drivers/gpu/drm/nouveau/core/core/event.c +++ b/drivers/gpu/drm/nouveau/core/core/event.c @@ -23,43 +23,64 @@ #include #include -static void -nouveau_event_put_locked(struct nouveau_event *event, int index, - struct nouveau_eventh *handler) -{ - if (!--event->index[index].refs) { - if (event->disable) - event->disable(event, index); - } - list_del(&handler->head); -} - void nouveau_event_put(struct nouveau_event *event, int index, struct nouveau_eventh *handler) { unsigned long flags; + if (index >= event->index_nr) + return; + spin_lock_irqsave(&event->lock, flags); - if (index < event->index_nr) - nouveau_event_put_locked(event, index, handler); + list_del(&handler->head); + + if (event->toggle_lock) + spin_lock(event->toggle_lock); + nouveau_event_disable_locked(event, index, 1); + if (event->toggle_lock) + spin_unlock(event->toggle_lock); + spin_unlock_irqrestore(&event->lock, flags); } void +nouveau_event_enable_locked(struct nouveau_event *event, int index) +{ + if (index >= event->index_nr) + return; + + if (!event->index[index].refs++ && event->enable) + event->enable(event, index); +} + +void +nouveau_event_disable_locked(struct nouveau_event *event, int index, int refs) +{ + if (index >= event->index_nr) + return; + + event->index[index].refs -= refs; + if (!event->index[index].refs && event->disable) + event->disable(event, index); +} + +void nouveau_event_get(struct nouveau_event *event, int index, struct nouveau_eventh *handler) { unsigned long flags; + if (index >= event->index_nr) + return; + spin_lock_irqsave(&event->lock, flags); - if (index < event->index_nr) { - list_add(&handler->head, &event->index[index].list); - if (!event->index[index].refs++) { - if (event->enable) - event->enable(event, index); - } - } + list_add(&handler->head, &event->index[index].list); + if (event->toggle_lock) + spin_lock(event->toggle_lock); + nouveau_event_enable_locked(event, index); + if (event->toggle_lock) + spin_unlock(event->toggle_lock); spin_unlock_irqrestore(&event->lock, flags); } @@ -68,6 +89,7 @@ nouveau_event_trigger(struct nouveau_event *event, int index) { struct nouveau_eventh *handler, *temp; unsigned long flags; + int refs = 0; if (index >= event->index_nr) return; @@ -75,9 +97,17 @@ nouveau_event_trigger(struct nouveau_event *event, int index) spin_lock_irqsave(&event->lock, flags); list_for_each_entry_safe(handler, temp, &event->index[index].list, head) { if (handler->func(handler, index) == NVKM_EVENT_DROP) { - nouveau_event_put_locked(event, index, handler); + list_del(&handler->head); + refs++; } } + if (refs) { + if (event->toggle_lock) + spin_lock(event->toggle_lock); + nouveau_event_disable_locked(event, index, refs); + if (event->toggle_lock) + spin_unlock(event->toggle_lock); + } spin_unlock_irqrestore(&event->lock, flags); } diff --git a/drivers/gpu/drm/nouveau/core/include/core/event.h b/drivers/gpu/drm/nouveau/core/include/core/event.h index 9e09440..b1a6c91 100644 --- a/drivers/gpu/drm/nouveau/core/include/core/event.h +++ b/drivers/gpu/drm/nouveau/core/include/core/event.h @@ -12,6 +12,7 @@ struct nouveau_eventh { struct nouveau_event { spinlock_t lock; + spinlock_t *toggle_lock; void *priv; void (*enable)(struct nouveau_event *, int index); @@ -33,4 +34,8 @@ void nouveau_event_get(struct nouveau_event *, int index, void nouveau_event_put(struct nouveau_event *, int index, struct nouveau_eventh *); +void nouveau_event_enable_locked(struct nouveau_event *event, int index); +void nouveau_event_disable_locked(struct nouveau_event *event, int index, + int refs); + #endif diff --git a/drivers/gpu/drm/nouveau/nouveau_crtc.h b/drivers/gpu/drm/nouveau/nouveau_crtc.h index d1e5890..398baa3 100644 --- a/drivers/gpu/drm/nouveau/nouveau_crtc.h +++ b/drivers/gpu/drm/nouveau/nouveau_crtc.h @@ -29,6 +29,7 @@ struct nouveau_crtc { struct drm_crtc base; + struct nouveau_eventh vblank; int index; diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h index 1ea3e47..4ba8cb5 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.h +++ b/drivers/gpu/drm/nouveau/nouveau_display.h @@ -72,6 +72,7 @@ int nouveau_display_dumb_destroy(struct drm_file *, struct drm_device *, u32 handle); void nouveau_hdmi_mode_set(struct drm_encoder *, struct drm_display_mode *); +int nouveau_drm_vblank_handler(struct nouveau_eventh *event, int head); #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT extern int nouveau_backlight_init(struct drm_device *); diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 4bf8dc3..bd301f4 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -46,6 +46,7 @@ #include "nouveau_pm.h" #include "nouveau_acpi.h" #include "nouveau_bios.h" +#include "nouveau_crtc.h" #include "nouveau_ioctl.h" #include "nouveau_abi16.h" #include "nouveau_fbcon.h" @@ -71,12 +72,13 @@ module_param_named(modeset, nouveau_modeset, int, 0400); static struct drm_driver driver; -static int +int nouveau_drm_vblank_handler(struct nouveau_eventh *event, int head) { - struct nouveau_drm *drm = - container_of(event, struct nouveau_drm, vblank[head]); - drm_handle_vblank(drm->dev, head); + struct nouveau_crtc *nv_crtc = + container_of(event, struct nouveau_crtc, vblank); + + drm_handle_vblank(nv_crtc->base.dev, head); return NVKM_EVENT_KEEP; } @@ -86,11 +88,9 @@ nouveau_drm_vblank_enable(struct drm_device *dev, int head) struct nouveau_drm *drm = nouveau_drm(dev); struct nouveau_disp *pdisp = nouveau_disp(drm->device); - if (WARN_ON_ONCE(head > ARRAY_SIZE(drm->vblank))) + if (head >= pdisp->vblank->index_nr) return -EIO; - WARN_ON_ONCE(drm->vblank[head].func); - drm->vblank[head].func = nouveau_drm_vblank_handler; - nouveau_event_get(pdisp->vblank, head, &drm->vblank[head]); + nouveau_event_enable_locked(pdisp->vblank, head); return 0; } @@ -99,11 +99,9 @@ nouveau_drm_vblank_disable(struct drm_device *dev, int head) { struct nouveau_drm *drm = nouveau_drm(dev); struct nouveau_disp *pdisp = nouveau_disp(drm->device); - if (drm->vblank[head].func) - nouveau_event_put(pdisp->vblank, head, &drm->vblank[head]); - else - WARN_ON_ONCE(1); - drm->vblank[head].func = NULL; + + if (head < pdisp->vblank->index_nr) + nouveau_event_disable_locked(pdisp->vblank, head, 1); } static u64 diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.h b/drivers/gpu/drm/nouveau/nouveau_drm.h index 41ff7e0..0d0ba3b 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.h +++ b/drivers/gpu/drm/nouveau/nouveau_drm.h @@ -125,7 +125,6 @@ struct nouveau_drm { struct nvbios vbios; struct nouveau_display *display; struct backlight_device *backlight; - struct nouveau_eventh vblank[4]; /* power management */ struct nouveau_pm *pm; diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c index 007731d..738d7a2 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include @@ -1280,15 +1281,21 @@ nv50_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, static void nv50_crtc_destroy(struct drm_crtc *crtc) { + struct nouveau_event *event = nouveau_disp(nouveau_dev(crtc->dev))->vblank; struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc); struct nv50_disp *disp = nv50_disp(crtc->dev); struct nv50_head *head = nv50_head(crtc); + unsigned long flags; nv50_dmac_destroy(disp->core, &head->ovly.base); nv50_pioc_destroy(disp->core, &head->oimm.base); nv50_dmac_destroy(disp->core, &head->sync.base); nv50_pioc_destroy(disp->core, &head->curs.base); + spin_lock_irqsave(&event->lock, flags); + list_del(&nv_crtc->vblank.head); + spin_unlock_irqrestore(&event->lock, flags); + /*XXX: this shouldn't be necessary, but the core doesn't call * disconnect() during the cleanup paths */ @@ -1344,10 +1351,12 @@ nv50_cursor_set_offset(struct nouveau_crtc *nv_crtc, uint32_t offset) static int nv50_crtc_create(struct drm_device *dev, struct nouveau_object *core, int index) { + struct nouveau_event *event = nouveau_disp(nouveau_dev(dev))->vblank; struct nv50_disp *disp = nv50_disp(dev); struct nv50_head *head; struct drm_crtc *crtc; int ret, i; + unsigned long flags; head = kzalloc(sizeof(*head), GFP_KERNEL); if (!head) @@ -1372,6 +1381,12 @@ nv50_crtc_create(struct drm_device *dev, struct nouveau_object *core, int index) drm_crtc_helper_add(crtc, &nv50_crtc_hfunc); drm_mode_crtc_set_gamma_size(crtc, 256); + spin_lock_irqsave(&event->lock, flags); + event->toggle_lock = &dev->vblank_time_lock; + head->base.vblank.func = nouveau_drm_vblank_handler; + list_add(&head->base.vblank.head, &event->index[index].list); + spin_unlock_irqrestore(&event->lock, flags); + ret = nouveau_bo_new(dev, 8192, 0x100, TTM_PL_FLAG_VRAM, 0, 0x0000, NULL, NULL, &head->base.lut.nvbo); if (!ret) {