On Wed, Sep 17, 2014 at 12:27:13PM +0300, Laurent Pinchart wrote: > Hi Ajay, > > On Wednesday 17 September 2014 14:37:30 Ajay kumar wrote: > > On Mon, Sep 15, 2014 at 11:07 PM, Laurent Pinchart wrote: > > > Hi Ajay, > > > > > > Thank you for the patch. > > > > > > I think we're moving in the right direction, but we're not there yet. > > > > > > On Saturday 26 July 2014 00:52:08 Ajay Kumar wrote: > > >> This patch tries to seperate drm_bridge implementation > > >> into 2 parts, a drm part and a non_drm part. > > >> > > >> A set of helper functions are defined in this patch to make > > >> bridge driver probe independent of the drm flow. > > >> > > >> The bridge devices register themselves on a lookup table > > >> when they get probed by calling "drm_bridge_add_for_lookup". > > >> > > >> The parent encoder driver waits till the bridge is available in the > > >> lookup table(by calling "of_drm_find_bridge") and then continues with > > >> its initialization. > > > > > > Before the introduction of the component framework I would have said this > > > is the way to go. Now, I think bridges should register themselves as > > > components, and the DRM master driver should use the component framework > > > to get a reference to the bridges it needs. > > > > Well, I have modified the bridge framework exactly the way Thierry wanted it > > to be, I mean the same way the current panel framework is. > > And, I don't think there is a problem with that. > > What problem are you facing with current bridge implementation? > > What is the advantage of using the component framework to register bridges? > > There are several advantages. > > - The component framework has been designed with this exact problem in mind, > piecing multiple components into a display device. No. Component framework was designed with multi-device drivers in mind. That is, drivers that need to combine two or more platform devices into a single logical device. Typically that includes display controllers and encoders (in various looks) for DRM. Panels and bridges are in my opinion different because they are outside of the DRM driver. They aren't part of the device complex that an SoC provides. They represent hardware that is external to the SoC and the DRM driver and can be shared across SoCs. Forcing panels and bridges to register as components will require all drivers to implement master/component support solely for accessing this external hardware. What you're suggesting is like saying that clocks or regulators should register as components so that their users can get them that way. In fact by that argument everything that's referenced by phandle would need to register as component (PHYs, LEDs, GPIOs, I2C controllers, ...). > This patch set introduces > yet another framework, without any compelling reason as far as I can see. > Today DRM drivers already need to use three different frameworks (component, > I2C slave encoder and panel), and we're adding a fourth one to make the mess > even messier. Panel and bridge aren't really frameworks. Rather they are a simple registry to allow drivers to register panels and bridges and display drivers to look them up. > This is really a headlong rush, we need to stop and fix the design mistakes. Can you point out specific design mistakes? I don't see any, but I'm obviously biased. > - The component framework solves the probe ordering problem. Bridges can use > deferred probing, but when a bridge requires a resources (such as a clock for > instance) provided by the display controller, this will break. Panel and bridges can support deferred probing without the component framework just fine. > > Without this patchset, you cannot bring an X server > > based display on snow and peach_pit. Also, day by day the number of > > platforms using drm_bridge is increasing. > > That's exactly why I'd like to use the component framework now, as the > conversion will become more complex as time goes by. No it won't. If we ever do decide that component framework is a better fit then the conversion may be more work but it would still be largely mechanical. Thierry
Attachment:
pgpqRRcrjH_5d.pgp
Description: PGP signature
