Hi Thierry, On Monday 22 September 2014 09:40:38 Thierry Reding wrote: > On Wed, Sep 17, 2014 at 12:27:13PM +0300, Laurent Pinchart wrote: > > 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. I disagree. AFAIK the component framework was designed to easily combine multiple devices into a single logical device, regardless of which bus each device is connected to. That's what makes the component framework useful : it allows master drivers to build logical devices from heterogeneous components without having to use one API per bus and/or component type. If the only goal had been to combine platform devices on an SoC, simpler device-specific solutions would likely have been used instead. > 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. They represent hardware external to the SoC, but internal to the logical DRM device. > 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, ...). No, that's very different. The device you list are clearly external resources, while the bridges and panels are components part of a logical display device. > > 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 oneto 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. Regardless of how you call them, we have three interfaces. > > 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 slave encoder / bridge split is what I consider a design mistake. Those two interfaces serve the same purpose, they should really be merged. > > - 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. Not if the bridge requires a clock provided by the display controller, in which case there's a dependency loop. > > > 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. Are you volunteering to perform the conversion ? :-) -- Regards, Laurent Pinchart
Attachment:
signature.asc
Description: This is a digitally signed message part.