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

[Andrzej Hajda]

Hi,

On 07/31/2015 04:48 PM, Rob Clark wrote:
> On Fri, Jul 31, 2015 at 8:58 AM, Boris Brezillon
> <boris.brezillon at free-electrons.com> wrote:
>> Hi Rob,
>>
>> On Fri, 31 Jul 2015 08:13:59 -0400
>> Rob Clark <robdclark at gmail.com> wrote:
>>

(...)

>>
>> Another problem I've seen with some drm bridge drivers is that they
>> directly create their own connector, which, as Archit stated, is wrong
>> if you decide to chain this bridge with another bridge. The other
> 
> I agree with Archit on this.  He took this approach w/ msm support for
> external bridges, and it seems sensible that the last bridge
> constructs the connector.  (Plus presumably that is where hpd, ddc
> probing, etc, is happing)

With this approach many bridges should construct connector conditionally,
depending if there is something behind them in pipeline (the same is true for
encoders and even crtcs). It works, but for me there is lot of unnecessary code
and all those conditional paths make things less friendly for development.
Wouldn't be better to move creation of the connector to the main drm driver,
it would require probably adding some opses/fields to drm_bridges but the
drivers would be simpler, I guess.


Regards
Andrzej

> 
>> reason why the bridge should not create the connector by its own is
>> that in some case the encoder supports different types of connectors (a
>> TDMS encoder can be used to output HDMI or DVI), and thus, selecting
>> the connector type should be left to the part responsible for creating
>> the display pipelines.
> 
> did you mean "should" instead of "should not"?  Otherwise I don't
> think I understand..
> 
> BR,
> -R
> 
>> This being said, I'm definitely not an expert in this area, so don't
>> hesitate to show me where I'm wrong.
>>
>> Best Regards,
>>
>> Boris
>>
>> --
>> Boris Brezillon, Free Electrons
>> Embedded Linux and Kernel engineering
>> http://free-electrons.com