[Intel-gfx] [PATCH 08/10] drm/i915: Pass a crtc state to ddi post_disable from MST code

[Maarten Lankhorst]

Op 27-10-17 om 13:49 schreef Ville Syrjälä:
> On Fri, Oct 27, 2017 at 01:39:00PM +0200, Maarten Lankhorst wrote:
>> Op 19-10-17 om 15:37 schreef Ville Syrjala:
>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>
>>> Pass an old crtc state to intel_ddi_post_disable() from the MST code.
>>>
>>> Note that this crtc state won't necessaitly match the one that was
>>> passed to intel_ddi_pre_enable() if the first stream to be enabled isn't
>>> the last stream to be disabled. But this is fine since the states should
>>> be identical in every important way. This does mean people frobbing
>>> the DDI pre_enable/post_disable hooks have to pay attention in what
>>> parts of the state they consult.
>>>
>>> The alternative would be to inline the relevant code into the MST code.
>>> That is actually what we used to do for pre_enable before
>>> commit e081c8463ac9 ("drm/i915: Remove duplicate DDI enabling logic
>>> from MST path"). For post_disable we've always called the DDI hook.
>> I woudl rather have it NULL throughout the entire disable sequence, it's going to be
>> dangerous anyway, and something dangerous shouldn't be easy. The fact that you have
>> to think
> It makes for really ugly code as the enable and disable don't look at
> all like each other when one has the NULL and the other doesn't. If it's
> dangerous for disable, then it's equally dangerous for enable. So I'd
> just go with whatever makes the code suck the least, which IMO is
> passing a crtc state to both.
>
> Probably the proper fix would be some kind of encoder state which would
> be assigned to mst->primary. But that's going to require actual design
> work.
Yeah, I just think it's dangerous regardless, I wish we could do something more safe about it, but this will have to do for now.

Theoretically we could add encoder state, but that probably requires a massive split in crtc_state.

Not sure what's wisdom here. :/