Hi, On 2012-11-26 11:07, Steffen Trumtrar wrote: > This adds support for reading display timings from DT into a struct > display_timings. The of_display_timing implementation supports multiple > subnodes. All children are read into an array, that can be queried. > > If no native mode is specified, the first subnode will be used. > > For cases where the graphics driver knows there can be only one > mode description or where the driver only supports one mode, a helper > function of_get_videomode is added, that gets a struct videomode from DT. > > Signed-off-by: Steffen Trumtrar <s.trumt...@pengutronix.de> > Signed-off-by: Philipp Zabel <p.za...@pengutronix.de> > Acked-by: Stephen Warren <swar...@nvidia.com> > Reviewed-by: Thierry Reding <thierry.red...@avionic-design.de> > Acked-by: Thierry Reding <thierry.red...@avionic-design.de> > Tested-by: Thierry Reding <thierry.red...@avionic-design.de> > Tested-by: Philipp Zabel <p.za...@pengutronix.de> > Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > Acked-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > --- > .../devicetree/bindings/video/display-timing.txt | 107 ++++++++++ > drivers/video/Kconfig | 15 ++ > drivers/video/Makefile | 2 + > drivers/video/of_display_timing.c | 219 > ++++++++++++++++++++ > drivers/video/of_videomode.c | 54 +++++ > include/linux/of_display_timing.h | 20 ++ > include/linux/of_videomode.h | 18 ++ > 7 files changed, 435 insertions(+) > create mode 100644 Documentation/devicetree/bindings/video/display-timing.txt > create mode 100644 drivers/video/of_display_timing.c > create mode 100644 drivers/video/of_videomode.c > create mode 100644 include/linux/of_display_timing.h > create mode 100644 include/linux/of_videomode.h > > diff --git a/Documentation/devicetree/bindings/video/display-timing.txt > b/Documentation/devicetree/bindings/video/display-timing.txt > new file mode 100644 > index 0000000..e238f27 > --- /dev/null > +++ b/Documentation/devicetree/bindings/video/display-timing.txt > @@ -0,0 +1,107 @@ > +display-timing bindings > +======================= > + > +display-timings node > +-------------------- > + > +required properties: > + - none > + > +optional properties: > + - native-mode: The native mode for the display, in case multiple modes are > + provided. When omitted, assume the first node is the native. > + > +timing subnode > +-------------- > + > +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-frequency: display clock in Hz > + > +optional properties: > + - hsync-active: hsync pulse is active low/high/ignored > + - vsync-active: vsync pulse is active low/high/ignored > + - de-active: data-enable pulse is active low/high/ignored > + - pixelclk-inverted: pixelclock is inverted (active on falling edge)/ > + non-inverted (active on rising edge)/ > + ignored (ignore property)
I think hsync-active and vsync-active are clear, and commonly used, and they are used for both drm and fb mode conversions in later patches. de-active is not used in drm and fb mode conversions, but I think it's also clear. pixelclk-inverted is not used in the mode conversions. It's also a bit unclear to me. What does it mean that pix clock is "active on rising edge"? The pixel data is driven on rising edge? How about the sync signals and DE, when are they driven? Does your HW have any settings related to those? OMAP has the invert pclk setting, but it also has a setting to define when the sync signals are driven. The options are: - syncs are driven on rising edge of pclk - syncs are driven on falling edge of pclk - syncs are driven on the opposite edge of pclk compared to the pixel data For DE there's no setting, except the active high/low. And if I'm not mistaken, if the optional properties are not defined, they are not ignored, but left to the default 0. Which means active low, or active on rising edge(?). I think it would be good to have a "undefined" value for the properties. > + - interlaced (bool): boolean to enable interlaced mode > + - doublescan (bool): boolean to enable doublescan mode > + - doubleclk (bool) As I mentioned in the other mail, doubleclk is not used nor documented here. > +All the optional properties that are not bool follow the following logic: > + <1>: high active > + <0>: low active > + omitted: not used on hardware > + > +There are different ways of describing the capabilities of a display. The > devicetree > +representation corresponds to the one commonly found in datasheets for > displays. > +If a display supports multiple signal timings, the native-mode can be > specified. I have some of the same concerns for this series than with the interpreted power sequences (on fbdev): when you publish the DT bindings, it's somewhat final version then, and fixing it later will be difficult. Of course, video modes are much clearer than the power sequences, so it's possible there won't be any problems with the DT bindings. However, I'd still feel safer if the series would be split to non-DT and DT parts. The non-DT parts could be merged quite easily, and people could start using them in their kernels. This should expose bugs/problems related to the code. The DT part could be merged later, when there's confidence that the timings are good for all platforms. Or, alternatively, all the non-common bindings (de-active, pck invert,...) that are not used for fbdev or drm currently could be left out for now. But I'd stil prefer merging it in two parts. Tomi
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
