On Thu, Apr 3, 2014 at 9:45 AM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> 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@xxxxxx>
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@xxxxxxxxx>
> ---
> 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
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
