From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.19]) (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 27327220F49 for ; Wed, 22 Apr 2026 14:30:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.19 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776868233; cv=none; b=VtE5qDOnQoJcIqQJ/s7Y1IAhfxu2at1sWB6z+r5Q3JIuGeTAhzNm0bzNY9uKD862GA+4cUmTk1AjNAMOCUgzGIHAPgtl8Kf9l54O3TCRb9OIQLEp3EdTj64sugqCyIA4J1dW9UnnZYPecqdLE6uooidPn77bQY8oxiL+mExsSuI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776868233; c=relaxed/simple; bh=ot8IubjSE5tltmob1IdZqcqTI7IlrxYETzLabAL9PHA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=PsqLGnKNOGXHS7xruRmV9TYiFJBxW0xSfWxGbbsx2yxRyZZmwAHQnxa4Beu9I7xdG7nNehR/G6/h0Cr/j7+Es/10DpvFpnhniNnXwKZ/gFwuymj/1u/WnZMo3WZeSWVSJsD3PTonJ8g/ITvrKtWivPy/xMKum20HOhI82eG6BKw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=AnhAyeV8; arc=none smtp.client-ip=192.198.163.19 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="AnhAyeV8" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1776868229; x=1808404229; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=ot8IubjSE5tltmob1IdZqcqTI7IlrxYETzLabAL9PHA=; b=AnhAyeV884sA9VASCi6TDfwhxOcOgtolCLAtntYMjWAlfWsNoWbNTRpr Jmk2d7ggqmUBWb7TuyYhoW6qgwxf0v8DsBaNVo53juegCZ+c4H8mDY8GF t7dx8kwmYOaDnuXbpM1Jw3D8KZ/mQ0Q+tYS2OyuXhp7g4NRKQdCNNsDnz fDR38ymcvWpL1rE+JR4no1OT9T8lqb6tRhmSDL55nRBf05KKMpnOWAdMB WgGs/3bvzGpyfo31c3mO1Xie3SMnHqzC76KYkOgBeA/H2mdwhjS6NBTs6 XLxpl0SVTveVVPPeApsBF7D9uZwOmBOpu24zr9s4FJh7FQ6x06vLxfrhM A==; X-CSE-ConnectionGUID: r2YdAI5gTCOUZ9AIO0LjEQ== X-CSE-MsgGUID: juX27LjiQTmhNWw9q9wPgg== X-IronPort-AV: E=McAfee;i="6800,10657,11764"; a="76853909" X-IronPort-AV: E=Sophos;i="6.23,193,1770624000"; d="scan'208";a="76853909" Received: from orviesa002.jf.intel.com ([10.64.159.142]) by fmvoesa113.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Apr 2026 07:30:28 -0700 X-CSE-ConnectionGUID: ND13U+ZtTSCQV3yh32gcvA== X-CSE-MsgGUID: VbHufv7ARGWGfAxE2JATfw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.23,193,1770624000"; d="scan'208";a="262757725" Received: from pgcooper-mobl3.ger.corp.intel.com (HELO localhost) ([10.245.244.10]) by orviesa002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Apr 2026 07:30:23 -0700 Date: Wed, 22 Apr 2026 17:30:19 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Nicolas Frattaroli Cc: Stanislav Lisovskiy , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Louis Chauvet , Haneen Mohammed , Melissa Wen , Daniel Stone , Ian Forbes , Dmitry Baryshkov , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, kernel@collabora.com, wayland-devel@lists.freedesktop.org, Marius Vlad Subject: Re: [PATCH v8 2/2] drm: Send per-connector hotplug events Message-ID: References: <20260422-hot-plug-passup-v8-0-5cfae6ba4119@collabora.com> <20260422-hot-plug-passup-v8-2-5cfae6ba4119@collabora.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260422-hot-plug-passup-v8-2-5cfae6ba4119@collabora.com> X-Patchwork-Hint: comment Organization: Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs Bertel Jungin Aukio 5, 02600 Espoo, Finland On Wed, Apr 22, 2026 at 02:35:32PM +0200, Nicolas Frattaroli wrote: > Try to send per-connector hotplug events as often as possible, rather > than connector-less global hotplug events. This does result in more > hotplug events if multiple connectors changed at the same time, but > give userspace more actionable information. > > Since the hotplug event needs to be sent outside of the mode_config > mutex to avoid a deadlock, pointers to all the changed connectors are > stored in a newly allocated array. > > The "changed" boolean in output_poll_execute now only serves to signal > that a non-connector-specific hotplug event needs to be sent from a > prior event that was delayed. So, rename it from "changed" to > "delayed_hp" to avoid any confusion. > > Co-developed-by: Marius Vlad > Signed-off-by: Marius Vlad > Signed-off-by: Nicolas Frattaroli > --- > drivers/gpu/drm/drm_probe_helper.c | 68 ++++++++++++++++++++++++-------------- > 1 file changed, 43 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index d4dc8cb45bce..3beed8aa32fe 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -757,17 +757,19 @@ static void output_poll_execute(struct work_struct *work) > { > struct delayed_work *delayed_work = to_delayed_work(work); > struct drm_device *dev = container_of(delayed_work, struct drm_device, mode_config.output_poll_work); > + struct drm_connector **changed_conns; > struct drm_connector *connector; > struct drm_connector_list_iter conn_iter; > enum drm_connector_status old_status; > - bool repoll = false, changed; > + unsigned int num_changed = 0, i; > + bool repoll = false, delayed_hp; > 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; > + delayed_hp = dev->mode_config.delayed_event; > dev->mode_config.delayed_event = false; > > if (!drm_kms_helper_poll) { > @@ -783,6 +785,13 @@ static void output_poll_execute(struct work_struct *work) > goto out; > } > > + changed_conns = kmalloc_array(dev->mode_config.num_connector, > + sizeof(*changed_conns), GFP_KERNEL); > + if (!changed_conns) { > + repoll = true; > + goto out; > + } > + > drm_connector_list_iter_begin(dev, &conn_iter); > drm_for_each_connector_iter(connector, &conn_iter) { > /* Ignore forced connectors. */ > @@ -836,15 +845,23 @@ static void output_poll_execute(struct work_struct *work) > connector->base.id, connector->name, > old_epoch_counter, connector->epoch_counter); > > - changed = true; > + drm_connector_get(connector); > + changed_conns[num_changed++] = connector; > } > } > drm_connector_list_iter_end(&conn_iter); > > mutex_unlock(&dev->mode_config.mutex); > > + for (i = 0; i < num_changed; i++) { > + drm_kms_helper_connector_hotplug_event(changed_conns[i]); > + drm_connector_put(changed_conns[i]); > + } > + > + kfree(changed_conns); > + > out: > - if (changed) > + if (delayed_hp) > drm_kms_helper_hotplug_event(dev); > > if (repoll) > @@ -1081,39 +1098,40 @@ EXPORT_SYMBOL(drm_connector_helper_hpd_irq_event); > */ > bool drm_helper_hpd_irq_event(struct drm_device *dev) > { > - struct drm_connector *connector, *first_changed_connector = NULL; > struct drm_connector_list_iter conn_iter; > - int changed = 0; > + struct drm_connector **changed_conns; > + struct drm_connector *connector; > + unsigned int changed = 0, i; > > if (!dev->mode_config.poll_enabled) > return false; > > - mutex_lock(&dev->mode_config.mutex); > - drm_connector_list_iter_begin(dev, &conn_iter); > - drm_for_each_connector_iter(connector, &conn_iter) { > - /* Only handle HPD capable connectors. */ > - if (!(connector->polled & DRM_CONNECTOR_POLL_HPD)) > - continue; > + scoped_guard(mutex, &dev->mode_config.mutex) { > + changed_conns = kmalloc_array(dev->mode_config.num_connector, > + sizeof(*changed_conns), GFP_KERNEL); > + if (!changed_conns) > + return false; I'm thinking you could avoid all this kmalloc_array() complication by doing the drm_sysfs_connector_hotplug_event() calls directly from the loop, and only defer the drm_client_dev_hotplug() to the end (to avoid locking issues). And ideally I think drm_client_dev_hotplug() should even queue its own work and do things asynchronously, but that's a much bigger change and would need more thought. > > - if (check_connector_changed(connector)) { > - if (!first_changed_connector) { > + drm_connector_list_iter_begin(dev, &conn_iter); > + drm_for_each_connector_iter(connector, &conn_iter) { > + /* Only handle HPD capable connectors. */ > + if (!(connector->polled & DRM_CONNECTOR_POLL_HPD)) > + continue; > + > + if (check_connector_changed(connector)) { > drm_connector_get(connector); > - first_changed_connector = connector; > + changed_conns[changed++] = connector; > } > - > - changed++; > } > + drm_connector_list_iter_end(&conn_iter); > } > - drm_connector_list_iter_end(&conn_iter); > - mutex_unlock(&dev->mode_config.mutex); > > - if (changed == 1) > - drm_kms_helper_connector_hotplug_event(first_changed_connector); > - else if (changed > 0) > - drm_kms_helper_hotplug_event(dev); > + for (i = 0; i < changed; i++) { > + drm_kms_helper_connector_hotplug_event(changed_conns[i]); > + drm_connector_put(changed_conns[i]); > + } > > - if (first_changed_connector) > - drm_connector_put(first_changed_connector); > + kfree(changed_conns); > > return changed; > } > > -- > 2.53.0 -- Ville Syrjälä Intel