Re: [PATCH 05/12] drm/i915/bxt: DSI encoder support in CRTC modeset

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 26 May 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, May 25, 2015 at 01:25:56PM +0300, Jani Nikula wrote:
>> On Fri, 22 May 2015, Uma Shankar <uma.shankar@intel.com> 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.

I guess the question is, what's the short term plan for DSI?

BR,
Jani.


> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx