Hi Tomi,
Thank you for the patch.
On Friday 19 February 2016 11:47:36 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 at 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(-)
>
> 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;
> + }
This effectively bypasses the wait until the DISPC outputs the first vsync
interrupt below. How does HDMI differ from other outputs in such a way to make
the wait unneeded ?
> if (dispc_mgr_is_enabled(channel) == enable)
> return;
--
Regards,
Laurent Pinchart
