From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.12]) (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 AAD9C28F5 for ; Fri, 17 Apr 2026 15:19:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.12 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776439198; cv=none; b=BO5HJblnabxs2L9Vy7QNJ8CdviHpx5qlXlhyA9joL9jRN8D/A4FWzgESZMcVlwgFTjI1fyL3H7POV5Vn99dv9vhQ73ltxb8QQxqJrEIRs9Cx0pSNDtsmZyadjMLnVFucaVFzE8phe8PP9Z7vYRbIa3n+kD8kYdqk51NOKkgcmbg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776439198; c=relaxed/simple; bh=8Ribu0mx6B+vvE2Mwq99HqyjC8jT4TmqC1ck/RrBEdI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=PbXjAkNIP9btbbMxXx3Mm7/f8tWvNjB/5PV/89zOnLaqtQnObX1Uv/vhdqYvLnrDvQTeNumAXx3TFEafDV0AfvgtG64/9GPLn66bc/aYByj269dnKI/YJFr6t7/Kq5yTdE+PLoB2dxgCSalDDX4XmC0cT3m7jTOu45BXlZoQdyw= 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=hvq48VEJ; arc=none smtp.client-ip=192.198.163.12 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="hvq48VEJ" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1776439197; x=1807975197; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=8Ribu0mx6B+vvE2Mwq99HqyjC8jT4TmqC1ck/RrBEdI=; b=hvq48VEJiMY8uQo8fn+nB2opXpOGPKYNOElGF7+4lqZrnYPKNegr9PYX 9fQ+KxDMxGFFttFi6kbNidNZgs25AVV4SpNDPRwKW8H1qzHh0ucXL0mn+ fTTvK4begU+KDlr1S4HPExdJ/aZYPJMhzCj9DY5t/ED2Yl46xDt6/dq8R KGG69BU5XD5Ndwrp/6fMxJ7FNf4RbecCpdaI2OEixwuG0wq01agiHa8TK OPy96wp/mv9nq6QCFg+GQ8AP+UWmrkxfr/+0GW8llItt/2Uw6Ui2mDd9+ /12cjm/KPtMh4pqNKxqoaUQpo4aCgPcZQBmJLbNBcomaZ0DnVMUzmpCZ5 w==; X-CSE-ConnectionGUID: gjpUtPifT7OM6YjpcHqgdw== X-CSE-MsgGUID: f+RVU0oyR6WZsS6B2ao3eg== X-IronPort-AV: E=McAfee;i="6800,10657,11762"; a="81334680" X-IronPort-AV: E=Sophos;i="6.23,184,1770624000"; d="scan'208";a="81334680" Received: from orviesa005.jf.intel.com ([10.64.159.145]) by fmvoesa106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Apr 2026 08:19:56 -0700 X-CSE-ConnectionGUID: baOP0N0GTLinZDFi7GOVSg== X-CSE-MsgGUID: vM7knwh8StaUKq0W+Jjemg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.23,184,1770624000"; d="scan'208";a="236043688" Received: from zzombora-mobl1 (HELO localhost) ([10.245.245.176]) by orviesa005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Apr 2026 08:19:50 -0700 Date: Fri, 17 Apr 2026 18:19:47 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Nicolas Frattaroli Cc: Michel =?iso-8859-1?Q?D=E4nzer?= , Julian Orth , 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 , Imre Deak Subject: Re: [PATCH RESEND v7 0/2] Pass down hot-plug CONNECTOR ID to user-space Message-ID: References: <20260415-hot-plug-passup-v7-0-9a27ef5e2428@collabora.com> <6501274.LvFx2qVVIh@workhorse> <5186396.44csPzL39Z@workhorse> 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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <5186396.44csPzL39Z@workhorse> 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 Fri, Apr 17, 2026 at 05:00:10PM +0200, Nicolas Frattaroli wrote: > On Friday, 17 April 2026 16:16:01 Central European Summer Time Ville Syrjälä wrote: > > On Fri, Apr 17, 2026 at 02:42:36PM +0200, Nicolas Frattaroli wrote: > > > On Friday, 17 April 2026 12:57:58 Central European Summer Time Ville Syrjälä wrote: > > > > On Fri, Apr 17, 2026 at 09:49:36AM +0200, Michel Dänzer wrote: > > > > > On 4/16/26 15:16, Julian Orth wrote: > > > > > > On Thu, Apr 16, 2026 at 9:46 AM Nicolas Frattaroli > > > > > > wrote: > > > > > >> On Wednesday, 15 April 2026 20:57:53 Central European Summer Time Julian Orth wrote: > > > > > >>> On Wed, Apr 15, 2026 at 8:19 PM Nicolas Frattaroli > > > > > >>> wrote: > > > > > >>>> > > > > > >>>> This series addresses a shortcoming whereby a hot plug event is sent > > > > > >>>> without it being passed the actual connector that caused it. This takes > > > > > >>>> into consideration both the polling path and the HPD (Hot Plug Detect) > > > > > >>>> path. It also adds support for the vkms driver (using ConfigFS) for > > > > > >>>> propagating the connector ID when changing the connector's status. > > > > > >>>> > > > > > >>>> The motivation is that user-space applications such as Weston would > > > > > >>>> previously receive non-connector-specific hotplug events, and then have > > > > > >>>> to figure out themselves which connector needs to have a modeset > > > > > >>>> executed on. This notably did not work when the hotplug events came in > > > > > >>>> too fast, resulting in Weston missing an on-off-on transition of a > > > > > >>>> connector, seeing that its state was unchanged from "on" so can't be the > > > > > >>>> one that was hotplugged, and skipping reinitialising it as it looks > > > > > >>>> through the other connectors that could've caused it. > > > > > >>> > > > > > >>> Have you considered adding a u64 serial number as a DRM connector > > > > > >>> property that is incremented every time the connector changes in some > > > > > >>> way? Userspace could then check this serial number to see if the > > > > > >>> connector has changed since the last time it queried the serial > > > > > >>> number. > > > > > >> > > > > > >> The connector internally already has an epoch_counter member which > > > > > >> could be used for this. However, for the particular thing this > > > > > >> series fixes, I don't think exposing it through the uAPI is necessary > > > > > >> or desirable. Sending hotplug events specific to the connector does > > > > > >> not need any additional handling on the userspace side as long as it > > > > > >> already listens to the per-connector hotplug events in order to > > > > > >> avoid the pitfall described in the cover letter. > > > > > > > > > > > > I currently do not handle per-connector hotplug events. Instead, > > > > > > whenever I get a UDEV change event for a device, I re-fetch the entire > > > > > > kernel state for the device. That is > > > > > > > > > > > > - DRM_IOCTL_MODE_GETRESOURCES > > > > > > - DRM_IOCTL_MODE_OBJ_GETPROPERTIES for each connector, crtc, plane > > > > > > - DRM_IOCTL_MODE_GETCONNECTOR for each connector > > > > > > - DRM_IOCTL_MODE_GETPROPERTY for each connector property > > > > > > - DRM_IOCTL_MODE_GETPROPBLOB for the EDID > > > > > > > > > > > > Once I have the new state, I compare it against the desired compositor > > > > > > state and perform a modeset if necessary. > > > > > > > > > > mutter is doing something similar as well. > > > > > > > > > > > > > > > Note that some are arguing a modeset is always required after a hotplug event, even if the state hasn't changed. > > > > > > > > > > The most convincing argument I've seen is the scenario of a GPU reset, after which a modeset is required to light up the displays again. > > > > > > > > GPU reset should relight the display on its own really. That's what > > > > i915 does, albeit somewhat badly at the moment. > > > > > > > > > A hotplug event seems the only mechanism available for the kernel to request a modeset from the compositor. (The kernel may not be able to reliably do the modeset on its own, e.g. due to interactions with user-space atomic commits) > > > > > > > > There's nothing preventing the kernel from doing extra atomic > > > > commits whenever it wants. But if you want to punt the thing to > > > > userspace then the kernel must set the link-status property to > > > > bad, and then fire the hotplug uevent. > > > > > > > > > If this "modeset required after hotplug event" rule is confirmed, it means that after a hotplug event without connector ID, the compositor must do a modeset for all connectors. > > > > > > > > Only for connectors where things changed, or the link-status shows bad. > > > > > > > > > > > > > > > > > > > P.S. https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/14420#note_2984697 even argued that two modesets are required after a hotplug event, one which turns things off and another one which turns them on again. I don't agree with that though, a single modeset should suffice. > > > > > > > > The actual argument is that you should not defer the hotplug > > > > handling when things get disconnected, mainly because of crap type-c > > > > firmware. > > > > > > > > I think the userspace behaviour there was that you get a disconnected, > > > > defer processing it, and then you get a reconnect, and then decide that > > > > nothing actually changed and a modeset is not needed after all. That is > > > > not correct IMO. Clearly a ->disconnect->reconnect should count as a > > > > change in the connector's state, and a full modeset is thus required. > > > > The kernel can of course then decide that the full modeset is not > > > > actually required and skip the modeset part during the commit. But > > > > userspace cannot make that determination. > > > > > > So just to loop around to the patches here: is sending per-connector > > > hotplug events not acceptable? Your review[1] on v5 indicated you had > > > a problem with the implementation, not a fundamental issue with the > > > behaviour the patch tried to change. > > > > What I was saying is that we already have the epoch_counter. If > > there are gaps in the implementation then those should just be > > fixed. We don't want a second implementation of the same > > mechanism. > > epoch_counter is explicitly documented as "used to detect any other changes > in connector, besides status". That's just some nonsense that got in by accident. It was always meant to be a "did anything for this connector change?" type of thing. Looks like the bogus docs were part of the original commit, but it even disagrees with the commit message itself. > pending_hp is changed if the status changes. > > If we use epoch_counter for this, I think we'll need to keep track of the > last epoch_counter an hpd was sent for each connector, so we wouldn't win > anything in reduced state being tracked. What's wrong with just looking at the epoch before and after the detect? > > > > > > > > I suppose what we could maybe do there to force userspace's hand is set > > > > the link-status to bad already when the thing gets disconnected, and keep > > > > it like that until a full disable+re-enable cycle has been done. Then > > > > userspace could not think that the ->disconnect->reconnect is a NOP > > > > (which I still think is incorrect behaviour). > > > > > > The documentation of drm_connector_set_link_status_property explicitly > > > states: > > > > > > * Note: Drivers cannot rely on userspace to support this property and > > > * issue a modeset. As such, they may choose to handle issues (like > > > * re-training a link) without userspace's intervention. > > > > > > which I think conflicts with your suggestion. > > > > In i915 we do the retraining in kernel. IIRC we only really use the bad > > link stuff once retraining has failed hard enough that userspace > > intervention is needed, ie. when we cannot reduce the link parameters > > anymore without userspace selecting a mode with a lower resolution. > > > > I think some more recent drivers may have opted to forego all in kernel > > retraining and just punt it all to userspace. I think the link-status > > property has existed for long enough that any userspace that doesn't > > support it could simply be considered broken. > > Is breaking userspace a better choice than sending per-connector hotplug > events rather than a single global hotplug event? I get where you're > coming from, in that userspace should not be trying to optimise away > modesets, as that's something the kernel can do in a race-free way. I'm > just not familiar enough with the landscape to know whether setting > link-status to bad will actually result in userspace doing a modeset, > and whether drivers in practice then optimise that modeset away if it > wasn't needed. I'm not suggesting using link-status for normal hotplug events where something on the connector actually changed. In those cases userspace should really be doing a full modeset anyway. > > > Actually I know of a single "userspace" that to my knowledge doesn't > > implement the link-status stuff, and that is the in kernel fb helper. > > That should probably be remedied... > > Kind regards, > Nicolas Frattaroli > -- Ville Syrjälä Intel