On Mon, Sep 24, 2012 at 05:09:30PM -0600, Stephen Warren wrote: > On 09/24/2012 12:26 PM, Rob Herring wrote: > > On 09/24/2012 10:45 AM, Stephen Warren wrote: > >> On 09/24/2012 07:42 AM, Rob Herring wrote: > >>> On 09/19/2012 03:20 AM, Steffen Trumtrar wrote: > >>>> This patch adds a helper function for parsing videomodes from the devicetree. > >>>> The videomode can be either converted to a struct drm_display_mode or a > >>>> struct fb_videomode. > >> > >>>> +++ b/Documentation/devicetree/bindings/video/displaymode > >>>> @@ -0,0 +1,74 @@ > >>>> +videomode bindings > >>>> +================== > >>>> + > >>>> +Required properties: > >>>> + - hactive, vactive: Display resolution > >>>> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters > >>>> + in pixels > >>>> + vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in > >>>> + lines > >>>> + - clock: displayclock in Hz > >>> > >>> A major piece missing is the LCD controller to display interface width > >>> and component ordering. > >> > >> I thought this binding was solely defining the timing of the video > >> signal (hence "video mode"). Any definition of the physical interface to > >> the LCD/display-connector is something entirely orthogonal, so it seems > >> entirely reasonable to represent that separately. > > > > It is not orthogonal because in many cases the LCD panel defines the > > mode. > > The LCD panel itself defines both the mode and the physical interface > properties. The mode does not imply anything about the physical > interface, nor does the physical interface imply anything about the > mode. So, they are in fact orthogonal. In other words, just because you > need both sets of information, doesn't make the two pieces of > information correlated. > > >>>> +Example: > >>>> + > >>>> + display@0 { > >>> > >>> It would be useful to have a compatible string here. We may not always > >>> know the panel type or have a fixed panel though. We could define > >>> "generic-lcd" or something for cases where the panel type is unknown. > >>> > >>>> + width-mm = <800>; > >>>> + height-mm = <480>; > >> > >> I would hope that everything in the example above this point is just > >> that - an example, and this binding only covers the display mode > >> definition - i.e. that part of the example below. > >> > > > > It's fairly clear this binding is being defined based on what Linux > > supports vs. what the h/w looks like. > > > >> If that's not the intent, as Rob says, there's a /ton/ of stuff missing. > > > > Assuming not, what all is missing? > > Everything related to the physical interface: > > * For DSI, whatever it needs to be configured. > * For LVDS, e.g. number of lanes of R, G, B. > * Perhaps multi-pumping rates (# of clock signals to send each data > value for, to satisfy any minimum clock rates) > * Built-in displays typically need to be coupled with a backlight and > all the associated control of that. > * Pinctrl interaction. > > and probably a bunch of other things I haven't thought about. I already added some of those things in my v5. For those who missed it <1348500924-8551-1-git-send-email-s.trumtrar@xxxxxxxxxxxxxx> I renamed from "of videomode helper" to "of display helper", seemed to be more clear what it is supposed to accomplish. Regards, Steffen -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel