public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Pi-Hsun Shih <pihsun@chromium.org>
Cc: Pi-Hsun Shih <pihsun@chromium.org>,
	Tzung-Bi Shih <tzungbi@google.com>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Robert Foss <robert.foss@linaro.org>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Xin Ji <xji@analogixsemi.com>, Hsin-Yi Wang <hsinyi@chromium.org>,
	Yu Jiahua <yujiahua1@huawei.com>,
	dri-devel@lists.freedesktop.org (open list:DRM DRIVERS),
	linux-kernel@vger.kernel.org (open list),
	linux-arm-kernel@lists.infradead.org (moderated
	list:ARM/Mediatek SoC support),
	linux-mediatek@lists.infradead.org (moderated list:ARM/Mediatek
	SoC support)
Subject: [PATCH] drm/bridge: anx7625: Use pm_runtime_force_{suspend,resume}
Date: Wed, 14 Jul 2021 14:01:59 +0800	[thread overview]
Message-ID: <20210714060221.1483752-1-pihsun@chromium.org> (raw)

Use pm_runtime_force_suspend and pm_runtime_force_resume to ensure that
anx7625 would always be powered off when suspended. Also update the
bridge enable hook to always ensure that the anx7625 is powered on
before starting DP operations.

Fixes: 409776fa3c42 ("drm/bridge: anx7625: add suspend / resume hooks")

Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>

---

An issue was found that the anx7625 driver won't power off when used as
eDP bridge on Asurada board if suspend is entered via VT2.

The reason is that in this case, anx7625_suspend won't power off anx7625
(since intp_irq is not set). And anx7625_bridge_disable is only called
indirectly by other driver's (mediatek-drm) suspend.
pm_runtime_put_sync won't do anything since it's already in system
suspend.

If not in VT2, the bridge disable is indirectly called when Chrome
stops, so anx7625 will be powered off correctly.

To fix the issue, the suspend resume hooks are changed to
pm_runtime_force_{suspend,resume} to ensure the runtime suspend / resume
is always called correctly when system suspend / resume.
(Note that IRQ no longer needs to be disabled on suspend after commit
f03ab6629c7b ("drm/bridge: anx7625: Make hpd workqueue freezable"))

Since bridge disable is called indirectly by mediatek-drm driver's
suspend, it might happens after anx7625 suspend is called. So a check
if the driver is already suspended via pm_runtime_force_suspend is also
added, to ensure that the anx7625_dp_stop won't be called when power
is off. And also since bridge enable might happens before anx7625 resume
is called, a check to that is also added, and would force resume the
device in this case.

I'm not sure if the approach to fix this is the most appropriate way,
since using pm_runtime_force_resume in bridge enable kinda feels hacky
to me. I'm open to any suggestions.

---
 drivers/gpu/drm/bridge/analogix/anx7625.c | 55 +++++++++--------------
 1 file changed, 20 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index a3d82377066b..9d0f5dc88b16 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1559,7 +1559,20 @@ static void anx7625_bridge_enable(struct drm_bridge *bridge)
 
 	DRM_DEV_DEBUG_DRIVER(dev, "drm enable\n");
 
-	pm_runtime_get_sync(dev);
+	/*
+	 * The only case where pm_runtime is disabled here is when the function
+	 * is called other driver's resume hook by
+	 * drm_mode_config_helper_resume, but when the pm_runtime_force_resume
+	 * hasn't been called on this device.
+	 *
+	 * pm_runtime_get_sync won't power on anx7625 in this case since we're
+	 * in system resume, so instead we force resume anx7625 to make sure
+	 * the following anx7625_dp_start would succeed.
+	 */
+	if (pm_runtime_enabled(dev))
+		pm_runtime_get_sync(dev);
+	else
+		pm_runtime_force_resume(dev);
 
 	anx7625_dp_start(ctx);
 }
@@ -1571,9 +1584,10 @@ static void anx7625_bridge_disable(struct drm_bridge *bridge)
 
 	DRM_DEV_DEBUG_DRIVER(dev, "drm disable\n");
 
-	anx7625_dp_stop(ctx);
-
-	pm_runtime_put_sync(dev);
+	if (pm_runtime_enabled(dev)) {
+		anx7625_dp_stop(ctx);
+		pm_runtime_put_sync(dev);
+	}
 }
 
 static enum drm_connector_status
@@ -1705,38 +1719,9 @@ static int __maybe_unused anx7625_runtime_pm_resume(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused anx7625_resume(struct device *dev)
-{
-	struct anx7625_data *ctx = dev_get_drvdata(dev);
-
-	if (!ctx->pdata.intp_irq)
-		return 0;
-
-	if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) {
-		enable_irq(ctx->pdata.intp_irq);
-		anx7625_runtime_pm_resume(dev);
-	}
-
-	return 0;
-}
-
-static int __maybe_unused anx7625_suspend(struct device *dev)
-{
-	struct anx7625_data *ctx = dev_get_drvdata(dev);
-
-	if (!ctx->pdata.intp_irq)
-		return 0;
-
-	if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) {
-		anx7625_runtime_pm_suspend(dev);
-		disable_irq(ctx->pdata.intp_irq);
-	}
-
-	return 0;
-}
-
 static const struct dev_pm_ops anx7625_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(anx7625_suspend, anx7625_resume)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
 	SET_RUNTIME_PM_OPS(anx7625_runtime_pm_suspend,
 			   anx7625_runtime_pm_resume, NULL)
 };

base-commit: c0d438dbc0b74901f1901d97a6c84f38daa0c831
-- 
2.32.0.93.g670b81a890-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

             reply	other threads:[~2021-07-14  6:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-14  6:01 Pi-Hsun Shih [this message]
2021-07-14 10:32 ` [PATCH] drm/bridge: anx7625: Use pm_runtime_force_{suspend,resume} Daniel Vetter
2021-07-16  8:26   ` [PATCH] drm/bridge: anx7625: Use pm_runtime_force_{suspend, resume} Pi-Hsun Shih
2021-07-16  9:15     ` [PATCH] drm/bridge: anx7625: Use pm_runtime_force_{suspend,resume} Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210714060221.1483752-1-pihsun@chromium.org \
    --to=pihsun@chromium.org \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=a.hajda@samsung.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hsinyi@chromium.org \
    --cc=jernej.skrabec@gmail.com \
    --cc=jonas@kwiboo.se \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=narmstrong@baylibre.com \
    --cc=robert.foss@linaro.org \
    --cc=tzungbi@google.com \
    --cc=xji@analogixsemi.com \
    --cc=yujiahua1@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox