[PATCH 2/3] drm/omap: fix enabling/disabling of video pipeline

[Rob Clark]

On Thu, Apr 3, 2014 at 9:45 AM, Tomi Valkeinen <tomi.valkeinen at ti.com> wrote:
> 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 at ti.com>

I had to go back to look at the code to realize the extra
'omap_crtc->full_update = false' removal didn't actually matter (it
was redundant).  So

Reviewed-by: Rob Clark <robdclark at gmail.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;
> --
> 1.8.3.2
>