Re: [PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume
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
> ---
>
> 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 ");
> MODULE_AUTHOR("Andy Yan ");
> MODULE_AUTHOR("Yakir Yang ");
> 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume
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 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
> > > > ---
> > > >
> > > > 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 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume
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 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
> > > ---
> > >
> > > 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume
Hi,
On Wed, May 15, 2019 at 10:58 AM Sean Paul 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
> > ---
> >
> > 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume
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
> ---
>
> 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 ");
> MODULE_AUTHOR("Andy Yan ");
> MODULE_AUTHOR("Yakir Yang ");
> 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
Hi From: Douglas Anderson Date: Thu, May 2, 2019 at 3:38 PM To: Heiko Stuebner, Sandy Huang, Andrzej Hajda, Laurent Pinchart Cc: , Neil Armstrong, , Sean Paul, Douglas Anderson, Zheng Yang, Sam Ravnborg, , , 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 > --- > > 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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: bridge: dw-hdmi: Add hooks for suspend/resume
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
---
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 ");
MODULE_AUTHOR("Andy Yan ");
MODULE_AUTHOR("Yakir Yang ");
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
