On Mon, May 25, 2015 at 01:25:56PM +0300, Jani Nikula wrote:
> On Fri, 22 May 2015, Uma Shankar <uma.shankar@xxxxxxxxx> wrote:
> > + * but DDI interface doesn't support DSI yet, so don't do anything
> > + * for DSI encoders
> > + */
> > + if (!(HAS_DDI(dev) && has_encoder_ddi(type))) {
>
> HAS_DDI() is always true here.
>
> Hmm. Perhaps it would be nicer if we added INVALID_PORT = -1 to enum
> port, and had intel_ddi_get_encoder_port() return that for DSI. Then we
> could leave most of the functions the same, with just
>
> if (port == INVALID_PORT)
> return;
>
> at the beginning.
>
> Daniel, opinions?
Layering in the ddi/hsw+ display code is a bit fumbled - a bunch of these
ddi enable/disable calls should be pushed down into encoder hooks.
Otherwise we need to sprinkle piles of if (type == foo) checks all over.
Well we already have them, but we'd need more :(
Generally the split between crtc and encoder should be at the cross-bar
for most of the ports (pch-split is special here with fdi vs cpu ports).
Especially here where we already have a ddi encoder to handle all the ddi
common code.
I've started with patches a while ago, but that didn't get all that far.
Imo the crucial bit is to get rid of intel_ddi_get_encoder_port is the
indicator for how much layering confusion there still is in the ddi code.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
