| Message ID | 1353920848-1705-4-git-send-email-s.trumtrar@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.trumtrar@pengutronix.de> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > Acked-by: Stephen Warren <swarren@nvidia.com> > Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de> > Acked-by: Thierry Reding <thierry.reding@avionic-design.de> > Tested-by: Thierry Reding <thierry.reding@avionic-design.de> > Tested-by: Philipp Zabel <p.zabel@pengutronix.de> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Acked-by: Laurent Pinchart <laurent.pinchart@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
Hi, On Mon, Nov 26, 2012 at 04:38:36PM +0200, Tomi Valkeinen wrote: > 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.trumtrar@pengutronix.de> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > > Acked-by: Stephen Warren <swarren@nvidia.com> > > Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de> > > Acked-by: Thierry Reding <thierry.reding@avionic-design.de> > > Tested-by: Thierry Reding <thierry.reding@avionic-design.de> > > Tested-by: Philipp Zabel <p.zabel@pengutronix.de> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Acked-by: Laurent Pinchart <laurent.pinchart@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? > Those are properties commonly found in display specs. That is why they are here. If the GPU does not support the property it can be omitted. > 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. > Yes. As mentioned in my other mail, the intention of the omitted properties do not propagate properly. Omitted must be a value < 0, so it is clear in a later stage, that this property shall not be used. And isn't unintentionally considered to be active low. > > + - 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. > Yes. Rebase mistake I overlooked. > > +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. > The binding is pretty much at the bare minimum after a lot of discussion about the properties. Even if the binding changes, I think it will rather grow than shrink. Take the doubleclock property for example. It got here mistakingly, because we had a display that has this feature. > 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. I don't say that I'm against it, but the whole reason for the series was getting the display timings from a DT into a graphics driver. And I think I remember seeing some other attempts at achieving this, but all specific to one special case. There is even already a mainline driver that uses an older version of the DT bindings (vt8500-fb). Regards, Steffen
On 2012-11-26 18:10, Steffen Trumtrar wrote: > Hi, > > On Mon, Nov 26, 2012 at 04:38:36PM +0200, Tomi Valkeinen wrote: >>> +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? >> > > Those are properties commonly found in display specs. That is why they are here. > If the GPU does not support the property it can be omitted. So what does the pixelclk-inverted mean? Normally the SoC drives pixel data on rising edge, and the panel samples it at falling edge? And vice-versa for inverted? Or the other way around? When is hsync/vsync set? On rising or falling edge of pclk? My point here is that the pixelclk-inverted is not crystal clear thing, like the hsync/vsync/de-active values are. And while thinking about this, I realized that the meaning of pixelclk-inverted depends on what component is it applied to. Presuming normal pixclk means "pixel data on rising edge", the meaning of that depends on do we consider the SoC or the panel. The panel needs to sample the data on the other edge from the one the SoC uses to drive the data. Does the videomode describe the panel, or does it describe the settings programmed to the SoC? >> 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. >> > > Yes. As mentioned in my other mail, the intention of the omitted properties do > not propagate properly. Omitted must be a value < 0, so it is clear in a later > stage, that this property shall not be used. And isn't unintentionally considered > to be active low. Ok. Just note that the values are currently stored into u32, and I don't think using negative error values with u32 is a good idea. >> 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. >> > > The binding is pretty much at the bare minimum after a lot of discussion about > the properties. Even if the binding changes, I think it will rather grow than > shrink. Take the doubleclock property for example. It got here mistakingly, > because we had a display that has this feature. Right. That's why I would leave the pixelclock-inverted out for now, if we're not totally sure how it's defined. Of course, it could be just me who is not understanding the pixclk-inverted =). >> 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. > > I don't say that I'm against it, but the whole reason for the series was > getting the display timings from a DT into a graphics driver. And I think > I remember seeing some other attempts at achieving this, but all specific > to one special case. There is even already a mainline driver that uses an older > version of the DT bindings (vt8500-fb). I think it'd be very useful even without DT bindings. But yes, I understand your need for it. You're now in v15 of the series. Are you sure v16 will be good enough to freeze the DT bindings, if 15 previous versions weren't? =). Perhaps I'm just overly cautious with DT bindings. APIs that are exposed outside the kernel scare me, as they should be just and not almost right. However, I want to also point out that where ever you're going to use the videomode DT bindings, when the common display framework is merged and presuming you'll use it, you may need to change the DT stuff again (for the SoC/gfx card, not the videomode bindings). Tomi
Hi, Am Montag, den 26.11.2012, 18:56 +0200 schrieb Tomi Valkeinen: > On 2012-11-26 18:10, Steffen Trumtrar wrote: > > Hi, > > > > On Mon, Nov 26, 2012 at 04:38:36PM +0200, Tomi Valkeinen wrote: > > >>> +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? > >> > > > > Those are properties commonly found in display specs. That is why they are here. > > If the GPU does not support the property it can be omitted. > > So what does the pixelclk-inverted mean? Normally the SoC drives pixel > data on rising edge, and the panel samples it at falling edge? And > vice-versa for inverted? Or the other way around? > > When is hsync/vsync set? On rising or falling edge of pclk? > > My point here is that the pixelclk-inverted is not crystal clear thing, > like the hsync/vsync/de-active values are. > > And while thinking about this, I realized that the meaning of > pixelclk-inverted depends on what component is it applied to. Presuming > normal pixclk means "pixel data on rising edge", the meaning of that > depends on do we consider the SoC or the panel. The panel needs to > sample the data on the other edge from the one the SoC uses to drive the > data. > > Does the videomode describe the panel, or does it describe the settings > programmed to the SoC? How about calling this property pixelclk-active, active high meaning driving pixel data on rising edges and sampling on falling edges (the pixel clock is high between driving and sampling the data), and active low meaning driving on falling edges and sampling on rising edges? It is the same from the SoC perspective and from the panel perspective, and it mirrors the usage of the other *-active properties. [...] regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Dec 07, 2012 at 03:12:48PM +0100, Philipp Zabel wrote: > Hi, > > Am Montag, den 26.11.2012, 18:56 +0200 schrieb Tomi Valkeinen: > > On 2012-11-26 18:10, Steffen Trumtrar wrote: > > > Hi, > > > > > > On Mon, Nov 26, 2012 at 04:38:36PM +0200, Tomi Valkeinen wrote: > > > > >>> +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? > > >> > > > > > > Those are properties commonly found in display specs. That is why they are here. > > > If the GPU does not support the property it can be omitted. > > > > So what does the pixelclk-inverted mean? Normally the SoC drives pixel > > data on rising edge, and the panel samples it at falling edge? And > > vice-versa for inverted? Or the other way around? > > > > When is hsync/vsync set? On rising or falling edge of pclk? > > > > My point here is that the pixelclk-inverted is not crystal clear thing, > > like the hsync/vsync/de-active values are. > > > > And while thinking about this, I realized that the meaning of > > pixelclk-inverted depends on what component is it applied to. Presuming > > normal pixclk means "pixel data on rising edge", the meaning of that > > depends on do we consider the SoC or the panel. The panel needs to > > sample the data on the other edge from the one the SoC uses to drive the > > data. > > > > Does the videomode describe the panel, or does it describe the settings > > programmed to the SoC? > > How about calling this property pixelclk-active, active high meaning > driving pixel data on rising edges and sampling on falling edges (the > pixel clock is high between driving and sampling the data), and active > low meaning driving on falling edges and sampling on rising edges? > It is the same from the SoC perspective and from the panel perspective, > and it mirrors the usage of the other *-active properties. > I think, this would not be a bad idea. I would include Philipps description in the display-timing.txt, as it makes the meaning pretty clear; at least to me. What do the others think about this? Regards, Steffen
On 2012-12-07 16:12, Philipp Zabel wrote: > Hi, > > Am Montag, den 26.11.2012, 18:56 +0200 schrieb Tomi Valkeinen: >> So what does the pixelclk-inverted mean? Normally the SoC drives pixel >> data on rising edge, and the panel samples it at falling edge? And >> vice-versa for inverted? Or the other way around? >> >> When is hsync/vsync set? On rising or falling edge of pclk? >> >> My point here is that the pixelclk-inverted is not crystal clear thing, >> like the hsync/vsync/de-active values are. >> >> And while thinking about this, I realized that the meaning of >> pixelclk-inverted depends on what component is it applied to. Presuming >> normal pixclk means "pixel data on rising edge", the meaning of that >> depends on do we consider the SoC or the panel. The panel needs to >> sample the data on the other edge from the one the SoC uses to drive the >> data. >> >> Does the videomode describe the panel, or does it describe the settings >> programmed to the SoC? > > How about calling this property pixelclk-active, active high meaning > driving pixel data on rising edges and sampling on falling edges (the > pixel clock is high between driving and sampling the data), and active > low meaning driving on falling edges and sampling on rising edges? > It is the same from the SoC perspective and from the panel perspective, > and it mirrors the usage of the other *-active properties. This sounds good to me. It's not quite correct, as neither pixelclock or pixel data are not really "active" when the clock is high/low, but it still makes sense and is clear (at least with a short description). Tomi
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) + - interlaced (bool): boolean to enable interlaced mode + - doublescan (bool): boolean to enable doublescan mode + - doubleclk (bool) + +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. + +The parameters are defined as + + +----------+---------------------------------------------+----------+-------+ + | | ? | | | + | | |vback_porch | | | + | | ? | | | + +----------###############################################----------+-------+ + | # ? # | | + | # | # | | + | hback # | # hfront | hsync | + | porch # | hactive # porch | len | + |<-------->#<---------------+--------------------------->#<-------->|<----->| + | # | # | | + | # |vactive # | | + | # | # | | + | # ? # | | + +----------###############################################----------+-------+ + | | ? | | | + | | |vfront_porch | | | + | | ? | | | + +----------+---------------------------------------------+----------+-------+ + | | ? | | | + | | |vsync_len | | | + | | ? | | | + +----------+---------------------------------------------+----------+-------+ + + +Example: + + display-timings { + native-mode = <&timing0>; + timing0: 1080p24 { + /* 1920x1080p24 */ + clock-frequency = <52000000>; + hactive = <1920>; + vactive = <1080>; + hfront-porch = <25>; + hback-porch = <25>; + hsync-len = <25>; + vback-porch = <2>; + vfront-porch = <2>; + vsync-len = <2>; + hsync-active = <1>; + }; + }; + +Every required property also supports the use of ranges, so the commonly used +datasheet description with <min typ max>-tuples can be used. + +Example: + + timing1: timing { + /* 1920x1080p24 */ + clock-frequency = <148500000>; + hactive = <1920>; + vactive = <1080>; + hsync-len = <0 44 60>; + hfront-porch = <80 88 95>; + hback-porch = <100 148 160>; + vfront-porch = <0 4 6>; + vback-porch = <0 36 50>; + vsync-len = <0 5 6>; + }; diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 2a23b18..c000f5a 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -39,6 +39,21 @@ config DISPLAY_TIMING config VIDEOMODE bool +config OF_DISPLAY_TIMING + bool "Enable device tree display timing support" + depends on OF + select DISPLAY_TIMING + help + helper to parse display timings from the devicetree + +config OF_VIDEOMODE + bool "Enable device tree videomode support" + depends on OF + select VIDEOMODE + select OF_DISPLAY_TIMING + help + helper to get videomodes from the devicetree + menuconfig FB tristate "Support for frame buffer devices" ---help--- diff --git a/drivers/video/Makefile b/drivers/video/Makefile index fc30439..b936b00 100644 --- a/drivers/video/Makefile +++ b/drivers/video/Makefile @@ -168,4 +168,6 @@ obj-$(CONFIG_FB_VIRTUAL) += vfb.o #video output switch sysfs driver obj-$(CONFIG_VIDEO_OUTPUT_CONTROL) += output.o obj-$(CONFIG_DISPLAY_TIMING) += display_timing.o +obj-$(CONFIG_OF_DISPLAY_TIMING) += of_display_timing.o obj-$(CONFIG_VIDEOMODE) += videomode.o +obj-$(CONFIG_OF_VIDEOMODE) += of_videomode.o diff --git a/drivers/video/of_display_timing.c b/drivers/video/of_display_timing.c new file mode 100644 index 0000000..b6c549e --- /dev/null +++ b/drivers/video/of_display_timing.c @@ -0,0 +1,219 @@ +/* + * OF helpers for parsing display timings + * + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix + * + * based on of_videomode.c by Sascha Hauer <s.hauer@pengutronix.de> + * + * This file is released under the GPLv2 + */ +#include <linux/display_timing.h> +#include <linux/export.h> +#include <linux/of.h> +#include <linux/of_display_timing.h> +#include <linux/slab.h> + +/** + * parse_property - parse timing_entry from device_node + * @np: device_node with the property + * @name: name of the property + * @result: will be set to the return value + * + * DESCRIPTION: + * Every display_timing can be specified with either just the typical value or + * a range consisting of min/typ/max. This function helps handling this + **/ +static int parse_property(const struct device_node *np, const char *name, + struct timing_entry *result) +{ + struct property *prop; + int length, cells, ret; + + prop = of_find_property(np, name, &length); + if (!prop) { + pr_err("%s: could not find property %s\n", __func__, name); + return -EINVAL; + } + + cells = length / sizeof(u32); + if (cells == 1) { + ret = of_property_read_u32(np, name, &result->typ); + result->min = result->typ; + result->max = result->typ; + } else if (cells == 3) { + ret = of_property_read_u32_array(np, name, &result->min, cells); + } else { + pr_err("%s: illegal timing specification in %s\n", __func__, + name); + return -EINVAL; + } + + return ret; +} + +/** + * of_get_display_timing - parse display_timing entry from device_node + * @np: device_node with the properties + **/ +static struct display_timing *of_get_display_timing(const struct device_node + *np) +{ + struct display_timing *dt; + int ret = 0; + + dt = kzalloc(sizeof(*dt), GFP_KERNEL); + if (!dt) { + pr_err("%s: could not allocate display_timing struct\n", + __func__); + return NULL; + } + + ret |= parse_property(np, "hback-porch", &dt->hback_porch); + ret |= parse_property(np, "hfront-porch", &dt->hfront_porch); + ret |= parse_property(np, "hactive", &dt->hactive); + ret |= parse_property(np, "hsync-len", &dt->hsync_len); + ret |= parse_property(np, "vback-porch", &dt->vback_porch); + ret |= parse_property(np, "vfront-porch", &dt->vfront_porch); + ret |= parse_property(np, "vactive", &dt->vactive); + ret |= parse_property(np, "vsync-len", &dt->vsync_len); + ret |= parse_property(np, "clock-frequency", &dt->pixelclock); + + of_property_read_u32(np, "vsync-active", &dt->vsync_pol_active); + of_property_read_u32(np, "hsync-active", &dt->hsync_pol_active); + of_property_read_u32(np, "de-active", &dt->de_pol_active); + of_property_read_u32(np, "pixelclk-inverted", &dt->pixelclk_pol); + dt->interlaced = of_property_read_bool(np, "interlaced"); + dt->doublescan = of_property_read_bool(np, "doublescan"); + + if (ret) { + pr_err("%s: error reading timing properties\n", __func__); + kfree(dt); + return NULL; + } + + return dt; +} + +/** + * of_get_display_timings - parse all display_timing entries from a device_node + * @np: device_node with the subnodes + **/ +struct display_timings *of_get_display_timings(struct device_node *np) +{ + struct device_node *timings_np; + struct device_node *entry; + struct device_node *native_mode; + struct display_timings *disp; + + if (!np) { + pr_err("%s: no devicenode given\n", __func__); + return NULL; + } + + timings_np = of_find_node_by_name(np, "display-timings"); + if (!timings_np) { + pr_err("%s: could not find display-timings node\n", __func__); + return NULL; + } + + disp = kzalloc(sizeof(*disp), GFP_KERNEL); + if (!disp) { + pr_err("%s: could not allocate struct disp'\n", __func__); + goto dispfail; + } + + entry = of_parse_phandle(timings_np, "native-mode", 0); + /* assume first child as native mode if none provided */ + if (!entry) + entry = of_get_next_child(np, NULL); + /* if there is no child, it is useless to go on */ + if (!entry) { + pr_err("%s: no timing specifications given\n", __func__); + goto entryfail; + } + + pr_info("%s: using %s as default timing\n", __func__, entry->name); + + native_mode = entry; + + disp->num_timings = of_get_child_count(timings_np); + if (disp->num_timings == 0) { + /* should never happen, as entry was already found above */ + pr_err("%s: no timings specified\n", __func__); + goto entryfail; + } + + disp->timings = kzalloc(sizeof(struct display_timing *) * disp->num_timings, + GFP_KERNEL); + if (!disp->timings) { + pr_err("%s: could not allocate timings array\n", __func__); + goto entryfail; + } + + disp->num_timings = 0; + disp->native_mode = 0; + + for_each_child_of_node(timings_np, entry) { + struct display_timing *dt; + + dt = of_get_display_timing(entry); + if (!dt) { + /* + * to not encourage wrong devicetrees, fail in case of + * an error + */ + pr_err("%s: error in timing %d\n", __func__, + disp->num_timings + 1); + goto timingfail; + } + + if (native_mode == entry) + disp->native_mode = disp->num_timings; + + disp->timings[disp->num_timings] = dt; + disp->num_timings++; + } + of_node_put(timings_np); + /* + * native_mode points to the device_node returned by of_parse_phandle + * therefore call of_node_put on it + */ + of_node_put(native_mode); + + pr_info("%s: got %d timings. Using timing #%d as default\n", + __func__, disp->num_timings, disp->native_mode + 1); + + return disp; + +timingfail: + if (native_mode) + of_node_put(native_mode); + display_timings_release(disp); +entryfail: + if (disp) + kfree(disp); +dispfail: + of_node_put(timings_np); + return NULL; +} +EXPORT_SYMBOL_GPL(of_get_display_timings); + +/** + * of_display_timings_exists - check if a display-timings node is provided + * @np: device_node with the timing + **/ +int of_display_timings_exists(struct device_node *np) +{ + struct device_node *timings_np; + + if (!np) + return -EINVAL; + + timings_np = of_parse_phandle(np, "display-timings", 0); + if (!timings_np) + return -EINVAL; + + of_node_put(timings_np); + return 1; +} +EXPORT_SYMBOL_GPL(of_display_timings_exists); diff --git a/drivers/video/of_videomode.c b/drivers/video/of_videomode.c new file mode 100644 index 0000000..38d4a64 --- /dev/null +++ b/drivers/video/of_videomode.c @@ -0,0 +1,54 @@ +/* + * generic videomode helper + * + * Copyright (c) 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de>, Pengutronix + * + * This file is released under the GPLv2 + */ +#include <linux/display_timing.h> +#include <linux/errno.h> +#include <linux/export.h> +#include <linux/of.h> +#include <linux/of_display_timing.h> +#include <linux/of_videomode.h> +#include <linux/videomode.h> + +/** + * of_get_videomode - get the videomode #<index> from devicetree + * @np - devicenode with the display_timings + * @vm - set to return value + * @index - index into list of display_timings + * (Set this to OF_USE_NATIVE_MODE to use whatever mode is + * specified as native mode in the DT.) + * + * DESCRIPTION: + * Get a list of all display timings and put the one + * specified by index into *vm. This function should only be used, if + * only one videomode is to be retrieved. A driver that needs to work + * with multiple/all videomodes should work with + * of_get_display_timings instead. + **/ +int of_get_videomode(struct device_node *np, struct videomode *vm, + int index) +{ + struct display_timings *disp; + int ret; + + disp = of_get_display_timings(np); + if (!disp) { + pr_err("%s: no timings specified\n", __func__); + return -EINVAL; + } + + if (index == OF_USE_NATIVE_MODE) + index = disp->native_mode; + + ret = videomode_from_timing(disp, vm, index); + if (ret) + return ret; + + display_timings_release(disp); + + return 0; +} +EXPORT_SYMBOL_GPL(of_get_videomode); diff --git a/include/linux/of_display_timing.h b/include/linux/of_display_timing.h new file mode 100644 index 0000000..31e9695 --- /dev/null +++ b/include/linux/of_display_timing.h @@ -0,0 +1,20 @@ +/* + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de> + * + * display timings of helpers + * + * This file is released under the GPLv2 + */ + +#ifndef __LINUX_OF_DISPLAY_TIMING_H +#define __LINUX_OF_DISPLAY_TIMING_H + +struct device_node; +struct display_timings; + +#define OF_USE_NATIVE_MODE -1 + +struct display_timings *of_get_display_timings(struct device_node *np); +int of_display_timings_exists(struct device_node *np); + +#endif diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h new file mode 100644 index 0000000..a07efcc --- /dev/null +++ b/include/linux/of_videomode.h @@ -0,0 +1,18 @@ +/* + * Copyright 2012 Steffen Trumtrar <s.trumtrar@pengutronix.de> + * + * videomode of-helpers + * + * This file is released under the GPLv2 + */ + +#ifndef __LINUX_OF_VIDEOMODE_H +#define __LINUX_OF_VIDEOMODE_H + +struct device_node; +struct videomode; + +int of_get_videomode(struct device_node *np, struct videomode *vm, + int index); + +#endif /* __LINUX_OF_VIDEOMODE_H */