At the moment the omap_crtc_pre_apply() handles the enabling, disabling and configuring of encoders and panels separately from the CRTC (i.e. the overlay manager).

However, this doesn't work correctly. The encoder driver has to be in control of its video input (i.e. the crtc) for correct operation.

This problem causes bugs with (at least) HDMI: the HDMI encoder supplies pixel clock for DISPC, and DISPC supplies video stream for HDMI. The current code first enables the HDMI encoder, and CRTC after that. However, the encoder expects the video stream to start during the encoder's enable, and if it doesn't, there will be sync lost errors.

The encoder enables its video source by calling src->enable(), and this call goes to omapdrm (omap_crtc_enable), but omapdrm doesn't do anything in that function. Similarly for disable, which goes to omap_crtc_disable().

This patch moves the code to setup and enable/disable the crtc to omap_crtc_enable. and omap_crtc_disable().

Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com --- drivers/gpu/drm/omapdrm/omap_crtc.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index beccff2ccf84..90916abb66ef 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -121,13 +121,25 @@ static void omap_crtc_start_update(struct omap_overlay_manager *mgr) { }

+static void set_enabled(struct drm_crtc *crtc, bool enable); + static int omap_crtc_enable(struct omap_overlay_manager *mgr) { + struct omap_crtc *omap_crtc = omap_crtcs[mgr->id]; + + dispc_mgr_setup(omap_crtc->channel, &omap_crtc->info); + dispc_mgr_set_timings(omap_crtc->channel, + &omap_crtc->timings); + set_enabled(&omap_crtc->base, true); + return 0; }

static void omap_crtc_disable(struct omap_overlay_manager *mgr) { + struct omap_crtc *omap_crtc = omap_crtcs[mgr->id]; + + set_enabled(&omap_crtc->base, false); }

static void omap_crtc_set_timings(struct omap_overlay_manager *mgr, @@ -600,7 +612,6 @@ static void omap_crtc_pre_apply(struct omap_drm_apply *apply) omap_crtc->current_encoder = encoder;

if (!omap_crtc->enabled) { - set_enabled(&omap_crtc->base, false); if (encoder) omap_encoder_set_enabled(encoder, false); } else { @@ -609,13 +620,7 @@ static void omap_crtc_pre_apply(struct omap_drm_apply *apply) omap_encoder_update(encoder, omap_crtc->mgr, &omap_crtc->timings); omap_encoder_set_enabled(encoder, true); - omap_crtc->full_update = false; } - - dispc_mgr_setup(omap_crtc->channel, &omap_crtc->info); - dispc_mgr_set_timings(omap_crtc->channel, - &omap_crtc->timings); - set_enabled(&omap_crtc->base, true); }

omap_crtc->full_update = false;

On Thu, Apr 3, 2014 at 9:45 AM, Tomi Valkeinen tomi.valkeinen@ti.com wrote:

When an encoder is no longer connected to a crtc, the driver will leave the encoder enabled.

This patch adds code to track the encoder used for a crtc, and when the encoder changes, the old one is disabled.

Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com

Reviewed-by: Rob Clark robdclark@gmail.com

---

drivers/gpu/drm/omapdrm/omap_crtc.c | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c index 4f624c59a660..beccff2ccf84 100644 --- a/drivers/gpu/drm/omapdrm/omap_crtc.c +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c @@ -33,6 +33,7 @@ struct omap_crtc { int pipe; enum omap_channel channel; struct omap_overlay_manager_info info;

•   struct drm_encoder *current_encoder;
  
    /*
     * Temporary: eventually this will go away, but it is needed
  

@@ -593,6 +594,11 @@ static void omap_crtc_pre_apply(struct omap_drm_apply *apply) } }

•   if (omap_crtc->current_encoder && encoder != omap_crtc->current_encoder)
  

•           omap_encoder_set_enabled(omap_crtc->current_encoder, false);
  

•   omap_crtc->current_encoder = encoder;
  

•   if (!omap_crtc->enabled) {
            set_enabled(&omap_crtc->base, false);
            if (encoder)
  

-- 1.8.3.2