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>
