Re: go7007 question
On Sat, Sep 08, 2012 at 10:23:31PM -0400, Adam Rosi-Kessel wrote: On Fri, Sep 07, 2012 at 06:18:31PM +0400, vol...@telros.ru wrote: On Thu, Sep 06, 2012 at 11:10:14PM +0400, Volokh Konstantin wrote: On Mon, Sep 03, 2012 at 02:37:16PM -0400, Adam Rosi-Kessel wrote: [469.928881] wis-saa7115: initializing SAA7115 at address 32 on WIS GO7007SB EZ-USB [469.989083] go7007: probing for module i2c:wis_saa7115 failed [470.004785] wis-uda1342: initializing UDA1342 at address 26 on WIS GO7007SB EZ-USB [470.005454] go7007: probing for module i2c:wis_uda1342 failed [470.011659] wis-sony-tuner: initializing tuner at address 96 on WIS GO7007SB EZ-USB Hi, I generated patchs, that u may in your own go7007/ folder It contains go7007 initialization and i2c_subdev fixing It was checked for 3.6 branch (compile only) So I have this installed now (patched with your 3.6 patch) but I'm not seeing the device. The module is there: [ 416.189030] Linux media interface: v0.10 [ 416.198616] Linux video capture interface: v2.00 [ 416.220656] wis_uda1342: module is from the staging directory, the quality is unknown, you have been warned. # lsmod|grep -i go7 go7007_usb 10059 0 go7007 46966 1 go7007_usb v4l2_common 4206 1 go7007 videodev 78250 2 go7007,v4l2_common # uname -a Linux storage 3.6.0-rc4.ajk+ #5 SMP Sat Sep 8 22:05:57 EDT 2012 i686 GNU/Linux # grep -i go7 /boot/config-`uname -r` CONFIG_VIDEO_GO7007=m CONFIG_VIDEO_GO7007_USB=m CONFIG_VIDEO_GO7007_OV7640=m # CONFIG_VIDEO_GO7007_SAA7113 is not set Linux must autoload these modules for working: wis-saa7115 wis-uda1342 wis-sony-tuner, so need set m below: # CONFIG_VIDEO_GO7007_SAA7115 is not set CONFIG_VIDEO_GO7007_TW9903=m CONFIG_VIDEO_GO7007_UDA1342=m CONFIG_VIDEO_GO7007_SONY_TUNER=m CONFIG_VIDEO_GO7007_TW2804=m after compilation need install modules, or handly load them. modprobe wis-saa7115, etc ... for each modules. But I'm not getting any device to appear: # ls /dev/video* ls: cannot access /dev/video*: No such file or directory # gorecord -format mpeg4 test.avi Driver loaded but no GO7007 devices found. Is the device connected properly? When I connect the device I see this: [ 585.705406] usb 1-4: udev 4, busnum 1, minor = 3 [ 585.705412] usb 1-4: New USB device found, idVendor=093b, idProduct=a004 [ 585.705415] usb 1-4: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [ 585.705532] usb 1-4: usb_probe_device [ 585.705535] usb 1-4: configuration #1 chosen from 1 choice [ 585.706233] usb 1-4: adding 1-4:1.0 (config #1, interface 0) But no video node. Am I missing something? Adam -- 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: go7007 question
On Sun, Sep 09, 2012 at 08:21:55AM -0400, Adam Rosi-Kessel wrote: On Sat, Sep 08, 2012 at 10:23:31PM -0400, Adam Rosi-Kessel wrote: On Fri, Sep 07, 2012 at 06:18:31PM +0400, vol...@telros.ru wrote: On Thu, Sep 06, 2012 at 11:10:14PM +0400, Volokh Konstantin wrote: e On Mon, Sep 03, 2012 at 02:37:16PM -0400, Adam Rosi-Kessel wrote: [469.928881] wis-saa7115: initializing SAA7115 at address 32 on WIS GO7007SB EZ-USB Hi, I generated patchs, that u may in your own go7007/ folder It contains go7007 initialization and i2c_subdev fixing It was checked for 3.6 branch (compile only) So I have this installed now (patched with your 3.6 patch) but I'm not seeing the device. OK, this was just a problem of needing to mount /proc/usb. With that mounted, the device is detected and the modules load automatically. gorecord just hangs, however: [ cut here ] WARNING: at drivers/usb/core/urb.c:414 usb_submit_urb+0x12a/0x3e0() Hardware name: Inspiron 530 Device: usb BOGUS urb xfer, pipe 1 != type 3 Modules linked in: wis_sony_tuner(C) wis_uda1342(C) go7007_usb(C) go7007(C) v4l2_common videodev media xt_LOG fuse ext4 jbd2 crc16 e1000e I`m not seeing wis-sa7115 module, (it`s main module for video device) [last unloaded: go7007] Pid: 18595, comm: gorecord Tainted: GWC 3.6.0-rc4 Call Trace: [c1033f4d] warn_slowpath_common+0x6d/0xa0 [c13bc8fa] ? usb_submit_urb+0x12a/0x3e0 Some error in URB, did u machine have any ATI chipset? [c13bc8fa] ? usb_submit_urb+0x12a/0x3e0 [c1033ffe] warn_slowpath_fmt+0x2e/0x30 [c13bc8fa] usb_submit_urb+0x12a/0x3e0 [f938625a] go7007_usb_read_interrupt+0x1a/0x40 [go7007_usb] [f9371af4] go7007_read_interrupt+0x24/0x100 [go7007] [f938615e] ? go7007_usb_send_firmware+0x3e/0x60 [go7007_usb] [f9371c4d] go7007_start_encoder+0x7d/0x120 [go7007] [c1624d84] ? mutex_lock+0x14/0x40 [f9370c2c] vidioc_streamon+0xdc/0xf0 [go7007] [f9004b75] v4l_streamon+0x15/0x20 [videodev] [f90070cc] __video_do_ioctl+0x28c/0x3a0 [videodev] [c1075933] ? ktime_get+0x43/0xf0 [c107b9fa] ? clockevents_program_event+0xca/0x180 [c122bd28] ? _copy_from_user+0x38/0x130 [f9008b83] video_usercopy+0x143/0x320 [videodev] [c103be44] ? irq_exit+0x54/0xc0 [c10218a4] ? smp_apic_timer_interrupt+0x54/0x90 [c113b631] ? inotify_handle_event+0x51/0xc0 [c113b510] ? idr_callback+0x80/0x80 [c1626f66] ? apic_timer_interrupt+0x2a/0x30 [f9008d72] video_ioctl2+0x12/0x20 [videodev] [f9006e40] ? v4l2_ioctl_get_lock+0x50/0x50 [videodev] [f9003913] v4l2_ioctl+0x103/0x150 [videodev] [f9003810] ? v4l2_open+0x140/0x140 [videodev] Can u send me: ls /dev/video* ls /dev/audio* ls /dev/tun* tail -n 500 /var/log/messages (after calling go_record) [c111440e] do_vfs_ioctl+0x7e/0x5c0 [c11e198a] ? file_has_perm+0x9a/0xc0 [c11e1e86] ? selinux_file_ioctl+0x56/0x110 [c11149cf] sys_ioctl+0x7f/0x90 [c162d18c] sysenter_do_call+0x12/0x22 ---[ end trace 04d11de2981e53e3 ]--- Thanks. -- 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 v5] media: v4l2-ctrls: add control for dpcm predictor
Hi Prabhakar, On Mon, Sep 10, 2012 at 11:27:55AM +0530, Prabhakar Lad wrote: ... + row id=v4l2-dpcm-predictor + entry spanname=descr Differential pulse-code modulation (DPCM) compression can + be used to compress the samples into fewer bits than they would otherwise require. + This is done by calculating the difference between consecutive samples and outputting + the difference which in average is much smaller than the values of the samples + themselves since there is generally lots of correlation between adjacent pixels. In + decompression the original samples are reconstructed. The process isn't lossless as + the encoded sample size in bits is less than the original. + + paraFormats using DPCM compression include xref linkend=pixfmt-srggb10dpcm8 /./para + + paraThis control is used to select the predictor used to encode the samples./para + + paraThe main difference between the simple and the advanced predictors is image quality, + with advanced predictor supposed to produce better quality images as a result. Simple + predictor can be used e.g. for testing purposes. For more information about DPCM see ulink + url=http://en.wikipedia.org/wiki/Differential_pulse-code_modulation;Wikipedia/ulink./para Could you fit the above text (description of the control) at 80 characters per line? With that change, Acked-by: Sakari Ailus sakari.ai...@iki.fi Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 03/16] media: coda: fix IRAM/AXI handling for i.MX53
On Fri, Aug 31, 2012 at 10:10:57AM +0200, Philipp Zabel wrote: This uses the ARCH_MXC specific iram_alloc API to allocate a work buffer in the SoC's on-chip SRAM and sets up the AXI_SRAM_USE register. In the future, the allocation will be converted to use the genalloc API. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/Kconfig |3 ++- drivers/media/platform/coda.c | 51 drivers/media/platform/coda.h | 21 + 3 files changed, 66 insertions(+), 9 deletions(-) diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index d4c034d..76f9a8f 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -130,9 +130,10 @@ if V4L_MEM2MEM_DRIVERS config VIDEO_CODA tristate ChipsMedia Coda multi-standard codec IP - depends on VIDEO_DEV VIDEO_V4L2 + depends on VIDEO_DEV VIDEO_V4L2 ARCH_MXC select VIDEOBUF2_DMA_CONTIG select V4L2_MEM2MEM_DEV + select IRAM_ALLOC if SOC_IMX53 ---help--- Coda is a range of video codec IPs that supports H.264, MPEG-4, and other video formats. diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 8ec2ff4..5c06bc1 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -24,6 +24,7 @@ #include linux/videodev2.h #include linux/of.h +#include mach/iram.h #include media/v4l2-ctrls.h #include media/v4l2-device.h #include media/v4l2-ioctl.h @@ -42,6 +43,7 @@ #define CODA7_WORK_BUF_SIZE (512 * 1024 + CODA_FMO_BUF_SIZE * 8 * 1024) #define CODA_PARA_BUF_SIZE (10 * 1024) #define CODA_ISRAM_SIZE (2048 * 2) +#define CODA7_IRAM_SIZE 0x14000 /* 81920 bytes */ #define CODA_OUTPUT_BUFS 4 #define CODA_CAPTURE_BUFS2 @@ -127,6 +129,8 @@ struct coda_dev { struct coda_aux_buf codebuf; struct coda_aux_buf workbuf; + long unsigned int iram_vaddr; should be void __iomem * ? + long unsigned int iram_paddr; spinlock_t irqlock; struct mutexdev_mutex; @@ -710,6 +714,13 @@ static void coda_device_run(void *m2m_priv) coda_write(dev, pic_stream_buffer_addr, CODA_CMD_ENC_PIC_BB_START); coda_write(dev, pic_stream_buffer_size / 1024, CODA_CMD_ENC_PIC_BB_SIZE); + + if (dev-devtype-product == CODA_7541) { + coda_write(dev, CODA7_USE_BIT_ENABLE | CODA7_USE_HOST_BIT_ENABLE | + CODA7_USE_ME_ENABLE | CODA7_USE_HOST_ME_ENABLE, + CODA7_REG_BIT_AXI_SRAM_USE); + } + coda_command_async(ctx, CODA_COMMAND_PIC_RUN); } @@ -941,8 +952,10 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) CODA7_STREAM_BUF_PIC_RESET, CODA_REG_BIT_STREAM_CTRL); } - /* Configure the coda */ - coda_write(dev, 0x4c00, CODA_REG_BIT_SEARCH_RAM_BASE_ADDR); + if (dev-devtype-product == CODA_DX6) { + /* Configure the coda */ + coda_write(dev, dev-iram_paddr, CODADX6_REG_BIT_SEARCH_RAM_BASE_ADDR); + } /* Could set rotation here if needed */ switch (dev-devtype-product) { @@ -1017,7 +1030,12 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) value = (FMO_SLICE_SAVE_BUF_SIZE 7); value |= (0 CODA_FMOPARAM_TYPE_MASK) CODA_FMOPARAM_TYPE_OFFSET; value |= 0 CODA_FMOPARAM_SLICENUM_MASK; - coda_write(dev, value, CODA_CMD_ENC_SEQ_FMO); + if (dev-devtype-product == CODA_DX6) { + coda_write(dev, value, CODADX6_CMD_ENC_SEQ_FMO); + } else { + coda_write(dev, dev-iram_paddr, CODA7_CMD_ENC_SEQ_SEARCH_BASE); + coda_write(dev, 48 * 1024, CODA7_CMD_ENC_SEQ_SEARCH_SIZE); + } } if (coda_command_sync(ctx, CODA_COMMAND_SEQ_INIT)) { @@ -1047,7 +1065,15 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) } coda_write(dev, src_vq-num_buffers, CODA_CMD_SET_FRAME_BUF_NUM); - coda_write(dev, q_data_src-width, CODA_CMD_SET_FRAME_BUF_STRIDE); + coda_write(dev, round_up(q_data_src-width, 8), CODA_CMD_SET_FRAME_BUF_STRIDE); + if (dev-devtype-product != CODA_DX6) { + coda_write(dev, round_up(q_data_src-width, 8), CODA7_CMD_SET_FRAME_SOURCE_BUF_STRIDE); + coda_write(dev, dev-iram_paddr + 48 * 1024, CODA7_CMD_SET_FRAME_AXI_DBKY_ADDR); + coda_write(dev, dev-iram_paddr + 53 * 1024, CODA7_CMD_SET_FRAME_AXI_DBKC_ADDR); + coda_write(dev, dev-iram_paddr + 58 * 1024, CODA7_CMD_SET_FRAME_AXI_BIT_ADDR); + coda_write(dev, dev-iram_paddr + 68 * 1024,
Re: [PATCH 01/10] staging: media: go7007: Some additional code for TW2804 driver functionality
This patch got corrupted somewhere so it doesn't apply. There are some minor style issues as well. On Wed, Aug 22, 2012 at 02:45:10PM +0400, Volokh Konstantin wrote: - using new v4l2 framework controls - function for reading volatile controls via i2c bus - separate V4L2_CID_ ctrls into each V4L2 calls Signed-off-by: Volokh Konstantin volok...@gmail.com --- drivers/staging/media/go7007/wis-tw2804.c | 248 + 1 files changed, 248 insertions(+), 0 deletions(-) diff --git a/drivers/staging/media/go7007/wis-tw2804.c b/drivers/staging/media/go7007/wis-tw2804.c index 9134f03..05851d3 100644 --- a/drivers/staging/media/go7007/wis-tw2804.c +++ b/drivers/staging/media/go7007/wis-tw2804.c @@ -21,10 +21,18 @@ #include linux/videodev2.h #include linux/ioctl.h #include linux/slab.h +#include media/v4l2-subdev.h +#include media/v4l2-device.h +#include media/v4l2-chip-ident.h +#include media/v4l2-ctrls.h #include wis-i2c.h struct wis_tw2804 { + struct v4l2_subdev sd; + struct v4l2_ctrl_handler hdl; + u8 channel:2; + u8 input:1; int channel; int norm; int brightness; @@ -116,9 +124,246 @@ static int write_regs(struct i2c_client *client, u8 *regs, int channel) if (i2c_smbus_write_byte_data(client, regs[i] | (channel 6), regs[i + 1]) 0) return -1; +static s32 read_reg(struct i2c_client *client, u8 reg, u8 channel) +{ Look at the five lines above. This patch introduces a new function in the middle of an existing function. + return i2c_smbus_read_byte_data(client, (reg) | (channel 6)); +} + +inline struct wis_tw2804 *to_state(struct v4l2_subdev *sd) Should this be static? Otherwise the name is very generic. +{ + return container_of(sd, struct wis_tw2804, sd); +} + +inline struct wis_tw2804 *to_state_from_ctrl(struct v4l2_ctrl *ctrl) +{ + return container_of(ctrl-handler, struct wis_tw2804, hdl); +} + +static int tw2804_log_status(struct v4l2_subdev *sd) +{ + struct wis_tw2804 *state = to_state(sd); Blank line here. + v4l2_info(sd, Standard: %s\n, + state-norm == V4L2_STD_NTSC ? NTSC : + state-norm == V4L2_STD_PAL ? PAL : unknown); + v4l2_info(sd, Channel: %d\n, state-channel); + v4l2_info(sd, Input: %d\n, state-input); + v4l2_ctrl_handler_log_status(state-hdl, sd-name); + return 0; +} + +static s32 get_ctrl_addr(int ctrl) Just use int instead of s32. s32 and int are the same but s32 is only needed in special circumstances. It is for when the hardware or protocol specifies a signed 32 bit. You use it throughout instead of using ints. +{ + switch (ctrl) { + case V4L2_CID_BRIGHTNESS: + return 0x12; + case V4L2_CID_CONTRAST: + return 0x11; + case V4L2_CID_SATURATION: + return 0x10; + case V4L2_CID_HUE: + return 0x0f; + case V4L2_CID_AUTOGAIN: + return 0x02; + case V4L2_CID_COLOR_KILLER: + return 0x14; + case V4L2_CID_GAIN: + return 0x3c; + case V4L2_CID_CHROMA_GAIN: + return 0x3d; + case V4L2_CID_RED_BALANCE: + return 0x3f; + case V4L2_CID_BLUE_BALANCE: + return 0x3e; + default: + return -EINVAL; + } +} + +static int tw2804_g_volatile_ctrl(struct v4l2_ctrl *ctrl) +{ + struct v4l2_subdev *sd = to_state_from_ctrl(ctrl)-sd; + struct i2c_client *client = v4l2_get_subdevdata(sd); + s32 addr = get_ctrl_addr(ctrl-id); + + if (addr == -EINVAL) + return -EINVAL; This should be: if (addr 0) return addr; + + switch (ctrl-id) { + case V4L2_CID_GAIN: + case V4L2_CID_CHROMA_GAIN: + case V4L2_CID_RED_BALANCE: + case V4L2_CID_BLUE_BALANCE: + ctrl-cur.val = read_reg(client, addr, 0); + break; + default: + return -EINVAL; + } + return 0; +} + +static int tw2804_s_ctrl(struct v4l2_ctrl *ctrl) +{ + struct wis_tw2804 *state = to_state_from_ctrl(ctrl); + struct v4l2_subdev *sd = state-sd; + struct i2c_client *client = v4l2_get_subdevdata(sd); + s32 reg = ctrl-val; + s32 addr = get_ctrl_addr(ctrl-id); + + if (addr == -EINVAL) + return -EINVAL; + + switch (ctrl-id) { + case V4L2_CID_AUTOGAIN: + reg = read_reg(client, addr, state-channel); + if (reg 0) { + if (ctrl-val == 0) + reg = ~(17); + else + reg |= 17; Spaces around the operations: reg = ~(1 7); else reg |= 1 7; + } else + return reg; +
Re: [PATCH v3 03/16] media: coda: fix IRAM/AXI handling for i.MX53
Hi Richard, thank you for your comments. Am Montag, den 10.09.2012, 16:20 +0800 schrieb Richard Zhao: On Fri, Aug 31, 2012 at 10:10:57AM +0200, Philipp Zabel wrote: This uses the ARCH_MXC specific iram_alloc API to allocate a work buffer in the SoC's on-chip SRAM and sets up the AXI_SRAM_USE register. In the future, the allocation will be converted to use the genalloc API. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/Kconfig |3 ++- drivers/media/platform/coda.c | 51 drivers/media/platform/coda.h | 21 + 3 files changed, 66 insertions(+), 9 deletions(-) diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index d4c034d..76f9a8f 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -130,9 +130,10 @@ if V4L_MEM2MEM_DRIVERS config VIDEO_CODA tristate ChipsMedia Coda multi-standard codec IP - depends on VIDEO_DEV VIDEO_V4L2 + depends on VIDEO_DEV VIDEO_V4L2 ARCH_MXC select VIDEOBUF2_DMA_CONTIG select V4L2_MEM2MEM_DEV + select IRAM_ALLOC if SOC_IMX53 ---help--- Coda is a range of video codec IPs that supports H.264, MPEG-4, and other video formats. diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 8ec2ff4..5c06bc1 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -24,6 +24,7 @@ #include linux/videodev2.h #include linux/of.h +#include mach/iram.h #include media/v4l2-ctrls.h #include media/v4l2-device.h #include media/v4l2-ioctl.h @@ -42,6 +43,7 @@ #define CODA7_WORK_BUF_SIZE(512 * 1024 + CODA_FMO_BUF_SIZE * 8 * 1024) #define CODA_PARA_BUF_SIZE (10 * 1024) #define CODA_ISRAM_SIZE(2048 * 2) +#define CODA7_IRAM_SIZE0x14000 /* 81920 bytes */ #define CODA_OUTPUT_BUFS 4 #define CODA_CAPTURE_BUFS 2 @@ -127,6 +129,8 @@ struct coda_dev { struct coda_aux_buf codebuf; struct coda_aux_buf workbuf; + long unsigned int iram_vaddr; should be void __iomem * ? I wrote this with the genalloc API in mind: gen_pool_alloc returns an unsigned long. We are not accessing this memory from the CPU, so iram_vaddr could just as well be an opaque token that only needs to be remembered for gen_pool_free. For the iram_alloc API in its current state, as you point out below, keeping track of iram_vaddr is not needed. + long unsigned int iram_paddr; spinlock_t irqlock; struct mutexdev_mutex; @@ -710,6 +714,13 @@ static void coda_device_run(void *m2m_priv) coda_write(dev, pic_stream_buffer_addr, CODA_CMD_ENC_PIC_BB_START); coda_write(dev, pic_stream_buffer_size / 1024, CODA_CMD_ENC_PIC_BB_SIZE); + + if (dev-devtype-product == CODA_7541) { + coda_write(dev, CODA7_USE_BIT_ENABLE | CODA7_USE_HOST_BIT_ENABLE | + CODA7_USE_ME_ENABLE | CODA7_USE_HOST_ME_ENABLE, + CODA7_REG_BIT_AXI_SRAM_USE); + } + coda_command_async(ctx, CODA_COMMAND_PIC_RUN); } @@ -941,8 +952,10 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) CODA7_STREAM_BUF_PIC_RESET, CODA_REG_BIT_STREAM_CTRL); } - /* Configure the coda */ - coda_write(dev, 0x4c00, CODA_REG_BIT_SEARCH_RAM_BASE_ADDR); + if (dev-devtype-product == CODA_DX6) { + /* Configure the coda */ + coda_write(dev, dev-iram_paddr, CODADX6_REG_BIT_SEARCH_RAM_BASE_ADDR); + } /* Could set rotation here if needed */ switch (dev-devtype-product) { @@ -1017,7 +1030,12 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) value = (FMO_SLICE_SAVE_BUF_SIZE 7); value |= (0 CODA_FMOPARAM_TYPE_MASK) CODA_FMOPARAM_TYPE_OFFSET; value |= 0 CODA_FMOPARAM_SLICENUM_MASK; - coda_write(dev, value, CODA_CMD_ENC_SEQ_FMO); + if (dev-devtype-product == CODA_DX6) { + coda_write(dev, value, CODADX6_CMD_ENC_SEQ_FMO); + } else { + coda_write(dev, dev-iram_paddr, CODA7_CMD_ENC_SEQ_SEARCH_BASE); + coda_write(dev, 48 * 1024, CODA7_CMD_ENC_SEQ_SEARCH_SIZE); + } } if (coda_command_sync(ctx, CODA_COMMAND_SEQ_INIT)) { @@ -1047,7 +1065,15 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) } coda_write(dev, src_vq-num_buffers, CODA_CMD_SET_FRAME_BUF_NUM); - coda_write(dev, q_data_src-width, CODA_CMD_SET_FRAME_BUF_STRIDE); + coda_write(dev, round_up(q_data_src-width, 8), CODA_CMD_SET_FRAME_BUF_STRIDE); + if (dev-devtype-product != CODA_DX6) { +
Re: [PATCH] Support for Asus MyCinema U3100Mini Plus
On 09/10/2012 12:58 PM, Oliver Schinagl wrote: Changed the address as recommended, which after reading 7bit and 8bit addressing makes perfect sense (drop the r/w bit and get the actual address). static struct fc2580_config af9035_fc2580_config = { - .i2c_addr = 0xac, + .i2c_addr = 0x56, .clock = 16384000, }; So now the address should actually be correct ;) Unfortunately, nothing. What other debug options do I need to enable besides CONFIG_DVB_USB_DEBUG to get more interesting output? For me it sees something happens as there is no I2C error seen anymore. AF9035 driver uses Kernel dynamic debugs. CONFIG_DVB_USB_DEBUG is legacy and proprietary DVB subsystem debug which should not be used anymore. You could order dynamic debugs like that: modprobe dvb_usb_af9035; echo -n 'module dvb_usb_af9035 +p' /sys/kernel/debug/dynamic_debug/control For tuner, demod and dvb_usbv2 similarly if needed. Anyway, dmesg reports the following. [60.071538] usb 1-3: new high-speed USB device number 3 using ehci_hcd [60.192627] usb 1-3: New USB device found, idVendor=0b05, idProduct=1779 [60.192638] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [60.192646] usb 1-3: Product: AF9035A USB Device [60.192652] usb 1-3: Manufacturer: Afa Technologies Inc. [60.192657] usb 1-3: SerialNumber: AF010asdfasdf12314 [60.198686] input: Afa Technologies Inc. AF9035A USB Device as /devices/pci:00/:00:12.2/usb1/1-3/1-3:1.1/input/input14 [60.198832] hid-generic 0003:0B05:1779.0003: input: USB HID v1.01 Keyboard [Afa Technologies Inc. AF9035A USB Device] on usb-:00:12.2-3/input1 [60.263893] usbcore: registered new interface driver dvb_usb_af9035 [60.264605] usb 1-3: dvb_usbv2: found a 'Asus U3100Mini Plus' in cold state [60.273924] usb 1-3: dvb_usbv2: downloading firmware from file 'dvb-usb-af9035-02.fw' [60.584267] dvb_usb_af9035: firmware version=11.5.9.0 [60.584287] usb 1-3: dvb_usbv2: found a 'Asus U3100Mini Plus' in warm state [60.586802] usb 1-3: dvb_usbv2: will pass the complete MPEG2 transport stream to the software demuxer [60.586871] DVB: registering new adapter (Asus U3100Mini Plus) [60.595637] af9033: firmware version: LINK=11.5.9.0 OFDM=5.17.9.1 [60.595654] usb 1-3: DVB: registering adapter 0 frontend 0 (Afatech AF9033 (DVB-T))... [60.599889] usb 1-3: dvb_usbv2: 'Asus U3100Mini Plus' error while loading driver (-19) I then tried using the firmware that came with said driver, as the version seems slightly different/newer. #define FW_RELEASE_VERSION v8_8_63_0 #define DVB_LL_VERSION1 11 #define DVB_LL_VERSION2 22 #define DVB_LL_VERSION3 12 #define DVB_LL_VERSION4 0 #define DVB_OFDM_VERSION1 5 #define DVB_OFDM_VERSION2 66 #define DVB_OFDM_VERSION3 12 #define DVB_OFDM_VERSION4 0 (which also gets displayed when loading the firmware, originally on the old kernel). This however results in a hard lock/dump when plugging in the device. Are there certain size restrictions etc? What I did to obtain said firmware was write a simple program that reads the array, static unsigned char Firmware_codes[] and outputs the read bytes to a file. From what I saw from the -02 firmware, the first few bytes are identical (header?) so should be right procedure. Firmare surely works but you make some mistake. I have extracted those from the windows driver. http://palosaari.fi/linux/v4l-dvb/firmware/af9035/ Btw, when using the -02 firmware and trying to unload the af9033 module, either with or without the stick plugged in, it just hangs there for a long time. Reboot fails too (it hangs at trying to disable swap). Only a sys-req-reisub successfully reboots. oliver Antti On 09/10/12 00:29, Antti Palosaari wrote: On 09/10/2012 01:26 AM, Oliver Schinagl wrote: On 09/09/12 23:51, Antti Palosaari wrote: On 09/09/2012 11:49 PM, Oliver Schinagl wrote: Hi All/Antti, I used Antti's previous patch to try to get some support in for the Asus MyCinema U3100Mini Plus as it uses a supported driver (af9035) and now supported tuner (FCI FC2580). It compiles fine and almost works :( Here's what I get, which I have no idea what causes it. dmesg output: [ 380.677434] usb 1-3: New USB device found, idVendor=0b05, idProduct=1779 [ 380.677445] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 380.677452] usb 1-3: Product: AF9035A USB Device [ 380.677458] usb 1-3: Manufacturer: Afa Technologies Inc. [ 380.677463] usb 1-3: SerialNumber: AF01020abcdef12301 [ 380.683361] input: Afa Technologies Inc. AF9035A USB Device as /devices/pci:00/:00:12.2/usb1/1-3/1-3:1.1/input/input15 [ 380.683505] hid-generic 0003:0B05:1779.0004: input: USB HID v1.01 Keyboard [Afa Technologies Inc. AF9035A USB Device] on usb-:00:12.2-3/input1 [ 380.703807] usbcore: registered new interface driver dvb_usb_af9035 [ 380.704553] usb 1-3: dvb_usbv2: found a 'Asus U3100Mini Plus' in cold state [ 380.705075] usb 1-3: dvb_usbv2: downloading firmware from file 'dvb-usb-af9035-02.fw' [ 381.014996]
[PATCH] media: v4l2-ctrl: add a helper fucntion to modify the menu
From: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Rob Landley r...@landley.net --- Documentation/video4linux/v4l2-controls.txt | 26 + drivers/media/v4l2-core/v4l2-ctrls.c| 28 +++ include/media/v4l2-ctrls.h | 14 + 3 files changed, 68 insertions(+), 0 deletions(-) diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt index 43da22b..54a9539 100644 --- a/Documentation/video4linux/v4l2-controls.txt +++ b/Documentation/video4linux/v4l2-controls.txt @@ -196,6 +196,32 @@ the error code at the end. Saves a lot of repetitive error checking. It is recommended to add controls in ascending control ID order: it will be a bit faster that way. +2.1) Changing the menu of a standard control: +There are suitations when the control is standard but the menu items may be +device specific, in such cases the framework provides the helper to do that. + +struct v4l2_ctrl * v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, + const char * const *qmenu, s32 min, s32 max, + u32 menu_skip_mask, s32 def); + +This helper, function is used to modify the menu, min, max, mask and +the default value to set. +Example for usage: + static const char * const test_pattern[] = { + Disabled, + Vertical Bars, + Vertical Bars, + Solid Black, + Solid White, + NULL + }; + struct v4l2_ctrl *test_pattern_ctrl = + v4l2_ctrl_new_std_menu(foo-ctrl_handler, foo_ctrl_ops, + V4L2_CID_TEST_PATTERN, V4L2_TEST_PATTERN_DISABLED, 0, + V4L2_TEST_PATTERN_DISABLED); + + v4l2_ctrl_modify_menu(test_pattern_ctrl, test_pattern, 0, 5, 0x3, 0); + 3) Optionally force initial control setup: v4l2_ctrl_handler_setup(foo-ctrl_handler); diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index d731422..ac0fb28 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -2666,3 +2666,31 @@ unsigned int v4l2_ctrl_poll(struct file *file, struct poll_table_struct *wait) return 0; } EXPORT_SYMBOL(v4l2_ctrl_poll); + +/* Helper function for standard menu controls to modify the menu */ +struct v4l2_ctrl *v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, +const char * const *qmenu, s32 min, +s32 max, u32 menu_skip_mask, s32 def) +{ + if (ctrl-type != V4L2_CTRL_TYPE_MENU) + return NULL; + + if (qmenu == NULL) + return NULL; + + /* Determine if it is standard menu control */ + if (!v4l2_ctrl_get_menu(ctrl-id)) + return NULL; + + if ((def max) || (max min)) + return NULL; + + ctrl-qmenu = qmenu; + ctrl-minimum = min; + ctrl-maximum = max; + ctrl-menu_skip_mask = menu_skip_mask; + ctrl-cur.val = ctrl-val = ctrl-default_value = def; + + return ctrl; +} +EXPORT_SYMBOL(v4l2_ctrl_modify_menu); diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index 776605f..5b0ea04 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -488,6 +488,20 @@ static inline void v4l2_ctrl_unlock(struct v4l2_ctrl *ctrl) mutex_unlock(ctrl-handler-lock); } +/** + * v4l2_ctrl_modify_menu() - Modify the menu. This function is used when the + * control is standard but the menu is specific to device. + * @ctrl: The control to change the menu. + * @qmenu: The menu to which control will point to. + * @min: Minimum value of the control. + * @max: Maximum valur of the control. + * @menu_skip_mask:The control's skip mask for menu controls. + * @def: The default value for control. + */ +struct v4l2_ctrl *v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, +const char * const *qmenu, s32 min, +s32 max, u32 menu_skip_mask, s32 def); + /** v4l2_ctrl_g_ctrl() - Helper function to get the control's value from within a driver. * @ctrl: The control. * -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6] media: v4l2-ctrls: add control for dpcm predictor
From: Lad, Prabhakar prabhakar@ti.com add V4L2_CID_DPCM_PREDICTOR control of type menu, which determines the dpcm predictor. The predictor can be either simple or advanced. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Acked-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Sakari Ailus sakari.ai...@iki.fi Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Rob Landley r...@landley.net --- This patches has one checkpatch warning for line over 80 characters altough it can be avoided I have kept it for consistency. Changes for v6: 1: Fitted the description within 80 characters per line, pointed by Sakari. Changes for v5: 1: Changed the control's name to 'Simple' and 'Advanced' as pointed by Sakari. 2: Changed the description of DPCM. Thanks to Sakari for providing the description. Changes for v4: 1: Aligned the description to fit appropriately in the para tag, pointed by Sylwester. Changes for v3: 1: Added better explanation for DPCM, pointed by Hans. Changes for v2: 1: Added documentaion in controls.xml pointed by Sylwester. 2: Chnaged V4L2_DPCM_PREDICTOR_ADVANCE to V4L2_DPCM_PREDICTOR_ADVANCED pointed by Sakari. Documentation/DocBook/media/v4l/controls.xml | 48 +- drivers/media/v4l2-core/v4l2-ctrls.c |9 + include/linux/videodev2.h|5 +++ 3 files changed, 61 insertions(+), 1 deletions(-) diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index 93b9c68..f0fb08d 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -4267,7 +4267,53 @@ interface and may change in the future./para pixels / second. /entry /row - rowentry/entry/row + row + entry spanname=id + constantV4L2_CID_DPCM_PREDICTOR/constant + /entry + entrymenu/entry + /row + row id=v4l2-dpcm-predictor + entry spanname=descr Differential pulse-code modulation (DPCM) + compression can be used to compress the samples into fewer bits + than they would otherwise require. This is done by calculating the + difference between consecutive samples and outputting the + difference which in average is much smaller than the values of the + samples themselves since there is generally lots of correlation + between adjacent pixels. In decompression the original samples are + reconstructed. The process isn't lossless as the encoded sample + size in bits is less than the original. + + paraFormats using DPCM compression include + xref linkend=pixfmt-srggb10dpcm8 /./para + + paraThis control is used to select the predictor used to encode + the samples./para + + paraThe main difference between the simple and the advanced + predictors is image quality, with advanced predictor supposed to + produce better quality images as a result. Simple predictor can be + used e.g. for testing purposes. For more information about DPCM see + ulink url= + http://en.wikipedia.org/wiki/Differential_pulse-code_modulation; + Wikipedia/ulink./para + /entry + /row + row + entrytbl spanname=descr cols=2 + tbody valign=top + row +entryconstantV4L2_DPCM_PREDICTOR_SIMPLE/constant/entry + entryPredictor type is simple/entry + /row + row + entryconstantV4L2_DPCM_PREDICTOR_ADVANCED/constant/entry + entryPredictor type is advanced/entry + /row + /tbody + /entrytbl + /row + rowentry/entry/row /tbody /tgroup /table diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index b6a2ee7..8f2f40b 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -425,6 +425,11 @@ const char * const *v4l2_ctrl_get_menu(u32 id) Gray, NULL, }; + static const char * const dpcm_predictor[] = { + Simple, + Advanced, + NULL, + }; switch (id) { case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ: @@ -502,6 +507,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id) return mpeg4_profile; case V4L2_CID_JPEG_CHROMA_SUBSAMPLING: return jpeg_chroma_subsampling; + case V4L2_CID_DPCM_PREDICTOR: + return dpcm_predictor;
Re: [PATCH] media: v4l2-ctrl: add a helper fucntion to modify the menu
On Mon September 10 2012 13:57:36 Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Rob Landley r...@landley.net --- Documentation/video4linux/v4l2-controls.txt | 26 + drivers/media/v4l2-core/v4l2-ctrls.c| 28 +++ include/media/v4l2-ctrls.h | 14 + 3 files changed, 68 insertions(+), 0 deletions(-) diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt index 43da22b..54a9539 100644 --- a/Documentation/video4linux/v4l2-controls.txt +++ b/Documentation/video4linux/v4l2-controls.txt @@ -196,6 +196,32 @@ the error code at the end. Saves a lot of repetitive error checking. It is recommended to add controls in ascending control ID order: it will be a bit faster that way. +2.1) Changing the menu of a standard control: +There are suitations when the control is standard but the menu items may be +device specific, in such cases the framework provides the helper to do that. The Menu Controls section in this file would be a better place to add this. + +struct v4l2_ctrl * v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, + const char * const *qmenu, s32 min, s32 max, + u32 menu_skip_mask, s32 def); + +This helper, function is used to modify the menu, min, max, mask and +the default value to set. +Example for usage: + static const char * const test_pattern[] = { + Disabled, + Vertical Bars, + Vertical Bars, + Solid Black, + Solid White, + NULL + }; + struct v4l2_ctrl *test_pattern_ctrl = + v4l2_ctrl_new_std_menu(foo-ctrl_handler, foo_ctrl_ops, + V4L2_CID_TEST_PATTERN, V4L2_TEST_PATTERN_DISABLED, 0, + V4L2_TEST_PATTERN_DISABLED); + + v4l2_ctrl_modify_menu(test_pattern_ctrl, test_pattern, 0, 5, 0x3, 0); + 3) Optionally force initial control setup: v4l2_ctrl_handler_setup(foo-ctrl_handler); diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index d731422..ac0fb28 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -2666,3 +2666,31 @@ unsigned int v4l2_ctrl_poll(struct file *file, struct poll_table_struct *wait) return 0; } EXPORT_SYMBOL(v4l2_ctrl_poll); + +/* Helper function for standard menu controls to modify the menu */ +struct v4l2_ctrl *v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, + const char * const *qmenu, s32 min, The minimum is always 0 for menu controls, so this argument should be removed. It's not part of v4l2_ctrl_new_std_menu either. There is no need to return the pointer here, so just set the return to void. + s32 max, u32 menu_skip_mask, s32 def) +{ + if (ctrl-type != V4L2_CTRL_TYPE_MENU) + return NULL; + + if (qmenu == NULL) + return NULL; Change this to: if (WARN_ON(ctrl-type != V4L2_CTRL_TYPE_MENU || qmenu == NULL)) return; + + /* Determine if it is standard menu control */ + if (!v4l2_ctrl_get_menu(ctrl-id)) + return NULL; This test can be removed. Why should we prohibit calling this function for non-standard controls? + + if ((def max) || (max min)) + return NULL; Also replace with if (WARN_ON(def 0 || def max)). All these errors are driver errors and so should just report it in the kernel log and then bail out. + + ctrl-qmenu = qmenu; + ctrl-minimum = min; No need to set minimum. It's already 0. + ctrl-maximum = max; + ctrl-menu_skip_mask = menu_skip_mask; + ctrl-cur.val = ctrl-val = ctrl-default_value = def; + + return ctrl; +} +EXPORT_SYMBOL(v4l2_ctrl_modify_menu); diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index 776605f..5b0ea04 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -488,6 +488,20 @@ static inline void v4l2_ctrl_unlock(struct v4l2_ctrl *ctrl) mutex_unlock(ctrl-handler-lock); } +/** + * v4l2_ctrl_modify_menu() - Modify the menu. This function is used when the + * control is standard but the menu is specific to device. + * @ctrl:The control to change the menu. + * @qmenu: The menu to which control will point to. + * @min: Minimum value of the control. + * @max:
Re: [PATCH] media: v4l2-ctrl: add a helper fucntion to modify the menu
Hi Hans, Thanks for the review. On Monday 10 September 2012 05:43 PM, Hans Verkuil wrote: On Mon September 10 2012 13:57:36 Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Rob Landley r...@landley.net --- Documentation/video4linux/v4l2-controls.txt | 26 + drivers/media/v4l2-core/v4l2-ctrls.c| 28 +++ include/media/v4l2-ctrls.h | 14 + 3 files changed, 68 insertions(+), 0 deletions(-) diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt index 43da22b..54a9539 100644 --- a/Documentation/video4linux/v4l2-controls.txt +++ b/Documentation/video4linux/v4l2-controls.txt @@ -196,6 +196,32 @@ the error code at the end. Saves a lot of repetitive error checking. It is recommended to add controls in ascending control ID order: it will be a bit faster that way. +2.1) Changing the menu of a standard control: +There are suitations when the control is standard but the menu items may be +device specific, in such cases the framework provides the helper to do that. The Menu Controls section in this file would be a better place to add this. Ok. + +struct v4l2_ctrl * v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, +const char * const *qmenu, s32 min, s32 max, +u32 menu_skip_mask, s32 def); + +This helper, function is used to modify the menu, min, max, mask and +the default value to set. +Example for usage: +static const char * const test_pattern[] = { +Disabled, +Vertical Bars, +Vertical Bars, +Solid Black, +Solid White, +NULL +}; +struct v4l2_ctrl *test_pattern_ctrl = +v4l2_ctrl_new_std_menu(foo-ctrl_handler, foo_ctrl_ops, +V4L2_CID_TEST_PATTERN, V4L2_TEST_PATTERN_DISABLED, 0, +V4L2_TEST_PATTERN_DISABLED); + +v4l2_ctrl_modify_menu(test_pattern_ctrl, test_pattern, 0, 5, 0x3, 0); + 3) Optionally force initial control setup: v4l2_ctrl_handler_setup(foo-ctrl_handler); diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index d731422..ac0fb28 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -2666,3 +2666,31 @@ unsigned int v4l2_ctrl_poll(struct file *file, struct poll_table_struct *wait) return 0; } EXPORT_SYMBOL(v4l2_ctrl_poll); + +/* Helper function for standard menu controls to modify the menu */ +struct v4l2_ctrl *v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, + const char * const *qmenu, s32 min, The minimum is always 0 for menu controls, so this argument should be removed. It's not part of v4l2_ctrl_new_std_menu either. There is no need to return the pointer here, so just set the return to void. Ok. + s32 max, u32 menu_skip_mask, s32 def) +{ +if (ctrl-type != V4L2_CTRL_TYPE_MENU) +return NULL; + +if (qmenu == NULL) +return NULL; Change this to: if (WARN_ON(ctrl-type != V4L2_CTRL_TYPE_MENU || qmenu == NULL)) return; Ok. + +/* Determine if it is standard menu control */ +if (!v4l2_ctrl_get_menu(ctrl-id)) +return NULL; This test can be removed. Why should we prohibit calling this function for non-standard controls? Any how how if its custom control, the menu will be provided by user itself, so why would he fell the necessity of this function ? But providing an option wont harm I'll remove it. + +if ((def max) || (max min)) +return NULL; Also replace with if (WARN_ON(def 0 || def max)). All these errors are driver errors and so should just report it in the kernel log and then bail out. Ok. + +ctrl-qmenu = qmenu; +ctrl-minimum = min; No need to set minimum. It's already 0. Ok. Regards, --Prabhakar Lad +ctrl-maximum = max; +ctrl-menu_skip_mask = menu_skip_mask; +ctrl-cur.val = ctrl-val = ctrl-default_value = def; + +return ctrl; +} +EXPORT_SYMBOL(v4l2_ctrl_modify_menu); diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index 776605f..5b0ea04 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -488,6 +488,20 @@ static inline void v4l2_ctrl_unlock(struct v4l2_ctrl *ctrl) mutex_unlock(ctrl-handler-lock); } +/** + *
Re: [PATCH] media: v4l2-ctrl: add a helper fucntion to modify the menu
On Monday 10 September 2012 06:06 PM, Manjunath Hadli wrote: Hi Hans, Thanks for the review. On Monday 10 September 2012 05:43 PM, Hans Verkuil wrote: On Mon September 10 2012 13:57:36 Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Mauro Carvalho Chehab mche...@infradead.org Cc: Hans de Goede hdego...@redhat.com Cc: Kyungmin Park kyungmin.p...@samsung.com Cc: Rob Landley r...@landley.net --- Documentation/video4linux/v4l2-controls.txt | 26 + drivers/media/v4l2-core/v4l2-ctrls.c| 28 +++ include/media/v4l2-ctrls.h | 14 + 3 files changed, 68 insertions(+), 0 deletions(-) diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt index 43da22b..54a9539 100644 --- a/Documentation/video4linux/v4l2-controls.txt +++ b/Documentation/video4linux/v4l2-controls.txt @@ -196,6 +196,32 @@ the error code at the end. Saves a lot of repetitive error checking. It is recommended to add controls in ascending control ID order: it will be a bit faster that way. +2.1) Changing the menu of a standard control: +There are suitations when the control is standard but the menu items may be +device specific, in such cases the framework provides the helper to do that. The Menu Controls section in this file would be a better place to add this. Ok. + +struct v4l2_ctrl * v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, + const char * const *qmenu, s32 min, s32 max, + u32 menu_skip_mask, s32 def); + +This helper, function is used to modify the menu, min, max, mask and +the default value to set. +Example for usage: + static const char * const test_pattern[] = { + Disabled, + Vertical Bars, + Vertical Bars, + Solid Black, + Solid White, + NULL + }; + struct v4l2_ctrl *test_pattern_ctrl = + v4l2_ctrl_new_std_menu(foo-ctrl_handler, foo_ctrl_ops, + V4L2_CID_TEST_PATTERN, V4L2_TEST_PATTERN_DISABLED, 0, + V4L2_TEST_PATTERN_DISABLED); + + v4l2_ctrl_modify_menu(test_pattern_ctrl, test_pattern, 0, 5, 0x3, 0); + 3) Optionally force initial control setup: v4l2_ctrl_handler_setup(foo-ctrl_handler); diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index d731422..ac0fb28 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -2666,3 +2666,31 @@ unsigned int v4l2_ctrl_poll(struct file *file, struct poll_table_struct *wait) return 0; } EXPORT_SYMBOL(v4l2_ctrl_poll); + +/* Helper function for standard menu controls to modify the menu */ +struct v4l2_ctrl *v4l2_ctrl_modify_menu(struct v4l2_ctrl *ctrl, +const char * const *qmenu, s32 min, The minimum is always 0 for menu controls, so this argument should be removed. It's not part of v4l2_ctrl_new_std_menu either. There is no need to return the pointer here, so just set the return to void. Ok. +s32 max, u32 menu_skip_mask, s32 def) +{ + if (ctrl-type != V4L2_CTRL_TYPE_MENU) + return NULL; + + if (qmenu == NULL) + return NULL; Change this to: if (WARN_ON(ctrl-type != V4L2_CTRL_TYPE_MENU || qmenu == NULL)) return; Ok. + + /* Determine if it is standard menu control */ + if (!v4l2_ctrl_get_menu(ctrl-id)) + return NULL; This test can be removed. Why should we prohibit calling this function for non-standard controls? Any how how if its custom control, the menu will be provided by user itself, so why would he fell the necessity of this function ? But providing an option wont harm I'll remove it. + + if ((def max) || (max min)) + return NULL; Also replace with if (WARN_ON(def 0 || def max)). All these errors are driver errors and so should just report it in the kernel log and then bail out. Ok. + + ctrl-qmenu = qmenu; + ctrl-minimum = min; No need to set minimum. It's already 0. Ok. Regards, --Prabhakar Lad My Bad :(. Replied with Manju's ID. Thanks, --Prabhakar + ctrl-maximum = max; + ctrl-menu_skip_mask = menu_skip_mask; + ctrl-cur.val = ctrl-val = ctrl-default_value = def; + + return ctrl; +} +EXPORT_SYMBOL(v4l2_ctrl_modify_menu); diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index 776605f..5b0ea04 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -488,6 +488,20 @@ static inline void
Re: [PATCH v2] drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c: fix error return code
You're replacing kmemdup for kstrdup, which is great, but that's not anywhere in the commit message. Sorry for that. I'm not sure if you should re-send, but you should definitely try to have better commit messages in the future! I'll kindly ask to ignore the V2 of this patch. I'll send other patch to be applied after the V1. The second patch will replace kmemdup for kstrdup. Please ignore the patch: http://patchwork.linuxtv.org/patch/14237/ Not to mention you're doing two things in one patch, and that makes very difficult to bisect. This is really bad thing to do in a single patch. Sorry for that too. Thanks (and sorry for the nitpick)... Thanks! Ezequiel. -- Peter -- 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] drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c: Replace kmemdup for kstrdup
From: Peter Senna Tschudin peter.se...@gmail.com Replace kmemdup for kstrdup and cleaning up the code. Signed-off-by: Peter Senna Tschudin peter.se...@gmail.com --- It depends on the patch http://patchwork.linuxtv.org/patch/14231/ tmp/cx25821-video-upstream-ch2.c | 27 +-- 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c index 273df94..b663dac 100644 --- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c +++ b/tmp/cx25821-video-upstream-ch2.c @@ -707,7 +707,6 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev, int channel_select, int err = 0; int data_frame_size = 0; int risc_buffer_size = 0; - int str_length = 0; if (dev-_is_running_ch2) { pr_info(Video Channel is still running so return!\n); @@ -743,24 +742,16 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev, int channel_select, risc_buffer_size = dev-_isNTSC_ch2 ? NTSC_RISC_BUF_SIZE : PAL_RISC_BUF_SIZE; - if (dev-input_filename_ch2) { - str_length = strlen(dev-input_filename_ch2); - dev-_filename_ch2 = kmemdup(dev-input_filename_ch2, -str_length + 1, GFP_KERNEL); - - if (!dev-_filename_ch2) { - err = -ENOENT; - goto error; - } - } else { - str_length = strlen(dev-_defaultname_ch2); - dev-_filename_ch2 = kmemdup(dev-_defaultname_ch2, -str_length + 1, GFP_KERNEL); + if (dev-input_filename_ch2) + dev-_filename_ch2 = kstrdup(dev-input_filename_ch2, + GFP_KERNEL); + else + dev-_filename_ch2 = kstrdup(dev-_defaultname_ch2, + GFP_KERNEL); - if (!dev-_filename_ch2) { - err = -ENOENT; - goto error; - } + if (!dev-_filename_ch2) { + err = -ENOENT; + goto error; } /* Default if filename is empty string */ -- 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 3/5] dvb_frontend: do not allow statistic IOCTLs when sleeping
Em 15-08-2012 21:28, Antti Palosaari escreveu: Demodulator cannot perform statistic IOCTLs when it is not tuned. Return -EAGAIN in such case. Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/dvb-core/dvb_frontend.c | 34 +- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c index 2bc80b1..7d079fb 100644 --- a/drivers/media/dvb-core/dvb_frontend.c +++ b/drivers/media/dvb-core/dvb_frontend.c @@ -2132,27 +2132,43 @@ static int dvb_frontend_ioctl_legacy(struct file *file, err = fe-ops.read_status(fe, status); break; } + case FE_READ_BER: - if (fe-ops.read_ber) - err = fe-ops.read_ber(fe, (__u32*) parg); + if (fe-ops.read_ber) { + if (fepriv-thread) + err = fe-ops.read_ber(fe, (__u32 *) parg); + else + err = -EAGAIN; + } break; case FE_READ_SIGNAL_STRENGTH: - if (fe-ops.read_signal_strength) - err = fe-ops.read_signal_strength(fe, (__u16*) parg); + if (fe-ops.read_signal_strength) { + if (fepriv-thread) + err = fe-ops.read_signal_strength(fe, (__u16 *) parg); + else + err = -EAGAIN; + } break; This one doesn't look right, as the frontend can be able to get the signal strength at the analog part (afaik, most DVB-S frontends do that). Also, some drivers just map it to the tuner RF strength. The proper approach for it is to break signal strength into two different statistics: - analog RF strength; - signal strength at the demod, after having demod locked. It makes sense to return -EAGAIN for the second case, but doing it for the first case is bad, as the RF strength can be used on DVB-S devices, in order to fine-adjust the antenna position. 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: [PATCH] Support for Asus MyCinema U3100Mini Plus
On 10-09-12 13:46, Antti Palosaari wrote: On 09/10/2012 12:58 PM, Oliver Schinagl wrote: Changed the address as recommended, which after reading 7bit and 8bit addressing makes perfect sense (drop the r/w bit and get the actual address). static struct fc2580_config af9035_fc2580_config = { - .i2c_addr = 0xac, + .i2c_addr = 0x56, .clock = 16384000, }; So now the address should actually be correct ;) Unfortunately, nothing. What other debug options do I need to enable besides CONFIG_DVB_USB_DEBUG to get more interesting output? For me it sees something happens as there is no I2C error seen anymore. AF9035 driver uses Kernel dynamic debugs. CONFIG_DVB_USB_DEBUG is legacy and proprietary DVB subsystem debug which should not be used anymore. You could order dynamic debugs like that: modprobe dvb_usb_af9035; echo -n 'module dvb_usb_af9035 +p' /sys/kernel/debug/dynamic_debug/control For tuner, demod and dvb_usbv2 similarly if needed. I've did and added output from control and dmesg output. I don't exactly know how to read the dynamic debug output, the only thing that jumped out at me, was: drivers/media/dvb-frontends/af9033.c:327 [af9033]af9033_init =p %s: unsupported tuner ID=%d\012 So I will search and see where in the driver the supported tunerID's are stored and fix that. Any other pointers/things you see I should look at? Anyway, dmesg reports the following. [60.071538] usb 1-3: new high-speed USB device number 3 using ehci_hcd [60.192627] usb 1-3: New USB device found, idVendor=0b05, idProduct=1779 [60.192638] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [60.192646] usb 1-3: Product: AF9035A USB Device [60.192652] usb 1-3: Manufacturer: Afa Technologies Inc. [60.192657] usb 1-3: SerialNumber: AF010asdfasdf12314 [60.198686] input: Afa Technologies Inc. AF9035A USB Device as /devices/pci:00/:00:12.2/usb1/1-3/1-3:1.1/input/input14 [60.198832] hid-generic 0003:0B05:1779.0003: input: USB HID v1.01 Keyboard [Afa Technologies Inc. AF9035A USB Device] on usb-:00:12.2-3/input1 [60.263893] usbcore: registered new interface driver dvb_usb_af9035 [60.264605] usb 1-3: dvb_usbv2: found a 'Asus U3100Mini Plus' in cold state [60.273924] usb 1-3: dvb_usbv2: downloading firmware from file 'dvb-usb-af9035-02.fw' [60.584267] dvb_usb_af9035: firmware version=11.5.9.0 [60.584287] usb 1-3: dvb_usbv2: found a 'Asus U3100Mini Plus' in warm state [60.586802] usb 1-3: dvb_usbv2: will pass the complete MPEG2 transport stream to the software demuxer [60.586871] DVB: registering new adapter (Asus U3100Mini Plus) [60.595637] af9033: firmware version: LINK=11.5.9.0 OFDM=5.17.9.1 [60.595654] usb 1-3: DVB: registering adapter 0 frontend 0 (Afatech AF9033 (DVB-T))... [60.599889] usb 1-3: dvb_usbv2: 'Asus U3100Mini Plus' error while loading driver (-19) I then tried using the firmware that came with said driver, as the version seems slightly different/newer. #define FW_RELEASE_VERSION v8_8_63_0 #define DVB_LL_VERSION1 11 #define DVB_LL_VERSION2 22 #define DVB_LL_VERSION3 12 #define DVB_LL_VERSION4 0 #define DVB_OFDM_VERSION1 5 #define DVB_OFDM_VERSION2 66 #define DVB_OFDM_VERSION3 12 #define DVB_OFDM_VERSION4 0 (which also gets displayed when loading the firmware, originally on the old kernel). This however results in a hard lock/dump when plugging in the device. Are there certain size restrictions etc? What I did to obtain said firmware was write a simple program that reads the array, static unsigned char Firmware_codes[] and outputs the read bytes to a file. From what I saw from the -02 firmware, the first few bytes are identical (header?) so should be right procedure. Firmare surely works but you make some mistake. I have extracted those from the windows driver. http://palosaari.fi/linux/v4l-dvb/firmware/af9035/ A link to, or your files should be listed at the linuxdvb firmware download page ;) I noticed your latest firmware is way newer then the one I had. So deffinatly using that one. Btw, when using the -02 firmware and trying to unload the af9033 module, either with or without the stick plugged in, it just hangs there for a long time. Reboot fails too (it hangs at trying to disable swap). Only a sys-req-reisub successfully reboots. oliver Antti Oliver On 09/10/12 00:29, Antti Palosaari wrote: On 09/10/2012 01:26 AM, Oliver Schinagl wrote: On 09/09/12 23:51, Antti Palosaari wrote: On 09/09/2012 11:49 PM, Oliver Schinagl wrote: Hi All/Antti, I used Antti's previous patch to try to get some support in for the Asus MyCinema U3100Mini Plus as it uses a supported driver (af9035) and now supported tuner (FCI FC2580). It compiles fine and almost works :( Here's what I get, which I have no idea what causes it. dmesg output: [ 380.677434] usb 1-3: New USB device found, idVendor=0b05, idProduct=1779 [ 380.677445] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 380.677452] usb 1-3: Product: AF9035A
Re: [PATCH 4/5] DocBook: update ioctl error codes
Em 15-08-2012 21:28, Antti Palosaari escreveu: ENOTTY is now returned for unimplemented ioctl by dvb-frontend. Old EOPNOTSUPP ENOSYS could be still returned by some drivers as well as other non standard error codes. EAGAIN is returned in case of device is in state where it cannot perform requested operation. This is for example sleep and statistics are queried. Quick check for few demodulator drivers reveals there is a lot of different error codes used in such case currently, few to mention still: EOPNOTSUPP, ENOSYS, EAGAIN ... Lets try harmonize. Signed-off-by: Antti Palosaari cr...@iki.fi --- Documentation/DocBook/media/v4l/gen-errors.xml | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Documentation/DocBook/media/v4l/gen-errors.xml b/Documentation/DocBook/media/v4l/gen-errors.xml index 5bbf3ce..737ecaa 100644 --- a/Documentation/DocBook/media/v4l/gen-errors.xml +++ b/Documentation/DocBook/media/v4l/gen-errors.xml @@ -7,6 +7,13 @@ tbody valign=top !-- Keep it ordered alphabetically -- row + entryEAGAIN/entry + entryThe ioctl can't be handled because the device is in state where +it can't perform it. This could happen for example in case where +device is sleeping and ioctl is performed to query statistics. While the comments here actually depend on your new version of patch 3/5, returning -EAGAIN is a valid return code, already used (there are 155 occurrences of it right now). So, I'll apply it. + /entry + /row + row entryEBADF/entry entryThe file descriptor is not a valid./entry /row @@ -51,11 +58,6 @@ for periodic transfers (up to 80% of the USB bandwidth)./entry /row row - entryENOSYS or EOPNOTSUPP/entry - entryFunction not available for this device (dvb API only. Will likely -be replaced anytime soon by ENOTTY)./entry - /row - row entryEPERM/entry entryPermission denied. Can be returned if the device needs write permission, or some special capabilities is needed 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: [PATCH 3/5] dvb_frontend: do not allow statistic IOCTLs when sleeping
On 09/10/2012 05:27 PM, Mauro Carvalho Chehab wrote: Em 15-08-2012 21:28, Antti Palosaari escreveu: Demodulator cannot perform statistic IOCTLs when it is not tuned. Return -EAGAIN in such case. Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/dvb-core/dvb_frontend.c | 34 +- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c index 2bc80b1..7d079fb 100644 --- a/drivers/media/dvb-core/dvb_frontend.c +++ b/drivers/media/dvb-core/dvb_frontend.c @@ -2132,27 +2132,43 @@ static int dvb_frontend_ioctl_legacy(struct file *file, err = fe-ops.read_status(fe, status); break; } + case FE_READ_BER: - if (fe-ops.read_ber) - err = fe-ops.read_ber(fe, (__u32*) parg); + if (fe-ops.read_ber) { + if (fepriv-thread) + err = fe-ops.read_ber(fe, (__u32 *) parg); + else + err = -EAGAIN; + } break; case FE_READ_SIGNAL_STRENGTH: - if (fe-ops.read_signal_strength) - err = fe-ops.read_signal_strength(fe, (__u16*) parg); + if (fe-ops.read_signal_strength) { + if (fepriv-thread) + err = fe-ops.read_signal_strength(fe, (__u16 *) parg); + else + err = -EAGAIN; + } break; This one doesn't look right, as the frontend can be able to get the signal strength at the analog part (afaik, most DVB-S frontends do that). Also, some drivers just map it to the tuner RF strength. The proper approach for it is to break signal strength into two different statistics: - analog RF strength; - signal strength at the demod, after having demod locked. It makes sense to return -EAGAIN for the second case, but doing it for the first case is bad, as the RF strength can be used on DVB-S devices, in order to fine-adjust the antenna position. I have to say I don't understand what you mean. That one is DVB frontend callback and it is not called in case of analog. Could you provide some example? Frontend thread is running always when frontend is opened. It is hard for me to see why frontend FE_READ_SIGNAL_STRENGTH should be allowed call even frontend is closed. OK, I will try to grep sources to see what you mean. regards Antti -- 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
Re: (still) NULL pointer crashes with nuvoton_cir driver
Em 17-08-2012 12:04, Luis Henriques escreveu: (Adding Mauro to CC has he is the maintainer) I'm actually waiting for Jarod's comments on it, as he wrote this driver and the similar ones. Jarod? Regards, Mauro On Thu, Aug 16, 2012 at 10:09:32AM +0200, Matthijs Kooijman wrote: Hi folks, I'm currently compiling a 3.5 kernel with just the rdev initialization moved up to see if this will fix my problem at all, but I'd like your view on this in the meantime as well. Ok, this seems to fix my problem: --- a/drivers/media/rc/nuvoton-cir.c +++ b/drivers/media/rc/nuvoton-cir.c @@ -1066,6 +1066,7 @@ /* tx bits */ rdev-tx_resolution = XYZ; #endif + nvt-rdev = rdev; This makes sense to me. Note however that there are more drivers with a similar problem (e.g., fintek-cir.c). ret = -EBUSY; /* now claim resources */ @@ -1090,7 +1091,6 @@ goto failure5; device_init_wakeup(pdev-dev, true); - nvt-rdev = rdev; nvt_pr(KERN_NOTICE, driver has been successfully loaded\n); if (debug) { cir_dump_regs(nvt); I'm still not sure if the rc_register_device shouldn't also be moved up. It seems this doesn't trigger a problem right now, but if there is a problem, I suspect its trigger window is a lot smaller than with the rdev initialization problem... I'm not sure as well, I'm not very familiar with this code. However, it looks like the IRQ request should actually be one of the last things to do here. Cheers, -- Luis -- 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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v6] media: add v4l2 subdev driver for S5K4ECGX sensor
Hi, On 09/09/2012 06:01 PM, Francesco Lavra wrote: +static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) +{ +const struct firmware *fw; +int err, i, regs_num; +struct i2c_client *client = v4l2_get_subdevdata(sd); +u16 val; +u32 addr, crc, crc_file, addr_inc = 0; + +err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev); +if (err) { +v4l2_err(sd, Failed to read firmware %s\n, S5K4ECGX_FIRMWARE); +return err; +} +regs_num = *(u32 *)(fw-data); +v4l2_dbg(3, debug, sd, FW: %s size %d register sets %d\n, + S5K4ECGX_FIRMWARE, fw-size, regs_num); +regs_num++; /* Add header */ +if (fw-size != regs_num * FW_RECORD_SIZE + FW_CRC_SIZE) { +err = -EINVAL; +goto fw_out; +} +crc_file = *(u32 *)(fw-data + regs_num * FW_RECORD_SIZE); Depending on the value of regs_num, this may result in unaligned access Thanks for the catch. I think it is not the only place where unaligned issues are possible. Since the data records are 4-byte address + 2-byte value there is also an issue with reading the address entries. Assuming fw-data is aligned to at least 2-bytes (not quite sure if we can assume that) there should be no problem with reading 2-byte register values. We could change the data types of the register values from u16 to u32, wasting some memory (there is approximately 3 000 records), so there is no other data types in the file structure than u32. Or use a patch as below. Not sure what's better. 8- From a970480b99bdb74e2bf48e1a321724231e6516a0 Mon Sep 17 00:00:00 2001 From: Sylwester Nawrocki sylvester.nawro...@gmail.com Date: Sun, 9 Sep 2012 19:56:31 +0200 Subject: [PATCH] s5k4ecgx: Fix unaligned access issues Signed-off-by: Sylwester Nawrocki sylvester.nawro...@gmail.com --- drivers/media/i2c/s5k4ecgx.c | 16 1 files changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c index 0ef0b7d..4c6439a 100644 --- a/drivers/media/i2c/s5k4ecgx.c +++ b/drivers/media/i2c/s5k4ecgx.c @@ -24,6 +24,7 @@ #include linux/module.h #include linux/regulator/consumer.h #include linux/slab.h +#include asm/unaligned.h #include media/media-entity.h #include media/s5k4ecgx.h @@ -331,6 +332,7 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) const struct firmware *fw; int err, i, regs_num; u32 addr, crc, crc_file, addr_inc = 0; + const u8 *ptr; u16 val; err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev); @@ -338,7 +340,7 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) v4l2_err(sd, Failed to read firmware %s\n, S5K4ECGX_FIRMWARE); return err; } - regs_num = le32_to_cpu(*(u32 *)fw-data); + regs_num = le32_to_cpu(get_unaligned((__le32 *)fw-data)); v4l2_dbg(3, debug, sd, FW: %s size %d register sets %d\n, S5K4ECGX_FIRMWARE, fw-size, regs_num); @@ -349,7 +351,8 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) goto fw_out; } - crc_file = *(u32 *)(fw-data + regs_num * FW_RECORD_SIZE); + memcpy(crc_file, fw-data + regs_num * FW_RECORD_SIZE, sizeof(u32)); + crc = crc32_le(~0, fw-data, regs_num * FW_RECORD_SIZE); if (crc != crc_file) { v4l2_err(sd, FW: invalid crc (%#x:%#x)\n, crc, crc_file); @@ -357,9 +360,14 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) goto fw_out; } + ptr = fw-data + FW_RECORD_SIZE; + for (i = 1; i regs_num; i++) { - addr = le32_to_cpu(*(u32 *)(fw-data + i * FW_RECORD_SIZE)); - val = le16_to_cpu(*(u16 *)(fw-data + i * FW_RECORD_SIZE + 4)); + addr = le32_to_cpu(get_unaligned((__le32 *)ptr)); + ptr += 4; + val = le16_to_cpu(*(__le16 *)ptr); + ptr += FW_RECORD_SIZE; + if (addr - addr_inc != 2) err = s5k4ecgx_write(client, addr, val); else -- 1.7.2.5 8 -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC API] Capture Overlay API ambiguities
Hi all, While working on making bttv compliant with the V4L2 API I managed to resolve all v4l2-compliance errors except for two: fail: v4l2-test-formats.cpp(339): !fmt.width || !fmt.height test VIDIOC_G_FBUF: FAIL fail: v4l2-test-formats.cpp(607): Video Overlay is valid, but no S_FMT was implemented test VIDIOC_S_FMT: FAIL After some analysis it turns out to be an ambiguity in VIDIOC_G_FBUF and VIDIOC_G_FMT for overlays. The first relates to what VIDIOC_G_FBUF should return if there is no framebuffer set? I.e. if VIDIOC_S_FBUF was never called? Should G_FBUF return an error, or provide non-zero but dummy struct v4l2_pix_format values and only leave base to NULL, or should the whole v4l2_framebuffer structure be zeroed (except for the capability field)? bttv just zeroes everything. Currently v4l2-compliance assumes that there is at least a dummy fmt setup. But I think that is unreasonably for destructive capture overlays. I would prefer to zero everything except for the capability field. Returning an error is not an option IMHO: even if there is no framebuffer defined, the capability field is still useful information to have. The second error is related to the first: what should happen if you try to set a capture overlay format with S_FMT and no framebuffer is defined? The driver cannot really validate the given format without knowing what the framebuffer is like. Currently I am returning some dummy values for G_FMT and TRY_FMT in that case, but for S_FMT I cannot do even that. While normally the G/S/TRY_FMT ioctls should never return an error, I think that in this particular case we should allow an error code. I am proposing ENODATA because, well, there is no data w.r.t. the framebuffer. The current bttv driver returns EINVAL for these cases, but that's too generic I think. So in the case of destructive capture overlays the G/S/TRY_FMT ioctls should return ENODATA if no framebuffer was configured. Comments? 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
[PATCH v4 0/16] Initial i.MX5/CODA7 support for the CODA driver
These patches contain initial firmware loading and encoding support for the CODA7 series VPU contained in i.MX51 and i.MX53 SoCs, and fix some multi-instance issues. Patches 13 and 14, touching files in arch/arm/*, are included for illustration purposes. Changes since v3: - Remove iram_vaddr from struct coda_dev, and use temporary void __iomem *iram_vaddr instead. - Rename framebuffers array to internal_frames to avoid confusion with v4l2/vb2 frame buffers. - Add struct coda_dev *dev pointer in coda_start_streaming. - Call cancel_delayed_work in coda_stop_streaming and in coda_irq_handler. - Lock the device mutex in coda_timeout to avoid a race with coda_release. - Complete dev-done in coda_timeout. regards Philipp --- arch/arm/boot/dts/imx51.dtsi|6 + arch/arm/boot/dts/imx53.dtsi|6 + arch/arm/mach-imx/clk-imx51-imx53.c |4 +- drivers/media/platform/Kconfig |3 +- drivers/media/platform/coda.c | 433 +-- drivers/media/platform/coda.h | 33 ++- 6 files changed, 358 insertions(+), 127 deletions(-) -- 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 v4 01/16] media: coda: firmware loading for 64-bit AXI bus width
Add support for loading a raw firmware with 16-bit chars ordered in little-endian 64-bit words, corresponding to the memory access pattern of CODA7 and above: When writing the boot code into the code download register, the chars have to be reordered back. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/coda.c | 34 ++ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 6908514..d4a5dd0 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -1510,7 +1510,7 @@ static char *coda_product_name(int product) } } -static int coda_hw_init(struct coda_dev *dev, const struct firmware *fw) +static int coda_hw_init(struct coda_dev *dev) { u16 product, major, minor, release; u32 data; @@ -1520,21 +1520,27 @@ static int coda_hw_init(struct coda_dev *dev, const struct firmware *fw) clk_prepare_enable(dev-clk_per); clk_prepare_enable(dev-clk_ahb); - /* Copy the whole firmware image to the code buffer */ - memcpy(dev-codebuf.vaddr, fw-data, fw-size); /* * Copy the first CODA_ISRAM_SIZE in the internal SRAM. -* This memory seems to be big-endian here, which is weird, since -* the internal ARM processor of the coda is little endian. +* The 16-bit chars in the code buffer are in memory access +* order, re-sort them to CODA order for register download. * Data in this SRAM survives a reboot. */ - p = (u16 *)fw-data; - for (i = 0; i (CODA_ISRAM_SIZE / 2); i++) { - data = CODA_DOWN_ADDRESS_SET(i) | - CODA_DOWN_DATA_SET(p[i ^ 1]); - coda_write(dev, data, CODA_REG_BIT_CODE_DOWN); + p = (u16 *)dev-codebuf.vaddr; + if (dev-devtype-product == CODA_DX6) { + for (i = 0; i (CODA_ISRAM_SIZE / 2); i++) { + data = CODA_DOWN_ADDRESS_SET(i) | + CODA_DOWN_DATA_SET(p[i ^ 1]); + coda_write(dev, data, CODA_REG_BIT_CODE_DOWN); + } + } else { + for (i = 0; i (CODA_ISRAM_SIZE / 2); i++) { + data = CODA_DOWN_ADDRESS_SET(i) | + CODA_DOWN_DATA_SET(p[round_down(i, 4) + + 3 - (i % 4)]); + coda_write(dev, data, CODA_REG_BIT_CODE_DOWN); + } } - release_firmware(fw); /* Tell the BIT where to find everything it needs */ coda_write(dev, dev-workbuf.paddr, @@ -1630,7 +1636,11 @@ static void coda_fw_callback(const struct firmware *fw, void *context) return; } - ret = coda_hw_init(dev, fw); + /* Copy the whole firmware image to the code buffer */ + memcpy(dev-codebuf.vaddr, fw-data, fw-size); + release_firmware(fw); + + ret = coda_hw_init(dev); if (ret) { v4l2_err(dev-v4l2_dev, HW initialization failed\n); return; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 03/16] media: coda: fix IRAM/AXI handling for i.MX53
This uses the ARCH_MXC specific iram_alloc API to allocate a work buffer in the SoC's on-chip SRAM and sets up the AXI_SRAM_USE register. In the future, the allocation will be converted to use the genalloc API. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- Changes since v3: - Remove iram_vaddr from struct coda_dev, and use temporary void __iomem *iram_vaddr instead. --- drivers/media/platform/Kconfig |3 ++- drivers/media/platform/coda.c | 52 drivers/media/platform/coda.h | 21 3 files changed, 67 insertions(+), 9 deletions(-) diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index d4c034d..76f9a8f 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -130,9 +130,10 @@ if V4L_MEM2MEM_DRIVERS config VIDEO_CODA tristate ChipsMedia Coda multi-standard codec IP - depends on VIDEO_DEV VIDEO_V4L2 + depends on VIDEO_DEV VIDEO_V4L2 ARCH_MXC select VIDEOBUF2_DMA_CONTIG select V4L2_MEM2MEM_DEV + select IRAM_ALLOC if SOC_IMX53 ---help--- Coda is a range of video codec IPs that supports H.264, MPEG-4, and other video formats. diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 8ec2ff4..5f4bd7c 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -24,6 +24,7 @@ #include linux/videodev2.h #include linux/of.h +#include mach/iram.h #include media/v4l2-ctrls.h #include media/v4l2-device.h #include media/v4l2-ioctl.h @@ -42,6 +43,7 @@ #define CODA7_WORK_BUF_SIZE(512 * 1024 + CODA_FMO_BUF_SIZE * 8 * 1024) #define CODA_PARA_BUF_SIZE (10 * 1024) #define CODA_ISRAM_SIZE(2048 * 2) +#define CODA7_IRAM_SIZE0x14000 /* 81920 bytes */ #define CODA_OUTPUT_BUFS 4 #define CODA_CAPTURE_BUFS 2 @@ -127,6 +129,7 @@ struct coda_dev { struct coda_aux_buf codebuf; struct coda_aux_buf workbuf; + long unsigned int iram_paddr; spinlock_t irqlock; struct mutexdev_mutex; @@ -710,6 +713,13 @@ static void coda_device_run(void *m2m_priv) coda_write(dev, pic_stream_buffer_addr, CODA_CMD_ENC_PIC_BB_START); coda_write(dev, pic_stream_buffer_size / 1024, CODA_CMD_ENC_PIC_BB_SIZE); + + if (dev-devtype-product == CODA_7541) { + coda_write(dev, CODA7_USE_BIT_ENABLE | CODA7_USE_HOST_BIT_ENABLE | + CODA7_USE_ME_ENABLE | CODA7_USE_HOST_ME_ENABLE, + CODA7_REG_BIT_AXI_SRAM_USE); + } + coda_command_async(ctx, CODA_COMMAND_PIC_RUN); } @@ -941,8 +951,10 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) CODA7_STREAM_BUF_PIC_RESET, CODA_REG_BIT_STREAM_CTRL); } - /* Configure the coda */ - coda_write(dev, 0x4c00, CODA_REG_BIT_SEARCH_RAM_BASE_ADDR); + if (dev-devtype-product == CODA_DX6) { + /* Configure the coda */ + coda_write(dev, dev-iram_paddr, CODADX6_REG_BIT_SEARCH_RAM_BASE_ADDR); + } /* Could set rotation here if needed */ switch (dev-devtype-product) { @@ -1017,7 +1029,12 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) value = (FMO_SLICE_SAVE_BUF_SIZE 7); value |= (0 CODA_FMOPARAM_TYPE_MASK) CODA_FMOPARAM_TYPE_OFFSET; value |= 0 CODA_FMOPARAM_SLICENUM_MASK; - coda_write(dev, value, CODA_CMD_ENC_SEQ_FMO); + if (dev-devtype-product == CODA_DX6) { + coda_write(dev, value, CODADX6_CMD_ENC_SEQ_FMO); + } else { + coda_write(dev, dev-iram_paddr, CODA7_CMD_ENC_SEQ_SEARCH_BASE); + coda_write(dev, 48 * 1024, CODA7_CMD_ENC_SEQ_SEARCH_SIZE); + } } if (coda_command_sync(ctx, CODA_COMMAND_SEQ_INIT)) { @@ -1047,7 +1064,15 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) } coda_write(dev, src_vq-num_buffers, CODA_CMD_SET_FRAME_BUF_NUM); - coda_write(dev, q_data_src-width, CODA_CMD_SET_FRAME_BUF_STRIDE); + coda_write(dev, round_up(q_data_src-width, 8), CODA_CMD_SET_FRAME_BUF_STRIDE); + if (dev-devtype-product != CODA_DX6) { + coda_write(dev, round_up(q_data_src-width, 8), CODA7_CMD_SET_FRAME_SOURCE_BUF_STRIDE); + coda_write(dev, dev-iram_paddr + 48 * 1024, CODA7_CMD_SET_FRAME_AXI_DBKY_ADDR); + coda_write(dev, dev-iram_paddr + 53 * 1024, CODA7_CMD_SET_FRAME_AXI_DBKC_ADDR); + coda_write(dev, dev-iram_paddr + 58 * 1024, CODA7_CMD_SET_FRAME_AXI_BIT_ADDR); + coda_write(dev, dev-iram_paddr + 68 * 1024, CODA7_CMD_SET_FRAME_AXI_IPACDC_ADDR); +
[PATCH v4 04/16] media: coda: allocate internal framebuffers separately from v4l2 buffers
Some codecs running on CODA need internal framebuffers for reference and reconstructed frames. Allocate them separately, and do not use the input vb2_buffers: those will be handed off to userspace regularly, and there is no way to signal to the CODA which of the registered framebuffers are off limits. As a consequence, userspace is now free to choose the number of v4l2 buffers. This patch also includes the code to set up the parameter buffer for CODA7 and above with 64-bit AXI bus width. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- Changes since v3: - Rename framebuffers array to internal_frames to avoid confusion with v4l2/vb2 frame buffers. --- drivers/media/platform/coda.c | 141 + 1 file changed, 86 insertions(+), 55 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 5f4bd7c..f4b4a6f 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -45,8 +45,7 @@ #define CODA_ISRAM_SIZE(2048 * 2) #define CODA7_IRAM_SIZE0x14000 /* 81920 bytes */ -#define CODA_OUTPUT_BUFS 4 -#define CODA_CAPTURE_BUFS 2 +#define CODA_MAX_FRAMEBUFFERS 2 #define MAX_W 720 #define MAX_H 576 @@ -164,11 +163,12 @@ struct coda_ctx { struct v4l2_m2m_ctx *m2m_ctx; struct v4l2_ctrl_handlerctrls; struct v4l2_fh fh; - struct vb2_buffer *reference; int gopcounter; charvpu_header[3][64]; int vpu_header_size[3]; struct coda_aux_buf parabuf; + struct coda_aux_buf internal_frames[CODA_MAX_FRAMEBUFFERS]; + int num_internal_frames; int idx; }; @@ -738,14 +738,6 @@ static int coda_job_ready(void *m2m_priv) return 0; } - /* For P frames a reference picture is needed too */ - if ((ctx-gopcounter != (ctx-params.gop_size - 1)) - !ctx-reference) { - v4l2_dbg(1, coda_debug, ctx-dev-v4l2_dev, -not ready: reference picture not available.\n); - return 0; - } - if (coda_isbusy(ctx-dev)) { v4l2_dbg(1, coda_debug, ctx-dev-v4l2_dev, not ready: coda is still busy.\n); @@ -799,7 +791,6 @@ static void set_default_params(struct coda_ctx *ctx) ctx-params.codec_mode = CODA_MODE_INVALID; ctx-colorspace = V4L2_COLORSPACE_REC709; ctx-params.framerate = 30; - ctx-reference = NULL; ctx-aborting = 0; /* Default formats for output and input queues */ @@ -825,7 +816,6 @@ static int coda_queue_setup(struct vb2_queue *vq, unsigned int size; if (vq-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { - *nbuffers = CODA_OUTPUT_BUFS; if (fmt) size = fmt-fmt.pix.width * fmt-fmt.pix.height * 3 / 2; @@ -833,7 +823,6 @@ static int coda_queue_setup(struct vb2_queue *vq, size = MAX_W * MAX_H * 3 / 2; } else { - *nbuffers = CODA_CAPTURE_BUFS; size = CODA_MAX_FRAME_SIZE; } @@ -886,6 +875,77 @@ static void coda_wait_finish(struct vb2_queue *q) coda_lock(ctx); } +static void coda_free_framebuffers(struct coda_ctx *ctx) +{ + int i; + + for (i = 0; i CODA_MAX_FRAMEBUFFERS; i++) { + if (ctx-internal_frames[i].vaddr) { + dma_free_coherent(ctx-dev-plat_dev-dev, + ctx-internal_frames[i].size, + ctx-internal_frames[i].vaddr, + ctx-internal_frames[i].paddr); + ctx-internal_frames[i].vaddr = NULL; + } + } +} + +static int coda_alloc_framebuffers(struct coda_ctx *ctx, struct coda_q_data *q_data, u32 fourcc) +{ + struct coda_dev *dev = ctx-dev; + + int height = q_data-height; + int width = q_data-width; + u32 *p; + int i; + + /* Allocate frame buffers */ + ctx-num_internal_frames = CODA_MAX_FRAMEBUFFERS; + for (i = 0; i ctx-num_internal_frames; i++) { + ctx-internal_frames[i].size = q_data-sizeimage; + if (fourcc == V4L2_PIX_FMT_H264 dev-devtype-product != CODA_DX6) + ctx-internal_frames[i].size += width / 2 * height / 2; + ctx-internal_frames[i].vaddr = dma_alloc_coherent( + dev-plat_dev-dev, ctx-internal_frames[i].size, + ctx-internal_frames[i].paddr, GFP_KERNEL); + if (!ctx-internal_frames[i].vaddr) { + coda_free_framebuffers(ctx);
[PATCH v4 05/16] media: coda: ignore coda busy status in coda_job_ready
job_ready is supposed to signal whether a context is ready to be added to the job queue, not whether the CODA is ready to run it immediately. Calling v4l2_m2m_job_finish at the end of coda_irq_handler already guarantees that the coda is ready when v4l2-mem2mem eventually tries to run the next queued job. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/coda.c |6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index f4b4a6f..d069787 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -738,12 +738,6 @@ static int coda_job_ready(void *m2m_priv) return 0; } - if (coda_isbusy(ctx-dev)) { - v4l2_dbg(1, coda_debug, ctx-dev-v4l2_dev, -not ready: coda is still busy.\n); - return 0; - } - v4l2_dbg(1, coda_debug, ctx-dev-v4l2_dev, job ready\n); return 1; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 10/16] media: coda: fix sizeimage setting in try_fmt
VIDIOC_TRY_FMT would incorrectly return bytesperline * height, instead of width * height * 3 / 2. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/coda.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index fe8a397..e8ed427 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -407,8 +407,8 @@ static int vidioc_try_fmt(struct coda_dev *dev, struct v4l2_format *f) W_ALIGN, f-fmt.pix.height, MIN_H, MAX_H, H_ALIGN, S_ALIGN); f-fmt.pix.bytesperline = round_up(f-fmt.pix.width, 2); - f-fmt.pix.sizeimage = f-fmt.pix.height * - f-fmt.pix.bytesperline; + f-fmt.pix.sizeimage = f-fmt.pix.width * + f-fmt.pix.height * 3 / 2; } else { /*encoded formats h.264/mpeg4 */ f-fmt.pix.bytesperline = 0; f-fmt.pix.sizeimage = CODA_MAX_FRAME_SIZE; @@ -492,11 +492,7 @@ static int vidioc_s_fmt(struct coda_ctx *ctx, struct v4l2_format *f) q_data-fmt = find_format(ctx-dev, f); q_data-width = f-fmt.pix.width; q_data-height = f-fmt.pix.height; - if (q_data-fmt-fourcc == V4L2_PIX_FMT_YUV420) { - q_data-sizeimage = q_data-width * q_data-height * 3 / 2; - } else { /* encoded format h.264/mpeg-4 */ - q_data-sizeimage = CODA_MAX_FRAME_SIZE; - } + q_data-sizeimage = f-fmt.pix.sizeimage; v4l2_dbg(1, coda_debug, ctx-dev-v4l2_dev, Setting format for type %d, wxh: %dx%d, fmt: %d\n, -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 16/16] media: coda: support 1024 px height on CODA7, set max frame size to 1080p
Increases the maximum encoded frame buffer size to 1 MiB. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/coda.c | 11 +-- drivers/media/platform/coda.h |3 ++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 4c3e100..defab64 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -47,16 +47,14 @@ #define CODA_MAX_FRAMEBUFFERS 2 -#define MAX_W 720 -#define MAX_H 576 -#define CODA_MAX_FRAME_SIZE0x9 +#define MAX_W 1920 +#define MAX_H 1080 +#define CODA_MAX_FRAME_SIZE0x10 #define FMO_SLICE_SAVE_BUF_SIZE (32) #define CODA_DEFAULT_GAMMA 4096 #define MIN_W 176 #define MIN_H 144 -#define MAX_W 720 -#define MAX_H 576 #define S_ALIGN1 /* multiple of 2 */ #define W_ALIGN1 /* multiple of 2 */ @@ -1016,11 +1014,12 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) switch (dev-devtype-product) { case CODA_DX6: value = (q_data_src-width CODADX6_PICWIDTH_MASK) CODADX6_PICWIDTH_OFFSET; + value |= (q_data_src-height CODADX6_PICHEIGHT_MASK) CODA_PICHEIGHT_OFFSET; break; default: value = (q_data_src-width CODA7_PICWIDTH_MASK) CODA7_PICWIDTH_OFFSET; + value |= (q_data_src-height CODA7_PICHEIGHT_MASK) CODA_PICHEIGHT_OFFSET; } - value |= (q_data_src-height CODA_PICHEIGHT_MASK) CODA_PICHEIGHT_OFFSET; coda_write(dev, value, CODA_CMD_ENC_SEQ_SRC_SIZE); coda_write(dev, ctx-params.framerate, CODA_CMD_ENC_SEQ_SRC_F_RATE); diff --git a/drivers/media/platform/coda.h b/drivers/media/platform/coda.h index f3f5e43..60338c3 100644 --- a/drivers/media/platform/coda.h +++ b/drivers/media/platform/coda.h @@ -117,7 +117,8 @@ #defineCODADX6_PICWIDTH_OFFSET 10 #defineCODADX6_PICWIDTH_MASK 0x3ff #defineCODA_PICHEIGHT_OFFSET 0 -#defineCODA_PICHEIGHT_MASK 0x3ff +#defineCODADX6_PICHEIGHT_MASK 0x3ff +#defineCODA7_PICHEIGHT_MASK0x #define CODA_CMD_ENC_SEQ_SRC_F_RATE0x194 #define CODA_CMD_ENC_SEQ_MP4_PARA 0x198 #defineCODA_MP4PARAM_VERID_OFFSET 6 -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 02/16] media: coda: add i.MX53 / CODA7541 platform support
Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/coda.c | 31 +++ 1 file changed, 31 insertions(+) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index d4a5dd0..8ec2ff4 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -84,6 +84,7 @@ enum coda_inst_type { enum coda_product { CODA_DX6 = 0xf001, + CODA_7541 = 0xf012, }; struct coda_fmt { @@ -261,6 +262,24 @@ static struct coda_fmt codadx6_formats[] = { }, }; +static struct coda_fmt coda7_formats[] = { + { + .name = YUV 4:2:0 Planar, + .fourcc = V4L2_PIX_FMT_YUV420, + .type = CODA_FMT_RAW, + }, + { + .name = H264 Encoded Stream, + .fourcc = V4L2_PIX_FMT_H264, + .type = CODA_FMT_ENC, + }, + { + .name = MPEG4 Encoded Stream, + .fourcc = V4L2_PIX_FMT_MPEG4, + .type = CODA_FMT_ENC, + }, +}; + static struct coda_fmt *find_format(struct coda_dev *dev, struct v4l2_format *f) { struct coda_fmt *formats = dev-devtype-formats; @@ -1485,6 +1504,7 @@ static irqreturn_t coda_irq_handler(int irq, void *data) static u32 coda_supported_firmwares[] = { CODA_FIRMWARE_VERNUM(CODA_DX6, 2, 2, 5), + CODA_FIRMWARE_VERNUM(CODA_7541, 13, 4, 29), }; static bool coda_firmware_supported(u32 vernum) @@ -1504,6 +1524,8 @@ static char *coda_product_name(int product) switch (product) { case CODA_DX6: return CodaDx6; + case CODA_7541: + return CODA7541; default: snprintf(buf, sizeof(buf), (0x%04x), product); return buf; @@ -1695,6 +1717,7 @@ static int coda_firmware_request(struct coda_dev *dev) enum coda_platform { CODA_IMX27, + CODA_IMX53, }; static const struct coda_devtype coda_devdata[] = { @@ -1704,10 +1727,17 @@ static const struct coda_devtype coda_devdata[] = { .formats = codadx6_formats, .num_formats = ARRAY_SIZE(codadx6_formats), }, + [CODA_IMX53] = { + .firmware= v4l-coda7541-imx53.bin, + .product = CODA_7541, + .formats = coda7_formats, + .num_formats = ARRAY_SIZE(coda7_formats), + }, }; static struct platform_device_id coda_platform_ids[] = { { .name = coda-imx27, .driver_data = CODA_IMX27 }, + { .name = coda-imx53, .driver_data = CODA_7541 }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(platform, coda_platform_ids); @@ -1715,6 +1745,7 @@ MODULE_DEVICE_TABLE(platform, coda_platform_ids); #ifdef CONFIG_OF static const struct of_device_id coda_dt_ids[] = { { .compatible = fsl,imx27-vpu, .data = coda_platform_ids[CODA_IMX27] }, + { .compatible = fsl,imx53-vpu, .data = coda_devdata[CODA_IMX53] }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, coda_dt_ids); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 15/16] media: coda: set up buffers to be sized as negotiated with s_fmt
This fixes a failure in vb2_qbuf in user pointer mode where __qbuf_userptr checks if the buffer queued by userspace is large enough. The failure would happen if coda_queue_setup was called with empty fmt (and thus set the expected buffer size to the maximum resolution), and userspace queues buffers of smaller size - corresponding to the negotiated dimensions - were queued. Explicitly setting sizeimage to the value negotiated via s_fmt fixes the issue. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/coda.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 863b96a..4c3e100 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -813,18 +813,11 @@ static int coda_queue_setup(struct vb2_queue *vq, unsigned int sizes[], void *alloc_ctxs[]) { struct coda_ctx *ctx = vb2_get_drv_priv(vq); + struct coda_q_data *q_data; unsigned int size; - if (vq-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { - if (fmt) - size = fmt-fmt.pix.width * - fmt-fmt.pix.height * 3 / 2; - else - size = MAX_W * - MAX_H * 3 / 2; - } else { - size = CODA_MAX_FRAME_SIZE; - } + q_data = get_q_data(ctx, vq-type); + size = q_data-sizeimage; *nplanes = 1; sizes[0] = size; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 14/16] ARM i.MX5: Add CODA7 to device tree for i.MX51 and i.MX53
Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- arch/arm/boot/dts/imx51.dtsi |6 ++ arch/arm/boot/dts/imx53.dtsi |6 ++ 2 files changed, 12 insertions(+) diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi index aba28dc..8f38d83 100644 --- a/arch/arm/boot/dts/imx51.dtsi +++ b/arch/arm/boot/dts/imx51.dtsi @@ -278,6 +278,12 @@ interrupts = 87; status = disabled; }; + + vpu@83ff4000 { + compatible = fsl,imx51-vpu; + reg = 0x83ff4000 0x1000; + interrupts = 9; + }; }; }; }; diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi index cd37165..4cf59e5 100644 --- a/arch/arm/boot/dts/imx53.dtsi +++ b/arch/arm/boot/dts/imx53.dtsi @@ -336,6 +336,12 @@ interrupts = 87; status = disabled; }; + + vpu@63ff4000 { + compatible = fsl,imx53-vpu; + reg = 0x63ff4000 0x1000; + interrupts = 9; + }; }; }; }; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 12/16] media: coda: add byte size slice limit control
Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/coda.c | 29 +++-- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 81e3401..863b96a 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -151,6 +151,7 @@ struct coda_params { enum v4l2_mpeg_video_multi_slice_mode slice_mode; u32 framerate; u16 bitrate; + u32 slice_max_bits; u32 slice_max_mb; }; @@ -1056,12 +1057,23 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) return -EINVAL; } - value = (ctx-params.slice_max_mb CODA_SLICING_SIZE_MASK) CODA_SLICING_SIZE_OFFSET; - value |= (1 CODA_SLICING_UNIT_MASK) CODA_SLICING_UNIT_OFFSET; - if (ctx-params.slice_mode == V4L2_MPEG_VIDEO_MULTI_SICE_MODE_MAX_MB) + switch (ctx-params.slice_mode) { + case V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_SINGLE: + value = 0; + break; + case V4L2_MPEG_VIDEO_MULTI_SICE_MODE_MAX_MB: + value = (ctx-params.slice_max_mb CODA_SLICING_SIZE_MASK) CODA_SLICING_SIZE_OFFSET; + value |= (1 CODA_SLICING_UNIT_MASK) CODA_SLICING_UNIT_OFFSET; + value |= 1 CODA_SLICING_MODE_MASK; + break; + case V4L2_MPEG_VIDEO_MULTI_SICE_MODE_MAX_BYTES: + value = (ctx-params.slice_max_bits CODA_SLICING_SIZE_MASK) CODA_SLICING_SIZE_OFFSET; + value |= (0 CODA_SLICING_UNIT_MASK) CODA_SLICING_UNIT_OFFSET; value |= 1 CODA_SLICING_MODE_MASK; + break; + } coda_write(dev, value, CODA_CMD_ENC_SEQ_SLICE_MODE); - value = ctx-params.gop_size CODA_GOP_SIZE_MASK; + value = ctx-params.gop_size CODA_GOP_SIZE_MASK; coda_write(dev, value, CODA_CMD_ENC_SEQ_GOP_SIZE); if (ctx-params.bitrate) { @@ -1308,6 +1320,9 @@ static int coda_s_ctrl(struct v4l2_ctrl *ctrl) case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MAX_MB: ctx-params.slice_max_mb = ctrl-val; break; + case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MAX_BYTES: + ctx-params.slice_max_bits = ctrl-val * 8; + break; case V4L2_CID_MPEG_VIDEO_HEADER_MODE: break; default: @@ -1346,10 +1361,12 @@ static int coda_ctrls_setup(struct coda_ctx *ctx) V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP, 1, 31, 1, 2); v4l2_ctrl_new_std_menu(ctx-ctrls, coda_ctrl_ops, V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE, - V4L2_MPEG_VIDEO_MULTI_SICE_MODE_MAX_MB, 0, - V4L2_MPEG_VIDEO_MULTI_SICE_MODE_MAX_MB); + V4L2_MPEG_VIDEO_MULTI_SICE_MODE_MAX_BYTES, 0x7, + V4L2_MPEG_VIDEO_MULTI_SLICE_MODE_SINGLE); v4l2_ctrl_new_std(ctx-ctrls, coda_ctrl_ops, V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MAX_MB, 1, 0x3fff, 1, 1); + v4l2_ctrl_new_std(ctx-ctrls, coda_ctrl_ops, + V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MAX_BYTES, 1, 0x3fff, 1, 500); v4l2_ctrl_new_std_menu(ctx-ctrls, coda_ctrl_ops, V4L2_CID_MPEG_VIDEO_HEADER_MODE, V4L2_MPEG_VIDEO_HEADER_MODE_JOINED_WITH_1ST_FRAME, -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 11/16] media: coda: add horizontal / vertical flipping support
The hardware can also rotate in 90° steps, but there is no corresponding V4L2_CID defined yet. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/coda.c | 19 ++- drivers/media/platform/coda.h |9 + 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index e8ed427..81e3401 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -141,6 +141,7 @@ struct coda_dev { }; struct coda_params { + u8 rot_mode; u8 h264_intra_qp; u8 h264_inter_qp; u8 mpeg4_intra_qp; @@ -695,7 +696,7 @@ static void coda_device_run(void *m2m_priv) } /* submit */ - coda_write(dev, 0, CODA_CMD_ENC_PIC_ROT_MODE); + coda_write(dev, CODA_ROT_MIR_ENABLE | ctx-params.rot_mode, CODA_CMD_ENC_PIC_ROT_MODE); coda_write(dev, quant_param, CODA_CMD_ENC_PIC_QS); @@ -1271,6 +1272,18 @@ static int coda_s_ctrl(struct v4l2_ctrl *ctrl) s_ctrl: id = %d, val = %d\n, ctrl-id, ctrl-val); switch (ctrl-id) { + case V4L2_CID_HFLIP: + if (ctrl-val) + ctx-params.rot_mode |= CODA_MIR_HOR; + else + ctx-params.rot_mode = ~CODA_MIR_HOR; + break; + case V4L2_CID_VFLIP: + if (ctrl-val) + ctx-params.rot_mode |= CODA_MIR_VER; + else + ctx-params.rot_mode = ~CODA_MIR_VER; + break; case V4L2_CID_MPEG_VIDEO_BITRATE: ctx-params.bitrate = ctrl-val / 1000; break; @@ -1316,6 +1329,10 @@ static int coda_ctrls_setup(struct coda_ctx *ctx) v4l2_ctrl_handler_init(ctx-ctrls, 9); v4l2_ctrl_new_std(ctx-ctrls, coda_ctrl_ops, + V4L2_CID_HFLIP, 0, 1, 1, 0); + v4l2_ctrl_new_std(ctx-ctrls, coda_ctrl_ops, + V4L2_CID_VFLIP, 0, 1, 1, 0); + v4l2_ctrl_new_std(ctx-ctrls, coda_ctrl_ops, V4L2_CID_MPEG_VIDEO_BITRATE, 0, 32767000, 1, 0); v4l2_ctrl_new_std(ctx-ctrls, coda_ctrl_ops, V4L2_CID_MPEG_VIDEO_GOP_SIZE, 1, 60, 1, 16); diff --git a/drivers/media/platform/coda.h b/drivers/media/platform/coda.h index 3324010..f3f5e43 100644 --- a/drivers/media/platform/coda.h +++ b/drivers/media/platform/coda.h @@ -188,6 +188,15 @@ #define CODA_CMD_ENC_PIC_SRC_ADDR_CR 0x188 #define CODA_CMD_ENC_PIC_QS0x18c #define CODA_CMD_ENC_PIC_ROT_MODE 0x190 +#defineCODA_ROT_MIR_ENABLE (1 4) +#defineCODA_ROT_0 (0x0 0) +#defineCODA_ROT_90 (0x1 0) +#defineCODA_ROT_180(0x2 0) +#defineCODA_ROT_270(0x3 0) +#defineCODA_MIR_NONE (0x0 2) +#defineCODA_MIR_VER(0x1 2) +#defineCODA_MIR_HOR(0x2 2) +#defineCODA_MIR_VER_HOR(0x3 2) #define CODA_CMD_ENC_PIC_OPTION0x194 #define CODA_CMD_ENC_PIC_BB_START 0x198 #define CODA_CMD_ENC_PIC_BB_SIZE 0x19c -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 13/16] ARM i.MX5: Fix CODA7 clock lookup for device tree on i.MX51 and i.MX53
Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- arch/arm/mach-imx/clk-imx51-imx53.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c index 4bdcaa9..64f9ceb 100644 --- a/arch/arm/mach-imx/clk-imx51-imx53.c +++ b/arch/arm/mach-imx/clk-imx51-imx53.c @@ -345,7 +345,7 @@ int __init mx51_clocks_init(unsigned long rate_ckil, unsigned long rate_osc, clk_register_clkdev(clk[hsi2c_gate], NULL, imx-i2c.2); clk_register_clkdev(clk[mx51_mipi], mipi_hsp, NULL); - clk_register_clkdev(clk[vpu_gate], NULL, imx51-vpu.0); + clk_register_clkdev(clk[vpu_gate], NULL, 83ff4000.vpu); clk_register_clkdev(clk[fec_gate], NULL, imx27-fec.0); clk_register_clkdev(clk[ipu_gate], bus, imx51-ipu); clk_register_clkdev(clk[ipu_di0_gate], di0, imx51-ipu); @@ -432,7 +432,7 @@ int __init mx53_clocks_init(unsigned long rate_ckil, unsigned long rate_osc, mx5_clocks_common_init(rate_ckil, rate_osc, rate_ckih1, rate_ckih2); - clk_register_clkdev(clk[vpu_gate], NULL, imx53-vpu.0); + clk_register_clkdev(clk[vpu_gate], NULL, 63ff4000.vpu); clk_register_clkdev(clk[i2c3_gate], NULL, imx-i2c.2); clk_register_clkdev(clk[fec_gate], NULL, imx25-fec.0); clk_register_clkdev(clk[ipu_gate], bus, imx53-ipu); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 09/16] media: coda: wait for picture run completion in start/stop_streaming
While the CODA is running a PIC_RUN command, its registers are not to be touched. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- Changes since v3: - Complete dev-done in coda_timeout. --- drivers/media/platform/coda.c | 42 - 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index e0595ce..fe8a397 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -137,6 +137,7 @@ struct coda_dev { struct list_headinstances; unsigned long instance_mask; struct delayed_work timeout; + struct completion done; }; struct coda_params { @@ -726,6 +727,7 @@ static void coda_device_run(void *m2m_priv) /* 1 second timeout in case CODA locks up */ schedule_delayed_work(dev-timeout, HZ); + INIT_COMPLETION(dev-done); coda_command_async(ctx, CODA_COMMAND_PIC_RUN); } @@ -970,6 +972,10 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) if (!(ctx-rawstreamon ctx-compstreamon)) return 0; + if (coda_isbusy(dev)) + if (wait_for_completion_interruptible_timeout(dev-done, HZ) = 0) + return -EBUSY; + ctx-gopcounter = ctx-params.gop_size - 1; q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT); @@ -1224,20 +1230,29 @@ static int coda_stop_streaming(struct vb2_queue *q) ctx-compstreamon = 0; } - if (!ctx-rawstreamon !ctx-compstreamon) { - cancel_delayed_work(dev-timeout); + /* Don't stop the coda unless both queues are off */ + if (ctx-rawstreamon || ctx-compstreamon) + return 0; - v4l2_dbg(1, coda_debug, ctx-dev-v4l2_dev, -%s: sent command 'SEQ_END' to coda\n, __func__); - if (coda_command_sync(ctx, CODA_COMMAND_SEQ_END)) { - v4l2_err(ctx-dev-v4l2_dev, -CODA_COMMAND_SEQ_END failed\n); - return -ETIMEDOUT; + if (coda_isbusy(dev)) { + if (wait_for_completion_interruptible_timeout(dev-done, HZ) = 0) { + v4l2_warn(dev-v4l2_dev, + %s: timeout, sending SEQ_END anyway\n, __func__); } + } + + cancel_delayed_work(dev-timeout); - coda_free_framebuffers(ctx); + v4l2_dbg(1, coda_debug, dev-v4l2_dev, +%s: sent command 'SEQ_END' to coda\n, __func__); + if (coda_command_sync(ctx, CODA_COMMAND_SEQ_END)) { + v4l2_err(dev-v4l2_dev, +CODA_COMMAND_SEQ_END failed\n); + return -ETIMEDOUT; } + coda_free_framebuffers(ctx); + return 0; } @@ -1524,6 +1539,8 @@ static irqreturn_t coda_irq_handler(int irq, void *data) return IRQ_NONE; } + complete(dev-done); + src_buf = v4l2_m2m_src_buf_remove(ctx-m2m_ctx); dst_buf = v4l2_m2m_dst_buf_remove(ctx-m2m_ctx); @@ -1579,6 +1596,11 @@ static void coda_timeout(struct work_struct *work) struct coda_dev *dev = container_of(to_delayed_work(work), struct coda_dev, timeout); + if (completion_done(dev-done)) + return; + + complete(dev-done); + v4l2_err(dev-v4l2_dev, CODA PIC_RUN timeout, stopping all streams\n); mutex_lock(dev-dev_mutex); @@ -1861,6 +1883,8 @@ static int __devinit coda_probe(struct platform_device *pdev) spin_lock_init(dev-irqlock); INIT_LIST_HEAD(dev-instances); INIT_DELAYED_WORK(dev-timeout, coda_timeout); + init_completion(dev-done); + complete(dev-done); dev-plat_dev = pdev; dev-clk_per = devm_clk_get(pdev-dev, per); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 06/16] media: coda: keep track of active instances
Determining the next free instance just by incrementing and decrementing an instance counter does not work: if there are two instances opened, 0 and 1, and instance 0 is released, the next call to coda_open will create a new instance with index 1, but instance 1 is already in use. Instead, scan a bitfield of active instances to determine the first free instance index. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/coda.c | 21 + 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index d069787..159df08 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -134,7 +134,8 @@ struct coda_dev { struct mutexdev_mutex; struct v4l2_m2m_dev *m2m_dev; struct vb2_alloc_ctx*alloc_ctx; - int instances; + struct list_headinstances; + unsigned long instance_mask; }; struct coda_params { @@ -152,6 +153,7 @@ struct coda_params { struct coda_ctx { struct coda_dev *dev; + struct list_headlist; int aborting; int rawstreamon; int compstreamon; @@ -1357,14 +1359,22 @@ static int coda_queue_init(void *priv, struct vb2_queue *src_vq, return vb2_queue_init(dst_vq); } +static int coda_next_free_instance(struct coda_dev *dev) +{ + return ffz(dev-instance_mask); +} + static int coda_open(struct file *file) { struct coda_dev *dev = video_drvdata(file); struct coda_ctx *ctx = NULL; int ret = 0; + int idx; - if (dev-instances = CODA_MAX_INSTANCES) + idx = coda_next_free_instance(dev); + if (idx = CODA_MAX_INSTANCES) return -EBUSY; + set_bit(idx, dev-instance_mask); ctx = kzalloc(sizeof *ctx, GFP_KERNEL); if (!ctx) @@ -1374,6 +1384,7 @@ static int coda_open(struct file *file) file-private_data = ctx-fh; v4l2_fh_add(ctx-fh); ctx-dev = dev; + ctx-idx = idx; set_default_params(ctx); ctx-m2m_ctx = v4l2_m2m_ctx_init(dev-m2m_dev, ctx, @@ -1402,7 +1413,7 @@ static int coda_open(struct file *file) } coda_lock(ctx); - ctx-idx = dev-instances++; + list_add(ctx-list, dev-instances); coda_unlock(ctx); clk_prepare_enable(dev-clk_per); @@ -1429,7 +1440,7 @@ static int coda_release(struct file *file) ctx); coda_lock(ctx); - dev-instances--; + list_del(ctx-list); coda_unlock(ctx); dma_free_coherent(dev-plat_dev-dev, CODA_PARA_BUF_SIZE, @@ -1440,6 +1451,7 @@ static int coda_release(struct file *file) clk_disable_unprepare(dev-clk_ahb); v4l2_fh_del(ctx-fh); v4l2_fh_exit(ctx-fh); + clear_bit(ctx-idx, dev-instance_mask); kfree(ctx); return 0; @@ -1822,6 +1834,7 @@ static int __devinit coda_probe(struct platform_device *pdev) } spin_lock_init(dev-irqlock); + INIT_LIST_HEAD(dev-instances); dev-plat_dev = pdev; dev-clk_per = devm_clk_get(pdev-dev, per); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 07/16] media: coda: stop all queues in case of lockup
Add a 1 second timeout for each PIC_RUN command to the CODA. In case it locks up, stop all queues and dequeue remaining buffers. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- Changes since v3: - Add struct coda_dev *dev pointer in coda_start_streaming. - Call cancel_delayed_work in coda_stop_streaming and in coda_irq_handler. - Lock the device mutex in coda_timeout to avoid a race with coda_release. --- drivers/media/platform/coda.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 159df08..7f6ec3a 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -136,6 +136,7 @@ struct coda_dev { struct vb2_alloc_ctx*alloc_ctx; struct list_headinstances; unsigned long instance_mask; + struct delayed_work timeout; }; struct coda_params { @@ -722,6 +723,9 @@ static void coda_device_run(void *m2m_priv) CODA7_REG_BIT_AXI_SRAM_USE); } + /* 1 second timeout in case CODA locks up */ + schedule_delayed_work(dev-timeout, HZ); + coda_command_async(ctx, CODA_COMMAND_PIC_RUN); } @@ -1208,6 +1212,7 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count) static int coda_stop_streaming(struct vb2_queue *q) { struct coda_ctx *ctx = vb2_get_drv_priv(q); + struct coda_dev *dev = ctx-dev; if (q-type == V4L2_BUF_TYPE_VIDEO_OUTPUT) { v4l2_dbg(1, coda_debug, ctx-dev-v4l2_dev, @@ -1220,6 +1225,8 @@ static int coda_stop_streaming(struct vb2_queue *q) } if (!ctx-rawstreamon !ctx-compstreamon) { + cancel_delayed_work(dev-timeout); + v4l2_dbg(1, coda_debug, ctx-dev-v4l2_dev, %s: sent command 'SEQ_END' to coda\n, __func__); if (coda_command_sync(ctx, CODA_COMMAND_SEQ_END)) { @@ -1492,6 +1499,8 @@ static irqreturn_t coda_irq_handler(int irq, void *data) u32 wr_ptr, start_ptr; struct coda_ctx *ctx; + __cancel_delayed_work(dev-timeout); + /* read status register to attend the IRQ */ coda_read(dev, CODA_REG_BIT_INT_STATUS); coda_write(dev, CODA_REG_BIT_INT_CLEAR_SET, @@ -1564,6 +1573,22 @@ static irqreturn_t coda_irq_handler(int irq, void *data) return IRQ_HANDLED; } +static void coda_timeout(struct work_struct *work) +{ + struct coda_ctx *ctx; + struct coda_dev *dev = container_of(to_delayed_work(work), + struct coda_dev, timeout); + + v4l2_err(dev-v4l2_dev, CODA PIC_RUN timeout, stopping all streams\n); + + mutex_lock(dev-dev_mutex); + list_for_each_entry(ctx, dev-instances, list) { + v4l2_m2m_streamoff(NULL, ctx-m2m_ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT); + v4l2_m2m_streamoff(NULL, ctx-m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE); + } + mutex_unlock(dev-dev_mutex); +} + static u32 coda_supported_firmwares[] = { CODA_FIRMWARE_VERNUM(CODA_DX6, 2, 2, 5), CODA_FIRMWARE_VERNUM(CODA_7541, 13, 4, 29), @@ -1835,6 +1860,7 @@ static int __devinit coda_probe(struct platform_device *pdev) spin_lock_init(dev-irqlock); INIT_LIST_HEAD(dev-instances); + INIT_DELAYED_WORK(dev-timeout, coda_timeout); dev-plat_dev = pdev; dev-clk_per = devm_clk_get(pdev-dev, per); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 08/16] media: coda: enable user pointer support
USERPTR buffer support is provided by the videobuf2 framework. Signed-off-by: Philipp Zabel p.za...@pengutronix.de --- drivers/media/platform/coda.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c index 7f6ec3a..e0595ce 100644 --- a/drivers/media/platform/coda.c +++ b/drivers/media/platform/coda.c @@ -1345,7 +1345,7 @@ static int coda_queue_init(void *priv, struct vb2_queue *src_vq, memset(src_vq, 0, sizeof(*src_vq)); src_vq-type = V4L2_BUF_TYPE_VIDEO_OUTPUT; - src_vq-io_modes = VB2_MMAP; + src_vq-io_modes = VB2_MMAP | VB2_USERPTR; src_vq-drv_priv = ctx; src_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); src_vq-ops = coda_qops; @@ -1357,7 +1357,7 @@ static int coda_queue_init(void *priv, struct vb2_queue *src_vq, memset(dst_vq, 0, sizeof(*dst_vq)); dst_vq-type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - dst_vq-io_modes = VB2_MMAP; + dst_vq-io_modes = VB2_MMAP | VB2_USERPTR; dst_vq-drv_priv = ctx; dst_vq-buf_struct_size = sizeof(struct v4l2_m2m_buffer); dst_vq-ops = coda_qops; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pac7302-webcams and libv4lconvert interaction
Hi Hans, Am 09.09.2012 23:20, schrieb Hans de Goede: Hi, On 09/06/2012 05:13 PM, Frank Schäfer wrote: Hi, I'm currently looking into the gspca_pac7302-driver and how it interacts with libv4lconvert. This is how it currently works - driver announces v4l2_pix_format 640x480 (width x height) - the frames (jpeg) passed to userspace are encoded as 480x640 and this complies with the jpeg-header we generate - libv4lconvert checks width/height in the jpeg-header and compares them with the image format announced by the kernel: a) values are the same: 1) V4LCONTROL_ROTATED_90_JPEG is NOT set for the device (standard case): = everything is fine, image is decoded 2) V4LCONTROL_ROTATED_90_JPEG is set for the device: = libv4lconvert bails out with -EIO displaying the error message unexpected width / height in JPEG header: expected: 640x480, header: 480x640 b) values are different: 1) V4LCONTROL_ROTATED_90_JPEG is NOT set: = libv4lconvert bails out with -EIO displaying the error message unexpected width / height in JPEG header: expected: 640x480, header: 480x640 2) V4LCONTROL_ROTATED_90_JPEG is set: = image is decoded and rotated correctly Thinking about this for some minutes: 1) shouldn't the kernel always announce the real image format (size) of the data it passes to userspace ? It is passing the real size, the data is just in a vary funky format which needs rotation as part of its decoding / decompression. It is first decoded, then rotated, right ? Are the frames encoded as 480x640 (that's what the header says) or 640x480 ? If the header is wrong, everything is fine. Otherwise we are passing the size we finally get AFTER decoding and rotation to userspace, which I think would be inconsistent. Current behavior seems inconsistent to me... Announcing the actual image size allows applications which trust the API value more than the value in the frame header to decode the image correctly without using libv4lconvert (although the image would still be rotated). That assumes that the app would know how to decompress the data which it will not know, these cams do not generate standard JPEG data, libv4lconvert's decompression code is the only decompression code for this fsck-ed up JPEG-s, short of the windows drivers code. Ok, that's true. Because of the special format, applications are forced to use libv4lconvert, so there is CURRENTLY no need to think about the kernel - userspace interface. But that might change in the future... 2) shouldn't libv4lconvert always rotate the image if V4LCONTROL_ROTATED_90_JPEG is set for a device ? It seems like a2) is a bug, because the expected size should be 640x480, too. rotating by 90 degrees also swaps the width and height, which are usually not the same, so rotating an image which starts at 640x480 will yield a final image of 480x640 which will not be what the app expects. Well, I can't see why 480x640 should be an invalid format !? Depends on the hardware. Applications really shouldn't rely on width beeing always larger then height. Otherwise I would call them buggy. Regards, Frank 3) because all pac7302 devices are sending rotated image data, we should add them ALL to libv4lconvert. Currently only 4 of the 14 devices are on the list. Do you want me to send a patch ? I see you've send a patch in the mean time, I'll reply in more detail to this to the patch mail. 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: [PATCH 2/3] libv4lconvert: pac7302-devices: remove unneeded flag V4LCONTROL_WANTS_WB
Am 09.09.2012 23:24, schrieb Hans de Goede: Hi, On 09/09/2012 08:36 PM, Frank Schäfer wrote: The gspca_pac7302 driver already provides this control. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- lib/libv4lconvert/control/libv4lcontrol.c | 12 1 files changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/libv4lconvert/control/libv4lcontrol.c b/lib/libv4lconvert/control/libv4lcontrol.c index 1272256..3d7a816 100644 --- a/lib/libv4lconvert/control/libv4lcontrol.c +++ b/lib/libv4lconvert/control/libv4lcontrol.c @@ -202,14 +202,10 @@ static const struct v4lcontrol_flags_info v4lcontrol_flags[] = { { 0x145f, 0x013a, 0,NULL, NULL, V4LCONTROL_WANTS_WB, 1500 }, { 0x2001, 0xf115, 0,NULL, NULL, V4LCONTROL_WANTS_WB, 1500 }, /* Pac7302 based devices */ -{ 0x093a, 0x2620, 0x0f, NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, -{ 0x06f8, 0x3009, 0,NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, -{ 0x06f8, 0x301b, 0,NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, -{ 0x145f, 0x013c, 0,NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, +{ 0x093a, 0x2620, 0x0f, NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, +{ 0x06f8, 0x3009, 0,NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, +{ 0x06f8, 0x301b, 0,NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, +{ 0x145f, 0x013c, 0,NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, /* Pac7311 based devices */ { 0x093a, 0x2600, 0x0f, NULL, NULL, V4LCONTROL_WANTS_WB }, /* sq905 devices */ WANTS_WB does not add a whitebalance control, which these cameras indeed already have, it adds a (software) autowhitebalance control, which enables libv4lconvert doing software whitebalance correction. Although your kernel patch for the pac7302 driver to pick a better default whitebalance value, probably helps a lot to get the colors less screwed up, in the end we still need some sort of awb to adjust to changing lightning conditions, that is what this flag adds, as the pac7302 driver lacks awb. Ok, so WANTS_WB is actually WANTS_AUTOWB. ;) But... IIRC... the software AWB control is always there, even without this flag !? Or is it just about switching AWB on by default ? And if AWB is on, the WB control should be disabled, right ? Regards, Frank 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: [PATCH] Support for Asus MyCinema U3100Mini Plus
On 09/10/12 16:29, Oliver Schinagl wrote: On 10-09-12 13:46, Antti Palosaari wrote: On 09/10/2012 12:58 PM, Oliver Schinagl wrote: Changed the address as recommended, which after reading 7bit and 8bit addressing makes perfect sense (drop the r/w bit and get the actual address). static struct fc2580_config af9035_fc2580_config = { - .i2c_addr = 0xac, + .i2c_addr = 0x56, .clock = 16384000, }; So now the address should actually be correct ;) Unfortunately, nothing. What other debug options do I need to enable besides CONFIG_DVB_USB_DEBUG to get more interesting output? For me it sees something happens as there is no I2C error seen anymore. AF9035 driver uses Kernel dynamic debugs. CONFIG_DVB_USB_DEBUG is legacy and proprietary DVB subsystem debug which should not be used anymore. You could order dynamic debugs like that: modprobe dvb_usb_af9035; echo -n 'module dvb_usb_af9035 +p' /sys/kernel/debug/dynamic_debug/control For tuner, demod and dvb_usbv2 similarly if needed. I've did and added output from control and dmesg output. I don't exactly know how to read the dynamic debug output, the only thing that jumped out at me, was: drivers/media/dvb-frontends/af9033.c:327 [af9033]af9033_init =p %s: unsupported tuner ID=%d\012 So I will search and see where in the driver the supported tunerID's are stored and fix that. Any other pointers/things you see I should look at? Appearantly, I setup the tuner, like the others, but it skips that because the tuner id is wrong/not set. case AF9033_TUNER_FC2580: len = ARRAY_SIZE(tuner_init_fc2580); init = tuner_init_fc2580; break; So where is the tuner set? I did find this bit: tatic int af9035_read_config(struct dvb_usb_device *d) { snip ret = af9035_rd_reg(d, EEPROM_1_TUNER_ID + eeprom_shift, tmp); which suggests that it comes from the actual eeprom. I assumed that the 'init/script/firmware' bit, the first 'message' was the ID, 0x32 in the case of this tuner. I guess I'm wrong? The log is not exactly helpful either: drivers/media/usb/dvb-usb-v2/af9035.c:542 [dvb_usb_af9035]af9035_read_config =_ %s: [%d]tuner=%02x\012 So close, yet so far. So if I'm right, the actual ID of the tuner and the first byte in the init are not always the same? Then why use the define in the first place there? And why would the 'official' code user 0x32 as tuner ID. Or is this simply a dec/hex conversion goof? Oliver Anyway, dmesg reports the following. [60.071538] usb 1-3: new high-speed USB device number 3 using ehci_hcd [60.192627] usb 1-3: New USB device found, idVendor=0b05, idProduct=1779 [60.192638] usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [60.192646] usb 1-3: Product: AF9035A USB Device [60.192652] usb 1-3: Manufacturer: Afa Technologies Inc. [60.192657] usb 1-3: SerialNumber: AF010asdfasdf12314 [60.198686] input: Afa Technologies Inc. AF9035A USB Device as /devices/pci:00/:00:12.2/usb1/1-3/1-3:1.1/input/input14 [60.198832] hid-generic 0003:0B05:1779.0003: input: USB HID v1.01 Keyboard [Afa Technologies Inc. AF9035A USB Device] on usb-:00:12.2-3/input1 [60.263893] usbcore: registered new interface driver dvb_usb_af9035 [60.264605] usb 1-3: dvb_usbv2: found a 'Asus U3100Mini Plus' in cold state [60.273924] usb 1-3: dvb_usbv2: downloading firmware from file 'dvb-usb-af9035-02.fw' [60.584267] dvb_usb_af9035: firmware version=11.5.9.0 [60.584287] usb 1-3: dvb_usbv2: found a 'Asus U3100Mini Plus' in warm state [60.586802] usb 1-3: dvb_usbv2: will pass the complete MPEG2 transport stream to the software demuxer [60.586871] DVB: registering new adapter (Asus U3100Mini Plus) [60.595637] af9033: firmware version: LINK=11.5.9.0 OFDM=5.17.9.1 [60.595654] usb 1-3: DVB: registering adapter 0 frontend 0 (Afatech AF9033 (DVB-T))... [60.599889] usb 1-3: dvb_usbv2: 'Asus U3100Mini Plus' error while loading driver (-19) I then tried using the firmware that came with said driver, as the version seems slightly different/newer. #define FW_RELEASE_VERSION v8_8_63_0 #define DVB_LL_VERSION1 11 #define DVB_LL_VERSION2 22 #define DVB_LL_VERSION3 12 #define DVB_LL_VERSION4 0 #define DVB_OFDM_VERSION1 5 #define DVB_OFDM_VERSION2 66 #define DVB_OFDM_VERSION3 12 #define DVB_OFDM_VERSION4 0 (which also gets displayed when loading the firmware, originally on the old kernel). This however results in a hard lock/dump when plugging in the device. Are there certain size restrictions etc? What I did to obtain said firmware was write a simple program that reads the array, static unsigned char Firmware_codes[] and outputs the read bytes to a file. From what I saw from the -02 firmware, the first few bytes are identical (header?) so should be right procedure. Firmare surely works but you make some mistake. I have extracted those from the windows driver. http://palosaari.fi/linux/v4l-dvb/firmware/af9035/ A link to, or your files should be listed at the linuxdvb firmware
Re: pac7302-webcams and libv4lconvert interaction
Hi, On 09/10/2012 05:36 PM, Frank Schäfer wrote: Hi Hans, Am 09.09.2012 23:20, schrieb Hans de Goede: Hi, On 09/06/2012 05:13 PM, Frank Schäfer wrote: Hi, I'm currently looking into the gspca_pac7302-driver and how it interacts with libv4lconvert. This is how it currently works - driver announces v4l2_pix_format 640x480 (width x height) - the frames (jpeg) passed to userspace are encoded as 480x640 and this complies with the jpeg-header we generate - libv4lconvert checks width/height in the jpeg-header and compares them with the image format announced by the kernel: a) values are the same: 1) V4LCONTROL_ROTATED_90_JPEG is NOT set for the device (standard case): = everything is fine, image is decoded 2) V4LCONTROL_ROTATED_90_JPEG is set for the device: = libv4lconvert bails out with -EIO displaying the error message unexpected width / height in JPEG header: expected: 640x480, header: 480x640 b) values are different: 1) V4LCONTROL_ROTATED_90_JPEG is NOT set: = libv4lconvert bails out with -EIO displaying the error message unexpected width / height in JPEG header: expected: 640x480, header: 480x640 2) V4LCONTROL_ROTATED_90_JPEG is set: = image is decoded and rotated correctly Thinking about this for some minutes: 1) shouldn't the kernel always announce the real image format (size) of the data it passes to userspace ? It is passing the real size, the data is just in a vary funky format which needs rotation as part of its decoding / decompression. It is first decoded, then rotated, right ? Yes, because rotating raw JPEG data is hard, but we believe that the rotation happening inside the cam is a side-effect (or rather a bug) of how the hardware encoding works. So that we split the 2 steps in software is just for convenience. The fact is that these cams have a sensor which has 640x480 pixels, not 480x640. So the correct size to report to userspace is 640x480, the rotation is a side effect of the special pixart pixel format / encoding. Are the frames encoded as 480x640 (that's what the header says) or 640x480 ? If the header is wrong, everything is fine. Otherwise we are passing the size we finally get AFTER decoding and rotation to userspace, which I think would be inconsistent. They are encoded as 480x640, which is why the header says that (otherwise even more pixart hacks to the jpeg decoder in tinyjpeg.c would be necessary), but again the sensor has a resolution of 640x480, not 480x640. So both the reported resolution and the header are right, they just don't agree because these (dirt cheap) cameras are really funky. Current behavior seems inconsistent to me... Announcing the actual image size allows applications which trust the API value more than the value in the frame header to decode the image correctly without using libv4lconvert (although the image would still be rotated). That assumes that the app would know how to decompress the data which it will not know, these cams do not generate standard JPEG data, libv4lconvert's decompression code is the only decompression code for this fsck-ed up JPEG-s, short of the windows drivers code. Ok, that's true. Because of the special format, applications are forced to use libv4lconvert, so there is CURRENTLY no need to think about the kernel - userspace interface. But that might change in the future... 2) shouldn't libv4lconvert always rotate the image if V4LCONTROL_ROTATED_90_JPEG is set for a device ? It seems like a2) is a bug, because the expected size should be 640x480, too. rotating by 90 degrees also swaps the width and height, which are usually not the same, so rotating an image which starts at 640x480 will yield a final image of 480x640 which will not be what the app expects. Well, I can't see why 480x640 should be an invalid format !? Depends on the hardware. What I'm trying to say is that for the image to be 640x480 after rotation by 90 degrees, it has to be 480x640 before rotation. So since the raw data needs rotation it reports itself as 480x640, where as the end result reports itself as 640x480 ... 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: [PATCH 2/3] libv4lconvert: pac7302-devices: remove unneeded flag V4LCONTROL_WANTS_WB
Hi, On 09/10/2012 05:41 PM, Frank Schäfer wrote: Am 09.09.2012 23:24, schrieb Hans de Goede: Hi, On 09/09/2012 08:36 PM, Frank Schäfer wrote: The gspca_pac7302 driver already provides this control. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- lib/libv4lconvert/control/libv4lcontrol.c | 12 1 files changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/libv4lconvert/control/libv4lcontrol.c b/lib/libv4lconvert/control/libv4lcontrol.c index 1272256..3d7a816 100644 --- a/lib/libv4lconvert/control/libv4lcontrol.c +++ b/lib/libv4lconvert/control/libv4lcontrol.c @@ -202,14 +202,10 @@ static const struct v4lcontrol_flags_info v4lcontrol_flags[] = { { 0x145f, 0x013a, 0,NULL, NULL, V4LCONTROL_WANTS_WB, 1500 }, { 0x2001, 0xf115, 0,NULL, NULL, V4LCONTROL_WANTS_WB, 1500 }, /* Pac7302 based devices */ -{ 0x093a, 0x2620, 0x0f, NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, -{ 0x06f8, 0x3009, 0,NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, -{ 0x06f8, 0x301b, 0,NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, -{ 0x145f, 0x013c, 0,NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, +{ 0x093a, 0x2620, 0x0f, NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, +{ 0x06f8, 0x3009, 0,NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, +{ 0x06f8, 0x301b, 0,NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, +{ 0x145f, 0x013c, 0,NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, /* Pac7311 based devices */ { 0x093a, 0x2600, 0x0f, NULL, NULL, V4LCONTROL_WANTS_WB }, /* sq905 devices */ WANTS_WB does not add a whitebalance control, which these cameras indeed already have, it adds a (software) autowhitebalance control, which enables libv4lconvert doing software whitebalance correction. Although your kernel patch for the pac7302 driver to pick a better default whitebalance value, probably helps a lot to get the colors less screwed up, in the end we still need some sort of awb to adjust to changing lightning conditions, that is what this flag adds, as the pac7302 driver lacks awb. Ok, so WANTS_WB is actually WANTS_AUTOWB. ;) But... IIRC... the software AWB control is always there, even without this flag !? Or is it just about switching AWB on by default ? Correct, also note that the awb control will only show up for devices which have non standard formats, since those need to always go through libv4lconvert anyways, it does not get added to standard cams unless specifically enabled through a quirk list entry. And if AWB is on, the WB control should be disabled, right ? No, the software AWB works by applying software rgb gains, so the hardware control is still useful, as the better the color balance of the input, the better the end-result will be. 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: [RFC PATCH v6] media: add v4l2 subdev driver for S5K4ECGX sensor
Hi, On 09/10/2012 05:04 PM, Sylwester Nawrocki wrote: Hi, On 09/09/2012 06:01 PM, Francesco Lavra wrote: +static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) +{ + const struct firmware *fw; + int err, i, regs_num; + struct i2c_client *client = v4l2_get_subdevdata(sd); + u16 val; + u32 addr, crc, crc_file, addr_inc = 0; + + err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev); + if (err) { + v4l2_err(sd, Failed to read firmware %s\n, S5K4ECGX_FIRMWARE); + return err; + } + regs_num = *(u32 *)(fw-data); + v4l2_dbg(3, debug, sd, FW: %s size %d register sets %d\n, +S5K4ECGX_FIRMWARE, fw-size, regs_num); + regs_num++; /* Add header */ + if (fw-size != regs_num * FW_RECORD_SIZE + FW_CRC_SIZE) { + err = -EINVAL; + goto fw_out; + } + crc_file = *(u32 *)(fw-data + regs_num * FW_RECORD_SIZE); Depending on the value of regs_num, this may result in unaligned access Thanks for the catch. I think it is not the only place where unaligned issues are possible. Since the data records are 4-byte address + 2-byte value there is also an issue with reading the address entries. Assuming fw-data is aligned to at least 2-bytes (not quite sure if we can assume that) there should be no problem with reading 2-byte register values. I'm not sure 2-byte alignment can be safely assumed, either. We could change the data types of the register values from u16 to u32, wasting some memory (there is approximately 3 000 records), so there is no other data types in the file structure than u32. Or use a patch as below. Not sure what's better. I prefer the approach of your patch below, but I would use get_unaligned to get the 2-byte values, too. Also there are another couple of glitches, see below. 8- From a970480b99bdb74e2bf48e1a321724231e6516a0 Mon Sep 17 00:00:00 2001 From: Sylwester Nawrocki sylvester.nawro...@gmail.com Date: Sun, 9 Sep 2012 19:56:31 +0200 Subject: [PATCH] s5k4ecgx: Fix unaligned access issues Signed-off-by: Sylwester Nawrocki sylvester.nawro...@gmail.com --- drivers/media/i2c/s5k4ecgx.c | 16 1 files changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c index 0ef0b7d..4c6439a 100644 --- a/drivers/media/i2c/s5k4ecgx.c +++ b/drivers/media/i2c/s5k4ecgx.c @@ -24,6 +24,7 @@ #include linux/module.h #include linux/regulator/consumer.h #include linux/slab.h +#include asm/unaligned.h #include media/media-entity.h #include media/s5k4ecgx.h @@ -331,6 +332,7 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) const struct firmware *fw; int err, i, regs_num; u32 addr, crc, crc_file, addr_inc = 0; + const u8 *ptr; u16 val; err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev); @@ -338,7 +340,7 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) v4l2_err(sd, Failed to read firmware %s\n, S5K4ECGX_FIRMWARE); return err; } - regs_num = le32_to_cpu(*(u32 *)fw-data); + regs_num = le32_to_cpu(get_unaligned((__le32 *)fw-data)); v4l2_dbg(3, debug, sd, FW: %s size %d register sets %d\n, S5K4ECGX_FIRMWARE, fw-size, regs_num); @@ -349,7 +351,8 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) goto fw_out; } - crc_file = *(u32 *)(fw-data + regs_num * FW_RECORD_SIZE); + memcpy(crc_file, fw-data + regs_num * FW_RECORD_SIZE, sizeof(u32)); crc_file should be converted from little endian to native endian. + crc = crc32_le(~0, fw-data, regs_num * FW_RECORD_SIZE); if (crc != crc_file) { v4l2_err(sd, FW: invalid crc (%#x:%#x)\n, crc, crc_file); @@ -357,9 +360,14 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) goto fw_out; } + ptr = fw-data + FW_RECORD_SIZE; + for (i = 1; i regs_num; i++) { - addr = le32_to_cpu(*(u32 *)(fw-data + i * FW_RECORD_SIZE)); - val = le16_to_cpu(*(u16 *)(fw-data + i * FW_RECORD_SIZE + 4)); + addr = le32_to_cpu(get_unaligned((__le32 *)ptr)); + ptr += 4; + val = le16_to_cpu(*(__le16 *)ptr); + ptr += FW_RECORD_SIZE; ptr is being incremented by (4 + FW_RECORD_SIZE) bytes at each iteration. + if (addr - addr_inc != 2) err = s5k4ecgx_write(client, addr, val); else -- Francesco -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: WARNINGS
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:Mon Sep 10 19:00:18 CEST 2012 git hash:79e8c7bebb467bbc3f2514d75bba669a3f354324 gcc version: i686-linux-gcc (GCC) 4.7.1 host hardware:x86_64 host os: 3.4.07-marune linux-git-arm-eabi-davinci: WARNINGS linux-git-arm-eabi-exynos: OK linux-git-arm-eabi-omap: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: WARNINGS linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-x86_64: WARNINGS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-x86_64: WARNINGS linux-3.2.1-x86_64: WARNINGS linux-3.3-x86_64: WARNINGS linux-3.4-x86_64: WARNINGS linux-3.5-x86_64: WARNINGS linux-3.6-rc2-x86_64: WARNINGS linux-2.6.31.12-i686: WARNINGS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-i686: WARNINGS linux-3.2.1-i686: WARNINGS linux-3.3-i686: WARNINGS linux-3.4-i686: WARNINGS linux-3.5-i686: WARNINGS linux-3.6-rc2-i686: WARNINGS apps: WARNINGS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pac7302-webcams and libv4lconvert interaction
Hi, Am 10.09.2012 20:31, schrieb Hans de Goede: Hi, On 09/10/2012 05:36 PM, Frank Schäfer wrote: Hi Hans, Am 09.09.2012 23:20, schrieb Hans de Goede: Hi, On 09/06/2012 05:13 PM, Frank Schäfer wrote: Hi, I'm currently looking into the gspca_pac7302-driver and how it interacts with libv4lconvert. This is how it currently works - driver announces v4l2_pix_format 640x480 (width x height) - the frames (jpeg) passed to userspace are encoded as 480x640 and this complies with the jpeg-header we generate - libv4lconvert checks width/height in the jpeg-header and compares them with the image format announced by the kernel: a) values are the same: 1) V4LCONTROL_ROTATED_90_JPEG is NOT set for the device (standard case): = everything is fine, image is decoded 2) V4LCONTROL_ROTATED_90_JPEG is set for the device: = libv4lconvert bails out with -EIO displaying the error message unexpected width / height in JPEG header: expected: 640x480, header: 480x640 b) values are different: 1) V4LCONTROL_ROTATED_90_JPEG is NOT set: = libv4lconvert bails out with -EIO displaying the error message unexpected width / height in JPEG header: expected: 640x480, header: 480x640 2) V4LCONTROL_ROTATED_90_JPEG is set: = image is decoded and rotated correctly Thinking about this for some minutes: 1) shouldn't the kernel always announce the real image format (size) of the data it passes to userspace ? It is passing the real size, the data is just in a vary funky format which needs rotation as part of its decoding / decompression. It is first decoded, then rotated, right ? Yes, because rotating raw JPEG data is hard, but we believe that the rotation happening inside the cam is a side-effect (or rather a bug) of how the hardware encoding works. So that we split the 2 steps in software is just for convenience. The fact is that these cams have a sensor which has 640x480 pixels, not 480x640. So the correct size to report to userspace is 640x480, the rotation is a side effect of the special pixart pixel format / encoding. I don't get it. What's the definition of the width+height values in v4l2_pix_format ? Does it really describe the pixel layout on the sensor ? Up to now, I thought it describes the size of the frames passed to userspace ! And if the images are encoded in 480x640, v4l2_pix_format should report 480x640, too !? What are we going to do if we find a pac7302 device with the sensor mounted rotated ? Following the current approach, we would need to modify the kernel to report 480x640 in this case. But the sensor is still 640x480, right ? Are the frames encoded as 480x640 (that's what the header says) or 640x480 ? If the header is wrong, everything is fine. Otherwise we are passing the size we finally get AFTER decoding and rotation to userspace, which I think would be inconsistent. They are encoded as 480x640, which is why the header says that (otherwise even more pixart hacks to the jpeg decoder in tinyjpeg.c would be necessary), but again the sensor has a resolution of 640x480, not 480x640. So both the reported resolution and the header are right, they just don't agree because these (dirt cheap) cameras are really funky. No, only the jpeg-header is right. The kernel currently reports the image size we want to have in the end (after rotating the image in userspace). Do you really think this is correct API design ? Shouldn't both things be kept separate ? Current behavior seems inconsistent to me... Announcing the actual image size allows applications which trust the API value more than the value in the frame header to decode the image correctly without using libv4lconvert (although the image would still be rotated). That assumes that the app would know how to decompress the data which it will not know, these cams do not generate standard JPEG data, libv4lconvert's decompression code is the only decompression code for this fsck-ed up JPEG-s, short of the windows drivers code. Ok, that's true. Because of the special format, applications are forced to use libv4lconvert, so there is CURRENTLY no need to think about the kernel - userspace interface. But that might change in the future... 2) shouldn't libv4lconvert always rotate the image if V4LCONTROL_ROTATED_90_JPEG is set for a device ? It seems like a2) is a bug, because the expected size should be 640x480, too. rotating by 90 degrees also swaps the width and height, which are usually not the same, so rotating an image which starts at 640x480 will yield a final image of 480x640 which will not be what the app expects. Well, I can't see why 480x640 should be an invalid format !? Depends on the hardware. What I'm trying to say is that for the image to be 640x480 after rotation by 90 degrees, it has to be 480x640 before rotation. Sure. But I would say in both, v4l2_pix_format AND
Re: [RFC PATCH v6] media: add v4l2 subdev driver for S5K4ECGX sensor
On 09/10/2012 08:52 PM, Francesco Lavra wrote: On 09/10/2012 05:04 PM, Sylwester Nawrocki wrote: On 09/09/2012 06:01 PM, Francesco Lavra wrote: +static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) +{ + const struct firmware *fw; + int err, i, regs_num; + struct i2c_client *client = v4l2_get_subdevdata(sd); + u16 val; + u32 addr, crc, crc_file, addr_inc = 0; + + err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev); + if (err) { + v4l2_err(sd, Failed to read firmware %s\n, S5K4ECGX_FIRMWARE); + return err; + } + regs_num = *(u32 *)(fw-data); + v4l2_dbg(3, debug, sd, FW: %s size %d register sets %d\n, + S5K4ECGX_FIRMWARE, fw-size, regs_num); + regs_num++; /* Add header */ + if (fw-size != regs_num * FW_RECORD_SIZE + FW_CRC_SIZE) { + err = -EINVAL; + goto fw_out; + } + crc_file = *(u32 *)(fw-data + regs_num * FW_RECORD_SIZE); Depending on the value of regs_num, this may result in unaligned access Thanks for the catch. I think it is not the only place where unaligned issues are possible. Since the data records are 4-byte address + 2-byte value there is also an issue with reading the address entries. Assuming fw-data is aligned to at least 2-bytes (not quite sure if we can assume that) there should be no problem with reading 2-byte register values. I'm not sure 2-byte alignment can be safely assumed, either. We could change the data types of the register values from u16 to u32, wasting some memory (there is approximately 3 000 records), so there is no other data types in the file structure than u32. Or use a patch as below. Not sure what's better. I prefer the approach of your patch below, but I would use get_unaligned to get the 2-byte values, too. Also there are another couple of glitches, see below. OK, thanks for the feedback. It was also my preference. The performance impact seems insignificant, given a write of each record takes time of 1 ms order. 8- From a970480b99bdb74e2bf48e1a321724231e6516a0 Mon Sep 17 00:00:00 2001 From: Sylwester Nawrockisylvester.nawro...@gmail.com Date: Sun, 9 Sep 2012 19:56:31 +0200 Subject: [PATCH] s5k4ecgx: Fix unaligned access issues Signed-off-by: Sylwester Nawrockisylvester.nawro...@gmail.com --- drivers/media/i2c/s5k4ecgx.c | 16 1 files changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c index 0ef0b7d..4c6439a 100644 --- a/drivers/media/i2c/s5k4ecgx.c +++ b/drivers/media/i2c/s5k4ecgx.c @@ -24,6 +24,7 @@ #includelinux/module.h #includelinux/regulator/consumer.h #includelinux/slab.h +#includeasm/unaligned.h #includemedia/media-entity.h #includemedia/s5k4ecgx.h @@ -331,6 +332,7 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) const struct firmware *fw; int err, i, regs_num; u32 addr, crc, crc_file, addr_inc = 0; +const u8 *ptr; u16 val; err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev); @@ -338,7 +340,7 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) v4l2_err(sd, Failed to read firmware %s\n, S5K4ECGX_FIRMWARE); return err; } -regs_num = le32_to_cpu(*(u32 *)fw-data); +regs_num = le32_to_cpu(get_unaligned((__le32 *)fw-data)); v4l2_dbg(3, debug, sd, FW: %s size %d register sets %d\n, S5K4ECGX_FIRMWARE, fw-size, regs_num); @@ -349,7 +351,8 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) goto fw_out; } -crc_file = *(u32 *)(fw-data + regs_num * FW_RECORD_SIZE); +memcpy(crc_file, fw-data + regs_num * FW_RECORD_SIZE, sizeof(u32)); crc_file should be converted from little endian to native endian. Right, I should have verified that crc32_le() return value is in native endianness. + crc = crc32_le(~0, fw-data, regs_num * FW_RECORD_SIZE); if (crc != crc_file) { v4l2_err(sd, FW: invalid crc (%#x:%#x)\n, crc, crc_file); @@ -357,9 +360,14 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) goto fw_out; } +ptr = fw-data + FW_RECORD_SIZE; + for (i = 1; i regs_num; i++) { -addr = le32_to_cpu(*(u32 *)(fw-data + i * FW_RECORD_SIZE)); -val = le16_to_cpu(*(u16 *)(fw-data + i * FW_RECORD_SIZE + 4)); +addr = le32_to_cpu(get_unaligned((__le32 *)ptr)); +ptr += 4; +val = le16_to_cpu(*(__le16 *)ptr); +ptr += FW_RECORD_SIZE; ptr is being incremented by (4 + FW_RECORD_SIZE) bytes at each iteration. Oops, I was to quick in sending that patch out. Indeed, that's wrong. Sangwook, FWIW, I just pushed the corrected patch to my tree (http://git.linuxtv.org/snawrocki/media.git/commitdiff/4a0ecad6f08ccbba3) -- Thanks, Sylwester -- To unsubscribe from this list:
Re: [PATCH 2/3] libv4lconvert: pac7302-devices: remove unneeded flag V4LCONTROL_WANTS_WB
Am 10.09.2012 20:33, schrieb Hans de Goede: Hi, On 09/10/2012 05:41 PM, Frank Schäfer wrote: Am 09.09.2012 23:24, schrieb Hans de Goede: Hi, On 09/09/2012 08:36 PM, Frank Schäfer wrote: The gspca_pac7302 driver already provides this control. Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- lib/libv4lconvert/control/libv4lcontrol.c | 12 1 files changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/libv4lconvert/control/libv4lcontrol.c b/lib/libv4lconvert/control/libv4lcontrol.c index 1272256..3d7a816 100644 --- a/lib/libv4lconvert/control/libv4lcontrol.c +++ b/lib/libv4lconvert/control/libv4lcontrol.c @@ -202,14 +202,10 @@ static const struct v4lcontrol_flags_info v4lcontrol_flags[] = { { 0x145f, 0x013a, 0,NULL, NULL, V4LCONTROL_WANTS_WB, 1500 }, { 0x2001, 0xf115, 0,NULL, NULL, V4LCONTROL_WANTS_WB, 1500 }, /* Pac7302 based devices */ -{ 0x093a, 0x2620, 0x0f, NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, -{ 0x06f8, 0x3009, 0,NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, -{ 0x06f8, 0x301b, 0,NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, -{ 0x145f, 0x013c, 0,NULL, NULL, -V4LCONTROL_ROTATED_90_JPEG | V4LCONTROL_WANTS_WB, 1500 }, +{ 0x093a, 0x2620, 0x0f, NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, +{ 0x06f8, 0x3009, 0,NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, +{ 0x06f8, 0x301b, 0,NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, +{ 0x145f, 0x013c, 0,NULL, NULL, V4LCONTROL_ROTATED_90_JPEG }, /* Pac7311 based devices */ { 0x093a, 0x2600, 0x0f, NULL, NULL, V4LCONTROL_WANTS_WB }, /* sq905 devices */ WANTS_WB does not add a whitebalance control, which these cameras indeed already have, it adds a (software) autowhitebalance control, which enables libv4lconvert doing software whitebalance correction. Although your kernel patch for the pac7302 driver to pick a better default whitebalance value, probably helps a lot to get the colors less screwed up, in the end we still need some sort of awb to adjust to changing lightning conditions, that is what this flag adds, as the pac7302 driver lacks awb. Ok, so WANTS_WB is actually WANTS_AUTOWB. ;) But... IIRC... the software AWB control is always there, even without this flag !? Or is it just about switching AWB on by default ? Correct, also note that the awb control will only show up for devices which have non standard formats, since those need to always go through libv4lconvert anyways, it does not get added to standard cams unless specifically enabled through a quirk list entry. Good to know that, thanks ! And if AWB is on, the WB control should be disabled, right ? No, the software AWB works by applying software rgb gains, so the hardware control is still useful, as the better the color balance of the input, the better the end-result will be. Hmm... auto-whitebalance should compensate the setting made with the manual hardware controlled whitebalance. But I guess they are working too differenty. Regards, Frank 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: [RFC API] Capture Overlay API ambiguities
On Mon, 2012-09-10 at 17:14 +0200, Hans Verkuil wrote: Hi all, While working on making bttv compliant with the V4L2 API I managed to resolve all v4l2-compliance errors except for two: fail: v4l2-test-formats.cpp(339): !fmt.width || !fmt.height test VIDIOC_G_FBUF: FAIL fail: v4l2-test-formats.cpp(607): Video Overlay is valid, but no S_FMT was implemented test VIDIOC_S_FMT: FAIL After some analysis it turns out to be an ambiguity in VIDIOC_G_FBUF and VIDIOC_G_FMT for overlays. The first relates to what VIDIOC_G_FBUF should return if there is no framebuffer set? I.e. if VIDIOC_S_FBUF was never called? Should G_FBUF return an error, or provide non-zero but dummy struct v4l2_pix_format values and only leave base to NULL, or should the whole v4l2_framebuffer structure be zeroed (except for the capability field)? bttv just zeroes everything. Currently v4l2-compliance assumes that there is at least a dummy fmt setup. But I think that is unreasonably for destructive capture overlays. I would prefer to zero everything except for the capability field. Returning an error is not an option IMHO: even if there is no framebuffer defined, the capability field is still useful information to have. The second error is related to the first: what should happen if you try to set a capture overlay format with S_FMT and no framebuffer is defined? The driver cannot really validate the given format without knowing what the framebuffer is like. Currently I am returning some dummy values for G_FMT and TRY_FMT in that case, but for S_FMT I cannot do even that. While normally the G/S/TRY_FMT ioctls should never return an error, I think that in this particular case we should allow an error code. I am proposing ENODATA because, well, there is no data w.r.t. the framebuffer. I agree that something better and more distinct than EINVAL is needed. Not that the exact error return code really matters, but ENODATA is one of those little used things in POSIX that looks like it was really only meant for read-data-releated POSIX STREAMS ioctl()s: http://pubs.opengroup.org/onlinepubs/009695399/functions/ioctl.html Maybe one of ENXIO, ENOTTY, or EPROTO would be better? http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_03.html#tag_02_03 Regards, Andy The current bttv driver returns EINVAL for these cases, but that's too generic I think. So in the case of destructive capture overlays the G/S/TRY_FMT ioctls should return ENODATA if no framebuffer was configured. Comments? 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 -- 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: go7007 question
On 9/10/2012 6:37 AM, vol...@telros.ru wrote: I`m not seeing wis-sa7115 module, (it`s main module for video device) OK, I recompiled with that built in. I had left it out because I had trouble compiling before, but now it went through fine. Seems to help. After recompiling, I have some progress--I can run gorecord and get audio, but there is no video signal in the resultant AVI. Some error in URB, did u machine have any ATI chipset? Yes: 01:00.0 VGA compatible controller: ATI Technologies Inc RV630 [Radeon HD 2600XT] (prog-if 00 [VGA controller]) 01:00.1 Audio device: ATI Technologies Inc RV630/M76 audio device [Radeon HD 2600 Series] Can u send me: ls /dev/video* ls /dev/audio* ls /dev/tun* # ls -l /dev/tun* /dev/aud* /dev/vid* /dev/dsp* ls: cannot access /dev/tun*: No such file or directory crw-rw+ 1 root audio 14, 4 Sep 10 16:57 /dev/audio crw-rw+ 1 root audio 14, 36 Sep 10 21:01 /dev/audio2 crw-rw+ 1 root audio 14, 3 Sep 10 16:57 /dev/dsp crw-rw+ 1 root audio 14, 35 Sep 10 21:01 /dev/dsp2 crw-rw+ 1 root video 81, 0 Sep 10 21:10 /dev/video0 tail -n 500 /var/log/messages (after calling go_record) Attached. Sep 10 21:05:40 storage kernel: [ 474.278489] [c113b510] ? idr_callback+0x80/0x80 Sep 10 21:05:40 storage kernel: [ 474.278492] [c11393c5] ? send_to_group+0xe5/0x140 Sep 10 21:05:40 storage kernel: [ 474.278497] [f9e25d72] video_ioctl2+0x12/0x20 [videodev] Sep 10 21:05:40 storage kernel: [ 474.278501] [f9e23e40] ? v4l2_ioctl_get_lock+0x50/0x50 [videodev] Sep 10 21:05:40 storage kernel: [ 474.278505] [f9e20913] v4l2_ioctl+0x103/0x150 [videodev] Sep 10 21:05:40 storage kernel: [ 474.278509] [f9e20810] ? v4l2_open+0x140/0x140 [videodev] Sep 10 21:05:40 storage kernel: [ 474.278513] [c111440e] do_vfs_ioctl+0x7e/0x5c0 Sep 10 21:05:40 storage kernel: [ 474.278518] [c11e108a] ? file_has_perm+0x9a/0xc0 Sep 10 21:05:40 storage kernel: [ 474.278522] [c11e1276] ? selinux_file_ioctl+0x56/0x110 Sep 10 21:05:40 storage kernel: [ 474.278524] [c11149cf] sys_ioctl+0x7f/0x90 Sep 10 21:05:40 storage kernel: [ 474.278528] [c15a44cc] sysenter_do_call+0x12/0x22 Sep 10 21:05:40 storage kernel: [ 474.278530] ---[ end trace 7ad74e28071d6ef7 ]--- Sep 10 21:05:45 storage kernel: [ 479.673046] [ cut here ] Sep 10 21:05:45 storage kernel: [ 479.673058] WARNING: at drivers/usb/core/urb.c:414 usb_submit_urb+0x12a/0x3e0() Sep 10 21:05:45 storage kernel: [ 479.673060] Hardware name: Inspiron 530 Sep 10 21:05:45 storage kernel: [ 479.673062] Device: usb Sep 10 21:05:45 storage kernel: [ 479.673062] BOGUS urb xfer, pipe 1 != type 3 Sep 10 21:05:45 storage kernel: [ 479.673064] Modules linked in: wis_sony_tuner(C) wis_uda1342(C) wis_saa7115(C) go7007_usb(C) go7007(C) v4l2_common videodev media ipt_MASQUERADE xt_tcpudp ipt_REDIRECT xt_conntrack iptable_mangle nf_conntrack_ftp ipt_REJECT xt_LOG xt_limit xt_multiport xt_state iptable_nat nf_nat nf_conntrack_ipv4 nf_conntrack nf_defrag_ipv4 iptable_filter ip_tables x_tables fuse ext4 jbd2 crc16 e1000e Sep 10 21:05:45 storage kernel: [ 479.673092] Pid: 11613, comm: gorecord-cvs Tainted: GWC 3.6.0-rc4.go7007.saa7115+ #3 Sep 10 21:05:45 storage kernel: [ 479.673094] Call Trace: Sep 10 21:05:45 storage kernel: [ 479.673100] [c1033f4d] warn_slowpath_common+0x6d/0xa0 Sep 10 21:05:45 storage kernel: [ 479.673103] [c1394eda] ? usb_submit_urb+0x12a/0x3e0 Sep 10 21:05:45 storage kernel: [ 479.673106] [c1394eda] ? usb_submit_urb+0x12a/0x3e0 Sep 10 21:05:45 storage kernel: [ 479.673109] [c1033ffe] warn_slowpath_fmt+0x2e/0x30 Sep 10 21:05:45 storage kernel: [ 479.673112] [c1394eda] usb_submit_urb+0x12a/0x3e0 Sep 10 21:05:45 storage kernel: [ 479.673115] [c10432a9] ? del_timer_sync+0x29/0x50 Sep 10 21:05:45 storage kernel: [ 479.673120] [c159bb99] ? schedule_timeout+0xf9/0x1b0 Sep 10 21:05:45 storage kernel: [ 479.673124] [f9e6225a] go7007_usb_read_interrupt+0x1a/0x40 [go7007_usb] Sep 10 21:05:45 storage kernel: [ 479.673127] [c1042d40] ? lock_timer_base+0x50/0x50 Sep 10 21:05:45 storage kernel: [ 479.673132] [f9e4daf4] go7007_read_interrupt+0x24/0x100 [go7007] Sep 10 21:05:45 storage kernel: [ 479.673135] [f9e626cc] go7007_usb_interface_reset+0x4c/0x130 [go7007_usb] Sep 10 21:05:45 storage kernel: [ 479.673139] [f9e4dd90] go7007_load_encoder+0xa0/0x180 [go7007] Sep 10 21:05:45 storage kernel: [ 479.673142] [c1394da0] ? usb_kill_urb+0x90/0xa0 Sep 10 21:05:45 storage kernel: [ 479.673146] [f9e4de7b] go7007_reset_encoder+0xb/0x20 [go7007] Sep 10 21:05:45 storage kernel: [ 479.673149] [f9e4cacb] go7007_streamoff+0xab/0xb0 [go7007] Sep 10 21:05:45 storage kernel: [ 479.673153] [f9e4caf9] vidioc_streamoff+0x29/0x80 [go7007] Sep 10 21:05:45 storage kernel: [ 479.673158] [f9e21b95] v4l_streamoff+0x15/0x20 [videodev] Sep 10 21:05:45 storage kernel: [ 479.673163] [f9e240cc] __video_do_ioctl+0x28c/0x3a0 [videodev] Sep 10 21:05:45 storage
[PATCH 1/2] dvb_usb_v2: rename module dvb_usbv2 = dvb_usb_v2
I think it is better name. At that phase renaming is quite painless as module is not yet merged to mainline Kernel. Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/usb/dvb-usb-v2/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/dvb-usb-v2/Makefile b/drivers/media/usb/dvb-usb-v2/Makefile index 58027fd..b76f58e 100644 --- a/drivers/media/usb/dvb-usb-v2/Makefile +++ b/drivers/media/usb/dvb-usb-v2/Makefile @@ -1,5 +1,5 @@ -dvb_usbv2-objs := dvb_usb_core.o dvb_usb_urb.o usb_urb.o -obj-$(CONFIG_DVB_USB_V2) += dvb_usbv2.o +dvb_usb_v2-objs := dvb_usb_core.o dvb_usb_urb.o usb_urb.o +obj-$(CONFIG_DVB_USB_V2) += dvb_usb_v2.o dvb_usb_cypress_firmware-objs := cypress_firmware.o obj-$(CONFIG_DVB_USB_CYPRESS_FIRMWARE) += dvb_usb_cypress_firmware.o -- 1.7.11.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] dvb_usb_v2: call streaming_ctrl() before kill urbs
Logically it is better ask hardware to stop streaming before killing urbs carrying stream. Earlier it was just opposite. Now code runs: * submit urbs * start streaming ** streaming ongoing ** * stop streaming * kill urbs Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c index e2d73e1..f990159 100644 --- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c +++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c @@ -265,7 +265,6 @@ static inline int dvb_usb_ctrl_feed(struct dvb_demux_feed *dvbdmxfeed, /* stop feeding if it is last pid */ if (adap-feed_count == 0) { dev_dbg(d-udev-dev, %s: stop feeding\n, __func__); - usb_urb_killv2(adap-stream); if (d-props-streaming_ctrl) { ret = d-props-streaming_ctrl( @@ -274,9 +273,11 @@ static inline int dvb_usb_ctrl_feed(struct dvb_demux_feed *dvbdmxfeed, dev_err(d-udev-dev, %s: streaming_ctrl() \ failed=%d\n, KBUILD_MODNAME, ret); + usb_urb_killv2(adap-stream); goto err_mutex_unlock; } } + usb_urb_killv2(adap-stream); mutex_unlock(adap-sync_mutex); } -- 1.7.11.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
ISPsupport
Hi all, I tried devel-ISPSUPPORT-IPIPE and devel-ISPSUPPORT, the kernel detected my image sensor (ov5650). But, when I execute the yavta /dev/video0 -c4 -n1 -s2592x1944 -fSGRBG10 -Fov5650-2592x1944-#.bin I was getting Unable to start streaming: Invalid argument (22).. I would like to know if anyone here can guide me a bit in order to have a working environment?. Regards, John -- 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