diff mbox series

[v4,2/2] drm/bridge: analogix_dp: Enable autosuspend

Message ID 20220301181107.v4.2.I48b18ab197c9b649d376cf8cfd934e59d338f86d@changeid (mailing list archive)
State New, archived
Headers show
Series [v4,1/2] drm/bridge: analogix_dp: Grab runtime PM reference for DP-AUX | expand

Commit Message

Brian Norris March 2, 2022, 2:11 a.m. UTC
DP AUX transactions can consist of many short operations. There's no
need to power things up/down in short intervals.

I pick an arbitrary 100ms; for the systems I'm testing (Rockchip
RK3399), runtime-PM transitions only take a few microseconds.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---

Changes in v4:
 - call pm_runtime_mark_last_busy() and
   pm_runtime_dont_use_autosuspend()
 - drop excess pm references around drm_get_edid(), now that we grab and
   hold in the dp-aux helper

Changes in v3:
 - New in v3

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Doug Anderson March 3, 2022, 11:02 p.m. UTC | #1
Hi,

On Tue, Mar 1, 2022 at 6:11 PM Brian Norris <briannorris@chromium.org> wrote:
>
> DP AUX transactions can consist of many short operations. There's no
> need to power things up/down in short intervals.
>
> I pick an arbitrary 100ms; for the systems I'm testing (Rockchip
> RK3399), runtime-PM transitions only take a few microseconds.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>
> Changes in v4:
>  - call pm_runtime_mark_last_busy() and
>    pm_runtime_dont_use_autosuspend()
>  - drop excess pm references around drm_get_edid(), now that we grab and
>    hold in the dp-aux helper
>
> Changes in v3:
>  - New in v3
>
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

This looks great to me now, thanks.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Though I'm not a massive expert on the Analogix DP driver, I'm pretty
confident about the DP AUX stuff that Brian is touching. I just
checked and I see that this driver isn't changing lots and the last
change landed in drm-misc, which means that I can commit this. Thus,
unless someone else shouts, I'll plan to wait until next week and
commit these two patches to drm-misc.

The first of the two patches is a "Fix" but since it's been broken
since 2016 I'll assume that nobody is chomping at the bit for these to
get into stable and that it would be easier to land both in
"drm-misc-next". Please yell if someone disagrees.

-Doug
Doug Anderson March 9, 2022, 4:04 p.m. UTC | #2
Hi,

On Thu, Mar 3, 2022 at 3:02 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Mar 1, 2022 at 6:11 PM Brian Norris <briannorris@chromium.org> wrote:
> >
> > DP AUX transactions can consist of many short operations. There's no
> > need to power things up/down in short intervals.
> >
> > I pick an arbitrary 100ms; for the systems I'm testing (Rockchip
> > RK3399), runtime-PM transitions only take a few microseconds.
> >
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > ---
> >
> > Changes in v4:
> >  - call pm_runtime_mark_last_busy() and
> >    pm_runtime_dont_use_autosuspend()
> >  - drop excess pm references around drm_get_edid(), now that we grab and
> >    hold in the dp-aux helper
> >
> > Changes in v3:
> >  - New in v3
> >
> >  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
>
> This looks great to me now, thanks.
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> Though I'm not a massive expert on the Analogix DP driver, I'm pretty
> confident about the DP AUX stuff that Brian is touching. I just
> checked and I see that this driver isn't changing lots and the last
> change landed in drm-misc, which means that I can commit this. Thus,
> unless someone else shouts, I'll plan to wait until next week and
> commit these two patches to drm-misc.
>
> The first of the two patches is a "Fix" but since it's been broken
> since 2016 I'll assume that nobody is chomping at the bit for these to
> get into stable and that it would be easier to land both in
> "drm-misc-next". Please yell if someone disagrees.

Pushed both to drm-misc-next:

f28dd5075675 drm/bridge: analogix_dp: Enable autosuspend
8fb6c44fe846 drm/bridge: analogix_dp: Grab runtime PM reference for DP-AUX

-Doug
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 16be279aed2c..b248d352f2bd 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1119,9 +1119,7 @@  static int analogix_dp_get_modes(struct drm_connector *connector)
 			return 0;
 		}
 
-		pm_runtime_get_sync(dp->dev);
 		edid = drm_get_edid(connector, &dp->aux.ddc);
-		pm_runtime_put(dp->dev);
 		if (edid) {
 			drm_connector_update_edid_property(&dp->connector,
 							   edid);
@@ -1642,7 +1640,8 @@  static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux,
 
 	ret = analogix_dp_transfer(dp, msg);
 out:
-	pm_runtime_put(dp->dev);
+	pm_runtime_mark_last_busy(dp->dev);
+	pm_runtime_put_autosuspend(dp->dev);
 
 	return ret;
 }
@@ -1775,6 +1774,8 @@  int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
 	if (ret)
 		return ret;
 
+	pm_runtime_use_autosuspend(dp->dev);
+	pm_runtime_set_autosuspend_delay(dp->dev, 100);
 	pm_runtime_enable(dp->dev);
 
 	ret = analogix_dp_create_bridge(drm_dev, dp);
@@ -1786,6 +1787,7 @@  int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
 	return 0;
 
 err_disable_pm_runtime:
+	pm_runtime_dont_use_autosuspend(dp->dev);
 	pm_runtime_disable(dp->dev);
 	drm_dp_aux_unregister(&dp->aux);
 
@@ -1804,6 +1806,7 @@  void analogix_dp_unbind(struct analogix_dp_device *dp)
 	}
 
 	drm_dp_aux_unregister(&dp->aux);
+	pm_runtime_dont_use_autosuspend(dp->dev);
 	pm_runtime_disable(dp->dev);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_unbind);