>-----Original Message-----
>From: Nikula, Jani
>Sent: Wednesday, September 23, 2015 6:24 PM
>To: Shankar, Uma; intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>Cc: Kumar, Shobhit; Deak, Imre; Sharma, Shashank; Shankar, Uma
>Subject: Re: [BXT MIPI PATCH v3 05/14] drm/i915/bxt: DSI encoder support in
>CRTC modeset
>
>On Tue, 01 Sep 2015, Uma Shankar <uma.shankar@xxxxxxxxx> wrote:
>> @@ -5057,13 +5060,16 @@ static void haswell_crtc_enable(struct drm_crtc
>*crtc)
>> if (intel_crtc->config->has_pch_encoder)
>> lpt_pch_enable(crtc);
>>
>> - if (intel_crtc->config->dp_encoder_is_mst)
>> + if (intel_crtc->config->dp_encoder_is_mst && !is_dsi)
>> intel_ddi_set_vc_payload_alloc(crtc, true);
>>
>> assert_vblank_disabled(crtc);
>> drm_crtc_vblank_on(crtc);
>>
>> for_each_encoder_on_crtc(dev, crtc, encoder) {
>> + if (encoder->pre_pll_enable)
>> + encoder->pre_pll_enable(encoder);
>> +
>
>We should not modify the crtc enable sequence to accommodate the needs of
>one encoder type only. The hook names should be taken as describing roughly
>when in the sequence they are called, not necessarily what they must do for
>each encoder. If an encoder requires a different ordering or sequence, it should
>handle this in what it does in its hooks - and this may possibly need to be
>different on each platform.
>
>Here, the ->pre_pll_enable call is added after the ->pre_enable call, making the
>sequence of calls surprising. Also, there is no point in calling ->pre_pll_enable in
>the same loop as ->enable; the encoder could just as well do everything in -
>>enable. Indeed, this may be what you should do on Broxton.
>
>I'm willing to ignore [1] if Daniel thinks my worry is unwarranted, but this part
>must be fixed.
>
>
>BR,
>Jani.
>
The pre_pll_enable callback was not being used earlier for any encoder in haswell functions.
This is the reason we used it for DSI at place appropriate for DSI sequence. I will remove the
callback and put the code in encoder->enable callback itself.
Regards,
Uma Shankar
>[1] http://mid.gmane.org/87twqlnw5k.fsf@xxxxxxxxx
>
>
>> encoder->enable(encoder);
>> intel_opregion_notify_encoder(encoder, true);
>> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
