[PATCH] OMAPDSS: HACK: Ensure DSS clock domain gets out of idle when HDMI is enabled

For DSS clock domain to transition from idle to active state, it's necessary
to enable the optional clock DSS_FCLK before we enable the module using the
MODULEMODE bits in the DSS clock domain's CM_DSS_DSS_CLKCTRL register.

This sequence was not followed correctly for the 'dss_hdmi' hwmod and it led
to DSS clock domain not getting out of idle when pm_runtime_get_sync() was
called for hdmi's platform device.

Since the clock domain failed to change it's state to active, the hwmod code
disables any clocks it had enabled before for this hwmod. This led to the clock
'dss_48mhz_clk' getting disabled.

When hdmi's runtime_resume() op is called, the call to dss_runtime_get()
correctly enables the DSS clock domain this time. But the clock 'dss_48mhz_clk'
disabled before is needed for HDMI's PHY to function. Hence, the driver fails
when it tries to enable HDMI's PHY.

Fix this for now by ensuring that dss_runtime_get() is called before we call
pm_runtime_get_sync() for hdmi's platform device. A correct fix for later would
be to modify the DSS related hwmod's mainclks, and also some changes in how
opt clocks are handled in the DSS driver.

This fixes the issue of HDMI not working when it's the default display. The
issue is not seen if any other display is already enabled as the first display
would have already correctly enabled the DSS clock domain.

Signed-off-by: Archit Taneja <redacted>
---
 drivers/video/omap2/dss/hdmi.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index d7aa3b0..ce16a91 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c

@@ -165,9 +165,21 @@ static int hdmi_runtime_get(void)
 
 	DSSDBG("hdmi_runtime_get\n");
 
+	r = dss_runtime_get();
+	if (r < 0)
+		goto err_get_dss;
+
 	r = pm_runtime_get_sync(&hdmi.pdev->dev);
 	WARN_ON(r < 0);
-	return r < 0 ? r : 0;
+	if (r < 0)
+		goto err_get_hdmi;
+
+	return 0;
+
+err_get_hdmi:
+	dss_runtime_put();
+err_get_dss:
+	return r;
 }
 
 static void hdmi_runtime_put(void)

@@ -178,6 +190,8 @@ static void hdmi_runtime_put(void)
 
 	r = pm_runtime_put_sync(&hdmi.pdev->dev);
 	WARN_ON(r < 0);
+
+	dss_runtime_put();
 }
 
 int hdmi_init_display(struct omap_dss_device *dssdev)

-- 
1.7.5.4

Hi,

On Thu, 2012-02-09 at 12:14 +0530, Archit Taneja wrote:

For DSS clock domain to transition from idle to active state, it's necessary
to enable the optional clock DSS_FCLK before we enable the module using the
MODULEMODE bits in the DSS clock domain's CM_DSS_DSS_CLKCTRL register.

This sequence was not followed correctly for the 'dss_hdmi' hwmod and it led
to DSS clock domain not getting out of idle when pm_runtime_get_sync() was
called for hdmi's platform device.

Since the clock domain failed to change it's state to active, the hwmod code
disables any clocks it had enabled before for this hwmod. This led to the clock
'dss_48mhz_clk' getting disabled.

When hdmi's runtime_resume() op is called, the call to dss_runtime_get()
correctly enables the DSS clock domain this time. But the clock 'dss_48mhz_clk'
disabled before is needed for HDMI's PHY to function. Hence, the driver fails

There's something wrong with the "But the clock..." sentence above.

The patch looks good, but I think it'd be better to add brief HACK
comments in the code also. Otherwise it's too easy to forget about this.

 Tomi

On Thursday 09 February 2012 05:32 PM, Tomi Valkeinen wrote:

Hi,

On Thu, 2012-02-09 at 12:14 +0530, Archit Taneja wrote:

quoted

For DSS clock domain to transition from idle to active state, it's necessary
to enable the optional clock DSS_FCLK before we enable the module using the
MODULEMODE bits in the DSS clock domain's CM_DSS_DSS_CLKCTRL register.

This sequence was not followed correctly for the 'dss_hdmi' hwmod and it led
to DSS clock domain not getting out of idle when pm_runtime_get_sync() was
called for hdmi's platform device.

Since the clock domain failed to change it's state to active, the hwmod code
disables any clocks it had enabled before for this hwmod. This led to the clock
'dss_48mhz_clk' getting disabled.

When hdmi's runtime_resume() op is called, the call to dss_runtime_get()
correctly enables the DSS clock domain this time. But the clock 'dss_48mhz_clk'
disabled before is needed for HDMI's PHY to function. Hence, the driver fails

There's something wrong with the "But the clock..." sentence above.

The patch looks good, but I think it'd be better to add brief HACK
comments in the code also. Otherwise it's too easy to forget about this.

I'll make the changes and repost.

Archit

  Tomi