From patchwork Wed Apr 16 14:45:21 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 4002161 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 1B60DC0DA2 for ; Wed, 16 Apr 2014 14:45:36 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D34562035D for ; Wed, 16 Apr 2014 14:45:34 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 2249C202B8 for ; Wed, 16 Apr 2014 14:45:32 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 79A796E253; Wed, 16 Apr 2014 07:45:31 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ee0-f41.google.com (mail-ee0-f41.google.com [74.125.83.41]) by gabe.freedesktop.org (Postfix) with ESMTP id DE5666E253 for ; Wed, 16 Apr 2014 07:45:30 -0700 (PDT) Received: by mail-ee0-f41.google.com with SMTP id t10so9018835eei.0 for ; Wed, 16 Apr 2014 07:45:28 -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=J+JqdPZLMrwuRE8/ZdOtxZkdVGlff9KWE/PhBNY5HTE=; b=EHimbwD1l0dKn6v9leK5TPxgcGUdXMEl6dUkERI4AwkqiskxTZ/avsG8EvbYwLBLCQ tAbupYQGMALpAzavLl0+2XB+s9nT4Cs864Lh0sxk4pzOV+2esMlSYm1wYkPpHuygWZVe jpcvv32DW6DEMlU4gwyiifI1WtTTOHptwZyc0= 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=J+JqdPZLMrwuRE8/ZdOtxZkdVGlff9KWE/PhBNY5HTE=; b=g7k1tsBWwq/ouwlvnbTL3PLsX7Lkk7Gbn4DCqMSO+PJxjZBNG4troLrY4bf8evhxn8 dWvWBRS7QOa5yfBnx9SkqSHZ8iS1iJU9wVVTd0f1KgEyzwpKIPi9nTwwqK6UY1JjL0U8 vW9IjYXRNIE0F8tCibdn6w5ys+YRgoA9JpeqFYRIQ5Xw+G5wD2LRpDpYqX5f6kRXCd1/ qFZPAqq7mBNgYg+uixIzSE+lchJvR+BIc2Rdpvd6moapF7lQ8IGyu9rdHSGbJvktd198 316d3WU5asMrnnlvyKUt8Cms5OF6zNzvQPsWCxjIRXit2wt8aqCaOSKmCDuAuw/5DiAC x2Zg== X-Gm-Message-State: ALoCoQl98yXvroHoZAkP2UmITRnaL8TnjnSzMgiai1JoqrnhyQNFNsxIDRwugHO7IfYccDSFi9I1 X-Received: by 10.14.4.201 with SMTP id 49mr5753718eej.13.1397659528470; Wed, 16 Apr 2014 07:45:28 -0700 (PDT) Received: from phenom.ffwll.local (84-73-67-144.dclient.hispeed.ch. [84.73.67.144]) by mx.google.com with ESMTPSA id 4sm58591948eeq.33.2014.04.16.07.45.26 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 16 Apr 2014 07:45:27 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development , DRI Development Date: Wed, 16 Apr 2014 16:45:21 +0200 Message-Id: <1397659521-32027-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.9.2 Cc: Daniel Vetter , Thierry Reding Subject: [Intel-gfx] [PATCH] drm/fb-helper: Fix hpd vs. initial config races X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.15 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=-4.7 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 Some drivers need to be able to have a perfect race-free fbcon setup. Current drivers only enable hotplug processing after the call to drm_fb_helper_initial_config which leaves a tiny but important race. This race is especially noticable on embedded platforms where the driver itself enables the voltage for the hdmi output, since only then will monitors (after a bit of delay, as usual) respond by asserting the hpd pin. Most of the infrastructure is already there with the split-out drm_fb_helper_init. And drm_fb_helper_initial_config already has all the required locking to handle concurrent hpd events since commit 53f1904bced78d7c00f5d874c662ec3ac85d0f9f Author: Daniel Vetter Date: Thu Mar 20 14:26:35 2014 +0100 drm/fb-helper: improve drm_fb_helper_initial_config locking The only missing bit is making drm_fb_helper_hotplug_event save against concurrent calls of drm_fb_helper_initial_config. The only unprotected bit is the check for fb_helper->fb. With that drivers can first initialize the fb helper, then enabel hotplug processing and then set up the initial config all in a completely race-free manner. Update kerneldoc and convert i915 as a proof of concept. Feature requested by Thierry since his tegra driver atm reliably boots slowly enough to misses the hotplug event for an external hdmi screen, but also reliably boots to quickly for the hpd pin to be asserted when the fb helper calls into the hdmi ->detect function. Cc: Thierry Reding Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_fb_helper.c | 11 +++++------ drivers/gpu/drm/i915/i915_dma.c | 3 --- drivers/gpu/drm/i915/i915_drv.c | 2 -- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/i915_irq.c | 4 ---- 5 files changed, 5 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 04d3fd3658f3..9b775ddb8697 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1578,8 +1578,10 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config); * either the output polling work or a work item launched from the driver's * hotplug interrupt). * - * Note that the driver must ensure that this is only called _after_ the fb has - * been fully set up, i.e. after the call to drm_fb_helper_initial_config. + * Note that drivers may call this even before calling + * drm_fb_helper_initial_config but only aftert drm_fb_helper_init. This allows + * for a race-free fbcon setup and will make sure that the fbdev emulation will + * not miss any hotplug events. * * RETURNS: * 0 on success and a non-zero error code otherwise. @@ -1589,11 +1591,8 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) struct drm_device *dev = fb_helper->dev; u32 max_width, max_height; - if (!fb_helper->fb) - return 0; - mutex_lock(&fb_helper->dev->mode_config.mutex); - if (!drm_fb_helper_is_bound(fb_helper)) { + if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) { fb_helper->delayed_hotplug = true; mutex_unlock(&fb_helper->dev->mode_config.mutex); return 0; diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 96177eec0a0e..8254056f22d8 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1369,9 +1369,6 @@ static int i915_load_modeset_init(struct drm_device *dev) */ intel_fbdev_initial_config(dev); - /* Only enable hotplug handling once the fbdev is fully set up. */ - dev_priv->enable_hotplug_processing = true; - drm_kms_helper_poll_init(dev); return 0; diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 82f4d1f47d3b..aba9552e95e2 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -448,7 +448,6 @@ static int i915_drm_freeze(struct drm_device *dev) cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work); drm_irq_uninstall(dev); - dev_priv->enable_hotplug_processing = false; /* * Disable CRTCs directly since we want to preserve sw state * for _thaw. @@ -589,7 +588,6 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) * notifications. * */ intel_hpd_init(dev); - dev_priv->enable_hotplug_processing = true; /* Config may have changed between suspend and resume */ intel_resume_hotplug(dev); } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 0905cd915589..646a1f9a0db9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1469,7 +1469,6 @@ typedef struct drm_i915_private { u32 pipestat_irq_mask[I915_MAX_PIPES]; struct work_struct hotplug_work; - bool enable_hotplug_processing; struct { unsigned long hpd_last_jiffies; int hpd_cnt; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 7753249b3a95..9e7c35a33c83 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -971,10 +971,6 @@ static void i915_hotplug_work_func(struct work_struct *work) bool changed = false; u32 hpd_event_bits; - /* HPD irq before everything is fully set up. */ - if (!dev_priv->enable_hotplug_processing) - return; - mutex_lock(&mode_config->mutex); DRM_DEBUG_KMS("running encoder hotplug functions\n");