mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] drm/tegra: dsi: Unconditionally manage reset line
@ 2026-09-30  4:54 Mikko Perttunen
  2026-09-30  7:40 ` Thierry Reding
  0 siblings, 1 reply; 5+ messages in thread
From: Mikko Perttunen @ 2026-09-30  4:54 UTC (permalink / raw)
  To: Thierry Reding, David Airlie, Simona Vetter, Jonathan Hunter
  Cc: Thierry Reding, dri-devel, linux-tegra, linux-kernel, Mikko Perttunen

The DSI driver ignores the reset line if a power domain is configured.
This was originally added to support Tegra210, where the power domain
provider has to control the reset line -- at that time, older SoCs
didn't have a power domain for DSI. Now, however, they do with the core
power domain.

This happens to work on most systems due to DSI already being out of
reset when booting the kernel, but on Tegra114 Dalmore, this is not the
case and causes the system to hang during boot.

Ownership of the reset line is no longer a problem with reset
acquire/release semantics, so control the DSI reset unconditionally
from the DSI driver (possibly in addition to the power domain driver).
The device tree bindings already require the reset and it is present
on all platforms, so this is safe to do.

Fixes: 4cc90d4c043e ("ARM: tegra: Configure Tegra114 power domains")
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/drm/tegra/dsi.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index cb88aafbd36f..7124cf648a1a 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -1113,14 +1113,14 @@ static int tegra_dsi_runtime_suspend(struct host1x_client *client)
 	struct device *dev = client->dev;
 	int err;
 
-	if (dsi->rst) {
-		err = reset_control_assert(dsi->rst);
-		if (err < 0) {
-			dev_err(dev, "failed to assert reset: %d\n", err);
-			return err;
-		}
+	err = reset_control_assert(dsi->rst);
+	if (err < 0) {
+		dev_err(dev, "failed to assert reset: %d\n", err);
+		return err;
 	}
 
+	reset_control_release(dsi->rst);
+
 	usleep_range(1000, 2000);
 
 	clk_disable_unprepare(dsi->clk_lp);
@@ -1164,16 +1164,22 @@ static int tegra_dsi_runtime_resume(struct host1x_client *client)
 
 	usleep_range(1000, 2000);
 
-	if (dsi->rst) {
-		err = reset_control_deassert(dsi->rst);
-		if (err < 0) {
-			dev_err(dev, "cannot assert reset: %d\n", err);
-			goto disable_clk_lp;
-		}
+	err = reset_control_acquire(dsi->rst);
+	if (err < 0) {
+		dev_err(dev, "failed to acquire reset: %d\n", err);
+		goto disable_clk_lp;
+	}
+
+	err = reset_control_deassert(dsi->rst);
+	if (err < 0) {
+		dev_err(dev, "cannot deassert reset: %d\n", err);
+		goto release_reset;
 	}
 
 	return 0;
 
+release_reset:
+	reset_control_release(dsi->rst);
 disable_clk_lp:
 	clk_disable_unprepare(dsi->clk_lp);
 disable_clk:
@@ -1623,12 +1629,10 @@ static int tegra_dsi_probe(struct platform_device *pdev)
 	dsi->format = MIPI_DSI_FMT_RGB888;
 	dsi->lanes = 4;
 
-	if (!pdev->dev.pm_domain) {
-		dsi->rst = devm_reset_control_get(&pdev->dev, "dsi");
-		if (IS_ERR(dsi->rst)) {
-			err = PTR_ERR(dsi->rst);
-			goto remove;
-		}
+	dsi->rst = devm_reset_control_get_exclusive_released(&pdev->dev, "dsi");
+	if (IS_ERR(dsi->rst)) {
+		err = PTR_ERR(dsi->rst);
+		goto remove;
 	}
 
 	dsi->clk = devm_clk_get(&pdev->dev, NULL);

---
base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
change-id: 20260930-dalmore-fixes-dsi-reset-4f3191dca886


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/tegra: dsi: Unconditionally manage reset line
  2026-09-30  4:54 [PATCH] drm/tegra: dsi: Unconditionally manage reset line Mikko Perttunen
@ 2026-09-30  7:40 ` Thierry Reding
  2026-09-30  8:29   ` Thierry Reding
  0 siblings, 1 reply; 5+ messages in thread
From: Thierry Reding @ 2026-09-30  7:40 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: David Airlie, Simona Vetter, Jonathan Hunter, Thierry Reding,
	dri-devel, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1730 bytes --]

On Wed, Sep 30, 2026 at 01:54:55PM +0900, Mikko Perttunen wrote:
> The DSI driver ignores the reset line if a power domain is configured.
> This was originally added to support Tegra210, where the power domain
> provider has to control the reset line -- at that time, older SoCs
> didn't have a power domain for DSI. Now, however, they do with the core
> power domain.
> 
> This happens to work on most systems due to DSI already being out of
> reset when booting the kernel, but on Tegra114 Dalmore, this is not the
> case and causes the system to hang during boot.
> 
> Ownership of the reset line is no longer a problem with reset
> acquire/release semantics, so control the DSI reset unconditionally
> from the DSI driver (possibly in addition to the power domain driver).
> The device tree bindings already require the reset and it is present
> on all platforms, so this is safe to do.

That seems backwards to me. The whole point of doing this via power
domains was because it's explicitly not safe to toggle that reset line
outside of the powergate switching sequence.

Also, the power domain code paths are supposed to work regardless of
whether the DSI was already out of reset or not. If that's not working
right now, I think that would qualify as a bug in the power domain code
rather than the DSI driver.

Shouldn't this be fixed at the powergate level? My recollection is that
DSI is tightly coupled to the display powergates, though it's been a
long time. Do we maybe need to reflect that in DT? There's a TODO in
tegra114.dtsi that seems to indicate that we're missing DIS and DISB
powergate implementations, so maybe that's where we should start to try
and resolve this.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/tegra: dsi: Unconditionally manage reset line
  2026-09-30  7:40 ` Thierry Reding
@ 2026-09-30  8:29   ` Thierry Reding
  2026-09-30  8:46     ` Mikko Perttunen
  0 siblings, 1 reply; 5+ messages in thread
From: Thierry Reding @ 2026-09-30  8:29 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: David Airlie, Simona Vetter, Jonathan Hunter, Thierry Reding,
	dri-devel, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2434 bytes --]

On Wed, Sep 30, 2026 at 09:40:51AM +0200, Thierry Reding wrote:
> On Wed, Sep 30, 2026 at 01:54:55PM +0900, Mikko Perttunen wrote:
> > The DSI driver ignores the reset line if a power domain is configured.
> > This was originally added to support Tegra210, where the power domain
> > provider has to control the reset line -- at that time, older SoCs
> > didn't have a power domain for DSI. Now, however, they do with the core
> > power domain.
> > 
> > This happens to work on most systems due to DSI already being out of
> > reset when booting the kernel, but on Tegra114 Dalmore, this is not the
> > case and causes the system to hang during boot.
> > 
> > Ownership of the reset line is no longer a problem with reset
> > acquire/release semantics, so control the DSI reset unconditionally
> > from the DSI driver (possibly in addition to the power domain driver).
> > The device tree bindings already require the reset and it is present
> > on all platforms, so this is safe to do.
> 
> That seems backwards to me. The whole point of doing this via power
> domains was because it's explicitly not safe to toggle that reset line
> outside of the powergate switching sequence.
> 
> Also, the power domain code paths are supposed to work regardless of
> whether the DSI was already out of reset or not. If that's not working
> right now, I think that would qualify as a bug in the power domain code
> rather than the DSI driver.
> 
> Shouldn't this be fixed at the powergate level? My recollection is that
> DSI is tightly coupled to the display powergates, though it's been a
> long time. Do we maybe need to reflect that in DT? There's a TODO in
> tegra114.dtsi that seems to indicate that we're missing DIS and DISB
> powergate implementations, so maybe that's where we should start to try
> and resolve this.

Looking at this a bit more, pd_core doesn't seem to make sense for DSI.
Basically what that does, semantically, is claim that there's a power
domain responsible for powering up and down the device, but in reality
it doesn't to squat.

Does the hang you observe go away if you remove the power-domains
property for DSI? pd_core should probably be removed for both display
controllers and HDMI as well. Probably also TSEC, though the TRM isn't
clear about what power partition that's in (probably DISB because it's
used primary for HDMI, which is in DISB as well).

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/tegra: dsi: Unconditionally manage reset line
  2026-09-30  8:29   ` Thierry Reding
@ 2026-09-30  8:46     ` Mikko Perttunen
  2026-09-30  9:22       ` Thierry Reding
  0 siblings, 1 reply; 5+ messages in thread
From: Mikko Perttunen @ 2026-09-30  8:46 UTC (permalink / raw)
  To: Thierry Reding
  Cc: David Airlie, Simona Vetter, Jonathan Hunter, Thierry Reding,
	dri-devel, linux-tegra, linux-kernel

On Wednesday, September 30, 2026 5:29 PM Thierry Reding wrote:
> On Wed, Sep 30, 2026 at 09:40:51AM +0200, Thierry Reding wrote:
> > On Wed, Sep 30, 2026 at 01:54:55PM +0900, Mikko Perttunen wrote:
> > > The DSI driver ignores the reset line if a power domain is configured.
> > > This was originally added to support Tegra210, where the power domain
> > > provider has to control the reset line -- at that time, older SoCs
> > > didn't have a power domain for DSI. Now, however, they do with the core
> > > power domain.
> > > 
> > > This happens to work on most systems due to DSI already being out of
> > > reset when booting the kernel, but on Tegra114 Dalmore, this is not the
> > > case and causes the system to hang during boot.
> > > 
> > > Ownership of the reset line is no longer a problem with reset
> > > acquire/release semantics, so control the DSI reset unconditionally
> > > from the DSI driver (possibly in addition to the power domain driver).
> > > The device tree bindings already require the reset and it is present
> > > on all platforms, so this is safe to do.
> > 
> > That seems backwards to me. The whole point of doing this via power
> > domains was because it's explicitly not safe to toggle that reset line
> > outside of the powergate switching sequence.

If you're referring to the MBIST workaround, unless I misremember, it 
doesn't mean the reset cannot be used outside the powergate sequence, 
just that it has to be used within in.

> > 
> > Also, the power domain code paths are supposed to work regardless of
> > whether the DSI was already out of reset or not. If that's not working
> > right now, I think that would qualify as a bug in the power domain code
> > rather than the DSI driver.
> > 
> > Shouldn't this be fixed at the powergate level? My recollection is that
> > DSI is tightly coupled to the display powergates, though it's been a
> > long time. Do we maybe need to reflect that in DT? There's a TODO in
> > tegra114.dtsi that seems to indicate that we're missing DIS and DISB
> > powergate implementations, so maybe that's where we should start to try
> > and resolve this.
> 
> Looking at this a bit more, pd_core doesn't seem to make sense for DSI.
> Basically what that does, semantically, is claim that there's a power
> domain responsible for powering up and down the device, but in reality
> it doesn't to squat.

Doesn't that reasoning apply for every device? Technically, pd_core 
corresponds to the core power rail. DSI is powered from it.. we 
certainly haven't always had an expectation that the power domain 
provider handles resets for the device.

On Tegra114 it looks like DSI indeed is in the DIS domain so that would 
handle it, but on Tegra20/Tegra30 that isn't the case (even if those 
device seem to be working right now anyway).

> 
> Does the hang you observe go away if you remove the power-domains
> property for DSI? pd_core should probably be removed for both display
> controllers and HDMI as well. Probably also TSEC, though the TRM isn't
> clear about what power partition that's in (probably DISB because it's
> used primary for HDMI, which is in DISB as well).

I'd expect it would go away since the reset code in the driver would 
activate then.

> 
> Thierry





^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/tegra: dsi: Unconditionally manage reset line
  2026-09-30  8:46     ` Mikko Perttunen
@ 2026-09-30  9:22       ` Thierry Reding
  0 siblings, 0 replies; 5+ messages in thread
From: Thierry Reding @ 2026-09-30  9:22 UTC (permalink / raw)
  To: Mikko Perttunen
  Cc: David Airlie, Simona Vetter, Jonathan Hunter, Thierry Reding,
	dri-devel, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 5891 bytes --]

On Wed, Sep 30, 2026 at 05:46:01PM +0900, Mikko Perttunen wrote:
> On Wednesday, September 30, 2026 5:29 PM Thierry Reding wrote:
> > On Wed, Sep 30, 2026 at 09:40:51AM +0200, Thierry Reding wrote:
> > > On Wed, Sep 30, 2026 at 01:54:55PM +0900, Mikko Perttunen wrote:
> > > > The DSI driver ignores the reset line if a power domain is configured.
> > > > This was originally added to support Tegra210, where the power domain
> > > > provider has to control the reset line -- at that time, older SoCs
> > > > didn't have a power domain for DSI. Now, however, they do with the core
> > > > power domain.
> > > > 
> > > > This happens to work on most systems due to DSI already being out of
> > > > reset when booting the kernel, but on Tegra114 Dalmore, this is not the
> > > > case and causes the system to hang during boot.
> > > > 
> > > > Ownership of the reset line is no longer a problem with reset
> > > > acquire/release semantics, so control the DSI reset unconditionally
> > > > from the DSI driver (possibly in addition to the power domain driver).
> > > > The device tree bindings already require the reset and it is present
> > > > on all platforms, so this is safe to do.
> > > 
> > > That seems backwards to me. The whole point of doing this via power
> > > domains was because it's explicitly not safe to toggle that reset line
> > > outside of the powergate switching sequence.
> 
> If you're referring to the MBIST workaround, unless I misremember, it 
> doesn't mean the reset cannot be used outside the powergate sequence, 
> just that it has to be used within in.

It's not just the MBIST workaround, but the officially recommended way
to powergate/ungate devices. Sure you could do whatever you want with
the reset outside of that sequence, but that doesn't mean it's a great
idea to do so. With runtime PM we can be reasonably sure that we're in
a known-good state at this point, which is the only reason we could get
away with doing this. That doesn't make it right, though.

> > > 
> > > Also, the power domain code paths are supposed to work regardless of
> > > whether the DSI was already out of reset or not. If that's not working
> > > right now, I think that would qualify as a bug in the power domain code
> > > rather than the DSI driver.
> > > 
> > > Shouldn't this be fixed at the powergate level? My recollection is that
> > > DSI is tightly coupled to the display powergates, though it's been a
> > > long time. Do we maybe need to reflect that in DT? There's a TODO in
> > > tegra114.dtsi that seems to indicate that we're missing DIS and DISB
> > > powergate implementations, so maybe that's where we should start to try
> > > and resolve this.
> > 
> > Looking at this a bit more, pd_core doesn't seem to make sense for DSI.
> > Basically what that does, semantically, is claim that there's a power
> > domain responsible for powering up and down the device, but in reality
> > it doesn't to squat.
> 
> Doesn't that reasoning apply for every device?

Not necessarily. There might be devices that are truly only powered by
the core domain.

>                                                Technically, pd_core 
> corresponds to the core power rail.

That only means it needs to be the top-level power domain, but it
doesn't imply that it is the only power domain involved to make any
particular device work.

>                                     DSI is powered from it.. we 
> certainly haven't always had an expectation that the power domain 
> provider handles resets for the device.

Well, as evidenced by the issue you are observing, that expectation is
indeed very much baked in. DSI is part of the DIS partition, which might
be powered by the core partition, but the DIS partition also needs to
control a bunch of clocks and resets associated with devices that are
part of it. If you omit that DIS partition, then, yes, things are
expected to not work because the clocks and resets are not handled at
all.

> On Tegra114 it looks like DSI indeed is in the DIS domain so that would 
> handle it, but on Tegra20/Tegra30 that isn't the case (even if those 
> device seem to be working right now anyway).

Tegra20 and Tegra30 also seem to be wrong, to be honest. If they work,
then they probably only do by some happy accident. Evidently I wasn't
paying enough attention at the time and was convinced it was all correct
because of the Tested-bys.

I'm now wondering how much testing was actually done for these. Once the
display is turned off and back on (via framebuffer blank or DPMS), it
shouldn't come up properly again. Well, except maybe if the disable and
enable sequences are good enough to not need the reset in the middle.

> > Does the hang you observe go away if you remove the power-domains
> > property for DSI? pd_core should probably be removed for both display
> > controllers and HDMI as well. Probably also TSEC, though the TRM isn't
> > clear about what power partition that's in (probably DISB because it's
> > used primary for HDMI, which is in DISB as well).
> 
> I'd expect it would go away since the reset code in the driver would 
> activate then.

Good, and that's the right thing in that case. Again, the power domains
were introduced to specifically address the (clock/)reset/powergate
sequence requirements. That's why the code uses the power domain code
paths in a mutually exclusive way with the resets. Either the power
domains take care of the entire power sequencing as recommended by the
TRM, or it doesn't (in which case we fall back to the more brittle
manual sequencing that we do).

So I stand by what I said earlier: if there's no power domain handling
the entirety of the DSI controller power sequencing, then the DT should
not have a power-domains property for the DSI controllers.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-30  9:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-30  4:54 [PATCH] drm/tegra: dsi: Unconditionally manage reset line Mikko Perttunen
2026-09-30  7:40 ` Thierry Reding
2026-09-30  8:29   ` Thierry Reding
2026-09-30  8:46     ` Mikko Perttunen
2026-09-30  9:22       ` Thierry Reding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®