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@xxxxxxxxxxxxxxx>
> >
> > 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.
>
> Also there's an unrelated hunk in intel_ddi_pre_enable?
Apparently I wanted to get rid of the local pipe variable.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
