Op 22-08-16 om 16:32 schreef Daniel Vetter:
> On Mon, Aug 22, 2016 at 10:06:22AM +0200, Maarten Lankhorst wrote:
>> Op 18-08-16 om 15:30 schreef Daniel Vetter:
>>> On Tue, Aug 09, 2016 at 05:04:04PM +0200, Maarten Lankhorst wrote:
>>>> This is mostly code churn, with exception of a few places:
>>>> - intel_display.c has changes in intel_sanitize_encoder
>>>> - intel_ddi.c has intel_ddi_fdi_disable calling intel_ddi_post_disable,
>>>>   and required a function change. Also affects intel_display.c
>>>> - intel_dp_mst.c passes a NULL crtc_state and conn_state to
>>>>   intel_ddi_post_disable for shutting down the real encoder.
>>> I didn't find this NULL callsite. Outdated commit message?
>> intel_dp_mst.c:
>>
>> +            intel_dig_port->base.post_disable(&intel_dig_port->base,
>> +                                              old_crtc_state, NULL);
>>
>> Though it seems I accidentally pass old_crtc_state, that one should be NULL 
>> too.
> Indeed I somehow missed that one. Still, why do we need to pass NULL
> there? We should be able to wire up the state passing completely.
In MST mode we have to do some enabling of the port regardless of crtc/MST 
connector.

If we would pass conn_state, then conn_state->connector != 
intel_dig_port->connector and conn_state->best_encoder != 
to_intel_encoder(intel_dig_port).
Why even bother passing conn_state at that point?

We also shouldn't pass crtc_state, because in that case the disabling sequence 
may potentially be different depending on which crtc is disabled last. Nice 
source of bugs. :)

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to