From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C6A93C8303C for ; Mon, 7 Jul 2025 11:00:19 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 31E0710E445; Mon, 7 Jul 2025 11:00:19 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=bootlin.com header.i=@bootlin.com header.b="GtZHU3TZ"; dkim-atps=neutral Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by gabe.freedesktop.org (Postfix) with ESMTPS id E8B8B10E443 for ; Mon, 7 Jul 2025 11:00:17 +0000 (UTC) Received: by mail.gandi.net (Postfix) with ESMTPSA id 6D98E43B13; Mon, 7 Jul 2025 11:00:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1751886016; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=KJ08fgVcPu2E4Q3b9o7vg8fIv0XfpbzcrN1iC3FdEfA=; b=GtZHU3TZ7ju8uMcajHenv3DzJN4WYFX1ZfCVz7exmCmvHb93NUHPrX65wgGZ90LUaPuzNh Z2tflzaSn/5gfHmx/sDi+89slTpro7FFXd2lSg2t/CA6Z3UMv7CqS7XJgnUu8ZcARJ3uwf UZo7HtudUx31BRRR70HRRv/4HBwHPggS6vA5y6NVrOjPtDok4OM2TGIPlXjE35Y5wg0m2Y +pT/4omko0gtyLOJOHQdVOFVPnEjufKqyH9n7j+TJ+ZiZF6IVmYzOn6+Tg+3HEEz4Rz9gI 2G1yCx+AnuTK6Esfm7swzrR3Z1iREUoJ0M9aLevWm44o/9wRz//ywDMs8Djl3Q== Date: Mon, 7 Jul 2025 12:59:54 +0200 From: Luca Ceresoli To: Marek Szyprowski Cc: Maxime Ripard , dri-devel@lists.freedesktop.org, Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Thomas Zimmermann , David Airlie , Simona Vetter , Dmitry Baryshkov , Douglas Anderson , Damon Ding Subject: Re: [PATCH] drm/bridge: analogix_dp: Use devm_drm_bridge_alloc() API Message-ID: <20250707125954.5e0bbaa8@booty> In-Reply-To: References: <20250627165652.580798-1-m.szyprowski@samsung.com> <20250630-famous-dark-boar-89bed7@houat> <20250701160219.20dc7466@booty> <20250701-petite-mutant-starling-24bbe5@houat> <20250703175032.6f49f862@booty> Organization: Bootlin X-Mailer: Claws Mail 4.3.1 (GTK 3.24.49; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-GND-State: clean X-GND-Score: -100 X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdefgdefudeivdcutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfitefpfffkpdcuggftfghnshhusghstghrihgsvgenuceurghilhhouhhtmecufedtudenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujfgurhepfffhvfevuffkjghfohfogggtgfesthhqredtredtjeenucfhrhhomhepnfhutggrucevvghrvghsohhlihcuoehluhgtrgdrtggvrhgvshholhhisegsohhothhlihhnrdgtohhmqeenucggtffrrghtthgvrhhnpefhteduuefgheehleeihfejjeduvdeltddutddtveeltdfhheeguefgteehkefgffenucffohhmrghinhepsghoohhtlhhinhdrtghomhenucfkphepkeejrdduvddtrddvudekrddvtdejnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepkeejrdduvddtrddvudekrddvtdejpdhhvghlohepsghoohhthidpmhgrihhlfhhrohhmpehluhgtrgdrtggvrhgvshholhhisegsohhothhlihhnrdgtohhmpdhnsggprhgtphhtthhopeduiedprhgtphhtthhopehmrdhsiiihphhrohifshhkihesshgrmhhsuhhnghdrtghomhdprhgtphhtthhopehmrhhiphgrrhgusehkvghrnhgvlhdrohhrghdprhgtphhtthhopegurhhiqdguvghvvghlsehlihhsthhsrdhfrhgvvgguvghskhhtohhprdhorhhgpdhrtghpthhtoheprghnughriigvjhdrhhgrjhgurgesihhnthgvlhdrtghomhdprhgtp hhtthhopehnvghilhdrrghrmhhsthhrohhngheslhhinhgrrhhordhorhhgpdhrtghpthhtoheprhhfohhssheskhgvrhhnvghlrdhorhhgpdhrtghpthhtohepnfgruhhrvghnthdrphhinhgthhgrrhhtsehiuggvrghsohhnsghorghrugdrtghomhdprhgtphhtthhopehjohhnrghssehkfihisghoohdrshgv X-GND-Sasl: luca.ceresoli@bootlin.com X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Mon, 7 Jul 2025 11:07:26 +0200 Marek Szyprowski wrote: > On 03.07.2025 17:50, Luca Ceresoli wrote: > > On Tue, 1 Jul 2025 16:27:54 +0200 > > Maxime Ripard wrote: > > =20 > >> On Tue, Jul 01, 2025 at 04:02:19PM +0200, Luca Ceresoli wrote: =20 > >>> Hello Marek, Maxime, > >>> > >>> thanks Marek for spotting the issue and sending a patch! > >>> > >>> On Mon, 30 Jun 2025 18:44:24 +0200 > >>> Maxime Ripard wrote: > >>> =20 > >>>>> @@ -1643,7 +1625,7 @@ int analogix_dp_bind(struct analogix_dp_devic= e *dp, struct drm_device *drm_dev) > >>>>> return ret; > >>>>> } > >>>>> =20 > >>>>> - ret =3D analogix_dp_create_bridge(drm_dev, dp); > >>>>> + ret =3D drm_bridge_attach(dp->encoder, &dp->bridge, NULL, 0); > >>>>> if (ret) { > >>>>> DRM_ERROR("failed to create bridge (%d)\n", ret); > >>>>> goto err_unregister_aux; =20 > >>>> It looks like you don't set bridge->driver_private anymore. Is it on= purpose? =20 > >>> This looks correct to me. In current code, driver_private is used to > >>> hold a pointer to the driver private struct (struct > >>> analogix_dp_device). With devm_drm_bridge_alloc() container_of() is n= ow > >>> enough, no pointer is needed. With the patch applied, driver_private > >>> becomes unused. =20 > >> Then we should remove it from the structure if it's unused. =20 > > Makes sense now that struct drm_bridge is meant to be always embedded > > in a driver-private struct. But several drivers are still using it, so > > those would need to be updated beforehand: > > > > $ git grep -l driver_private -- drivers/gpu/drm/ | wc -l > > 23 > > $ > > > > So I think this patch should be taken as it fixes a regression. Do you > > agree on this? =20 >=20 > Yes, please apply it as a fix :) >=20 >=20 > BTW, there are 2 more bridge drivers that need a fix similar to the=20 > $subject patch: >=20 > $ git grep "bridge =3D devm_kzalloc" drivers/gpu > drivers/gpu/drm/sti/sti_hda.c:=C2=A0 bridge =3D devm_kzalloc(dev,=20 > sizeof(*bridge), GFP_KERNEL); > drivers/gpu/drm/sti/sti_hdmi.c: bridge =3D devm_kzalloc(dev,=20 > sizeof(*bridge), GFP_KERNEL); Ouch. My grep logic was probably too clever and missed these obvious ones. I'm taking care of converting these ones later this week as time permits, unless patches are sent before. Thanks for reporting! Luca --=20 Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com