From patchwork Wed Jan 21 14:41:00 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 5677471 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.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 9C8D39F2ED for ; Wed, 21 Jan 2015 14:41:13 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A0FE1204D1 for ; Wed, 21 Jan 2015 14:41:12 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 5B971204C9 for ; Wed, 21 Jan 2015 14:41:11 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EEA2F6E19C; Wed, 21 Jan 2015 06:41:09 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-we0-f179.google.com (mail-we0-f179.google.com [74.125.82.179]) by gabe.freedesktop.org (Postfix) with ESMTP id 94D7A6E19C for ; Wed, 21 Jan 2015 06:41:09 -0800 (PST) Received: by mail-we0-f179.google.com with SMTP id q59so19945533wes.10 for ; Wed, 21 Jan 2015 06:41:08 -0800 (PST) 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=4CROCrLew0a4I9oIVZvpL1WN8picwhPHs2sSQNoyWtU=; b=VURAxEhQv8f5ZLZSv5TVEzuQ2pY0gntvMozwphOXwqAMuQf4EUCVFEMVVYXKUWvh0X E4eHtaSEU33XJptiT5de0rTiF0cPL5RKGHnsmMyC/u9GRp1ZlLxEJMejvx2bb14VtoWk fBfVFAGrwnV/FacdpolW6HcMeV9uLqvF5heZE= 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:in-reply-to :references; bh=4CROCrLew0a4I9oIVZvpL1WN8picwhPHs2sSQNoyWtU=; b=clxQobRTJfyKMM7XJ6b1PJncpcFiueyKc4Z6t8tWpcTVGBr9jkst1+jYgF6F+6WYef DeH7PFGFCkyce0355WjMU1kALa7AmkiTxWbKqId8uHGwnkQb4h6GztPea8KWP2EON8YK A2bnUxJhvVBJnA72sNsGaXuUc7FpBI+9Zc2dZcSUY8q/ZwEfP3UDhty046wV/WB4/wiS SSr8P8eBS6jI9l7HpD0vlv7haaYB37IdmHzt8T8og7aLkVTbl0kPBmJL7Y/woTIOjakX d3SU4SYMwd3/4enmR3TFiKym6yRm6QDX2WzvQUX5B4k5HkohB4fL8LpE754JNAuq3gln PaBA== X-Gm-Message-State: ALoCoQlgmkNFAqSzzWerMbJOLodRvrlsp7OmULmZjCrNEOWCCSMdWArcjBvadgs0J/JWmARbre71 X-Received: by 10.194.122.233 with SMTP id lv9mr30026714wjb.95.1421851268587; Wed, 21 Jan 2015 06:41:08 -0800 (PST) Received: from phenom.ffwll.local (84-73-67-144.dclient.hispeed.ch. [84.73.67.144]) by mx.google.com with ESMTPSA id a12sm56050wjs.10.2015.01.21.06.41.06 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 21 Jan 2015 06:41:07 -0800 (PST) From: Daniel Vetter To: DRI Development Subject: [PATCH] drm/probe-helper: don't lose hotplug event Date: Wed, 21 Jan 2015 15:41:00 +0100 Message-Id: <1421851260-23297-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1421826322-7592-1-git-send-email-daniel.vetter@ffwll.ch> References: <1421826322-7592-1-git-send-email-daniel.vetter@ffwll.ch> Cc: Daniel Vetter , Intel Graphics Development X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_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 There's a race window (small for hpd, 10s large for polled outputs) where userspace could sneak in with an unrelated connnector probe ioctl call and eat the hotplug event (since neither the hpd nor the poll code see a state change). To avoid this, check whether the connector state changes in all other ->detect calls (in the current helper code that's only probe_single) and if that's the case, fire off a hotplug event. Note that we can't directly call the hotplug event handler, since that expects that no locks are held (due to reentrancy with the fb code to update the kms console). Also, this requires that drivers using the probe_single helper function set up the poll work. All current drivers do that already, and with the reworked hpd handling there'll be no downside to unconditionally setting up the poll work any more. v2: Review from Rob Clark - Don't bail out of the output poll work immediately if it's disabled to make sure we deliver the delayed hoptplug events. Instead just jump to the tail. - Don't scheduel the work when it's not set up. Would be a driver bug since using probe helpers for anything dynamic without them initialized makes them all noops. Reviewed-by: Chris Wilson (v1) Cc: Rob Clark Signed-off-by: Daniel Vetter Reviewed-by: Rob Clark Tested-by: Rob Clark --- drivers/gpu/drm/drm_probe_helper.c | 36 ++++++++++++++++++++++++++++++++++-- include/drm/drm_crtc.h | 1 + 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 2fbdcca7ca9a..33bf550a1d3f 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -103,6 +103,7 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect int count = 0; int mode_flags = 0; bool verbose_prune = true; + enum drm_connector_status old_status; WARN_ON(!mutex_is_locked(&dev->mode_config.mutex)); @@ -121,7 +122,33 @@ static int drm_helper_probe_single_connector_modes_merge_bits(struct drm_connect if (connector->funcs->force) connector->funcs->force(connector); } else { + old_status = connector->status; + connector->status = connector->funcs->detect(connector, true); + + /* + * Normally either the driver's hpd code or the poll loop should + * pick up any changes and fire the hotplug event. But if + * userspace sneaks in a probe, we might miss a change. Hence + * check here, and if anything changed start the hotplug code. + */ + if (old_status != connector->status) { + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n", + connector->base.id, + connector->name, + old_status, connector->status); + + /* + * The hotplug event code might call into the fb + * helpers, and so expects that we do not hold any + * locks. Fire up the poll struct instead, it will + * disable itself again. + */ + dev->mode_config.delayed_event = true; + if (dev->mode_config.poll_enabled) + schedule_delayed_work(&dev->mode_config.output_poll_work, + 0); + } } /* Re-enable polling in case the global poll config changed. */ @@ -274,10 +301,14 @@ static void output_poll_execute(struct work_struct *work) struct drm_device *dev = container_of(delayed_work, struct drm_device, mode_config.output_poll_work); struct drm_connector *connector; enum drm_connector_status old_status; - bool repoll = false, changed = false; + bool repoll = false, changed; + + /* Pick up any changes detected by the probe functions. */ + changed = dev->mode_config.delayed_event; + dev->mode_config.delayed_event = false; if (!drm_kms_helper_poll) - return; + goto out; mutex_lock(&dev->mode_config.mutex); list_for_each_entry(connector, &dev->mode_config.connector_list, head) { @@ -319,6 +350,7 @@ static void output_poll_execute(struct work_struct *work) mutex_unlock(&dev->mode_config.mutex); +out: if (changed) drm_kms_helper_hotplug_event(dev); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index f444263055c5..65da9fb939a7 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1089,6 +1089,7 @@ struct drm_mode_config { /* output poll support */ bool poll_enabled; bool poll_running; + bool delayed_event; struct delayed_work output_poll_work; /* pointers to standard properties */