Hi Daniel, On Tuesday 29 Nov 2016 10:54:09 Daniel Vetter wrote: > On Tue, Nov 29, 2016 at 11:04:36AM +0200, Laurent Pinchart wrote: > > The LVDS encoder driver is a DRM bridge driver that supports the > > parallel to LVDS encoders that don't require any configuration. The > > driver thus doesn't interact with the device, but creates an LVDS > > connector for the panel and exposes its size and timing based on > > information retrieved from DT. > > > > Signed-off-by: Laurent Pinchart > > <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > > Since it's 100% dummy, why put LVDS into the name? This little thing here > could be our generic "wrap drm_panel and attach it to a chain" helper. So > what about calling this _The_ drm_panel_bridge, and also linking it into > docs to feature it a bit more prominently. I'm not opposed to that, except that this driver should not be considered as just a helper to link a panel. It should only be used to support a real hardware bridge that requires no control. > I came up with this because I spotted some refactoring belows for building > this helper, until I realized that this driver _is_ the helper I think we > want ;-) Only thing missing is an exported function to instantiate a > bridge with just a drm_panel as the parameter. And putting it into the > drm_kms_helper.ko module. What would that be used for ? The bridge should be instantiated by this bridge driver, bound to a bridge device instantiated from DT (or the platform firmware of your choice). > > +static enum drm_connector_status > > +lvds_connector_detect(struct drm_connector *connector, bool force) > > +{ > > + return connector_status_connected; > > +} > > We have piles of this exact dummy callback all over, maybe make it the > default and rip them all out? Done, "[PATCH] drm: Make the connector .detect() callback optional". -- Regards, Laurent Pinchart