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

[Tomi Valkeinen]

On 03/04/14 17:58, Rob Clark wrote:
> 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

The final 'omap_crtc->full_update = false;' is visible in the patch
below also, so you didn't need to go look at the code ;)

Looking at it, that removal is actually extra, not exactly part of this fix.

> Reviewed-by: Rob Clark <robdclark at gmail.com>

Thanks!

 Tom

> 
>> ---
>>  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
>>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140403/48ff0bb2/attachment-0001.sig>