Re: [PATCH 3/5] drm/i2c: adv7511: Refactor encoder slave functions

From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Archit Taneja <architt@codeaurora.org>
Cc: linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	andy.green@linaro.org, srinivas.kandagatla@linaro.org,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH 3/5] drm/i2c: adv7511: Refactor encoder slave functions
Date: Tue, 28 Jul 2015 16:38:28 +0200	[thread overview]
Message-ID: <20150728163828.59b7ebcc@bbrezillon> (raw)
In-Reply-To: <55B73AA1.3040008@codeaurora.org>

Archit, Laurent,

On Tue, 28 Jul 2015 13:47:37 +0530
Archit Taneja <architt@codeaurora.org> wrote:

> Hi,
> 
> On 07/27/2015 02:29 PM, Laurent Pinchart wrote:
> > Hi Archit,
> >
> > (CC'ing Boris Brezillon)
> >
> > Thank you for the patch.
> >
> > On Monday 27 July 2015 11:46:57 Archit Taneja wrote:
> >> ADV7511 is represented as an i2c drm slave encoder device. ADV7533, on
> >> the other hand, is going be a normal i2c client device creating bridge
> >> and connector entities.
> >
> > Please, no. It's really time to stop piling hacks and fix the problem
> > properly. There's no reason to have separate APIs for I2C slave encoders and
> > DRM bridges. Those two APIs need to be merged, and then you'll find it much
> > easier to implement ADV7533 support.
> 
> i2c slave encoders and bridges aren't exactly the same. slave encoders
> are used when the there is no real 'encoder' in the display chain.
> bridges are used when there is already an encoder available, and the
> bridge entity represents another encoder in the chain.
> 
> ADV7511 takes in RGB/MIPI DPI data, which is generally the output of a 
> crtc for drm drivers.
> 
> ADV7533 takes in MIPI DSI data, which is generally the output of an
> encoder for drm drivers.
> 
> Therefore, having i2c slave encoder for the former and bridge for the
> latter made sense to me.
> 
> I do agree that it would be better if they were somehow merged. It
> would prevent the fragmentation we currently have among encoder
> chips.
> 
> One possible way would be to convert slave encoder to bridge. With
> this, an i2c slave encoder would be a 'dummy encoder' and a bridge.
> i2c slave encoders even now just tie the slave encoder helper funcs
> to encoder helper funcs. So it's not really any different.
> 
> Merging these 2 entities would be nice, but we're still shying away
> from the larger problem of creating generic encoder chains. The
> ideal solution would be for bridges and slave encoders to just be
> 'encoders', and the facility to connect on encoder output to the
> input of another. I don't know how easy it is to do this, and
> whether it'll break userspace.

Yes, that's pretty much what I was trying to do.
I'd also like to ease display pipelines creation by providing helper
functions, so that display controller don't have to worry about encoders
and connectors creation if these ones are attached to external encoders.

> 
> Archit
> 
> >
> > Boris, I know you were experimenting with that, do you have anything to report
> > ?

Nope, I didn't work on it since last time we talked about it. I pushed
my work here if you want to have a look [1].

Best Regards,

Boris

[1]https://github.com/bbrezillon/linux-at91/tree/drm-encoder-chain-WIP

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel