From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from sender4-pp-f112.zoho.com (sender4-pp-f112.zoho.com [136.143.188.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 01EBE38B122 for ; Fri, 22 May 2026 13:06:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=pass smtp.client-ip=136.143.188.112 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779455178; cv=pass; b=eVeA8csM5A2UoNcT81LcApRbiH7+G04cyOjs4GnUc9DtXdbdAsNLyAAwogHv4tvNsGrduS2/2NclRsz/WJRc07aD/CtbALr+bu1xAT0eZ5ALDrBXbu97dmTmAPuJv5XFtPoGqh6Z3HluksiMc2RsS0GDtzgpDGMchkMIgsyznFg= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779455178; c=relaxed/simple; bh=GeAjj104LYPPBOzMJ9AywXPPhaLqVfcuMiUBKmAobZA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=FIC3m3YeiyisR160Yv9KwuGcVuhAsvyd+Fr6j+W9+YkA0tc63e0lA0UtgtRriekzwkUy1Ruiu3VT9IZZFirFDpC+VYSj6BlrDthBpaQxGZvqHHh2259PY2WkCGk30ZVQZKU7KFwYsSW2A/RGwhjdf4UErGgra+gBnNszl3S//Yo= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (1024-bit key) header.d=collabora.com header.i=nicolas.frattaroli@collabora.com header.b=YjjKTIHe; arc=pass smtp.client-ip=136.143.188.112 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=collabora.com header.i=nicolas.frattaroli@collabora.com header.b="YjjKTIHe" ARC-Seal: i=1; a=rsa-sha256; t=1779455150; cv=none; d=zohomail.com; s=zohoarc; b=IehLlswx/AfitDB+3716t3XSL/LlQmsZbSjm6lP57PTWJ7DzWXqMkT+97KHsNarAel7BylaujhVPFBbAe2iFrHg0HbMA86Vc1duUt2PRLOxOc0b+rWutE5vT4fwQ7I4LpNdsSCfeWOEzuon4hRd3PQJJ4Lt1sWc0fvc18YNlxL8= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1779455150; h=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=ywrhEfK4rP1qhOtmTxkDuwQOzsmmGDJqWoXLNIRX8Io=; b=lvqlJNFagMnp79CLLIfvw6ULFUaQRHhlnITWtb/sa0TGKsFMIsNvPkLG8T1/92p0E3GdGyuXMIBeP71yiu2r3czAW7UVvfsIKxPNq+D+CSSlzpJ9filFj/Fm3XnIQxxDb3fcbtCXKblokTrjSm76I+QcSDmtFr9ErRbux9urd6I= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=collabora.com; spf=pass smtp.mailfrom=nicolas.frattaroli@collabora.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1779455150; s=zohomail; d=collabora.com; i=nicolas.frattaroli@collabora.com; h=From:From:To:To:Cc:Cc:Subject:Subject:Date:Date:Message-ID:In-Reply-To:References:MIME-Version:Content-Transfer-Encoding:Content-Type:Message-Id:Reply-To; bh=ywrhEfK4rP1qhOtmTxkDuwQOzsmmGDJqWoXLNIRX8Io=; b=YjjKTIHeuU1pZ1/N7uhtKgFlXd8ojTbOnWqIr8k0xocivw1qd+QQW9NWS7NKbWWK 5QWaIbfg8r+M5CkBHBZakRmXM4E8aexzArMnjZt4Bg/WlsrPrcwBhw16+KyPp7Vwpei n7YdnsakHlLEDKOTSLFos6mAYfrysHR3UgMoXSSk= Received: by mx.zohomail.com with SMTPS id 1779455149489577.3622477463117; Fri, 22 May 2026 06:05:49 -0700 (PDT) From: Nicolas Frattaroli To: Daniel Stone , Simona Vetter Cc: Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Stanislav Lisovskiy , Ville =?UTF-8?B?U3lyasOkbMOk?= , Louis Chauvet , Haneen Mohammed , Melissa Wen , Daniel Stone , Ian Forbes , Dmitry Baryshkov , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, kernel@collabora.com, wayland-devel@lists.freedesktop.org, Marius Vlad Subject: Re: [PATCH v9 2/2] drm: Send per-connector hotplug events Date: Fri, 22 May 2026 15:05:42 +0200 Message-ID: In-Reply-To: References: <20260422-hot-plug-passup-v9-0-aef804255986@collabora.com> <20260422-hot-plug-passup-v9-2-aef804255986@collabora.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" +To: Simona Vetter, who wrote two of the commits relevant to this question. On Thursday, 21 May 2026 17:00:17 Central European Summer Time Daniel Stone wrote: > Hi, > > On Wed, 22 Apr 2026 at 19:24, Nicolas Frattaroli > wrote: > > @@ -760,16 +760,12 @@ static void output_poll_execute(struct work_struct *work) > > struct drm_connector *connector; > > struct drm_connector_list_iter conn_iter; > > enum drm_connector_status old_status; > > - bool repoll = false, changed; > > + bool repoll = false, changed = false; > > u64 old_epoch_counter; > > > > if (!dev->mode_config.poll_enabled) > > return; > > > > - /* 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) { > > if (dev->mode_config.poll_running) { > > drm_kms_helper_disable_hpd(dev); > > @@ -844,8 +841,15 @@ static void output_poll_execute(struct work_struct *work) > > mutex_unlock(&dev->mode_config.mutex); > > > > out: > > + /* Pick up any changes detected by the probe functions. */ > > + if (dev->mode_config.delayed_event) { > > Not your doing, as it was just the same before, but doesn't this read > need to be protected by the mode_config mutex? It looks like the fuzzy meaning of the mode_config.mutex came to bite here. It is set to true with the lock held in drm_helper_probe_single_connector_modes(), but set to false in this function without the lock, and always read without the lock (drm_kms_helper_poll_enable(), reschedule_output_poll_work()). Some of these calls happen in paths where it might be inconvenient to take a lock this coarse. Specifically, drm_helper_probe_single_connector_modes() has this comment right before setting delayed_event to true: /* * 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. */ So there was a problem with accessing parts under a lock before, so if we're reintroducing the coarse lock wherever the boolean is accessed we might be going backwards here. The relevant commits are: Commit 162b6a57ac50 (drm/probe-helper: don't lose hotplug event, 2015-01-21) Commit ed293f775493 (drm/sysfs: Send out uevent when connector->force changes, 2015-11-19) One obvious move would be to change it to an atomic so at least we punch through any caches, but that doesn't help if the intended design was that it's in a consistent state with the rest of mode_config. Though I don't think that's the case? Kind regards, Nicolas Frattaroli > > The series is very satisfying otherwise; both patches are: > Reviewed-by: Daniel Stone > > Cheers, > Daniel >