[v3] drm/bridge: ti-sn65dsi86: ensure bridge suspend happens during PM sleep
diff mbox series

Message ID 20200609120455.20458-1-harigovi@codeaurora.org
State New
Headers show
Series
  • [v3] drm/bridge: ti-sn65dsi86: ensure bridge suspend happens during PM sleep
Related show

Commit Message

Harigovindan P June 9, 2020, 12:04 p.m. UTC
ti-sn65dsi86 bridge is enumerated as a runtime device. When
suspend is triggered, PM core adds a refcount on all the
devices and calls device suspend, since usage count is
already incremented, runtime suspend will not be called
and it kept the bridge regulators and gpios ON which resulted
in platform not entering into XO shutdown.

Add changes to force suspend on the runtime device during pm sleep.

Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
---

Changes in v2:
	- Include bridge name in the commit message and 
	remove dependent patchwork link from the commit
	text as bridge is independent of OEM(Stephen Boyd)

Changes in v3:
	- Updating changelog to explain the need for patch

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Doug Anderson June 10, 2020, 6:57 p.m. UTC | #1
Hi,

On Tue, Jun 9, 2020 at 5:05 AM Harigovindan P <harigovi@codeaurora.org> wrote:
>
> ti-sn65dsi86 bridge is enumerated as a runtime device. When
> suspend is triggered, PM core adds a refcount on all the
> devices and calls device suspend, since usage count is
> already incremented, runtime suspend will not be called
> and it kept the bridge regulators and gpios ON which resulted
> in platform not entering into XO shutdown.

The "XO shutdown" concept is a very Qualcomm-specific concept but this
bridge chip could be used on any board.  ...but I think it's fine to
leave it as-is since the meaning is clear enough.


> Add changes to force suspend on the runtime device during pm sleep.
>
> Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
> ---
>
> Changes in v2:
>         - Include bridge name in the commit message and
>         remove dependent patchwork link from the commit
>         text as bridge is independent of OEM(Stephen Boyd)
>
> Changes in v3:
>         - Updating changelog to explain the need for patch
>
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 ++
>  1 file changed, 2 insertions(+)

Note that it's generally considered nice to CC folks who commented on
previous versions of the patch since they are folks who are almost
certainly interested in it.  In this case that would be me and Stephen
Boyd.

You could also, in theory, carry Stephen's Reviewed-by patch from v1
since you have fixed the subject as he requested and you have now also
fixed the commit message to have some details.

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Stephen Boyd June 11, 2020, 9:56 a.m. UTC | #2
Quoting Harigovindan P (2020-06-09 05:04:55)
> ti-sn65dsi86 bridge is enumerated as a runtime device. When
> suspend is triggered, PM core adds a refcount on all the
> devices and calls device suspend, since usage count is
> already incremented, runtime suspend will not be called
> and it kept the bridge regulators and gpios ON which resulted
> in platform not entering into XO shutdown.
> 
> Add changes to force suspend on the runtime device during pm sleep.
> 
> Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Doug Anderson June 18, 2020, 10:09 p.m. UTC | #3
Hi,

On Tue, Jun 9, 2020 at 5:05 AM Harigovindan P <harigovi@codeaurora.org> wrote:
>
> ti-sn65dsi86 bridge is enumerated as a runtime device. When
> suspend is triggered, PM core adds a refcount on all the
> devices and calls device suspend, since usage count is
> already incremented, runtime suspend will not be called
> and it kept the bridge regulators and gpios ON which resulted
> in platform not entering into XO shutdown.
>
> Add changes to force suspend on the runtime device during pm sleep.
>
> Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
> ---
>
> Changes in v2:
>         - Include bridge name in the commit message and
>         remove dependent patchwork link from the commit
>         text as bridge is independent of OEM(Stephen Boyd)
>
> Changes in v3:
>         - Updating changelog to explain the need for patch
>
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 ++
>  1 file changed, 2 insertions(+)

I think this patch is good to go now (has both Stephen's and my
reviews).  I noticed that Neil landed my other patches to this driver
recently (thanks!) and wondered why he didn't land this one.  Then, I
realized that you didn't send it to him or the other bridge
maintainer.  :(  Have you tried running get_maintainer?

$ ./scripts/get_maintainer.pl -f drivers/gpu/drm/bridge/ti-sn65dsi86.c
Andrzej Hajda <a.hajda@samsung.com> (maintainer:DRM DRIVERS FOR BRIDGE CHIPS)
Neil Armstrong <narmstrong@baylibre.com> (maintainer:DRM DRIVERS FOR
BRIDGE CHIPS)
Laurent Pinchart <Laurent.pinchart@ideasonboard.com> (reviewer:DRM
DRIVERS FOR BRIDGE CHIPS)
Jonas Karlman <jonas@kwiboo.se> (reviewer:DRM DRIVERS FOR BRIDGE CHIPS)
Jernej Skrabec <jernej.skrabec@siol.net> (reviewer:DRM DRIVERS FOR BRIDGE CHIPS)
David Airlie <airlied@linux.ie> (maintainer:DRM DRIVERS)
Daniel Vetter <daniel@ffwll.ch> (maintainer:DRM DRIVERS)
dri-devel@lists.freedesktop.org (open list:DRM DRIVERS)
linux-kernel@vger.kernel.org (open list)

In any case, unless someone has extra feedback on this patch I think
it's ready to land.

Neil: If you're willing to land this patch too, can you let
Harigovindan know if it needs to be re-sent with you in the "To:" list
or if you can find it on the dri-devel list?

Thanks!

-Doug
Neil Armstrong June 26, 2020, 11:55 a.m. UTC | #4
Hi,

On 19/06/2020 00:09, Doug Anderson wrote:
> Hi,
> 
> On Tue, Jun 9, 2020 at 5:05 AM Harigovindan P <harigovi@codeaurora.org> wrote:
>>
>> ti-sn65dsi86 bridge is enumerated as a runtime device. When
>> suspend is triggered, PM core adds a refcount on all the
>> devices and calls device suspend, since usage count is
>> already incremented, runtime suspend will not be called
>> and it kept the bridge regulators and gpios ON which resulted
>> in platform not entering into XO shutdown.
>>
>> Add changes to force suspend on the runtime device during pm sleep.
>>
>> Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
>> ---
>>
>> Changes in v2:
>>         - Include bridge name in the commit message and
>>         remove dependent patchwork link from the commit
>>         text as bridge is independent of OEM(Stephen Boyd)
>>
>> Changes in v3:
>>         - Updating changelog to explain the need for patch
>>
>>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 
> I think this patch is good to go now (has both Stephen's and my
> reviews).  I noticed that Neil landed my other patches to this driver
> recently (thanks!) and wondered why he didn't land this one.  Then, I
> realized that you didn't send it to him or the other bridge
> maintainer.  :(  Have you tried running get_maintainer?
> 
> $ ./scripts/get_maintainer.pl -f drivers/gpu/drm/bridge/ti-sn65dsi86.c
> Andrzej Hajda <a.hajda@samsung.com> (maintainer:DRM DRIVERS FOR BRIDGE CHIPS)
> Neil Armstrong <narmstrong@baylibre.com> (maintainer:DRM DRIVERS FOR
> BRIDGE CHIPS)
> Laurent Pinchart <Laurent.pinchart@ideasonboard.com> (reviewer:DRM
> DRIVERS FOR BRIDGE CHIPS)
> Jonas Karlman <jonas@kwiboo.se> (reviewer:DRM DRIVERS FOR BRIDGE CHIPS)
> Jernej Skrabec <jernej.skrabec@siol.net> (reviewer:DRM DRIVERS FOR BRIDGE CHIPS)
> David Airlie <airlied@linux.ie> (maintainer:DRM DRIVERS)
> Daniel Vetter <daniel@ffwll.ch> (maintainer:DRM DRIVERS)
> dri-devel@lists.freedesktop.org (open list:DRM DRIVERS)
> linux-kernel@vger.kernel.org (open list)
> 
> In any case, unless someone has extra feedback on this patch I think
> it's ready to land.
> 
> Neil: If you're willing to land this patch too, can you let
> Harigovindan know if it needs to be re-sent with you in the "To:" list
> or if you can find it on the dri-devel list?

Sorry missed this one,

Applying to drm-misc-next

Neil

> 
> Thanks!
> 
> -Doug
>

Patch
diff mbox series

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 6ad688b320ae..2eef755b2917 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -159,6 +159,8 @@  static int __maybe_unused ti_sn_bridge_suspend(struct device *dev)
 
 static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
 	SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
 };
 
 static int status_show(struct seq_file *s, void *data)