On 08/04/2015 05:54 PM, Rob Clark wrote: > On Tue, Aug 4, 2015 at 1:16 AM, Andrzej Hajda <a.hajda at samsung.com> wrote: >> On 08/03/2015 04:04 PM, Rob Clark wrote: >>> 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.. >> >> The difference is that instead of duplicating connector related code in every >> driver you will call one helper from the main drm driver/core. > > Which isn't more than a few lines of code.. I mean, looking at a > couple connectors, the bulk of the code is hpd and ddc related. That > doesn't magically go away. There isn't that many lines of > boilerplate, so meh.
Could we get to a consensus for this? Is it okay for bridge and i2c_encoder to co-exist for now? Thanks, Archit > > BR, > -R > >> Regards >> Andrzej >> >>> >>> 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 >> -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
