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 B8CE8C54798 for ; Thu, 29 Feb 2024 09:47:31 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E21CE10E030; Thu, 29 Feb 2024 09:47:30 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=bootlin.com header.i=@bootlin.com header.b="QtFI7VAj"; dkim-atps=neutral Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by gabe.freedesktop.org (Postfix) with ESMTPS id A98EE10E030 for ; Thu, 29 Feb 2024 09:47:28 +0000 (UTC) Received: by mail.gandi.net (Postfix) with ESMTPSA id 8F9E91C0005; Thu, 29 Feb 2024 09:47:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1709200045; 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=dLOsKkSstGKuygF2kpbVdpnYpVw5jbt/6cncLhtyez0=; b=QtFI7VAjXnK+tE327My6hHmbvUSrD+WZYY/2aNSrzZT4j5I+DkObdfq/RQbSE08gioGmVX MSzlBTopz/ofQ7fJmePRAYW1AlhnYwXKokSNTuDq92YKYDyYhhk2F5IL3VijCK56+Vgy8E DuckYgCxpPKpi2krotxqpR2oRIPs5f8eQW8VAytgncHCP4os6n5/zoa99vYU4H8Gq6FTxw Q36B7aR1Ir2hiJFVAJMekUR5hgM+j9/tTc+d9u+jTgk10BbPXQA5j1323Fnqqj+djIKIJ9 wkJ7LEaIEVtRJEGArs90KC130qS6N8+y1zcUMLjmDTrBfVcWTHr6p9HHJFTQKw== Date: Thu, 29 Feb 2024 10:47:23 +0100 From: Luca Ceresoli To: Alexander Stein Cc: Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , David Airlie , Daniel Vetter , dri-devel@lists.freedesktop.org Subject: Re: [PATCH 1/1] drm/bridge: ti-sn65dsi83: Fix enable error path Message-ID: <20240229104723.7aa71075@booty> In-Reply-To: <3798602.kQq0lBPeGt@steina-w> References: <20230504065316.2640739-1-alexander.stein@ew.tq-group.com> <1885005.tdWV9SEqCh@steina-w> <20240227184144.19729521@booty> <3798602.kQq0lBPeGt@steina-w> Organization: Bootlin X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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" Hello Alexander, On Wed, 28 Feb 2024 09:15:46 +0100 Alexander Stein wrote: [...] > Oh I mistook this DSI-LVDS bridge with the DSI-DP bridge on a different > board, my bad. I hope I can provide some insights. My platform is > imx8mm-tqma8mqml-mba8mx-lvds-tm070jvhg33.dtb. > I can easily cause a PLL lock failure by reducing the delay for the > enable-gpios 'gpio_delays'. This will result in a PLL lock faiure. > On my platform the vcc-supply counters do look sane: > > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1 > > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:0 Interesting. Thanks for taking time to report your initial issue! > Once I remove the ti_sn65dsi83 module, the open_count decrements to 0 as > well. Looks sane to me. > > If I revert commit c81cd8f7c774 ("Revert "drm/bridge: ti-sn65dsi83: > Fix enable error path""), vcc-supply counters are: > > /sys/kernel/debug/regulator/SN65DSI83_1V8/open_count:1 > > /sys/kernel/debug/regulator/SN65DSI83_1V8/use_count:1 > > So in my case the use_count does not decrease! If I remove the module > ti_sn65dsi83, I get the WARN_ON (enable_count is still non-zero): > > WARNING: CPU: 2 PID: 402 at drivers/regulator/core.c:2398 _regulator_put+0x15c/0x164 > > This is on 6.8.0-rc6-next-20240228 with the following diff applied: > --->8--- > diff --git a/arch/arm64/boot/dts/freescale/mba8mx.dtsi b/arch/arm64/boot/dts/freescale/mba8mx.dtsi > index 427467df42bf..8461e1fd396f 100644 > --- a/arch/arm64/boot/dts/freescale/mba8mx.dtsi > +++ b/arch/arm64/boot/dts/freescale/mba8mx.dtsi > @@ -285,7 +285,7 @@ &i2c3 { > dsi_lvds_bridge: bridge@2d { > compatible = "ti,sn65dsi84"; > reg = <0x2d>; > - enable-gpios = <&gpio_delays 0 130000 0>; > + enable-gpios = <&gpio_delays 0 0 0>; > vcc-supply = <®_sn65dsi83_1v8>; > status = "disabled"; > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > index 4814b7b6d1fd..57a7ed13f996 100644 > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > @@ -478,7 +478,6 @@ static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge, > dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret); > /* On failure, disable PLL again and exit. */ > regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00); > - regulator_disable(ctx->vcc); > return; > } > --->8--- > > So my patch indeed did fix an actual problem. On the other hand it seems > sn65dsi83_atomic_disable is not called in my case for some reason. So you remove the module and atomic_disable is not called, after having called atomic_pre_enable? I'm very possibly missing something, but this looks like a bug in the DRM bridge code at first sight. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com