subject:"\[PATCHv16 5\/7\] fbmon\: add of

Re: [PATCHv16 5/7] fbmon: add of_videomode helpers

2013-01-07 Thread Steffen Trumtrar

Hi Afzal,

On Mon, Jan 07, 2013 at 06:10:13AM +, Mohammed, Afzal wrote:
 Hi Steffen,
 
 On Tue, Dec 18, 2012 at 22:34:14, Steffen Trumtrar wrote:
  Add helper to get fb_videomode from devicetree.
 
   drivers/video/fbmon.c |   42 ++
   include/linux/fb.h|4 
 
 This breaks DaVinci (da8xx_omapl_defconfig), following change was
 required to get it build if OF_VIDEOMODE or/and FB_MODE_HELPERS
 is not defined. There may be better solutions, following was the
 one that was used by me to test this series.
 
 ---8--
 
 diff --git a/include/linux/fb.h b/include/linux/fb.h
 index 58b9860..0ce30d1 100644
 --- a/include/linux/fb.h
 +++ b/include/linux/fb.h
 @@ -716,9 +716,19 @@ extern void fb_destroy_modedb(struct fb_videomode 
 *modedb);
  extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
  extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
 +#if defined(CONFIG_OF_VIDEOMODE)  defined(CONFIG_FB_MODE_HELPERS)
  extern int of_get_fb_videomode(struct device_node *np,
struct fb_videomode *fb,
int index);
 +#else
 +static inline int of_get_fb_videomode(struct device_node *np,
 + struct fb_videomode *fb,
 + int index)
 +{
 +   return -EINVAL;
 +}
 +#endif
 +
  extern int fb_videomode_from_videomode(const struct videomode *vm,
struct fb_videomode *fbmode);
 
 ---8--
 

I just did a quick make da8xx_omapl_defconfig  make and it builds just fine.
On what version did you apply the series?
At the moment I have the series sitting on 3.7. Didn't try any 3.8-rcx yet.
But fixing this shouldn't be a problem.

 
  +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
 
 As _OF_VIDEOMODE is a bool type CONFIG, isn't,
 
 #ifdef CONFIG_OF_VIDEOMODE
 
 sufficient ?
 

Yes, that is right. But I think IS_ENABLED is the preferred way to do it, isn't 
it?

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- |
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCHv16 5/7] fbmon: add of_videomode helpers

2013-01-07 Thread Mohammed, Afzal

Hi Steffen,

On Mon, Jan 07, 2013 at 13:36:48, Steffen Trumtrar wrote:
 On Mon, Jan 07, 2013 at 06:10:13AM +, Mohammed, Afzal wrote:

  This breaks DaVinci (da8xx_omapl_defconfig), following change was
  required to get it build if OF_VIDEOMODE or/and FB_MODE_HELPERS
  is not defined. There may be better solutions, following was the
  one that was used by me to test this series.

 I just did a quick make da8xx_omapl_defconfig  make and it builds just 
 fine.
 On what version did you apply the series?
 At the moment I have the series sitting on 3.7. Didn't try any 3.8-rcx yet.
 But fixing this shouldn't be a problem.

You are right, me idiot, error will happen only upon try to make use of
of_get_fb_videomode() (defined in this patch) in the da8xx-fb driver
(with da8xx_omapl_defconfig), to be exact upon adding,

video: da8xx-fb: obtain fb_videomode info from dt of my patch series.

The change as I mentioned or something similar would be required as
any driver that is going to make use of of_get_fb_videomode() would
break if CONFIG_OF_VIDEOMODE or CONFIG_FB_MODE_HELPERS is not defined.

And testing was done over v3.8-rc2.

   +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
  
  As _OF_VIDEOMODE is a bool type CONFIG, isn't,
  
  #ifdef CONFIG_OF_VIDEOMODE
  
  sufficient ?
  
 
 Yes, that is right. But I think IS_ENABLED is the preferred way to do it, 
 isn't it?

Now I realize it is.

Regards
Afzal

Re: [PATCHv16 5/7] fbmon: add of_videomode helpers

2013-01-07 Thread Rob Clark

On Mon, Jan 7, 2013 at 2:46 AM, Mohammed, Afzal af...@ti.com wrote:
 Hi Steffen,

 On Mon, Jan 07, 2013 at 13:36:48, Steffen Trumtrar wrote:
 On Mon, Jan 07, 2013 at 06:10:13AM +, Mohammed, Afzal wrote:

  This breaks DaVinci (da8xx_omapl_defconfig), following change was
  required to get it build if OF_VIDEOMODE or/and FB_MODE_HELPERS
  is not defined. There may be better solutions, following was the
  one that was used by me to test this series.

 I just did a quick make da8xx_omapl_defconfig  make and it builds just 
 fine.
 On what version did you apply the series?
 At the moment I have the series sitting on 3.7. Didn't try any 3.8-rcx yet.
 But fixing this shouldn't be a problem.

 You are right, me idiot, error will happen only upon try to make use of
 of_get_fb_videomode() (defined in this patch) in the da8xx-fb driver
 (with da8xx_omapl_defconfig), to be exact upon adding,

 video: da8xx-fb: obtain fb_videomode info from dt of my patch series.

 The change as I mentioned or something similar would be required as
 any driver that is going to make use of of_get_fb_videomode() would
 break if CONFIG_OF_VIDEOMODE or CONFIG_FB_MODE_HELPERS is not defined.

Shouldn't the driver that depends on CONFIG_OF_VIDEOMODE and
CONFIG_FB_MODE_HELPERS, explicitly select them?  I don't really see
the point of having the static-inline fallbacks.

fwiw, using 'select' is what I was doing for lcd panel support for
lcdc/da8xx drm driver (which was using the of videomode helpers,
albeit a slightly earlier version of the patches):

https://github.com/robclark/kernel-omap4/commit/e2aef5f281348afaaaeaa132699efc2831aa8384

BR,
-R


 And testing was done over v3.8-rc2.

   +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
 
  As _OF_VIDEOMODE is a bool type CONFIG, isn't,
 
  #ifdef CONFIG_OF_VIDEOMODE
 
  sufficient ?
 

 Yes, that is right. But I think IS_ENABLED is the preferred way to do it, 
 isn't it?

 Now I realize it is.

 Regards
 Afzal
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCHv16 5/7] fbmon: add of_videomode helpers

2013-01-07 Thread Mohammed, Afzal

Hi Rob,

On Tue, Jan 08, 2013 at 01:36:50, Rob Clark wrote:
 On Mon, Jan 7, 2013 at 2:46 AM, Mohammed, Afzal af...@ti.com wrote:
  On Mon, Jan 07, 2013 at 13:36:48, Steffen Trumtrar wrote:

  I just did a quick make da8xx_omapl_defconfig  make and it builds just 
  fine.
  On what version did you apply the series?
  At the moment I have the series sitting on 3.7. Didn't try any 3.8-rcx yet.
  But fixing this shouldn't be a problem.

  The change as I mentioned or something similar would be required as
  any driver that is going to make use of of_get_fb_videomode() would
  break if CONFIG_OF_VIDEOMODE or CONFIG_FB_MODE_HELPERS is not defined.

 Shouldn't the driver that depends on CONFIG_OF_VIDEOMODE and
 CONFIG_FB_MODE_HELPERS, explicitly select them?  I don't really see
 the point of having the static-inline fallbacks.

But here da8xx-fb driver does not depend on _OF_VIDEOMODE and
_FB_MODE_HELPERS, currently it works as a pure platform driver
for DaVinci SoC's without those CONFIG's. It is only upon
enhancing the driver to make use of of_get_fb_videomode() for
DT support those CONFIG's are being made use of.

As the driver can work w/o these CONFIG's and so as it is not a
dependency for driver on non-DT boot (as in the case of DaVinci),
I disagree in selecting those options always, but rather giving
user an option to select.

And selecting these options always will bring in some amount of code
onto Kernel image w/o any purpose in the case of DaVinci builds.

Another option would be to sprinkle driver with ifdef's to avoid
inline fallbacks, which is not a good thing to do.

Moreover having a static inline fallback is more in line with other
of_*'s.

 fwiw, using 'select' is what I was doing for lcd panel support for
 lcdc/da8xx drm driver (which was using the of videomode helpers,
 albeit a slightly earlier version of the patches):

In your case as it is a new driver  is meant only for DT, that
is fine, but here it is an existing driver that works w/o these.

Regards
Afzal

RE: [PATCHv16 5/7] fbmon: add of_videomode helpers

2013-01-06 Thread Mohammed, Afzal

Hi Steffen,

On Tue, Dec 18, 2012 at 22:34:14, Steffen Trumtrar wrote:
 Add helper to get fb_videomode from devicetree.

  drivers/video/fbmon.c |   42 ++
  include/linux/fb.h|4 

This breaks DaVinci (da8xx_omapl_defconfig), following change was
required to get it build if OF_VIDEOMODE or/and FB_MODE_HELPERS
is not defined. There may be better solutions, following was the
one that was used by me to test this series.

---8--

diff --git a/include/linux/fb.h b/include/linux/fb.h
index 58b9860..0ce30d1 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -716,9 +716,19 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);

+#if defined(CONFIG_OF_VIDEOMODE)  defined(CONFIG_FB_MODE_HELPERS)
 extern int of_get_fb_videomode(struct device_node *np,
   struct fb_videomode *fb,
   int index);
+#else
+static inline int of_get_fb_videomode(struct device_node *np,
+ struct fb_videomode *fb,
+ int index)
+{
+   return -EINVAL;
+}
+#endif
+
 extern int fb_videomode_from_videomode(const struct videomode *vm,
   struct fb_videomode *fbmode);

---8--


 +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)

As _OF_VIDEOMODE is a bool type CONFIG, isn't,

#ifdef CONFIG_OF_VIDEOMODE

sufficient ?

Regards
Afzal

[PATCHv16 5/7] fbmon: add of_videomode helpers

2012-12-18 Thread Steffen Trumtrar

Add helper to get fb_videomode from devicetree.

Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
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
---
 drivers/video/fbmon.c |   42 ++
 include/linux/fb.h|4 
 2 files changed, 46 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 17ce135..94ad0f7 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include linux/pci.h
 #include linux/slab.h
 #include video/edid.h
+#include video/of_videomode.h
 #include video/videomode.h
 #ifdef CONFIG_PPC_OF
 #include asm/prom.h
@@ -1425,6 +1426,47 @@ int fb_videomode_from_videomode(const struct videomode 
*vm,
 EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static inline void dump_fb_videomode(const struct fb_videomode *m)
+{
+   pr_debug(fb_videomode = %ux%u@%uHz (%ukHz) %u %u %u %u %u %u %u %u 
%u\n,
+m-xres, m-yres, m-refresh, m-pixclock, m-left_margin,
+m-right_margin, m-upper_margin, m-lower_margin,
+m-hsync_len, m-vsync_len, m-sync, m-vmode, m-flag);
+}
+
+/**
+ * of_get_fb_videomode - get a fb_videomode from devicetree
+ * @np: device_node with the timing specification
+ * @fb: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function is expensive and should only be used, if only one mode is to 
be
+ * read from DT. To get multiple modes start with of_get_display_timings ond
+ * work with that instead.
+ */
+int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+   int index)
+{
+   struct videomode vm;
+   int ret;
+
+   ret = of_get_videomode(np, vm, index);
+   if (ret)
+   return ret;
+
+   fb_videomode_from_videomode(vm, fb);
+
+   pr_debug(%s: got %dx%d display mode from %s\n,
+   of_node_full_name(np), vm.hactive, vm.vactive, np-name);
+   dump_fb_videomode(fb);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_fb_videomode);
+#endif
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 100a176..58b9860 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -20,6 +20,7 @@ struct fb_info;
 struct device;
 struct file;
 struct videomode;
+struct device_node;
 
 /* Definitions below are used in the parsed monitor specs */
 #define FB_DPMS_ACTIVE_OFF 1
@@ -715,6 +716,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+extern int of_get_fb_videomode(struct device_node *np,
+  struct fb_videomode *fb,
+  int index);
 extern int fb_videomode_from_videomode(const struct videomode *vm,
   struct fb_videomode *fbmode);
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv16 5/7] fbmon: add of_videomode helpers

2012-12-18 Thread Steffen Trumtrar

Add helper to get fb_videomode from devicetree.

Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
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
---
 drivers/video/fbmon.c |   42 ++
 include/linux/fb.h|4 
 2 files changed, 46 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 17ce135..94ad0f7 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,6 +31,7 @@
 #include linux/pci.h
 #include linux/slab.h
 #include video/edid.h
+#include video/of_videomode.h
 #include video/videomode.h
 #ifdef CONFIG_PPC_OF
 #include asm/prom.h
@@ -1425,6 +1426,47 @@ int fb_videomode_from_videomode(const struct videomode 
*vm,
 EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static inline void dump_fb_videomode(const struct fb_videomode *m)
+{
+   pr_debug(fb_videomode = %ux%u@%uHz (%ukHz) %u %u %u %u %u %u %u %u 
%u\n,
+m-xres, m-yres, m-refresh, m-pixclock, m-left_margin,
+m-right_margin, m-upper_margin, m-lower_margin,
+m-hsync_len, m-vsync_len, m-sync, m-vmode, m-flag);
+}
+
+/**
+ * of_get_fb_videomode - get a fb_videomode from devicetree
+ * @np: device_node with the timing specification
+ * @fb: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * DESCRIPTION:
+ * This function is expensive and should only be used, if only one mode is to 
be
+ * read from DT. To get multiple modes start with of_get_display_timings ond
+ * work with that instead.
+ */
+int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+   int index)
+{
+   struct videomode vm;
+   int ret;
+
+   ret = of_get_videomode(np, vm, index);
+   if (ret)
+   return ret;
+
+   fb_videomode_from_videomode(vm, fb);
+
+   pr_debug(%s: got %dx%d display mode from %s\n,
+   of_node_full_name(np), vm.hactive, vm.vactive, np-name);
+   dump_fb_videomode(fb);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_fb_videomode);
+#endif
+
 #else
 int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
 {
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 100a176..58b9860 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -20,6 +20,7 @@ struct fb_info;
 struct device;
 struct file;
 struct videomode;
+struct device_node;
 
 /* Definitions below are used in the parsed monitor specs */
 #define FB_DPMS_ACTIVE_OFF 1
@@ -715,6 +716,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
 extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
 extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
 
+extern int of_get_fb_videomode(struct device_node *np,
+  struct fb_videomode *fb,
+  int index);
 extern int fb_videomode_from_videomode(const struct videomode *vm,
   struct fb_videomode *fbmode);
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv16 5/7] fbmon: add of_videomode helpers

RE: [PATCHv16 5/7] fbmon: add of_videomode helpers

Re: [PATCHv16 5/7] fbmon: add of_videomode helpers

RE: [PATCHv16 5/7] fbmon: add of_videomode helpers

RE: [PATCHv16 5/7] fbmon: add of_videomode helpers

[PATCHv16 5/7] fbmon: add of_videomode helpers

[PATCHv16 5/7] fbmon: add of_videomode helpers

7 matches

Site Navigation

Mail list logo

Footer information