date:20121114

Re: Support for Terratec Cinergy 2400i DT in kernel 3.x

2012-11-14 Thread Frédéric

Le mardi 13 novembre 2012, Patrice Chotard a écrit :

 Two patches have been already submitted and are available since v3.7-rc1
 
 media] ngene: add support for Terratec Cynergy 2400i Dual DVB-T  :
 397e972350c42cbaf3228fe2eec23fecf6a69903
 
 and
 
 media] dvb: add support for Thomson DTT7520X :
 5fb67074c6657edc34867cba78255b6f5b505f12

I had a look at your patches. I don't see the '.fw_version' param anymore in 
the 'ngene_info' 
structure... Is it normal?

-- 
   Frédéric
--
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: [PATCH 1/1] media: Entities with sink pads must have at least one enabled link

2012-11-14 Thread Sylwester Nawrocki

Hi Sakari,

On 11/13/2012 03:24 PM, Sakari Ailus wrote:
 Hi all,
 
 Comments would be appreciated, either positive or negative. The omap3isp
 driver does the same check itself currently, but I think this is more
 generic than that.
 
 Thanks.
 
 On Fri, Oct 26, 2012 at 10:46:17PM +0300, Sakari Ailus wrote:
 If an entity has sink pads, at least one of them must be connected to
 another pad with an enabled link. If a driver with multiple sink pads has
 more strict requirements the check should be done in the driver itself.

 Just requiring one sink pad is connected with an enabled link is enough
 API-wise: entities with sink pads with only disabled links should not be
 allowed to stream in the first place, but also in a different operation mode
 a device might require only one of its pads connected with an active link.

 If an entity has an ability to function as a source entity another logical
 entity connected to the aforementioned one should be used for the purpose.

Why not leave it to individual drivers ? I'm not sure if it is a good idea
not to allow an entity with sink pads to be used as a source only. It might
be appropriate for most of the cases but likely not all. I'm inclined not to
add this requirement in the API. Just my opinion though.

--
Thanks,
Sylwester

--
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

[GIT PULL FOR v3.8] V4L subdev fix

2012-11-14 Thread Laurent Pinchart

Hi Mauro,

The following changes since commit 2c4e11b7c15af70580625657a154ea7ea70b8c76:

  [media] siano: fix RC compilation (2012-11-07 11:09:08 +0100)

are available in the git repository at:
  git://linuxtv.org/pinchartl/media.git v4l2/core

Laurent Pinchart (1):
  v4l: Don't warn during link validation when encountering a V4L2 devnode

 drivers/media/v4l2-core/v4l2-subdev.c |   22 +++---
 1 files changed, 11 insertions(+), 11 deletions(-

-- 
Regards,

Laurent Pinchart

--
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: Support for Terratec Cinergy 2400i DT in kernel 3.x

2012-11-14 Thread Frédéric

Le mercredi 14 novembre 2012, Frédéric a écrit :

 I had a look at your patches. I don't see the '.fw_version' param anymore
 in the 'ngene_info' structure... Is it normal?

I also noticed some differences in the PLL presets:

In your patch:

.entries = {
{  30500, 17, 0xb4, 0x12 },
{  40500, 17, 0xbc, 0x12 },
{  44500, 17, 0xbc, 0x12 },
{  46500, 17, 0xf4, 0x18 },
{  73500, 17, 0xfc, 0x18 },
{  83500, 17, 0xbc, 0x18 },
{  9, 17, 0xfc, 0x18 },
},

In original patch:

if (freq17700 || freq85800)
return -EINVAL;
else if (freq30500) { c1=0xb4; c2=0x12; }
else if (freq40500) { c1=0xbc; c2=0x12; }
else if (freq44500) { c1=0xf4; c2=0x12; }
else if (freq46500) { c1=0xfc; c2=0x12; }
else if (freq73500) { c1=0xbc; c2=0x18; }
else if (freq83500) { c1=0xf4; c2=0x18; }
else { c1=0xfc; c2=0x18; }

-- 
   Frédéric
--
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: DVB V5 API: Event Model

2012-11-14 Thread Martin Rudge

Thanks Mauro.

I'm not familiar with the code, however I'll review the design/code and see 
what could be done.

A quick look suggests the events are originating from the tuning loop, so it's 
down to the timing of copying the parms for voltage/tone (not currently 
instigated by a DTV_TUNE).

Thanks
Martin

On 13 Nov 2012, at 09:24, Mauro Carvalho Chehab mche...@redhat.com wrote:

 Em 12-11-2012 11:04, Martin Rudge escreveu:
 Hello All,
 
 When using the V5 API (DVB-S/S2) for the DVB frontend device (with the
 now merged SEC functionality), setting properties DTV_VOLTAGE and/or
 DTV_TONE generates extra (unwanted?) events.  This is due to utilising
 the legacy FE_SET_FRONTEND IOCTL in their respective implementations.
 
 Depending on their placement in one atomic FE_SET_PROPERTY call,
 they can cause an incorrect (premature) SYNC/LOCK event to be
 generated.  For example, when looping issuing tune requests in
 succession during a scan operation. This was with a fairly recent
 media build (pulled Saturday).
 
 I suspect that this is an undesirable behavior, likely there since the initial
 version of DVB-S2 API. It could be too late to fix, as userspace apps may be
 trusting on this behavior.
 
 Maybe you could propose a patch and ask app developers what they thing about
 it.
 
 Conversly using DTV_CLEAR clears the cached values, but doesn't affect
 the frontend (LNB).  This is probably desirable behaviour.
 
 This is desirable.
 
 Any thoughts, working as designed/intended?
 
 Regards,
 Mauro
--
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

Concerning OMAP v4l2 driver (omap_vout)

2012-11-14 Thread Tomi Valkeinen

Hi Vaibhav,

I'd like to get clarity on the omap_vout maintenance. You've been the
maintainer of omap_vout, but you have lately been quite inactive in this
role, and getting omap_vout patches merged has not been as fluent as it
could be.

Do you still want to continue in this role? Will you have time for it?
Any ideas or suggestions how we should manage omap_vout in the future?

 Tomi



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v8 1/6] video: add display_timing and videomode

2012-11-14 Thread Thierry Reding

On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
[...]
 diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
[...]
 +void display_timings_release(struct display_timings *disp)
 +{
 + if (disp-timings) {
 + unsigned int i;
 +
 + for (i = 0; i  disp-num_timings; i++)
 + kfree(disp-timings[i]);
 + kfree(disp-timings);
 + }
 + kfree(disp);
 +}

I think this is still missing an EXPORT_SYMBOL_GPL. Otherwise it can't
be used from modules.

Thierry


pgpBUyLwaWzk6.pgp
Description: PGP signature

Re: [PATCH 1/1] media: Entities with sink pads must have at least one enabled link

2012-11-14 Thread Laurent Pinchart

Hi Sylwester,

On Wednesday 14 November 2012 10:23:19 Sylwester Nawrocki wrote:
 On 11/13/2012 03:24 PM, Sakari Ailus wrote:
  Hi all,
  
  Comments would be appreciated, either positive or negative. The omap3isp
  driver does the same check itself currently, but I think this is more
  generic than that.
  
  Thanks.
  
  On Fri, Oct 26, 2012 at 10:46:17PM +0300, Sakari Ailus wrote:
  If an entity has sink pads, at least one of them must be connected to
  another pad with an enabled link. If a driver with multiple sink pads has
  more strict requirements the check should be done in the driver itself.
  
  Just requiring one sink pad is connected with an enabled link is enough
  API-wise: entities with sink pads with only disabled links should not be
  allowed to stream in the first place, but also in a different operation
  mode a device might require only one of its pads connected with an
  active link.
  
  If an entity has an ability to function as a source entity another
  logical entity connected to the aforementioned one should be used for the
  purpose.
 
 Why not leave it to individual drivers ? I'm not sure if it is a good idea
 not to allow an entity with sink pads to be used as a source only. It might
 be appropriate for most of the cases but likely not all. I'm inclined not to
 add this requirement in the API. Just my opinion though.

I have mixed feelings about this patch too, which is why I've asked Sakari to 
cross-post it. It's pretty easy to add this check to the core now, but pushing 
it back to drivers late if we realize it's too restrictive would be difficult. 
I think my preference would go for a helper function that drivers can use, 
possibly first waiting until a second driver requires this kind of checks 
before implementing it.

-- 
Regards,

Laurent Pinchart

--
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: [PATCH v8 2/6] video: add of helper for videomode

2012-11-14 Thread Thierry Reding

On Mon, Nov 12, 2012 at 04:37:02PM +0100, Steffen Trumtrar wrote:
[...]
 diff --git a/include/linux/of_display_timings.h 
 b/include/linux/of_display_timings.h
[...]
 +#ifndef __LINUX_OF_DISPLAY_TIMINGS_H
 +#define __LINUX_OF_DISPLAY_TIMINGS_H
 +
 +#include linux/display_timing.h
 +
 +#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);

This either needs to include linux/of.h or a forward declaration of
struct device_node. Otherwise this will fail to compile if the file
where this is included from doesn't pull linux/of.h in explicitly.

 diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
[...]
 +#ifndef __LINUX_OF_VIDEOMODE_H
 +#define __LINUX_OF_VIDEOMODE_H
 +
 +#include linux/videomode.h
 +
 +int of_get_videomode(struct device_node *np, struct videomode *vm, int 
 index);

Same here.

Thierry


pgpDu4puXcSWO.pgp
Description: PGP signature

Re: [PATCH v8 1/6] video: add display_timing and videomode

2012-11-14 Thread Steffen Trumtrar

On Wed, Nov 14, 2012 at 11:56:34AM +0100, Thierry Reding wrote:
 On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
 [...]
  diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
 [...]
  +void display_timings_release(struct display_timings *disp)
  +{
  +   if (disp-timings) {
  +   unsigned int i;
  +
  +   for (i = 0; i  disp-num_timings; i++)
  +   kfree(disp-timings[i]);
  +   kfree(disp-timings);
  +   }
  +   kfree(disp);
  +}
 
 I think this is still missing an EXPORT_SYMBOL_GPL. Otherwise it can't
 be used from modules.
 
 Thierry

Yes. Just in time. I was just starting to type the send-email command ;-)

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: [PATCH v8 1/6] video: add display_timing and videomode

2012-11-14 Thread Thierry Reding

On Wed, Nov 14, 2012 at 11:59:25AM +0100, Steffen Trumtrar wrote:
 On Wed, Nov 14, 2012 at 11:56:34AM +0100, Thierry Reding wrote:
  On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
  [...]
   diff --git a/drivers/video/display_timing.c 
   b/drivers/video/display_timing.c
  [...]
   +void display_timings_release(struct display_timings *disp)
   +{
   + if (disp-timings) {
   + unsigned int i;
   +
   + for (i = 0; i  disp-num_timings; i++)
   + kfree(disp-timings[i]);
   + kfree(disp-timings);
   + }
   + kfree(disp);
   +}
  
  I think this is still missing an EXPORT_SYMBOL_GPL. Otherwise it can't
  be used from modules.
  
  Thierry
 
 Yes. Just in time. I was just starting to type the send-email command ;-)

Great! In that case don't forget to also look at my other email before
sending. =)

Thierry


pgpwvfZVVkKPO.pgp
Description: PGP signature

Re: [PATCH v8 1/6] video: add display_timing and videomode

2012-11-14 Thread Steffen Trumtrar

On Wed, Nov 14, 2012 at 12:02:15PM +0100, Thierry Reding wrote:
 On Wed, Nov 14, 2012 at 11:59:25AM +0100, Steffen Trumtrar wrote:
  On Wed, Nov 14, 2012 at 11:56:34AM +0100, Thierry Reding wrote:
   On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
   [...]
diff --git a/drivers/video/display_timing.c 
b/drivers/video/display_timing.c
   [...]
+void display_timings_release(struct display_timings *disp)
+{
+   if (disp-timings) {
+   unsigned int i;
+
+   for (i = 0; i  disp-num_timings; i++)
+   kfree(disp-timings[i]);
+   kfree(disp-timings);
+   }
+   kfree(disp);
+}
   
   I think this is still missing an EXPORT_SYMBOL_GPL. Otherwise it can't
   be used from modules.
   
   Thierry
  
  Yes. Just in time. I was just starting to type the send-email command ;-)
 
 Great! In that case don't forget to also look at my other email before
 sending. =)
 
Sure.


-- 
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: [PATCH v8 1/6] video: add display_timing and videomode

2012-11-14 Thread Thierry Reding

On Wed, Nov 14, 2012 at 12:10:15PM +0100, Steffen Trumtrar wrote:
 On Wed, Nov 14, 2012 at 12:02:15PM +0100, Thierry Reding wrote:
  On Wed, Nov 14, 2012 at 11:59:25AM +0100, Steffen Trumtrar wrote:
   On Wed, Nov 14, 2012 at 11:56:34AM +0100, Thierry Reding wrote:
On Mon, Nov 12, 2012 at 04:37:01PM +0100, Steffen Trumtrar wrote:
[...]
 diff --git a/drivers/video/display_timing.c 
 b/drivers/video/display_timing.c
[...]
 +void display_timings_release(struct display_timings *disp)
 +{
 + if (disp-timings) {
 + unsigned int i;
 +
 + for (i = 0; i  disp-num_timings; i++)
 + kfree(disp-timings[i]);
 + kfree(disp-timings);
 + }
 + kfree(disp);
 +}

I think this is still missing an EXPORT_SYMBOL_GPL. Otherwise it can't
be used from modules.

Thierry
   
   Yes. Just in time. I was just starting to type the send-email command ;-)
  
  Great! In that case don't forget to also look at my other email before
  sending. =)
  
 Sure.

Besides those comments (and those from other people) I think your
patchset is in pretty good shape. Have you thought about how and when
you want to get things merged?

Thierry


pgp6Ncscso1x3.pgp
Description: PGP signature

[PATCH v9 6/6] drm_modes: add of_videomode helpers

2012-11-14 Thread Steffen Trumtrar

Add helper to get drm_display_mode from devicetree.

Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
 drivers/gpu/drm/drm_modes.c |   35 ++-
 include/drm/drmP.h  |6 ++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 42ea099..c3ae5d2 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,7 +35,8 @@
 #include linux/export.h
 #include drm/drmP.h
 #include drm/drm_crtc.h
-#include linux/videomode.h
+#include linux/of.h
+#include linux/of_videomode.h
 
 /**
  * drm_mode_debug_printmodeline - debug print a mode
@@ -540,6 +541,38 @@ int display_mode_from_videomode(struct videomode *vm, 
struct drm_display_mode *d
 EXPORT_SYMBOL_GPL(display_mode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+/**
+ * of_get_drm_display_mode - get a drm_display_mode from devicetree
+ * @np: device_node with the timing specification
+ * @dmode: will be set to the return value
+ * @index: index into the list of display timings in devicetree
+ *
+ * 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 and
+ * work with that instead.
+ */
+int of_get_drm_display_mode(struct device_node *np, struct drm_display_mode 
*dmode,
+   unsigned int index)
+{
+   struct videomode vm;
+   int ret;
+
+   ret = of_get_videomode(np, vm, index);
+   if (ret)
+   return ret;
+
+   display_mode_from_videomode(vm, dmode);
+
+   pr_info(%s: got %dx%d display mode from %s\n, __func__, vm.hactive,
+   vm.vactive, np-name);
+   drm_mode_debug_printmodeline(dmode);
+
+   return 0;
+
+}
+EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
+#endif
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 1e0d846..e8f46a1 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -56,6 +56,7 @@
 #include linux/cdev.h
 #include linux/mutex.h
 #include linux/slab.h
+#include linux/of.h
 #include linux/videomode.h
 #if defined(__alpha__) || defined(__powerpc__)
 #include asm/pgtable.h   /* For pte_wrprotect */
@@ -1459,6 +1460,11 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
 extern int display_mode_from_videomode(struct videomode *vm,
   struct drm_display_mode *dmode);
 #endif
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+extern int of_get_drm_display_mode(struct device_node *np,
+  struct drm_display_mode *dmode,
+  unsigned int index);
+#endif
 
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
-- 
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

[PATCH v9 1/6] video: add display_timing and videomode

2012-11-14 Thread Steffen Trumtrar

Add display_timing structure and the according helper functions. This allows
the description of a display via its supported timing parameters.

Every timing parameter can be specified as a single value or a range
min typ max.

Also, add helper functions to convert from display timings to a generic 
videomode
structure. This videomode can then be converted to the corresponding subsystem
mode representation (e.g. fb_videomode).

Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
 drivers/video/Kconfig  |6 
 drivers/video/Makefile |2 ++
 drivers/video/display_timing.c |   24 ++
 drivers/video/videomode.c  |   45 ++
 include/linux/display_timing.h |   69 
 include/linux/videomode.h  |   39 +++
 6 files changed, 185 insertions(+)
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/linux/display_timing.h
 create mode 100644 include/linux/videomode.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d08d799..2a23b18 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -33,6 +33,12 @@ config VIDEO_OUTPUT_CONTROL
  This framework adds support for low-level control of the video 
  output switch.
 
+config DISPLAY_TIMING
+   bool
+
+config VIDEOMODE
+   bool
+
 menuconfig FB
tristate Support for frame buffer devices
---help---
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 23e948e..fc30439 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -167,3 +167,5 @@ 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_VIDEOMODE) += videomode.o
diff --git a/drivers/video/display_timing.c b/drivers/video/display_timing.c
new file mode 100644
index 000..ac9bbbc
--- /dev/null
+++ b/drivers/video/display_timing.c
@@ -0,0 +1,24 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include linux/display_timing.h
+#include linux/export.h
+#include linux/slab.h
+
+void display_timings_release(struct display_timings *disp)
+{
+   if (disp-timings) {
+   unsigned int i;
+
+   for (i = 0; i  disp-num_timings; i++)
+   kfree(disp-timings[i]);
+   kfree(disp-timings);
+   }
+   kfree(disp);
+}
+EXPORT_SYMBOL_GPL(display_timings_release);
diff --git a/drivers/video/videomode.c b/drivers/video/videomode.c
new file mode 100644
index 000..087374a
--- /dev/null
+++ b/drivers/video/videomode.c
@@ -0,0 +1,45 @@
+/*
+ * generic display timing functions
+ *
+ * Copyright (c) 2012 Steffen Trumtrar s.trumt...@pengutronix.de, Pengutronix
+ *
+ * This file is released under the GPLv2
+ */
+
+#include linux/export.h
+#include linux/errno.h
+#include linux/display_timing.h
+#include linux/kernel.h
+#include linux/videomode.h
+
+int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
+ unsigned int index)
+{
+   struct display_timing *dt;
+
+   dt = display_timings_get(disp, index);
+   if (!dt)
+   return -EINVAL;
+
+   vm-pixelclock = display_timing_get_value(dt-pixelclock, 0);
+   vm-hactive = display_timing_get_value(dt-hactive, 0);
+   vm-hfront_porch = display_timing_get_value(dt-hfront_porch, 0);
+   vm-hback_porch = display_timing_get_value(dt-hback_porch, 0);
+   vm-hsync_len = display_timing_get_value(dt-hsync_len, 0);
+
+   vm-vactive = display_timing_get_value(dt-vactive, 0);
+   vm-vfront_porch = display_timing_get_value(dt-vfront_porch, 0);
+   vm-vback_porch = display_timing_get_value(dt-vback_porch, 0);
+   vm-vsync_len = display_timing_get_value(dt-vsync_len, 0);
+
+   vm-vah = dt-vsync_pol_active;
+   vm-hah = dt-hsync_pol_active;
+   vm-de = dt-de_pol_active;
+   vm-pixelclk_pol = dt-pixelclk_pol;
+
+   vm-interlaced = dt-interlaced;
+   vm-doublescan = dt-doublescan;
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(videomode_from_timing);
diff --git a/include/linux/display_timing.h b/include/linux/display_timing.h
new file mode 100644
index 000..caee2a8
--- /dev/null
+++ b/include/linux/display_timing.h
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2012 Steffen Trumtrar s.trumt...@pengutronix.de
+ *
+ * description of display timings
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_DISPLAY_TIMINGS_H
+#define __LINUX_DISPLAY_TIMINGS_H
+
+#include linux/types.h
+
+struct timing_entry {
+   u32 min;
+   u32 typ;
+   u32 max;
+};
+
+struct display_timing {
+   struct timing_entry pixelclock;
+
+   struct timing_entry hactive;
+

[PATCH v9 5/6] drm_modes: add videomode helpers

2012-11-14 Thread Steffen Trumtrar

Add conversion from videomode to drm_display_mode

Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
 drivers/gpu/drm/drm_modes.c |   36 
 include/drm/drmP.h  |6 ++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 59450f3..42ea099 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -35,6 +35,7 @@
 #include linux/export.h
 #include drm/drmP.h
 #include drm/drm_crtc.h
+#include linux/videomode.h
 
 /**
  * drm_mode_debug_printmodeline - debug print a mode
@@ -504,6 +505,41 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int 
vdisplay, int vrefresh,
 }
 EXPORT_SYMBOL(drm_gtf_mode);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int display_mode_from_videomode(struct videomode *vm, struct drm_display_mode 
*dmode)
+{
+   dmode-hdisplay = vm-hactive;
+   dmode-hsync_start = dmode-hdisplay + vm-hfront_porch;
+   dmode-hsync_end = dmode-hsync_start + vm-hsync_len;
+   dmode-htotal = dmode-hsync_end + vm-hback_porch;
+
+   dmode-vdisplay = vm-vactive;
+   dmode-vsync_start = dmode-vdisplay + vm-vfront_porch;
+   dmode-vsync_end = dmode-vsync_start + vm-vsync_len;
+   dmode-vtotal = dmode-vsync_end + vm-vback_porch;
+
+   dmode-clock = vm-pixelclock / 1000;
+
+   dmode-flags = 0;
+   if (vm-hah)
+   dmode-flags |= DRM_MODE_FLAG_PHSYNC;
+   else
+   dmode-flags |= DRM_MODE_FLAG_NHSYNC;
+   if (vm-vah)
+   dmode-flags |= DRM_MODE_FLAG_PVSYNC;
+   else
+   dmode-flags |= DRM_MODE_FLAG_NVSYNC;
+   if (vm-interlaced)
+   dmode-flags |= DRM_MODE_FLAG_INTERLACE;
+   if (vm-doublescan)
+   dmode-flags |= DRM_MODE_FLAG_DBLSCAN;
+   drm_mode_set_name(dmode);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(display_mode_from_videomode);
+#endif
+
 /**
  * drm_mode_set_name - set the name on a mode
  * @mode: name will be set in this mode
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 3fd8280..1e0d846 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -56,6 +56,7 @@
 #include linux/cdev.h
 #include linux/mutex.h
 #include linux/slab.h
+#include linux/videomode.h
 #if defined(__alpha__) || defined(__powerpc__)
 #include asm/pgtable.h   /* For pte_wrprotect */
 #endif
@@ -1454,6 +1455,11 @@ extern struct drm_display_mode *
 drm_mode_create_from_cmdline_mode(struct drm_device *dev,
  struct drm_cmdline_mode *cmd);
 
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+extern int display_mode_from_videomode(struct videomode *vm,
+  struct drm_display_mode *dmode);
+#endif
+
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
 extern void drm_vblank_post_modeset(struct drm_device *dev, int crtc);
-- 
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

[PATCH v9 0/6] of: add display helper

2012-11-14 Thread Steffen Trumtrar

Hi!

Changes since v8:
- fix memory leaks
- change API to be more consistent (foo_from_bar(struct bar, struct 
foo))
- include headers were necessary
- misc minor bufixes

Regards,
Steffen


Steffen Trumtrar (6):
  video: add display_timing and videomode
  video: add of helper for videomode
  fbmon: add videomode helpers
  fbmon: add of_videomode helpers
  drm_modes: add videomode helpers
  drm_modes: add of_videomode helpers

 .../devicetree/bindings/video/display-timings.txt  |  107 ++
 drivers/gpu/drm/drm_modes.c|   69 +++
 drivers/video/Kconfig  |   19 ++
 drivers/video/Makefile |4 +
 drivers/video/display_timing.c |   24 +++
 drivers/video/fbmon.c  |   78 
 drivers/video/of_display_timing.c  |  211 
 drivers/video/of_videomode.c   |   47 +
 drivers/video/videomode.c  |   45 +
 include/drm/drmP.h |   12 ++
 include/linux/display_timing.h |   69 +++
 include/linux/fb.h |   11 +
 include/linux/of_display_timings.h |   20 ++
 include/linux/of_videomode.h   |   16 ++
 include/linux/videomode.h  |   39 
 15 files changed, 771 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timings.txt
 create mode 100644 drivers/video/display_timing.c
 create mode 100644 drivers/video/of_display_timing.c
 create mode 100644 drivers/video/of_videomode.c
 create mode 100644 drivers/video/videomode.c
 create mode 100644 include/linux/display_timing.h
 create mode 100644 include/linux/of_display_timings.h
 create mode 100644 include/linux/of_videomode.h
 create mode 100644 include/linux/videomode.h

-- 
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

[PATCH v9 3/6] fbmon: add videomode helpers

2012-11-14 Thread Steffen Trumtrar

Add a function to convert from the generic videomode to a fb_videomode.

Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
 drivers/video/fbmon.c |   38 ++
 include/linux/fb.h|5 +
 2 files changed, 43 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cef6557..cccef17 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 linux/videomode.h
 #ifdef CONFIG_PPC_OF
 #include asm/prom.h
 #include asm/pci-bridge.h
@@ -1373,6 +1374,43 @@ int fb_get_mode(int flags, u32 val, struct 
fb_var_screeninfo *var, struct fb_inf
kfree(timings);
return err;
 }
+
+#if IS_ENABLED(CONFIG_VIDEOMODE)
+int fb_videomode_from_videomode(struct videomode *vm, struct fb_videomode 
*fbmode)
+{
+   fbmode-xres = vm-hactive;
+   fbmode-left_margin = vm-hback_porch;
+   fbmode-right_margin = vm-hfront_porch;
+   fbmode-hsync_len = vm-hsync_len;
+
+   fbmode-yres = vm-vactive;
+   fbmode-upper_margin = vm-vback_porch;
+   fbmode-lower_margin = vm-vfront_porch;
+   fbmode-vsync_len = vm-vsync_len;
+
+   fbmode-pixclock = KHZ2PICOS(vm-pixelclock / 1000);
+
+   fbmode-sync = 0;
+   fbmode-vmode = 0;
+   if (vm-hah)
+   fbmode-sync |= FB_SYNC_HOR_HIGH_ACT;
+   if (vm-vah)
+   fbmode-sync |= FB_SYNC_VERT_HIGH_ACT;
+   if (vm-interlaced)
+   fbmode-vmode |= FB_VMODE_INTERLACED;
+   if (vm-doublescan)
+   fbmode-vmode |= FB_VMODE_DOUBLE;
+   if (vm-de)
+   fbmode-sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
+   fbmode-refresh = (vm-pixelclock*1000) / (vm-hactive * vm-vactive);
+   fbmode-flag = 0;
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(fb_videomode_from_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 c7a9571..6a3a675 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -14,6 +14,7 @@
 #include linux/backlight.h
 #include linux/slab.h
 #include asm/io.h
+#include linux/videomode.h
 
 struct vm_area_struct;
 struct fb_info;
@@ -714,6 +715,10 @@ 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 IS_ENABLED(CONFIG_VIDEOMODE)
+extern int fb_videomode_from_videomode(struct videomode *vm,
+  struct fb_videomode *fbmode);
+#endif
 /* drivers/video/modedb.c */
 #define VESA_MODEDB_SIZE 34
 extern void fb_var_to_videomode(struct fb_videomode *mode,
-- 
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

[PATCH v9 4/6] fbmon: add of_videomode helpers

2012-11-14 Thread Steffen Trumtrar

Add helper to get fb_videomode from devicetree.

Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
---
 drivers/video/fbmon.c |   42 +-
 include/linux/fb.h|6 ++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index cccef17..3a48abd 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -31,7 +31,7 @@
 #include linux/pci.h
 #include linux/slab.h
 #include video/edid.h
-#include linux/videomode.h
+#include linux/of_videomode.h
 #ifdef CONFIG_PPC_OF
 #include asm/prom.h
 #include asm/pci-bridge.h
@@ -1410,6 +1410,46 @@ int fb_videomode_from_videomode(struct videomode *vm, 
struct fb_videomode *fbmod
 EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
 #endif
 
+#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
+static void dump_fb_videomode(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,
+   unsigned 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_info(%s: got %dx%d display mode from %s\n, __func__, 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 6a3a675..8aeece8 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -15,6 +15,8 @@
 #include linux/slab.h
 #include asm/io.h
 #include linux/videomode.h
+#include linux/of.h
+#include linux/of_videomode.h
 
 struct vm_area_struct;
 struct fb_info;
@@ -715,6 +717,10 @@ 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 IS_ENABLED(CONFIG_OF_VIDEOMODE)
+extern int of_get_fb_videomode(struct device_node *np, struct fb_videomode *fb,
+  unsigned int index);
+#endif
 #if IS_ENABLED(CONFIG_VIDEOMODE)
 extern int fb_videomode_from_videomode(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

[PATCH v9 2/6] video: add of helper for videomode

2012-11-14 Thread Steffen Trumtrar

This adds support for reading display timings from DT or/and convert one of 
those
timings to a videomode.
The of_display_timing implementation supports multiple children where each
property can have up to 3 values. All children are read into an array, that
can be queried.
of_get_videomode converts exactly one of that timings to a struct videomode.

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
---
 .../devicetree/bindings/video/display-timings.txt  |  107 ++
 drivers/video/Kconfig  |   13 ++
 drivers/video/Makefile |2 +
 drivers/video/of_display_timing.c  |  211 
 drivers/video/of_videomode.c   |   47 +
 include/linux/of_display_timings.h |   20 ++
 include/linux/of_videomode.h   |   16 ++
 7 files changed, 416 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/display-timings.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_timings.h
 create mode 100644 include/linux/of_videomode.h

diff --git a/Documentation/devicetree/bindings/video/display-timings.txt 
b/Documentation/devicetree/bindings/video/display-timings.txt
new file mode 100644
index 000..c9d9e1b
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/display-timings.txt
@@ -0,0 +1,107 @@
+display-timings 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.
+
+timings 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: displayclock 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/non-inverted/ignored
+ - interlaced (bool)
+ - doublescan (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
+
+struct display_timing
+=
+
+  +--+-+--+---+
+  |  |↑|  |   |
+  |  ||vback_porch |  |   |
+  |  |↓|  |   |
+  +--###--+---+
+  |  #↑#  |   |
+  |  #|#  |   |
+  |  hback   #|#  hfront  | hsync |
+  |   porch  #|   hactive  #  porch   |  len  |
+  |#---+---#|-|
+  |  #|#  |   |
+  |  #|vactive #  |   |
+  |  #|#  |   |
+  |  #↓#  |   |
+  +--###--+---+
+  |  |↑|  |   |
+  |  ||vfront_porch|  |   |
+  |  |↓|  |   |
+  +--+-+--+---+
+  |  |↑|  |   |
+  |  ||vsync_len   |  |   |
+  |  |↓|  |   |
+  +--+-+--+---+
+
+
+Example:
+
+   display-timings {
+   native-mode = timing0;
+   timing0: 1920p24 {
+

Re: [PATCH v9 2/6] video: add of helper for videomode

2012-11-14 Thread Thierry Reding

On Wed, Nov 14, 2012 at 12:43:19PM +0100, Steffen Trumtrar wrote:
[...]
 +display-timings bindings
 +
 +
 +display timings node

I didn't express myself very clearly here =). The way I think this
should be written is display-timings node.

 +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: displayclock in Hz

I still think displayclock should be two words: display clock.

 +/**
 + * 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)
 +{
[...]
 + 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;

I believe you're still potentially leaking memory here. In case you have
5 entries for instance, and the last one fails to parse, then this will
cause the memory allocated for the 4 correct entries to be lost.

Can't you just call display_timings_release() in this case and then jump
to dispfail? That would still leak the native_mode device node. Maybe it
would be better to keep timingfail but modify it to free the display
timings with display_timings_release() instead? See below.

 + }
 +
 + if (native_mode == entry)
 + disp-native_mode = disp-num_timings;
 +
 + disp-timings[disp-num_timings] = dt;
 + disp-num_timings++;
 + }
 + of_node_put(timings_np);
 + of_node_put(native_mode);
 +
 + if (disp-num_timings  0)
 + pr_info(%s: got %d timings. Using timing #%d as default\n, 
 __func__,
 + disp-num_timings , disp-native_mode + 1);
 + else {
 + pr_err(%s: no valid timings specified\n, __func__);
 + display_timings_release(disp);
 + return NULL;
 + }
 + return disp;
 +
 +timingfail:
 + if (native_mode)
 + of_node_put(native_mode);
 + kfree(disp-timings);

Call display_timings_release(disp) instead of kfree(disp-timings)?

 diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
 new file mode 100644
 index 000..4138758
 --- /dev/null
 +++ b/include/linux/of_videomode.h
 @@ -0,0 +1,16 @@
 +/*
 + * Copyright 2012 Steffen Trumtrar s.trumt...@pengutronix.de
 + *
 + * videomode of-helpers
 + *
 + * This file is released under the GPLv2
 + */
 +
 +#ifndef __LINUX_OF_VIDEOMODE_H
 +#define __LINUX_OF_VIDEOMODE_H
 +
 +#include linux/videomode.h
 +#include linux/of.h
 +
 +int of_get_videomode(struct device_node *np, struct videomode *vm, int 
 index);
 +#endif /* __LINUX_OF_VIDEOMODE_H */

Nit: should have a blank line before #endif.

Thierry


pgpxUoOQUBWUW.pgp
Description: PGP signature

Re: [PATCH v9 1/6] video: add display_timing and videomode

2012-11-14 Thread Thierry Reding

On Wed, Nov 14, 2012 at 12:43:18PM +0100, Steffen Trumtrar wrote:
[...]
 diff --git a/include/linux/videomode.h b/include/linux/videomode.h
[...]
 +int videomode_from_timing(struct display_timings *disp, struct videomode *vm,
 +   unsigned int index);
 +#endif

Nit: should have a blank line before the #endif.

Thierry


pgpHYim5YCYcT.pgp
Description: PGP signature

Re: [PATCH v9 2/6] video: add of helper for videomode

2012-11-14 Thread Thierry Reding

On Wed, Nov 14, 2012 at 12:43:19PM +0100, Steffen Trumtrar wrote:
[...]
 +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.

I forgot: The first sentence in this description should also start with
a capital letter as you terminate with a full stop.

Thierry


pgpSVTBNIhaJm.pgp
Description: PGP signature

Re: [PATCH v9 3/6] fbmon: add videomode helpers

2012-11-14 Thread Thierry Reding

On Wed, Nov 14, 2012 at 12:43:20PM +0100, Steffen Trumtrar wrote:
 Add a function to convert from the generic videomode to a fb_videomode.
 
 Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
 ---
  drivers/video/fbmon.c |   38 ++
  include/linux/fb.h|5 +
  2 files changed, 43 insertions(+)
 
 diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
 index cef6557..cccef17 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 linux/videomode.h
  #ifdef CONFIG_PPC_OF
  #include asm/prom.h
  #include asm/pci-bridge.h
 @@ -1373,6 +1374,43 @@ int fb_get_mode(int flags, u32 val, struct 
 fb_var_screeninfo *var, struct fb_inf
   kfree(timings);
   return err;
  }
 +
 +#if IS_ENABLED(CONFIG_VIDEOMODE)
 +int fb_videomode_from_videomode(struct videomode *vm, struct fb_videomode 
 *fbmode)
 +{
 + fbmode-xres = vm-hactive;
 + fbmode-left_margin = vm-hback_porch;
 + fbmode-right_margin = vm-hfront_porch;
 + fbmode-hsync_len = vm-hsync_len;
 +
 + fbmode-yres = vm-vactive;
 + fbmode-upper_margin = vm-vback_porch;
 + fbmode-lower_margin = vm-vfront_porch;
 + fbmode-vsync_len = vm-vsync_len;
 +
 + fbmode-pixclock = KHZ2PICOS(vm-pixelclock / 1000);
 +
 + fbmode-sync = 0;
 + fbmode-vmode = 0;
 + if (vm-hah)
 + fbmode-sync |= FB_SYNC_HOR_HIGH_ACT;
 + if (vm-vah)
 + fbmode-sync |= FB_SYNC_VERT_HIGH_ACT;
 + if (vm-interlaced)
 + fbmode-vmode |= FB_VMODE_INTERLACED;
 + if (vm-doublescan)
 + fbmode-vmode |= FB_VMODE_DOUBLE;
 + if (vm-de)
 + fbmode-sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
 + fbmode-refresh = (vm-pixelclock*1000) / (vm-hactive * vm-vactive);

CodingStyle that you should have spaces around '*'. Also I'm not sure if
that formula is correct. Shouldn't the blanking intervals be counted as
well? So:

unsigned int htotal = vm-hactive + vm-hfront_porch +
  vm-hback_porch + vm-hsync_len;
unsigned int vtotal = vm-vactive + vm-vfront_porch +
  vm-vback_porch + vm-vsync_len;

fbmode-refresh = (vm-pixelclock * 1000) / (htotal * vtotal);

?

 + fbmode-flag = 0;
 +
 + return 0;
 +}
 +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
 +#endif
 +
 +

Gratuitous blank line.

  #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 c7a9571..6a3a675 100644
 --- a/include/linux/fb.h
 +++ b/include/linux/fb.h
 @@ -14,6 +14,7 @@
  #include linux/backlight.h
  #include linux/slab.h
  #include asm/io.h
 +#include linux/videomode.h
  
  struct vm_area_struct;
  struct fb_info;
 @@ -714,6 +715,10 @@ 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 IS_ENABLED(CONFIG_VIDEOMODE)
 +extern int fb_videomode_from_videomode(struct videomode *vm,
 +struct fb_videomode *fbmode);
 +#endif
  /* drivers/video/modedb.c */

These in turn could use an extra blank line.

Thierry


pgpyzxQ3iqS2A.pgp
Description: PGP signature

Re: [PATCH v9 4/6] fbmon: add of_videomode helpers

2012-11-14 Thread Thierry Reding

On Wed, Nov 14, 2012 at 12:43:21PM +0100, Steffen Trumtrar wrote:
[...]
 diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
[...]
 +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
 +static void dump_fb_videomode(struct fb_videomode *m)

static inline?

Thierry


pgpjEMXjaOx0a.pgp
Description: PGP signature

Re: [PATCH v9 5/6] drm_modes: add videomode helpers

2012-11-14 Thread Thierry Reding

On Wed, Nov 14, 2012 at 12:43:22PM +0100, Steffen Trumtrar wrote:
[...]
 diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
[...]
 @@ -504,6 +505,41 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int 
 vdisplay, int vrefresh,
  }
  EXPORT_SYMBOL(drm_gtf_mode);
  
 +#if IS_ENABLED(CONFIG_VIDEOMODE)
 +int display_mode_from_videomode(struct videomode *vm, struct 
 drm_display_mode *dmode)

Given that this is still a DRM core function, maybe it should get a drm_
prefix? Also the line is too long, so you may want to wrap the argument
list.

Thierry


pgpDFVj0CkymV.pgp
Description: PGP signature

Re: [PATCH v9 6/6] drm_modes: add of_videomode helpers

2012-11-14 Thread Thierry Reding

On Wed, Nov 14, 2012 at 12:43:23PM +0100, Steffen Trumtrar wrote:
[...]
 +EXPORT_SYMBOL_GPL(of_get_drm_display_mode);
 +#endif
  /**

Nit: there should be a blank line between the last two.

Thierry


pgpTTm3RVeMhV.pgp
Description: PGP signature

[PATCH] [media] coda: Fix build due to iram.h rename

2012-11-14 Thread Fabio Estevam

commit c045e3f13 (ARM: imx: include iram.h rather than mach/iram.h) changed the
location of iram.h, which causes the following build error when building the 
coda
driver:

drivers/media/platform/coda.c:27:23: error: mach/iram.h: No such file or 
directory
drivers/media/platform/coda.c: In function 'coda_probe':
drivers/media/platform/coda.c:2000: error: implicit declaration of function 
'iram_alloc'
drivers/media/platform/coda.c:2001: warning: assignment makes pointer from 
integer without a cast
drivers/media/platform/coda.c: In function 'coda_remove':
drivers/media/platform/coda.c:2024: error: implicit declaration of function 
'iram_free

Since the content of iram.h is not imx specific, move it to include/linux/iram.h
instead.

Signed-off-by: Fabio Estevam fabio.este...@freescale.com
---
 arch/arm/mach-imx/iram_alloc.c  |3 +--
 drivers/media/platform/coda.c   |2 +-
 {arch/arm/mach-imx = include/linux}/iram.h |0
 3 files changed, 2 insertions(+), 3 deletions(-)
 rename {arch/arm/mach-imx = include/linux}/iram.h (100%)

diff --git a/arch/arm/mach-imx/iram_alloc.c b/arch/arm/mach-imx/iram_alloc.c
index 6c80424..11e067f 100644
--- a/arch/arm/mach-imx/iram_alloc.c
+++ b/arch/arm/mach-imx/iram_alloc.c
@@ -22,8 +22,7 @@
 #include linux/module.h
 #include linux/spinlock.h
 #include linux/genalloc.h
-
-#include iram.h
+#include linux/iram.h
 
 static unsigned long iram_phys_base;
 static void __iomem *iram_virt_base;
diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index cd04ae2..5c66162 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -23,8 +23,8 @@
 #include linux/slab.h
 #include linux/videodev2.h
 #include linux/of.h
+#include linux/iram.h
 
-#include mach/iram.h
 #include media/v4l2-ctrls.h
 #include media/v4l2-device.h
 #include media/v4l2-ioctl.h
diff --git a/arch/arm/mach-imx/iram.h b/include/linux/iram.h
similarity index 100%
rename from arch/arm/mach-imx/iram.h
rename to include/linux/iram.h
-- 
1.7.9.5


--
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: [PATCH v2] media: V4L2: add temporary clock helpers

2012-11-14 Thread Laurent Pinchart

Hi Sakari,

On Tuesday 13 November 2012 01:37:51 Sakari Ailus wrote:
 On Mon, Nov 12, 2012 at 12:06:50PM +0100, Laurent Pinchart wrote:
  On Monday 12 November 2012 00:33:17 Sakari Ailus wrote:
   On Wed, Oct 31, 2012 at 02:02:54PM +0100, Guennadi Liakhovetski wrote:
On Wed, 31 Oct 2012, Laurent Pinchart wrote:
   ...
   
  +#include linux/atomic.h
  +#include linux/errno.h
  +#include linux/list.h
  +#include linux/module.h
  +#include linux/mutex.h
  +#include linux/string.h
  +
  +#include media/v4l2-clk.h
  +#include media/v4l2-subdev.h
  +
  +static DEFINE_MUTEX(clk_lock);
  +static LIST_HEAD(v4l2_clk);
 
 As Sylwester mentioned, what about s/v4l2_clk/v4l2_clks/ ?

Don't you think naming of a static variable isn't important enough?
;-) I think code authors should have enough freedom to at least pick
up single vs. plural form:-) clks is too many consonants for my
taste, if it were anything important I'd rather agree to clk_head or
clk_list or something similar.
   
   clk_list makes sense IMO since the clk_ prefis is the same.
   
   ...
   
  +void v4l2_clk_put(struct v4l2_clk *clk)
  +{
  +   if (!IS_ERR(clk))
  +   module_put(clk-ops-owner);
  +}
  +EXPORT_SYMBOL(v4l2_clk_put);
  +
  +int v4l2_clk_enable(struct v4l2_clk *clk)
  +{
  +   if (atomic_inc_return(clk-enable) == 1  clk-ops-enable) {
  +   int ret = clk-ops-enable(clk);
  +   if (ret  0)
  +   atomic_dec(clk-enable);
  +   return ret;
  +   }
 
 I think you need a spinlock here instead of atomic operations. You
 could get preempted after atomic_inc_return() and before
 clk-ops-enable() by another process that would call
 v4l2_clk_enable(). The function would return with enabling the
 clock.

Sorry, what's the problem then? Our instance will succeed and call
-enable() and the preempting instance will see the enable count  1
and just return.
   
   The clock is guaranteed to be enabled only after the call has returned.
   The second caller of v4lw_clk_enable() thus may proceed without the
   clock being enabled.
   
   In principle enable() might also want to sleep, so how about using a
   mutex for the purpose instead of a spinlock?
  
  If enable() needs to sleep we should split the enable call into prepare
  and enable, like the common clock framework did.
 
 I'm pretty sure we won't need to toggle this from interrupt context which is
 what the clock framework does, AFAIU. Accessing i2c subdevs mandates
 sleeping already.
 
 We might not need to have a mutex either if no driver needs to sleep for
 this, still I guess this is more likely. I'm ok with both; just thought to
 mention this.

Right, I'm fine with a mutex for now, we'll split enable into enable and 
prepare later if needed.

-- 
Regards,

Laurent Pinchart

--
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

[PATCH] [media] s5p-mfc: Handle multi-frame input buffer

2012-11-14 Thread Arun Kumar K

When one input buffer has multiple frames, it should be fed
again to the hardware with the remaining bytes. Removed the
check for P frame in this scenario as this condition can come with
all frame types.

Signed-off-by: Arun Kumar K arun...@samsung.com
Signed-off-by: ARUN MANKUZHI aru...@samsung.com
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c |7 ++-
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 0ca8dbb..d3cd738 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -382,11 +382,8 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx,
ctx-consumed_stream += s5p_mfc_hw_call(dev-mfc_ops,
get_consumed_stream, dev);
if (ctx-codec_mode != S5P_MFC_CODEC_H264_DEC 
-   s5p_mfc_hw_call(dev-mfc_ops,
-   get_dec_frame_type, dev) ==
-   S5P_FIMV_DECODE_FRAME_P_FRAME
-ctx-consumed_stream + STUFF_BYTE 
-   src_buf-b-v4l2_planes[0].bytesused) {
+   ctx-consumed_stream + STUFF_BYTE 
+   src_buf-b-v4l2_planes[0].bytesused) {
/* Run MFC again on the same buffer */
mfc_debug(2, Running again the same buffer\n);
ctx-after_packed_pb = 1;
-- 
1.7.0.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: [RFC/PATCH] v4l: Add V4L2_CID_FLASH_HW_STROBE_MODE control

2012-11-14 Thread Laurent Pinchart

Hi Sakari,

On Tuesday 13 November 2012 23:53:46 Sakari Ailus wrote:
 On Wed, Oct 24, 2012 at 11:04:11AM +0200, Sylwester Nawrocki wrote:
  On 10/23/2012 10:07 PM, Sakari Ailus wrote:
   On Tue, Oct 16, 2012 at 11:45:59PM +0200, Sylwester Nawrocki wrote:
   On 10/14/2012 08:30 PM, Sakari Ailus wrote:
   Currently the flash control reference states that The V4L2 flash
   controls are intended to provide generic access to flash controller
   devices. Flash controller devices are typically used in digital
   cameras.
   
   Whether or not higher level controls should be part of the same class
   is a valid question. The controls intended to expose certain frames
   with flash are quite different from those used to control the low-
   level flash chip: the user is fully responsible for timing and the
   flash sequence.
   
   For higher level controls that could be implemented using the
   low-level controls for the end user, the user would likely prefer to
   say things like please give me a frame exposed with flash. Since the
   timing is no longer implemented by the user, the user would need to
   know which frames have been exposed and how, at least in a general
   case. Getting around this could involve configuring the sensor before
   starting streaming. Perhaps this is an assumption we could accept now,
   before we have proper means for passing frame-related parameters to
   user space.
   
   Yes, right. This auto strobe control seems to be a higher level one,
   since we have a firmware program that is taking care of some things
   that normally would be done through the existing Flash class controls
   by a user space application/library.
   
   I'm not really sure if we need a new class. It's even hard to name it.
   I don't see such an auto strobe control as a high level one, from an
   end application POV. It's more like the existing controls are low
   level.
   
   After thinking about it awhile, an alternative I see to this is to put
   it to the camera class. It's got other high level controls as well,
   those related to e.g. AF. I have to admit I'm not certain which one
   would be a better choice in the long run. I'm leaning towards the
   camera class, though.
 
  At first I thought it might be fine to put this control in the camera
  class. But didn't we agree we classify controls by functionality, not by
  where they are used ?
  Since we already have a Flash controller functionality class it seems to
  me more correct, or less wrong, to put V4L2_CID_FLASH_STROBE_AUTO there.
  
  In an example configuration, where there is a Flash controller subdev and
  the Flash controller is strobed in hardware by a camera sensor module, we
  would have some Flash functionality control at the camera sensor subdev
  and the Flash subdev. Let's focus on the camera subdev for a moment.
  It would have V4L2_CID_FLASH_LED_MODE - to switch between Flash and Torch
  mode, and V4L2_CID_FLASH_STROBE_AUTO - to determine the flash strobing
  behaviour. It would look fishy to have one of these controls in the Camera
  class and the other in the Flash class. I would find it hard to explain
  to someone new learning about v4l2.
 
 Good points. The flash mode control is still used even if the flash timing
 would be handles by the sensor silently.
 
 I'm fine with putting it to the flash class.

I agree with that as well. We should update the flash control class 
documentation to explain that flash controls apply to both flash controller 
devices and to devices that control the flash controller such as sensors (with 
a better wording than that of course :-)).

-- 
Regards,

Laurent Pinchart

--
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: [PATCH v9 2/6] video: add of helper for videomode

2012-11-14 Thread Steffen Trumtrar

On Wed, Nov 14, 2012 at 01:00:45PM +0100, Thierry Reding wrote:
 On Wed, Nov 14, 2012 at 12:43:19PM +0100, Steffen Trumtrar wrote:
 [...]
  +display-timings bindings
  +
  +
  +display timings node
 
 I didn't express myself very clearly here =). The way I think this
 should be written is display-timings node.
 
  +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: displayclock in Hz
 
 I still think displayclock should be two words: display clock.
 
  +/**
  + * 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)
  +{
 [...]
  +   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;
 
 I believe you're still potentially leaking memory here. In case you have
 5 entries for instance, and the last one fails to parse, then this will
 cause the memory allocated for the 4 correct entries to be lost.
 
 Can't you just call display_timings_release() in this case and then jump
 to dispfail? That would still leak the native_mode device node. Maybe it
 would be better to keep timingfail but modify it to free the display
 timings with display_timings_release() instead? See below.
 
  +   }
  +
  +   if (native_mode == entry)
  +   disp-native_mode = disp-num_timings;
  +
  +   disp-timings[disp-num_timings] = dt;
  +   disp-num_timings++;
  +   }
  +   of_node_put(timings_np);
  +   of_node_put(native_mode);
  +
  +   if (disp-num_timings  0)
  +   pr_info(%s: got %d timings. Using timing #%d as default\n, 
  __func__,
  +   disp-num_timings , disp-native_mode + 1);
  +   else {
  +   pr_err(%s: no valid timings specified\n, __func__);
  +   display_timings_release(disp);
  +   return NULL;
  +   }
  +   return disp;
  +
  +timingfail:
  +   if (native_mode)
  +   of_node_put(native_mode);
  +   kfree(disp-timings);
 
 Call display_timings_release(disp) instead of kfree(disp-timings)?
 

Yes. That would be the appropriate way to fail here. Done.

  diff --git a/include/linux/of_videomode.h b/include/linux/of_videomode.h
  new file mode 100644
  index 000..4138758
  --- /dev/null
  +++ b/include/linux/of_videomode.h
  @@ -0,0 +1,16 @@
  +/*
  + * Copyright 2012 Steffen Trumtrar s.trumt...@pengutronix.de
  + *
  + * videomode of-helpers
  + *
  + * This file is released under the GPLv2
  + */
  +
  +#ifndef __LINUX_OF_VIDEOMODE_H
  +#define __LINUX_OF_VIDEOMODE_H
  +
  +#include linux/videomode.h
  +#include linux/of.h
  +
  +int of_get_videomode(struct device_node *np, struct videomode *vm, int 
  index);
  +#endif /* __LINUX_OF_VIDEOMODE_H */
 
 Nit: should have a blank line before #endif.
 
 Thierry



 ___
 devicetree-discuss mailing list
 devicetree-disc...@lists.ozlabs.org
 https://lists.ozlabs.org/listinfo/devicetree-discuss


-- 
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: [PATCH v9 3/6] fbmon: add videomode helpers

2012-11-14 Thread Steffen Trumtrar

On Wed, Nov 14, 2012 at 01:12:07PM +0100, Thierry Reding wrote:
 On Wed, Nov 14, 2012 at 12:43:20PM +0100, Steffen Trumtrar wrote:
  Add a function to convert from the generic videomode to a fb_videomode.
  
  Signed-off-by: Steffen Trumtrar s.trumt...@pengutronix.de
  ---
   drivers/video/fbmon.c |   38 ++
   include/linux/fb.h|5 +
   2 files changed, 43 insertions(+)
  
  diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
  index cef6557..cccef17 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 linux/videomode.h
   #ifdef CONFIG_PPC_OF
   #include asm/prom.h
   #include asm/pci-bridge.h
  @@ -1373,6 +1374,43 @@ int fb_get_mode(int flags, u32 val, struct 
  fb_var_screeninfo *var, struct fb_inf
  kfree(timings);
  return err;
   }
  +
  +#if IS_ENABLED(CONFIG_VIDEOMODE)
  +int fb_videomode_from_videomode(struct videomode *vm, struct fb_videomode 
  *fbmode)
  +{
  +   fbmode-xres = vm-hactive;
  +   fbmode-left_margin = vm-hback_porch;
  +   fbmode-right_margin = vm-hfront_porch;
  +   fbmode-hsync_len = vm-hsync_len;
  +
  +   fbmode-yres = vm-vactive;
  +   fbmode-upper_margin = vm-vback_porch;
  +   fbmode-lower_margin = vm-vfront_porch;
  +   fbmode-vsync_len = vm-vsync_len;
  +
  +   fbmode-pixclock = KHZ2PICOS(vm-pixelclock / 1000);
  +
  +   fbmode-sync = 0;
  +   fbmode-vmode = 0;
  +   if (vm-hah)
  +   fbmode-sync |= FB_SYNC_HOR_HIGH_ACT;
  +   if (vm-vah)
  +   fbmode-sync |= FB_SYNC_VERT_HIGH_ACT;
  +   if (vm-interlaced)
  +   fbmode-vmode |= FB_VMODE_INTERLACED;
  +   if (vm-doublescan)
  +   fbmode-vmode |= FB_VMODE_DOUBLE;
  +   if (vm-de)
  +   fbmode-sync |= FB_SYNC_DATA_ENABLE_HIGH_ACT;
  +   fbmode-refresh = (vm-pixelclock*1000) / (vm-hactive * vm-vactive);
 
 CodingStyle that you should have spaces around '*'. Also I'm not sure if
 that formula is correct. Shouldn't the blanking intervals be counted as
 well? So:
 
   unsigned int htotal = vm-hactive + vm-hfront_porch +
 vm-hback_porch + vm-hsync_len;
   unsigned int vtotal = vm-vactive + vm-vfront_porch +
 vm-vback_porch + vm-vsync_len;
 
   fbmode-refresh = (vm-pixelclock * 1000) / (htotal * vtotal);
 
 ?
 

Sounds correct. Done.

  +   fbmode-flag = 0;
  +
  +   return 0;
  +}
  +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
  +#endif
  +
  +
 
 Gratuitous blank line.
 
   #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 c7a9571..6a3a675 100644
  --- a/include/linux/fb.h
  +++ b/include/linux/fb.h
  @@ -14,6 +14,7 @@
   #include linux/backlight.h
   #include linux/slab.h
   #include asm/io.h
  +#include linux/videomode.h
   
   struct vm_area_struct;
   struct fb_info;
  @@ -714,6 +715,10 @@ 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 IS_ENABLED(CONFIG_VIDEOMODE)
  +extern int fb_videomode_from_videomode(struct videomode *vm,
  +  struct fb_videomode *fbmode);
  +#endif
   /* drivers/video/modedb.c */
 
 These in turn could use an extra blank line.
 
 Thierry



 ___
 devicetree-discuss mailing list
 devicetree-disc...@lists.ozlabs.org
 https://lists.ozlabs.org/listinfo/devicetree-discuss


-- 
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: [PATCH v9 5/6] drm_modes: add videomode helpers

2012-11-14 Thread Steffen Trumtrar

On Wed, Nov 14, 2012 at 01:49:44PM +0100, Thierry Reding wrote:
 On Wed, Nov 14, 2012 at 12:43:22PM +0100, Steffen Trumtrar wrote:
 [...]
  diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
 [...]
  @@ -504,6 +505,41 @@ drm_gtf_mode(struct drm_device *dev, int hdisplay, int 
  vdisplay, int vrefresh,
   }
   EXPORT_SYMBOL(drm_gtf_mode);
   
  +#if IS_ENABLED(CONFIG_VIDEOMODE)
  +int display_mode_from_videomode(struct videomode *vm, struct 
  drm_display_mode *dmode)
 
 Given that this is still a DRM core function, maybe it should get a drm_
 prefix? Also the line is too long, so you may want to wrap the argument
 list.
 
 Thierry

Yes, seems to fit better to the rest of the file.

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: The em28xx driver error

2012-11-14 Thread Frank Schäfer

Am 11.11.2012 05:46, schrieb Michael Yang:
 Hi I am using a v4l2 usb video capturer (em28xx based) on the TI-DM3730 board
 I used the  default driver ,the video can't be captured. I solve this issue 
 by 
 change the em28xx driver :

 linux-stable/drivers/media/video/em28xx/em28xx-core.c

 /* FIXME: this only function read values from dev */
 int em28xx_resolution_set(struct em28xx *dev)
 {
 int width, height;
 width = norm_maxw(dev);
 height = norm_maxh(dev);

 /* Properly setup VBI */
 dev-vbi_width = 720;
 if (dev-norm  V4L2_STD_525_60)
 dev-vbi_height = 12;
 else
 dev-vbi_height = 18;

 if (!dev-progressive)
 height = norm_maxh(dev) ;//change to height = norm_maxh(dev)  1 ;
This looks indeed like a bug.
a = b means a = a  b, which in this case means shifting height 480
or 576 bits to the right...
height  1 means height /= 2 which seems to be sane for interlaced devices.
OTOH, I wonder why it seems to be working on other platforms !?
Unfortunately I don't have an interlaced device here for testing. :(


 em28xx_set_outfmt(dev);



 Then I can capture the video.But  about 3 minutes later, the os throw out 
 errors:

 Read a frame, the size is:325 
 Read a frame, the size is:304 
 ehci-omap ehci-omap.0: request c15b1000 would overflow (3898+63 = 3936)  
 //the 
 video shut up
 ehci-omap ehci-omap.0: request c15b would overflow (3906+63 = 3936) 
 ehci-omap ehci-omap.0: request c1558800 would overflow (3915+63 = 3936) 
 ehci-omap ehci-omap.0: request c15b0800 would overflow (3924+63 = 3936) 
 Read a frame, the size is:253 
 ehci-omap ehci-omap.0: request c143f800 would overflow (3909+63 = 3936) 
Couldn't this be an ehci-omap issue ?

 usb 1-2.2: kworker/0:2 timed out on ep0in len=0/1 
 usb 1-2.2: kworker/0:2 timed out on ep0in len=8/1 
 
 usb 1-2.2: kworker/0:2 timed out on ep0in len=0/1 
 usb 1-2.2: kworker/0:2 timed out on ep0in len=8/1 
 ^Cusb 1-2.2: test_h264 timed out on ep0in len=0/1 
 usb 1-2.2: kworker/0:2 timed out on ep0in len=0/1 
 ^Cusb 1-2.2: kworker/0:2 timed out on ep0in len=0/1 
 ^Cusb 1-2.2: kworker/0:2 timed out on ep0in len=0/1 

 usb 1-2.2: kworker/0:2 timed out on ep0in len=0/1 
 usb 1-2.2: kworker/0:2 timed out on ep0in len=0/1 

 usb 1-2.2: kworker/0:2 timed out on ep0in len=0/1 
 ^C 
 usb 1-2.2: kworker/0:2 timed out on ep0in len=0/1 
 ^Cusb 1-2.2: kworker/0:2 timed out on ep0in len=0/1 
 usb 1-2.2: test_h264 timed out on ep0out len=8/0 
 em28xx #0: cannot change alternate number to 0 (error=-110) 
This means usb_set_interface() failed with -ETIMEDOUT. No idea what that
means.
I also wonder why the driver tries to switch to alternate setting 0...
Could you please post the output of lsusb -v for this device ?

Regards,
Frank


--
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: The em28xx driver error

2012-11-14 Thread Devin Heitmueller

On Wed, Nov 14, 2012 at 11:58 AM, Frank Schäfer
fschaefer@googlemail.com wrote:
 This looks indeed like a bug.
 a = b means a = a  b, which in this case means shifting height 480
 or 576 bits to the right...
 height  1 means height /= 2 which seems to be sane for interlaced devices.
 OTOH, I wonder why it seems to be working on other platforms !?
 Unfortunately I don't have an interlaced device here for testing. :(

It's definitely a bug.  I think Mauro put a patch in for 3.7 or 3.8.
The reason it works under x86 is because shifting an arbitrary number
of bits  32 causes indeterminate behavior, and out of dumb luck it
has no effect on x86.

But yeah, I changed the code to shift by one bit and it's been working
fine on ARM for months in my environment (DM3730).

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
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: [PATCH 2/3] tcm825x: Remove driver

2012-11-14 Thread David Cohen

On Mon, Nov 12, 2012 at 2:47 PM, Sakari Ailus sakari.ai...@iki.fi wrote:

 Remove tcm825x driver. It uses the obsolete V4L2 int device framework, and
 can only work with omap24xxcam driver.

 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi

I wonder if you will ever fix and send it again :)

Acked-by: David Cohen daco...@gmail.com

 ---
  drivers/media/i2c/Kconfig   |8 -
  drivers/media/i2c/Makefile  |1 -
  drivers/media/i2c/tcm825x.c |  937 
 ---
  drivers/media/i2c/tcm825x.h |  200 -
  4 files changed, 0 insertions(+), 1146 deletions(-)
  delete mode 100644 drivers/media/i2c/tcm825x.c
  delete mode 100644 drivers/media/i2c/tcm825x.h

 diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
 index 24d78e2..aa8f18c 100644
 --- a/drivers/media/i2c/Kconfig
 +++ b/drivers/media/i2c/Kconfig
 @@ -475,14 +475,6 @@ config VIDEO_MT9V032
   This is a Video4Linux2 sensor-level driver for the Micron
   MT9V032 752x480 CMOS sensor.

 -config VIDEO_TCM825X
 -   tristate TCM825x camera sensor support
 -   depends on I2C  VIDEO_V4L2
 -   depends on MEDIA_CAMERA_SUPPORT
 -   ---help---
 - This is a driver for the Toshiba TCM825x VGA camera sensor.
 - It is used for example in Nokia N800.
 -
  config VIDEO_SR030PC30
 tristate Siliconfile SR030PC30 sensor support
 depends on I2C  VIDEO_V4L2
 diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
 index b1d62df..eb1757f 100644
 --- a/drivers/media/i2c/Makefile
 +++ b/drivers/media/i2c/Makefile
 @@ -47,7 +47,6 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
  obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
  obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
 -obj-$(CONFIG_VIDEO_TCM825X) += tcm825x.o
  obj-$(CONFIG_VIDEO_TVEEPROM) += tveeprom.o
  obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o
  obj-$(CONFIG_VIDEO_MT9P031) += mt9p031.o
 diff --git a/drivers/media/i2c/tcm825x.c b/drivers/media/i2c/tcm825x.c
 deleted file mode 100644
 index 9252529..000
 --- a/drivers/media/i2c/tcm825x.c
 +++ /dev/null
 @@ -1,937 +0,0 @@
 -/*
 - * drivers/media/i2c/tcm825x.c
 - *
 - * TCM825X camera sensor driver.
 - *
 - * Copyright (C) 2007 Nokia Corporation.
 - *
 - * Contact: Sakari Ailus sakari.ai...@nokia.com
 - *
 - * Based on code from David Cohen david.co...@indt.org.br
 - *
 - * This driver was based on ov9640 sensor driver from MontaVista
 - *
 - * This program is free software; you can redistribute it and/or
 - * modify it under the terms of the GNU General Public License
 - * version 2 as published by the Free Software Foundation.
 - *
 - * This program is distributed in the hope that it will be useful, but
 - * WITHOUT ANY WARRANTY; without even the implied warranty of
 - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 - * General Public License for more details.
 - *
 - * You should have received a copy of the GNU General Public License
 - * along with this program; if not, write to the Free Software
 - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
 - * 02110-1301 USA
 - */
 -
 -#include linux/i2c.h
 -#include linux/module.h
 -#include media/v4l2-int-device.h
 -
 -#include tcm825x.h
 -
 -/*
 - * The sensor has two fps modes: the lower one just gives half the fps
 - * at the same xclk than the high one.
 - */
 -#define MAX_FPS 30
 -#define MIN_FPS 8
 -#define MAX_HALF_FPS (MAX_FPS / 2)
 -#define HIGH_FPS_MODE_LOWER_LIMIT 14
 -#define DEFAULT_FPS MAX_HALF_FPS
 -
 -struct tcm825x_sensor {
 -   const struct tcm825x_platform_data *platform_data;
 -   struct v4l2_int_device *v4l2_int_device;
 -   struct i2c_client *i2c_client;
 -   struct v4l2_pix_format pix;
 -   struct v4l2_fract timeperframe;
 -};
 -
 -/* list of image formats supported by TCM825X sensor */
 -static const struct v4l2_fmtdesc tcm825x_formats[] = {
 -   {
 -   .description = YUYV (YUV 4:2:2), packed,
 -   .pixelformat = V4L2_PIX_FMT_UYVY,
 -   }, {
 -   /* Note:  V4L2 defines RGB565 as:
 -*
 -*  Byte 0Byte 1
 -*  g2 g1 g0 r4 r3 r2 r1 r0   b4 b3 b2 b1 b0 g5 g4 g3
 -*
 -* We interpret RGB565 as:
 -*
 -*  Byte 0Byte 1
 -*  g2 g1 g0 b4 b3 b2 b1 b0   r4 r3 r2 r1 r0 g5 g4 g3
 -*/
 -   .description = RGB565, le,
 -   .pixelformat = V4L2_PIX_FMT_RGB565,
 -   },
 -};
 -
 -#define TCM825X_NUM_CAPTURE_FORMATSARRAY_SIZE(tcm825x_formats)
 -
 -/*
 - * TCM825X register configuration for all combinations of pixel format and
 - * image size
 - */
 -static const struct tcm825x_reg subqcif=   { 0x20, 
 TCM825X_PICSIZ };
 -static const struct tcm825x_reg qcif   =   { 0x18, TCM825X_PICSIZ };
 -static const struct tcm825x_reg

Re: [PATCH v4 2/2] mx2_camera: Fix regression caused by clock conversion

2012-11-14 Thread Fabio Estevam

Hi Sascha,

On Wed, Oct 31, 2012 at 9:57 AM, Mauro Carvalho Chehab
mche...@infradead.org wrote:

 As it seems that those patches depend on some patches at the arm tree,
 the better is to merge them via -arm tree.

 So,

 Acked-by: Mauro Carvalho Chehab mche...@redhat.com

Could you please apply this series via your tree?

Thanks,

Fabio Estevam
--
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: [PATCH v4 2/2] mx2_camera: Fix regression caused by clock conversion

2012-11-14 Thread Sascha Hauer

On Wed, Nov 14, 2012 at 04:22:40PM -0200, Fabio Estevam wrote:
 Hi Sascha,
 
 On Wed, Oct 31, 2012 at 9:57 AM, Mauro Carvalho Chehab
 mche...@infradead.org wrote:
 
  As it seems that those patches depend on some patches at the arm tree,
  the better is to merge them via -arm tree.
 
  So,
 
  Acked-by: Mauro Carvalho Chehab mche...@redhat.com
 
 Could you please apply this series via your tree?

Sure, I already have this in my queue.

Sascha

-- 
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

Hauppauge WinTV HVR 900 (M/R 65018/B3C0) doesn't work anymore since linux 3.6.6

2012-11-14 Thread Philippe Valembois - Phil

Hello,
I have posted a bug report here :
https://bugzilla.kernel.org/show_bug.cgi?id=50361 and I have been told
to send it to the ML too.

The commit causing the bug has been pushed to kernel between linux-3.5
and linux-3.6.

Here is my bug summary :

The WinTV HVR900 DVB-T usb stick has stopped working in Linux 3.6.6.
The tuner fails at tuning and no DVB channel can be watched.

Reverting the commit 3de9e9624b36263618470c6e134f22eabf8f2551 fixes the
problem
and the tuner can tune again. It still seems there is some delay between the
moment when the USB stick is plugged and when it can tune : running
dvbscan too
fast makes the first channels tuning fail but after several seconds it tunes
perfectly.

Don't hesitate to ask me for additional debug.

Regards,
Philippe Valembois
--
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: Support for Terratec Cinergy 2400i DT in kernel 3.x

2012-11-14 Thread Patrice Chotard

Hi Frédéric,

You are right, in the ngene initial commit
(dae52d009fc950b5c209260d50fcc000f5becd3c), no fw_version was set, so by
default the ngene_15.fw is selected.

But in the patch available here
http://wiki.ubuntuusers.de/_attachment?target=/Terratec_Cinergy_2400i_DT/ngene_p11.tar.gz,
fw_version = 17 was set in ngene_info_terratec struct.

Before submitting the ngene patch set i have done tests with all
available firmware without noticing any difference.

I really don't known what are the difference between ngene_15.fw and
ngene_17.fw

Perhaps Ralph or Mauro has the answer ?



On 14/11/2012 09:48, Frédéric wrote:
 Le mardi 13 novembre 2012, Patrice Chotard a écrit :
 
 Two patches have been already submitted and are available since v3.7-rc1

 media] ngene: add support for Terratec Cynergy 2400i Dual DVB-T  :
 397e972350c42cbaf3228fe2eec23fecf6a69903

 and

 media] dvb: add support for Thomson DTT7520X :
 5fb67074c6657edc34867cba78255b6f5b505f12
 
 I had a look at your patches. I don't see the '.fw_version' param anymore in 
 the 'ngene_info' 
 structure... Is it normal?
 
--
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: Support for Terratec Cinergy 2400i DT in kernel 3.x

2012-11-14 Thread Patrice Chotard

Yes, there are differences, simply because algorithms which use these
tables are different ;-).


On 14/11/2012 10:36, Frédéric wrote:
 Le mercredi 14 novembre 2012, Frédéric a écrit :
 
 I had a look at your patches. I don't see the '.fw_version' param anymore
 in the 'ngene_info' structure... Is it normal?
 
 I also noticed some differences in the PLL presets:
 
 In your patch:
 
 .entries = {
 {  30500, 17, 0xb4, 0x12 },
 {  40500, 17, 0xbc, 0x12 },
 {  44500, 17, 0xbc, 0x12 },
 {  46500, 17, 0xf4, 0x18 },
 {  73500, 17, 0xfc, 0x18 },
 {  83500, 17, 0xbc, 0x18 },
 {  9, 17, 0xfc, 0x18 },
 },
 
 In original patch:
 
 if (freq17700 || freq85800)
 return -EINVAL;
 else if (freq30500) { c1=0xb4; c2=0x12; }
 else if (freq40500) { c1=0xbc; c2=0x12; }
 else if (freq44500) { c1=0xf4; c2=0x12; }
 else if (freq46500) { c1=0xfc; c2=0x12; }
 else if (freq73500) { c1=0xbc; c2=0x18; }
 else if (freq83500) { c1=0xf4; c2=0x18; }
 else { c1=0xfc; c2=0x18; }
 
--
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

cron job: media_tree daily build: WARNINGS

2012-11-14 Thread Hans Verkuil

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:Wed Nov 14 19:00:19 CET 2012
git hash:2c4e11b7c15af70580625657a154ea7ea70b8c76
gcc version:  i686-linux-gcc (GCC) 4.7.1
host hardware:x86_64
host os:  3.4.07-marune

linux-git-arm-eabi-davinci: WARNINGS
linux-git-arm-eabi-exynos: WARNINGS
linux-git-arm-eabi-omap: WARNINGS
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: WARNINGS
linux-git-x86_64: OK
linux-2.6.31.12-i686: WARNINGS
linux-2.6.32.6-i686: WARNINGS
linux-2.6.33-i686: WARNINGS
linux-2.6.34-i686: WARNINGS
linux-2.6.35.3-i686: WARNINGS
linux-2.6.36-i686: WARNINGS
linux-2.6.37-i686: WARNINGS
linux-2.6.38.2-i686: WARNINGS
linux-2.6.39.1-i686: WARNINGS
linux-3.0-i686: WARNINGS
linux-3.1-i686: WARNINGS
linux-3.2.1-i686: WARNINGS
linux-3.3-i686: WARNINGS
linux-3.4-i686: WARNINGS
linux-3.5-i686: WARNINGS
linux-3.6-i686: WARNINGS
linux-3.7-rc1-i686: WARNINGS
linux-2.6.31.12-x86_64: WARNINGS
linux-2.6.32.6-x86_64: WARNINGS
linux-2.6.33-x86_64: WARNINGS
linux-2.6.34-x86_64: WARNINGS
linux-2.6.35.3-x86_64: WARNINGS
linux-2.6.36-x86_64: WARNINGS
linux-2.6.37-x86_64: WARNINGS
linux-2.6.38.2-x86_64: WARNINGS
linux-2.6.39.1-x86_64: WARNINGS
linux-3.0-x86_64: WARNINGS
linux-3.1-x86_64: WARNINGS
linux-3.2.1-x86_64: WARNINGS
linux-3.3-x86_64: WARNINGS
linux-3.4-x86_64: WARNINGS
linux-3.5-x86_64: WARNINGS
linux-3.6-x86_64: WARNINGS
linux-3.7-rc1-x86_64: WARNINGS
apps: WARNINGS
spec-git: WARNINGS
sparse: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Wednesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2

The V4L-DVB specification from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
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: [PATCH 1/1] media: Entities with sink pads must have at least one enabled link

2012-11-14 Thread Sakari Ailus

Hi Sylwester,

Thanks for the comments.

On Wed, Nov 14, 2012 at 10:23:19AM +0100, Sylwester Nawrocki wrote:
 On 11/13/2012 03:24 PM, Sakari Ailus wrote:
  Hi all,
  
  Comments would be appreciated, either positive or negative. The omap3isp
  driver does the same check itself currently, but I think this is more
  generic than that.
  
  Thanks.
  
  On Fri, Oct 26, 2012 at 10:46:17PM +0300, Sakari Ailus wrote:
  If an entity has sink pads, at least one of them must be connected to
  another pad with an enabled link. If a driver with multiple sink pads has
  more strict requirements the check should be done in the driver itself.
 
  Just requiring one sink pad is connected with an enabled link is enough
  API-wise: entities with sink pads with only disabled links should not be
  allowed to stream in the first place, but also in a different operation 
  mode
  a device might require only one of its pads connected with an active link.
 
  If an entity has an ability to function as a source entity another logical
  entity connected to the aforementioned one should be used for the purpose.
 
 Why not leave it to individual drivers ? I'm not sure if it is a good idea
 not to allow an entity with sink pads to be used as a source only. It might
 be appropriate for most of the cases but likely not all. I'm inclined not to
 add this requirement in the API. Just my opinion though.

I'm just wondering what would be the use case for that.

What comes closest is generating a test pattern, but even that should be a
separate subdev: the test pattern can be enabled by enabling the link from
the test pattern generator subdev.

As it seems not everyone is outright happy about the idea of making this
mandatory, then how about making it optional?

I'd hate having a link validate function for each subdev e.g. in the OMAP 3
ISP driver that just checks that its sink pad is actually connected with an
enabled link. That'd be lots of mostly useless code. If this is done in the
framework, the drivers will be spared from copying this code in a number of
places. Which was why I originally wrote this patch. The alternative is to
re-parse the whole graph in the driver which I'd also like to avoid.

One opion I can think of is to call link_validate op of struct
media_entity_operations also for disabled links on entities that are
connected through active links (on V4L2 to a video node right before
streaming, for example).

That'd make it easy to perform the check in the drivers.

What do you think?

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
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: [PATCH 1/1] media: Entities with sink pads must have at least one enabled link

2012-11-14 Thread Sakari Ailus

On Wed, Nov 14, 2012 at 11:58:42AM +0100, Laurent Pinchart wrote:
 I think my preference would go for a helper function that drivers can use, 
 possibly first waiting until a second driver requires this kind of checks 
 before implementing it.

I'd like to see a driver that doesn't. Quite likelye either it has no
configurable links or it's broken. :-)

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
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: Regarding bulk transfers on stk1160

2012-11-14 Thread Ezequiel Garcia

Hi Greg,

On Tue, Nov 13, 2012 at 11:58 AM, Greg KH g...@kroah.com wrote:

 Or better yet, buy a board with a working USB port, like a BeagleBone or
 the like :)


Michael Hartup (the interested user) *has* a beaglebone.

I'm trying to help him getting it ready for stk1160.
However, Michael is getting choppy video capture.
(dmesg doesn't show anything relevant)

@Michael, could you upload those captures somewhere
and post the links for everyone to see?

Is this related to beaglebone's known usb dma issues?

https://github.com/RobertCNelson/linux-dev/issues/2
https://groups.google.com/forum/?fromgroups=#!topic/beagleboard/J94PUlo0wzs

Unfortunately, I don't own a beaglebone (and I can't afford one right now)
so I can't really see for myself what's going on.

Any help, greatly appreciated.

Ezequiel
--
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: Regarding bulk transfers on stk1160

2012-11-14 Thread Michael Hartup


On 14 Nov 2012, at 22:53, Ezequiel Garcia wrote:

 Hi Greg,
 
 On Tue, Nov 13, 2012 at 11:58 AM, Greg KH g...@kroah.com wrote:
 
 Or better yet, buy a board with a working USB port, like a BeagleBone or
 the like :)
 
 
 Michael Hartup (the interested user) *has* a beaglebone.
 
 I'm trying to help him getting it ready for stk1160.
 However, Michael is getting choppy video capture.
 (dmesg doesn't show anything relevant)
 
 @Michael, could you upload those captures somewhere
 and post the links for everyone to see?

 
 Is this related to beaglebone's known usb dma issues?
 
 https://github.com/RobertCNelson/linux-dev/issues/2
 https://groups.google.com/forum/?fromgroups=#!topic/beagleboard/J94PUlo0wzs
 
 Unfortunately, I don't own a beaglebone (and I can't afford one right now)
 so I can't really see for myself what's going on.
 
 Any help, greatly appreciated.
 
Ezequiel



Thanks for looking at this gentlemen. I have posted some examples here;

http://bufobufomagic.blogspot.co.uk/2012/11/image-corruption-on-beaglebone-with.html


Michael.--
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

[PATCH] ad5820: Voice coil motor controller driver

2012-11-14 Thread Laurent Pinchart

The AD5820 is a voice coil motor controller typically used to control
lens position in digital cameras.

Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
 drivers/media/i2c/Kconfig  |9 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ad5820.c |  496 
 include/media/ad5820.h |   30 +++
 4 files changed, 536 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/i2c/ad5820.c
 create mode 100644 include/media/ad5820.h

Hi Florian,

This is the ad5820 driver I've told you about. The code is compile-tested only
as I haven't had time to try it on an N900 (the only device I own that
includes an ad5820).

It should be quite easy to adapt the driver to support both the ad5820 and the
ad5821. Would you have time to give it a try ?

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 24d78e2..65597cf 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -534,6 +534,15 @@ config VIDEO_AS3645A
  This is a driver for the AS3645A and LM3555 flash controllers. It has
  build in control for flash, torch and indicator LEDs.
 
+comment Lens controllers
+
+config VIDEO_AD5820
+   tristate AD5820 lens voice coil support
+   depends on I2C  VIDEO_V4L2  MEDIA_CONTROLLER
+   ---help---
+ This is a driver for the AD5820 camera lens voice coil.
+ It is used for example in Nokia RX51.
+
 comment Video improvement chips
 
 config VIDEO_UPD64031A
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index b1d62df..975cfb8 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
 obj-$(CONFIG_VIDEO_NOON010PC30)+= noon010pc30.o
 obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
 obj-$(CONFIG_VIDEO_S5K4ECGX)   += s5k4ecgx.o
+obj-$(CONFIG_VIDEO_AD5820) += ad5820.o
 obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o
 obj-$(CONFIG_VIDEO_AS3645A)+= as3645a.o
 obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o
diff --git a/drivers/media/i2c/ad5820.c b/drivers/media/i2c/ad5820.c
new file mode 100644
index 000..995774f
--- /dev/null
+++ b/drivers/media/i2c/ad5820.c
@@ -0,0 +1,496 @@
+/*
+ * AD5820 DAC driver for camera voice coil focus.
+ *
+ * Copyright (C) 2008 Nokia Corporation
+ * Copyright (C) 2007 Texas Instruments
+ *
+ * Contact: Laurent Pinchart laurent.pinch...@ideasonboard.com
+ *  Sakari Ailus sakari.ai...@iki.fi
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include linux/bitops.h
+#include linux/delay.h
+#include linux/errno.h
+#include linux/gpio.h
+#include linux/i2c.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/regulator/consumer.h
+#include linux/sched.h
+#include linux/slab.h
+
+#include media/ad5820.h
+#include media/v4l2-ctrls.h
+#include media/v4l2-device.h
+#include media/v4l2-subdev.h
+
+/* Register definitions */
+#define AD5820_POWER_DOWN  (1  15)
+#define AD5820_DAC_SHIFT   4
+#define AD5820_RAMP_MODE_LINEAR(0  3)
+#define AD5820_RAMP_MODE_64_16 (1  3)
+
+#define CODE_TO_RAMP_US(s) ((s) == 0 ? 0 : (1  ((s) - 1)) * 50)
+#define RAMP_US_TO_CODE(c) fls(((c) + ((c)1)) / 50)
+
+struct ad5820_device {
+   struct v4l2_subdev subdev;
+
+   struct regulator *vana;
+   int xshutdown;
+
+   struct v4l2_ctrl_handler ctrls;
+   u32 focus_absolute;
+   u32 focus_ramp_time;
+   u32 focus_ramp_mode;
+
+   struct mutex power_lock;
+   int power_count;
+
+   int standby:1;
+};
+
+#define to_ad5820_device(sd)   container_of(sd, struct ad5820_device, subdev)
+
+/**
+ * @brief I2C write using i2c_transfer().
+ * @param coil - the driver data structure
+ * @param data - register value to be written
+ * @returns nonnegative on success, negative if failed
+ */
+static int ad5820_write(struct ad5820_device *coil, u16 data)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(coil-subdev);
+   struct i2c_msg msg;
+   int r;
+
+   if (!client-adapter)
+   return -ENODEV;
+
+   data = cpu_to_be16(data);
+   msg.addr  = client-addr;
+   msg.flags = 0;
+   msg.len   = 2;
+   msg.buf   = (u8 *)data;
+
+   r = i2c_transfer(client-adapter, msg, 1);
+   if (r  0) {
+   dev_err(client-dev, write failed, error %d\n, r);
+   return r;
+   }
+
+   return 0;
+}
+
+/**
+ * @brief I2C read using i2c_transfer().
+ * @param coil - the driver data structure
+ *

Re: AW: [omap3-isp-live] Autofocus buffer interpretation of H3A engine

2012-11-14 Thread Laurent Pinchart

Hi Florian,

On Friday 09 November 2012 15:57:58 Florian Neuhaus wrote:
 Laurent Pinchart wrote on 2012-11-04:
  The AD5821 is similar to the AD5820, for which I have a driver that I need
  to clean up and post. Would you be interested in that ?
 
 Yes, you can send me the driver.

I've just sent it to you and CC'ed the list.

 Just as a note:
 I (probably) found an error in the current ad5398 and ad5821 driver
 http://lxr.free-electrons.com/source/drivers/regulator/ad5398.c
 It seems that the enable and disable functions are switched
 (at least for the ad5821).

I think you're right.

 Also it's not possible to set the maximum current. I've done a patch but
 didn't have the time to submit it. Is this the right place for it?

As the ad5398 driver implements the regulator driver API you should send the 
patch to the appropriate mailing lists.

$ ./scripts/get_maintainer.pl -f drivers/regulator/ad5398.c 
Michael Hennerich michael.henner...@analog.com (supporter:AD5398 CURRENT 
RE...)
Liam Girdwood l...@ti.com (supporter:VOLTAGE AND CURRE...)
Mark Brown broo...@opensource.wolfsonmicro.com (supporter:VOLTAGE AND 
CURRE...)
device-drivers-de...@blackfin.uclinux.org (open list:AD5398 CURRENT RE...)
linux-ker...@vger.kernel.org (open list)

  Even though that buffer structure is pretty simple, I'm afraid I can't
  provide that information as it's covered by an NDA.
 
 I have written the TI-support and here's the answer:
 START QUOTE
  After checking it seems that we unfortunately do not make the DM37x H3A
 documention available. Implementing Autofocus is quite complexe and the
 documentation is likely not going to provide enough help. A lot of
 experience is required to handle the mecanics and control loop aspect for
 the Autofocus.
 
 We have partners that do have experience with the H3A module and that might
 be able to help. For example MMS that mentionned is the below document:
  http://www.ti.com/lit/ml/swpt052/swpt052.pdf
 Also Leopard Imaging has experience on the H3A:
 https://www.leopardimaging.com/Services.html
 END QUOTE
 
 It's sad that they can't provide any further information, because we are not
 that far to get this stuff working...

Try pushing a bit more, sometimes it helps. Ask what it would take to get 
access to the documentation, possibly as a partner.

  could try to figure it out ourselves. Looking at the FCam project, I've
  found
  
  http://vcs.maemo.org/svn/fcam/fcam-
  dev/tags/1.1.0/src/N900/V4L2Sensor.cpp
  
  Does that help figuring out what the buffer contains ?
 
 That helps a lot! Thank you. I found also a little info here:
 http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/m
 sg18438.html

If you can figure out the format and the configuration parameters from the 
above sources, I would greatly appreciate if you could write a bit of 
documentation and send a patch for Documentation/video4linux/omap3isp.txt. I 
unfortunately can't do it myself without TI's approval as I've had access to 
the documentation.

  I haven't started, and it's currently not on my to-do list I'm afraid.
 
 That's too bad, as your help is always appreciated.

Spare time is unfortunately limited :-)

 Have you already seen my patch for the rotation in your omap3-isp-live
 program?

It's still in my mailbox, I'll try to process it in the near future.

 I have seen also another little issue, if you are interested (snapshot_init
 should be after aewb as the output format changes during snapshot-init).

Sure, please send it.

 I am now off for 3 weeks (annual refresher course in the Swiss armed
 forces).

-- 
Regards,

Laurent Pinchart

--
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: [PATCH 1/1] media: Entities with sink pads must have at least one enabled link

2012-11-14 Thread Laurent Pinchart

Hi Sakari,

On Wednesday 14 November 2012 23:13:45 Sakari Ailus wrote:
 On Wed, Nov 14, 2012 at 10:23:19AM +0100, Sylwester Nawrocki wrote:
  On 11/13/2012 03:24 PM, Sakari Ailus wrote:
   Hi all,
   
   Comments would be appreciated, either positive or negative. The omap3isp
   driver does the same check itself currently, but I think this is more
   generic than that.
   
   Thanks.
   
   On Fri, Oct 26, 2012 at 10:46:17PM +0300, Sakari Ailus wrote:
   If an entity has sink pads, at least one of them must be connected to
   another pad with an enabled link. If a driver with multiple sink pads
   has more strict requirements the check should be done in the driver
   itself.
   
   Just requiring one sink pad is connected with an enabled link is enough
   API-wise: entities with sink pads with only disabled links should not
   be allowed to stream in the first place, but also in a different
   operation mode a device might require only one of its pads connected
   with an active link.
   
   If an entity has an ability to function as a source entity another
   logical entity connected to the aforementioned one should be used for
   the purpose.
  
  Why not leave it to individual drivers ? I'm not sure if it is a good idea
  not to allow an entity with sink pads to be used as a source only. It
  might be appropriate for most of the cases but likely not all. I'm
  inclined not to add this requirement in the API. Just my opinion though.
 
 I'm just wondering what would be the use case for that.
 
 What comes closest is generating a test pattern, but even that should be a
 separate subdev: the test pattern can be enabled by enabling the link from
 the test pattern generator subdev.

That would force creating a separate subdev just to support test pattern 
generation, I'm not sure if I want to push for that. There might also be other 
use cases not related to V4L (thus the cross-posting).

 As it seems not everyone is outright happy about the idea of making this
 mandatory, then how about making it optional?
 
 I'd hate having a link validate function for each subdev e.g. in the OMAP 3
 ISP driver that just checks that its sink pad is actually connected with an
 enabled link. That'd be lots of mostly useless code.

Agreed.

 If this is done in the framework, the drivers will be spared from copying
 this code in a number of places. Which was why I originally wrote this
 patch. The alternative is to re-parse the whole graph in the driver which
 I'd also like to avoid.

I'd also prefer to avoid that *if*possible*, as we already walk the graph 
during link validation.

 One opion I can think of is to call link_validate op of struct
 media_entity_operations also for disabled links on entities that are
 connected through active links (on V4L2 to a video node right before
 streaming, for example).
 
 That'd make it easy to perform the check in the drivers.

 What do you think?

I think that would be a bit too complex. Drivers (or the V4L core) would need 
to gather data from multiple links in some state object to find out if the 
complete pipeline is valid or not.

Another option would be to set a flag somewhere to indicate whether the check 
should be performed by the media core or left to drivers. As different types 
of drivers might need different types of checks, I think I would prefer for 
now to walk the graph one more time in the OMAP3 ISP driver, as currently 
done, and revisit this issue when we will have a couple of drivers 
implementing pipeline validity checks. I'm just a bit uncomfortable adding 
core code for a feature that has a single user at the moment without a clear 
view regarding how it would scale.

-- 
Regards,

Laurent Pinchart

--
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: [PATCH] [media] vpif_display: fix return value check in vpif_reqbufs()

2012-11-14 Thread Prabhakar Lad

Hi Wei,

On Mon, Nov 12, 2012 at 2:14 PM, Wei Yongjun weiyj...@gmail.com wrote:
 Hi Prabhakar,

 On 11/09/2012 08:33 PM, Prabhakar Lad wrote:
 Hi Wei,

 Thanks for the patch.

 On Wed, Oct 24, 2012 at 4:59 PM, Wei Yongjun weiyj...@gmail.com wrote:
 From: Wei Yongjun yongjun_...@trendmicro.com.cn

 In case of error, the function vb2_dma_contig_init_ctx() returns
 ERR_PTR() and never returns NULL. The NULL test in the return value
 check should be replaced with IS_ERR().

 dpatch engine is used to auto generate this patch.
 (https://github.com/weiyj/dpatch)

 Signed-off-by: Wei Yongjun yongjun_...@trendmicro.com.cn
 ---
  drivers/media/platform/davinci/vpif_display.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/drivers/media/platform/davinci/vpif_display.c 
 b/drivers/media/platform/davinci/vpif_display.c
 index b716fbd..5453bbb 100644
 --- a/drivers/media/platform/davinci/vpif_display.c
 +++ b/drivers/media/platform/davinci/vpif_display.c
 @@ -972,9 +972,9 @@ static int vpif_reqbufs(struct file *file, void *priv,
 }
 /* Initialize videobuf2 queue as per the buffer type */
 common-alloc_ctx = vb2_dma_contig_init_ctx(vpif_dev);
 -   if (!common-alloc_ctx) {
 +   if (IS_ERR(common-alloc_ctx)) {
 Right check would be IS_ERR_OR_NULL(). Can you merge this
 patch 'vpif_capture: fix return value check in vpif_reqbufs()' with
 this one  and post a v2 with above changes ?

 I will merge those two patch into one.
 And I never see vb2_dma_contig_init_ctx() can return NULL as a return
 value, we still would using IS_ERR_OR_NULL()?

IS_ERR() should be Ok.

Regards,
--Prabhakar Lad

 ---
 void *vb2_dma_contig_init_ctx(struct device *dev)
 {
struct vb2_dc_conf *conf;

conf = kzalloc(sizeof *conf, GFP_KERNEL);
if (!conf)
  return ERR_PTR(-ENOMEM);

conf-dev = dev;

return conf;
 }
 ---




--
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

51 matches

Mail list logo