All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] drm: bridge: dw-hdmi: Add hook for resume
@ 2019-05-16 21:40 Douglas Anderson
  2019-05-16 21:40   ` Douglas Anderson
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Douglas Anderson @ 2019-05-16 21:40 UTC (permalink / raw)
  To: Heiko Stuebner, Sandy Huang, Andrzej Hajda, Laurent Pinchart
  Cc: linux-rockchip, Neil Armstrong, mka, Sean Paul, Douglas Anderson,
	Jonas Karlman, Sam Ravnborg, dri-devel, linux-kernel,
	Ville Syrjälä,
	David Airlie, 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.

As part of this change we'll refactor the hardware init bits of
dw-hdmi to happen all in one function and all at the same time.  Since
we need to init the interrupt mutes before we request the IRQ, this
means moving the hardware init earlier in the function, but there
should be no problems with that.  Also as part of this we now
unconditionally init the "i2c" parts of dw-hdmi, but again that ought
to be fine.

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

Changes in v2:
- No empty stub for suspend (Laurent)
- Refactor to use the same code in probe and resume (Laurent)
- Unconditionally init i2c (seems OK + needed before hdmi->i2c init)
- Combine "init" of i2c and "setup" of i2c (no reason to split)

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 50 ++++++++++++++---------
 include/drm/bridge/dw_hdmi.h              |  2 +
 2 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ab7968c8f6a2..636d55d1398c 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -227,6 +227,13 @@ static void hdmi_mask_writeb(struct dw_hdmi *hdmi, u8 data, unsigned int reg,
 
 static void dw_hdmi_i2c_init(struct dw_hdmi *hdmi)
 {
+	hdmi_writeb(hdmi, HDMI_PHY_I2CM_INT_ADDR_DONE_POL,
+		    HDMI_PHY_I2CM_INT_ADDR);
+
+	hdmi_writeb(hdmi, HDMI_PHY_I2CM_CTLINT_ADDR_NAC_POL |
+		    HDMI_PHY_I2CM_CTLINT_ADDR_ARBITRATION_POL,
+		    HDMI_PHY_I2CM_CTLINT_ADDR);
+
 	/* Software reset */
 	hdmi_writeb(hdmi, 0x00, HDMI_I2CM_SOFTRSTZ);
 
@@ -1925,16 +1932,6 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 	return 0;
 }
 
-static void dw_hdmi_setup_i2c(struct dw_hdmi *hdmi)
-{
-	hdmi_writeb(hdmi, HDMI_PHY_I2CM_INT_ADDR_DONE_POL,
-		    HDMI_PHY_I2CM_INT_ADDR);
-
-	hdmi_writeb(hdmi, HDMI_PHY_I2CM_CTLINT_ADDR_NAC_POL |
-		    HDMI_PHY_I2CM_CTLINT_ADDR_ARBITRATION_POL,
-		    HDMI_PHY_I2CM_CTLINT_ADDR);
-}
-
 static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi)
 {
 	u8 ih_mute;
@@ -2435,6 +2432,21 @@ static const struct regmap_config hdmi_regmap_32bit_config = {
 	.max_register	= HDMI_I2CM_FS_SCL_LCNT_0_ADDR << 2,
 };
 
+static void dw_hdmi_init_hw(struct dw_hdmi *hdmi)
+{
+	initialize_hdmi_ih_mutes(hdmi);
+
+	/*
+	 * Reset HDMI DDC I2C master controller and mute I2CM interrupts.
+	 * Even if we are using a separate i2c adapter doing this doesn't
+	 * hurt.
+	 */
+	dw_hdmi_i2c_init(hdmi);
+
+	if (hdmi->phy.ops->setup_hpd)
+		hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
+}
+
 static struct dw_hdmi *
 __dw_hdmi_probe(struct platform_device *pdev,
 		const struct dw_hdmi_plat_data *plat_data)
@@ -2586,7 +2598,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		 prod_id1 & HDMI_PRODUCT_ID1_HDCP ? "with" : "without",
 		 hdmi->phy.name);
 
-	initialize_hdmi_ih_mutes(hdmi);
+	dw_hdmi_init_hw(hdmi);
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
@@ -2625,10 +2637,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	hdmi->bridge.of_node = pdev->dev.of_node;
 #endif
 
-	dw_hdmi_setup_i2c(hdmi);
-	if (hdmi->phy.ops->setup_hpd)
-		hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
-
 	memset(&pdevinfo, 0, sizeof(pdevinfo));
 	pdevinfo.parent = dev;
 	pdevinfo.id = PLATFORM_DEVID_AUTO;
@@ -2681,10 +2689,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		hdmi->cec = platform_device_register_full(&pdevinfo);
 	}
 
-	/* Reset HDMI DDC I2C master controller and mute I2CM interrupts */
-	if (hdmi->i2c)
-		dw_hdmi_i2c_init(hdmi);
-
 	return hdmi;
 
 err_iahb:
@@ -2788,6 +2792,14 @@ void dw_hdmi_unbind(struct dw_hdmi *hdmi)
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
 
+int dw_hdmi_resume(struct dw_hdmi *hdmi)
+{
+	dw_hdmi_init_hw(hdmi);
+
+	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..1626731e1681 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -154,6 +154,8 @@ 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_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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] drm/rockchip: dw_hdmi: Handle suspend/resume
  2019-05-16 21:40 [PATCH v2 1/2] drm: bridge: dw-hdmi: Add hook for resume Douglas Anderson
@ 2019-05-16 21:40   ` Douglas Anderson
  2019-06-03 21:37   ` Doug Anderson
  2019-06-04 12:49 ` Laurent Pinchart
  2 siblings, 0 replies; 8+ messages in thread
From: Douglas Anderson @ 2019-05-16 21:40 UTC (permalink / raw)
  To: Heiko Stuebner, Sandy Huang, Andrzej Hajda, Laurent Pinchart
  Cc: linux-rockchip, Neil Armstrong, mka, Sean Paul, Douglas Anderson,
	linux-kernel, dri-devel, David Airlie, linux-arm-kernel,
	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 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>
---

Changes in v2:
- Add forgotten static (Laurent)
- No empty stub for suspend (Laurent)

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

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 4cdc9f86c2e5..beffe44c248a 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -542,11 +542,23 @@ static int dw_hdmi_rockchip_remove(struct platform_device *pdev)
 	return 0;
 }
 
+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);
+}
+
+static const struct dev_pm_ops dw_hdmi_rockchip_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(NULL, 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] drm/rockchip: dw_hdmi: Handle suspend/resume
@ 2019-05-16 21:40   ` Douglas Anderson
  0 siblings, 0 replies; 8+ messages in thread
From: Douglas Anderson @ 2019-05-16 21:40 UTC (permalink / raw)
  To: Heiko Stuebner, Sandy Huang, Andrzej Hajda, Laurent Pinchart
  Cc: Neil Armstrong, David Airlie, Douglas Anderson, dri-devel,
	linux-kernel, linux-rockchip, mka, Sean Paul, Daniel Vetter,
	linux-arm-kernel

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>
---

Changes in v2:
- Add forgotten static (Laurent)
- No empty stub for suspend (Laurent)

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

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 4cdc9f86c2e5..beffe44c248a 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -542,11 +542,23 @@ static int dw_hdmi_rockchip_remove(struct platform_device *pdev)
 	return 0;
 }
 
+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);
+}
+
+static const struct dev_pm_ops dw_hdmi_rockchip_pm = {
+	SET_SYSTEM_SLEEP_PM_OPS(NULL, 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


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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] drm/rockchip: dw_hdmi: Handle suspend/resume
  2019-05-16 21:40   ` Douglas Anderson
@ 2019-05-16 22:44     ` Doug Anderson
  -1 siblings, 0 replies; 8+ messages in thread
From: Doug Anderson @ 2019-05-16 22:44 UTC (permalink / raw)
  To: Heiko Stuebner, Sandy Huang, Andrzej Hajda, Laurent Pinchart
  Cc: open list:ARM/Rockchip SoC...,
	Neil Armstrong, Matthias Kaehlcke, Sean Paul, LKML, dri-devel,
	David Airlie, Linux ARM, Daniel Vetter

Hi,

On Thu, May 16, 2019 at 2:40 PM Douglas Anderson <dianders@chromium.org> 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.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v2:
> - Add forgotten static (Laurent)
> - No empty stub for suspend (Laurent)
>
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Whoops, forgot that I should have carried forward:

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

-Doug

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] drm/rockchip: dw_hdmi: Handle suspend/resume
@ 2019-05-16 22:44     ` Doug Anderson
  0 siblings, 0 replies; 8+ messages in thread
From: Doug Anderson @ 2019-05-16 22:44 UTC (permalink / raw)
  To: Heiko Stuebner, Sandy Huang, Andrzej Hajda, Laurent Pinchart
  Cc: Neil Armstrong, David Airlie, LKML, dri-devel,
	open list:ARM/Rockchip SoC...,
	Matthias Kaehlcke, Sean Paul, Daniel Vetter, Linux ARM

Hi,

On Thu, May 16, 2019 at 2:40 PM Douglas Anderson <dianders@chromium.org> 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.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v2:
> - Add forgotten static (Laurent)
> - No empty stub for suspend (Laurent)
>
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Whoops, forgot that I should have carried forward:

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

-Doug

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drm: bridge: dw-hdmi: Add hook for resume
@ 2019-06-03 21:37   ` Doug Anderson
  0 siblings, 0 replies; 8+ messages in thread
From: Doug Anderson @ 2019-06-03 21:37 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: open list:ARM/Rockchip SoC...,
	Neil Armstrong, Matthias Kaehlcke, Sean Paul, Jonas Karlman,
	Sam Ravnborg, dri-devel, LKML, Ville Syrjälä,
	David Airlie, Daniel Vetter, Heiko Stuebner, Andrzej Hajda,
	Sandy Huang

Laurent,

On Thu, May 16, 2019 at 2:40 PM Douglas Anderson <dianders@chromium.org> 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.
>
> As part of this change we'll refactor the hardware init bits of
> dw-hdmi to happen all in one function and all at the same time.  Since
> we need to init the interrupt mutes before we request the IRQ, this
> means moving the hardware init earlier in the function, but there
> should be no problems with that.  Also as part of this we now
> unconditionally init the "i2c" parts of dw-hdmi, but again that ought
> to be fine.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v2:
> - No empty stub for suspend (Laurent)
> - Refactor to use the same code in probe and resume (Laurent)
> - Unconditionally init i2c (seems OK + needed before hdmi->i2c init)
> - Combine "init" of i2c and "setup" of i2c (no reason to split)

Are you happy with this now?  Even if you feel like you don't want to
give it a full Reviewed-by, it'd good if you could confirm that I
handled your suggestions properly.

Thanks!  :-)

-Doug

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drm: bridge: dw-hdmi: Add hook for resume
@ 2019-06-03 21:37   ` Doug Anderson
  0 siblings, 0 replies; 8+ messages in thread
From: Doug Anderson @ 2019-06-03 21:37 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Heiko Stuebner, Jonas Karlman, David Airlie, Neil Armstrong,
	LKML, dri-devel, Sandy Huang, Andrzej Hajda,
	open list:ARM/Rockchip SoC...,
	Matthias Kaehlcke, Sean Paul, Daniel Vetter, Sam Ravnborg,
	Ville Syrjälä

Laurent,

On Thu, May 16, 2019 at 2:40 PM Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 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.
>
> As part of this change we'll refactor the hardware init bits of
> dw-hdmi to happen all in one function and all at the same time.  Since
> we need to init the interrupt mutes before we request the IRQ, this
> means moving the hardware init earlier in the function, but there
> should be no problems with that.  Also as part of this we now
> unconditionally init the "i2c" parts of dw-hdmi, but again that ought
> to be fine.
>
> Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>
> Changes in v2:
> - No empty stub for suspend (Laurent)
> - Refactor to use the same code in probe and resume (Laurent)
> - Unconditionally init i2c (seems OK + needed before hdmi->i2c init)
> - Combine "init" of i2c and "setup" of i2c (no reason to split)

Are you happy with this now?  Even if you feel like you don't want to
give it a full Reviewed-by, it'd good if you could confirm that I
handled your suggestions properly.

Thanks!  :-)

-Doug

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] drm: bridge: dw-hdmi: Add hook for resume
  2019-05-16 21:40 [PATCH v2 1/2] drm: bridge: dw-hdmi: Add hook for resume Douglas Anderson
  2019-05-16 21:40   ` Douglas Anderson
  2019-06-03 21:37   ` Doug Anderson
@ 2019-06-04 12:49 ` Laurent Pinchart
  2 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2019-06-04 12:49 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Heiko Stuebner, Sandy Huang, Andrzej Hajda, Jonas Karlman,
	David Airlie, Neil Armstrong, dri-devel, linux-kernel,
	linux-rockchip, mka, Sean Paul, Sam Ravnborg

Hi Douglas,

Thank you for the patch.

On Thu, May 16, 2019 at 02:40:21PM -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.
> 
> As part of this change we'll refactor the hardware init bits of
> dw-hdmi to happen all in one function and all at the same time.  Since
> we need to init the interrupt mutes before we request the IRQ, this
> means moving the hardware init earlier in the function, but there
> should be no problems with that.  Also as part of this we now
> unconditionally init the "i2c" parts of dw-hdmi, but again that ought
> to be fine.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
> Changes in v2:
> - No empty stub for suspend (Laurent)
> - Refactor to use the same code in probe and resume (Laurent)
> - Unconditionally init i2c (seems OK + needed before hdmi->i2c init)
> - Combine "init" of i2c and "setup" of i2c (no reason to split)
> 
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 50 ++++++++++++++---------
>  include/drm/bridge/dw_hdmi.h              |  2 +
>  2 files changed, 33 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index ab7968c8f6a2..636d55d1398c 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -227,6 +227,13 @@ static void hdmi_mask_writeb(struct dw_hdmi *hdmi, u8 data, unsigned int reg,
>  
>  static void dw_hdmi_i2c_init(struct dw_hdmi *hdmi)
>  {
> +	hdmi_writeb(hdmi, HDMI_PHY_I2CM_INT_ADDR_DONE_POL,
> +		    HDMI_PHY_I2CM_INT_ADDR);
> +
> +	hdmi_writeb(hdmi, HDMI_PHY_I2CM_CTLINT_ADDR_NAC_POL |
> +		    HDMI_PHY_I2CM_CTLINT_ADDR_ARBITRATION_POL,
> +		    HDMI_PHY_I2CM_CTLINT_ADDR);
> +
>  	/* Software reset */
>  	hdmi_writeb(hdmi, 0x00, HDMI_I2CM_SOFTRSTZ);
>  
> @@ -1925,16 +1932,6 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>  	return 0;
>  }
>  
> -static void dw_hdmi_setup_i2c(struct dw_hdmi *hdmi)
> -{
> -	hdmi_writeb(hdmi, HDMI_PHY_I2CM_INT_ADDR_DONE_POL,
> -		    HDMI_PHY_I2CM_INT_ADDR);
> -
> -	hdmi_writeb(hdmi, HDMI_PHY_I2CM_CTLINT_ADDR_NAC_POL |
> -		    HDMI_PHY_I2CM_CTLINT_ADDR_ARBITRATION_POL,
> -		    HDMI_PHY_I2CM_CTLINT_ADDR);
> -}
> -
>  static void initialize_hdmi_ih_mutes(struct dw_hdmi *hdmi)
>  {
>  	u8 ih_mute;
> @@ -2435,6 +2432,21 @@ static const struct regmap_config hdmi_regmap_32bit_config = {
>  	.max_register	= HDMI_I2CM_FS_SCL_LCNT_0_ADDR << 2,
>  };
>  
> +static void dw_hdmi_init_hw(struct dw_hdmi *hdmi)
> +{
> +	initialize_hdmi_ih_mutes(hdmi);
> +
> +	/*
> +	 * Reset HDMI DDC I2C master controller and mute I2CM interrupts.
> +	 * Even if we are using a separate i2c adapter doing this doesn't
> +	 * hurt.
> +	 */
> +	dw_hdmi_i2c_init(hdmi);
> +
> +	if (hdmi->phy.ops->setup_hpd)
> +		hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
> +}
> +
>  static struct dw_hdmi *
>  __dw_hdmi_probe(struct platform_device *pdev,
>  		const struct dw_hdmi_plat_data *plat_data)
> @@ -2586,7 +2598,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		 prod_id1 & HDMI_PRODUCT_ID1_HDCP ? "with" : "without",
>  		 hdmi->phy.name);
>  
> -	initialize_hdmi_ih_mutes(hdmi);
> +	dw_hdmi_init_hw(hdmi);
>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
> @@ -2625,10 +2637,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  	hdmi->bridge.of_node = pdev->dev.of_node;
>  #endif
>  
> -	dw_hdmi_setup_i2c(hdmi);
> -	if (hdmi->phy.ops->setup_hpd)
> -		hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data);
> -
>  	memset(&pdevinfo, 0, sizeof(pdevinfo));
>  	pdevinfo.parent = dev;
>  	pdevinfo.id = PLATFORM_DEVID_AUTO;
> @@ -2681,10 +2689,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		hdmi->cec = platform_device_register_full(&pdevinfo);
>  	}
>  
> -	/* Reset HDMI DDC I2C master controller and mute I2CM interrupts */
> -	if (hdmi->i2c)
> -		dw_hdmi_i2c_init(hdmi);
> -
>  	return hdmi;
>  
>  err_iahb:
> @@ -2788,6 +2792,14 @@ void dw_hdmi_unbind(struct dw_hdmi *hdmi)
>  }
>  EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
>  
> +int dw_hdmi_resume(struct dw_hdmi *hdmi)
> +{
> +	dw_hdmi_init_hw(hdmi);
> +
> +	return 0;
> +}

If the function always returns 0 I would make it void. Apart from that,

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

> +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..1626731e1681 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -154,6 +154,8 @@ 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_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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-06-04 12:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-16 21:40 [PATCH v2 1/2] drm: bridge: dw-hdmi: Add hook for resume Douglas Anderson
2019-05-16 21:40 ` [PATCH v2 2/2] drm/rockchip: dw_hdmi: Handle suspend/resume Douglas Anderson
2019-05-16 21:40   ` Douglas Anderson
2019-05-16 22:44   ` Doug Anderson
2019-05-16 22:44     ` Doug Anderson
2019-06-03 21:37 ` [PATCH v2 1/2] drm: bridge: dw-hdmi: Add hook for resume Doug Anderson
2019-06-03 21:37   ` Doug Anderson
2019-06-04 12:49 ` Laurent Pinchart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.