[PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume

[PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume

[Douglas Anderson]

On Rockchip rk3288-based Chromebooks when you do a suspend/resume
cycle:

1. You lose the ability to detect an HDMI device being plugged in.

2. If you're using the i2c bus built in to dw_hdmi then it stops
working.

Let's add a hook to the core dw-hdmi driver so that we can call it in
dw_hdmi-rockchip in the next commit.

NOTE: the exact set of steps I've done here in resume come from
looking at the normal dw_hdmi init sequence in upstream Linux plus the
sequence that we did in downstream Chrome OS 3.14.  Testing show that
it seems to work, but if an extra step is needed or something here is
not needed we could improve it.

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

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 21 +++++++++++++++++++++
 include/drm/bridge/dw_hdmi.h              |  3 +++
 2 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index db761329a1e3..4b38bfd43e59 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2780,6 +2780,27 @@ void dw_hdmi_unbind(struct dw_hdmi *hdmi)
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
 
+int dw_hdmi_suspend(struct dw_hdmi *hdmi)
+{
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_suspend);
+
+int dw_hdmi_resume(struct dw_hdmi *hdmi)
+{
+	initialize_hdmi_ih_mutes(hdmi);
+
+	dw_hdmi_setup_i2c(hdmi);
+	if (hdmi->i2c)
+		dw_hdmi_i2c_init(hdmi);
+
+	if (hdmi->phy.ops->setup_hpd)
+		hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_resume);
+
 MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
 MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com>");
 MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 66e70770cce5..c4132e9a5ae3 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -154,6 +154,9 @@ struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
 			     struct drm_encoder *encoder,
 			     const struct dw_hdmi_plat_data *plat_data);
 
+int dw_hdmi_suspend(struct dw_hdmi *hdmi);
+int dw_hdmi_resume(struct dw_hdmi *hdmi);
+
 void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
 
 void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
-- 
2.21.0.1020.gf2820cf01a-goog

[PATCH 2/2] drm/rockchip: dw_hdmi: Handle suspend/resume

[Douglas Anderson]

On Rockchip rk3288-based Chromebooks when you do a suspend/resume
cycle:

1. You lose the ability to detect an HDMI device being plugged in.

2. If you're using the i2c bus built in to dw_hdmi then it stops
working.

Let's call the core dw-hdmi's suspend/resume functions to restore
things.

NOTE: in downstream Chrome OS (based on kernel 3.14) we used the
"late/early" versions of suspend/resume because we found that the VOP
was sometimes resuming before dw_hdmi and then calling into us before
we were fully resumed.  For now I have gone back to the normal
suspend/resume because I can't reproduce the problems.

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

 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 4cdc9f86c2e5..deb0e8c30c03 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -542,11 +542,31 @@ static int dw_hdmi_rockchip_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int __maybe_unused dw_hdmi_rockchip_suspend(struct device *dev)
+{
+	struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
+
+	return dw_hdmi_suspend(hdmi->hdmi);
+}
+
+static int __maybe_unused dw_hdmi_rockchip_resume(struct device *dev)
+{
+	struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
+
+	return dw_hdmi_resume(hdmi->hdmi);
+}
+
+const struct dev_pm_ops dw_hdmi_rockchip_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(dw_hdmi_rockchip_suspend,
+				dw_hdmi_rockchip_resume)
+};
+
 struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
 	.probe  = dw_hdmi_rockchip_probe,
 	.remove = dw_hdmi_rockchip_remove,
 	.driver = {
 		.name = "dwhdmi-rockchip",
+		.pm = &dw_hdmi_rockchip_pm,
 		.of_match_table = dw_hdmi_rockchip_dt_ids,
 	},
 };
-- 
2.21.0.1020.gf2820cf01a-goog

Re: [PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume

[Doug Anderson]

Hi

From: Douglas Anderson <dianders@chromium.org>
Date: Thu, May 2, 2019 at 3:38 PM
To: Heiko Stuebner, Sandy Huang, Andrzej Hajda, Laurent Pinchart
Cc: <linux-rockchip@lists.infradead.org>, Neil Armstrong,
<mka@chromium.org>, Sean Paul, Douglas Anderson, Zheng Yang, Sam
Ravnborg, <dri-devel@lists.freedesktop.org>,
<linux-kernel@vger.kernel.org>, Ville Syrjälä, David Airlie, Jernej
Skrabec, Daniel Vetter

> On Rockchip rk3288-based Chromebooks when you do a suspend/resume
> cycle:
>
> 1. You lose the ability to detect an HDMI device being plugged in.
>
> 2. If you're using the i2c bus built in to dw_hdmi then it stops
> working.
>
> Let's add a hook to the core dw-hdmi driver so that we can call it in
> dw_hdmi-rockchip in the next commit.
>
> NOTE: the exact set of steps I've done here in resume come from
> looking at the normal dw_hdmi init sequence in upstream Linux plus the
> sequence that we did in downstream Chrome OS 3.14.  Testing show that
> it seems to work, but if an extra step is needed or something here is
> not needed we could improve it.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 21 +++++++++++++++++++++
>  include/drm/bridge/dw_hdmi.h              |  3 +++
>  2 files changed, 24 insertions(+)

Did anyone have any thoughts on this patch series?  Thanks!  :-)

-Doug

Re: [PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume

[Sean Paul]

On Thu, May 02, 2019 at 03:38:07PM -0700, Douglas Anderson wrote:
> On Rockchip rk3288-based Chromebooks when you do a suspend/resume
> cycle:
> 
> 1. You lose the ability to detect an HDMI device being plugged in.
> 
> 2. If you're using the i2c bus built in to dw_hdmi then it stops
> working.
> 
> Let's add a hook to the core dw-hdmi driver so that we can call it in
> dw_hdmi-rockchip in the next commit.
> 
> NOTE: the exact set of steps I've done here in resume come from
> looking at the normal dw_hdmi init sequence in upstream Linux plus the
> sequence that we did in downstream Chrome OS 3.14.  Testing show that
> it seems to work, but if an extra step is needed or something here is
> not needed we could improve it.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 21 +++++++++++++++++++++
>  include/drm/bridge/dw_hdmi.h              |  3 +++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index db761329a1e3..4b38bfd43e59 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2780,6 +2780,27 @@ void dw_hdmi_unbind(struct dw_hdmi *hdmi)
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
>  
> +int dw_hdmi_suspend(struct dw_hdmi *hdmi)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_suspend);
> +
> +int dw_hdmi_resume(struct dw_hdmi *hdmi)
> +{
> +	initialize_hdmi_ih_mutes(hdmi);
> +
> +	dw_hdmi_setup_i2c(hdmi);
> +	if (hdmi->i2c)
> +		dw_hdmi_i2c_init(hdmi);
> +
> +	if (hdmi->phy.ops->setup_hpd)
> +		hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_resume);

Both patches look good to me, I'd probably prefer to just smash them together,
but meh.

If no one more authoritative chimes in, I'll apply them to -misc in a few days.

Sean

> +
>  MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
>  MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com>");
>  MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 66e70770cce5..c4132e9a5ae3 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -154,6 +154,9 @@ struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
>  			     struct drm_encoder *encoder,
>  			     const struct dw_hdmi_plat_data *plat_data);
>  
> +int dw_hdmi_suspend(struct dw_hdmi *hdmi);
> +int dw_hdmi_resume(struct dw_hdmi *hdmi);
> +
>  void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
>  
>  void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
> -- 
> 2.21.0.1020.gf2820cf01a-goog
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume

[Doug Anderson]

Hi,

On Wed, May 15, 2019 at 10:58 AM Sean Paul <sean@poorly.run> wrote:

> On Thu, May 02, 2019 at 03:38:07PM -0700, Douglas Anderson wrote:
> > On Rockchip rk3288-based Chromebooks when you do a suspend/resume
> > cycle:
> >
> > 1. You lose the ability to detect an HDMI device being plugged in.
> >
> > 2. If you're using the i2c bus built in to dw_hdmi then it stops
> > working.
> >
> > Let's add a hook to the core dw-hdmi driver so that we can call it in
> > dw_hdmi-rockchip in the next commit.
> >
> > NOTE: the exact set of steps I've done here in resume come from
> > looking at the normal dw_hdmi init sequence in upstream Linux plus the
> > sequence that we did in downstream Chrome OS 3.14.  Testing show that
> > it seems to work, but if an extra step is needed or something here is
> > not needed we could improve it.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 21 +++++++++++++++++++++
> >  include/drm/bridge/dw_hdmi.h              |  3 +++
> >  2 files changed, 24 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > index db761329a1e3..4b38bfd43e59 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -2780,6 +2780,27 @@ void dw_hdmi_unbind(struct dw_hdmi *hdmi)
> >  }
> >  EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
> >
> > +int dw_hdmi_suspend(struct dw_hdmi *hdmi)
> > +{
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(dw_hdmi_suspend);
> > +
> > +int dw_hdmi_resume(struct dw_hdmi *hdmi)
> > +{
> > +     initialize_hdmi_ih_mutes(hdmi);
> > +
> > +     dw_hdmi_setup_i2c(hdmi);
> > +     if (hdmi->i2c)
> > +             dw_hdmi_i2c_init(hdmi);
> > +
> > +     if (hdmi->phy.ops->setup_hpd)
> > +             hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
> > +
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(dw_hdmi_resume);
>
> Both patches look good to me, I'd probably prefer to just smash them together,
> but meh.
>
> If no one more authoritative chimes in, I'll apply them to -misc in a few days.

Sure.  I can smash them and re-post or you can smash them for me or we
can keep them as-is.  I originally separated because I wasn't sure if
they'd eventually go through different trees.  Just let me know!  :-)

-Doug

Re: [PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume

[Sean Paul]

On Wed, May 15, 2019 at 11:01:26AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Wed, May 15, 2019 at 10:58 AM Sean Paul <sean@poorly.run> wrote:
> 
> > On Thu, May 02, 2019 at 03:38:07PM -0700, Douglas Anderson wrote:
> > > On Rockchip rk3288-based Chromebooks when you do a suspend/resume
> > > cycle:
> > >
> > > 1. You lose the ability to detect an HDMI device being plugged in.
> > >
> > > 2. If you're using the i2c bus built in to dw_hdmi then it stops
> > > working.
> > >
> > > Let's add a hook to the core dw-hdmi driver so that we can call it in
> > > dw_hdmi-rockchip in the next commit.
> > >
> > > NOTE: the exact set of steps I've done here in resume come from
> > > looking at the normal dw_hdmi init sequence in upstream Linux plus the
> > > sequence that we did in downstream Chrome OS 3.14.  Testing show that
> > > it seems to work, but if an extra step is needed or something here is
> > > not needed we could improve it.
> > >
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > ---
> > >
> > >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 21 +++++++++++++++++++++
> > >  include/drm/bridge/dw_hdmi.h              |  3 +++
> > >  2 files changed, 24 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > index db761329a1e3..4b38bfd43e59 100644
> > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > @@ -2780,6 +2780,27 @@ void dw_hdmi_unbind(struct dw_hdmi *hdmi)
> > >  }
> > >  EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
> > >
> > > +int dw_hdmi_suspend(struct dw_hdmi *hdmi)
> > > +{
> > > +     return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(dw_hdmi_suspend);
> > > +
> > > +int dw_hdmi_resume(struct dw_hdmi *hdmi)
> > > +{
> > > +     initialize_hdmi_ih_mutes(hdmi);
> > > +
> > > +     dw_hdmi_setup_i2c(hdmi);
> > > +     if (hdmi->i2c)
> > > +             dw_hdmi_i2c_init(hdmi);
> > > +
> > > +     if (hdmi->phy.ops->setup_hpd)
> > > +             hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
> > > +
> > > +     return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(dw_hdmi_resume);
> >
> > Both patches look good to me, I'd probably prefer to just smash them together,
> > but meh.
> >
> > If no one more authoritative chimes in, I'll apply them to -misc in a few days.
> 
> Sure.  I can smash them and re-post or you can smash them for me or we
> can keep them as-is.  I originally separated because I wasn't sure if
> they'd eventually go through different trees.  Just let me know!  :-)

Definitely no need to repost. It's entirely possible Andrzej or Heiko prefer to
have the dw-hdmi stuff broken out anyways. My opinion is of little value here :)

Sean

> 
> -Doug

-- 
Sean Paul, Software Engineer, Google / Chromium OS

Re: [PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume

[Heiko Stübner]

Am Mittwoch, 15. Mai 2019, 20:05:03 CEST schrieb Sean Paul:
> On Wed, May 15, 2019 at 11:01:26AM -0700, Doug Anderson wrote:
> > Hi,
> > 
> > On Wed, May 15, 2019 at 10:58 AM Sean Paul <sean@poorly.run> wrote:
> > 
> > > On Thu, May 02, 2019 at 03:38:07PM -0700, Douglas Anderson wrote:
> > > > On Rockchip rk3288-based Chromebooks when you do a suspend/resume
> > > > cycle:
> > > >
> > > > 1. You lose the ability to detect an HDMI device being plugged in.
> > > >
> > > > 2. If you're using the i2c bus built in to dw_hdmi then it stops
> > > > working.
> > > >
> > > > Let's add a hook to the core dw-hdmi driver so that we can call it in
> > > > dw_hdmi-rockchip in the next commit.
> > > >
> > > > NOTE: the exact set of steps I've done here in resume come from
> > > > looking at the normal dw_hdmi init sequence in upstream Linux plus the
> > > > sequence that we did in downstream Chrome OS 3.14.  Testing show that
> > > > it seems to work, but if an extra step is needed or something here is
> > > > not needed we could improve it.
> > > >
> > > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > > ---
> > > >
> > > >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 21 +++++++++++++++++++++
> > > >  include/drm/bridge/dw_hdmi.h              |  3 +++
> > > >  2 files changed, 24 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > > index db761329a1e3..4b38bfd43e59 100644
> > > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > > @@ -2780,6 +2780,27 @@ void dw_hdmi_unbind(struct dw_hdmi *hdmi)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
> > > >
> > > > +int dw_hdmi_suspend(struct dw_hdmi *hdmi)
> > > > +{
> > > > +     return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(dw_hdmi_suspend);
> > > > +
> > > > +int dw_hdmi_resume(struct dw_hdmi *hdmi)
> > > > +{
> > > > +     initialize_hdmi_ih_mutes(hdmi);
> > > > +
> > > > +     dw_hdmi_setup_i2c(hdmi);
> > > > +     if (hdmi->i2c)
> > > > +             dw_hdmi_i2c_init(hdmi);
> > > > +
> > > > +     if (hdmi->phy.ops->setup_hpd)
> > > > +             hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(dw_hdmi_resume);
> > >
> > > Both patches look good to me, I'd probably prefer to just smash them together,
> > > but meh.
> > >
> > > If no one more authoritative chimes in, I'll apply them to -misc in a few days.
> > 
> > Sure.  I can smash them and re-post or you can smash them for me or we
> > can keep them as-is.  I originally separated because I wasn't sure if
> > they'd eventually go through different trees.  Just let me know!  :-)
> 
> Definitely no need to repost. It's entirely possible Andrzej or Heiko prefer to
> have the dw-hdmi stuff broken out anyways. My opinion is of little value here :)

I guess my own preference is to keep them as they are now - so separate.
It makes it easier to see what gets done and also keeps the boundary on
where to split pretty clear.


Heiko

Re: [PATCH 2/2] drm/rockchip: dw_hdmi: Handle suspend/resume

[Heiko Stübner]

Am Freitag, 3. Mai 2019, 00:38:08 CEST schrieb Douglas Anderson:
> On Rockchip rk3288-based Chromebooks when you do a suspend/resume
> cycle:
> 
> 1. You lose the ability to detect an HDMI device being plugged in.
> 
> 2. If you're using the i2c bus built in to dw_hdmi then it stops
> working.
> 
> Let's call the core dw-hdmi's suspend/resume functions to restore
> things.
> 
> NOTE: in downstream Chrome OS (based on kernel 3.14) we used the
> "late/early" versions of suspend/resume because we found that the VOP
> was sometimes resuming before dw_hdmi and then calling into us before
> we were fully resumed.  For now I have gone back to the normal
> suspend/resume because I can't reproduce the problems.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

Re: [PATCH 2/2] drm/rockchip: dw_hdmi: Handle suspend/resume

[Laurent Pinchart]

Hi Douglas,

Thank you for the patch.

On Thu, May 02, 2019 at 03:38:08PM -0700, Douglas Anderson wrote:
> On Rockchip rk3288-based Chromebooks when you do a suspend/resume
> cycle:
> 
> 1. You lose the ability to detect an HDMI device being plugged in.
> 
> 2. If you're using the i2c bus built in to dw_hdmi then it stops
> working.
> 
> Let's call the core dw-hdmi's suspend/resume functions to restore
> things.
> 
> NOTE: in downstream Chrome OS (based on kernel 3.14) we used the
> "late/early" versions of suspend/resume because we found that the VOP
> was sometimes resuming before dw_hdmi and then calling into us before
> we were fully resumed.  For now I have gone back to the normal
> suspend/resume because I can't reproduce the problems.

Should this be solved with device links if needed ?

> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 4cdc9f86c2e5..deb0e8c30c03 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -542,11 +542,31 @@ static int dw_hdmi_rockchip_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static int __maybe_unused dw_hdmi_rockchip_suspend(struct device *dev)
> +{
> +	struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +	return dw_hdmi_suspend(hdmi->hdmi);
> +}
> +
> +static int __maybe_unused dw_hdmi_rockchip_resume(struct device *dev)
> +{
> +	struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
> +
> +	return dw_hdmi_resume(hdmi->hdmi);
> +}
> +
> +const struct dev_pm_ops dw_hdmi_rockchip_pm = {

Missing static keyword ?

Apart from this,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	SET_SYSTEM_SLEEP_PM_OPS(dw_hdmi_rockchip_suspend,
> +				dw_hdmi_rockchip_resume)
> +};
> +
>  struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
>  	.probe  = dw_hdmi_rockchip_probe,
>  	.remove = dw_hdmi_rockchip_remove,
>  	.driver = {
>  		.name = "dwhdmi-rockchip",
> +		.pm = &dw_hdmi_rockchip_pm,
>  		.of_match_table = dw_hdmi_rockchip_dt_ids,
>  	},
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume

[Laurent Pinchart]

Hi Douglas,

Thank you for the patch.

On Thu, May 02, 2019 at 03:38:07PM -0700, Douglas Anderson wrote:
> On Rockchip rk3288-based Chromebooks when you do a suspend/resume
> cycle:
> 
> 1. You lose the ability to detect an HDMI device being plugged in.
> 
> 2. If you're using the i2c bus built in to dw_hdmi then it stops
> working.
> 
> Let's add a hook to the core dw-hdmi driver so that we can call it in
> dw_hdmi-rockchip in the next commit.
> 
> NOTE: the exact set of steps I've done here in resume come from
> looking at the normal dw_hdmi init sequence in upstream Linux plus the
> sequence that we did in downstream Chrome OS 3.14.  Testing show that
> it seems to work, but if an extra step is needed or something here is
> not needed we could improve it.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 21 +++++++++++++++++++++
>  include/drm/bridge/dw_hdmi.h              |  3 +++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index db761329a1e3..4b38bfd43e59 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2780,6 +2780,27 @@ void dw_hdmi_unbind(struct dw_hdmi *hdmi)
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
>  
> +int dw_hdmi_suspend(struct dw_hdmi *hdmi)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_suspend);
> +

As this is empty, should we leave it out ? It adds a bit of bloat to the
kernel for no real reason, and we can add it later if required.

> +int dw_hdmi_resume(struct dw_hdmi *hdmi)
> +{
> +	initialize_hdmi_ih_mutes(hdmi);
> +
> +	dw_hdmi_setup_i2c(hdmi);
> +	if (hdmi->i2c)
> +		dw_hdmi_i2c_init(hdmi);
> +
> +	if (hdmi->phy.ops->setup_hpd)
> +		hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
> +
> +	return 0;

How about refactoring the probe function to extract hardware
initialisation to a separate function, and calling it from here ?

> +}
> +EXPORT_SYMBOL_GPL(dw_hdmi_resume);
> +
>  MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
>  MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com>");
>  MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 66e70770cce5..c4132e9a5ae3 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -154,6 +154,9 @@ struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
>  			     struct drm_encoder *encoder,
>  			     const struct dw_hdmi_plat_data *plat_data);
>  
> +int dw_hdmi_suspend(struct dw_hdmi *hdmi);
> +int dw_hdmi_resume(struct dw_hdmi *hdmi);
> +
>  void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
>  
>  void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);

-- 
Regards,

Laurent Pinchart