From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-02.galae.net (smtpout-02.galae.net [185.246.84.56]) (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 4C8641FA859; Fri, 19 Jun 2026 16:25:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.246.84.56 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781886306; cv=none; b=hjecI/P1C8Arrs4oV2b/zBzZYd0jRspDfu1Y2SZAayRl9tGee2Rp/QmrqdoyynTeIUUZPjuPFCHezuQozSPEzyc0NQlDfNuL5Y7bUu8KMMBfYNUQikMWmGbMrwcAQAgA+7mVIBLolHs3gmupibtEzs/xWLE8+DgkpNqlgSfryyU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781886306; c=relaxed/simple; bh=0R25ISdpc+AAKuQJQAHUFKomgtbaxc7SSl/fsBbkLEs=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From: References:In-Reply-To; b=e+bu73OKR3umS5rETRYgoXyVEa/QNBmL5Y6caQJitpTQ2v6TAYxceb8KSEFQ/UrbKoH+Gfw1eszjVNM2R3GlAzX6AMYGo1v0bi003V/R6X9tBwMZEwcAZa2raeB99vbmjBiYyWjpnx4K8qlQqVFLtKN0xD8hPFekYB9zEcHTdg0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=jY8Egj58; arc=none smtp.client-ip=185.246.84.56 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="jY8Egj58" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-02.galae.net (Postfix) with ESMTPS id A99E91A3A21; Fri, 19 Jun 2026 16:25:01 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id 6D0F4601AD; Fri, 19 Jun 2026 16:25:01 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 08E5B106C81BC; Fri, 19 Jun 2026 18:24:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1781886299; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=Xrx5VoO25eYm5EH6Wc6vNdgK+6vpuoEcmvN4eX6Cvok=; b=jY8Egj58zUWOmsPwj9vynk2NQy9hCh/a+LGJvBDk3DqRfyL+FJQIuyDJQRRohvDhOBXcnW //AvVJmy/V7tylN+21mDLl593BLUfcG3dz090WmD7UcN98JIRVRLQ/m5l7+EC2wkNUXoNu OY3r/fPM8V0Ey5z04hzY+MHXR+FXcsckzsiV8jrezwluFXHT8MZBukmQNizYV+LllFdQFg Nq8mjjsoK8DWc4IXuaLUWlBpL/WCM0pLIK5BmeG/ZousyZJb+/ny8SsYGoB3m1Nbz1HyNz 3bGNW8rAc8osqfVrmerKyh25ch/VkV9XZdD23tG58EnuIwQFtxnCNKSDDWhrYg== Precedence: bulk X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 19 Jun 2026 18:24:46 +0200 Message-Id: Subject: Re: [PATCH v6 15/19] drm/connector: Add new atomic_create_state callback Cc: , , , "Daniel Stone" , , , , , "Laurent Pinchart" To: "Maxime Ripard" , "Maarten Lankhorst" , "Thomas Zimmermann" , "David Airlie" , "Simona Vetter" , "Jonathan Corbet" , "Shuah Khan" , "Dmitry Baryshkov" , "Jyri Sarha" , "Tomi Valkeinen" , "Andrzej Hajda" , "Neil Armstrong" , "Robert Foss" , "Laurent Pinchart" , "Jonas Karlman" , "Jernej Skrabec" , "Simon Ser" , "Harry Wentland" , "Melissa Wen" , "Sebastian Wick" , "Alex Hung" , "Jani Nikula" , "Rodrigo Vivi" , "Joonas Lahtinen" , "Tvrtko Ursulin" , "Chen-Yu Tsai" , "Samuel Holland" , "Dave Stevenson" , =?utf-8?q?Ma=C3=ADra_Canal?= , "Raspberry Pi Kernel Maintenance" From: "Luca Ceresoli" X-Mailer: aerc 0.21.0 References: <20260526-drm-mode-config-init-v6-0-852346394200@kernel.org> <20260526-drm-mode-config-init-v6-15-852346394200@kernel.org> In-Reply-To: <20260526-drm-mode-config-init-v6-15-852346394200@kernel.org> X-Last-TLS-Session-Version: TLSv1.3 Hello Maxime, Dmitry, all, On Tue May 26, 2026 at 6:46 PM CEST, Maxime Ripard wrote: > Commit 47b5ac7daa46 ("drm/atomic: Add new atomic_create_state callback > to drm_private_obj") introduced a new pattern for allocating drm object > states. > > Instead of relying on the reset() callback, it created a new > atomic_create_state hook. This is helpful because reset is a bit > overloaded: it's used to create the initial software state, reset it, > but also reset the hardware. > > It can also be used either at probe time, to create the initial state > and possibly reset the hardware to an expected default, but also during > suspend/resume. > > Both these cases come with different expectations too: during the > initialization, we want to initialize all states, but during > suspend/resume, drm_private_states for example are expected to be kept > around. > > reset() also isn't fallible, which makes it harder to handle > initialization errors properly. This is only really relevant for some > drivers though, since all the helpers for reset only create a new > state, and don't touch the hardware at all. > > It was thus decided to create a new hook that would allocate and > initialize a pristine state without any side effect: > atomic_create_state to untangle a bit some of it, and to separate the > initialization with the actual reset one might need during a > suspend/resume. > > Continue the transition to the new pattern with connectors. > > Reviewed-by: Dmitry Baryshkov > Reviewed-by: Laurent Pinchart > Reviewed-by: Thomas Zimmermann > Signed-off-by: Maxime Ripard As I'm rebasing another series on current drm-misc-next, which now includes this patch, I ran into troubles and I'm not sure what is the right thing to do. I hope you can help me clarify this. See below for my question. FTR the series I'm rebasing is "drm bridge hotplug", but the question is not specific to that series. > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -616,11 +616,19 @@ int drmm_connector_hdmi_init(struct drm_device *dev= , > > /* > * drm_connector_attach_max_bpc_property() requires the > * connector to have a state. > */ > - if (connector->funcs->reset) > + if (connector->funcs->atomic_create_state) { > + struct drm_connector_state *state; > + > + state =3D connector->funcs->atomic_create_state(connector); > + if (IS_ERR(state)) > + return PTR_ERR(state); > + > + connector->state =3D state; > + } else if (connector->funcs->reset) > connector->funcs->reset(connector); Here a state is added to connector->state, and that's fine. However non-HDMI connectors don't get a state created by default. I was hit by this with the drm_bridge_connector which it can add either an HDMI or a non-HDMI connector [0]. In the former case it calls drmm_connector_hdmi_init(), which creates the state (in the hunk quoted above). In the latter case, as I experienced at runtime and confirmed by code inspection, it does not create a state: no one calls connector->funcs->atomic_create_state. I suspect this is related to patch 19/19 which converted the drm_bridge_connector from drm_atomic_helper_connector_reset() to drm_atomic_helper_connector_create_state(), and only the former sets 'connector->state =3D conn_state'. Generally speaking, looks like a state is created only for HDMI connectors. The hardware I have uses the drm_bridge_connector in the non-HDMI case, so the state is not created and this results in a NULL pointer deref later on, in my case it's in in drm_atomic_connector_get_property(). Am I missing anything obvious? For now I've come up with a quick workaround, adding (roughly after connector init at [1]): if (!connector->state) connector->state =3D drm_bridge_connector_create_state(conn= ector); I'm not sure which would be the best solution. Maybe taking the whole atomic_create_state/reset state creation calls [2] from drmm_connector_hdmi_init() and hoist them up into drmm_connector_init(), so all connectors benefit? Let me know what you think. [0] https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/7a921d11181065267= 2e02c392b35fdcefa4d5030/drivers/gpu/drm/display/drm_bridge_connector.c#L995= -1029 [1] https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/7a921d11181065267= 2e02c392b35fdcefa4d5030/drivers/gpu/drm/display/drm_bridge_connector.c#L103= 0 [2] https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/7a921d11181065267= 2e02c392b35fdcefa4d5030/drivers/gpu/drm/drm_connector.c#L617-631 Kind regards, Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com