From patchwork Thu Jul 16 14:48:47 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 6807711 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id B49379F2E8 for ; Thu, 16 Jul 2015 14:46:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BD927206EB for ; Thu, 16 Jul 2015 14:46:14 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 2869C206E8 for ; Thu, 16 Jul 2015 14:46:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1AB756EC63; Thu, 16 Jul 2015 07:46:12 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wg0-f43.google.com (mail-wg0-f43.google.com [74.125.82.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 885C66EC63 for ; Thu, 16 Jul 2015 07:46:10 -0700 (PDT) Received: by wgmn9 with SMTP id n9so60448056wgm.0 for ; Thu, 16 Jul 2015 07:46:09 -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; bh=vZHJak+iTTlC9ZlfufD3n4YYMXa21ZvNp5A9zx/2KCs=; b=AQk2D0dnXlH6n7PFHRRj7jjT1BNyrI5WK7UJtWziI7py1boe2NQxKp0vC/lF7H7Q8S pGuDtWYmeLHEvvmJOYLVIy+ztwjVWx5dHVmhw5zhu0+w9mOGjFKMXOzYFydtoX9gN6bI MT/uzP/yFquJmNKUm6P9Zv4hYEk+115Khx+rg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=vZHJak+iTTlC9ZlfufD3n4YYMXa21ZvNp5A9zx/2KCs=; b=Q87+jaaSwnKeZgAfctrG60MfXdU34xs0Gwz+pOBeRF6FaQZqqbbFlZKcHQMVMxUGpl AGiI6ST58NAeyID80MXa+vIQ9UM01BfCRazWUaPJ7sxgC/fU4R+LkK/Gw5ADdBT9mTdI 2arnn3PJek6qh8DdsaZXmpv2BrGLPVS6vDsvafkL/rY3CDB7IZdMLax1u488GUa9C3ZM C2tTEq8UdRVghc210f17xJIGiCNuadFBsWBP3sFAmU9ljD2lc2UGt6Vrp6DXsBVZf1zz uM2aiSWsV6hZXUVAHW0kdwHA3xIu+vB6EMok8EukyfhWl12whZdv8sldaikks65uCLk2 oZQQ== X-Gm-Message-State: ALoCoQkIX0lWb/P0BkEdq2vj3OmgHutfJrdVQSKaJMJ9Vbh/h0JofxHlKRm2O8WEppRS92bKA9I6 X-Received: by 10.180.9.7 with SMTP id v7mr7493098wia.60.1437057969165; Thu, 16 Jul 2015 07:46:09 -0700 (PDT) Received: from phenom.ffwll.local (212-51-149-109.fiber7.init7.net. [212.51.149.109]) by smtp.gmail.com with ESMTPSA id j7sm13474300wjz.11.2015.07.16.07.46.07 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 16 Jul 2015 07:46:08 -0700 (PDT) From: Daniel Vetter To: DRI Development , Intel Graphics Development Date: Thu, 16 Jul 2015 16:48:47 +0200 Message-Id: <1437058127-4203-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.1.4 Cc: Daniel Vetter , Daniel Vetter Subject: [Intel-gfx] [PATCH] drm/fbdev: Return -EBUSY when oopsing 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-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,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 Trying to do anything with kms drivers when oopsing has become a failing proposition. But since we can end up in the fbdev code simply due to the console unblanking that's done unconditionally just removing our panic handler isn't enough. We need to block all fbdev callbacks when oopsing. There was already one in the blank handler, but it failed silently. That makes it impossible for drivers (like i915) who subclass these functions to figure this out. Instead consistently return -EBUSY so that everyone knows that we really don't want to be bothered right now. This also allows us to remove a pile of FIXMEs from the i915 fbdev code (since due to the failure code they now won't attempt to grab dangerous locks any more). Cc: Dave Airlie Cc: Rodrigo Vivi Signed-off-by: Daniel Vetter Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) --- drivers/gpu/drm/drm_fb_helper.c | 24 ++++++++++++------------ drivers/gpu/drm/i915/intel_fbdev.c | 21 --------------------- 2 files changed, 12 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 329d08167b77..22056d0807bb 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -509,14 +509,6 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode) int i, j; /* - * fbdev->blank can be called from irq context in case of a panic. - * Since we already have our own special panic handler which will - * restore the fbdev console mode completely, just bail out early. - */ - if (oops_in_progress) - return; - - /* * For each CRTC in this fb, turn the connectors on/off. */ drm_modeset_lock_all(dev); @@ -549,6 +541,9 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode) */ int drm_fb_helper_blank(int blank, struct fb_info *info) { + if (oops_in_progress) + return -EBUSY; + switch (blank) { /* Display: On; HSync: On, VSync: On */ case FB_BLANK_UNBLANK: @@ -776,9 +771,10 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info) int i, j, rc = 0; int start; - if (__drm_modeset_lock_all(dev, !!oops_in_progress)) { + if (oops_in_progress) return -EBUSY; - } + + drm_modeset_lock_all(dev); if (!drm_fb_helper_is_bound(fb_helper)) { drm_modeset_unlock_all(dev); return -EBUSY; @@ -927,6 +923,9 @@ int drm_fb_helper_set_par(struct fb_info *info) struct drm_fb_helper *fb_helper = info->par; struct fb_var_screeninfo *var = &info->var; + if (oops_in_progress) + return -EBUSY; + if (var->pixclock != 0) { DRM_ERROR("PIXEL CLOCK SET\n"); return -EINVAL; @@ -952,9 +951,10 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var, int ret = 0; int i; - if (__drm_modeset_lock_all(dev, !!oops_in_progress)) { + if (oops_in_progress) return -EBUSY; - } + + drm_modeset_lock_all(dev); if (!drm_fb_helper_is_bound(fb_helper)) { drm_modeset_unlock_all(dev); return -EBUSY; diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index eef54feb7545..b763f24e20ef 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -55,13 +55,6 @@ static int intel_fbdev_set_par(struct fb_info *info) ret = drm_fb_helper_set_par(info); if (ret == 0) { - /* - * FIXME: fbdev presumes that all callbacks also work from - * atomic contexts and relies on that for emergency oops - * printing. KMS totally doesn't do that and the locking here is - * by far not the only place this goes wrong. Ignore this for - * now until we solve this for real. - */ mutex_lock(&fb_helper->dev->struct_mutex); intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); mutex_unlock(&fb_helper->dev->struct_mutex); @@ -80,13 +73,6 @@ static int intel_fbdev_blank(int blank, struct fb_info *info) ret = drm_fb_helper_blank(blank, info); if (ret == 0) { - /* - * FIXME: fbdev presumes that all callbacks also work from - * atomic contexts and relies on that for emergency oops - * printing. KMS totally doesn't do that and the locking here is - * by far not the only place this goes wrong. Ignore this for - * now until we solve this for real. - */ mutex_lock(&fb_helper->dev->struct_mutex); intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); mutex_unlock(&fb_helper->dev->struct_mutex); @@ -106,13 +92,6 @@ static int intel_fbdev_pan_display(struct fb_var_screeninfo *var, ret = drm_fb_helper_pan_display(var, info); if (ret == 0) { - /* - * FIXME: fbdev presumes that all callbacks also work from - * atomic contexts and relies on that for emergency oops - * printing. KMS totally doesn't do that and the locking here is - * by far not the only place this goes wrong. Ignore this for - * now until we solve this for real. - */ mutex_lock(&fb_helper->dev->struct_mutex); intel_fb_obj_invalidate(ifbdev->fb->obj, ORIGIN_GTT); mutex_unlock(&fb_helper->dev->struct_mutex);