cron job: media_tree daily build: WARNINGS
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: Thu Feb 13 04:00:26 CET 2014 git branch: test git hash: 37e59f876bc710d67a30b660826a5e83e07101ce gcc version:i686-linux-gcc (GCC) 4.8.2 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.12-6.slh.2-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: WARNINGS linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.14-i686: WARNINGS linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12-i686: OK linux-3.13-i686: OK linux-3.14-rc1-i686: OK linux-2.6.31.14-x86_64: WARNINGS linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12-x86_64: OK linux-3.13-x86_64: OK linux-3.14-rc1-x86_64: OK apps: OK spec-git: OK sparse version: 0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The Media Infrastructure API 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] s2255drv: upgrade to videobuf2
Updated: Only error is readbuffers now. I'll fix it and submit a v2 patch with 0 errors. FYI, the fix for the readbuffers v4l2-compliance fail will be setting v4l2_streamparm.capture.readbuffers to the minimum buffer value. Thanks, On 2014-02-12 10:10, Hans Verkuil wrote: On 02/12/14 17:01, Dean Anderson wrote: "./utils/v4l2-compliance/v4l2-compliance -s" Driver Info: Driver name : s2255 Card type : s2255 Bus info : usb-:00:1a.7-3.6 Driver version: 3.13.0 Capabilities : 0x8401 Video Capture Streaming Device Capabilities Device Caps : 0x0401 Video Capture Streaming Compliance test for device /dev/video0 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Control ioctls: test VIDIOC_QUERYCTRL/MENU: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK warn: v4l2-test-controls.cpp(753): The VIDIOC_G_JPEGCOMP ioctl is deprecated! warn: v4l2-test-controls.cpp(770): The VIDIOC_S_JPEGCOMP ioctl is deprecated! test VIDIOC_G/S_JPEGCOMP: OK Standard Controls: 7 Private Controls: 1 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK warn: v4l2-test-formats.cpp(599): TRY_FMT cannot handle an invalid pixelformat. warn: v4l2-test-formats.cpp(600): This may or may not be a problem. For more information see: warn: v4l2-test-formats.cpp(601): http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html test VIDIOC_TRY_FMT: OK warn: v4l2-test-formats.cpp(786): S_FMT cannot handle an invalid pixelformat. warn: v4l2-test-formats.cpp(787): This may or may not be a problem. For more information see: warn: v4l2-test-formats.cpp(788): http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: warn: v4l2-test-buffers.cpp(343): VIDIOC_CREATE_BUFS not supported test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK fail: v4l2-test-buffers.cpp(379): ret < 0 && errno != EINVAL You added read() support, but did not add V4L2_CAP_READWRITE to querycap. The following errors are a knock-on effect of that since the driver is still in read() mode so attempts to call REQBUFS will fail. I should see if I can improve that in v4l2-compliance. Regards, Hans test read/write: FAIL fail: v4l2-test-buffers.cpp(537): can_stream test MMAP: FAIL fail: v4l2-test-buffers.cpp(641): can_stream test USERPTR: FAIL Total: 39, Succeeded: 36, Failed: 3, Warnings: 9 -- 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] s2255drv: upgrade to videobuf2
"./v4l2-compliance -s"
Driver Info:
Driver name : s2255
Card type : s2255
Bus info : usb-:00:1a.7-3.6
Driver version: 3.13.0
Capabilities : 0x8501
Video Capture
Read/Write
Streaming
Device Capabilities
Device Caps : 0x0501
Video Capture
Read/Write
Streaming
Compliance test for device /dev/video0 (not using libv4l2):
Required ioctls:
test VIDIOC_QUERYCAP: OK
Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK
Input ioctls:
test VIDIOC_G/S_TUNER: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 Audio Inputs: 0 Tuners: 0
Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0
Control ioctls:
test VIDIOC_QUERYCTRL/MENU: OK
test VIDIOC_G/S_CTRL: OK
test VIDIOC_G/S/TRY_EXT_CTRLS: OK
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
warn: v4l2-test-controls.cpp(753): The VIDIOC_G_JPEGCOMP ioctl is
deprecated!
warn: v4l2-test-controls.cpp(770): The VIDIOC_S_JPEGCOMP ioctl is
deprecated!
test VIDIOC_G/S_JPEGCOMP: OK
Standard Controls: 7 Private Controls: 1
Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
warn: v4l2-test-formats.cpp(599): TRY_FMT cannot handle an invalid
pixelformat.
warn: v4l2-test-formats.cpp(600): This may or may not be a problem.
For more information see:
warn: v4l2-test-formats.cpp(601):
http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html
test VIDIOC_TRY_FMT: OK
warn: v4l2-test-formats.cpp(786): S_FMT cannot handle an invalid
pixelformat.
warn: v4l2-test-formats.cpp(787): This may or may not be a problem.
For more information see:
warn: v4l2-test-formats.cpp(788):
http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
Buffer ioctls:
warn: v4l2-test-buffers.cpp(343): VIDIOC_CREATE_BUFS not
supported
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test read/write: OK
test MMAP: OK
test USERPTR: OK
Total: 39, Succeeded: 39, Failed: 0, Warnings: 9
On 2014-02-12 14:25, Dean Anderson wrote:
Upgrade to videobuf2 libraries.
No errors reported with "v4l2-compliance -s".
Signed-off-by: Dean Anderson
---
drivers/media/usb/s2255/s2255drv.c | 512
+++-
1 file changed, 152 insertions(+), 360 deletions(-)
diff --git a/drivers/media/usb/s2255/s2255drv.c
b/drivers/media/usb/s2255/s2255drv.c
index e0663ce..ef66b1b 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -45,14 +45,14 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
#include
#include
-#define S2255_VERSION "1.24.1"
+#define S2255_VERSION "1.25.1"
#define FIRMWARE_FILE_NAME "f2255usb.bin"
/* default JPEG quality */
@@ -229,8 +229,6 @@ struct s2255_vc {
struct v4l2_captureparm cap_parm;
int cur_frame;
int last_frame;
-
- int b_acquire;
/* allocated image size */
unsigned long req_image_size;
/* received packet size */
@@ -249,8 +247,12 @@ struct s2255_vc {
int vidstatus_ready;
unsigned intwidth;
unsigned intheight;
+ enum v4l2_field field;
const struct s2255_fmt *fmt;
int idx; /* channel number on device, 0-3 */
+ struct vb2_queue vb_vidq;
+ struct mutex vb_lock; /* streaming lock */
+ spinlock_t qlock;
};
@@ -270,7 +272,6 @@ stru
[PATCH v2] s2255drv: upgrade to videobuf2
Upgrade to videobuf2 libraries.
No errors reported with "v4l2-compliance -s".
Signed-off-by: Dean Anderson
---
drivers/media/usb/s2255/s2255drv.c | 512 +++-
1 file changed, 152 insertions(+), 360 deletions(-)
diff --git a/drivers/media/usb/s2255/s2255drv.c
b/drivers/media/usb/s2255/s2255drv.c
index e0663ce..ef66b1b 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -45,14 +45,14 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
#include
#include
-#define S2255_VERSION "1.24.1"
+#define S2255_VERSION "1.25.1"
#define FIRMWARE_FILE_NAME "f2255usb.bin"
/* default JPEG quality */
@@ -229,8 +229,6 @@ struct s2255_vc {
struct v4l2_captureparm cap_parm;
int cur_frame;
int last_frame;
-
- int b_acquire;
/* allocated image size */
unsigned long req_image_size;
/* received packet size */
@@ -249,8 +247,12 @@ struct s2255_vc {
int vidstatus_ready;
unsigned intwidth;
unsigned intheight;
+ enum v4l2_field field;
const struct s2255_fmt *fmt;
int idx; /* channel number on device, 0-3 */
+ struct vb2_queue vb_vidq;
+ struct mutex vb_lock; /* streaming lock */
+ spinlock_t qlock;
};
@@ -270,7 +272,6 @@ struct s2255_dev {
u32 cc; /* current channel */
int frame_ready;
int chn_ready;
- spinlock_t slock;
/* dsp firmware version (f2255usb.bin) */
int dsp_fw_ver;
u16 pid; /* product id */
@@ -292,16 +293,10 @@ struct s2255_fmt {
/* buffer for one video frame */
struct s2255_buffer {
/* common v4l buffer stuff -- must be first */
- struct videobuf_buffer vb;
+ struct vb2_buffer vb;
+ struct list_head list;
};
-struct s2255_fh {
- /* this must be the first field in this struct */
- struct v4l2_fh fh;
- struct videobuf_queue vb_vidq;
- struct s2255_vc *vc;
- int resources;
-};
/* current cypress EEPROM firmware version */
#define S2255_CUR_USB_FWVER((3 << 8) | 12)
@@ -569,21 +564,20 @@ static int s2255_got_frame(struct s2255_vc *vc, int
jpgsize)
struct s2255_dev *dev = to_s2255_dev(vc->vdev.v4l2_dev);
unsigned long flags = 0;
int rc = 0;
- spin_lock_irqsave(&dev->slock, flags);
+ spin_lock_irqsave(&vc->qlock, flags);
if (list_empty(&vc->buf_list)) {
dprintk(dev, 1, "No active queue to serve\n");
rc = -1;
goto unlock;
}
buf = list_entry(vc->buf_list.next,
-struct s2255_buffer, vb.queue);
- list_del(&buf->vb.queue);
- v4l2_get_timestamp(&buf->vb.ts);
+struct s2255_buffer, list);
+ list_del(&buf->list);
+ v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
s2255_fillbuff(vc, buf, jpgsize);
- wake_up(&buf->vb.done);
- dprintk(dev, 2, "%s: [buf/i] [%p/%d]\n", __func__, buf, buf->vb.i);
+ dprintk(dev, 2, "%s: [buf] [%p]\n", __func__, buf);
unlock:
- spin_unlock_irqrestore(&dev->slock, flags);
+ spin_unlock_irqrestore(&vc->qlock, flags);
return rc;
}
@@ -615,7 +609,7 @@ static void s2255_fillbuff(struct s2255_vc *vc,
{
int pos = 0;
const char *tmpbuf;
- char *vbuf = videobuf_to_vmalloc(&buf->vb);
+ char *vbuf = vb2_plane_vaddr(&buf->vb, 0);
unsigned long last_frame;
struct s2255_dev *dev = vc->dev;
@@ -629,21 +623,21 @@ static void s2255_fillbuff(struct s2255_vc *vc,
case V4L2_PIX_FMT_YUYV:
case V4L2_PIX_FMT_UYVY:
planar422p_to_yuv_packed((const unsigned char *)tmpbuf,
-vbuf, buf->vb.width,
-buf->vb.height,
+vbuf, vc->width,
+vc->height,
vc->fmt->fourcc);
break;
case V4L2_PIX_FMT_GREY:
- memcpy(vbuf, tmpbuf, buf->vb.width * buf->vb.height);
+ memcpy(vbuf, tmpbuf, vc->width * vc->height);
break;
case V4L2_PIX_FMT_JPEG:
case V4L2_PIX_FMT_MJPEG:
- buf->vb.size = jpgsize;
- memcpy(vbuf, tmpbuf, buf->vb.size);
+ buf->vb.v4l2_buf.length = jpgsize;
+ memcpy(vbuf, tmpbuf, jpgsize);
[PATCH v2] media: soc-camera: OF cameras
From: Guennadi Liakhovetski
With OF we aren't getting platform data any more. To minimise changes we
create all the missing data ourselves, including compulsory struct
soc_camera_link objects. Host-client linking is now done, based on the OF
data. Media bus numbers also have to be assigned dynamically.
OF probing reuses the V4L2 Async API which is used by async non-OF probing.
Signed-off-by: Guennadi Liakhovetski
Signed-off-by: Bryan Wu
---
v2:
- move to use V4L2 Async API
- cleanup some coding style issue
- allocate struct soc_camera_desc sdesc on stack
- cleanup unbalanced mutex operation
drivers/media/platform/soc_camera/soc_camera.c | 232 -
include/media/soc_camera.h | 5 +
2 files changed, 233 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c
b/drivers/media/platform/soc_camera/soc_camera.c
index 4b8c024..ffe1254 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -36,6 +37,7 @@
#include
#include
#include
+#include
#include
#include
@@ -49,6 +51,7 @@
vb2_is_streaming(&(icd)->vb2_vidq))
#define MAP_MAX_NUM 32
+static DECLARE_BITMAP(host_map, MAP_MAX_NUM);
static DECLARE_BITMAP(device_map, MAP_MAX_NUM);
static LIST_HEAD(hosts);
static LIST_HEAD(devices);
@@ -65,6 +68,17 @@ struct soc_camera_async_client {
struct list_head list; /* needed for clean up */
};
+struct soc_camera_of_client {
+ struct soc_camera_desc *sdesc;
+ struct soc_camera_async_client sasc;
+ struct v4l2_of_endpoint node;
+ struct dev_archdata archdata;
+ struct device_node *link_node;
+ union {
+ struct i2c_board_info i2c_info;
+ };
+};
+
static int soc_camera_video_start(struct soc_camera_device *icd);
static int video_dev_create(struct soc_camera_device *icd);
@@ -1336,6 +1350,10 @@ static int soc_camera_i2c_init(struct soc_camera_device
*icd,
return -EPROBE_DEFER;
}
+ /* OF probing skips following I2C init */
+ if (shd->host_wait)
+ return 0;
+
ici = to_soc_camera_host(icd->parent);
adap = i2c_get_adapter(shd->i2c_adapter_id);
if (!adap) {
@@ -1573,10 +1591,180 @@ static void scan_async_host(struct soc_camera_host
*ici)
asd += ici->asd_sizes[j];
}
}
+
+static void soc_camera_of_i2c_ifill(struct soc_camera_of_client *sofc,
+ struct i2c_client *client)
+{
+ struct i2c_board_info *info = &sofc->i2c_info;
+ struct soc_camera_desc *sdesc = sofc->sdesc;
+
+ /* on OF I2C devices platform_data == NULL */
+ info->flags = client->flags;
+ info->addr = client->addr;
+ info->irq = client->irq;
+ info->archdata = &sofc->archdata;
+
+ /* archdata is always empty on OF I2C devices */
+ strlcpy(info->type, client->name, sizeof(info->type));
+
+ sdesc->host_desc.i2c_adapter_id = client->adapter->nr;
+}
+
+static void soc_camera_of_i2c_info(struct device_node *node,
+ struct soc_camera_of_client *sofc)
+{
+ struct i2c_client *client;
+ struct soc_camera_desc *sdesc = sofc->sdesc;
+ struct i2c_board_info *info = &sofc->i2c_info;
+ struct device_node *port, *sensor, *bus;
+
+ port = v4l2_of_get_remote_port(node);
+ if (!port)
+ return;
+
+ /* Check the bus */
+ sensor = of_get_parent(port);
+ bus = of_get_parent(sensor);
+
+ if (of_node_cmp(bus->name, "i2c")) {
+ of_node_put(port);
+ of_node_put(sensor);
+ of_node_put(bus);
+ return;
+ }
+
+ info->of_node = sensor;
+ sdesc->host_desc.board_info = info;
+
+ client = of_find_i2c_device_by_node(sensor);
+ /*
+* of_i2c_register_devices() took a reference to the OF node, it is not
+* dropped, when the I2C device is removed, so, we don't need an
+* additional reference.
+*/
+ of_node_put(sensor);
+ if (client) {
+ soc_camera_of_i2c_ifill(sofc, client);
+ sofc->link_node = sensor;
+ put_device(&client->dev);
+ }
+
+ /* client hasn't attached to I2C yet */
+}
+
+static struct soc_camera_of_client *soc_camera_of_alloc_client(const struct
soc_camera_host *ici,
+ struct
device_node *node)
+{
+ struct soc_camera_of_client *sofc;
+ struct v4l2_async_subdev *sensor;
+ struct soc_camera_desc sdesc = { .host_desc.host_wait = true,};
+ int i, ret;
+
+ sofc = devm_kzalloc(ici->v4l2_dev.dev, sizeof(*sofc), GFP_KERNEL);
+ if (!sofc)
+ return NULL;
+
+ /* Allocate v4
[GIT PULL] AF9035 / IT9135
The following changes since commit 37e59f876bc710d67a30b660826a5e83e07101ce: [media, edac] Change my email address (2014-02-07 08:03:07 -0200) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git af9035 for you to fetch changes up to 57f53ffd5db1c45f7b5253c1f6fadad7d87715ae: af9035: use default i2c slave address for af9035 too (2014-02-12 21:44:13 +0200) Antti Palosaari (1): af9035: use default i2c slave address for af9035 too Malcolm Priestley (3): af9035: Move it913x single devices to af9035 af9035: add default 0x9135 slave I2C address af9035: Add remaining it913x dual ids to af9035. drivers/media/usb/dvb-usb-v2/af9035.c | 39 --- drivers/media/usb/dvb-usb-v2/it913x.c | 29 + 2 files changed, 37 insertions(+), 31 deletions(-) -- http://palosaari.fi/ -- 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
[REVIEW PATCH 2/4] af9035: add default 0x9135 slave I2C address
From: Malcolm Priestley
On some devices the vendor has not set EEPROM_2ND_DEMOD_ADDR.
Checks tmp is not zero after call to get EEPROM_2ND_DEMOD_ADDR and sets the
default slave address of 0x3a on 0x9135 devices.
Signed-off-by: Malcolm Priestley
Signed-off-by: Antti Palosaari
---
drivers/media/usb/dvb-usb-v2/af9035.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c
b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3825c2f..4f682ad 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -576,6 +576,10 @@ static int af9035_download_firmware(struct dvb_usb_device
*d,
goto err;
if (state->chip_type == 0x9135) {
+ if (!tmp)
+ /* default 0x9135 slave I2C address */
+ tmp = 0x3a;
+
ret = af9035_wr_reg(d, 0x004bfb, tmp);
if (ret < 0)
goto err;
@@ -684,6 +688,10 @@ static int af9035_read_config(struct dvb_usb_device *d)
if (ret < 0)
goto err;
+ if (!tmp && state->chip_type == 0x9135)
+ /* default 0x9135 slave I2C address */
+ tmp = 0x3a;
+
state->af9033_config[1].i2c_addr = tmp;
dev_dbg(&d->udev->dev, "%s: 2nd demod I2C addr=%02x\n",
__func__, tmp);
--
1.8.5.3
--
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
[REVIEW PATCH 3/4] af9035: Add remaining it913x dual ids to af9035.
From: Malcolm Priestley
As follow on to patch
af9035: Move it913x single devices to af9035
and patch 1.
SNR is reported as db/10 values.
All dual ids are added to af9035 and it913x driver disabled.
it913x/it913x-fe removal patches to follow.
Signed-off-by: Malcolm Priestley
Signed-off-by: Antti Palosaari
---
drivers/media/usb/dvb-usb-v2/af9035.c | 8
drivers/media/usb/dvb-usb-v2/it913x.c | 5 +
2 files changed, 13 insertions(+)
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c
b/drivers/media/usb/dvb-usb-v2/af9035.c
index 4f682ad..49e8360 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1552,6 +1552,14 @@ static const struct usb_device_id af9035_id_table[] = {
&af9035_props, "Avermedia A835B(4835)", RC_MAP_IT913X_V2) },
{ DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_H335,
&af9035_props, "Avermedia H335", RC_MAP_IT913X_V2) },
+ { DVB_USB_DEVICE(USB_VID_KWORLD_2, USB_PID_KWORLD_UB499_2T_T09,
+ &af9035_props, "Kworld UB499-2T T09", RC_MAP_IT913X_V1) },
+ { DVB_USB_DEVICE(USB_VID_KWORLD_2, USB_PID_SVEON_STV22_IT9137,
+ &af9035_props, "Sveon STV22 Dual DVB-T HDTV",
+ RC_MAP_IT913X_V1) },
+ { DVB_USB_DEVICE(USB_VID_KWORLD_2, USB_PID_CTVDIGDUAL_V2,
+ &af9035_props, "Digital Dual TV Receiver CTVDIGDUAL_V2",
+ RC_MAP_IT913X_V1) },
/* XXX: that same ID [0ccd:0099] is used by af9015 driver too */
{ DVB_USB_DEVICE(USB_VID_TERRATEC, 0x0099,
&af9035_props, "TerraTec Cinergy T Stick Dual RC (rev. 2)",
NULL) },
diff --git a/drivers/media/usb/dvb-usb-v2/it913x.c
b/drivers/media/usb/dvb-usb-v2/it913x.c
index 78bf8fd..39488f8 100644
--- a/drivers/media/usb/dvb-usb-v2/it913x.c
+++ b/drivers/media/usb/dvb-usb-v2/it913x.c
@@ -781,6 +781,8 @@ static const struct usb_device_id it913x_id_table[] = {
{} /* Terminating entry */
};
+#if 0
+
MODULE_DEVICE_TABLE(usb, it913x_id_table);
static struct usb_driver it913x_driver = {
@@ -792,8 +794,11 @@ static struct usb_driver it913x_driver = {
.id_table = it913x_id_table,
};
+
module_usb_driver(it913x_driver);
+#endif
+
MODULE_AUTHOR("Malcolm Priestley ");
MODULE_DESCRIPTION("it913x USB 2 Driver");
MODULE_VERSION("1.33");
--
1.8.5.3
--
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
[REVIEW PATCH 4/4] af9035: use default i2c slave address for af9035 too
Some device vendors has forgotten set correct slave demod I2C address
to eeprom. Use default I2C address when eeprom has no address at all.
Signed-off-by: Antti Palosaari
---
drivers/media/usb/dvb-usb-v2/af9035.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c
b/drivers/media/usb/dvb-usb-v2/af9035.c
index 49e8360..1434d37 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -575,11 +575,11 @@ static int af9035_download_firmware(struct dvb_usb_device
*d,
if (ret < 0)
goto err;
- if (state->chip_type == 0x9135) {
- if (!tmp)
- /* default 0x9135 slave I2C address */
- tmp = 0x3a;
+ /* use default I2C address if eeprom has no address set */
+ if (!tmp)
+ tmp = 0x3a;
+ if (state->chip_type == 0x9135) {
ret = af9035_wr_reg(d, 0x004bfb, tmp);
if (ret < 0)
goto err;
@@ -641,6 +641,7 @@ static int af9035_read_config(struct dvb_usb_device *d)
/* demod I2C "address" */
state->af9033_config[0].i2c_addr = 0x38;
+ state->af9033_config[1].i2c_addr = 0x3a;
state->af9033_config[0].adc_multiplier = AF9033_ADC_MULTIPLIER_2X;
state->af9033_config[1].adc_multiplier = AF9033_ADC_MULTIPLIER_2X;
state->af9033_config[0].ts_mode = AF9033_TS_MODE_USB;
@@ -688,11 +689,9 @@ static int af9035_read_config(struct dvb_usb_device *d)
if (ret < 0)
goto err;
- if (!tmp && state->chip_type == 0x9135)
- /* default 0x9135 slave I2C address */
- tmp = 0x3a;
+ if (tmp)
+ state->af9033_config[1].i2c_addr = tmp;
- state->af9033_config[1].i2c_addr = tmp;
dev_dbg(&d->udev->dev, "%s: 2nd demod I2C addr=%02x\n",
__func__, tmp);
}
--
1.8.5.3
--
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
[REVIEW PATCH 1/4] af9035: Move it913x single devices to af9035
From: Malcolm Priestley
The generic v1 and v2 devices have been all tested.
IDs tested
USB_PID_ITETECH_IT9135 v1 & v2
USB_PID_ITETECH_IT9135_9005 v1
USB_PID_ITETECH_IT9135_9006 v2
Current Issues
There is no signal on
USB_PID_ITETECH_IT9135 v2
No SNR reported all devices.
All single devices tune and scan fine.
All remotes tested okay.
Dual device failed to register second adapter
USB_PID_KWORLD_UB499_2T_T09
It is not clear what the problem is at the moment.
So only single IDs are transferred in this patch.
Signed-off-by: Malcolm Priestley
Signed-off-by: Antti Palosaari
---
drivers/media/usb/dvb-usb-v2/af9035.c | 22 --
drivers/media/usb/dvb-usb-v2/it913x.c | 24
2 files changed, 16 insertions(+), 30 deletions(-)
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c
b/drivers/media/usb/dvb-usb-v2/af9035.c
index 8ede8ea..3825c2f 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1528,12 +1528,22 @@ static const struct usb_device_id af9035_id_table[] = {
{ DVB_USB_DEVICE(USB_VID_TERRATEC, 0x00aa,
&af9035_props, "TerraTec Cinergy T Stick (rev. 2)", NULL) },
/* IT9135 devices */
-#if 0
- { DVB_USB_DEVICE(0x048d, 0x9135,
- &af9035_props, "IT9135 reference design", NULL) },
- { DVB_USB_DEVICE(0x048d, 0x9006,
- &af9035_props, "IT9135 reference design", NULL) },
-#endif
+ { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9135,
+ &af9035_props, "ITE 9135 Generic", RC_MAP_IT913X_V1) },
+ { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9135_9005,
+ &af9035_props, "ITE 9135(9005) Generic", RC_MAP_IT913X_V2) },
+ { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9135_9006,
+ &af9035_props, "ITE 9135(9006) Generic", RC_MAP_IT913X_V1) },
+ { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_A835B_1835,
+ &af9035_props, "Avermedia A835B(1835)", RC_MAP_IT913X_V2) },
+ { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_A835B_2835,
+ &af9035_props, "Avermedia A835B(2835)", RC_MAP_IT913X_V2) },
+ { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_A835B_3835,
+ &af9035_props, "Avermedia A835B(3835)", RC_MAP_IT913X_V2) },
+ { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_A835B_4835,
+ &af9035_props, "Avermedia A835B(4835)", RC_MAP_IT913X_V2) },
+ { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_H335,
+ &af9035_props, "Avermedia H335", RC_MAP_IT913X_V2) },
/* XXX: that same ID [0ccd:0099] is used by af9015 driver too */
{ DVB_USB_DEVICE(USB_VID_TERRATEC, 0x0099,
&af9035_props, "TerraTec Cinergy T Stick Dual RC (rev. 2)",
NULL) },
diff --git a/drivers/media/usb/dvb-usb-v2/it913x.c
b/drivers/media/usb/dvb-usb-v2/it913x.c
index fe95a58..78bf8fd 100644
--- a/drivers/media/usb/dvb-usb-v2/it913x.c
+++ b/drivers/media/usb/dvb-usb-v2/it913x.c
@@ -772,36 +772,12 @@ static const struct usb_device_id it913x_id_table[] = {
{ DVB_USB_DEVICE(USB_VID_KWORLD_2, USB_PID_KWORLD_UB499_2T_T09,
&it913x_properties, "Kworld UB499-2T T09(IT9137)",
RC_MAP_IT913X_V1) },
- { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9135,
- &it913x_properties, "ITE 9135 Generic",
- RC_MAP_IT913X_V1) },
{ DVB_USB_DEVICE(USB_VID_KWORLD_2, USB_PID_SVEON_STV22_IT9137,
&it913x_properties, "Sveon STV22 Dual DVB-T HDTV(IT9137)",
RC_MAP_IT913X_V1) },
- { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9135_9005,
- &it913x_properties, "ITE 9135(9005) Generic",
- RC_MAP_IT913X_V2) },
- { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9135_9006,
- &it913x_properties, "ITE 9135(9006) Generic",
- RC_MAP_IT913X_V1) },
- { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_A835B_1835,
- &it913x_properties, "Avermedia A835B(1835)",
- RC_MAP_IT913X_V2) },
- { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_A835B_2835,
- &it913x_properties, "Avermedia A835B(2835)",
- RC_MAP_IT913X_V2) },
- { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_A835B_3835,
- &it913x_properties, "Avermedia A835B(3835)",
- RC_MAP_IT913X_V2) },
- { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_A835B_4835,
- &it913x_properties, "Avermedia A835B(4835)",
- RC_MAP_IT913X_V2) },
{ DVB_USB_DEVICE(USB_VID_KWORLD_2, USB_PID_CTVDIGDUAL_V2,
&it913x_properties, "Digital Dual TV Receiver CTVDIGDUAL_V2",
RC_MAP_IT913X_V1) },
- { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERM
[GIT PULL] rtl28xxu: add ID [0ccd:00b4] TerraTec NOXON DAB Stick (rev 3)
The following changes since commit 37e59f876bc710d67a30b660826a5e83e07101ce: [media, edac] Change my email address (2014-02-07 08:03:07 -0200) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git rtl28xxu_id for you to fetch changes up to 46a1dac26913fae389e79b7fc83373382b8ba548: rtl28xxu: add ID [0ccd:00b4] TerraTec NOXON DAB Stick (rev 3) (2014-02-12 21:35:21 +0200) Till Dörges (1): rtl28xxu: add ID [0ccd:00b4] TerraTec NOXON DAB Stick (rev 3) drivers/media/dvb-core/dvb-usb-ids.h| 1 + drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 2 ++ 2 files changed, 3 insertions(+) -- http://palosaari.fi/ -- 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] tda10071 changes
The following changes since commit 37e59f876bc710d67a30b660826a5e83e07101ce: [media, edac] Change my email address (2014-02-07 08:03:07 -0200) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git tda10071 for you to fetch changes up to a47dd5674fda7c48d693cd785f48f92fcc20b8c9: tda10071: coding style issues (2014-02-12 20:17:33 +0200) Antti Palosaari (2): tda10071: do not check tuner PLL lock on read_status() tda10071: coding style issues Dan Carpenter (1): tda10071: remove a duplicative test drivers/media/dvb-frontends/tda10071.c | 68 drivers/media/dvb-frontends/tda10071.h | 2 +- 2 files changed, 37 insertions(+), 33 deletions(-) -- http://palosaari.fi/ -- 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
[REVIEW PATCH 1/3] tda10071: remove a duplicative test
From: Dan Carpenter "ret" is an error code here, we already tested that. Signed-off-by: Dan Carpenter Acked-by: Antti Palosaari Reviewed-by: Antti Palosaari Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/tda10071.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/media/dvb-frontends/tda10071.c b/drivers/media/dvb-frontends/tda10071.c index 8ad3a57..a76df29 100644 --- a/drivers/media/dvb-frontends/tda10071.c +++ b/drivers/media/dvb-frontends/tda10071.c @@ -1006,8 +1006,7 @@ static int tda10071_init(struct dvb_frontend *fe) dev_err(&priv->i2c->dev, "%s: firmware " \ "download failed=%d\n", KBUILD_MODNAME, ret); - if (ret) - goto error_release_firmware; + goto error_release_firmware; } } release_firmware(fw); -- 1.8.5.3 -- 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
[REVIEW PATCH 2/3] tda10071: do not check tuner PLL lock on read_status()
Tuner PLL lock flag was mapped to FE_HAS_SIGNAL, which is wrong. PLL lock has nothing to do with received signal. In real life that flag is always set. Signed-off-by: Antti Palosaari --- drivers/media/dvb-frontends/tda10071.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/media/dvb-frontends/tda10071.c b/drivers/media/dvb-frontends/tda10071.c index a76df29..13c823a 100644 --- a/drivers/media/dvb-frontends/tda10071.c +++ b/drivers/media/dvb-frontends/tda10071.c @@ -491,10 +491,9 @@ static int tda10071_read_status(struct dvb_frontend *fe, fe_status_t *status) if (ret) goto error; - if (tmp & 0x01) /* tuner PLL */ - *status |= FE_HAS_SIGNAL; + /* 0x39[0] tuner PLL */ if (tmp & 0x02) /* demod PLL */ - *status |= FE_HAS_CARRIER; + *status |= FE_HAS_SIGNAL | FE_HAS_CARRIER; if (tmp & 0x04) /* viterbi or LDPC*/ *status |= FE_HAS_VITERBI; if (tmp & 0x08) /* RS or BCH */ -- 1.8.5.3 -- 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
[REVIEW PATCH 3/3] tda10071: coding style issues
Fix some coding style issues, mostly reported by checkpatch.pl.
Signed-off-by: Antti Palosaari
---
drivers/media/dvb-frontends/tda10071.c | 60 +++---
drivers/media/dvb-frontends/tda10071.h | 2 +-
2 files changed, 34 insertions(+), 28 deletions(-)
diff --git a/drivers/media/dvb-frontends/tda10071.c
b/drivers/media/dvb-frontends/tda10071.c
index 13c823a..522fe00 100644
--- a/drivers/media/dvb-frontends/tda10071.c
+++ b/drivers/media/dvb-frontends/tda10071.c
@@ -42,8 +42,8 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, u8
reg, u8 *val,
if (1 + len > sizeof(buf)) {
dev_warn(&priv->i2c->dev,
-"%s: i2c wr reg=%04x: len=%d is too big!\n",
-KBUILD_MODNAME, reg, len);
+ "%s: i2c wr reg=%04x: len=%d is too big!\n",
+ KBUILD_MODNAME, reg, len);
return -EINVAL;
}
@@ -54,8 +54,9 @@ static int tda10071_wr_regs(struct tda10071_priv *priv, u8
reg, u8 *val,
if (ret == 1) {
ret = 0;
} else {
- dev_warn(&priv->i2c->dev, "%s: i2c wr failed=%d reg=%02x " \
- "len=%d\n", KBUILD_MODNAME, ret, reg, len);
+ dev_warn(&priv->i2c->dev,
+ "%s: i2c wr failed=%d reg=%02x len=%d\n",
+ KBUILD_MODNAME, ret, reg, len);
ret = -EREMOTEIO;
}
return ret;
@@ -83,8 +84,8 @@ static int tda10071_rd_regs(struct tda10071_priv *priv, u8
reg, u8 *val,
if (len > sizeof(buf)) {
dev_warn(&priv->i2c->dev,
-"%s: i2c wr reg=%04x: len=%d is too big!\n",
-KBUILD_MODNAME, reg, len);
+ "%s: i2c wr reg=%04x: len=%d is too big!\n",
+ KBUILD_MODNAME, reg, len);
return -EINVAL;
}
@@ -93,8 +94,9 @@ static int tda10071_rd_regs(struct tda10071_priv *priv, u8
reg, u8 *val,
memcpy(val, buf, len);
ret = 0;
} else {
- dev_warn(&priv->i2c->dev, "%s: i2c rd failed=%d reg=%02x " \
- "len=%d\n", KBUILD_MODNAME, ret, reg, len);
+ dev_warn(&priv->i2c->dev,
+ "%s: i2c rd failed=%d reg=%02x len=%d\n",
+ KBUILD_MODNAME, ret, reg, len);
ret = -EREMOTEIO;
}
return ret;
@@ -667,11 +669,11 @@ static int tda10071_set_frontend(struct dvb_frontend *fe)
int ret, i;
u8 mode, rolloff, pilot, inversion, div;
- dev_dbg(&priv->i2c->dev, "%s: delivery_system=%d modulation=%d " \
- "frequency=%d symbol_rate=%d inversion=%d pilot=%d " \
- "rolloff=%d\n", __func__, c->delivery_system, c->modulation,
- c->frequency, c->symbol_rate, c->inversion, c->pilot,
- c->rolloff);
+ dev_dbg(&priv->i2c->dev,
+ "%s: delivery_system=%d modulation=%d frequency=%d
symbol_rate=%d inversion=%d pilot=%d rolloff=%d\n",
+ __func__, c->delivery_system, c->modulation,
+ c->frequency, c->symbol_rate, c->inversion, c->pilot,
+ c->rolloff);
priv->delivery_system = SYS_UNDEFINED;
@@ -951,10 +953,8 @@ static int tda10071_init(struct dvb_frontend *fe)
/* request the firmware, this will block and timeout */
ret = request_firmware(&fw, fw_file, priv->i2c->dev.parent);
if (ret) {
- dev_err(&priv->i2c->dev, "%s: did not find the " \
- "firmware file. (%s) Please see " \
- "linux/Documentation/dvb/ for more " \
- "details on firmware-problems. (%d)\n",
+ dev_err(&priv->i2c->dev,
+ "%s: did not find the firmware file.
(%s) Please see linux/Documentation/dvb/ for more details on firmware-problems.
(%d)\n",
KBUILD_MODNAME, fw_file, ret);
goto error;
}
@@ -984,11 +984,12 @@ static int tda10071_init(struct dvb_frontend *fe)
if (ret)
goto error_release_firmware;
- dev_info(&priv->i2c->dev, "%s: found a '%s' in cold state, " \
- "will try to load a firmware\n", KBUILD_MODNAME,
- tda10071_ops.info.name);
- dev_info(&priv->i2c->dev, "%s: downloading firmware from " \
- "file '%s'\n", KBUILD_MODNAME, fw_file);
+ dev_info(&priv->i2c->dev,
+ "%s: found a '%s' in cold state, will try to
load a firmware\n",
+
[REVIEW PATCH] rtl28xxu: add ID [0ccd:00b4] TerraTec NOXON DAB Stick (rev 3)
From: Till Dörges
I've got the following DAB USB stick that also works fine with the
DVB_USB_RTL28XXU driver after I added its USB ID:
--- snip ---
user@box:~> lsusb -d 0ccd:00b4
Bus 001 Device 009: ID 0ccd:00b4 TerraTec Electronic GmbH
--- snap ---
[cr...@iki.fi: apply patch partly manually]
Signed-off-by: Till Dörges
Signed-off-by: Antti Palosaari
---
drivers/media/dvb-core/dvb-usb-ids.h| 1 +
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/media/dvb-core/dvb-usb-ids.h
b/drivers/media/dvb-core/dvb-usb-ids.h
index f19a2cc..1bdc0e7 100644
--- a/drivers/media/dvb-core/dvb-usb-ids.h
+++ b/drivers/media/dvb-core/dvb-usb-ids.h
@@ -257,6 +257,7 @@
#define USB_PID_TERRATEC_T50x10a1
#define USB_PID_NOXON_DAB_STICK0x00b3
#define USB_PID_NOXON_DAB_STICK_REV2 0x00e0
+#define USB_PID_NOXON_DAB_STICK_REV3 0x00b4
#define USB_PID_PINNACLE_EXPRESSCARD_320CX 0x022e
#define USB_PID_PINNACLE_PCTV2000E 0x022c
#define USB_PID_PINNACLE_PCTV_DVB_T_FLASH 0x0228
diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index fda5c64..8e61523 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -1401,6 +1401,8 @@ static const struct usb_device_id rtl28xxu_id_table[] = {
&rtl2832u_props, "TerraTec NOXON DAB Stick", NULL) },
{ DVB_USB_DEVICE(USB_VID_TERRATEC, USB_PID_NOXON_DAB_STICK_REV2,
&rtl2832u_props, "TerraTec NOXON DAB Stick (rev 2)", NULL) },
+ { DVB_USB_DEVICE(USB_VID_TERRATEC, USB_PID_NOXON_DAB_STICK_REV3,
+ &rtl2832u_props, "TerraTec NOXON DAB Stick (rev 3)", NULL) },
{ DVB_USB_DEVICE(USB_VID_GTEK, USB_PID_TREKSTOR_TERRES_2_0,
&rtl2832u_props, "Trekstor DVB-T Stick Terres 2.0", NULL) },
{ DVB_USB_DEVICE(USB_VID_DEXATEK, 0x1101,
--
1.8.5.3
--
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 3.14] em28xx-dvb m88ts2022 tuner binding fix
The following changes since commit 37e59f876bc710d67a30b660826a5e83e07101ce: [media, edac] Change my email address (2014-02-07 08:03:07 -0200) are available in the git repository at: git://linuxtv.org/anttip/media_tree.git pctv_461e_fix for you to fetch changes up to 6d56b487e83787e7c532ba8f45de8f71a93aab76: em28xx-dvb: fix PCTV 461e tuner I2C binding (2014-02-12 20:05:05 +0200) Antti Palosaari (1): em28xx-dvb: fix PCTV 461e tuner I2C binding drivers/media/usb/em28xx/em28xx-dvb.c | 27 +-- 1 file changed, 25 insertions(+), 2 deletions(-) -- http://palosaari.fi/ -- 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 FOR 3.14] em28xx-dvb: fix PCTV 461e tuner I2C binding
Add missing m88ts2022 module reference counts as removing that module
is not allowed when it is used by em28xx-dvb module. That same module
was not unregistered correctly, fix it too.
Error cases validated by returning errors from m88ds3103, m88ts2022
and a8293 probe().
Signed-off-by: Antti Palosaari
---
drivers/media/usb/em28xx/em28xx-dvb.c | 27 +--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c
b/drivers/media/usb/em28xx/em28xx-dvb.c
index a0a669e..defac24 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -1374,6 +1374,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
{
/* demod I2C adapter */
struct i2c_adapter *i2c_adapter;
+ struct i2c_client *client;
struct i2c_board_info info;
struct m88ts2022_config m88ts2022_config = {
.clock = 2700,
@@ -1396,7 +1397,19 @@ static int em28xx_dvb_init(struct em28xx *dev)
info.addr = 0x60;
info.platform_data = &m88ts2022_config;
request_module("m88ts2022");
- dvb->i2c_client_tuner = i2c_new_device(i2c_adapter,
&info);
+ client = i2c_new_device(i2c_adapter, &info);
+ if (client == NULL || client->dev.driver == NULL) {
+ dvb_frontend_detach(dvb->fe[0]);
+ result = -ENODEV;
+ goto out_free;
+ }
+
+ if (!try_module_get(client->dev.driver->owner)) {
+ i2c_unregister_device(client);
+ dvb_frontend_detach(dvb->fe[0]);
+ result = -ENODEV;
+ goto out_free;
+ }
/* delegate signal strength measurement to tuner */
dvb->fe[0]->ops.read_signal_strength =
@@ -1406,10 +1419,14 @@ static int em28xx_dvb_init(struct em28xx *dev)
if (!dvb_attach(a8293_attach, dvb->fe[0],
&dev->i2c_adap[dev->def_i2c_bus],
&em28xx_a8293_config)) {
+ module_put(client->dev.driver->owner);
+ i2c_unregister_device(client);
dvb_frontend_detach(dvb->fe[0]);
result = -ENODEV;
goto out_free;
}
+
+ dvb->i2c_client_tuner = client;
}
break;
default:
@@ -1471,6 +1488,7 @@ static int em28xx_dvb_fini(struct em28xx *dev)
if (dev->dvb) {
struct em28xx_dvb *dvb = dev->dvb;
+ struct i2c_client *client = dvb->i2c_client_tuner;
em28xx_uninit_usb_xfer(dev, EM28XX_DIGITAL_MODE);
@@ -1483,7 +1501,12 @@ static int em28xx_dvb_fini(struct em28xx *dev)
prevent_sleep(&dvb->fe[1]->ops);
}
- i2c_release_client(dvb->i2c_client_tuner);
+ /* remove I2C tuner */
+ if (client) {
+ module_put(client->dev.driver->owner);
+ i2c_unregister_device(client);
+ }
+
em28xx_unregister_dvb(dvb);
kfree(dvb);
dev->dvb = NULL;
--
1.8.5.3
--
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] em28xx: Only deallocate struct em28xx after finishing all extensions
Am 09.02.2014 21:09, schrieb Mauro Carvalho Chehab: >> Ping ! >> > Are you going to continue working on this patch ? >> > I've planned to come up with some follow-up changes... ;-) > Yes, but I'm currently without time to do your proposal changes and > test, as I'm in a 2-week business trip. I didn't bring any test machine. Ok, good to know. > If you want, I can commit this one as-is, and you can do your > proposed changes on a separate patch. No problem, I can wait. The current version of the patch would reintroduce the sysfs group warnings, so I think its safer to do the changes all at once. 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: [PATCH v2] [media] of: move graph helpers from drivers/media/v4l2-core to drivers/media
Hi Mauro, Am Mittwoch, den 12.02.2014, 22:35 +0900 schrieb Mauro Carvalho Chehab: > Em Wed, 12 Feb 2014 10:11:54 +0100 > Philipp Zabel escreveu: > > > Hi Mauro, > > > > Am Mittwoch, den 12.02.2014, 06:53 +0900 schrieb Mauro Carvalho Chehab: > > [...] > > > > diff --git a/include/media/of_graph.h b/include/media/of_graph.h > > > > new file mode 100644 > > > > index 000..3bbeb60 > > > > --- /dev/null > > > > +++ b/include/media/of_graph.h > > > > @@ -0,0 +1,46 @@ > > > > +/* > > > > + * OF graph binding parsing helpers > > > > + * > > > > + * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd. > > > > + * Author: Sylwester Nawrocki > > > > + * > > > > + * Copyright (C) 2012 Renesas Electronics Corp. > > > > + * Author: Guennadi Liakhovetski > > > > + * > > > > + * This program is free software; you can redistribute it and/or modify > > > > + * it under the terms of version 2 of the GNU General Public License as > > > > + * published by the Free Software Foundation. > > > > + */ > > > > +#ifndef __LINUX_OF_GRAPH_H > > > > +#define __LINUX_OF_GRAPH_H > > > > + > > > > +#ifdef CONFIG_OF > > > > > > As a matter of consistency, it would be better to test here for > > > CONFIG_OF_GRAPH instead, to reflect the same symbol that enables such > > > functions as used on Kconfig/Makefile. > > > > Maybe I'm trying to be too clever for my own good, but my reasoning was > > as follows: > > > > Suppose I newly use the of_graph_ helpers in a subsystem that does not > > yet select OF_GRAPH. In that case I'd rather get linking errors earlier > > rather than stubbed out functions that silently fail to parse the DT > > later. > > I see your point, but, imagining that someone pushed a patch using those > symbols upstream, that would break compilation and git bisection, with > will hurt everyone, and not only the very few of us that would actually > need the OF_GRAPH symbol for an specific driver. > > Also, such push would mean that someone forgot to do his homework and > to test if the committed functionality is actually working. > > So, it seems more fair that the one that did the mistake will be the one > that will suffer the consequences for his errors instead of applying a > penalty to everybody's else ;) point taken. > > Since there is > > config VIDEO_DEV > > select OF_GRAPH if OF > > already and the same should be added for other users of device tree > > graphs, I think stubbing out the functions only if OF is disabled should > > be enough. > > Well, if you want to be sure that the graph will always be there if OF, then > you could do, instead: > > config OF_GRAPH > bool > default OF > > (that would actually make OF_GRAPH just an alias to OF - so we could just > use OF instead). > > In any case, I think that we should use the same config name at Makefile, > Kconfig and of_graph.h (either OF_GRAPH or just OF). If nobody states a strong preference, I'll just get rid of the CONFIG_OF_GRAPH option altogether and use CONFIG_OF directly. regards Philipp -- 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] s2255drv: upgrade to videobuf2
"./utils/v4l2-compliance/v4l2-compliance -s" Driver Info: Driver name : s2255 Card type : s2255 Bus info : usb-:00:1a.7-3.6 Driver version: 3.13.0 Capabilities : 0x8401 Video Capture Streaming Device Capabilities Device Caps : 0x0401 Video Capture Streaming Compliance test for device /dev/video0 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Control ioctls: test VIDIOC_QUERYCTRL/MENU: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK warn: v4l2-test-controls.cpp(753): The VIDIOC_G_JPEGCOMP ioctl is deprecated! warn: v4l2-test-controls.cpp(770): The VIDIOC_S_JPEGCOMP ioctl is deprecated! test VIDIOC_G/S_JPEGCOMP: OK Standard Controls: 7 Private Controls: 1 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK warn: v4l2-test-formats.cpp(599): TRY_FMT cannot handle an invalid pixelformat. warn: v4l2-test-formats.cpp(600): This may or may not be a problem. For more information see: warn: v4l2-test-formats.cpp(601): http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html test VIDIOC_TRY_FMT: OK warn: v4l2-test-formats.cpp(786): S_FMT cannot handle an invalid pixelformat. warn: v4l2-test-formats.cpp(787): This may or may not be a problem. For more information see: warn: v4l2-test-formats.cpp(788): http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: warn: v4l2-test-buffers.cpp(343): VIDIOC_CREATE_BUFS not supported test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK fail: v4l2-test-buffers.cpp(379): ret < 0 && errno != EINVAL test read/write: FAIL fail: v4l2-test-buffers.cpp(537): can_stream test MMAP: FAIL fail: v4l2-test-buffers.cpp(641): can_stream test USERPTR: FAIL Total: 39, Succeeded: 36, Failed: 3, Warnings: 9 On 2014-02-11 17:07, Hans Verkuil wrote: Hi Dean, On 02/12/2014 12:05 AM, Dean Anderson wrote: The output of v4l2-compliance looks fine. The warnings can be ignored for this patch. Can you update to the latest v4l2-compliance that I just pushed this morning? I added support for the streaming I/O ioctls (use the -s option and make sure that you have a valid video signal). That will be very useful. Regards, Hans "./v4l2-compliance ": Driver Info: Driver name : s2255 Card type : s2255 Bus info : usb-:00:1a.7-3.6 Driver version: 3.13.0 Capabilities : 0x8401 Video Capture Streaming Device Capabilities Device Caps : 0x0401 Video Capture Streaming Compliance test for device /dev/video0 (not using libv4l2): Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second video open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test
Re: [PATCH] s2255drv: upgrade to videobuf2
On 02/12/14 17:01, Dean Anderson wrote: > "./utils/v4l2-compliance/v4l2-compliance -s" > > Driver Info: > Driver name : s2255 > Card type : s2255 > Bus info : usb-:00:1a.7-3.6 > Driver version: 3.13.0 > Capabilities : 0x8401 > Video Capture > Streaming > Device Capabilities > Device Caps : 0x0401 > Video Capture > Streaming > > Compliance test for device /dev/video0 (not using libv4l2): > > Required ioctls: > test VIDIOC_QUERYCAP: OK > > Allow for multiple opens: > test second video open: OK > test VIDIOC_QUERYCAP: OK > test VIDIOC_G/S_PRIORITY: OK > > Debug ioctls: > test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) > test VIDIOC_LOG_STATUS: OK > > Input ioctls: > test VIDIOC_G/S_TUNER: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 1 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Control ioctls: > test VIDIOC_QUERYCTRL/MENU: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > warn: v4l2-test-controls.cpp(753): The VIDIOC_G_JPEGCOMP ioctl is > deprecated! > warn: v4l2-test-controls.cpp(770): The VIDIOC_S_JPEGCOMP ioctl is > deprecated! > test VIDIOC_G/S_JPEGCOMP: OK > Standard Controls: 7 Private Controls: 1 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > > Format ioctls: > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK > test VIDIOC_G/S_PARM: OK > test VIDIOC_G_FBUF: OK (Not Supported) > test VIDIOC_G_FMT: OK > warn: v4l2-test-formats.cpp(599): TRY_FMT cannot handle an invalid > pixelformat. > warn: v4l2-test-formats.cpp(600): This may or may not be a problem. > For more information see: > warn: v4l2-test-formats.cpp(601): > http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html > test VIDIOC_TRY_FMT: OK > warn: v4l2-test-formats.cpp(786): S_FMT cannot handle an invalid > pixelformat. > warn: v4l2-test-formats.cpp(787): This may or may not be a problem. > For more information see: > warn: v4l2-test-formats.cpp(788): > http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html > test VIDIOC_S_FMT: OK > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) > test VIDIOC_G_ENC_INDEX: OK (Not Supported) > test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) > > Buffer ioctls: > warn: v4l2-test-buffers.cpp(343): VIDIOC_CREATE_BUFS not supported > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK > fail: v4l2-test-buffers.cpp(379): ret < 0 && errno != EINVAL You added read() support, but did not add V4L2_CAP_READWRITE to querycap. The following errors are a knock-on effect of that since the driver is still in read() mode so attempts to call REQBUFS will fail. I should see if I can improve that in v4l2-compliance. Regards, Hans > test read/write: FAIL > fail: v4l2-test-buffers.cpp(537): can_stream > test MMAP: FAIL > fail: v4l2-test-buffers.cpp(641): can_stream > test USERPTR: FAIL > > Total: 39, Succeeded: 36, Failed: 3, Warnings: 9 -- 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: OMAP3 ISP capabilities
Hello Laurent, > > (3) it should be possible to use the ISP resizer input / output > > (memory-to-memory) independently; it there any example code doing this? > I haven't written any sample code as such for memory-to-memory operation. I > usually use the following media-ctl and yavta commands to test memory-to- > memory resizing : > yavta -f YUYV -s 2048x1536 -n 4 --capture=100 \ > `media-ctl -e "OMAP3 ISP resizer input"` > resizer-input.log 2>&1 & > yavta -f YUYV -s 1024x768 -n 4 --capture=100 \ > `./media-ctl -e "OMAP3 ISP resizer output"` > resizer-output.log 2>&1 & thanks for the suggestion; I didn't understand yavta/v4l enough to see how it can feed data to the ISP resizer input p. -- Peter Meerwald +43-664-218 (mobile) -- 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: OMAP3 ISP capabilities
Hi Peter, On Tuesday 11 February 2014 15:54:00 Peter Meerwald wrote: > Hello Laurent, > > some quick question about the OMAP3 ISP pipeline capabilities: > > (1) can the OMAP3 ISP CCDC output concurrently to memory AND the resizer > in YUV mode? I think the answer is no due to hardware limitation Based on the TRM I would say that the hardware is capable of doing so, but this isn't implemented in the driver. > (2) in RAW mode, I think it should be possible to connect pad 1 of the > OMAP3 ISP CCDC to CCDC output and pad 2 to the ISP preview and > subsequently to the resizer? so two stream can be read concurrently from > video2 and video6? That's my understanding as well, but once again this isn't supported by the driver. > (3) it should be possible to use the ISP resizer input / output > (memory-to-memory) independently; it there any example code doing this? I haven't written any sample code as such for memory-to-memory operation. I usually use the following media-ctl and yavta commands to test memory-to- memory resizing : media-ctl -r media-ctl -l '"OMAP3 ISP resizer input":0->"OMAP3 ISP resizer":0[1]' media-ctl -l '"OMAP3 ISP resizer":1->"OMAP3 ISP resizer output":0[1]' media-ctl -V '"OMAP3 ISP resizer":0[YUYV 2048x1536]' media-ctl -V '"OMAP3 ISP resizer":1[YUYV 1024x768]' yavta -f YUYV -s 2048x1536 -n 4 --capture=100 \ `media-ctl -e "OMAP3 ISP resizer input"` > resizer-input.log 2>&1 & yavta -f YUYV -s 1024x768 -n 4 --capture=100 \ `./media-ctl -e "OMAP3 ISP resizer output"` > resizer-output.log 2>&1 & You can also have a look at the omap3-isp-live application available at http://git.ideasonboard.org/omap3-isp-live.git It contains a library that offers viewfinder, snapshot and scaling functions on top of the ISP and two sample applications that use the library. The resizer memory-to-memory is used by the live application to scale captured snapshots when displaying them on screen. -- 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
OMAP3 ISP capabilities, resizer
Hello,
some more findings:
* the driver bug seen below was observed with kernel 3.7 and is already
fixed in more recent kernels, likely with commit 864a121274,
[media] v4l: Don't warn during link validation when encountering a V4L2
devnode
* there still is a a confusing warning:
omap3isp omap3isp: can't find source, failing now
which may be addressed by the patch here https://linuxtv.org/patch/15200/,
but has not been applied
* I have a test program, http://pmeerw.net/scaler.c, which exercises the
OMAP3 ISP resizer standalone with the pipeline given below; it crashes the
system quite reliably on 3.7 and recent kernels :(
the reason for the crash is that the ISP resizer can still be busy and
doesn't like to be turned off then; a little sleep before
omap3isp_sbl_disable() helps (or waiting for the ISP resize to become
idle, see the patch below)
I'm not sure what omap3isp_module_sync_idle() is supposed to do, it
immediately returns since we are in SINGLESHOT mode and
isp_pipeline_ready() is false
with below patch I am happily resizing...
// snip, RFC
From: Peter Meerwald
Date: Wed, 12 Feb 2014 15:59:20 +0100
Subject: [PATCH] omap3isp: Wait for resizer to become idle before
disabling
---
drivers/media/platform/omap3isp/ispresizer.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/media/platform/omap3isp/ispresizer.c
b/drivers/media/platform/omap3isp/ispresizer.c
index d11fb26..fea98f7 100644
--- a/drivers/media/platform/omap3isp/ispresizer.c
+++ b/drivers/media/platform/omap3isp/ispresizer.c
@@ -1145,6 +1145,7 @@ static int resizer_set_stream(struct v4l2_subdev *sd, int
enable)
struct isp_video *video_out = &res->video_out;
struct isp_device *isp = to_isp_device(res);
struct device *dev = to_device(res);
+ unsigned long timeout = 0;
if (res->state == ISP_PIPELINE_STREAM_STOPPED) {
if (enable == ISP_PIPELINE_STREAM_STOPPED)
@@ -1176,6 +1177,15 @@ static int resizer_set_stream(struct v4l2_subdev *sd,
int enable)
if (omap3isp_module_sync_idle(&sd->entity, &res->wait,
&res->stopping))
dev_dbg(dev, "%s: module stop timeout.\n", sd->name);
+
+ while (omap3isp_resizer_busy(res)) {
+ if (timeout++ > 1000) {
+ dev_alert(isp->dev, "ISP resizer does not
become idle\n");
+ return -ETIMEDOUT;
+ }
+ udelay(100);
+ }
+
omap3isp_sbl_disable(isp, OMAP3_ISP_SBL_RESIZER_READ |
OMAP3_ISP_SBL_RESIZER_WRITE);
omap3isp_subclk_disable(isp, OMAP3_ISP_SUBCLK_RESIZER);
// snip
regards, p.
--
Peter Meerwald
+43-664-218 (mobile)
-- Forwarded message --
Date: Tue, 11 Feb 2014 17:02:23 +0100 (CET)
From: Peter Meerwald
To: Laurent Pinchart
Cc: linux-media@vger.kernel.org, linux-o...@vger.kernel.org
Subject: OMAP3 ISP capabilities (fwd)
Hello,
trying (3) below and hitting
[ 6241.536071] WARNING: at drivers/media/v4l2-core/v4l2-subdev.c:424
v4l2_subdev_link_validate_get_format+0x90/0xa8()
[ 6241.573150] Driver bug! Wrong media entity type 65536, entity OMAP3 ISP
resizer input
[ 6241.595153] [] (unwind_backtrace+0x0/0xe0) from []
(warn_slowpath_common+0x4c/0x64)
[ 6241.605651] [] (warn_slowpath_common+0x4c/0x64) from []
(warn_slowpath_fmt+0x2c/0x3c)
[ 6241.615997] [] (warn_slowpath_fmt+0x2c/0x3c) from []
(v4l2_subdev_link_validate_get_format+0x90/0xa8)
[ 6241.632751] [] (v4l2_subdev_link_validate_get_format+0x90/0xa8)
from [] (v4l2_subdev_link_validate+0x18)
[ 6241.645324] [] (v4l2_subdev_link_validate+0x18/0xac) from
[] (media_entity_pipeline_start+0xc8/0x170)
[ 6241.657012] [] (media_entity_pipeline_start+0xc8/0x170) from
[] (isp_video_streamon+0xa4/0x314)
[ 6241.672027] [] (isp_video_streamon+0xa4/0x314) from []
(v4l_streamon+0x18/0x1c)
[ 6241.681884] [] (v4l_streamon+0x18/0x1c) from []
(__video_do_ioctl+0x1c4/0x2e4)
[ 6241.691558] [] (__video_do_ioctl+0x1c4/0x2e4) from []
(video_usercopy+0x2a4/0x3e0)
[ 6241.701507] [] (video_usercopy+0x2a4/0x3e0) from []
(v4l2_ioctl+0x6c/0x110)
[ 6241.715240] [] (v4l2_ioctl+0x6c/0x110) from []
(do_vfs_ioctl+0x548/0x5b8)
[ 6241.724792] [] (do_vfs_ioctl+0x548/0x5b8) from []
(sys_ioctl+0x38/0x54)
[ 6241.733795] [] (sys_ioctl+0x38/0x54) from []
(ret_fast_syscall+0x0/0x30)
[ 6241.743438] ---[ end trace 3ce601d6bddf2d7e ]---
pipeline is
media-ctl -r
media-ctl -l '"OMAP3 ISP resizer input":0->"OMAP3 ISP resizer":0[1]'
media-ctl -l '"OMAP3 ISP resizer":1->"OMAP3 ISP resizer output":0[1]'
media-ctl -V '"OMAP3 ISP resizer":0 [YUYV2X8 640x480]'
media-ctl -V '"OMAP3 ISP resizer":1 [YUYV2X8 320x240]'
I'm opening /dev/video5 and /dev/video6, do S_FMT on both as VIDEO_OUTPUT
and VIDEO_CAPTURE, respectively, configure buffers and then STREAMON --
bang!
regards, p.
--
Re: [PATCH 36/47] adv7604: Make output format configurable through pad format operations
On 02/05/14 17:42, Laurent Pinchart wrote:
> Replace the dummy video format operations by pad format operations that
> configure the output format.
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/media/i2c/adv7604.c | 243
> +++-
> include/media/adv7604.h | 47 ++---
> 2 files changed, 225 insertions(+), 65 deletions(-)
>
> diff --git a/include/media/adv7604.h b/include/media/adv7604.h
> index 22811d3..2cc8e16 100644
> --- a/include/media/adv7604.h
> +++ b/include/media/adv7604.h
> @@ -32,16 +32,6 @@ enum adv7604_ain_sel {
> ADV7604_AIN9_4_5_6_SYNC_2_1 = 4,
> };
>
> -/* Bus rotation and reordering (IO register 0x04, [7:5]) */
> -enum adv7604_op_ch_sel {
> - ADV7604_OP_CH_SEL_GBR = 0,
> - ADV7604_OP_CH_SEL_GRB = 1,
> - ADV7604_OP_CH_SEL_BGR = 2,
> - ADV7604_OP_CH_SEL_RGB = 3,
> - ADV7604_OP_CH_SEL_BRG = 4,
> - ADV7604_OP_CH_SEL_RBG = 5,
> -};
> -
> /* Input Color Space (IO register 0x02, [7:4]) */
> enum adv7604_inp_color_space {
> ADV7604_INP_COLOR_SPACE_LIM_RGB = 0,
> @@ -55,29 +45,11 @@ enum adv7604_inp_color_space {
> ADV7604_INP_COLOR_SPACE_AUTO = 0xf,
> };
>
> -/* Select output format (IO register 0x03, [7:0]) */
> -enum adv7604_op_format_sel {
> - ADV7604_OP_FORMAT_SEL_SDR_ITU656_8 = 0x00,
> - ADV7604_OP_FORMAT_SEL_SDR_ITU656_10 = 0x01,
> - ADV7604_OP_FORMAT_SEL_SDR_ITU656_12_MODE0 = 0x02,
> - ADV7604_OP_FORMAT_SEL_SDR_ITU656_12_MODE1 = 0x06,
> - ADV7604_OP_FORMAT_SEL_SDR_ITU656_12_MODE2 = 0x0a,
> - ADV7604_OP_FORMAT_SEL_DDR_422_8 = 0x20,
> - ADV7604_OP_FORMAT_SEL_DDR_422_10 = 0x21,
> - ADV7604_OP_FORMAT_SEL_DDR_422_12_MODE0 = 0x22,
> - ADV7604_OP_FORMAT_SEL_DDR_422_12_MODE1 = 0x23,
> - ADV7604_OP_FORMAT_SEL_DDR_422_12_MODE2 = 0x24,
> - ADV7604_OP_FORMAT_SEL_SDR_444_24 = 0x40,
> - ADV7604_OP_FORMAT_SEL_SDR_444_30 = 0x41,
> - ADV7604_OP_FORMAT_SEL_SDR_444_36_MODE0 = 0x42,
> - ADV7604_OP_FORMAT_SEL_DDR_444_24 = 0x60,
> - ADV7604_OP_FORMAT_SEL_DDR_444_30 = 0x61,
> - ADV7604_OP_FORMAT_SEL_DDR_444_36 = 0x62,
> - ADV7604_OP_FORMAT_SEL_SDR_ITU656_16 = 0x80,
> - ADV7604_OP_FORMAT_SEL_SDR_ITU656_20 = 0x81,
> - ADV7604_OP_FORMAT_SEL_SDR_ITU656_24_MODE0 = 0x82,
> - ADV7604_OP_FORMAT_SEL_SDR_ITU656_24_MODE1 = 0x86,
> - ADV7604_OP_FORMAT_SEL_SDR_ITU656_24_MODE2 = 0x8a,
> +/* Select output format (IO register 0x03, [4:2]) */
> +enum adv7604_op_format_mode_sel {
> + ADV7604_OP_FORMAT_MODE0 = 0x00,
> + ADV7604_OP_FORMAT_MODE1 = 0x04,
> + ADV7604_OP_FORMAT_MODE2 = 0x08,
> };
>
> enum adv7604_drive_strength {
> @@ -104,11 +76,8 @@ struct adv7604_platform_data {
> /* Analog input muxing mode */
> enum adv7604_ain_sel ain_sel;
>
> - /* Bus rotation and reordering */
> - enum adv7604_op_ch_sel op_ch_sel;
I would keep this as part of the platform_data. This is typically used if things
are wired up in a non-standard way and so is specific to the hardware. It is not
something that will change from format to format.
Other than this it all looks quite nice! Much more flexible than before.
Regards,
Hans
> -
> - /* Select output format */
> - enum adv7604_op_format_sel op_format_sel;
> + /* Select output format mode */
> + enum adv7604_op_format_mode_sel op_format_mode_sel;
>
> /* Configuration of the INT1 pin */
> enum adv7604_int1_config int1_config;
> @@ -116,14 +85,12 @@ struct adv7604_platform_data {
> /* IO register 0x02 */
> unsigned alt_gamma:1;
> unsigned op_656_range:1;
> - unsigned rgb_out:1;
> unsigned alt_data_sat:1;
>
> /* IO register 0x05 */
> unsigned blank_data:1;
> unsigned insert_av_codes:1;
> unsigned replicate_av_codes:1;
> - unsigned invert_cbcr:1;
>
> /* IO register 0x06 */
> unsigned inv_vs_pol:1;
>
--
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: [REVIEWv2 PATCH 24/34] v4l2-ctrls/videodev2.h: add u8 and u16 types.
On 02/12/14 14:40, Ricardo Ribalda Delgado wrote:
> Hello Hans
>
>
>
> On Wed, Feb 12, 2014 at 2:26 PM, Hans Verkuil wrote:
>> On 02/12/14 14:13, Ricardo Ribalda Delgado wrote:
>>> Hello Hans
>>>
>>> Thanks for you promptly response
>>>
>>> On Wed, Feb 12, 2014 at 1:40 PM, Hans Verkuil wrote:
On 02/12/14 13:11, Ricardo Ribalda Delgado wrote:
> Hi Hans
>
> Thanks for your reply
>
> On Wed, Feb 12, 2014 at 12:20 PM, Hans Verkuil wrote:
>> Hi Ricardo,
>>
>> On 02/12/14 11:44, Ricardo Ribalda Delgado wrote:
>>> Hello Hans
>>>
>>> In the case of U8 and U16 data types. Why dont you fill the elem_size
>>> automatically in v4l2_ctrl and request the driver to fill the field?
>>
>> When you create the control the control framework has to know the element
>> size beforehand as it will use that to allocate the memory containing the
>> control's value. The control framework is aware of the 'old' control
>> types
>> and will fill in the elem_size accordingly, but it cannot do that in the
>> general case for these complex types. I guess it could be filled in by
>> the
>> framework for the more common types (U8, U16) but I felt it was more
>> consistent to just require drivers to fill it in manually, rather than
>> have
>> it set for some types but not for others.
>>
>>>
>>> Other option would be not declaring the basic data types (U8, U16,
>>> U32...) and use elem_size. Ie. If type==V4L2_CTRL_COMPLEX_TYPES, then
>>> the type is basic and elem_size is the size of the type. If the type
V4L2_CTRL_COMPLEX_TYPES the type is not basic.
>>
>> You still need to know the type. Applications have to be able to check
>> for
>> the type, the element size by itself doesn't tell you how to interpret
>> the
>> data, you need the type identifier as well.
>
> I think that the driver is setting twice the same info. I see no gain
> in declaring U8, U16 types etc if we still have to set the element
> size. This is why I believe that we should only declare the "structs".
Just to make sure I understand you: for simple types like U8/U16 you want
the control framework to fill in elem_size, for more complex types
(structs)
you want the driver to fill in elem_size?
>>>
>>> I dont like that the type contains the size of the element, and then I
>>> have to provide the size again. (Hungarian notation)
>>>
>>> Instead, I think it is better:
>>>
>>> Defines ONLY this two types for simple types:
>>> V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER and
>>> V4L2_CTRL_COMPLEX_TYPE_UNSIGNED_INTEGER and use elem_size to determine
>>> the size.
>>
>> It sounds great, but it isn't in practice because this will produce awful
>> code like this:
>>
>> switch (type) {
>> case V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER:
>> switch (elem_size) {
>> case 1: // it's a u8!
>> break;
>> case 2: // it's a u16!
>> break;
>> }
>> etc.
>> }
>
> I slightly disagree here. Your proposal will produce this code:
>
>
> case V4L2_CTRL_TYPE_U8:
> ptr.p_u8[idx] = ctrl->default_value;
> break;
> case V4L2_CTRL_TYPE_U16:
> ptr.p_u16[idx] = ctrl->default_value;
> break;
> case V4L2_CTRL_TYPE_U32:
> ptr.p_s32[idx] = ctrl->default_value;
> break;
> case V4L2_CTRL_TYPE_U64:
> ptr.p_s64[idx] = ctrl->default_value;
> break;
> case V4L2_CTRL_TYPE_S8:
> ptr.p_s8[idx] = ctrl->default_value;
> break;
> case V4L2_CTRL_TYPE_S16:
> ptr.p_s16[idx] = ctrl->default_value;
> break;
> case V4L2_CTRL_TYPE_S32:
> ptr.p_s32[idx] = ctrl->default_value;
> break;
> case V4L2_CTRL_TYPE_S64:
> ptr.p_s64[idx] = ctrl->default_value;
> break;
>
> instead of:
>
>
> case V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER:
> case V4L2_CTRL_COMPLEX_TYPE_UNSIGNED_INTEGER:
> memcpy(&ptr.p[idx],&ctrl->default_value,ctrl->elem_size)
Well, no. default_value is a 64-bit value that you can't just memcpy to
a u8 (certainly not on a big-endian system). It's basically why I split
it up, otherwise I would have used the same trick instead of writing it
out for every type.
Regards,
Hans
--
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: [REVIEWv2 PATCH 24/34] v4l2-ctrls/videodev2.h: add u8 and u16 types.
Hello Hans
On Wed, Feb 12, 2014 at 2:26 PM, Hans Verkuil wrote:
> On 02/12/14 14:13, Ricardo Ribalda Delgado wrote:
>> Hello Hans
>>
>> Thanks for you promptly response
>>
>> On Wed, Feb 12, 2014 at 1:40 PM, Hans Verkuil wrote:
>>> On 02/12/14 13:11, Ricardo Ribalda Delgado wrote:
Hi Hans
Thanks for your reply
On Wed, Feb 12, 2014 at 12:20 PM, Hans Verkuil wrote:
> Hi Ricardo,
>
> On 02/12/14 11:44, Ricardo Ribalda Delgado wrote:
>> Hello Hans
>>
>> In the case of U8 and U16 data types. Why dont you fill the elem_size
>> automatically in v4l2_ctrl and request the driver to fill the field?
>
> When you create the control the control framework has to know the element
> size beforehand as it will use that to allocate the memory containing the
> control's value. The control framework is aware of the 'old' control types
> and will fill in the elem_size accordingly, but it cannot do that in the
> general case for these complex types. I guess it could be filled in by the
> framework for the more common types (U8, U16) but I felt it was more
> consistent to just require drivers to fill it in manually, rather than
> have
> it set for some types but not for others.
>
>>
>> Other option would be not declaring the basic data types (U8, U16,
>> U32...) and use elem_size. Ie. If type==V4L2_CTRL_COMPLEX_TYPES, then
>> the type is basic and elem_size is the size of the type. If the type
>>> V4L2_CTRL_COMPLEX_TYPES the type is not basic.
>
> You still need to know the type. Applications have to be able to check for
> the type, the element size by itself doesn't tell you how to interpret the
> data, you need the type identifier as well.
I think that the driver is setting twice the same info. I see no gain
in declaring U8, U16 types etc if we still have to set the element
size. This is why I believe that we should only declare the "structs".
>>>
>>> Just to make sure I understand you: for simple types like U8/U16 you want
>>> the control framework to fill in elem_size, for more complex types (structs)
>>> you want the driver to fill in elem_size?
>>
>> I dont like that the type contains the size of the element, and then I
>> have to provide the size again. (Hungarian notation)
>>
>> Instead, I think it is better:
>>
>> Defines ONLY this two types for simple types:
>> V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER and
>> V4L2_CTRL_COMPLEX_TYPE_UNSIGNED_INTEGER and use elem_size to determine
>> the size.
>
> It sounds great, but it isn't in practice because this will produce awful
> code like this:
>
> switch (type) {
> case V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER:
> switch (elem_size) {
> case 1: // it's a u8!
> break;
> case 2: // it's a u16!
> break;
> }
> etc.
> }
I slightly disagree here. Your proposal will produce this code:
case V4L2_CTRL_TYPE_U8:
ptr.p_u8[idx] = ctrl->default_value;
break;
case V4L2_CTRL_TYPE_U16:
ptr.p_u16[idx] = ctrl->default_value;
break;
case V4L2_CTRL_TYPE_U32:
ptr.p_s32[idx] = ctrl->default_value;
break;
case V4L2_CTRL_TYPE_U64:
ptr.p_s64[idx] = ctrl->default_value;
break;
case V4L2_CTRL_TYPE_S8:
ptr.p_s8[idx] = ctrl->default_value;
break;
case V4L2_CTRL_TYPE_S16:
ptr.p_s16[idx] = ctrl->default_value;
break;
case V4L2_CTRL_TYPE_S32:
ptr.p_s32[idx] = ctrl->default_value;
break;
case V4L2_CTRL_TYPE_S64:
ptr.p_s64[idx] = ctrl->default_value;
break;
instead of:
case V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER:
case V4L2_CTRL_COMPLEX_TYPE_UNSIGNED_INTEGER:
memcpy(&ptr.p[idx],&ctrl->default_value,ctrl->elem_size)
Anyway, this is just a opinion, technically, both solutions are fine,
and yours is already implemented and reviewed.
Regards!
>
> It makes for very awkward code, both in the kernel and in applications.
>
>> And then one define per "structured types" ie:
>> V4L2_CTRL_COMPLEX_TYPE_POINT V4L2_CTRL_COMPLEX_TYPE_IRRATIONAL.. with
>> elem_size determining the size.
>>
>> But if you dont like that idea, as second preference then I think
>> elem_size should be filled by the subsystem for simple types.
>
> I think having the framework fill in elem_size for the basic types such
> as u8 and u16 does make sense. These are already handled by the standard
> number validators, so we should probably have the elem_size set as well.
>
> Regards,
>
> Hans
>
>>
>>
>> Thanks!
>>>
what about something like: V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER +
size, V4L2_CTRL_COMPLEX_TYPES_UNSIGNED_INTEGER + size instead of
V4L2_CTRL_COMPLEX_TYPES_U8, V4L2_CTRL_COMPLEX_TYPES_U16,
V4L2_CTRL_COMPLEX_TYPES_U32, V4L2_CTRL_COMPLEX_TYPES_S8
Btw, I am trying to implement a dead pixel control on the top of you
api. Shall I wait until you patchset is merged or shall I send the
patches right away?
>>>
>>> You're free to experiment, but I am not going to ask Mauro to p
Re: [PATCH v2] [media] of: move graph helpers from drivers/media/v4l2-core to drivers/media
Em Wed, 12 Feb 2014 10:11:54 +0100 Philipp Zabel escreveu: > Hi Mauro, > > Am Mittwoch, den 12.02.2014, 06:53 +0900 schrieb Mauro Carvalho Chehab: > [...] > > > diff --git a/include/media/of_graph.h b/include/media/of_graph.h > > > new file mode 100644 > > > index 000..3bbeb60 > > > --- /dev/null > > > +++ b/include/media/of_graph.h > > > @@ -0,0 +1,46 @@ > > > +/* > > > + * OF graph binding parsing helpers > > > + * > > > + * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd. > > > + * Author: Sylwester Nawrocki > > > + * > > > + * Copyright (C) 2012 Renesas Electronics Corp. > > > + * Author: Guennadi Liakhovetski > > > + * > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of version 2 of the GNU General Public License as > > > + * published by the Free Software Foundation. > > > + */ > > > +#ifndef __LINUX_OF_GRAPH_H > > > +#define __LINUX_OF_GRAPH_H > > > + > > > +#ifdef CONFIG_OF > > > > As a matter of consistency, it would be better to test here for > > CONFIG_OF_GRAPH instead, to reflect the same symbol that enables such > > functions as used on Kconfig/Makefile. > > Maybe I'm trying to be too clever for my own good, but my reasoning was > as follows: > > Suppose I newly use the of_graph_ helpers in a subsystem that does not > yet select OF_GRAPH. In that case I'd rather get linking errors earlier > rather than stubbed out functions that silently fail to parse the DT > later. I see your point, but, imagining that someone pushed a patch using those symbols upstream, that would break compilation and git bisection, with will hurt everyone, and not only the very few of us that would actually need the OF_GRAPH symbol for an specific driver. Also, such push would mean that someone forgot to do his homework and to test if the committed functionality is actually working. So, it seems more fair that the one that did the mistake will be the one that will suffer the consequences for his errors instead of applying a penalty to everybody's else ;) > Since there is > config VIDEO_DEV > select OF_GRAPH if OF > already and the same should be added for other users of device tree > graphs, I think stubbing out the functions only if OF is disabled should > be enough. Well, if you want to be sure that the graph will always be there if OF, then you could do, instead: config OF_GRAPH bool default OF (that would actually make OF_GRAPH just an alias to OF - so we could just use OF instead). In any case, I think that we should use the same config name at Makefile, Kconfig and of_graph.h (either OF_GRAPH or just OF). 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
Re: [REVIEWv2 PATCH 24/34] v4l2-ctrls/videodev2.h: add u8 and u16 types.
On 02/12/14 14:13, Ricardo Ribalda Delgado wrote:
> Hello Hans
>
> Thanks for you promptly response
>
> On Wed, Feb 12, 2014 at 1:40 PM, Hans Verkuil wrote:
>> On 02/12/14 13:11, Ricardo Ribalda Delgado wrote:
>>> Hi Hans
>>>
>>> Thanks for your reply
>>>
>>> On Wed, Feb 12, 2014 at 12:20 PM, Hans Verkuil wrote:
Hi Ricardo,
On 02/12/14 11:44, Ricardo Ribalda Delgado wrote:
> Hello Hans
>
> In the case of U8 and U16 data types. Why dont you fill the elem_size
> automatically in v4l2_ctrl and request the driver to fill the field?
When you create the control the control framework has to know the element
size beforehand as it will use that to allocate the memory containing the
control's value. The control framework is aware of the 'old' control types
and will fill in the elem_size accordingly, but it cannot do that in the
general case for these complex types. I guess it could be filled in by the
framework for the more common types (U8, U16) but I felt it was more
consistent to just require drivers to fill it in manually, rather than have
it set for some types but not for others.
>
> Other option would be not declaring the basic data types (U8, U16,
> U32...) and use elem_size. Ie. If type==V4L2_CTRL_COMPLEX_TYPES, then
> the type is basic and elem_size is the size of the type. If the type
>> V4L2_CTRL_COMPLEX_TYPES the type is not basic.
You still need to know the type. Applications have to be able to check for
the type, the element size by itself doesn't tell you how to interpret the
data, you need the type identifier as well.
>>>
>>> I think that the driver is setting twice the same info. I see no gain
>>> in declaring U8, U16 types etc if we still have to set the element
>>> size. This is why I believe that we should only declare the "structs".
>>
>> Just to make sure I understand you: for simple types like U8/U16 you want
>> the control framework to fill in elem_size, for more complex types (structs)
>> you want the driver to fill in elem_size?
>
> I dont like that the type contains the size of the element, and then I
> have to provide the size again. (Hungarian notation)
>
> Instead, I think it is better:
>
> Defines ONLY this two types for simple types:
> V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER and
> V4L2_CTRL_COMPLEX_TYPE_UNSIGNED_INTEGER and use elem_size to determine
> the size.
It sounds great, but it isn't in practice because this will produce awful
code like this:
switch (type) {
case V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER:
switch (elem_size) {
case 1: // it's a u8!
break;
case 2: // it's a u16!
break;
}
etc.
}
It makes for very awkward code, both in the kernel and in applications.
> And then one define per "structured types" ie:
> V4L2_CTRL_COMPLEX_TYPE_POINT V4L2_CTRL_COMPLEX_TYPE_IRRATIONAL.. with
> elem_size determining the size.
>
> But if you dont like that idea, as second preference then I think
> elem_size should be filled by the subsystem for simple types.
I think having the framework fill in elem_size for the basic types such
as u8 and u16 does make sense. These are already handled by the standard
number validators, so we should probably have the elem_size set as well.
Regards,
Hans
>
>
> Thanks!
>>
>>> what about something like: V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER +
>>> size, V4L2_CTRL_COMPLEX_TYPES_UNSIGNED_INTEGER + size instead of
>>> V4L2_CTRL_COMPLEX_TYPES_U8, V4L2_CTRL_COMPLEX_TYPES_U16,
>>> V4L2_CTRL_COMPLEX_TYPES_U32, V4L2_CTRL_COMPLEX_TYPES_S8
>>>
>>> Btw, I am trying to implement a dead pixel control on the top of you
>>> api. Shall I wait until you patchset is merged or shall I send the
>>> patches right away?
>>
>> You're free to experiment, but I am not going to ask Mauro to pull additional
>> patches as long as this initial patch set isn't merged.
>>
>> Regards,
>>
>> Hans
>
>
>
>
>
--
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: [REVIEWv2 PATCH 24/34] v4l2-ctrls/videodev2.h: add u8 and u16 types.
Hello Hans Thanks for you promptly response On Wed, Feb 12, 2014 at 1:40 PM, Hans Verkuil wrote: > On 02/12/14 13:11, Ricardo Ribalda Delgado wrote: >> Hi Hans >> >> Thanks for your reply >> >> On Wed, Feb 12, 2014 at 12:20 PM, Hans Verkuil wrote: >>> Hi Ricardo, >>> >>> On 02/12/14 11:44, Ricardo Ribalda Delgado wrote: Hello Hans In the case of U8 and U16 data types. Why dont you fill the elem_size automatically in v4l2_ctrl and request the driver to fill the field? >>> >>> When you create the control the control framework has to know the element >>> size beforehand as it will use that to allocate the memory containing the >>> control's value. The control framework is aware of the 'old' control types >>> and will fill in the elem_size accordingly, but it cannot do that in the >>> general case for these complex types. I guess it could be filled in by the >>> framework for the more common types (U8, U16) but I felt it was more >>> consistent to just require drivers to fill it in manually, rather than have >>> it set for some types but not for others. >>> Other option would be not declaring the basic data types (U8, U16, U32...) and use elem_size. Ie. If type==V4L2_CTRL_COMPLEX_TYPES, then the type is basic and elem_size is the size of the type. If the type > V4L2_CTRL_COMPLEX_TYPES the type is not basic. >>> >>> You still need to know the type. Applications have to be able to check for >>> the type, the element size by itself doesn't tell you how to interpret the >>> data, you need the type identifier as well. >> >> I think that the driver is setting twice the same info. I see no gain >> in declaring U8, U16 types etc if we still have to set the element >> size. This is why I believe that we should only declare the "structs". > > Just to make sure I understand you: for simple types like U8/U16 you want > the control framework to fill in elem_size, for more complex types (structs) > you want the driver to fill in elem_size? I dont like that the type contains the size of the element, and then I have to provide the size again. (Hungarian notation) Instead, I think it is better: Defines ONLY this two types for simple types: V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER and V4L2_CTRL_COMPLEX_TYPE_UNSIGNED_INTEGER and use elem_size to determine the size. And then one define per "structured types" ie: V4L2_CTRL_COMPLEX_TYPE_POINT V4L2_CTRL_COMPLEX_TYPE_IRRATIONAL.. with elem_size determining the size. But if you dont like that idea, as second preference then I think elem_size should be filled by the subsystem for simple types. Thanks! > >> what about something like: V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER + >> size, V4L2_CTRL_COMPLEX_TYPES_UNSIGNED_INTEGER + size instead of >> V4L2_CTRL_COMPLEX_TYPES_U8, V4L2_CTRL_COMPLEX_TYPES_U16, >> V4L2_CTRL_COMPLEX_TYPES_U32, V4L2_CTRL_COMPLEX_TYPES_S8 >> >> Btw, I am trying to implement a dead pixel control on the top of you >> api. Shall I wait until you patchset is merged or shall I send the >> patches right away? > > You're free to experiment, but I am not going to ask Mauro to pull additional > patches as long as this initial patch set isn't merged. > > Regards, > > Hans -- Ricardo Ribalda -- 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: [REVIEWv2 PATCH 08/34] v4l2-ctrls: create type_ops.
Agree
I wont complain until I have some performance measurement :)
Thanks!
On Wed, Feb 12, 2014 at 1:31 PM, Hans Verkuil wrote:
> On 02/12/14 13:03, Ricardo Ribalda Delgado wrote:
>> Hello Hans
>>
>> My usercase is different gain and offset per pixel (non uniformity
>> correction). My sensor is now 4 Mpx, and probably 12 Mpx soon :)
>
> The hardware actually supports per pixel gain and offset?! Wow.
>
> Anyway, I am not opposed to making changes in this area (although I want
> this to be merged first), provided you also do some performance measurements.
>
> I am also assuming that such large matrices only rarely change, so to
> what extent performance improvements have any real-life effect may be
> debatable.
>
> Regards,
>
> Hans
>
>>
>> Right now it is implemented as a video output... but I think that I am
>> exploiting an API :)
>>
>> Regards!
>>
>> On Wed, Feb 12, 2014 at 12:36 PM, Hans Verkuil wrote:
>>> On 02/12/14 11:55, Ricardo Ribalda Delgado wrote:
Hello Hans
If we have a matrix control with a huge number of elements (ie, the
number of pixels), a control validation will imply over ahuge number
of function calls.
In the case of the matrix you are always validating/init all the
values of of the array. Why dont we modify the type_ops so they
operate on the whole matrix (remove the idx from the function).
>>>
>>> I actually considered that, but rejected it (for now) because I want
>>> to see a realistic use-case first. If you get such large matrices, then
>>> you have to ask yourself whether the control framework is the right
>>> approach anyway, and if it is, whether you want to do these validations
>>> in the first place.
>>>
>>> Also note that validations typically will still have to be done on a
>>> per-element basis (so pushing down the loop over all elements into the
>>> validation function will have little effect) and ditto for the more
>>> complex comparisons. Only if you are able to do something like memcmp
>>> to compare two matrices will it become beneficial to optimize this.
>>>
>>> I see this as a possible future performance enhancement and before doing
>>> that I need to see a good use-case and also see some performence
>>> measurements.
>>>
Anyway we should modify
for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++)
ctrl_changed |= !ctrl->type_ops->equal(ctrl, idx,
ctrl->stores[0], ctrl->new);
to
for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++)
if (!ctrl->type_ops->equal(ctrl, idx, ctrl->stores[0],
ctrl->new))
break
if (idx==ctrl->rows * ctrl->col)
ctrol_changed=true;
Saving us some calls when we already know that the control has changed.
>>>
>>> Oops! Absolutely, thanks for pointing this out. That's a left-over from
>>> older code.
>>>
>>> I'll post a patch for this.
>>>
>>> Regards,
>>>
>>> Hans
>>>
Thanks!
ps: consider all my code pseudocode :P
On Mon, Feb 10, 2014 at 9:46 AM, Hans Verkuil wrote:
> From: Hans Verkuil
>
> Since complex controls can have non-standard types we need to be able to
> do
> type-specific checks etc. In order to make that easy type operations are
> added.
> There are four operations:
>
> - equal: check if two values are equal
> - init: initialize a value
> - log: log the value
> - validate: validate a new value
>
> This patch uses the v4l2_ctrl_ptr union for the first time.
>
> Signed-off-by: Hans Verkuil
> Reviewed-by: Sylwester Nawrocki
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 267
> ++-
> include/media/v4l2-ctrls.h | 21 +++
> 2 files changed, 190 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 67e5d1e..988a2bd8 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1132,6 +1132,149 @@ static void send_event(struct v4l2_fh *fh, struct
> v4l2_ctrl *ctrl, u32 changes)
> v4l2_event_queue_fh(sev->fh, &ev);
> }
>
> +static bool std_equal(const struct v4l2_ctrl *ctrl,
> + union v4l2_ctrl_ptr ptr1,
> + union v4l2_ctrl_ptr ptr2)
> +{
> + switch (ctrl->type) {
> + case V4L2_CTRL_TYPE_BUTTON:
> + return false;
> + case V4L2_CTRL_TYPE_STRING:
> + /* strings are always 0-terminated */
> + return !strcmp(ptr1.p_char, ptr2.p_char);
> + case V4L2_CTRL_TYPE_INTEGER64:
> + return *ptr1.p_s64 == *ptr2.p_s64;
> + default:
> + if (ctrl->is_ptr)
> + retur
Re: [PATCH 25/47] v4l: subdev: Remove deprecated video-level DV timings operations
On 02/05/14 17:42, Laurent Pinchart wrote:
> The video enum_dv_timings and dv_timings_cap operations are deprecated
> and unused. Remove them.
FYI: after this the adv7604 fails to compile since it is still using the
video ops. You should move the patches that convert the adv7604 before
this patch.
Regards,
Hans
>
> Signed-off-by: Laurent Pinchart
> ---
> include/media/v4l2-subdev.h | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 2c7355a..ddea899 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -326,12 +326,8 @@ struct v4l2_subdev_video_ops {
> struct v4l2_dv_timings *timings);
> int (*g_dv_timings)(struct v4l2_subdev *sd,
> struct v4l2_dv_timings *timings);
> - int (*enum_dv_timings)(struct v4l2_subdev *sd,
> - struct v4l2_enum_dv_timings *timings);
> int (*query_dv_timings)(struct v4l2_subdev *sd,
> struct v4l2_dv_timings *timings);
> - int (*dv_timings_cap)(struct v4l2_subdev *sd,
> - struct v4l2_dv_timings_cap *cap);
> int (*enum_mbus_fmt)(struct v4l2_subdev *sd, unsigned int index,
>enum v4l2_mbus_pixelcode *code);
> int (*enum_mbus_fsizes)(struct v4l2_subdev *sd,
>
--
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: [REVIEWv2 PATCH 24/34] v4l2-ctrls/videodev2.h: add u8 and u16 types.
On 02/12/14 13:11, Ricardo Ribalda Delgado wrote: > Hi Hans > > Thanks for your reply > > On Wed, Feb 12, 2014 at 12:20 PM, Hans Verkuil wrote: >> Hi Ricardo, >> >> On 02/12/14 11:44, Ricardo Ribalda Delgado wrote: >>> Hello Hans >>> >>> In the case of U8 and U16 data types. Why dont you fill the elem_size >>> automatically in v4l2_ctrl and request the driver to fill the field? >> >> When you create the control the control framework has to know the element >> size beforehand as it will use that to allocate the memory containing the >> control's value. The control framework is aware of the 'old' control types >> and will fill in the elem_size accordingly, but it cannot do that in the >> general case for these complex types. I guess it could be filled in by the >> framework for the more common types (U8, U16) but I felt it was more >> consistent to just require drivers to fill it in manually, rather than have >> it set for some types but not for others. >> >>> >>> Other option would be not declaring the basic data types (U8, U16, >>> U32...) and use elem_size. Ie. If type==V4L2_CTRL_COMPLEX_TYPES, then >>> the type is basic and elem_size is the size of the type. If the type V4L2_CTRL_COMPLEX_TYPES the type is not basic. >> >> You still need to know the type. Applications have to be able to check for >> the type, the element size by itself doesn't tell you how to interpret the >> data, you need the type identifier as well. > > I think that the driver is setting twice the same info. I see no gain > in declaring U8, U16 types etc if we still have to set the element > size. This is why I believe that we should only declare the "structs". Just to make sure I understand you: for simple types like U8/U16 you want the control framework to fill in elem_size, for more complex types (structs) you want the driver to fill in elem_size? > what about something like: V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER + > size, V4L2_CTRL_COMPLEX_TYPES_UNSIGNED_INTEGER + size instead of > V4L2_CTRL_COMPLEX_TYPES_U8, V4L2_CTRL_COMPLEX_TYPES_U16, > V4L2_CTRL_COMPLEX_TYPES_U32, V4L2_CTRL_COMPLEX_TYPES_S8 > > Btw, I am trying to implement a dead pixel control on the top of you > api. Shall I wait until you patchset is merged or shall I send the > patches right away? You're free to experiment, but I am not going to ask Mauro to pull additional patches as long as this initial patch set isn't merged. Regards, Hans -- 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: [REVIEWv2 PATCH 08/34] v4l2-ctrls: create type_ops.
On 02/12/14 13:03, Ricardo Ribalda Delgado wrote:
> Hello Hans
>
> My usercase is different gain and offset per pixel (non uniformity
> correction). My sensor is now 4 Mpx, and probably 12 Mpx soon :)
The hardware actually supports per pixel gain and offset?! Wow.
Anyway, I am not opposed to making changes in this area (although I want
this to be merged first), provided you also do some performance measurements.
I am also assuming that such large matrices only rarely change, so to
what extent performance improvements have any real-life effect may be
debatable.
Regards,
Hans
>
> Right now it is implemented as a video output... but I think that I am
> exploiting an API :)
>
> Regards!
>
> On Wed, Feb 12, 2014 at 12:36 PM, Hans Verkuil wrote:
>> On 02/12/14 11:55, Ricardo Ribalda Delgado wrote:
>>> Hello Hans
>>>
>>> If we have a matrix control with a huge number of elements (ie, the
>>> number of pixels), a control validation will imply over ahuge number
>>> of function calls.
>>>
>>> In the case of the matrix you are always validating/init all the
>>> values of of the array. Why dont we modify the type_ops so they
>>> operate on the whole matrix (remove the idx from the function).
>>
>> I actually considered that, but rejected it (for now) because I want
>> to see a realistic use-case first. If you get such large matrices, then
>> you have to ask yourself whether the control framework is the right
>> approach anyway, and if it is, whether you want to do these validations
>> in the first place.
>>
>> Also note that validations typically will still have to be done on a
>> per-element basis (so pushing down the loop over all elements into the
>> validation function will have little effect) and ditto for the more
>> complex comparisons. Only if you are able to do something like memcmp
>> to compare two matrices will it become beneficial to optimize this.
>>
>> I see this as a possible future performance enhancement and before doing
>> that I need to see a good use-case and also see some performence
>> measurements.
>>
>>> Anyway we should modify
>>>
>>> for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++)
>>>ctrl_changed |= !ctrl->type_ops->equal(ctrl, idx,
>>> ctrl->stores[0], ctrl->new);
>>>
>>> to
>>>
>>> for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++)
>>>if (!ctrl->type_ops->equal(ctrl, idx, ctrl->stores[0],
>>> ctrl->new))
>>> break
>>>
>>> if (idx==ctrl->rows * ctrl->col)
>>> ctrol_changed=true;
>>>
>>> Saving us some calls when we already know that the control has changed.
>>
>> Oops! Absolutely, thanks for pointing this out. That's a left-over from
>> older code.
>>
>> I'll post a patch for this.
>>
>> Regards,
>>
>> Hans
>>
>>>
>>>
>>> Thanks!
>>>
>>>
>>> ps: consider all my code pseudocode :P
>>>
>>>
>>> On Mon, Feb 10, 2014 at 9:46 AM, Hans Verkuil wrote:
From: Hans Verkuil
Since complex controls can have non-standard types we need to be able to do
type-specific checks etc. In order to make that easy type operations are
added.
There are four operations:
- equal: check if two values are equal
- init: initialize a value
- log: log the value
- validate: validate a new value
This patch uses the v4l2_ctrl_ptr union for the first time.
Signed-off-by: Hans Verkuil
Reviewed-by: Sylwester Nawrocki
---
drivers/media/v4l2-core/v4l2-ctrls.c | 267
++-
include/media/v4l2-ctrls.h | 21 +++
2 files changed, 190 insertions(+), 98 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 67e5d1e..988a2bd8 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1132,6 +1132,149 @@ static void send_event(struct v4l2_fh *fh, struct
v4l2_ctrl *ctrl, u32 changes)
v4l2_event_queue_fh(sev->fh, &ev);
}
+static bool std_equal(const struct v4l2_ctrl *ctrl,
+ union v4l2_ctrl_ptr ptr1,
+ union v4l2_ctrl_ptr ptr2)
+{
+ switch (ctrl->type) {
+ case V4L2_CTRL_TYPE_BUTTON:
+ return false;
+ case V4L2_CTRL_TYPE_STRING:
+ /* strings are always 0-terminated */
+ return !strcmp(ptr1.p_char, ptr2.p_char);
+ case V4L2_CTRL_TYPE_INTEGER64:
+ return *ptr1.p_s64 == *ptr2.p_s64;
+ default:
+ if (ctrl->is_ptr)
+ return !memcmp(ptr1.p, ptr2.p, ctrl->elem_size);
+ return *ptr1.p_s32 == *ptr2.p_s32;
+ }
+}
+
+static void std_init(const struct v4l2_ctrl *ctrl,
+union v4l2_ctrl_ptr ptr)
+{
+ switch (ctrl->type) {
>
Re: [REVIEWv2 PATCH 24/34] v4l2-ctrls/videodev2.h: add u8 and u16 types.
Hi Hans
Thanks for your reply
On Wed, Feb 12, 2014 at 12:20 PM, Hans Verkuil wrote:
> Hi Ricardo,
>
> On 02/12/14 11:44, Ricardo Ribalda Delgado wrote:
>> Hello Hans
>>
>> In the case of U8 and U16 data types. Why dont you fill the elem_size
>> automatically in v4l2_ctrl and request the driver to fill the field?
>
> When you create the control the control framework has to know the element
> size beforehand as it will use that to allocate the memory containing the
> control's value. The control framework is aware of the 'old' control types
> and will fill in the elem_size accordingly, but it cannot do that in the
> general case for these complex types. I guess it could be filled in by the
> framework for the more common types (U8, U16) but I felt it was more
> consistent to just require drivers to fill it in manually, rather than have
> it set for some types but not for others.
>
>>
>> Other option would be not declaring the basic data types (U8, U16,
>> U32...) and use elem_size. Ie. If type==V4L2_CTRL_COMPLEX_TYPES, then
>> the type is basic and elem_size is the size of the type. If the type
>>> V4L2_CTRL_COMPLEX_TYPES the type is not basic.
>
> You still need to know the type. Applications have to be able to check for
> the type, the element size by itself doesn't tell you how to interpret the
> data, you need the type identifier as well.
I think that the driver is setting twice the same info. I see no gain
in declaring U8, U16 types etc if we still have to set the element
size. This is why I believe that we should only declare the "structs".
what about something like: V4L2_CTRL_COMPLEX_TYPE_SIGNED_INTEGER +
size, V4L2_CTRL_COMPLEX_TYPES_UNSIGNED_INTEGER + size instead of
V4L2_CTRL_COMPLEX_TYPES_U8, V4L2_CTRL_COMPLEX_TYPES_U16,
V4L2_CTRL_COMPLEX_TYPES_U32, V4L2_CTRL_COMPLEX_TYPES_S8
Btw, I am trying to implement a dead pixel control on the top of you
api. Shall I wait until you patchset is merged or shall I send the
patches right away?
Thanks
>
> Regards,
>
> Hans
>
>>
>> Thanks!
>>
>> On Mon, Feb 10, 2014 at 9:46 AM, Hans Verkuil wrote:
>>> From: Hans Verkuil
>>>
>>> These are needed by the upcoming patches for the motion detection
>>> matrices.
>>>
>>> Signed-off-by: Hans Verkuil
>>> ---
>>> drivers/media/v4l2-core/v4l2-ctrls.c | 24
>>> include/media/v4l2-ctrls.h | 4
>>> include/uapi/linux/videodev2.h | 4
>>> 3 files changed, 32 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> index c81ebcf..0b200dd 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> @@ -1145,6 +1145,10 @@ static bool std_equal(const struct v4l2_ctrl *ctrl,
>>> u32 idx,
>>> return !strcmp(ptr1.p_char + idx, ptr2.p_char + idx);
>>> case V4L2_CTRL_TYPE_INTEGER64:
>>> return ptr1.p_s64[idx] == ptr2.p_s64[idx];
>>> + case V4L2_CTRL_TYPE_U8:
>>> + return ptr1.p_u8[idx] == ptr2.p_u8[idx];
>>> + case V4L2_CTRL_TYPE_U16:
>>> + return ptr1.p_u16[idx] == ptr2.p_u16[idx];
>>> default:
>>> if (ctrl->is_int)
>>> return ptr1.p_s32[idx] == ptr2.p_s32[idx];
>>> @@ -1172,6 +1176,12 @@ static void std_init(const struct v4l2_ctrl *ctrl,
>>> u32 idx,
>>> case V4L2_CTRL_TYPE_BOOLEAN:
>>> ptr.p_s32[idx] = ctrl->default_value;
>>> break;
>>> + case V4L2_CTRL_TYPE_U8:
>>> + ptr.p_u8[idx] = ctrl->default_value;
>>> + break;
>>> + case V4L2_CTRL_TYPE_U16:
>>> + ptr.p_u16[idx] = ctrl->default_value;
>>> + break;
>>> default:
>>> idx *= ctrl->elem_size;
>>> memset(ptr.p + idx, 0, ctrl->elem_size);
>>> @@ -1208,6 +1218,12 @@ static void std_log(const struct v4l2_ctrl *ctrl)
>>> case V4L2_CTRL_TYPE_STRING:
>>> pr_cont("%s", ptr.p_char);
>>> break;
>>> + case V4L2_CTRL_TYPE_U8:
>>> + pr_cont("%u", (unsigned)*ptr.p_u8);
>>> + break;
>>> + case V4L2_CTRL_TYPE_U16:
>>> + pr_cont("%u", (unsigned)*ptr.p_u16);
>>> + break;
>>> default:
>>> pr_cont("unknown type %d", ctrl->type);
>>> break;
>>> @@ -1238,6 +1254,10 @@ static int std_validate(const struct v4l2_ctrl
>>> *ctrl, u32 idx,
>>> return ROUND_TO_RANGE(ptr.p_s32[idx], u32, ctrl);
>>> case V4L2_CTRL_TYPE_INTEGER64:
>>> return ROUND_TO_RANGE(ptr.p_s64[idx], u64, ctrl);
>>> + case V4L2_CTRL_TYPE_U8:
>>> + return ROUND_TO_RANGE(ptr.p_u8[idx], u8, ctrl);
>>> + case V4L2_CTRL_TYPE_U16:
>>> + return ROUND_TO_RANGE(ptr.p_u16[idx], u16, ctrl);
>>>
>>> case V4L2_CTRL_TYPE_BOOLEAN:
>>> ptr.p_s32[i
Re: [REVIEWv2 PATCH 08/34] v4l2-ctrls: create type_ops.
Hello Hans
My usercase is different gain and offset per pixel (non uniformity
correction). My sensor is now 4 Mpx, and probably 12 Mpx soon :)
Right now it is implemented as a video output... but I think that I am
exploiting an API :)
Regards!
On Wed, Feb 12, 2014 at 12:36 PM, Hans Verkuil wrote:
> On 02/12/14 11:55, Ricardo Ribalda Delgado wrote:
>> Hello Hans
>>
>> If we have a matrix control with a huge number of elements (ie, the
>> number of pixels), a control validation will imply over ahuge number
>> of function calls.
>>
>> In the case of the matrix you are always validating/init all the
>> values of of the array. Why dont we modify the type_ops so they
>> operate on the whole matrix (remove the idx from the function).
>
> I actually considered that, but rejected it (for now) because I want
> to see a realistic use-case first. If you get such large matrices, then
> you have to ask yourself whether the control framework is the right
> approach anyway, and if it is, whether you want to do these validations
> in the first place.
>
> Also note that validations typically will still have to be done on a
> per-element basis (so pushing down the loop over all elements into the
> validation function will have little effect) and ditto for the more
> complex comparisons. Only if you are able to do something like memcmp
> to compare two matrices will it become beneficial to optimize this.
>
> I see this as a possible future performance enhancement and before doing
> that I need to see a good use-case and also see some performence measurements.
>
>> Anyway we should modify
>>
>> for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++)
>>ctrl_changed |= !ctrl->type_ops->equal(ctrl, idx,
>> ctrl->stores[0], ctrl->new);
>>
>> to
>>
>> for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++)
>>if (!ctrl->type_ops->equal(ctrl, idx, ctrl->stores[0],
>> ctrl->new))
>> break
>>
>> if (idx==ctrl->rows * ctrl->col)
>> ctrol_changed=true;
>>
>> Saving us some calls when we already know that the control has changed.
>
> Oops! Absolutely, thanks for pointing this out. That's a left-over from
> older code.
>
> I'll post a patch for this.
>
> Regards,
>
> Hans
>
>>
>>
>> Thanks!
>>
>>
>> ps: consider all my code pseudocode :P
>>
>>
>> On Mon, Feb 10, 2014 at 9:46 AM, Hans Verkuil wrote:
>>> From: Hans Verkuil
>>>
>>> Since complex controls can have non-standard types we need to be able to do
>>> type-specific checks etc. In order to make that easy type operations are
>>> added.
>>> There are four operations:
>>>
>>> - equal: check if two values are equal
>>> - init: initialize a value
>>> - log: log the value
>>> - validate: validate a new value
>>>
>>> This patch uses the v4l2_ctrl_ptr union for the first time.
>>>
>>> Signed-off-by: Hans Verkuil
>>> Reviewed-by: Sylwester Nawrocki
>>> ---
>>> drivers/media/v4l2-core/v4l2-ctrls.c | 267
>>> ++-
>>> include/media/v4l2-ctrls.h | 21 +++
>>> 2 files changed, 190 insertions(+), 98 deletions(-)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> index 67e5d1e..988a2bd8 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> @@ -1132,6 +1132,149 @@ static void send_event(struct v4l2_fh *fh, struct
>>> v4l2_ctrl *ctrl, u32 changes)
>>> v4l2_event_queue_fh(sev->fh, &ev);
>>> }
>>>
>>> +static bool std_equal(const struct v4l2_ctrl *ctrl,
>>> + union v4l2_ctrl_ptr ptr1,
>>> + union v4l2_ctrl_ptr ptr2)
>>> +{
>>> + switch (ctrl->type) {
>>> + case V4L2_CTRL_TYPE_BUTTON:
>>> + return false;
>>> + case V4L2_CTRL_TYPE_STRING:
>>> + /* strings are always 0-terminated */
>>> + return !strcmp(ptr1.p_char, ptr2.p_char);
>>> + case V4L2_CTRL_TYPE_INTEGER64:
>>> + return *ptr1.p_s64 == *ptr2.p_s64;
>>> + default:
>>> + if (ctrl->is_ptr)
>>> + return !memcmp(ptr1.p, ptr2.p, ctrl->elem_size);
>>> + return *ptr1.p_s32 == *ptr2.p_s32;
>>> + }
>>> +}
>>> +
>>> +static void std_init(const struct v4l2_ctrl *ctrl,
>>> +union v4l2_ctrl_ptr ptr)
>>> +{
>>> + switch (ctrl->type) {
>>> + case V4L2_CTRL_TYPE_STRING:
>>> + memset(ptr.p_char, ' ', ctrl->minimum);
>>> + ptr.p_char[ctrl->minimum] = '\0';
>>> + break;
>>> + case V4L2_CTRL_TYPE_INTEGER64:
>>> + *ptr.p_s64 = ctrl->default_value;
>>> + break;
>>> + case V4L2_CTRL_TYPE_INTEGER:
>>> + case V4L2_CTRL_TYPE_INTEGER_MENU:
>>> + case V4L2_CTRL_TYPE_MENU:
>>> + case V4L2_CTRL_TYPE_BITMASK:
>>> + case V4L2_CTRL_TYPE_BOOLEAN:
>>> + *ptr.p_s32 = ctrl->default_value;
>>> + br
[REVIEWv2 PATCH 36/34] v4l2-ctrls: break off loop on first changed element
There is no need to loop over all elements of a matrix checking if there are changes. Just stop at the first changed element. Signed-off-by: Hans Verkuil Reported-by: Ricardo Ribalda Delgado --- drivers/media/v4l2-core/v4l2-ctrls.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index e8e2caa..23febc4 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -1502,8 +1502,9 @@ static int cluster_changed(struct v4l2_ctrl *master) if (ctrl == NULL) continue; - for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++) - ctrl_changed |= !ctrl->type_ops->equal(ctrl, idx, + for (idx = 0; !ctrl_changed && + idx < ctrl->rows * ctrl->cols; idx++) + ctrl_changed = !ctrl->type_ops->equal(ctrl, idx, ctrl->stores[0], ctrl->new); ctrl->has_changed = ctrl_changed; changed |= ctrl->has_changed; -- 1.8.5.2 -- 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: [REVIEWv2 PATCH 08/34] v4l2-ctrls: create type_ops.
On 02/12/14 11:55, Ricardo Ribalda Delgado wrote:
> Hello Hans
>
> If we have a matrix control with a huge number of elements (ie, the
> number of pixels), a control validation will imply over ahuge number
> of function calls.
>
> In the case of the matrix you are always validating/init all the
> values of of the array. Why dont we modify the type_ops so they
> operate on the whole matrix (remove the idx from the function).
I actually considered that, but rejected it (for now) because I want
to see a realistic use-case first. If you get such large matrices, then
you have to ask yourself whether the control framework is the right
approach anyway, and if it is, whether you want to do these validations
in the first place.
Also note that validations typically will still have to be done on a
per-element basis (so pushing down the loop over all elements into the
validation function will have little effect) and ditto for the more
complex comparisons. Only if you are able to do something like memcmp
to compare two matrices will it become beneficial to optimize this.
I see this as a possible future performance enhancement and before doing
that I need to see a good use-case and also see some performence measurements.
> Anyway we should modify
>
> for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++)
>ctrl_changed |= !ctrl->type_ops->equal(ctrl, idx,
> ctrl->stores[0], ctrl->new);
>
> to
>
> for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++)
>if (!ctrl->type_ops->equal(ctrl, idx, ctrl->stores[0],
> ctrl->new))
> break
>
> if (idx==ctrl->rows * ctrl->col)
> ctrol_changed=true;
>
> Saving us some calls when we already know that the control has changed.
Oops! Absolutely, thanks for pointing this out. That's a left-over from
older code.
I'll post a patch for this.
Regards,
Hans
>
>
> Thanks!
>
>
> ps: consider all my code pseudocode :P
>
>
> On Mon, Feb 10, 2014 at 9:46 AM, Hans Verkuil wrote:
>> From: Hans Verkuil
>>
>> Since complex controls can have non-standard types we need to be able to do
>> type-specific checks etc. In order to make that easy type operations are
>> added.
>> There are four operations:
>>
>> - equal: check if two values are equal
>> - init: initialize a value
>> - log: log the value
>> - validate: validate a new value
>>
>> This patch uses the v4l2_ctrl_ptr union for the first time.
>>
>> Signed-off-by: Hans Verkuil
>> Reviewed-by: Sylwester Nawrocki
>> ---
>> drivers/media/v4l2-core/v4l2-ctrls.c | 267
>> ++-
>> include/media/v4l2-ctrls.h | 21 +++
>> 2 files changed, 190 insertions(+), 98 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index 67e5d1e..988a2bd8 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -1132,6 +1132,149 @@ static void send_event(struct v4l2_fh *fh, struct
>> v4l2_ctrl *ctrl, u32 changes)
>> v4l2_event_queue_fh(sev->fh, &ev);
>> }
>>
>> +static bool std_equal(const struct v4l2_ctrl *ctrl,
>> + union v4l2_ctrl_ptr ptr1,
>> + union v4l2_ctrl_ptr ptr2)
>> +{
>> + switch (ctrl->type) {
>> + case V4L2_CTRL_TYPE_BUTTON:
>> + return false;
>> + case V4L2_CTRL_TYPE_STRING:
>> + /* strings are always 0-terminated */
>> + return !strcmp(ptr1.p_char, ptr2.p_char);
>> + case V4L2_CTRL_TYPE_INTEGER64:
>> + return *ptr1.p_s64 == *ptr2.p_s64;
>> + default:
>> + if (ctrl->is_ptr)
>> + return !memcmp(ptr1.p, ptr2.p, ctrl->elem_size);
>> + return *ptr1.p_s32 == *ptr2.p_s32;
>> + }
>> +}
>> +
>> +static void std_init(const struct v4l2_ctrl *ctrl,
>> +union v4l2_ctrl_ptr ptr)
>> +{
>> + switch (ctrl->type) {
>> + case V4L2_CTRL_TYPE_STRING:
>> + memset(ptr.p_char, ' ', ctrl->minimum);
>> + ptr.p_char[ctrl->minimum] = '\0';
>> + break;
>> + case V4L2_CTRL_TYPE_INTEGER64:
>> + *ptr.p_s64 = ctrl->default_value;
>> + break;
>> + case V4L2_CTRL_TYPE_INTEGER:
>> + case V4L2_CTRL_TYPE_INTEGER_MENU:
>> + case V4L2_CTRL_TYPE_MENU:
>> + case V4L2_CTRL_TYPE_BITMASK:
>> + case V4L2_CTRL_TYPE_BOOLEAN:
>> + *ptr.p_s32 = ctrl->default_value;
>> + break;
>> + default:
>> + break;
>> + }
>> +}
>> +
>> +static void std_log(const struct v4l2_ctrl *ctrl)
>> +{
>> + union v4l2_ctrl_ptr ptr = ctrl->stores[0];
>> +
>> + switch (ctrl->type) {
>> + case V4L2_CTRL_TYPE_INTEGER:
>> + pr_cont("%d", *ptr.p_s32);
>> + break;
>> + case V4L2_CTRL_TYPE_BOOLEAN:
>> + pr_cont("%s", *ptr.p_s32 ? "true" : "fals
Re: [REVIEWv2 PATCH 24/34] v4l2-ctrls/videodev2.h: add u8 and u16 types.
Hi Ricardo,
On 02/12/14 11:44, Ricardo Ribalda Delgado wrote:
> Hello Hans
>
> In the case of U8 and U16 data types. Why dont you fill the elem_size
> automatically in v4l2_ctrl and request the driver to fill the field?
When you create the control the control framework has to know the element
size beforehand as it will use that to allocate the memory containing the
control's value. The control framework is aware of the 'old' control types
and will fill in the elem_size accordingly, but it cannot do that in the
general case for these complex types. I guess it could be filled in by the
framework for the more common types (U8, U16) but I felt it was more
consistent to just require drivers to fill it in manually, rather than have
it set for some types but not for others.
>
> Other option would be not declaring the basic data types (U8, U16,
> U32...) and use elem_size. Ie. If type==V4L2_CTRL_COMPLEX_TYPES, then
> the type is basic and elem_size is the size of the type. If the type
>> V4L2_CTRL_COMPLEX_TYPES the type is not basic.
You still need to know the type. Applications have to be able to check for
the type, the element size by itself doesn't tell you how to interpret the
data, you need the type identifier as well.
Regards,
Hans
>
> Thanks!
>
> On Mon, Feb 10, 2014 at 9:46 AM, Hans Verkuil wrote:
>> From: Hans Verkuil
>>
>> These are needed by the upcoming patches for the motion detection
>> matrices.
>>
>> Signed-off-by: Hans Verkuil
>> ---
>> drivers/media/v4l2-core/v4l2-ctrls.c | 24
>> include/media/v4l2-ctrls.h | 4
>> include/uapi/linux/videodev2.h | 4
>> 3 files changed, 32 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index c81ebcf..0b200dd 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -1145,6 +1145,10 @@ static bool std_equal(const struct v4l2_ctrl *ctrl,
>> u32 idx,
>> return !strcmp(ptr1.p_char + idx, ptr2.p_char + idx);
>> case V4L2_CTRL_TYPE_INTEGER64:
>> return ptr1.p_s64[idx] == ptr2.p_s64[idx];
>> + case V4L2_CTRL_TYPE_U8:
>> + return ptr1.p_u8[idx] == ptr2.p_u8[idx];
>> + case V4L2_CTRL_TYPE_U16:
>> + return ptr1.p_u16[idx] == ptr2.p_u16[idx];
>> default:
>> if (ctrl->is_int)
>> return ptr1.p_s32[idx] == ptr2.p_s32[idx];
>> @@ -1172,6 +1176,12 @@ static void std_init(const struct v4l2_ctrl *ctrl,
>> u32 idx,
>> case V4L2_CTRL_TYPE_BOOLEAN:
>> ptr.p_s32[idx] = ctrl->default_value;
>> break;
>> + case V4L2_CTRL_TYPE_U8:
>> + ptr.p_u8[idx] = ctrl->default_value;
>> + break;
>> + case V4L2_CTRL_TYPE_U16:
>> + ptr.p_u16[idx] = ctrl->default_value;
>> + break;
>> default:
>> idx *= ctrl->elem_size;
>> memset(ptr.p + idx, 0, ctrl->elem_size);
>> @@ -1208,6 +1218,12 @@ static void std_log(const struct v4l2_ctrl *ctrl)
>> case V4L2_CTRL_TYPE_STRING:
>> pr_cont("%s", ptr.p_char);
>> break;
>> + case V4L2_CTRL_TYPE_U8:
>> + pr_cont("%u", (unsigned)*ptr.p_u8);
>> + break;
>> + case V4L2_CTRL_TYPE_U16:
>> + pr_cont("%u", (unsigned)*ptr.p_u16);
>> + break;
>> default:
>> pr_cont("unknown type %d", ctrl->type);
>> break;
>> @@ -1238,6 +1254,10 @@ static int std_validate(const struct v4l2_ctrl *ctrl,
>> u32 idx,
>> return ROUND_TO_RANGE(ptr.p_s32[idx], u32, ctrl);
>> case V4L2_CTRL_TYPE_INTEGER64:
>> return ROUND_TO_RANGE(ptr.p_s64[idx], u64, ctrl);
>> + case V4L2_CTRL_TYPE_U8:
>> + return ROUND_TO_RANGE(ptr.p_u8[idx], u8, ctrl);
>> + case V4L2_CTRL_TYPE_U16:
>> + return ROUND_TO_RANGE(ptr.p_u16[idx], u16, ctrl);
>>
>> case V4L2_CTRL_TYPE_BOOLEAN:
>> ptr.p_s32[idx] = !!ptr.p_s32[idx];
>> @@ -1469,6 +1489,8 @@ static int check_range(enum v4l2_ctrl_type type,
>> if (step != 1 || max > 1 || min < 0)
>> return -ERANGE;
>> /* fall through */
>> + case V4L2_CTRL_TYPE_U8:
>> + case V4L2_CTRL_TYPE_U16:
>> case V4L2_CTRL_TYPE_INTEGER:
>> case V4L2_CTRL_TYPE_INTEGER64:
>> if (step == 0 || min > max || def < min || def > max)
>> @@ -3119,6 +3141,8 @@ int v4l2_ctrl_modify_range(struct v4l2_ctrl *ctrl,
>> case V4L2_CTRL_TYPE_MENU:
>> case V4L2_CTRL_TYPE_INTEGER_MENU:
>> case V4L2_CTRL_TYPE_BITMASK:
>> + case V4L2_CTRL_TYPE_U8:
>> + case V4L2_CTRL_TYPE_U16:
>> if (ctrl->is_matrix)
>> return -EINVAL;
>> ret
Re: [REVIEWv2 PATCH 08/34] v4l2-ctrls: create type_ops.
Hello Hans
If we have a matrix control with a huge number of elements (ie, the
number of pixels), a control validation will imply over ahuge number
of function calls.
In the case of the matrix you are always validating/init all the
values of of the array. Why dont we modify the type_ops so they
operate on the whole matrix (remove the idx from the function).
Anyway we should modify
for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++)
ctrl_changed |= !ctrl->type_ops->equal(ctrl, idx,
ctrl->stores[0], ctrl->new);
to
for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++)
if (!ctrl->type_ops->equal(ctrl, idx, ctrl->stores[0],
ctrl->new))
break
if (idx==ctrl->rows * ctrl->col)
ctrol_changed=true;
Saving us some calls when we already know that the control has changed.
Thanks!
ps: consider all my code pseudocode :P
On Mon, Feb 10, 2014 at 9:46 AM, Hans Verkuil wrote:
> From: Hans Verkuil
>
> Since complex controls can have non-standard types we need to be able to do
> type-specific checks etc. In order to make that easy type operations are
> added.
> There are four operations:
>
> - equal: check if two values are equal
> - init: initialize a value
> - log: log the value
> - validate: validate a new value
>
> This patch uses the v4l2_ctrl_ptr union for the first time.
>
> Signed-off-by: Hans Verkuil
> Reviewed-by: Sylwester Nawrocki
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 267
> ++-
> include/media/v4l2-ctrls.h | 21 +++
> 2 files changed, 190 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 67e5d1e..988a2bd8 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1132,6 +1132,149 @@ static void send_event(struct v4l2_fh *fh, struct
> v4l2_ctrl *ctrl, u32 changes)
> v4l2_event_queue_fh(sev->fh, &ev);
> }
>
> +static bool std_equal(const struct v4l2_ctrl *ctrl,
> + union v4l2_ctrl_ptr ptr1,
> + union v4l2_ctrl_ptr ptr2)
> +{
> + switch (ctrl->type) {
> + case V4L2_CTRL_TYPE_BUTTON:
> + return false;
> + case V4L2_CTRL_TYPE_STRING:
> + /* strings are always 0-terminated */
> + return !strcmp(ptr1.p_char, ptr2.p_char);
> + case V4L2_CTRL_TYPE_INTEGER64:
> + return *ptr1.p_s64 == *ptr2.p_s64;
> + default:
> + if (ctrl->is_ptr)
> + return !memcmp(ptr1.p, ptr2.p, ctrl->elem_size);
> + return *ptr1.p_s32 == *ptr2.p_s32;
> + }
> +}
> +
> +static void std_init(const struct v4l2_ctrl *ctrl,
> +union v4l2_ctrl_ptr ptr)
> +{
> + switch (ctrl->type) {
> + case V4L2_CTRL_TYPE_STRING:
> + memset(ptr.p_char, ' ', ctrl->minimum);
> + ptr.p_char[ctrl->minimum] = '\0';
> + break;
> + case V4L2_CTRL_TYPE_INTEGER64:
> + *ptr.p_s64 = ctrl->default_value;
> + break;
> + case V4L2_CTRL_TYPE_INTEGER:
> + case V4L2_CTRL_TYPE_INTEGER_MENU:
> + case V4L2_CTRL_TYPE_MENU:
> + case V4L2_CTRL_TYPE_BITMASK:
> + case V4L2_CTRL_TYPE_BOOLEAN:
> + *ptr.p_s32 = ctrl->default_value;
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static void std_log(const struct v4l2_ctrl *ctrl)
> +{
> + union v4l2_ctrl_ptr ptr = ctrl->stores[0];
> +
> + switch (ctrl->type) {
> + case V4L2_CTRL_TYPE_INTEGER:
> + pr_cont("%d", *ptr.p_s32);
> + break;
> + case V4L2_CTRL_TYPE_BOOLEAN:
> + pr_cont("%s", *ptr.p_s32 ? "true" : "false");
> + break;
> + case V4L2_CTRL_TYPE_MENU:
> + pr_cont("%s", ctrl->qmenu[*ptr.p_s32]);
> + break;
> + case V4L2_CTRL_TYPE_INTEGER_MENU:
> + pr_cont("%lld", ctrl->qmenu_int[*ptr.p_s32]);
> + break;
> + case V4L2_CTRL_TYPE_BITMASK:
> + pr_cont("0x%08x", *ptr.p_s32);
> + break;
> + case V4L2_CTRL_TYPE_INTEGER64:
> + pr_cont("%lld", *ptr.p_s64);
> + break;
> + case V4L2_CTRL_TYPE_STRING:
> + pr_cont("%s", ptr.p_char);
> + break;
> + default:
> + pr_cont("unknown type %d", ctrl->type);
> + break;
> + }
> +}
> +
> +/* Round towards the closest legal value */
> +#define ROUND_TO_RANGE(val, offset_type, ctrl) \
> +({ \
> + offset_type offset; \
> + val += (ctrl)->step / 2;\
> + val = clamp_t(typeof(val), val, \
> +
Re: [REVIEWv2 PATCH 24/34] v4l2-ctrls/videodev2.h: add u8 and u16 types.
Hello Hans
In the case of U8 and U16 data types. Why dont you fill the elem_size
automatically in v4l2_ctrl and request the driver to fill the field?
Other option would be not declaring the basic data types (U8, U16,
U32...) and use elem_size. Ie. If type==V4L2_CTRL_COMPLEX_TYPES, then
the type is basic and elem_size is the size of the type. If the type
>V4L2_CTRL_COMPLEX_TYPES the type is not basic.
Thanks!
On Mon, Feb 10, 2014 at 9:46 AM, Hans Verkuil wrote:
> From: Hans Verkuil
>
> These are needed by the upcoming patches for the motion detection
> matrices.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 24
> include/media/v4l2-ctrls.h | 4
> include/uapi/linux/videodev2.h | 4
> 3 files changed, 32 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index c81ebcf..0b200dd 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1145,6 +1145,10 @@ static bool std_equal(const struct v4l2_ctrl *ctrl,
> u32 idx,
> return !strcmp(ptr1.p_char + idx, ptr2.p_char + idx);
> case V4L2_CTRL_TYPE_INTEGER64:
> return ptr1.p_s64[idx] == ptr2.p_s64[idx];
> + case V4L2_CTRL_TYPE_U8:
> + return ptr1.p_u8[idx] == ptr2.p_u8[idx];
> + case V4L2_CTRL_TYPE_U16:
> + return ptr1.p_u16[idx] == ptr2.p_u16[idx];
> default:
> if (ctrl->is_int)
> return ptr1.p_s32[idx] == ptr2.p_s32[idx];
> @@ -1172,6 +1176,12 @@ static void std_init(const struct v4l2_ctrl *ctrl, u32
> idx,
> case V4L2_CTRL_TYPE_BOOLEAN:
> ptr.p_s32[idx] = ctrl->default_value;
> break;
> + case V4L2_CTRL_TYPE_U8:
> + ptr.p_u8[idx] = ctrl->default_value;
> + break;
> + case V4L2_CTRL_TYPE_U16:
> + ptr.p_u16[idx] = ctrl->default_value;
> + break;
> default:
> idx *= ctrl->elem_size;
> memset(ptr.p + idx, 0, ctrl->elem_size);
> @@ -1208,6 +1218,12 @@ static void std_log(const struct v4l2_ctrl *ctrl)
> case V4L2_CTRL_TYPE_STRING:
> pr_cont("%s", ptr.p_char);
> break;
> + case V4L2_CTRL_TYPE_U8:
> + pr_cont("%u", (unsigned)*ptr.p_u8);
> + break;
> + case V4L2_CTRL_TYPE_U16:
> + pr_cont("%u", (unsigned)*ptr.p_u16);
> + break;
> default:
> pr_cont("unknown type %d", ctrl->type);
> break;
> @@ -1238,6 +1254,10 @@ static int std_validate(const struct v4l2_ctrl *ctrl,
> u32 idx,
> return ROUND_TO_RANGE(ptr.p_s32[idx], u32, ctrl);
> case V4L2_CTRL_TYPE_INTEGER64:
> return ROUND_TO_RANGE(ptr.p_s64[idx], u64, ctrl);
> + case V4L2_CTRL_TYPE_U8:
> + return ROUND_TO_RANGE(ptr.p_u8[idx], u8, ctrl);
> + case V4L2_CTRL_TYPE_U16:
> + return ROUND_TO_RANGE(ptr.p_u16[idx], u16, ctrl);
>
> case V4L2_CTRL_TYPE_BOOLEAN:
> ptr.p_s32[idx] = !!ptr.p_s32[idx];
> @@ -1469,6 +1489,8 @@ static int check_range(enum v4l2_ctrl_type type,
> if (step != 1 || max > 1 || min < 0)
> return -ERANGE;
> /* fall through */
> + case V4L2_CTRL_TYPE_U8:
> + case V4L2_CTRL_TYPE_U16:
> case V4L2_CTRL_TYPE_INTEGER:
> case V4L2_CTRL_TYPE_INTEGER64:
> if (step == 0 || min > max || def < min || def > max)
> @@ -3119,6 +3141,8 @@ int v4l2_ctrl_modify_range(struct v4l2_ctrl *ctrl,
> case V4L2_CTRL_TYPE_MENU:
> case V4L2_CTRL_TYPE_INTEGER_MENU:
> case V4L2_CTRL_TYPE_BITMASK:
> + case V4L2_CTRL_TYPE_U8:
> + case V4L2_CTRL_TYPE_U16:
> if (ctrl->is_matrix)
> return -EINVAL;
> ret = check_range(ctrl->type, min, max, step, def);
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index 7d72328..2ccad5f 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -39,12 +39,16 @@ struct poll_table_struct;
> /** union v4l2_ctrl_ptr - A pointer to a control value.
> * @p_s32: Pointer to a 32-bit signed value.
> * @p_s64: Pointer to a 64-bit signed value.
> + * @p_u8: Pointer to a 8-bit unsigned value.
> + * @p_u16: Pointer to a 16-bit unsigned value.
> * @p_char:Pointer to a string.
> * @p: Pointer to a complex value.
> */
> union v4l2_ctrl_ptr {
> s32 *p_s32;
> s64 *p_s64;
> + u8 *p_u8;
> + u16 *p_u16;
> char *p_char;
> void *p;
> };
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 858a6f3..8b70f51 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/includ
[PATCH] [media] s5p-fimc: Remove reference to outdated macro
The Kconfig symbol S5P_SETUP_MIPIPHY was removed in v3.13. Remove a
reference to its macro from a list of Kconfig options.
Signed-off-by: Paul Bolle
---
See commit e66f233dc7f7 ("ARM: Samsung: Remove the MIPI PHY setup
code"). Should one or more options be added to replace
S5P_SETUP_MIPIPHY? I couldn't say. It's safe to remove this one anyway.
Documentation/video4linux/fimc.txt | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/Documentation/video4linux/fimc.txt
b/Documentation/video4linux/fimc.txt
index e51f1b5..7d6e160 100644
--- a/Documentation/video4linux/fimc.txt
+++ b/Documentation/video4linux/fimc.txt
@@ -151,9 +151,8 @@ CONFIG_S5P_DEV_FIMC1 \
CONFIG_S5P_DEV_FIMC2 |optional
CONFIG_S5P_DEV_FIMC3 |
CONFIG_S5P_SETUP_FIMC /
-CONFIG_S5P_SETUP_MIPIPHY \
-CONFIG_S5P_DEV_CSIS0 | optional for MIPI-CSI interface
-CONFIG_S5P_DEV_CSIS1 /
+CONFIG_S5P_DEV_CSIS0 \optional for MIPI-CSI interface
+CONFIG_S5P_DEV_CSIS1 /
Except that, relevant s5p_device_fimc? should be registered in the machine code
in addition to a "s5p-fimc-md" platform device to which the media device driver
--
1.8.5.3
--
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
Cash Awaiting Pick Up..
This is to re-notify you that you have $500,000.00 waiting for pick-up at Money Gram, Contact Mrs Hillary Florence via email : heritd...@xtra.co.nz for claims. -- 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] of: move graph helpers from drivers/media/v4l2-core to drivers/media
Hi Tomi, Am Mittwoch, den 12.02.2014, 09:15 +0200 schrieb Tomi Valkeinen: > Hi, > > On 11/02/14 23:41, Philipp Zabel wrote: > > From: Philipp Zabel > > > > This patch moves the parsing helpers used to parse connected graphs > > in the device tree, like the video interface bindings documented in > > Documentation/devicetree/bindings/media/video-interfaces.txt, from > > drivers/media/v4l2-core to drivers/media. > > > > This allows to reuse the same parser code from outside the V4L2 > > framework, most importantly from display drivers. > > The functions v4l2_of_get_next_endpoint, v4l2_of_get_remote_port, > > and v4l2_of_get_remote_port_parent are moved. They are renamed to > > of_graph_get_next_endpoint, of_graph_get_remote_port, and > > of_graph_get_remote_port_parent, respectively. > > Since there are not that many current users, switch all of them > > to the new functions right away. > > > > Signed-off-by: Philipp Zabel > > Acked-by: Mauro Carvalho Chehab > > Acked-by: Guennadi Liakhovetski > > I don't think the graphs or the parsing code has anything video > specific. It could well be used for anything, whenever there's need to > describe connections between devices which are not handled by the normal > child-parent relationships. So the code could well reside in some > generic place, in my opinion. > > Also, I have no problem with having it in drivers/media, but > drivers/video should work also. We already have other, generic, video > related things there like hdmi infoframes and display timings. I agree. In case anybody wants to use this for audio in the future, media already sounds more generic than video. > And last, it's fine to move the funcs as-is in the first place, but I > think they should be improved a bit before non-v4l2 users use them. The get_remote_port(_parent) are fine, I think. > There are a couple of things I tried to accomplish with the omapdss > specific versions in > https://www.mail-archive.com/linux-omap@vger.kernel.org/msg100761.html: > > - Iterating ports and endpoints separately. If a node has multiple > ports, I would think that the driver needs to know which port and > endpoint combination is the current one during iteration. It's not > enough to just get the endpoint. Yes, I'd already have a use-case for that in enumerating the encoders/panels connected to a single display interface (port). On the other hand if you just want to enumerate components from the device tree, iterating over all endpoints of all ports is useful, too. > - Simplify cases when there's just one port and one endpoint, in which > case the port node can be omitted from the DT data. Also, I'd like to drop the prev reference in get_next_endpoint, then a for_each_endpoint macro could be made from that. regards Philipp -- 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] of: move graph helpers from drivers/media/v4l2-core to drivers/media
Hi Mauro, Am Mittwoch, den 12.02.2014, 06:53 +0900 schrieb Mauro Carvalho Chehab: [...] > > diff --git a/include/media/of_graph.h b/include/media/of_graph.h > > new file mode 100644 > > index 000..3bbeb60 > > --- /dev/null > > +++ b/include/media/of_graph.h > > @@ -0,0 +1,46 @@ > > +/* > > + * OF graph binding parsing helpers > > + * > > + * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd. > > + * Author: Sylwester Nawrocki > > + * > > + * Copyright (C) 2012 Renesas Electronics Corp. > > + * Author: Guennadi Liakhovetski > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of version 2 of the GNU General Public License as > > + * published by the Free Software Foundation. > > + */ > > +#ifndef __LINUX_OF_GRAPH_H > > +#define __LINUX_OF_GRAPH_H > > + > > +#ifdef CONFIG_OF > > As a matter of consistency, it would be better to test here for > CONFIG_OF_GRAPH instead, to reflect the same symbol that enables such > functions as used on Kconfig/Makefile. Maybe I'm trying to be too clever for my own good, but my reasoning was as follows: Suppose I newly use the of_graph_ helpers in a subsystem that does not yet select OF_GRAPH. In that case I'd rather get linking errors earlier rather than stubbed out functions that silently fail to parse the DT later. Since there is config VIDEO_DEV select OF_GRAPH if OF already and the same should be added for other users of device tree graphs, I think stubbing out the functions only if OF is disabled should be enough. regards Philipp -- 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 1/3] lm3560: remove FSF address from the license
There is no need to keep the FSF address inside each file. Moreover, it might change in future which will make this one obsolete. There is no functional change. Signed-off-by: Andy Shevchenko --- drivers/media/i2c/lm3560.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c index d98ca3a..fea37a3 100644 --- a/drivers/media/i2c/lm3560.c +++ b/drivers/media/i2c/lm3560.c @@ -15,12 +15,6 @@ * 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 -- 1.9.0.rc3 -- 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 2/3] lm3560: keep style for the comments
Let's keep the style for all comments in the code, namely using small letters
whenever it's possible.
There is no functional change.
Signed-off-by: Andy Shevchenko
---
drivers/media/i2c/lm3560.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
index fea37a3..93e5227 100644
--- a/drivers/media/i2c/lm3560.c
+++ b/drivers/media/i2c/lm3560.c
@@ -36,7 +36,7 @@
#define REG_FLAG 0xd0
#define REG_CONFIG10xe0
-/* Fault Mask */
+/* fault mask */
#define FAULT_TIMEOUT (1<<0)
#define FAULT_OVERTEMP (1<<1)
#define FAULT_SHORT_CIRCUIT(1<<2)
@@ -47,7 +47,8 @@ enum led_enable {
MODE_FLASH = 0x3,
};
-/* struct lm3560_flash
+/**
+ * struct lm3560_flash
*
* @pdata: platform data
* @regmap: reg. map for i2c
@@ -92,7 +93,7 @@ static int lm3560_mode_ctrl(struct lm3560_flash *flash)
return rval;
}
-/* led1/2 enable/disable */
+/* led1/2 enable/disable */
static int lm3560_enable_ctrl(struct lm3560_flash *flash,
enum lm3560_led_id led_no, bool on)
{
@@ -162,7 +163,7 @@ static int lm3560_flash_brt_ctrl(struct lm3560_flash *flash,
return rval;
}
-/* V4L2 controls */
+/* v4l2 controls */
static int lm3560_get_ctrl(struct v4l2_ctrl *ctrl, enum lm3560_led_id led_no)
{
struct lm3560_flash *flash = to_lm3560_flash(ctrl, led_no);
@@ -291,6 +292,7 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
const struct v4l2_ctrl_ops *ops = &lm3560_led_ctrl_ops[led_no];
v4l2_ctrl_handler_init(hdl, 8);
+
/* flash mode */
v4l2_ctrl_new_std_menu(hdl, ops, V4L2_CID_FLASH_LED_MODE,
V4L2_FLASH_LED_MODE_TORCH, ~0x7,
@@ -303,6 +305,7 @@ static int lm3560_init_controls(struct lm3560_flash *flash,
/* flash strobe */
v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_STROBE, 0, 0, 0, 0);
+
/* flash strobe stop */
v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FLASH_STROBE_STOP, 0, 0, 0, 0);
@@ -389,7 +392,7 @@ static int lm3560_init_device(struct lm3560_flash *flash)
rval = lm3560_mode_ctrl(flash);
if (rval < 0)
return rval;
- /* Reset faults */
+ /* reset faults */
rval = regmap_read(flash->regmap, REG_FLAG, ®_val);
return rval;
}
--
1.9.0.rc3
--
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 3/3] lm3560: prevent memory leak in case of pdata absence
If we have no pdata defined and driver fails to register we leak memory.
Converting to devm_kzalloc prevents this to happen.
Signed-off-by: Andy Shevchenko
---
drivers/media/i2c/lm3560.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
index 93e5227..c23de59 100644
--- a/drivers/media/i2c/lm3560.c
+++ b/drivers/media/i2c/lm3560.c
@@ -416,8 +416,7 @@ static int lm3560_probe(struct i2c_client *client,
/* if there is no platform data, use chip default value */
if (pdata == NULL) {
- pdata =
- kzalloc(sizeof(struct lm3560_platform_data), GFP_KERNEL);
+ pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
if (pdata == NULL)
return -ENODEV;
pdata->peak = LM3560_PEAK_3600mA;
--
1.9.0.rc3
--
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
