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

[Rob Clark]

On Mon, Aug 3, 2015 at 8:03 AM, Andrzej Hajda <a.hajda at samsung.com> wrote:
> 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.

six of one, half dozen of the other..   you'd still need to implement
the hpd and ddc probe bits, and that sort of thing *somewhere*.
Whether it is directly by implementing drm_connector in the bridge, or
indirectly via some extra drm_bridge op's called by a shim connector
in the main drm driver, doesn't really seem to change things..

BR,
-R

>
> 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
>