From patchwork Fri Jul 14 19:14:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 9841581 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0184E60212 for ; Fri, 14 Jul 2017 19:14:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EB591287B4 for ; Fri, 14 Jul 2017 19:14:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DE7CD287B7; Fri, 14 Jul 2017 19:14:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 8050B287B0 for ; Fri, 14 Jul 2017 19:14:55 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5E5436E8BC; Fri, 14 Jul 2017 19:14:52 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wm0-x243.google.com (mail-wm0-x243.google.com [IPv6:2a00:1450:400c:c09::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7CFBB6E038 for ; Fri, 14 Jul 2017 19:14:49 +0000 (UTC) Received: by mail-wm0-x243.google.com with SMTP id u23so12387188wma.2 for ; Fri, 14 Jul 2017 12:14:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=F86A1P+nZE1jxr2Iw3BSSLlbwY97KOSf0E0IHg6pazQ=; b=No6fc09RhzTQrOw3dJrTP73wSdgDYxC6b7JVkV0NA9Lo0cv2a3hH3iloYHwr69CnBG 5+0sCDnu2foWei8WS2C82H825B0XiN2RzaEhKVSsp0Hna42xxDuAYJhlC0pMzoBkpBIG 92X7VOE1+7QeEqPSSV2V7h7lW7hGIE6CJTX84= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=F86A1P+nZE1jxr2Iw3BSSLlbwY97KOSf0E0IHg6pazQ=; b=WRXAqnvDhV4odN8Odicc8ViX/k/S2nfcSmrkEBIvKrIh6Zi7NVb/6vLQtck3+0sh6k mOxA1bxBpf5PE44efw5cGfkbMZpM2dqNiWqIdc/3rXbk/Ar54SJXUj+Jp21noMxhJG0A iqMhbaL8B1Laye0pAaQWpDB9m0geK2nAcPJNmGJLUNFh2zl/PG3HYhNGt1aOWw6YR/lm dUysvzEsL4iENNktgArzGfHdmmgK27RAAxEHyCjHl+Paj0/llbGvzUJDOTNXp2e7Pu94 XAKGF/xKFlUFEWAPA9nSY8RF4CW8dpVsWzGXEMR3diXXS6dHg8aLhnsec3nfLdfoatv5 3krg== X-Gm-Message-State: AIVw110ASvFqSRebj1bqx7dROTy7PAyWzRDJ5uKEt6lXZwjs196tlo5z uZFLq7fCcC3MouNH X-Received: by 10.80.188.21 with SMTP id j21mr7791904edh.159.1500059688158; Fri, 14 Jul 2017 12:14:48 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:5640:0:960b:2678:e223:c1c6]) by smtp.gmail.com with ESMTPSA id x36sm4676940edb.64.2017.07.14.12.14.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 14 Jul 2017 12:14:47 -0700 (PDT) From: Daniel Vetter To: DRI Development Date: Fri, 14 Jul 2017 21:14:38 +0200 Message-Id: <20170714191439.31169-2-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.13.2 In-Reply-To: <20170714191439.31169-1-daniel.vetter@ffwll.ch> References: <20170714191439.31169-1-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Daniel Vetter , Intel Graphics Development , martin.peres@free.fr Subject: [Intel-gfx] [PATCH 2/3] drm/i915: Fix fbdev unload sequence X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP First thing we need to do is unregister the fbdev instance, but we can't just go ahead and kfree it. That must wait until the hotplug and polling work are stopped, since they can race with the with the teardown. That means we need to split up the fbdev teardown into the unregister part and the cleanup part. I originally suspected that this was broken in one of the unload shuffles, but on closer inspection the oldest sequence I've dug out also gets this wrong. Just not quite so badly. I've run drv_module_reload a few hundred times and it's rock solid compared to insta-death beforehand. This bug seems to have been uncovered by commit 88be58be886f1215cc73dc8c273c985eecd7385c Author: Daniel Vetter Date: Thu Jul 6 15:00:19 2017 +0200 drm/i915/fbdev: Always forward hotplug events But the effect of that seems to only be to increase the race window enough to make it blow up easier. I'm not exactly clear on what's going on there ... Testcase: igt/drv_module_reload Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101791 Cc: martin.peres@free.fr Cc: chris@chris-wilson.co.uk Signed-off-by: Daniel Vetter Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.c | 3 +-- drivers/gpu/drm/i915/intel_display.c | 3 +++ drivers/gpu/drm/i915/intel_drv.h | 9 +++++++-- drivers/gpu/drm/i915/intel_fbdev.c | 21 +++++++++++++++------ 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index d310d8245dca..3a6dc04bd51e 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1240,6 +1240,7 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) */ static void i915_driver_unregister(struct drm_i915_private *dev_priv) { + intel_fbdev_unregister(dev_priv); intel_audio_deinit(dev_priv); intel_gpu_ips_teardown(); @@ -1371,7 +1372,6 @@ void i915_driver_unload(struct drm_device *dev) struct drm_i915_private *dev_priv = to_i915(dev); struct pci_dev *pdev = dev_priv->drm.pdev; - intel_fbdev_fini(dev); if (i915_gem_suspend(dev_priv)) DRM_ERROR("failed to idle hardware; continuing to unload!\n"); @@ -1383,7 +1383,6 @@ void i915_driver_unload(struct drm_device *dev) intel_gvt_cleanup(dev_priv); i915_driver_unregister(dev_priv); - intel_modeset_cleanup(dev); /* diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ecb4e30673fc..e10fae2faa88 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15111,6 +15111,9 @@ void intel_modeset_cleanup(struct drm_device *dev) */ drm_kms_helper_poll_fini(dev); + /* poll work can call into fbdev, hence clean that up afterwards */ + intel_fbdev_fini(dev_priv); + intel_unregister_dsm_handler(); intel_fbc_global_disable(dev_priv); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 3132260f18ce..7361e983d953 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1597,7 +1597,8 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv); #ifdef CONFIG_DRM_FBDEV_EMULATION extern int intel_fbdev_init(struct drm_device *dev); extern void intel_fbdev_initial_config_async(struct drm_device *dev); -extern void intel_fbdev_fini(struct drm_device *dev); +extern void intel_fbdev_unregister(struct drm_i915_private *dev_priv); +extern void intel_fbdev_fini(struct drm_i915_private *dev_priv); extern void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous); extern void intel_fbdev_output_poll_changed(struct drm_device *dev); extern void intel_fbdev_restore_mode(struct drm_device *dev); @@ -1611,7 +1612,11 @@ static inline void intel_fbdev_initial_config_async(struct drm_device *dev) { } -static inline void intel_fbdev_fini(struct drm_device *dev) +static inline void intel_fbdev_unregister(struct drm_i915_private *dev_priv) +{ +} + +static inline void intel_fbdev_fini(struct drm_i915_private *dev_priv) { } diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index b953365a3eec..82280e104692 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -538,8 +538,6 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) * trying to rectify all the possible error paths leading here. */ - drm_fb_helper_unregister_fbi(&ifbdev->helper); - drm_fb_helper_fini(&ifbdev->helper); if (ifbdev->vma) { @@ -727,8 +725,10 @@ static void intel_fbdev_initial_config(void *data, async_cookie_t cookie) /* Due to peculiar init order wrt to hpd handling this is separate. */ if (drm_fb_helper_initial_config(&ifbdev->helper, - ifbdev->preferred_bpp)) - intel_fbdev_fini(ifbdev->helper.dev); + ifbdev->preferred_bpp)) { + intel_fbdev_unregister(to_i915(ifbdev->helper.dev)); + intel_fbdev_fini(to_i915(ifbdev->helper.dev)); + } } void intel_fbdev_initial_config_async(struct drm_device *dev) @@ -751,9 +751,8 @@ static void intel_fbdev_sync(struct intel_fbdev *ifbdev) ifbdev->cookie = 0; } -void intel_fbdev_fini(struct drm_device *dev) +void intel_fbdev_unregister(struct drm_i915_private *dev_priv) { - struct drm_i915_private *dev_priv = to_i915(dev); struct intel_fbdev *ifbdev = dev_priv->fbdev; if (!ifbdev) @@ -763,6 +762,16 @@ void intel_fbdev_fini(struct drm_device *dev) if (!current_is_async()) intel_fbdev_sync(ifbdev); + drm_fb_helper_unregister_fbi(&ifbdev->helper); +} + +void intel_fbdev_fini(struct drm_i915_private *dev_priv) +{ + struct intel_fbdev *ifbdev = dev_priv->fbdev; + + if (!ifbdev) + return; + intel_fbdev_destroy(ifbdev); dev_priv->fbdev = NULL; }