[PATCHv2,01/31] drm/omap: HDMI: change enable/disable to avoid sync-losts

Submitted by Tomi Valkeinen on Feb. 26, 2016, 9:35 a.m.

Details

Message ID 1456479379-6086-2-git-send-email-tomi.valkeinen@ti.com
State New
Headers show
Series "drm/omap: patches for v4.6 part 1" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Tomi Valkeinen Feb. 26, 2016, 9:35 a.m.
We occasionally see DISPC sync-lost errors when enabling and disabling
HDMI. Sometimes we get only a few, which get handled (ignored) by the
driver, but sometimes there's a flood of the errors which doesn't seem
to stop.

The HW team has root caused this to the order in which HDMI and DISPC
are enabled/disabled. Currently we enable HDMI first, and then DISPC,
and vice versa when disabling. HW team's suggestion is to do it the
other way around.

This patch changes the order, but this has two side effects as the pixel
clock is produced by HDMI, and the clock is not running when we
enable/disable DISPC:

* When enabling DISPC first, we don't get vertical sync events
* When disabling DISPC last, we don't get FRAMEDONE event

At the moment we use both of those to verify that DISPC has been
enabled/disabled properly. Thus this patch also needs to change the
omapdrm and omapdss which handle the DISPC side.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/hdmi4.c | 16 ++++++++--------
 drivers/gpu/drm/omapdrm/dss/hdmi5.c | 16 ++++++++--------
 drivers/gpu/drm/omapdrm/omap_crtc.c |  5 +++++
 3 files changed, 21 insertions(+), 16 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index 7103c659a534..b09ce9ee82fa 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -214,22 +214,22 @@  static int hdmi_power_on_full(struct omap_dss_device *dssdev)
 	/* tv size */
 	dss_mgr_set_timings(mgr, p);
 
-	r = hdmi_wp_video_start(&hdmi.wp);
-	if (r)
-		goto err_vid_enable;
-
 	r = dss_mgr_enable(mgr);
 	if (r)
 		goto err_mgr_enable;
 
+	r = hdmi_wp_video_start(&hdmi.wp);
+	if (r)
+		goto err_vid_enable;
+
 	hdmi_wp_set_irqenable(wp,
 		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT);
 
 	return 0;
 
-err_mgr_enable:
-	hdmi_wp_video_stop(&hdmi.wp);
 err_vid_enable:
+	dss_mgr_disable(mgr);
+err_mgr_enable:
 	hdmi_wp_set_phy_pwr(&hdmi.wp, HDMI_PHYPWRCMD_OFF);
 err_phy_pwr:
 err_phy_cfg:
@@ -246,10 +246,10 @@  static void hdmi_power_off_full(struct omap_dss_device *dssdev)
 
 	hdmi_wp_clear_irqenable(&hdmi.wp, 0xffffffff);
 
-	dss_mgr_disable(mgr);
-
 	hdmi_wp_video_stop(&hdmi.wp);
 
+	dss_mgr_disable(mgr);
+
 	hdmi_wp_set_phy_pwr(&hdmi.wp, HDMI_PHYPWRCMD_OFF);
 
 	dss_pll_disable(&hdmi.pll.pll);
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
index a955a2c4c061..4485a1c37bd8 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
@@ -231,22 +231,22 @@  static int hdmi_power_on_full(struct omap_dss_device *dssdev)
 	/* tv size */
 	dss_mgr_set_timings(mgr, p);
 
-	r = hdmi_wp_video_start(&hdmi.wp);
-	if (r)
-		goto err_vid_enable;
-
 	r = dss_mgr_enable(mgr);
 	if (r)
 		goto err_mgr_enable;
 
+	r = hdmi_wp_video_start(&hdmi.wp);
+	if (r)
+		goto err_vid_enable;
+
 	hdmi_wp_set_irqenable(&hdmi.wp,
 			HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT);
 
 	return 0;
 
-err_mgr_enable:
-	hdmi_wp_video_stop(&hdmi.wp);
 err_vid_enable:
+	dss_mgr_disable(mgr);
+err_mgr_enable:
 	hdmi_wp_set_phy_pwr(&hdmi.wp, HDMI_PHYPWRCMD_OFF);
 err_phy_pwr:
 err_phy_cfg:
@@ -263,10 +263,10 @@  static void hdmi_power_off_full(struct omap_dss_device *dssdev)
 
 	hdmi_wp_clear_irqenable(&hdmi.wp, 0xffffffff);
 
-	dss_mgr_disable(mgr);
-
 	hdmi_wp_video_stop(&hdmi.wp);
 
+	dss_mgr_disable(mgr);
+
 	hdmi_wp_set_phy_pwr(&hdmi.wp, HDMI_PHYPWRCMD_OFF);
 
 	dss_pll_disable(&hdmi.pll.pll);
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 2ed0754ed19e..7dd3d44a93e5 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -138,6 +138,11 @@  static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 	u32 framedone_irq, vsync_irq;
 	int ret;
 
+	if (omap_crtc->mgr->output->output_type == OMAP_DISPLAY_TYPE_HDMI) {
+		dispc_mgr_enable(channel, enable);
+		return;
+	}
+
 	if (dispc_mgr_is_enabled(channel) == enable)
 		return;
 

Comments

Hi Tomi,

Thank you for the patch.

On Friday 26 February 2016 11:35:49 Tomi Valkeinen wrote:
> We occasionally see DISPC sync-lost errors when enabling and disabling
> HDMI. Sometimes we get only a few, which get handled (ignored) by the
> driver, but sometimes there's a flood of the errors which doesn't seem
> to stop.
> 
> The HW team has root caused this to the order in which HDMI and DISPC
> are enabled/disabled. Currently we enable HDMI first, and then DISPC,
> and vice versa when disabling. HW team's suggestion is to do it the
> other way around.
> 
> This patch changes the order, but this has two side effects as the pixel
> clock is produced by HDMI, and the clock is not running when we
> enable/disable DISPC:
> 
> * When enabling DISPC first, we don't get vertical sync events
> * When disabling DISPC last, we don't get FRAMEDONE event
> 
> At the moment we use both of those to verify that DISPC has been
> enabled/disabled properly. Thus this patch also needs to change the
> omapdrm and omapdss which handle the DISPC side.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

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

> ---
>  drivers/gpu/drm/omapdrm/dss/hdmi4.c | 16 ++++++++--------
>  drivers/gpu/drm/omapdrm/dss/hdmi5.c | 16 ++++++++--------
>  drivers/gpu/drm/omapdrm/omap_crtc.c |  5 +++++
>  3 files changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> b/drivers/gpu/drm/omapdrm/dss/hdmi4.c index 7103c659a534..b09ce9ee82fa
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> @@ -214,22 +214,22 @@ static int hdmi_power_on_full(struct omap_dss_device
> *dssdev) /* tv size */
>  	dss_mgr_set_timings(mgr, p);
> 
> -	r = hdmi_wp_video_start(&hdmi.wp);
> -	if (r)
> -		goto err_vid_enable;
> -
>  	r = dss_mgr_enable(mgr);
>  	if (r)
>  		goto err_mgr_enable;
> 
> +	r = hdmi_wp_video_start(&hdmi.wp);
> +	if (r)
> +		goto err_vid_enable;
> +
>  	hdmi_wp_set_irqenable(wp,
>  		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT);
> 
>  	return 0;
> 
> -err_mgr_enable:
> -	hdmi_wp_video_stop(&hdmi.wp);
>  err_vid_enable:
> +	dss_mgr_disable(mgr);
> +err_mgr_enable:
>  	hdmi_wp_set_phy_pwr(&hdmi.wp, HDMI_PHYPWRCMD_OFF);
>  err_phy_pwr:
>  err_phy_cfg:
> @@ -246,10 +246,10 @@ static void hdmi_power_off_full(struct omap_dss_device
> *dssdev)
> 
>  	hdmi_wp_clear_irqenable(&hdmi.wp, 0xffffffff);
> 
> -	dss_mgr_disable(mgr);
> -
>  	hdmi_wp_video_stop(&hdmi.wp);
> 
> +	dss_mgr_disable(mgr);
> +
>  	hdmi_wp_set_phy_pwr(&hdmi.wp, HDMI_PHYPWRCMD_OFF);
> 
>  	dss_pll_disable(&hdmi.pll.pll);
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> b/drivers/gpu/drm/omapdrm/dss/hdmi5.c index a955a2c4c061..4485a1c37bd8
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> @@ -231,22 +231,22 @@ static int hdmi_power_on_full(struct omap_dss_device
> *dssdev) /* tv size */
>  	dss_mgr_set_timings(mgr, p);
> 
> -	r = hdmi_wp_video_start(&hdmi.wp);
> -	if (r)
> -		goto err_vid_enable;
> -
>  	r = dss_mgr_enable(mgr);
>  	if (r)
>  		goto err_mgr_enable;
> 
> +	r = hdmi_wp_video_start(&hdmi.wp);
> +	if (r)
> +		goto err_vid_enable;
> +
>  	hdmi_wp_set_irqenable(&hdmi.wp,
>  			HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT);
> 
>  	return 0;
> 
> -err_mgr_enable:
> -	hdmi_wp_video_stop(&hdmi.wp);
>  err_vid_enable:
> +	dss_mgr_disable(mgr);
> +err_mgr_enable:
>  	hdmi_wp_set_phy_pwr(&hdmi.wp, HDMI_PHYPWRCMD_OFF);
>  err_phy_pwr:
>  err_phy_cfg:
> @@ -263,10 +263,10 @@ static void hdmi_power_off_full(struct omap_dss_device
> *dssdev)
> 
>  	hdmi_wp_clear_irqenable(&hdmi.wp, 0xffffffff);
> 
> -	dss_mgr_disable(mgr);
> -
>  	hdmi_wp_video_stop(&hdmi.wp);
> 
> +	dss_mgr_disable(mgr);
> +
>  	hdmi_wp_set_phy_pwr(&hdmi.wp, HDMI_PHYPWRCMD_OFF);
> 
>  	dss_pll_disable(&hdmi.pll.pll);
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c index 2ed0754ed19e..7dd3d44a93e5
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -138,6 +138,11 @@ static void omap_crtc_set_enabled(struct drm_crtc
> *crtc, bool enable) u32 framedone_irq, vsync_irq;
>  	int ret;
> 
> +	if (omap_crtc->mgr->output->output_type == OMAP_DISPLAY_TYPE_HDMI) {
> +		dispc_mgr_enable(channel, enable);
> +		return;
> +	}
> +
>  	if (dispc_mgr_is_enabled(channel) == enable)
>  		return;