Re: [PATCH] media: fix a typo CONFIG_HAVE_GENERIC_DMA_COHERENT in videobuf2-dma-contig.c
Hi, On Tue, Nov 27, 2012 at 1:21 PM, Marek Szyprowski m.szyprow...@samsung.com wrote: Hello, On 11/27/2012 8:39 AM, Prabhakar Lad wrote: Hi Marek, On Tue, Nov 27, 2012 at 12:53 PM, Marek Szyprowski m.szyprow...@samsung.com wrote: Hello, On 11/27/2012 6:59 AM, Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar@ti.com from commit 93049b9368a2e257ace66252ab2cc066f3399cad, which adds a check HAVE_GENERIC_DMA_COHERENT for dma ops, the check was wrongly made it should have been HAVE_GENERIC_DMA_COHERENT but it was CONFIG_HAVE_GENERIC_DMA_COHERENT. This patch fixes the typo, which was causing following build error: videobuf2-dma-contig.c:743: error: 'vb2_dc_get_dmabuf' undeclared here (not in a function) make[3]: *** [drivers/media/v4l2-core/videobuf2-dma-contig.o] Error 1 Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com The CONFIG_HAVE_GENERIC_DMA_COHERENT based patch was a quick workaround for the build problem in linux-next and should be reverted now. The correct patch has been posted for drivers/base/dma-mapping.c to LKML, see http://www.spinics.net/lists/linux-next/msg22890.html I was referring to this patch from Mauro, http://git.linuxtv.org/media_tree.git/commitdiff/93049b9368a2e257ace66252ab2cc066f3399cad which introduced this build error. I know, with my patch the workaround introduced by Mauro is not needed at all. Thanks for clarifying I'll drop this patch, I hope Mauro will revert his changes. Regards, --Prabhakar Best regards -- Marek Szyprowski Samsung Poland RD Center -- 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 01/15] [media] marvell-ccic: use internal variable replace global frame stats variable
Hi Albert On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch replaces the global frame stats variables by using internal variables in mcam_camera structure. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com Thanks for doing this work! Looks good just one remark below. --- drivers/media/platform/marvell-ccic/mcam-core.c | 30 ++- drivers/media/platform/marvell-ccic/mcam-core.h |9 +++ 2 files changed, 22 insertions(+), 17 deletions(-) [snip] diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h index bd6acba..e226de4 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.h +++ b/drivers/media/platform/marvell-ccic/mcam-core.h @@ -73,6 +73,14 @@ static inline int mcam_buffer_mode_supported(enum mcam_buffer_mode mode) } } +/* + * Basic frame states + */ +struct mmp_frame_state { I think this should be struct mcam_frame_state - don't think we need to introduce a whole new namespace in this header just because of this struct. + unsigned int frames; + unsigned int singles; + unsigned int delivered; +}; /* * A description of one of our devices. @@ -108,6 +116,7 @@ struct mcam_camera { unsigned long flags;/* Buffer status, mainly (dev_lock) */ int users; /* How many open FDs */ + struct mmp_frame_state frame_state; /* Frame state counter */ /* * Subsystem structures. */ -- 1.7.9.5 Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver
Hi Albert A general question first: is the MIPI CSI-2 implementation common to all ccic variants or specific to your SoC? On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch adds the MIPI support for marvell-ccic. Board driver should determine whether using MIPI or not. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.c | 60 ++ drivers/media/platform/marvell-ccic/mcam-core.h | 21 ++- drivers/media/platform/marvell-ccic/mmp-driver.c | 72 +- include/media/mmp-camera.h |9 +++ 4 files changed, 160 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 7012913f..b111f0d 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -253,6 +253,46 @@ static void mcam_ctlr_stop(struct mcam_camera *cam) mcam_reg_clear_bit(cam, REG_CTRL0, C0_ENABLE); } +static int mcam_config_mipi(struct mcam_camera *mcam, int enable) +{ + if (mcam-bus_type == V4L2_MBUS_CSI2_LANES enable) { V4L2_MBUS_CSI2_LANES is not a bus-type, it's a mask of all possible lane-number configurations. You probably want to use V4L2_MBUS_CSI2 throught the driver + /* Using MIPI mode and enable MIPI */ + cam_dbg(mcam, camera: DPHY3=0x%x, DPHY5=0x%x, DPHY6=0x%x\n, + (*mcam-dphy)[0], (*mcam-dphy)[1], (*mcam-dphy)[2]); + mcam_reg_write(mcam, REG_CSI2_DPHY3, (*mcam-dphy)[0]); + mcam_reg_write(mcam, REG_CSI2_DPHY6, (*mcam-dphy)[2]); + mcam_reg_write(mcam, REG_CSI2_DPHY5, (*mcam-dphy)[1]); If you change your mcam-dphy as proposed below, the above would simplify to mcam-dphy[x] + + if (mcam-mipi_enabled == 0) { + /* + * 0x41 actives 1 lane + * 0x43 actives 2 lanes + * 0x47 actives 4 lanes + * There is no 3 lanes case + */ + if (mcam-lane == 1) Use switch (mcam-lane) + mcam_reg_write(mcam, REG_CSI2_CTRL0, 0x41); + else if (mcam-lane == 2) + mcam_reg_write(mcam, REG_CSI2_CTRL0, 0x43); + else if (mcam-lane == 4) + mcam_reg_write(mcam, REG_CSI2_CTRL0, 0x47); + else { + cam_err(mcam, camera: lane number set err); + return -EINVAL; + } + mcam-mipi_enabled = 1; + } + } else { + /* Using para mode or disable MIPI */ + mcam_reg_write(mcam, REG_CSI2_DPHY3, 0x0); + mcam_reg_write(mcam, REG_CSI2_DPHY6, 0x0); + mcam_reg_write(mcam, REG_CSI2_DPHY5, 0x0); + mcam_reg_write(mcam, REG_CSI2_CTRL0, 0x0); + mcam-mipi_enabled = 0; + } + return 0; +} + /* --- */ #ifdef MCAM_MODE_VMALLOC @@ -656,6 +696,15 @@ static void mcam_ctlr_image(struct mcam_camera *cam) */ mcam_reg_write_mask(cam, REG_CTRL0, C0_SIF_HVSYNC, C0_SIFM_MASK); + + /* + * This field controls the generation of EOF(DVP only) + */ + if (cam-bus_type != V4L2_MBUS_CSI2_LANES) { + mcam_reg_set_bit(cam, REG_CTRL0, + C0_EOF_VSYNC | C0_VEDGE_CTRL); + mcam_reg_write(cam, REG_CTRL3, 0x4); This will also now be run on existing configurations... Have to verify, whether this is safe there. + } } @@ -886,6 +935,16 @@ static int mcam_read_setup(struct mcam_camera *cam) spin_lock_irqsave(cam-dev_lock, flags); clear_bit(CF_DMA_ACTIVE, cam-flags); mcam_reset_buffers(cam); + /* + * Update CSI2_DPHY value + */ + if (cam-calc_dphy) + cam-calc_dphy(cam); + cam_dbg(cam, camera: DPHY sets: dphy3=0x%x, dphy5=0x%x, dphy6=0x%x\n, + (*cam-dphy)[0], (*cam-dphy)[1], (*cam-dphy)[2]); + ret = mcam_config_mipi(cam, 1); + if (ret 0) + return ret; mcam_ctlr_irq_enable(cam); cam-state = S_STREAMING; if (!test_bit(CF_SG_RESTART, cam-flags)) @@ -1569,6 +1628,7 @@ static int mcam_v4l_release(struct file *filp) if (cam-users == 0) { mcam_ctlr_stop_dma(cam); mcam_cleanup_vb2(cam); + mcam_config_mipi(cam, 0); mcam_ctlr_power_down(cam); if (cam-buffer_mode == B_vmalloc alloc_bufs_at_read) mcam_free_dma_bufs(cam); diff --git
Re: [PATCH 03/15] [media] marvell-ccic: add clock tree support for marvell-ccic driver
On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch adds the clock tree support for marvell-ccic. Each board may require different clk enabling sequence. Developer need add the clk_name in correct sequence in board driver to use this feature. Signed-off-by: Libin Yang lby...@marvell.com Signed-off-by: Albert Wang twan...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.h |6 +++ drivers/media/platform/marvell-ccic/mmp-driver.c | 57 ++ include/media/mmp-camera.h |5 ++ 3 files changed, 68 insertions(+) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h index 2d444a1..0df6b1c 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.h +++ b/drivers/media/platform/marvell-ccic/mcam-core.h @@ -88,6 +88,7 @@ struct mmp_frame_state { * the dev_lock spinlock; they are marked as such by comments. * dev_lock is also required for access to device registers. */ +#define NR_MCAM_CLK 4 struct mcam_camera { /* * These fields should be set by the platform code prior to @@ -107,6 +108,11 @@ struct mcam_camera { int (*dphy)[3]; int mipi_enabled; int lane; /* lane number */ + + /* clock tree support */ + struct clk *clk[NR_MCAM_CLK]; + int clk_num; + /* * Callbacks from the core to the platform code. */ diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index 9d7aa79..80977b0 100755 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -104,6 +104,23 @@ static struct mmp_camera *mmpcam_find_device(struct platform_device *pdev) #define REG_CCIC_DCGCR 0x28/* CCIC dyn clock gate ctrl reg */ #define REG_CCIC_CRCR0x50/* CCIC clk reset ctrl reg */ +static void mcam_clk_set(struct mcam_camera *mcam, int on) +{ + unsigned int i; + + if (on) { + for (i = 0; i mcam-clk_num; i++) { + if (mcam-clk[i]) From your init below, mcam-clk[i] can be a negative error code. + clk_enable(mcam-clk[i]); + } + } else { + for (i = 0; i mcam-clk_num; i++) { + if (mcam-clk[i]) + clk_disable(mcam-clk[i]); + } + } +} + /* * Power control. */ @@ -134,6 +151,8 @@ static void mmpcam_power_up(struct mcam_camera *mcam) mdelay(5); gpio_set_value(pdata-sensor_reset_gpio, 1); /* reset is active low */ mdelay(5); + + mcam_clk_set(mcam, 1); } static void mmpcam_power_down(struct mcam_camera *mcam) @@ -151,6 +170,8 @@ static void mmpcam_power_down(struct mcam_camera *mcam) pdata = cam-pdev-dev.platform_data; gpio_set_value(pdata-sensor_power_gpio, 0); gpio_set_value(pdata-sensor_reset_gpio, 0); + + mcam_clk_set(mcam, 0); } /* @@ -229,6 +250,37 @@ static irqreturn_t mmpcam_irq(int irq, void *data) return IRQ_RETVAL(handled); } +static void mcam_init_clk(struct mcam_camera *mcam, + struct mmp_camera_platform_data *pdata, int init) +{ + unsigned int i; + + if (NR_MCAM_CLK pdata-clk_num) { + dev_warn(mcam-dev, Too many mcam clocks defined\n); + mcam-clk_num = 0; + return; + } + + if (init) { + for (i = 0; i pdata-clk_num; i++) { + if (pdata-clk_name[i] != NULL) + mcam-clk[i] = clk_get(mcam-dev, + pdata-clk_name[i]); + if (IS_ERR(mcam-clk[i])) + dev_warn(mcam-dev, Could not get clk: %s\n, + pdata-clk_name[i]); You issue a warning but continue initialisation, leaving mcam-clk[i] set to an error value. + } + mcam-clk_num = pdata-clk_num; + } else { + for (i = 0; i pdata-clk_num; i++) { + if (mcam-clk[i]) { + clk_put(mcam-clk[i]); + mcam-clk[i] = NULL; + } + } + mcam-clk_num = 0; + } +} Don't think I like this. IIUC, your driver should only try to use clocks, that it knows about, not some random clocks, passed from the platform data. So, you should be using explicit clock names. In your platform data you can set whether a specific clock should be used or not, but not pass clock names down. Also you might want to consider using devm_clk_get() and be more careful with error handling. static int mmpcam_probe(struct platform_device *pdev)
Re: [PATCH 04/15] [media] marvell-ccic: reset ccic phy when stop streaming for stability
On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch adds the reset ccic phy operation when stop streaming. Without reset ccic phy, the next start streaming may be unstable. Also need add CCIC2 definition when PXA688/PXA2128 support dual ccics. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.c |5 + drivers/media/platform/marvell-ccic/mcam-core.h |2 ++ drivers/media/platform/marvell-ccic/mmp-driver.c | 25 ++ 3 files changed, 32 insertions(+) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index b111f0d..760e8ea 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -1053,6 +1053,11 @@ static int mcam_vb_stop_streaming(struct vb2_queue *vq) return -EINVAL; mcam_ctlr_stop_dma(cam); /* + * Reset the CCIC PHY after stopping streaming, + * otherwise, the CCIC may be unstable. + */ + cam-ctlr_reset(cam); Aren't you breaking the cafe driver by calling .ctrl_reset() without checking for NULL? Same holds for your .calc_dphy() callback too. + /* * VB2 reclaims the buffers, so we need to forget * about them. */ diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h index 0df6b1c..40368f6 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.h +++ b/drivers/media/platform/marvell-ccic/mcam-core.h @@ -103,6 +103,7 @@ struct mcam_camera { short int use_smbus;/* SMBUS or straight I2c? */ enum mcam_buffer_mode buffer_mode; + int ccic_id; /* MIPI support */ int bus_type; int (*dphy)[3]; @@ -119,6 +120,7 @@ struct mcam_camera { void (*plat_power_up) (struct mcam_camera *cam); void (*plat_power_down) (struct mcam_camera *cam); void (*calc_dphy)(struct mcam_camera *cam); + void (*ctlr_reset)(struct mcam_camera *cam); /* * Everything below here is private to the mcam core and diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index 80977b0..20046d0 100755 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -103,6 +103,7 @@ static struct mmp_camera *mmpcam_find_device(struct platform_device *pdev) #define CPU_SUBSYS_PMU_BASE 0xd4282800 #define REG_CCIC_DCGCR 0x28/* CCIC dyn clock gate ctrl reg */ #define REG_CCIC_CRCR0x50/* CCIC clk reset ctrl reg */ +#define REG_CCIC2_CRCR 0xf4/* CCIC2 clk reset ctrl reg */ static void mcam_clk_set(struct mcam_camera *mcam, int on) { @@ -174,6 +175,28 @@ static void mmpcam_power_down(struct mcam_camera *mcam) mcam_clk_set(mcam, 0); } +void mcam_ctlr_reset(struct mcam_camera *mcam) +{ + unsigned long val; + struct mmp_camera *cam = mcam_to_cam(mcam); + + if (mcam-ccic_id) { + /* + * Using CCIC2 + */ + val = ioread32(cam-power_regs + REG_CCIC2_CRCR); + iowrite32(val ~0x2, cam-power_regs + REG_CCIC2_CRCR); + iowrite32(val | 0x2, cam-power_regs + REG_CCIC2_CRCR); + } else { + /* + * Using CCIC1 + */ + val = ioread32(cam-power_regs + REG_CCIC_CRCR); + iowrite32(val ~0x2, cam-power_regs + REG_CCIC_CRCR); + iowrite32(val | 0x2, cam-power_regs + REG_CCIC_CRCR); + } +} + /* * calc the dphy register values * There are three dphy registers being used. @@ -301,9 +324,11 @@ static int mmpcam_probe(struct platform_device *pdev) mcam = cam-mcam; mcam-plat_power_up = mmpcam_power_up; mcam-plat_power_down = mmpcam_power_down; + mcam-ctlr_reset = mcam_ctlr_reset; mcam-calc_dphy = mmpcam_calc_dphy; mcam-dev = pdev-dev; mcam-use_smbus = 0; + mcam-ccic_id = pdev-id; mcam-bus_type = pdata-bus_type; mcam-dphy = (pdata-dphy); mcam-mipi_enabled = 0; -- 1.7.9.5 Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 01/15] [media] marvell-ccic: use internal variable replace global frame stats variable
Hi, Guennadi Nice to hear you again after holidays. :) -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 18:16 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 01/15] [media] marvell-ccic: use internal variable replace global frame stats variable Hi Albert On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch replaces the global frame stats variables by using internal variables in mcam_camera structure. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com Thanks for doing this work! Looks good just one remark below. --- drivers/media/platform/marvell-ccic/mcam-core.c | 30 ++- drivers/media/platform/marvell-ccic/mcam-core.h |9 +++ 2 files changed, 22 insertions(+), 17 deletions(-) [snip] diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h index bd6acba..e226de4 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.h +++ b/drivers/media/platform/marvell-ccic/mcam-core.h @@ -73,6 +73,14 @@ static inline int mcam_buffer_mode_supported(enum mcam_buffer_mode mode) } } +/* + * Basic frame states + */ +struct mmp_frame_state { I think this should be struct mcam_frame_state - don't think we need to introduce a whole new namespace in this header just because of this struct. Yes, you are right. We should keep same namespace in this header. Maybe we did a typo. +unsigned int frames; +unsigned int singles; +unsigned int delivered; +}; /* * A description of one of our devices. @@ -108,6 +116,7 @@ struct mcam_camera { unsigned long flags;/* Buffer status, mainly (dev_lock) */ int users; /* How many open FDs */ +struct mmp_frame_state frame_state; /* Frame state counter */ /* * Subsystem structures. */ -- 1.7.9.5 Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 05/15] [media] marvell-ccic: refine mcam_set_contig_buffer function
On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch refines mcam_set_contig_buffer() in mcam core Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com Looks good in general, just will have to be tested on currently supported platforms, because it changes the order, in which registers are written. So, if this patch is not too important for you, maybe it would be better to drop it, it doesn't seem to improve any functionality? If it is decided to use this, you can add my Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Thanks Guennadi --- drivers/media/platform/marvell-ccic/mcam-core.c | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 760e8ea..67d4f2f 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -481,22 +481,21 @@ static void mcam_set_contig_buffer(struct mcam_camera *cam, int frame) */ if (list_empty(cam-buffers)) { buf = cam-vb_bufs[frame ^ 0x1]; - cam-vb_bufs[frame] = buf; - mcam_reg_write(cam, frame == 0 ? REG_Y0BAR : REG_Y1BAR, - vb2_dma_contig_plane_dma_addr(buf-vb_buf, 0)); set_bit(CF_SINGLE_BUFFER, cam-flags); cam-frame_state.singles++; - return; + } else { + /* + * OK, we have a buffer we can use. + */ + buf = list_first_entry(cam-buffers, struct mcam_vb_buffer, + queue); + list_del_init(buf-queue); + clear_bit(CF_SINGLE_BUFFER, cam-flags); } - /* - * OK, we have a buffer we can use. - */ - buf = list_first_entry(cam-buffers, struct mcam_vb_buffer, queue); - list_del_init(buf-queue); + + cam-vb_bufs[frame] = buf; mcam_reg_write(cam, frame == 0 ? REG_Y0BAR : REG_Y1BAR, vb2_dma_contig_plane_dma_addr(buf-vb_buf, 0)); - cam-vb_bufs[frame] = buf; - clear_bit(CF_SINGLE_BUFFER, cam-flags); } /* -- 1.7.9.5 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver
Hi, Guennadi We will update the patch by following your good suggestion! :) -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 18:43 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver Hi Albert A general question first: is the MIPI CSI-2 implementation common to all ccic variants or specific to your SoC? I think it's for all marvell ccic. On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch adds the MIPI support for marvell-ccic. Board driver should determine whether using MIPI or not. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.c | 60 ++ drivers/media/platform/marvell-ccic/mcam-core.h | 21 ++- drivers/media/platform/marvell-ccic/mmp-driver.c | 72 +- include/media/mmp-camera.h |9 +++ 4 files changed, 160 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 7012913f..b111f0d 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -253,6 +253,46 @@ static void mcam_ctlr_stop(struct mcam_camera *cam) mcam_reg_clear_bit(cam, REG_CTRL0, C0_ENABLE); } +static int mcam_config_mipi(struct mcam_camera *mcam, int enable) { +if (mcam-bus_type == V4L2_MBUS_CSI2_LANES enable) { V4L2_MBUS_CSI2_LANES is not a bus-type, it's a mask of all possible lane-number configurations. You probably want to use V4L2_MBUS_CSI2 throught the driver Yes, bus_type should be enum v4l2_mbus_type. We can change it soon. +/* Using MIPI mode and enable MIPI */ +cam_dbg(mcam, camera: DPHY3=0x%x, DPHY5=0x%x, DPHY6=0x%x\n, +(*mcam-dphy)[0], (*mcam-dphy)[1], (*mcam-dphy)[2]); +mcam_reg_write(mcam, REG_CSI2_DPHY3, (*mcam-dphy)[0]); +mcam_reg_write(mcam, REG_CSI2_DPHY6, (*mcam-dphy)[2]); +mcam_reg_write(mcam, REG_CSI2_DPHY5, (*mcam-dphy)[1]); If you change your mcam-dphy as proposed below, the above would simplify to mcam- dphy[x] + +if (mcam-mipi_enabled == 0) { +/* + * 0x41 actives 1 lane + * 0x43 actives 2 lanes + * 0x47 actives 4 lanes + * There is no 3 lanes case + */ +if (mcam-lane == 1) Use switch (mcam-lane) OK. +mcam_reg_write(mcam, REG_CSI2_CTRL0, 0x41); +else if (mcam-lane == 2) +mcam_reg_write(mcam, REG_CSI2_CTRL0, 0x43); +else if (mcam-lane == 4) +mcam_reg_write(mcam, REG_CSI2_CTRL0, 0x47); +else { +cam_err(mcam, camera: lane number set err); +return -EINVAL; +} +mcam-mipi_enabled = 1; +} +} else { +/* Using para mode or disable MIPI */ +mcam_reg_write(mcam, REG_CSI2_DPHY3, 0x0); +mcam_reg_write(mcam, REG_CSI2_DPHY6, 0x0); +mcam_reg_write(mcam, REG_CSI2_DPHY5, 0x0); +mcam_reg_write(mcam, REG_CSI2_CTRL0, 0x0); +mcam-mipi_enabled = 0; +} +return 0; +} + /* --- */ #ifdef MCAM_MODE_VMALLOC @@ -656,6 +696,15 @@ static void mcam_ctlr_image(struct mcam_camera *cam) */ mcam_reg_write_mask(cam, REG_CTRL0, C0_SIF_HVSYNC, C0_SIFM_MASK); + +/* + * This field controls the generation of EOF(DVP only) + */ +if (cam-bus_type != V4L2_MBUS_CSI2_LANES) { +mcam_reg_set_bit(cam, REG_CTRL0, +C0_EOF_VSYNC | C0_VEDGE_CTRL); +mcam_reg_write(cam, REG_CTRL3, 0x4); This will also now be run on existing configurations... Have to verify, whether this is safe there. We have verified it on our existed platform on hand. We just follow up our spec. +} } @@ -886,6 +935,16 @@ static int mcam_read_setup(struct mcam_camera *cam) spin_lock_irqsave(cam-dev_lock, flags); clear_bit(CF_DMA_ACTIVE, cam-flags); mcam_reset_buffers(cam); +/* + * Update CSI2_DPHY value + */ +if (cam-calc_dphy) +cam-calc_dphy(cam); +cam_dbg(cam, camera: DPHY sets: dphy3=0x%x, dphy5=0x%x, dphy6=0x%x\n, +(*cam-dphy)[0], (*cam-dphy)[1], (*cam-dphy)[2]); +ret = mcam_config_mipi(cam, 1); +if (ret 0) +return ret;
RE: [PATCH 03/15] [media] marvell-ccic: add clock tree support for marvell-ccic driver
Hi, Guennadi We will update it soon. -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 18:50 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 03/15] [media] marvell-ccic: add clock tree support for marvell-ccic driver On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch adds the clock tree support for marvell-ccic. Each board may require different clk enabling sequence. Developer need add the clk_name in correct sequence in board driver to use this feature. Signed-off-by: Libin Yang lby...@marvell.com Signed-off-by: Albert Wang twan...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.h |6 +++ drivers/media/platform/marvell-ccic/mmp-driver.c | 57 ++ include/media/mmp-camera.h |5 ++ 3 files changed, 68 insertions(+) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h index 2d444a1..0df6b1c 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.h +++ b/drivers/media/platform/marvell-ccic/mcam-core.h @@ -88,6 +88,7 @@ struct mmp_frame_state { * the dev_lock spinlock; they are marked as such by comments. * dev_lock is also required for access to device registers. */ +#define NR_MCAM_CLK 4 struct mcam_camera { /* * These fields should be set by the platform code prior to @@ -107,6 +108,11 @@ struct mcam_camera { int (*dphy)[3]; int mipi_enabled; int lane; /* lane number */ + +/* clock tree support */ +struct clk *clk[NR_MCAM_CLK]; +int clk_num; + /* * Callbacks from the core to the platform code. */ diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index 9d7aa79..80977b0 100755 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -104,6 +104,23 @@ static struct mmp_camera *mmpcam_find_device(struct platform_device *pdev) #define REG_CCIC_DCGCR 0x28/* CCIC dyn clock gate ctrl reg */ #define REG_CCIC_CRCR 0x50/* CCIC clk reset ctrl reg */ +static void mcam_clk_set(struct mcam_camera *mcam, int on) { +unsigned int i; + +if (on) { +for (i = 0; i mcam-clk_num; i++) { +if (mcam-clk[i]) From your init below, mcam-clk[i] can be a negative error code. Yes. We will fix it. +clk_enable(mcam-clk[i]); +} +} else { +for (i = 0; i mcam-clk_num; i++) { +if (mcam-clk[i]) +clk_disable(mcam-clk[i]); +} +} +} + /* * Power control. */ @@ -134,6 +151,8 @@ static void mmpcam_power_up(struct mcam_camera *mcam) mdelay(5); gpio_set_value(pdata-sensor_reset_gpio, 1); /* reset is active low */ mdelay(5); + +mcam_clk_set(mcam, 1); } static void mmpcam_power_down(struct mcam_camera *mcam) @@ -151,6 +170,8 @@ static void mmpcam_power_down(struct mcam_camera *mcam) pdata = cam-pdev-dev.platform_data; gpio_set_value(pdata-sensor_power_gpio, 0); gpio_set_value(pdata-sensor_reset_gpio, 0); + +mcam_clk_set(mcam, 0); } /* @@ -229,6 +250,37 @@ static irqreturn_t mmpcam_irq(int irq, void *data) return IRQ_RETVAL(handled); } +static void mcam_init_clk(struct mcam_camera *mcam, +struct mmp_camera_platform_data *pdata, int init) { +unsigned int i; + +if (NR_MCAM_CLK pdata-clk_num) { +dev_warn(mcam-dev, Too many mcam clocks defined\n); +mcam-clk_num = 0; +return; +} + +if (init) { +for (i = 0; i pdata-clk_num; i++) { +if (pdata-clk_name[i] != NULL) +mcam-clk[i] = clk_get(mcam-dev, +pdata-clk_name[i]); +if (IS_ERR(mcam-clk[i])) +dev_warn(mcam-dev, Could not get clk: %s\n, + pdata-clk_name[i]); You issue a warning but continue initialisation, leaving mcam-clk[i] set to an error value. Yes, thanks to point it out. +} +mcam-clk_num = pdata-clk_num; +} else { +for (i = 0; i pdata-clk_num; i++) { +if (mcam-clk[i]) { +clk_put(mcam-clk[i]); +mcam-clk[i] = NULL; +} +} +mcam-clk_num = 0; +} +} Don't think I like this. IIUC, your driver should only try to use clocks, that it knows about, not some random clocks, passed from the platform data. So, you
Re: [PATCH] media: fix a typo CONFIG_HAVE_GENERIC_DMA_COHERENT in videobuf2-dma-contig.c
Em Tue, 27 Nov 2012 14:55:10 +0530 Prabhakar Lad prabhakar.cse...@gmail.com escreveu: Hi, On Tue, Nov 27, 2012 at 1:21 PM, Marek Szyprowski m.szyprow...@samsung.com wrote: Hello, On 11/27/2012 8:39 AM, Prabhakar Lad wrote: Hi Marek, On Tue, Nov 27, 2012 at 12:53 PM, Marek Szyprowski m.szyprow...@samsung.com wrote: Hello, On 11/27/2012 6:59 AM, Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar@ti.com from commit 93049b9368a2e257ace66252ab2cc066f3399cad, which adds a check HAVE_GENERIC_DMA_COHERENT for dma ops, the check was wrongly made it should have been HAVE_GENERIC_DMA_COHERENT but it was CONFIG_HAVE_GENERIC_DMA_COHERENT. This patch fixes the typo, which was causing following build error: videobuf2-dma-contig.c:743: error: 'vb2_dc_get_dmabuf' undeclared here (not in a function) make[3]: *** [drivers/media/v4l2-core/videobuf2-dma-contig.o] Error 1 Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com The CONFIG_HAVE_GENERIC_DMA_COHERENT based patch was a quick workaround for the build problem in linux-next and should be reverted now. The correct patch has been posted for drivers/base/dma-mapping.c to LKML, see http://www.spinics.net/lists/linux-next/msg22890.html I was referring to this patch from Mauro, http://git.linuxtv.org/media_tree.git/commitdiff/93049b9368a2e257ace66252ab2cc066f3399cad which introduced this build error. I know, with my patch the workaround introduced by Mauro is not needed at all. Thanks for clarifying I'll drop this patch, I hope Mauro will revert his changes. Yeah, I'm reverting it right now and applying Marek's one, with Greg's ack. Regards, --Prabhakar Best regards -- Marek Szyprowski Samsung Poland RD Center -- 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 04/15] [media] marvell-ccic: reset ccic phy when stop streaming for stability
Hi, Guennadi Thanks for your review! -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 18:57 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 04/15] [media] marvell-ccic: reset ccic phy when stop streaming for stability On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch adds the reset ccic phy operation when stop streaming. Without reset ccic phy, the next start streaming may be unstable. Also need add CCIC2 definition when PXA688/PXA2128 support dual ccics. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.c |5 + drivers/media/platform/marvell-ccic/mcam-core.h |2 ++ drivers/media/platform/marvell-ccic/mmp-driver.c | 25 ++ 3 files changed, 32 insertions(+) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index b111f0d..760e8ea 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -1053,6 +1053,11 @@ static int mcam_vb_stop_streaming(struct vb2_queue *vq) return -EINVAL; mcam_ctlr_stop_dma(cam); /* + * Reset the CCIC PHY after stopping streaming, + * otherwise, the CCIC may be unstable. + */ +cam-ctlr_reset(cam); Aren't you breaking the cafe driver by calling .ctrl_reset() without checking for NULL? Same holds for your .calc_dphy() callback too. Sorry, it's our mistake. We will fix it soon. +/* * VB2 reclaims the buffers, so we need to forget * about them. */ diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h index 0df6b1c..40368f6 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.h +++ b/drivers/media/platform/marvell-ccic/mcam-core.h @@ -103,6 +103,7 @@ struct mcam_camera { short int use_smbus;/* SMBUS or straight I2c? */ enum mcam_buffer_mode buffer_mode; +int ccic_id; /* MIPI support */ int bus_type; int (*dphy)[3]; @@ -119,6 +120,7 @@ struct mcam_camera { void (*plat_power_up) (struct mcam_camera *cam); void (*plat_power_down) (struct mcam_camera *cam); void (*calc_dphy)(struct mcam_camera *cam); +void (*ctlr_reset)(struct mcam_camera *cam); /* * Everything below here is private to the mcam core and diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index 80977b0..20046d0 100755 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -103,6 +103,7 @@ static struct mmp_camera *mmpcam_find_device(struct platform_device *pdev) #define CPU_SUBSYS_PMU_BASE 0xd4282800 #define REG_CCIC_DCGCR 0x28/* CCIC dyn clock gate ctrl reg */ #define REG_CCIC_CRCR 0x50/* CCIC clk reset ctrl reg */ +#define REG_CCIC2_CRCR 0xf4/* CCIC2 clk reset ctrl reg */ static void mcam_clk_set(struct mcam_camera *mcam, int on) { @@ -174,6 +175,28 @@ static void mmpcam_power_down(struct mcam_camera *mcam) mcam_clk_set(mcam, 0); } +void mcam_ctlr_reset(struct mcam_camera *mcam) { +unsigned long val; +struct mmp_camera *cam = mcam_to_cam(mcam); + +if (mcam-ccic_id) { +/* + * Using CCIC2 + */ +val = ioread32(cam-power_regs + REG_CCIC2_CRCR); +iowrite32(val ~0x2, cam-power_regs + REG_CCIC2_CRCR); +iowrite32(val | 0x2, cam-power_regs + REG_CCIC2_CRCR); +} else { +/* + * Using CCIC1 + */ +val = ioread32(cam-power_regs + REG_CCIC_CRCR); +iowrite32(val ~0x2, cam-power_regs + REG_CCIC_CRCR); +iowrite32(val | 0x2, cam-power_regs + REG_CCIC_CRCR); +} +} + /* * calc the dphy register values * There are three dphy registers being used. @@ -301,9 +324,11 @@ static int mmpcam_probe(struct platform_device *pdev) mcam = cam-mcam; mcam-plat_power_up = mmpcam_power_up; mcam-plat_power_down = mmpcam_power_down; +mcam-ctlr_reset = mcam_ctlr_reset; mcam-calc_dphy = mmpcam_calc_dphy; mcam-dev = pdev-dev; mcam-use_smbus = 0; +mcam-ccic_id = pdev-id; mcam-bus_type = pdata-bus_type; mcam-dphy = (pdata-dphy); mcam-mipi_enabled = 0; -- 1.7.9.5 Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
RE: [PATCH 05/15] [media] marvell-ccic: refine mcam_set_contig_buffer function
Hi, Guennadi -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 19:04 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 05/15] [media] marvell-ccic: refine mcam_set_contig_buffer function On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch refines mcam_set_contig_buffer() in mcam core Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com Looks good in general, just will have to be tested on currently supported platforms, because it changes the order, in which registers are written. So, if this patch is not too important for you, maybe it would be better to drop it, it doesn't seem to improve any functionality? It didn't improve the functionality, just prepare for the 3 frame buffer support patch. :) If it is decided to use this, you can add my Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Thanks Guennadi --- drivers/media/platform/marvell-ccic/mcam-core.c | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 760e8ea..67d4f2f 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -481,22 +481,21 @@ static void mcam_set_contig_buffer(struct mcam_camera *cam, int frame) */ if (list_empty(cam-buffers)) { buf = cam-vb_bufs[frame ^ 0x1]; -cam-vb_bufs[frame] = buf; -mcam_reg_write(cam, frame == 0 ? REG_Y0BAR : REG_Y1BAR, -vb2_dma_contig_plane_dma_addr(buf-vb_buf, 0)); set_bit(CF_SINGLE_BUFFER, cam-flags); cam-frame_state.singles++; -return; +} else { +/* + * OK, we have a buffer we can use. + */ +buf = list_first_entry(cam-buffers, struct mcam_vb_buffer, +queue); +list_del_init(buf-queue); +clear_bit(CF_SINGLE_BUFFER, cam-flags); } -/* - * OK, we have a buffer we can use. - */ -buf = list_first_entry(cam-buffers, struct mcam_vb_buffer, queue); -list_del_init(buf-queue); + +cam-vb_bufs[frame] = buf; mcam_reg_write(cam, frame == 0 ? REG_Y0BAR : REG_Y1BAR, vb2_dma_contig_plane_dma_addr(buf-vb_buf, 0)); -cam-vb_bufs[frame] = buf; -clear_bit(CF_SINGLE_BUFFER, cam-flags); } /* -- 1.7.9.5 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver
On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch adds the new formats support for marvell-ccic. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.c | 178 ++- drivers/media/platform/marvell-ccic/mcam-core.h |6 + 2 files changed, 151 insertions(+), 33 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 67d4f2f..c9f7250 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -110,6 +110,30 @@ static struct mcam_format_struct { .bpp= 2, }, { + .desc = UYVY 4:2:2, + .pixelformat= V4L2_PIX_FMT_UYVY, + .mbus_code = V4L2_MBUS_FMT_UYVY8_2X8, + .bpp= 2, + }, + { + .desc = YUV 4:2:2 PLANAR, + .pixelformat= V4L2_PIX_FMT_YUV422P, + .mbus_code = V4L2_MBUS_FMT_UYVY8_2X8, + .bpp= 2, + }, + { + .desc = YUV 4:2:0 PLANAR, + .pixelformat= V4L2_PIX_FMT_YUV420, + .mbus_code = V4L2_MBUS_FMT_YUYV8_1_5X8, + .bpp= 2, + }, + { + .desc = YVU 4:2:0 PLANAR, + .pixelformat= V4L2_PIX_FMT_YVU420, + .mbus_code = V4L2_MBUS_FMT_YVYU8_1_5X8, + .bpp= 2, + }, + { .desc = RGB 444, .pixelformat= V4L2_PIX_FMT_RGB444, .mbus_code = V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, @@ -168,6 +192,12 @@ struct mcam_dma_desc { u32 segment_len; }; +struct yuv_pointer_t { + dma_addr_t y; + dma_addr_t u; + dma_addr_t v; +}; + /* * Our buffer type for working with videobuf2. Note that the vb2 * developers have decreed that struct vb2_buffer must be at the @@ -179,6 +209,7 @@ struct mcam_vb_buffer { struct mcam_dma_desc *dma_desc; /* Descriptor virtual address */ dma_addr_t dma_desc_pa; /* Descriptor physical address */ int dma_desc_nent; /* Number of mapped descriptors */ + struct yuv_pointer_t yuv_p; }; static inline struct mcam_vb_buffer *vb_to_mvb(struct vb2_buffer *vb) @@ -465,6 +496,18 @@ static inline int mcam_check_dma_buffers(struct mcam_camera *cam) /* * DMA-contiguous code. */ + +static bool mcam_fmt_is_planar(__u32 pfmt) +{ + switch (pfmt) { + case V4L2_PIX_FMT_YUV422P: + case V4L2_PIX_FMT_YUV420: + case V4L2_PIX_FMT_YVU420: + return true; + } + return false; +} + /* * Set up a contiguous buffer for the given frame. Here also is where * the underrun strategy is set: if there is no buffer available, reuse @@ -476,6 +519,8 @@ static inline int mcam_check_dma_buffers(struct mcam_camera *cam) static void mcam_set_contig_buffer(struct mcam_camera *cam, int frame) { struct mcam_vb_buffer *buf; + struct v4l2_pix_format *fmt = cam-pix_format; + /* * If there are no available buffers, go into single mode */ @@ -494,8 +539,13 @@ static void mcam_set_contig_buffer(struct mcam_camera *cam, int frame) } cam-vb_bufs[frame] = buf; - mcam_reg_write(cam, frame == 0 ? REG_Y0BAR : REG_Y1BAR, - vb2_dma_contig_plane_dma_addr(buf-vb_buf, 0)); + mcam_reg_write(cam, frame == 0 ? REG_Y0BAR : REG_Y1BAR, buf-yuv_p.y); + if (mcam_fmt_is_planar(fmt-pixelformat)) { + mcam_reg_write(cam, frame == 0 ? + REG_U0BAR : REG_U1BAR, buf-yuv_p.u); + mcam_reg_write(cam, frame == 0 ? + REG_V0BAR : REG_V1BAR, buf-yuv_p.v); + } } /* @@ -653,49 +703,91 @@ static inline void mcam_sg_restart(struct mcam_camera *cam) */ static void mcam_ctlr_image(struct mcam_camera *cam) { - int imgsz; struct v4l2_pix_format *fmt = cam-pix_format; + u32 widthy = 0, widthuv = 0, imgsz_h, imgsz_w; + + cam_dbg(cam, camera: bytesperline = %d; height = %d\n, + fmt-bytesperline, fmt-sizeimage / fmt-bytesperline); + imgsz_h = (fmt-height IMGSZ_V_SHIFT) IMGSZ_V_MASK; + imgsz_w = fmt-bytesperline IMGSZ_H_MASK; + + if (fmt-pixelformat == V4L2_PIX_FMT_YUV420 + || fmt-pixelformat == V4L2_PIX_FMT_YVU420) + imgsz_w = (fmt-bytesperline * 4 / 3) IMGSZ_H_MASK; + else if (fmt-pixelformat == V4L2_PIX_FMT_JPEG) + imgsz_h = (fmt-sizeimage / fmt-bytesperline) IMGSZ_V_SHIFT; + + switch (fmt-pixelformat) { + case V4L2_PIX_FMT_YUYV: + case V4L2_PIX_FMT_UYVY: +
Re: [PATCH 07/15] [media] marvell-ccic: add SOF / EOF pair check for marvell-ccic driver
On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch adds the SOFx/EOFx pair check for marvell-ccic. When switching format, the last EOF may not arrive when stop streamning. And the EOF will be detected in the next start streaming. Must ensure clear the obsolete frame flags before every really start streaming. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.c | 33 ++- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index c9f7250..16da8ae 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -93,6 +93,9 @@ MODULE_PARM_DESC(buffer_mode, #define CF_CONFIG_NEEDED 4 /* Must configure hardware */ #define CF_SINGLE_BUFFER 5 /* Running with a single buffer */ #define CF_SG_RESTART 6 /* SG restart needed */ +#define CF_FRAME_SOF0 7 /* Frame 0 started */ +#define CF_FRAME_SOF1 8 +#define CF_FRAME_SOF2 9 #define sensor_call(cam, o, f, args...) \ v4l2_subdev_call(cam-sensor, o, f, ##args) @@ -250,8 +253,10 @@ static void mcam_reset_buffers(struct mcam_camera *cam) int i; cam-next_buf = -1; - for (i = 0; i cam-nbufs; i++) + for (i = 0; i cam-nbufs; i++) { clear_bit(i, cam-flags); + clear_bit(CF_FRAME_SOF0 + i, cam-flags); + } } static inline int mcam_needs_config(struct mcam_camera *cam) @@ -1130,6 +1135,7 @@ static void mcam_vb_wait_finish(struct vb2_queue *vq) static int mcam_vb_start_streaming(struct vb2_queue *vq, unsigned int count) { struct mcam_camera *cam = vb2_get_drv_priv(vq); + unsigned int frame; if (cam-state != S_IDLE) { INIT_LIST_HEAD(cam-buffers); @@ -1147,6 +1153,14 @@ static int mcam_vb_start_streaming(struct vb2_queue *vq, unsigned int count) cam-state = S_BUFWAIT; return 0; } + + /* + * Ensure clear the obsolete frame flags + * before every really start streaming + */ + for (frame = 0; frame cam-nbufs; frame++) + clear_bit(CF_FRAME_SOF0 + frame, cam-flags); + return mcam_read_setup(cam); } @@ -1865,9 +1879,11 @@ int mccic_irq(struct mcam_camera *cam, unsigned int irqs) * each time. */ for (frame = 0; frame cam-nbufs; frame++) - if (irqs (IRQ_EOF0 frame)) { + if (irqs (IRQ_EOF0 frame) + test_bit(CF_FRAME_SOF0 + frame, cam-flags)) { mcam_frame_complete(cam, frame); handled = 1; + clear_bit(CF_FRAME_SOF0 + frame, cam-flags); if (cam-buffer_mode == B_DMA_sg) break; } @@ -1876,11 +1892,14 @@ int mccic_irq(struct mcam_camera *cam, unsigned int irqs) * code assumes that we won't get multiple frame interrupts * at once; may want to rethink that. */ - if (irqs (IRQ_SOF0 | IRQ_SOF1 | IRQ_SOF2)) { - set_bit(CF_DMA_ACTIVE, cam-flags); - handled = 1; - if (cam-buffer_mode == B_DMA_sg) - mcam_ctlr_stop(cam); + for (frame = 0; frame cam-nbufs; frame++) { + if (irqs (IRQ_SOF0 frame)) { + set_bit(CF_DMA_ACTIVE, cam-flags); + set_bit(CF_FRAME_SOF0 + frame, cam-flags); + handled = IRQ_HANDLED; + if (cam-buffer_mode == B_DMA_sg) + mcam_ctlr_stop(cam); + } Maybe it would be good to be more careful here. Is it actually possible that more than one IRQ_SOFx bit is set here? It probably is. In this case your loop will perform some actions like calling mcam_ctlr_stop() multiple times. So, maybe you could do something like + for (frame = 0; frame cam-nbufs; frame++) { + if (irqs (IRQ_SOF0 frame)) { + set_bit(CF_FRAME_SOF0 + frame, cam-flags); + handled = IRQ_HANDLED; + } + } + + if (handled == IRQ_HANDLED) { + set_bit(CF_DMA_ACTIVE, cam-flags); + if (cam-buffer_mode == B_DMA_sg) + mcam_ctlr_stop(cam); + } } return handled; } -- 1.7.9.5 Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 08/15] [media] marvell-ccic: switch to resource managed allocation and request
On Fri, 23 Nov 2012, twan...@marvell.com wrote: From: Libin Yang lby...@marvell.com This patch switchs to resource managed allocation and request in mmp-driver. It can remove free resource operations. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com Nice! You can also use devm_gpio_request() :-) Thanks Guennadi --- drivers/media/platform/marvell-ccic/mmp-driver.c | 35 +++--- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index 20046d0..c3031e7 100755 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -315,7 +315,7 @@ static int mmpcam_probe(struct platform_device *pdev) pdata = pdev-dev.platform_data; - cam = kzalloc(sizeof(*cam), GFP_KERNEL); + cam = devm_kzalloc(pdev-dev, sizeof(*cam), GFP_KERNEL); if (cam == NULL) return -ENOMEM; cam-pdev = pdev; @@ -342,14 +342,12 @@ static int mmpcam_probe(struct platform_device *pdev) res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (res == NULL) { dev_err(pdev-dev, no iomem resource!\n); - ret = -ENODEV; - goto out_free; + return -ENODEV; } - mcam-regs = ioremap(res-start, resource_size(res)); + mcam-regs = devm_request_and_ioremap(pdev-dev, res); if (mcam-regs == NULL) { dev_err(pdev-dev, MMIO ioremap fail\n); - ret = -ENODEV; - goto out_free; + return -ENODEV; } /* * Power/clock memory is elsewhere; get it too. Perhaps this @@ -358,14 +356,12 @@ static int mmpcam_probe(struct platform_device *pdev) res = platform_get_resource(pdev, IORESOURCE_MEM, 1); if (res == NULL) { dev_err(pdev-dev, no power resource!\n); - ret = -ENODEV; - goto out_unmap1; + return -ENODEV; } - cam-power_regs = ioremap(res-start, resource_size(res)); + cam-power_regs = devm_request_and_ioremap(pdev-dev, res); if (cam-power_regs == NULL) { dev_err(pdev-dev, power MMIO ioremap fail\n); - ret = -ENODEV; - goto out_unmap1; + return -ENODEV; } mcam_init_clk(mcam, pdata, 1); @@ -375,9 +371,8 @@ static int mmpcam_probe(struct platform_device *pdev) */ mcam-i2c_adapter = platform_get_drvdata(pdata-i2c_device); if (mcam-i2c_adapter == NULL) { - ret = -ENODEV; dev_err(pdev-dev, No i2c adapter\n); - goto out_unmap2; + return -ENODEV; } /* * Sensor GPIO pins. @@ -386,7 +381,7 @@ static int mmpcam_probe(struct platform_device *pdev) if (ret) { dev_err(pdev-dev, Can't get sensor power gpio %d, pdata-sensor_power_gpio); - goto out_unmap2; + return ret; } gpio_direction_output(pdata-sensor_power_gpio, 0); ret = gpio_request(pdata-sensor_reset_gpio, cam-reset); @@ -414,7 +409,7 @@ static int mmpcam_probe(struct platform_device *pdev) goto out_unregister; } cam-irq = res-start; - ret = request_irq(cam-irq, mmpcam_irq, IRQF_SHARED, + ret = devm_request_irq(pdev-dev, cam-irq, mmpcam_irq, IRQF_SHARED, mmp-camera, mcam); if (ret == 0) { mmpcam_add_device(cam); @@ -428,13 +423,7 @@ out_gpio2: gpio_free(pdata-sensor_reset_gpio); out_gpio: gpio_free(pdata-sensor_power_gpio); -out_unmap2: mcam_init_clk(mcam, pdata, 0); - iounmap(cam-power_regs); -out_unmap1: - iounmap(mcam-regs); -out_free: - kfree(cam); return ret; } @@ -445,16 +434,12 @@ static int mmpcam_remove(struct mmp_camera *cam) struct mmp_camera_platform_data *pdata; mmpcam_remove_device(cam); - free_irq(cam-irq, mcam); mccic_shutdown(mcam); mmpcam_power_down(mcam); pdata = cam-pdev-dev.platform_data; gpio_free(pdata-sensor_reset_gpio); gpio_free(pdata-sensor_power_gpio); - iounmap(cam-power_regs); - iounmap(mcam-regs); mcam_init_clk(mcam, pdata, 0); - kfree(cam); return 0; } -- 1.7.9.5 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver
Thanks Albert Wang 86-21-61092656 -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 19:45 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch adds the new formats support for marvell-ccic. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.c | 178 ++- drivers/media/platform/marvell-ccic/mcam-core.h |6 + 2 files changed, 151 insertions(+), 33 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 67d4f2f..c9f7250 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -110,6 +110,30 @@ static struct mcam_format_struct { .bpp= 2, }, { +.desc = UYVY 4:2:2, +.pixelformat= V4L2_PIX_FMT_UYVY, +.mbus_code = V4L2_MBUS_FMT_UYVY8_2X8, +.bpp= 2, +}, +{ +.desc = YUV 4:2:2 PLANAR, +.pixelformat= V4L2_PIX_FMT_YUV422P, +.mbus_code = V4L2_MBUS_FMT_UYVY8_2X8, +.bpp= 2, +}, +{ +.desc = YUV 4:2:0 PLANAR, +.pixelformat= V4L2_PIX_FMT_YUV420, +.mbus_code = V4L2_MBUS_FMT_YUYV8_1_5X8, +.bpp= 2, +}, +{ +.desc = YVU 4:2:0 PLANAR, +.pixelformat= V4L2_PIX_FMT_YVU420, +.mbus_code = V4L2_MBUS_FMT_YVYU8_1_5X8, +.bpp= 2, +}, +{ .desc = RGB 444, .pixelformat= V4L2_PIX_FMT_RGB444, .mbus_code = V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, @@ -168,6 +192,12 @@ struct mcam_dma_desc { u32 segment_len; }; +struct yuv_pointer_t { +dma_addr_t y; +dma_addr_t u; +dma_addr_t v; +}; + /* * Our buffer type for working with videobuf2. Note that the vb2 * developers have decreed that struct vb2_buffer must be at the @@ -179,6 +209,7 @@ struct mcam_vb_buffer { struct mcam_dma_desc *dma_desc; /* Descriptor virtual address */ dma_addr_t dma_desc_pa; /* Descriptor physical address */ int dma_desc_nent; /* Number of mapped descriptors */ +struct yuv_pointer_t yuv_p; }; static inline struct mcam_vb_buffer *vb_to_mvb(struct vb2_buffer *vb) @@ -465,6 +496,18 @@ static inline int mcam_check_dma_buffers(struct mcam_camera *cam) /* * DMA-contiguous code. */ + +static bool mcam_fmt_is_planar(__u32 pfmt) { +switch (pfmt) { +case V4L2_PIX_FMT_YUV422P: +case V4L2_PIX_FMT_YUV420: +case V4L2_PIX_FMT_YVU420: +return true; +} +return false; +} + /* * Set up a contiguous buffer for the given frame. Here also is where * the underrun strategy is set: if there is no buffer available, reuse @@ -476,6 +519,8 @@ static inline int mcam_check_dma_buffers(struct mcam_camera *cam) static void mcam_set_contig_buffer(struct mcam_camera *cam, int frame) { struct mcam_vb_buffer *buf; +struct v4l2_pix_format *fmt = cam-pix_format; + /* * If there are no available buffers, go into single mode */ @@ -494,8 +539,13 @@ static void mcam_set_contig_buffer(struct mcam_camera *cam, int frame) } cam-vb_bufs[frame] = buf; -mcam_reg_write(cam, frame == 0 ? REG_Y0BAR : REG_Y1BAR, -vb2_dma_contig_plane_dma_addr(buf-vb_buf, 0)); +mcam_reg_write(cam, frame == 0 ? REG_Y0BAR : REG_Y1BAR, buf-yuv_p.y); +if (mcam_fmt_is_planar(fmt-pixelformat)) { +mcam_reg_write(cam, frame == 0 ? +REG_U0BAR : REG_U1BAR, buf-yuv_p.u); +mcam_reg_write(cam, frame == 0 ? +REG_V0BAR : REG_V1BAR, buf-yuv_p.v); +} } /* @@ -653,49 +703,91 @@ static inline void mcam_sg_restart(struct mcam_camera *cam) */ static void mcam_ctlr_image(struct mcam_camera *cam) { -int imgsz; struct v4l2_pix_format *fmt = cam-pix_format; +u32 widthy = 0, widthuv = 0, imgsz_h, imgsz_w; + +cam_dbg(cam, camera: bytesperline = %d; height = %d\n, +fmt-bytesperline, fmt-sizeimage / fmt-bytesperline); +imgsz_h = (fmt-height IMGSZ_V_SHIFT) IMGSZ_V_MASK; +imgsz_w = fmt-bytesperline IMGSZ_H_MASK; + +if (fmt-pixelformat == V4L2_PIX_FMT_YUV420 +|| fmt-pixelformat == V4L2_PIX_FMT_YVU420) +imgsz_w = (fmt-bytesperline * 4 / 3)
RE: [PATCH 08/15] [media] marvell-ccic: switch to resource managed allocation and request
Hi, Guennadi -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 20:00 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 08/15] [media] marvell-ccic: switch to resource managed allocation and request On Fri, 23 Nov 2012, twan...@marvell.com wrote: From: Libin Yang lby...@marvell.com This patch switchs to resource managed allocation and request in mmp-driver. It can remove free resource operations. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com Nice! You can also use devm_gpio_request() :-) Fine, it looks we forgot it. We will change it. Thanks Guennadi Thanks Albert Wang 86-21-61092656 --- drivers/media/platform/marvell-ccic/mmp-driver.c | 35 +++--- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index 20046d0..c3031e7 100755 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -315,7 +315,7 @@ static int mmpcam_probe(struct platform_device *pdev) pdata = pdev-dev.platform_data; -cam = kzalloc(sizeof(*cam), GFP_KERNEL); +cam = devm_kzalloc(pdev-dev, sizeof(*cam), GFP_KERNEL); if (cam == NULL) return -ENOMEM; cam-pdev = pdev; @@ -342,14 +342,12 @@ static int mmpcam_probe(struct platform_device *pdev) res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (res == NULL) { dev_err(pdev-dev, no iomem resource!\n); -ret = -ENODEV; -goto out_free; +return -ENODEV; } -mcam-regs = ioremap(res-start, resource_size(res)); +mcam-regs = devm_request_and_ioremap(pdev-dev, res); if (mcam-regs == NULL) { dev_err(pdev-dev, MMIO ioremap fail\n); -ret = -ENODEV; -goto out_free; +return -ENODEV; } /* * Power/clock memory is elsewhere; get it too. Perhaps this @@ -358,14 +356,12 @@ static int mmpcam_probe(struct platform_device *pdev) res = platform_get_resource(pdev, IORESOURCE_MEM, 1); if (res == NULL) { dev_err(pdev-dev, no power resource!\n); -ret = -ENODEV; -goto out_unmap1; +return -ENODEV; } -cam-power_regs = ioremap(res-start, resource_size(res)); +cam-power_regs = devm_request_and_ioremap(pdev-dev, res); if (cam-power_regs == NULL) { dev_err(pdev-dev, power MMIO ioremap fail\n); -ret = -ENODEV; -goto out_unmap1; +return -ENODEV; } mcam_init_clk(mcam, pdata, 1); @@ -375,9 +371,8 @@ static int mmpcam_probe(struct platform_device *pdev) */ mcam-i2c_adapter = platform_get_drvdata(pdata-i2c_device); if (mcam-i2c_adapter == NULL) { -ret = -ENODEV; dev_err(pdev-dev, No i2c adapter\n); -goto out_unmap2; +return -ENODEV; } /* * Sensor GPIO pins. @@ -386,7 +381,7 @@ static int mmpcam_probe(struct platform_device *pdev) if (ret) { dev_err(pdev-dev, Can't get sensor power gpio %d, pdata-sensor_power_gpio); -goto out_unmap2; +return ret; } gpio_direction_output(pdata-sensor_power_gpio, 0); ret = gpio_request(pdata-sensor_reset_gpio, cam-reset); @@ -414,7 +409,7 @@ static int mmpcam_probe(struct platform_device *pdev) goto out_unregister; } cam-irq = res-start; -ret = request_irq(cam-irq, mmpcam_irq, IRQF_SHARED, +ret = devm_request_irq(pdev-dev, cam-irq, mmpcam_irq, +IRQF_SHARED, mmp-camera, mcam); if (ret == 0) { mmpcam_add_device(cam); @@ -428,13 +423,7 @@ out_gpio2: gpio_free(pdata-sensor_reset_gpio); out_gpio: gpio_free(pdata-sensor_power_gpio); -out_unmap2: mcam_init_clk(mcam, pdata, 0); -iounmap(cam-power_regs); -out_unmap1: -iounmap(mcam-regs); -out_free: -kfree(cam); return ret; } @@ -445,16 +434,12 @@ static int mmpcam_remove(struct mmp_camera *cam) struct mmp_camera_platform_data *pdata; mmpcam_remove_device(cam); -free_irq(cam-irq, mcam); mccic_shutdown(mcam); mmpcam_power_down(mcam); pdata = cam-pdev-dev.platform_data; gpio_free(pdata-sensor_reset_gpio); gpio_free(pdata-sensor_power_gpio); -iounmap(cam-power_regs); -iounmap(mcam-regs); mcam_init_clk(mcam, pdata, 0); -kfree(cam); return 0; } -- 1.7.9.5 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a
[PATCH] [media] exynos-gsc: modify number of output/capture buffers
G-Scaler src buffer count as well as destination buffer count is increased to 32. This is required for G-Scaler to interface with MFC, as MFC demands 32 capture buffers for some H264 streams. Signed-off-by: Shaik Ameer Basha shaik.am...@samsung.com --- drivers/media/platform/exynos-gsc/gsc-core.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index cc7b218..4856dd7 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -935,8 +935,8 @@ static struct gsc_variant gsc_v_100_variant = { .pix_max= gsc_v_100_max, .pix_min= gsc_v_100_min, .pix_align = gsc_v_100_align, - .in_buf_cnt = 8, - .out_buf_cnt= 16, + .in_buf_cnt = 32, + .out_buf_cnt= 32, .sc_up_max = 8, .sc_down_max= 16, .poly_sc_down_max = 4, -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 07/15] [media] marvell-ccic: add SOF / EOF pair check for marvell-ccic driver
Hi, Guennadi -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 19:56 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 07/15] [media] marvell-ccic: add SOF / EOF pair check for marvell- ccic driver On Fri, 23 Nov 2012, Albert Wang wrote: From: Libin Yang lby...@marvell.com This patch adds the SOFx/EOFx pair check for marvell-ccic. When switching format, the last EOF may not arrive when stop streamning. And the EOF will be detected in the next start streaming. Must ensure clear the obsolete frame flags before every really start streaming. Signed-off-by: Albert Wang twan...@marvell.com Signed-off-by: Libin Yang lby...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.c | 33 ++- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index c9f7250..16da8ae 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -93,6 +93,9 @@ MODULE_PARM_DESC(buffer_mode, #define CF_CONFIG_NEEDED 4 /* Must configure hardware */ #define CF_SINGLE_BUFFER 5 /* Running with a single buffer */ #define CF_SG_RESTART6 /* SG restart needed */ +#define CF_FRAME_SOF07 /* Frame 0 started */ +#define CF_FRAME_SOF18 +#define CF_FRAME_SOF29 #define sensor_call(cam, o, f, args...) \ v4l2_subdev_call(cam-sensor, o, f, ##args) @@ -250,8 +253,10 @@ static void mcam_reset_buffers(struct mcam_camera *cam) int i; cam-next_buf = -1; -for (i = 0; i cam-nbufs; i++) +for (i = 0; i cam-nbufs; i++) { clear_bit(i, cam-flags); +clear_bit(CF_FRAME_SOF0 + i, cam-flags); +} } static inline int mcam_needs_config(struct mcam_camera *cam) @@ -1130,6 +1135,7 @@ static void mcam_vb_wait_finish(struct vb2_queue *vq) static int mcam_vb_start_streaming(struct vb2_queue *vq, unsigned int count) { struct mcam_camera *cam = vb2_get_drv_priv(vq); +unsigned int frame; if (cam-state != S_IDLE) { INIT_LIST_HEAD(cam-buffers); @@ -1147,6 +1153,14 @@ static int mcam_vb_start_streaming(struct vb2_queue *vq, unsigned int count) cam-state = S_BUFWAIT; return 0; } + +/* + * Ensure clear the obsolete frame flags + * before every really start streaming + */ +for (frame = 0; frame cam-nbufs; frame++) +clear_bit(CF_FRAME_SOF0 + frame, cam-flags); + return mcam_read_setup(cam); } @@ -1865,9 +1879,11 @@ int mccic_irq(struct mcam_camera *cam, unsigned int irqs) * each time. */ for (frame = 0; frame cam-nbufs; frame++) -if (irqs (IRQ_EOF0 frame)) { +if (irqs (IRQ_EOF0 frame) +test_bit(CF_FRAME_SOF0 + frame, cam-flags)) { mcam_frame_complete(cam, frame); handled = 1; +clear_bit(CF_FRAME_SOF0 + frame, cam-flags); if (cam-buffer_mode == B_DMA_sg) break; } @@ -1876,11 +1892,14 @@ int mccic_irq(struct mcam_camera *cam, unsigned int irqs) * code assumes that we won't get multiple frame interrupts * at once; may want to rethink that. */ -if (irqs (IRQ_SOF0 | IRQ_SOF1 | IRQ_SOF2)) { -set_bit(CF_DMA_ACTIVE, cam-flags); -handled = 1; -if (cam-buffer_mode == B_DMA_sg) -mcam_ctlr_stop(cam); +for (frame = 0; frame cam-nbufs; frame++) { +if (irqs (IRQ_SOF0 frame)) { +set_bit(CF_DMA_ACTIVE, cam-flags); +set_bit(CF_FRAME_SOF0 + frame, cam-flags); +handled = IRQ_HANDLED; +if (cam-buffer_mode == B_DMA_sg) +mcam_ctlr_stop(cam); +} Maybe it would be good to be more careful here. Is it actually possible that more than one IRQ_SOFx bit is set here? It probably is. In this case your loop will perform some actions like calling mcam_ctlr_stop() multiple times. So, maybe you could do something like Actually, we thought about this case before we do that. The answer is we think this case won't occur. But we still think your proposal is safer than ours, we will adopt your suggestion. + for (frame = 0; frame cam-nbufs; frame++) { + if (irqs (IRQ_SOF0 frame)) { + set_bit(CF_FRAME_SOF0 + frame, cam-flags); + handled = IRQ_HANDLED; + } + } + + if (handled == IRQ_HANDLED) { + set_bit(CF_DMA_ACTIVE, cam-flags); + if (cam-buffer_mode == B_DMA_sg) +
Re: [RFC v2 2/5] video: panel: Add DPI panel support
Hi, On 2012-11-22 23:45, Laurent Pinchart wrote: +static void panel_dpi_release(struct display_entity *entity) +{ + struct panel_dpi *panel = to_panel_dpi(entity); + + kfree(panel); +} + +static int panel_dpi_remove(struct platform_device *pdev) +{ + struct panel_dpi *panel = platform_get_drvdata(pdev); + + platform_set_drvdata(pdev, NULL); + display_entity_unregister(panel-entity); + + return 0; +} + +static int __devinit panel_dpi_probe(struct platform_device *pdev) +{ + const struct panel_dpi_platform_data *pdata = pdev-dev.platform_data; + struct panel_dpi *panel; + int ret; + + if (pdata == NULL) + return -ENODEV; + + panel = kzalloc(sizeof(*panel), GFP_KERNEL); + if (panel == NULL) + return -ENOMEM; + + panel-pdata = pdata; + panel-entity.dev = pdev-dev; + panel-entity.release = panel_dpi_release; + panel-entity.ops.ctrl = panel_dpi_control_ops; + + ret = display_entity_register(panel-entity); + if (ret 0) { + kfree(panel); + return ret; + } + + platform_set_drvdata(pdev, panel); + + return 0; +} + +static const struct dev_pm_ops panel_dpi_dev_pm_ops = { +}; + +static struct platform_driver panel_dpi_driver = { + .probe = panel_dpi_probe, + .remove = panel_dpi_remove, + .driver = { + .name = panel_dpi, + .owner = THIS_MODULE, + .pm = panel_dpi_dev_pm_ops, + }, +}; I'm not sure of how the free/release works. The release func is called when the ref count drops to zero. But... The object in question, the panel_dpi struct which contains the display entity, is not only about data, it's also about code located in this module. So I don't see anything preventing from unloading this module, while some other component is holding a ref for the display entity. While its holding the ref, it's valid to call ops in the display entity, but the code for the ops in this module is already unloaded. I don't really know how the kref can be used properly in this use case... Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/4] media: mx2_camera: Remove i.mx25 support.
Hi Guennadi/Javier, On Tue, Oct 30, 2012 at 12:57 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Fabio, I wasn't in favour of removing mx25 support initially and I still don't quite fancy it, but the delta is getting too large. If we remove it now you still have the git history, so, you'll be able to restore the latest state before removal. OTOH, it would be easier for me to review a 50-line fix patch, than a 400-line revert-and-fix patch, so, this has an adbantage too. Sorry for the delay. I just added the camera support to mach-mx25_3ds.c (which I will submit it soon to arm kernel list) and it works fine: soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 mx2-camera imx25-camera.0: Camera driver attached to camera 0 ov2640 0-0030: ov2640 Product ID 26:42 Manufacturer ID 7f:a2 i2c i2c-0: OV2640 Probed mx2-camera imx25-camera.0: Camera driver detached from camera 0 mx2-camera imx25-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 2216 Could we please keep the mx25 support? Thanks, Fabio Estevam -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] exynos-gsc: propagate timestamps from src to dst buffers
Hi Sakari, On Sunday 25 November 2012 14:09:50 Sakari Ailus wrote: On Thu, Nov 22, 2012 at 10:44:22PM +0100, Sylwester Nawrocki wrote: the data will not be displayed before this time, secondary to the nominal frame rate determined by the current video standard in enqueued order. Applications can for example zero this field to display frames as soon as possible. The driver stores the time at which the first data byte was actually sent out in the timestamp field. This permits applications to monitor the drift between the video and system clock. In some use cases it might be useful to know exact frame processing time, where driver would be filling OUTPUT and CAPTURE value with exact monotonic clock values corresponding to a frame processing start and end time. Shouldn't this always be done in memory-to-memory processing? I could imagine only performance measurements can benefit from other kind of timestamps. We could use different timestamp type to tell the timestamp source isn't any system clock but an input buffer. What do you think? Yes, it makes sense to me to report with the buffer flag that the source of timestamp is just an OUTPUT buffer. At least this would solve the reporting part of the issue. Oh wait, could applications tell by setting buffer flag what timestamping behaviour they expect from a driver ? I'd prefer not to. Timestamps not involving use of video nodes or buffers would have no way to choose this. Timestamps only make sense if they're all the same kind of, so you can cmopare them, with possibly some exceptions this could be one of. I agree, I'd rather select the timestamp type without involving the buffer. If we used buffer flags to select the timestamp type we would essentially delay timestamp type selection until QBUF time, which could be too late for some devices. In memory-to-memory processing we could possibly also force such timestamps, but that'd require making vb2 timestamp source-aware. I certainly have nothing against that: it's been already planned. Handling queryctrl requirest that, and I prefer to avoid involving drivers in it. (Cc Laurent.) I can't see an important use of timestamping m2m buffers at device drivers. How not important is it then? Performance measurement can probably be done in user space with sufficient accuracy as well. However, it wouldn't be difficult for drivers to I agree. implement multiple time stamping techniques, e.g. OUTPUT - CAPTURE timestamp copying or getting timestamps from monotonic clock at frame processing beginning and end for OUTPUT and CAPTURE respectively. I believe the buffer flags might be a good solution. Have you looked at the monotonic timestamp patches ([PATCH 0/4] Monotonic timestamps)? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2 1/5] video: Add generic display entity core
Hi, On 2012-11-22 23:45, Laurent Pinchart wrote: +/** + * display_entity_get_modes - Get video modes supported by the display entity + * @entity The display entity + * @modes: Pointer to an array of modes + * + * Fill the modes argument with a pointer to an array of video modes. The array + * is owned by the display entity. + * + * Return the number of supported modes on success (including 0 if no mode is + * supported) or a negative error code otherwise. + */ +int display_entity_get_modes(struct display_entity *entity, + const struct videomode **modes) +{ + if (!entity-ops.ctrl || !entity-ops.ctrl-get_modes) + return 0; + + return entity-ops.ctrl-get_modes(entity, modes); +} +EXPORT_SYMBOL_GPL(display_entity_get_modes); + +/** + * display_entity_get_size - Get display entity physical size + * @entity: The display entity + * @width: Physical width in millimeters + * @height: Physical height in millimeters + * + * When applicable, for instance for display panels, retrieve the display + * physical size in millimeters. + * + * Return 0 on success or a negative error code otherwise. + */ +int display_entity_get_size(struct display_entity *entity, + unsigned int *width, unsigned int *height) +{ + if (!entity-ops.ctrl || !entity-ops.ctrl-get_size) + return -EOPNOTSUPP; + + return entity-ops.ctrl-get_size(entity, width, height); +} +EXPORT_SYMBOL_GPL(display_entity_get_size); How do you envision these to be used with, say, DVI monitors with EDID data? Should each panel driver, that manages a device with EDID, read and parse the EDID itself? I guess that shouldn't be too difficult with a common EDID lib, but that will only expose some of the information found from EDID. Should the upper levels also have a way to get the raw EDID data, in addition to funcs like above? Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH 10/15] [media] marvell-ccic: split mcam core into 2 parts for soc_camera support
On Fri, 23 Nov 2012, Albert Wang wrote: This patch splits mcam core into 2 parts to prepare for soc_camera support. The first part remains in mcam-core. This part includes the HW operations and vb2 callback functions. The second part is moved to mcam-core-standard.c. This part is relevant with the implementation of using v4l2. Signed-off-by: Libin Yang lby...@marvell.com Signed-off-by: Albert Wang twan...@marvell.com --- drivers/media/platform/marvell-ccic/Makefile |4 +- .../platform/marvell-ccic/mcam-core-standard.c | 767 + .../platform/marvell-ccic/mcam-core-standard.h | 28 + drivers/media/platform/marvell-ccic/mcam-core.c| 873 +--- drivers/media/platform/marvell-ccic/mcam-core.h| 45 + 5 files changed, 883 insertions(+), 834 deletions(-) create mode 100644 drivers/media/platform/marvell-ccic/mcam-core-standard.c create mode 100644 drivers/media/platform/marvell-ccic/mcam-core-standard.h Nice :-) I hope, you'll excuse me, that I won't be verifying this patch thoroughly, instead, I'll trust you to move the code around without actually changing anything in it. Actually, you did change a couple of things - like replaced printk() with cam_err(), and actually here: + cam_err(cam, marvell-cam: Cafe can't do S/G I/O, \ + attempting vmalloc mode instead\n); and here + cam_warn(cam, Unable to alloc DMA buffers at load \ + will try again later\n); the backslashes are not needed... Also in these declarations: -static inline int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime) +inline int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime) { return 0; } -static inline void mcam_free_dma_bufs(struct mcam_camera *cam) +inline void mcam_free_dma_bufs(struct mcam_camera *cam) { return; } -static inline int mcam_check_dma_buffers(struct mcam_camera *cam) +inline int mcam_check_dma_buffers(struct mcam_camera *cam) { return 0; } please also remove inline. Yet another hunk: -static void mcam_ctlr_stop(struct mcam_camera *cam) +void mcam_ctlr_stop(struct mcam_camera *cam) doesn't seem to be needed. In the header: diff --git a/drivers/media/platform/marvell-ccic/mcam-core-standard.h b/drivers/media/platform/marvell-ccic/mcam-core-standard.h new file mode 100644 index 000..148a1a1 --- /dev/null +++ b/drivers/media/platform/marvell-ccic/mcam-core-standard.h @@ -0,0 +1,28 @@ +/* + * Marvell camera core structures. + * + * Copyright 2011 Jonathan Corbet cor...@lwn.net + */ +extern bool alloc_bufs_at_read; +extern int n_dma_bufs; +extern int buffer_mode; +extern const struct vb2_ops mcam_vb2_sg_ops; +extern const struct vb2_ops mcam_vb2_ops; Do all these variables really have to be exported? If yes - please prefix them all with mcam_... to avoid polluting the kernel name-space. You don't want to make a symbol named like n_dma_bufs or buffer_mode be visible to the entire kernel;-) In function declarations: +extern void mcam_ctlr_stop_dma(struct mcam_camera *cam); +extern int mcam_config_mipi(struct mcam_camera *mcam, int enable); +extern void mcam_ctlr_power_up(struct mcam_camera *cam); +extern void mcam_ctlr_power_down(struct mcam_camera *cam); +extern void mcam_ctlr_init(struct mcam_camera *cam); +extern int mcam_cam_init(struct mcam_camera *cam); +extern void mcam_free_dma_bufs(struct mcam_camera *cam); +extern void mcam_ctlr_dma_sg(struct mcam_camera *cam); +extern void mcam_dma_sg_done(struct mcam_camera *cam, int frame); +extern int mcam_check_dma_buffers(struct mcam_camera *cam); +extern void mcam_set_config_needed(struct mcam_camera *cam, int needed); +extern int __mcam_cam_reset(struct mcam_camera *cam); +extern int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime); +extern void mcam_ctlr_dma_contig(struct mcam_camera *cam); +extern void mcam_dma_contig_done(struct mcam_camera *cam, int frame); +extern void mcam_ctlr_dma_vmalloc(struct mcam_camera *cam); +extern void mcam_vmalloc_done(struct mcam_camera *cam, int frame); the keyword extern isn't needed. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] media: V4L2: add temporary clock helpers
Hello, On Sunday 25 November 2012 13:04:09 Sakari Ailus wrote: Sylwester Nawrocki wrote: On 11/14/2012 02:06 PM, Laurent Pinchart wrote: ... + +static DEFINE_MUTEX(clk_lock); +static LIST_HEAD(v4l2_clk); As Sylwester mentioned, what about s/v4l2_clk/v4l2_clks/ ? Don't you think naming of a static variable isn't important enough? ;-) I think code authors should have enough freedom to at least pick up single vs. plural form:-) clks is too many consonants for my taste, if it were anything important I'd rather agree to clk_head or clk_list or something similar. clk_list makes sense IMO since the clk_ prefis is the same. FWIW, clk_list looks fine for me as well. +void v4l2_clk_put(struct v4l2_clk *clk) +{ +if (!IS_ERR(clk)) +module_put(clk-ops-owner); +} +EXPORT_SYMBOL(v4l2_clk_put); + +int v4l2_clk_enable(struct v4l2_clk *clk) +{ +if (atomic_inc_return(clk-enable) == 1 clk-ops-enable) { +int ret = clk-ops-enable(clk); +if (ret 0) +atomic_dec(clk-enable); +return ret; +} I think you need a spinlock here instead of atomic operations. You could get preempted after atomic_inc_return() and before clk-ops-enable() by another process that would call v4l2_clk_enable(). The function would return with enabling the clock. Sorry, what's the problem then? Our instance will succeed and call -enable() and the preempting instance will see the enable count 1 and just return. The clock is guaranteed to be enabled only after the call has returned. The second caller of v4lw_clk_enable() thus may proceed without the clock being enabled. In principle enable() might also want to sleep, so how about using a mutex for the purpose instead of a spinlock? If enable() needs to sleep we should split the enable call into prepare and enable, like the common clock framework did. I'm pretty sure we won't need to toggle this from interrupt context which is what the clock framework does, AFAIU. Accessing i2c subdevs mandates sleeping already. We might not need to have a mutex either if no driver needs to sleep for this, still I guess this is more likely. I'm ok with both; just thought to mention this. Right, I'm fine with a mutex for now, we'll split enable into enable and prepare later if needed. How about just dropping reference counting from this code entirely ? What would be use cases for multiple users of a single clock ? E.g. multiple sensors case where each one uses same clock provided by a host interface ? If we allow the sensor subdev drivers to be setting the clock frequency and each sensor uses different frequency, then I can't see how this can work reliably. I mean it's the clock's provider that should coordinate and reference count the clock users. If a clock is enabled for one sensor and some other sensor is attempting to set different frequency then the set_rate callback should return an error. The clock provider will need use internally a lock for the clock anyway, and to track the clock reference count too. So I'm inclined to leave all this refcounting bits out to individual clock providers. The common clock framework achieves this through notifiers. That'd be probably overkill in this case. What comes to the implementation now, would it be enough if changing the clock rate would work once after the clock first had users, with this capability renewed once all users are gone? I wonder if enabling the clock should be allowed at all if the rate hasn't been explicitly set. I don't know of a sensor driver which would be able to use a non-predefined clock frequency. OK, maybe we should try to refocus here. The purpose of the V4L2 clock helpers is to get rid of clock-related board callbacks (or other direct clock provider-consumer dependencies) without waiting for the common clock framework to be available on a particular platform. With that in mind, the following problems need to be solved. - Multiple consumers for a single clock A video sensor and the associated lens and/or flash controllers could share the same input clock. How to arbitrate rate requests from the individual consumers is still an open problem, but that's true for the common clock framework too. I wouldn't vote against requiring the common clock framework for this use case. This would mean that most of the locking and reference counting could (but doesn't have to) be dropped from the V4L2 clock helpers (reference counting can still be useful for debugging purposes). - Clock provider/consumer outside of V4L2 If the clock provider isn't a V4L2 device, I think the common clock framework must be used by the consumer to access the clock. Similarly, if the provider is a V4L2 device and the consumer isn't, the common clock framework must be used as well (although I don't think this case will happen in practice,
Re: [PATCH 11/15] [media] marvell-ccic: add soc_camera support in mcam core
On Fri, 23 Nov 2012, twan...@marvell.com wrote: From: Albert Wang twan...@marvell.com This patch adds the soc_camera support in mcam core. It creates the file mcam-core-soc.c and mcam-core-soc.h to support soc_camera in mcam core. To use the soc_camera feature, platform driver, such as mmp-driver.c, should also be modified. Signed-off-by: Libin Yang lby...@marvell.com Signed-off-by: Albert Wang twan...@marvell.com --- drivers/media/platform/marvell-ccic/Makefile |2 + .../media/platform/marvell-ccic/mcam-core-soc.c| 414 .../media/platform/marvell-ccic/mcam-core-soc.h| 19 + drivers/media/platform/marvell-ccic/mcam-core.c| 13 +- drivers/media/platform/marvell-ccic/mcam-core.h| 37 ++ include/media/mmp-camera.h |3 + 6 files changed, 485 insertions(+), 3 deletions(-) create mode 100644 drivers/media/platform/marvell-ccic/mcam-core-soc.c create mode 100644 drivers/media/platform/marvell-ccic/mcam-core-soc.h diff --git a/drivers/media/platform/marvell-ccic/Makefile b/drivers/media/platform/marvell-ccic/Makefile index 595ebdf..b3e87d4 100755 --- a/drivers/media/platform/marvell-ccic/Makefile +++ b/drivers/media/platform/marvell-ccic/Makefile @@ -4,3 +4,5 @@ cafe_ccic-y := cafe-driver.o mcam-core.o obj-$(CONFIG_VIDEO_MMP_CAMERA) += mmp_camera_standard.o mmp_camera_standard-y := mmp-driver.o mcam-core.o mcam-core-standard.o +obj-$(CONFIG_VIDEO_MMP_SOC_CAMERA) += mmp_camera_soc.o +mmp_camera_soc-y := mmp-driver.o mcam-core.o mcam-core-soc.o diff --git a/drivers/media/platform/marvell-ccic/mcam-core-soc.c b/drivers/media/platform/marvell-ccic/mcam-core-soc.c new file mode 100644 index 000..a0df8b4 --- /dev/null +++ b/drivers/media/platform/marvell-ccic/mcam-core-soc.c @@ -0,0 +1,414 @@ +/* + * The Marvell camera soc core. This device appears in a number of settings, + * so it needs platform-specific support outside of the core. + * + * Copyright (C) 2009-2012 Marvell International Ltd. + * + * Author: Libin Yang lby...@marvell.com + * Albert Wang twan...@marvell.com + * + */ +#include linux/kernel.h +#include linux/module.h +#include linux/fs.h +#include linux/mm.h +#include linux/i2c.h +#include linux/interrupt.h You really need the above 3 headers? +#include linux/spinlock.h +#include linux/slab.h +#include linux/device.h +#include linux/wait.h +#include linux/list.h +#include linux/dma-mapping.h +#include linux/delay.h +#include linux/vmalloc.h really? +#include linux/io.h +#include linux/videodev2.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h +#include media/v4l2-chip-ident.h +#include media/videobuf2-vmalloc.h and the 2 above? +#include media/videobuf2-dma-contig.h +#include media/soc_camera.h +#include media/soc_mediabus.h please, double-check, which headers you really need. I don't mean, that only headers needed for compilation should be included, some helpers you should include explicitly, even if they're already included via some other headers. But headers, from which nothing is used, shouldn't be included just in case ;-) + +#include mcam-core.h +#include mcam-core-soc.h + +static const enum v4l2_mbus_pixelcode mcam_def_mbus_code = + V4L2_MBUS_FMT_YUYV8_2X8; + +static const struct soc_mbus_pixelfmt mcam_formats[] = { + { + .fourcc = V4L2_PIX_FMT_UYVY, + .name = YUV422PACKED, + .bits_per_sample = 8, + .packing = SOC_MBUS_PACKING_2X8_PADLO, + .order = SOC_MBUS_ORDER_LE, + }, + { + .fourcc = V4L2_PIX_FMT_YUV422P, + .name = YUV422PLANAR, + .bits_per_sample = 8, + .packing = SOC_MBUS_PACKING_2X8_PADLO, + .order = SOC_MBUS_ORDER_LE, + }, + { + .fourcc = V4L2_PIX_FMT_YUV420, + .name = YUV420PLANAR, + .bits_per_sample = 12, + .packing = SOC_MBUS_PACKING_NONE, + .order = SOC_MBUS_ORDER_LE, + }, + { + .fourcc = V4L2_PIX_FMT_YVU420, + .name = YVU420PLANAR, + .bits_per_sample = 12, + .packing = SOC_MBUS_PACKING_NONE, + .order = SOC_MBUS_ORDER_LE, + }, +}; + +static const struct v4l2_pix_format mcam_def_pix_format = { + .width = VGA_WIDTH, + .height = VGA_HEIGHT, + .pixelformat= V4L2_PIX_FMT_YUYV, + .field = V4L2_FIELD_NONE, + .bytesperline = VGA_WIDTH*2, + .sizeimage = VGA_WIDTH*VGA_HEIGHT*2, +}; Hm, you have the same (or similar) in mcam common code. We can duplicate it for now, but would be good to consolidate later. + +static int mcam_camera_add_device(struct soc_camera_device *icd) +{ + struct soc_camera_host *ici = to_soc_camera_host(icd-parent); + struct mcam_camera
RE: [PATCH 10/15] [media] marvell-ccic: split mcam core into 2 parts for soc_camera support
Hi, Guennadi -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 22:13 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 10/15] [media] marvell-ccic: split mcam core into 2 parts for soc_camera support On Fri, 23 Nov 2012, Albert Wang wrote: This patch splits mcam core into 2 parts to prepare for soc_camera support. The first part remains in mcam-core. This part includes the HW operations and vb2 callback functions. The second part is moved to mcam-core-standard.c. This part is relevant with the implementation of using v4l2. Signed-off-by: Libin Yang lby...@marvell.com Signed-off-by: Albert Wang twan...@marvell.com --- drivers/media/platform/marvell-ccic/Makefile |4 +- .../platform/marvell-ccic/mcam-core-standard.c | 767 + .../platform/marvell-ccic/mcam-core-standard.h | 28 + drivers/media/platform/marvell-ccic/mcam-core.c| 873 +--- drivers/media/platform/marvell-ccic/mcam-core.h| 45 + 5 files changed, 883 insertions(+), 834 deletions(-) create mode 100644 drivers/media/platform/marvell-ccic/mcam-core-standard.c create mode 100644 drivers/media/platform/marvell-ccic/mcam-core-standard.h Nice :-) I hope, you'll excuse me, that I won't be verifying this patch thoroughly, instead, I'll trust you to move the code around without actually changing anything in it. Actually, you did change a couple of things - like replaced printk() with cam_err(), and actually here: +cam_err(cam, marvell-cam: Cafe can't do S/G I/O, \ +attempting vmalloc mode instead\n); and here +cam_warn(cam, Unable to alloc DMA buffers at load \ +will try again later\n); the backslashes are not needed... Also in these declarations: Sorry, I have to clarify it. :) I replaced printk() and add backslashes just because the tool scripts/checkpatch.pl. It will report error when remove the blackslash and report warning when using printk(). But these errors and warnings will be reported only in latest kernel code. :) If you think we can ignore these errors and warnings, I'm OK to get back to the original code. :) -static inline int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime) +inline int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime) { return 0; } -static inline void mcam_free_dma_bufs(struct mcam_camera *cam) +inline void mcam_free_dma_bufs(struct mcam_camera *cam) { return; } -static inline int mcam_check_dma_buffers(struct mcam_camera *cam) +inline int mcam_check_dma_buffers(struct mcam_camera *cam) { return 0; } please also remove inline. Yet another hunk: It looks this is the bug in original code. OK, I can remove inline key word. -static void mcam_ctlr_stop(struct mcam_camera *cam) +void mcam_ctlr_stop(struct mcam_camera *cam) doesn't seem to be needed. In the header: I will re-check it. diff --git a/drivers/media/platform/marvell-ccic/mcam-core-standard.h b/drivers/media/platform/marvell-ccic/mcam-core-standard.h new file mode 100644 index 000..148a1a1 --- /dev/null +++ b/drivers/media/platform/marvell-ccic/mcam-core-standard.h @@ -0,0 +1,28 @@ +/* + * Marvell camera core structures. + * + * Copyright 2011 Jonathan Corbet cor...@lwn.net */ extern bool +alloc_bufs_at_read; extern int n_dma_bufs; extern int buffer_mode; +extern const struct vb2_ops mcam_vb2_sg_ops; extern const struct +vb2_ops mcam_vb2_ops; Do all these variables really have to be exported? If yes - please prefix them all with mcam_... to avoid polluting the kernel name-space. You don't want to make a symbol named like n_dma_bufs or buffer_mode be visible to the entire kernel;-) In function declarations: +extern void mcam_ctlr_stop_dma(struct mcam_camera *cam); extern int +mcam_config_mipi(struct mcam_camera *mcam, int enable); extern void +mcam_ctlr_power_up(struct mcam_camera *cam); extern void +mcam_ctlr_power_down(struct mcam_camera *cam); extern void +mcam_ctlr_init(struct mcam_camera *cam); extern int +mcam_cam_init(struct mcam_camera *cam); extern void +mcam_free_dma_bufs(struct mcam_camera *cam); extern void +mcam_ctlr_dma_sg(struct mcam_camera *cam); extern void +mcam_dma_sg_done(struct mcam_camera *cam, int frame); extern int +mcam_check_dma_buffers(struct mcam_camera *cam); extern void +mcam_set_config_needed(struct mcam_camera *cam, int needed); extern +int __mcam_cam_reset(struct mcam_camera *cam); extern int +mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime); extern +void mcam_ctlr_dma_contig(struct mcam_camera *cam); extern void +mcam_dma_contig_done(struct mcam_camera *cam, int frame); extern void +mcam_ctlr_dma_vmalloc(struct mcam_camera *cam); extern void +mcam_vmalloc_done(struct mcam_camera *cam, int frame); the keyword extern
RE: [PATCH 11/15] [media] marvell-ccic: add soc_camera support in mcam core
Hi, Guennadi Thanks a lot for your continuing support! :) -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 23:11 To: Albert Wang Cc: cor...@lwn.net; Linux Media Mailing List; Libin Yang Subject: Re: [PATCH 11/15] [media] marvell-ccic: add soc_camera support in mcam core On Fri, 23 Nov 2012, twan...@marvell.com wrote: From: Albert Wang twan...@marvell.com This patch adds the soc_camera support in mcam core. It creates the file mcam-core-soc.c and mcam-core-soc.h to support soc_camera in mcam core. To use the soc_camera feature, platform driver, such as mmp-driver.c, should also be modified. Signed-off-by: Libin Yang lby...@marvell.com Signed-off-by: Albert Wang twan...@marvell.com --- drivers/media/platform/marvell-ccic/Makefile |2 + .../media/platform/marvell-ccic/mcam-core-soc.c| 414 .../media/platform/marvell-ccic/mcam-core-soc.h| 19 + drivers/media/platform/marvell-ccic/mcam-core.c| 13 +- drivers/media/platform/marvell-ccic/mcam-core.h| 37 ++ include/media/mmp-camera.h |3 + 6 files changed, 485 insertions(+), 3 deletions(-) create mode 100644 drivers/media/platform/marvell-ccic/mcam-core-soc.c create mode 100644 drivers/media/platform/marvell-ccic/mcam-core-soc.h diff --git a/drivers/media/platform/marvell-ccic/Makefile b/drivers/media/platform/marvell-ccic/Makefile index 595ebdf..b3e87d4 100755 --- a/drivers/media/platform/marvell-ccic/Makefile +++ b/drivers/media/platform/marvell-ccic/Makefile @@ -4,3 +4,5 @@ cafe_ccic-y := cafe-driver.o mcam-core.o obj-$(CONFIG_VIDEO_MMP_CAMERA) += mmp_camera_standard.o mmp_camera_standard-y := mmp-driver.o mcam-core.o mcam-core-standard.o +obj-$(CONFIG_VIDEO_MMP_SOC_CAMERA) += mmp_camera_soc.o +mmp_camera_soc-y := mmp-driver.o mcam-core.o mcam-core-soc.o diff --git a/drivers/media/platform/marvell-ccic/mcam-core-soc.c b/drivers/media/platform/marvell-ccic/mcam-core-soc.c new file mode 100644 index 000..a0df8b4 --- /dev/null +++ b/drivers/media/platform/marvell-ccic/mcam-core-soc.c @@ -0,0 +1,414 @@ +/* + * The Marvell camera soc core. This device appears in a number of settings, + * so it needs platform-specific support outside of the core. + * + * Copyright (C) 2009-2012 Marvell International Ltd. + * + * Author: Libin Yang lby...@marvell.com + * Albert Wang twan...@marvell.com + * + */ +#include linux/kernel.h +#include linux/module.h +#include linux/fs.h +#include linux/mm.h +#include linux/i2c.h +#include linux/interrupt.h You really need the above 3 headers? +#include linux/spinlock.h +#include linux/slab.h +#include linux/device.h +#include linux/wait.h +#include linux/list.h +#include linux/dma-mapping.h +#include linux/delay.h +#include linux/vmalloc.h really? +#include linux/io.h +#include linux/videodev2.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h +#include media/v4l2-chip-ident.h +#include media/videobuf2-vmalloc.h and the 2 above? +#include media/videobuf2-dma-contig.h #include media/soc_camera.h +#include media/soc_mediabus.h please, double-check, which headers you really need. I don't mean, that only headers needed for compilation should be included, some helpers you should include explicitly, even if they're already included via some other headers. But headers, from which nothing is used, shouldn't be included just in case ;-) Sorry, we will double-check them. + +#include mcam-core.h +#include mcam-core-soc.h + +static const enum v4l2_mbus_pixelcode mcam_def_mbus_code = +V4L2_MBUS_FMT_YUYV8_2X8; + +static const struct soc_mbus_pixelfmt mcam_formats[] = { +{ +.fourcc = V4L2_PIX_FMT_UYVY, +.name = YUV422PACKED, +.bits_per_sample = 8, +.packing = SOC_MBUS_PACKING_2X8_PADLO, +.order = SOC_MBUS_ORDER_LE, +}, +{ +.fourcc = V4L2_PIX_FMT_YUV422P, +.name = YUV422PLANAR, +.bits_per_sample = 8, +.packing = SOC_MBUS_PACKING_2X8_PADLO, +.order = SOC_MBUS_ORDER_LE, +}, +{ +.fourcc = V4L2_PIX_FMT_YUV420, +.name = YUV420PLANAR, +.bits_per_sample = 12, +.packing = SOC_MBUS_PACKING_NONE, +.order = SOC_MBUS_ORDER_LE, +}, +{ +.fourcc = V4L2_PIX_FMT_YVU420, +.name = YVU420PLANAR, +.bits_per_sample = 12, +.packing = SOC_MBUS_PACKING_NONE, +.order = SOC_MBUS_ORDER_LE, +}, +}; + +static const struct v4l2_pix_format mcam_def_pix_format = { +.width = VGA_WIDTH, +.height = VGA_HEIGHT, +.pixelformat= V4L2_PIX_FMT_YUYV, +.field = V4L2_FIELD_NONE, +.bytesperline = VGA_WIDTH*2, +.sizeimage
Re: [PATCH 12/15] [media] marvell-ccic: add soc_camera support in mmp driver
On Fri, 23 Nov 2012, Albert Wang wrote: This patch adds the soc_camera support in the platform driver: mmp-driver.c. Specified board driver also should be modified to support soc_camera by passing some platform datas to platform driver. Currently the soc_camera mode in mmp driver only supports B_DMA_contig mode. Signed-off-by: Libin Yang lby...@marvell.com Signed-off-by: Albert Wang twan...@marvell.com --- drivers/media/platform/Makefile |4 +- drivers/media/platform/marvell-ccic/Kconfig | 22 ++ drivers/media/platform/marvell-ccic/mmp-driver.c | 80 +++--- include/media/mmp-camera.h |2 + 4 files changed, 97 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index baaa550..feae003 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -11,8 +11,8 @@ obj-$(CONFIG_VIDEO_TIMBERDALE) += timblogiw.o obj-$(CONFIG_VIDEO_M32R_AR_M64278) += arv.o obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o -obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/ -obj-$(CONFIG_VIDEO_MMP_CAMERA) += marvell-ccic/ + +obj-$(CONFIG_VIDEO_MARVELL_CCIC) += marvell-ccic/ obj-$(CONFIG_VIDEO_OMAP2)+= omap2cam.o obj-$(CONFIG_VIDEO_OMAP3)+= omap3isp/ diff --git a/drivers/media/platform/marvell-ccic/Kconfig b/drivers/media/platform/marvell-ccic/Kconfig index bf739e3..6e3eaa0 100755 --- a/drivers/media/platform/marvell-ccic/Kconfig +++ b/drivers/media/platform/marvell-ccic/Kconfig @@ -1,23 +1,45 @@ +config VIDEO_MARVELL_CCIC + tristate +config VIDEO_MRVL_SOC_CAMERA + tristate The latter you can make a bool + config VIDEO_CAFE_CCIC tristate Marvell 88ALP01 (Cafe) CMOS Camera Controller support depends on PCI I2C VIDEO_V4L2 select VIDEO_OV7670 select VIDEOBUF2_VMALLOC select VIDEOBUF2_DMA_CONTIG + select VIDEO_MARVELL_CCIC ---help--- This is a video4linux2 driver for the Marvell 88ALP01 integrated CMOS camera controller. This is the controller found on first- generation OLPC systems. +choice + prompt Camera support on Marvell MMP + depends on ARCH_MMP VIDEO_V4L2 + optional config VIDEO_MMP_CAMERA tristate Marvell Armada 610 integrated camera controller support depends on ARCH_MMP I2C VIDEO_V4L2 select VIDEO_OV7670 select I2C_GPIO select VIDEOBUF2_DMA_SG + select VIDEO_MARVELL_CCIC ---help--- This is a Video4Linux2 driver for the integrated camera controller found on Marvell Armada 610 application processors (and likely beyond). This is the controller found in OLPC XO 1.75 systems. +config VIDEO_MMP_SOC_CAMERA + bool Marvell MMP camera driver based on SOC_CAMERA + depends on VIDEO_DEV SOC_CAMERA ARCH_MMP VIDEO_V4L2 + select VIDEOBUF2_DMA_CONTIG + select VIDEO_MARVELL_CCIC + select VIDEO_MRVL_SOC_CAMERA + ---help--- + This is a Video4Linux2 driver for the Marvell Mobile Soc + PXA910/PXA688/PXA2128/PXA988 CCIC + (CMOS Camera Interface Controller) +endchoice diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index c3031e7..bea7224 100755 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -4,6 +4,12 @@ * * Copyright 2011 Jonathan Corbet cor...@lwn.net * + * History: + * Support Soc Camera + * Support MIPI interface and Dual CCICs in Soc Camera mode + * Sep-2012: Libin Yang lby...@marvell.com + * Albert Wang twan...@marvell.com + * * This file may be distributed under the terms of the GNU General * Public License, version 2. */ @@ -28,6 +34,10 @@ #include linux/list.h #include linux/pm.h #include linux/clk.h +#include linux/regulator/consumer.h +#include media/videobuf2-dma-contig.h +#include media/soc_camera.h +#include media/soc_mediabus.h #include mcam-core.h @@ -40,6 +50,8 @@ struct mmp_camera { struct platform_device *pdev; struct mcam_camera mcam; struct list_head devlist; + /* will change here */ + struct clk *clk[3]; /* CCIC_GATE, CCIC_RST, CCIC_DBG clocks */ int irq; }; @@ -135,7 +147,9 @@ static void mmpcam_power_up_ctlr(struct mmp_camera *cam) static void mmpcam_power_up(struct mcam_camera *mcam) { struct mmp_camera *cam = mcam_to_cam(mcam); +#ifndef CONFIG_VIDEO_MMP_SOC_CAMERA struct mmp_camera_platform_data *pdata; +#endif /* * Turn on power and clocks to the controller. */ @@ -144,34 +158,40 @@ static void mmpcam_power_up(struct mcam_camera *mcam) * Provide power to the sensor. */ mcam_reg_write(mcam, REG_CLKCTRL, 0x6002); +#ifndef CONFIG_VIDEO_MMP_SOC_CAMERA
Re: [PATCH 13/15] [media] marvell-ccic: add dma burst mode support in marvell-ccic driver
On Fri, 23 Nov 2012, Albert Wang wrote: This patch adds the dma burst size config support for marvell-ccic. Developer can set the dma burst size in specified board driver. Signed-off-by: Libin Yang lby...@marvell.com Signed-off-by: Albert Wang twan...@marvell.com --- .../media/platform/marvell-ccic/mcam-core-soc.c|2 +- drivers/media/platform/marvell-ccic/mcam-core.h|7 --- drivers/media/platform/marvell-ccic/mmp-driver.c | 11 +++ include/media/mmp-camera.h |1 + 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core-soc.c b/drivers/media/platform/marvell-ccic/mcam-core-soc.c index a0df8b4..518e6dc 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core-soc.c +++ b/drivers/media/platform/marvell-ccic/mcam-core-soc.c @@ -100,7 +100,7 @@ static int mcam_camera_add_device(struct soc_camera_device *icd) mcam_ctlr_stop(mcam); mcam_set_config_needed(mcam, 1); mcam_reg_write(mcam, REG_CTRL1, -C1_RESERVED | C1_DMAPOSTED); + mcam-burst | C1_RESERVED | C1_DMAPOSTED); mcam_reg_write(mcam, REG_CLKCTRL, (mcam-mclk_src 29) | mcam-mclk_div); cam_dbg(mcam, camera: set sensor mclk = %dMHz\n, mcam-mclk_min); diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h index e149aa3..999b581 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.h +++ b/drivers/media/platform/marvell-ccic/mcam-core.h @@ -132,6 +132,7 @@ struct mcam_camera { short int use_smbus;/* SMBUS or straight I2c? */ enum mcam_buffer_mode buffer_mode; + int burst; int mclk_min; int mclk_src; int mclk_div; @@ -419,9 +420,9 @@ int mcam_soc_camera_host_register(struct mcam_camera *mcam); #define C1_DESC_3WORD 0x0200 /* Three-word descriptors used */ #defineC1_444ALPHA 0x00f0/* Alpha field in RGB444 */ #defineC1_ALPHA_SHFT 20 -#defineC1_DMAB32 0x/* 32-byte DMA burst */ -#defineC1_DMAB16 0x0200/* 16-byte DMA burst */ -#defineC1_DMAB64 0x0400/* 64-byte DMA burst */ +#defineC1_DMAB64 0x/* 64-byte DMA burst */ +#defineC1_DMAB128 0x0200/* 128-byte DMA burst */ +#defineC1_DMAB256 0x0400/* 256-byte DMA burst */ Was this a bug in the driver or is it a different IP version? Thanks Guennadi #defineC1_DMAB_MASK0x0600 #defineC1_TWOBUFS 0x0800/* Use only two DMA buffers */ #defineC1_PWRDWN 0x1000/* Power down */ diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index bea7224..e840941 100755 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -365,6 +365,17 @@ static int mmpcam_probe(struct platform_device *pdev) mcam-dphy = (pdata-dphy); mcam-mipi_enabled = 0; mcam-lane = pdata-lane; + switch (pdata-dma_burst) { + case 128: + mcam-burst = C1_DMAB128; + break; + case 256: + mcam-burst = C1_DMAB256; + break; + default: + mcam-burst = C1_DMAB64; + break; + } INIT_LIST_HEAD(mcam-buffers); /* diff --git a/include/media/mmp-camera.h b/include/media/mmp-camera.h index 731f81f..7a5e63c 100755 --- a/include/media/mmp-camera.h +++ b/include/media/mmp-camera.h @@ -11,6 +11,7 @@ struct mmp_camera_platform_data { int mclk_src; int mclk_div; int chip_id; + int dma_burst; /* * MIPI support */ -- 1.7.9.5 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 14/15] [media] marvell-ccic: use unsigned int type replace int type
On Fri, 23 Nov 2012, Albert Wang wrote: This patch use unsigned int type replace int type in marvell-ccic. These variables: frame number, buf number, irq... should be unsigned. Several issues to be considered here: * most these variables will never take values INT_MAX, so, this isn't very important * sometimes it is convenient to use a variable, that normally should only take positive values, to also check for negative values. These variables should be signed. * compile-time compatibility: if variables are used as arguments of functions or are compared or assigned to other variables, their types should be compatible. * my old cross-compiler was hiding many such problems, I'm sure at marvell you use new enough compilers to warn you about any such issues:-) So, mainly, just make sure you get no compiler warnings, otherwise it's not very important, IMHO. Thanks Guennadi Signed-off-by: Albert Wang twan...@marvell.com --- .../media/platform/marvell-ccic/mcam-core-soc.h|2 +- .../platform/marvell-ccic/mcam-core-standard.h | 10 - drivers/media/platform/marvell-ccic/mcam-core.c| 22 ++-- drivers/media/platform/marvell-ccic/mcam-core.h|2 +- drivers/media/platform/marvell-ccic/mmp-driver.c |2 +- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core-soc.h b/drivers/media/platform/marvell-ccic/mcam-core-soc.h index a5b5fa6..bff8b2a 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core-soc.h +++ b/drivers/media/platform/marvell-ccic/mcam-core-soc.h @@ -11,7 +11,7 @@ extern const struct vb2_ops mcam_soc_vb2_ops; extern void mcam_ctlr_power_up(struct mcam_camera *cam); extern void mcam_ctlr_power_down(struct mcam_camera *cam); -extern void mcam_dma_contig_done(struct mcam_camera *cam, int frame); +extern void mcam_dma_contig_done(struct mcam_camera *cam, unsigned int frame); extern void mcam_ctlr_stop(struct mcam_camera *cam); extern int mcam_config_mipi(struct mcam_camera *mcam, int enable); extern void mcam_ctlr_image(struct mcam_camera *cam); diff --git a/drivers/media/platform/marvell-ccic/mcam-core-standard.h b/drivers/media/platform/marvell-ccic/mcam-core-standard.h index 148a1a1..090c1a2 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core-standard.h +++ b/drivers/media/platform/marvell-ccic/mcam-core-standard.h @@ -4,8 +4,8 @@ * Copyright 2011 Jonathan Corbet cor...@lwn.net */ extern bool alloc_bufs_at_read; -extern int n_dma_bufs; -extern int buffer_mode; +extern unsigned int n_dma_bufs; +extern unsigned int buffer_mode; extern const struct vb2_ops mcam_vb2_sg_ops; extern const struct vb2_ops mcam_vb2_ops; @@ -17,12 +17,12 @@ extern void mcam_ctlr_init(struct mcam_camera *cam); extern int mcam_cam_init(struct mcam_camera *cam); extern void mcam_free_dma_bufs(struct mcam_camera *cam); extern void mcam_ctlr_dma_sg(struct mcam_camera *cam); -extern void mcam_dma_sg_done(struct mcam_camera *cam, int frame); +extern void mcam_dma_sg_done(struct mcam_camera *cam, unsigned int frame); extern int mcam_check_dma_buffers(struct mcam_camera *cam); extern void mcam_set_config_needed(struct mcam_camera *cam, int needed); extern int __mcam_cam_reset(struct mcam_camera *cam); extern int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime); extern void mcam_ctlr_dma_contig(struct mcam_camera *cam); -extern void mcam_dma_contig_done(struct mcam_camera *cam, int frame); +extern void mcam_dma_contig_done(struct mcam_camera *cam, unsigned int frame); extern void mcam_ctlr_dma_vmalloc(struct mcam_camera *cam); -extern void mcam_vmalloc_done(struct mcam_camera *cam, int frame); +extern void mcam_vmalloc_done(struct mcam_camera *cam, unsigned int frame); diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 3b05d8c..2d200d6 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -111,7 +111,7 @@ static inline struct mcam_vb_buffer *vb_to_mvb(struct vb2_buffer *vb) /* * Hand a completed buffer back to user space. */ -static void mcam_buffer_done(struct mcam_camera *cam, int frame, +static void mcam_buffer_done(struct mcam_camera *cam, unsigned int frame, struct vb2_buffer *vbuf) { vbuf-v4l2_buf.bytesused = cam-pix_format.sizeimage; @@ -125,7 +125,7 @@ static void mcam_buffer_done(struct mcam_camera *cam, int frame, */ static void mcam_reset_buffers(struct mcam_camera *cam) { - int i; + unsigned int i; cam-next_buf = -1; for (i = 0; i cam-nbufs; i++) { @@ -216,7 +216,7 @@ int mcam_config_mipi(struct mcam_camera *mcam, int enable) */ int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime) { - int i; + unsigned int i; mcam_set_config_needed(cam,
Re: [PATCH v1.2 1/4] v4l: Define video buffer flags for timestamp types
Hello, On Thursday 22 November 2012 01:59:00 Sakari Ailus wrote: On Wed, Nov 21, 2012 at 11:53:02PM +0100, Hans Verkuil wrote: On Wed November 21 2012 20:13:22 Sakari Ailus wrote: Define video buffer flags for different timestamp types. Everything up to now have used either realtime clock or monotonic clock, without a way to tell which clock the timestamp was taken from. Also document that the clock source of the timestamp in the timestamp field depends on buffer flags. Signed-off-by: Sakari Ailus sakari.ai...@iki.fi Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Acked-by: Hans Verkuil hans.verk...@cisco.com Thanks! :-) But see my comments below for a separate matter... --- Since v1.1: - Change the description of the timestamp field; say that the type of the timestamp is dependent on the flags field. Documentation/DocBook/media/v4l/compat.xml | 12 ++ Documentation/DocBook/media/v4l/io.xml | 53 + Documentation/DocBook/media/v4l/v4l2.xml | 12 ++- include/uapi/linux/videodev2.h |4 ++ 4 files changed, 69 insertions(+), 12 deletions(-) diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml index 4fdf6b5..651ca52 100644 --- a/Documentation/DocBook/media/v4l/compat.xml +++ b/Documentation/DocBook/media/v4l/compat.xml @@ -2477,6 +2477,18 @@ that used it. It was originally scheduled for removal in 2.6.35. /orderedlist /section +section + titleV4L2 in Linux 3.8/title + orderedlist +listitem + paraAdded timestamp types to + structfieldflags/structfield field in + structnamev4l2_buffer/structname. See xref + linkend=buffer-flags /./para +/listitem + /orderedlist +/section + section id=other titleRelation of V4L2 to other Linux multimedia APIs/title diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml index 7e2f3d7..1243fa1 100644 --- a/Documentation/DocBook/media/v4l/io.xml +++ b/Documentation/DocBook/media/v4l/io.xml @@ -582,17 +582,19 @@ applications when an output stream./entry entrystruct timeval/entry entrystructfieldtimestamp/structfield/entry entry/entry - entryparaFor input streams this is the -system time (as returned by the functiongettimeofday()/function -function) when the first data byte was captured. For output streams -the data will not be displayed before this time, secondary to the -nominal frame rate determined by the current video standard in -enqueued order. Applications can for example zero this field to -display frames as soon as possible. The driver stores the time at -which the first data byte was actually sent out in the -structfieldtimestamp/structfield field. This permits -applications to monitor the drift between the video and system -clock./para/entry + entryparaFor input streams this is time when the first data + byte was captured, What should we do with this? In most drivers the timestamp is actually the time that the *last* byte was captured. The reality is that the application doesn't know whether it is the first or the last. One option is to add a new flag for this, or to leave it open. The last makes me uncomfortable, since there can be quite a difference between the time of the first or last byte, and that definitely has an effect on the A/V sync. Very true. I'd also prefer to have this defined so the information would be available to the user space. This is a separate topic that should be handled in a separate patch, but I do think we need to take a closer look at this. I'm not against one more buffer flag to tell which one it is. :-) There are hardly any other options than the frame start and frame end. On the other hand, the FRAME_SYNC event is supported by some drivers and that can be used to obtain the timestamp from frame start. Not all drivers support it nor the applications can be expected to use this just to get a timestamp, though. as returned by the + functionclock_gettime()/function function for the relevant + clock id; see constantV4L2_BUF_FLAG_TIMESTAMP_*/constant in + xref linkend=buffer-flags /. For output streams the data + will not be displayed before this time, secondary to the nominal + frame rate determined by the current video standard in enqueued + order. Applications can for example zero this field to display + frames as soon as possible. There is not a single driver that supports this feature. There is also no way an application can query the driver whether this feature is supported. Personally I don't think this should be the task of a driver anyway: if you want to
RE: [PATCH 12/15] [media] marvell-ccic: add soc_camera support in mmp driver
Hi, Guennadi -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 23:54 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 12/15] [media] marvell-ccic: add soc_camera support in mmp driver On Fri, 23 Nov 2012, Albert Wang wrote: This patch adds the soc_camera support in the platform driver: mmp-driver.c. Specified board driver also should be modified to support soc_camera by passing some platform datas to platform driver. Currently the soc_camera mode in mmp driver only supports B_DMA_contig mode. Signed-off-by: Libin Yang lby...@marvell.com Signed-off-by: Albert Wang twan...@marvell.com --- drivers/media/platform/Makefile |4 +- drivers/media/platform/marvell-ccic/Kconfig | 22 ++ drivers/media/platform/marvell-ccic/mmp-driver.c | 80 +++--- include/media/mmp-camera.h |2 + 4 files changed, 97 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index baaa550..feae003 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -11,8 +11,8 @@ obj-$(CONFIG_VIDEO_TIMBERDALE) += timblogiw.o obj-$(CONFIG_VIDEO_M32R_AR_M64278) += arv.o obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o -obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/ -obj-$(CONFIG_VIDEO_MMP_CAMERA) += marvell-ccic/ + +obj-$(CONFIG_VIDEO_MARVELL_CCIC) += marvell-ccic/ obj-$(CONFIG_VIDEO_OMAP2) += omap2cam.o obj-$(CONFIG_VIDEO_OMAP3) += omap3isp/ diff --git a/drivers/media/platform/marvell-ccic/Kconfig b/drivers/media/platform/marvell-ccic/Kconfig index bf739e3..6e3eaa0 100755 --- a/drivers/media/platform/marvell-ccic/Kconfig +++ b/drivers/media/platform/marvell-ccic/Kconfig @@ -1,23 +1,45 @@ +config VIDEO_MARVELL_CCIC + tristate +config VIDEO_MRVL_SOC_CAMERA + tristate The latter you can make a bool OK. + config VIDEO_CAFE_CCIC tristate Marvell 88ALP01 (Cafe) CMOS Camera Controller support depends on PCI I2C VIDEO_V4L2 select VIDEO_OV7670 select VIDEOBUF2_VMALLOC select VIDEOBUF2_DMA_CONTIG +select VIDEO_MARVELL_CCIC ---help--- This is a video4linux2 driver for the Marvell 88ALP01 integrated CMOS camera controller. This is the controller found on first- generation OLPC systems. +choice +prompt Camera support on Marvell MMP +depends on ARCH_MMP VIDEO_V4L2 +optional config VIDEO_MMP_CAMERA tristate Marvell Armada 610 integrated camera controller support depends on ARCH_MMP I2C VIDEO_V4L2 select VIDEO_OV7670 select I2C_GPIO select VIDEOBUF2_DMA_SG +select VIDEO_MARVELL_CCIC ---help--- This is a Video4Linux2 driver for the integrated camera controller found on Marvell Armada 610 application processors (and likely beyond). This is the controller found in OLPC XO 1.75 systems. +config VIDEO_MMP_SOC_CAMERA +bool Marvell MMP camera driver based on SOC_CAMERA +depends on VIDEO_DEV SOC_CAMERA ARCH_MMP VIDEO_V4L2 +select VIDEOBUF2_DMA_CONTIG +select VIDEO_MARVELL_CCIC +select VIDEO_MRVL_SOC_CAMERA +---help--- + This is a Video4Linux2 driver for the Marvell Mobile Soc + PXA910/PXA688/PXA2128/PXA988 CCIC + (CMOS Camera Interface Controller) endchoice diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index c3031e7..bea7224 100755 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -4,6 +4,12 @@ * * Copyright 2011 Jonathan Corbet cor...@lwn.net * + * History: + * Support Soc Camera + * Support MIPI interface and Dual CCICs in Soc Camera mode + * Sep-2012: Libin Yang lby...@marvell.com + * Albert Wang twan...@marvell.com + * * This file may be distributed under the terms of the GNU General * Public License, version 2. */ @@ -28,6 +34,10 @@ #include linux/list.h #include linux/pm.h #include linux/clk.h +#include linux/regulator/consumer.h #include +media/videobuf2-dma-contig.h #include media/soc_camera.h #include +media/soc_mediabus.h #include mcam-core.h @@ -40,6 +50,8 @@ struct mmp_camera { struct platform_device *pdev; struct mcam_camera mcam; struct list_head devlist; +/* will change here */ +struct clk *clk[3]; /* CCIC_GATE, CCIC_RST, CCIC_DBG clocks */ int irq; }; @@ -135,7 +147,9 @@ static void mmpcam_power_up_ctlr(struct mmp_camera *cam) static void mmpcam_power_up(struct mcam_camera *mcam) { struct mmp_camera *cam = mcam_to_cam(mcam); +#ifndef CONFIG_VIDEO_MMP_SOC_CAMERA struct mmp_camera_platform_data *pdata; +#endif /* * Turn on power and clocks to
RE: [PATCH 13/15] [media] marvell-ccic: add dma burst mode support in marvell-ccic driver
Hi, Guennadi -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 23:56 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 13/15] [media] marvell-ccic: add dma burst mode support in marvell-ccic driver On Fri, 23 Nov 2012, Albert Wang wrote: This patch adds the dma burst size config support for marvell-ccic. Developer can set the dma burst size in specified board driver. Signed-off-by: Libin Yang lby...@marvell.com Signed-off-by: Albert Wang twan...@marvell.com --- .../media/platform/marvell-ccic/mcam-core-soc.c|2 +- drivers/media/platform/marvell-ccic/mcam-core.h|7 --- drivers/media/platform/marvell-ccic/mmp-driver.c | 11 +++ include/media/mmp-camera.h |1 + 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core-soc.c b/drivers/media/platform/marvell-ccic/mcam-core-soc.c index a0df8b4..518e6dc 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core-soc.c +++ b/drivers/media/platform/marvell-ccic/mcam-core-soc.c @@ -100,7 +100,7 @@ static int mcam_camera_add_device(struct soc_camera_device *icd) mcam_ctlr_stop(mcam); mcam_set_config_needed(mcam, 1); mcam_reg_write(mcam, REG_CTRL1, - C1_RESERVED | C1_DMAPOSTED); +mcam-burst | C1_RESERVED | C1_DMAPOSTED); mcam_reg_write(mcam, REG_CLKCTRL, (mcam-mclk_src 29) | mcam-mclk_div); cam_dbg(mcam, camera: set sensor mclk = %dMHz\n, mcam-mclk_min); diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h index e149aa3..999b581 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.h +++ b/drivers/media/platform/marvell-ccic/mcam-core.h @@ -132,6 +132,7 @@ struct mcam_camera { short int use_smbus;/* SMBUS or straight I2c? */ enum mcam_buffer_mode buffer_mode; +int burst; int mclk_min; int mclk_src; int mclk_div; @@ -419,9 +420,9 @@ int mcam_soc_camera_host_register(struct mcam_camera *mcam); #define C1_DESC_3WORD 0x0200/* Three-word descriptors used */ #define C1_444ALPHA 0x00f0/* Alpha field in RGB444 */ #define C1_ALPHA_SHFT 20 -#define C1_DMAB32 0x/* 32-byte DMA burst */ -#define C1_DMAB16 0x0200/* 16-byte DMA burst */ -#define C1_DMAB64 0x0400/* 64-byte DMA burst */ +#define C1_DMAB64 0x/* 64-byte DMA burst */ +#define C1_DMAB128 0x0200/* 128-byte DMA burst */ +#define C1_DMAB256 0x0400/* 256-byte DMA burst */ Was this a bug in the driver or is it a different IP version? I think it's a bug in old code. We didn't find the definition in our specs. Thanks Guennadi #define C1_DMAB_MASK0x0600 #define C1_TWOBUFS 0x0800/* Use only two DMA buffers */ #define C1_PWRDWN 0x1000/* Power down */ diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c index bea7224..e840941 100755 --- a/drivers/media/platform/marvell-ccic/mmp-driver.c +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c @@ -365,6 +365,17 @@ static int mmpcam_probe(struct platform_device *pdev) mcam-dphy = (pdata-dphy); mcam-mipi_enabled = 0; mcam-lane = pdata-lane; +switch (pdata-dma_burst) { +case 128: +mcam-burst = C1_DMAB128; +break; +case 256: +mcam-burst = C1_DMAB256; +break; +default: +mcam-burst = C1_DMAB64; +break; +} INIT_LIST_HEAD(mcam-buffers); /* diff --git a/include/media/mmp-camera.h b/include/media/mmp-camera.h index 731f81f..7a5e63c 100755 --- a/include/media/mmp-camera.h +++ b/include/media/mmp-camera.h @@ -11,6 +11,7 @@ struct mmp_camera_platform_data { int mclk_src; int mclk_div; int chip_id; +int dma_burst; /* * MIPI support */ -- 1.7.9.5 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 14/15] [media] marvell-ccic: use unsigned int type replace int type
Hi, Guennadi -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Wednesday, 28 November, 2012 00:02 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 14/15] [media] marvell-ccic: use unsigned int type replace int type On Fri, 23 Nov 2012, Albert Wang wrote: This patch use unsigned int type replace int type in marvell-ccic. These variables: frame number, buf number, irq... should be unsigned. Several issues to be considered here: * most these variables will never take values INT_MAX, so, this isn't very important * sometimes it is convenient to use a variable, that normally should only take positive values, to also check for negative values. These variables should be signed. * compile-time compatibility: if variables are used as arguments of functions or are compared or assigned to other variables, their types should be compatible. * my old cross-compiler was hiding many such problems, I'm sure at marvell you use new enough compilers to warn you about any such issues:-) So, mainly, just make sure you get no compiler warnings, otherwise it's not very important, IMHO. Yes, usually you are right. For frame number and buf number, the one of prime reasons is it is prepared for the next patch: add 3 frame buffer support, we must use unsigned type in some operations. :) For irq, it's for compiler warning. :) Thanks Guennadi Thanks Albert Wang 86-21-61092656 Signed-off-by: Albert Wang twan...@marvell.com --- .../media/platform/marvell-ccic/mcam-core-soc.h|2 +- .../platform/marvell-ccic/mcam-core-standard.h | 10 - drivers/media/platform/marvell-ccic/mcam-core.c| 22 ++-- drivers/media/platform/marvell-ccic/mcam-core.h|2 +- drivers/media/platform/marvell-ccic/mmp-driver.c |2 +- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core-soc.h b/drivers/media/platform/marvell-ccic/mcam-core-soc.h index a5b5fa6..bff8b2a 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core-soc.h +++ b/drivers/media/platform/marvell-ccic/mcam-core-soc.h @@ -11,7 +11,7 @@ extern const struct vb2_ops mcam_soc_vb2_ops; extern void mcam_ctlr_power_up(struct mcam_camera *cam); extern void mcam_ctlr_power_down(struct mcam_camera *cam); -extern void mcam_dma_contig_done(struct mcam_camera *cam, int frame); +extern void mcam_dma_contig_done(struct mcam_camera *cam, unsigned +int frame); extern void mcam_ctlr_stop(struct mcam_camera *cam); extern int mcam_config_mipi(struct mcam_camera *mcam, int enable); extern void mcam_ctlr_image(struct mcam_camera *cam); diff --git a/drivers/media/platform/marvell-ccic/mcam-core-standard.h b/drivers/media/platform/marvell-ccic/mcam-core-standard.h index 148a1a1..090c1a2 100644 --- a/drivers/media/platform/marvell-ccic/mcam-core-standard.h +++ b/drivers/media/platform/marvell-ccic/mcam-core-standard.h @@ -4,8 +4,8 @@ * Copyright 2011 Jonathan Corbet cor...@lwn.net */ extern bool alloc_bufs_at_read; -extern int n_dma_bufs; -extern int buffer_mode; +extern unsigned int n_dma_bufs; +extern unsigned int buffer_mode; extern const struct vb2_ops mcam_vb2_sg_ops; extern const struct vb2_ops mcam_vb2_ops; @@ -17,12 +17,12 @@ extern void mcam_ctlr_init(struct mcam_camera *cam); extern int mcam_cam_init(struct mcam_camera *cam); extern void mcam_free_dma_bufs(struct mcam_camera *cam); extern void mcam_ctlr_dma_sg(struct mcam_camera *cam); -extern void mcam_dma_sg_done(struct mcam_camera *cam, int frame); +extern void mcam_dma_sg_done(struct mcam_camera *cam, unsigned int +frame); extern int mcam_check_dma_buffers(struct mcam_camera *cam); extern void mcam_set_config_needed(struct mcam_camera *cam, int needed); extern int __mcam_cam_reset(struct mcam_camera *cam); extern int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime); extern void mcam_ctlr_dma_contig(struct mcam_camera *cam); -extern void mcam_dma_contig_done(struct mcam_camera *cam, int frame); +extern void mcam_dma_contig_done(struct mcam_camera *cam, unsigned +int frame); extern void mcam_ctlr_dma_vmalloc(struct mcam_camera *cam); -extern void mcam_vmalloc_done(struct mcam_camera *cam, int frame); +extern void mcam_vmalloc_done(struct mcam_camera *cam, unsigned int +frame); diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 3b05d8c..2d200d6 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -111,7 +111,7 @@ static inline struct mcam_vb_buffer *vb_to_mvb(struct vb2_buffer *vb) /* * Hand a completed buffer back to user space. */ -static void mcam_buffer_done(struct mcam_camera *cam, int frame, +static void mcam_buffer_done(struct mcam_camera *cam, unsigned int +frame, struct
Re: [RFC] Selections targets at V4L2 video mem-to-mem interface
On Thursday 22 November 2012 14:25:18 Shaik Ameer Basha wrote: On Wed, Nov 7, 2012 at 3:52 AM, Sylwester Nawrocki wrote: Hi All, I'd like to clarify the meaning of selection targets on a mem-to-mem video device, in order to document it and to make sure new m2m drivers get it right, and also that the existing ones, using originally the crop ioctls, are converted to the selection ioctls properly. Until the selections API was introduced we used the CROP ioctls to configure cropping on OUTPUT buffer queue and composition onto CAPTURE buffer. Looking at Figure 1.2, [1] it seems obvious that there should be applied following mapping of the CROP to SELECTION ioctls: S_CROP(V4L2_BUF_TYPE_VIDEO_OUTPUT) - S_SELECTION(V4L2_BUF_TYPE_VIDEO_OUTPUT, V4L2_SEL_TGT_CROP) S_CROP(V4L2_BUF_TYPE_VIDEO_CAPTURE) - S_SELECTION(V4L2_BUF_TYPE_VIDEO_CAPTURE, V4L2_SEL_TGT_COMPOSE) And that's how selections are currently documented at video output and capture interfaces: -- *Configuration of video output* For output devices targets and ioctls are used similarly to the video capture case. The composing rectangle refers to the insertion of an image into avideo signal. The cropping rectangles refer to a memory buffer. *Configuration of video capture* ... The top left corner, width and height of the source rectangle, that is the area actually sampled, is given by the V4L2_SEL_TGT_CROP target. ... The composing targets refer to a memory buffer. -- If we apply this mapping, then current VIDIOC_S/G_CROP - VIDIOC_S/G_SELECTION ioctl fallback code wouldn't be valid, as we have there, e.g. static int v4l_s_crop(const struct v4l2_ioctl_ops *ops, struct file *file, void *fh, void *arg) { struct v4l2_crop *p = arg; struct v4l2_selection s = { .type = p-type, .r = p-c, }; if (ops-vidioc_s_crop) return ops-vidioc_s_crop(file, fh, p); /* simulate capture crop using selection api */ /* crop means compose for output devices */ if (V4L2_TYPE_IS_OUTPUT(p-type)) s.target = V4L2_SEL_TGT_COMPOSE_ACTIVE; else s.target = V4L2_SEL_TGT_CROP_ACTIVE; return ops-vidioc_s_selection(file, fh, s); } i.e. it does exactly opposite to what we would expect for M2M. You are right. Instead of handling this confusion in driver, as you mentioned, we can use vfl_dir field to select the target before sending it to the driver. apart from using this vfl_dir field, I can't able to see any other solution here. One possible solution would be to get hold of struct video_device and do proper targets conversion after checking the vfl_dir field. Does anyone have suggestions on this ? As the video_device can easily be retrieved with video_devdata(file) I think that's the easiest solution (and the only practical one I can see as well). BTW, we still have some V4L2_SEL_TGT*_ACTIVE symbols left, I'll write a patch to clean this up. [1] http://hverkuil.home.xs4all.nl/spec/media.html#idp9025504 [2] http://hverkuil.home.xs4all.nl/spec/media.html#idp9031840 -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/15] [media] marvell-ccic: add 3 frame buffers support in DMA_CONTIG mode
On Fri, 23 Nov 2012, Albert Wang wrote: This patch adds support of 3 frame buffers in DMA-contiguous mode. In current DMA_CONTIG mode, only 2 frame buffers can be supported. Actually, Marvell CCIC can support at most 3 frame buffers. Currently 2 frame buffers mode will be used by default. To use 3 frame buffers mode, can do: define MAX_FRAME_BUFS 3 in mcam-core.h Signed-off-by: Albert Wang twan...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.c | 59 +-- drivers/media/platform/marvell-ccic/mcam-core.h | 11 + 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 2d200d6..3b75594 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -401,13 +401,32 @@ static void mcam_set_contig_buffer(struct mcam_camera *cam, unsigned int frame) struct mcam_vb_buffer *buf; struct v4l2_pix_format *fmt = cam-pix_format; - /* - * If there are no available buffers, go into single mode - */ if (list_empty(cam-buffers)) { - buf = cam-vb_bufs[frame ^ 0x1]; - set_bit(CF_SINGLE_BUFFER, cam-flags); - cam-frame_state.singles++; + /* + * If there are no available buffers + * go into single buffer mode + * + * If CCIC use Two Buffers mode + * will use another remaining frame buffer + * frame 0 - buf 1 + * frame 1 - buf 0 + * + * If CCIC use Three Buffers mode + * will use the 2rd remaining frame buffer + * frame 0 - buf 2 + * frame 1 - buf 0 + * frame 2 - buf 1 + */ + buf = cam-vb_bufs[(frame + (MAX_FRAME_BUFS - 1)) + % MAX_FRAME_BUFS]; + if (cam-frame_state.tribufs == 0) + cam-frame_state.tribufs++; TBH, I don't understand what the tribuf field means and what it is doing. Could you explain a bit? + else { + set_bit(CF_SINGLE_BUFFER, cam-flags); + cam-frame_state.singles++; + if (cam-frame_state.tribufs 2) + cam-frame_state.tribufs++; This seems to be the only location, where tribuf affects the control flow. So, it looks like, it controls, if no more buffers are on the queue, wheather you need to set the CF_SINGLE_BUFFER flag and increment the singles count. Thanks Guennadi + } } else { /* * OK, we have a buffer we can use. @@ -416,15 +435,15 @@ static void mcam_set_contig_buffer(struct mcam_camera *cam, unsigned int frame) queue); list_del_init(buf-queue); clear_bit(CF_SINGLE_BUFFER, cam-flags); + if (cam-frame_state.tribufs != (3 - MAX_FRAME_BUFS)) + cam-frame_state.tribufs--; } cam-vb_bufs[frame] = buf; - mcam_reg_write(cam, frame == 0 ? REG_Y0BAR : REG_Y1BAR, buf-yuv_p.y); + mcam_reg_write(cam, REG_Y0BAR + (frame 2), buf-yuv_p.y); if (mcam_fmt_is_planar(fmt-pixelformat)) { - mcam_reg_write(cam, frame == 0 ? - REG_U0BAR : REG_U1BAR, buf-yuv_p.u); - mcam_reg_write(cam, frame == 0 ? - REG_V0BAR : REG_V1BAR, buf-yuv_p.v); + mcam_reg_write(cam, REG_U0BAR + (frame 2), buf-yuv_p.u); + mcam_reg_write(cam, REG_V0BAR + (frame 2), buf-yuv_p.v); } } @@ -433,10 +452,14 @@ static void mcam_set_contig_buffer(struct mcam_camera *cam, unsigned int frame) */ void mcam_ctlr_dma_contig(struct mcam_camera *cam) { - mcam_reg_set_bit(cam, REG_CTRL1, C1_TWOBUFS); - cam-nbufs = 2; - mcam_set_contig_buffer(cam, 0); - mcam_set_contig_buffer(cam, 1); + unsigned int frame; + + cam-nbufs = MAX_FRAME_BUFS; + for (frame = 0; frame cam-nbufs; frame++) + mcam_set_contig_buffer(cam, frame); + + if (cam-nbufs == 2) + mcam_reg_set_bit(cam, REG_CTRL1, C1_TWOBUFS); } /* @@ -1043,6 +1066,12 @@ static int mcam_vb_start_streaming(struct vb2_queue *vq, unsigned int count) for (frame = 0; frame cam-nbufs; frame++) clear_bit(CF_FRAME_SOF0 + frame, cam-flags); + /* + * If CCIC use Two Buffers mode, init tribufs == 1 + * If CCIC use Three Buffers mode, init tribufs == 0 + */ + cam-frame_state.tribufs = 3 - MAX_FRAME_BUFS; + return mcam_read_setup(cam); } diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h
Re: [yavta PATCH 0/2] Timestamp type support
Hi Sakari, On Friday 16 November 2012 00:09:33 Sakari Ailus wrote: Hi, This patchset adds timestamp type printing to yavta. Thank you for the patches. I'll apply them when timestamp type support will reach mainline (I will then update 1/2 with the mainline kernel headers). -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [yavta PATCH 2/2] Print v4l2_buffer timestamp type
Hi Sakari, Thanks for the patch. On Friday 16 November 2012 00:09:44 Sakari Ailus wrote: Signed-off-by: Sakari Ailus sakari.ai...@iki.fi --- yavta.c | 12 +++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/yavta.c b/yavta.c index bf3e096..a50f11e 100644 --- a/yavta.c +++ b/yavta.c @@ -464,6 +464,7 @@ static int video_alloc_buffers(struct device *dev, int nbufs, /* Map the buffers. */ for (i = 0; i rb.count; ++i) { + const char *ts_type = invalid; Any reason for not moving this to a default case ? That shouldn't be the most common case, it will nearly always get overwritten so that's hardly an optimization :-) memset(buf, 0, sizeof buf); buf.index = i; buf.type = dev-type; @@ -474,7 +475,16 @@ static int video_alloc_buffers(struct device *dev, int nbufs, strerror(errno), errno); return ret; } - printf(length: %u offset: %u\n, buf.length, buf.m.offset); + switch (buf.flags V4L2_BUF_FLAG_TIMESTAMP_MASK) { + case V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN: + ts_type = unknown; + break; + case V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC: + ts_type = monotonic; + break; + } + printf(length: %u offset: %u timestamp type: %s\n, +buf.length, buf.m.offset, ts_type); switch (dev-memtype) { case V4L2_MEMORY_MMAP: -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 10/15] [media] marvell-ccic: split mcam core into 2 parts for soc_camera support
On Tue, 27 Nov 2012, Albert Wang wrote: [snip] you did change a couple of things - like replaced printk() with cam_err(), and actually here: + cam_err(cam, marvell-cam: Cafe can't do S/G I/O, \ + attempting vmalloc mode instead\n); and here + cam_warn(cam, Unable to alloc DMA buffers at load \ + will try again later\n); the backslashes are not needed... Also in these declarations: Sorry, I have to clarify it. :) I replaced printk() and add backslashes just because the tool scripts/checkpatch.pl. It will report error when remove the blackslash and report warning when using printk(). But these errors and warnings will be reported only in latest kernel code. :) If you think we can ignore these errors and warnings, I'm OK to get back to the original code. :) Replacing printk() with cam_*() is ok, just please remove the backslashes. Actually, there are also spaces missing in above strings - when they'll be pasted together. As for checkpatch, I would ignore this its warning, because this is not new code, this has been there also in the original driver, you're just moving the code around. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 15/15] [media] marvell-ccic: add 3 frame buffers support in DMA_CONTIG mode
Hi, Guennadi Thanks a lot for your review! :) -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Wednesday, 28 November, 2012 00:30 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 15/15] [media] marvell-ccic: add 3 frame buffers support in DMA_CONTIG mode On Fri, 23 Nov 2012, Albert Wang wrote: This patch adds support of 3 frame buffers in DMA-contiguous mode. In current DMA_CONTIG mode, only 2 frame buffers can be supported. Actually, Marvell CCIC can support at most 3 frame buffers. Currently 2 frame buffers mode will be used by default. To use 3 frame buffers mode, can do: define MAX_FRAME_BUFS 3 in mcam-core.h Signed-off-by: Albert Wang twan...@marvell.com --- drivers/media/platform/marvell-ccic/mcam-core.c | 59 +-- drivers/media/platform/marvell-ccic/mcam-core.h | 11 + 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c index 2d200d6..3b75594 100755 --- a/drivers/media/platform/marvell-ccic/mcam-core.c +++ b/drivers/media/platform/marvell-ccic/mcam-core.c @@ -401,13 +401,32 @@ static void mcam_set_contig_buffer(struct mcam_camera *cam, unsigned int frame) struct mcam_vb_buffer *buf; struct v4l2_pix_format *fmt = cam-pix_format; -/* - * If there are no available buffers, go into single mode - */ if (list_empty(cam-buffers)) { -buf = cam-vb_bufs[frame ^ 0x1]; -set_bit(CF_SINGLE_BUFFER, cam-flags); -cam-frame_state.singles++; +/* + * If there are no available buffers + * go into single buffer mode + * + * If CCIC use Two Buffers mode + * will use another remaining frame buffer + * frame 0 - buf 1 + * frame 1 - buf 0 + * + * If CCIC use Three Buffers mode + * will use the 2rd remaining frame buffer + * frame 0 - buf 2 + * frame 1 - buf 0 + * frame 2 - buf 1 + */ +buf = cam-vb_bufs[(frame + (MAX_FRAME_BUFS - 1)) +% MAX_FRAME_BUFS]; +if (cam-frame_state.tribufs == 0) +cam-frame_state.tribufs++; TBH, I don't understand what the tribuf field means and what it is doing. Could you explain a bit? Yes, in the first version, I just use tribufs in the 3 frame buffer mode. Then I consolidated the controls of 2 buffers mode and 3 buffers mode according to Jonathan's suggestion. But still continue to use the tribufs, maybe it's confused. +else { +set_bit(CF_SINGLE_BUFFER, cam-flags); +cam-frame_state.singles++; +if (cam-frame_state.tribufs 2) +cam-frame_state.tribufs++; This seems to be the only location, where tribuf affects the control flow. So, it looks like, it controls, if no more buffers are on the queue, wheather you need to set the CF_SINGLE_BUFFER flag and increment the singles count. Yes, the tribufs indicates which conditions we need set the single buffer flag. Thanks Guennadi +} } else { /* * OK, we have a buffer we can use. @@ -416,15 +435,15 @@ static void mcam_set_contig_buffer(struct mcam_camera *cam, unsigned int frame) queue); list_del_init(buf-queue); clear_bit(CF_SINGLE_BUFFER, cam-flags); +if (cam-frame_state.tribufs != (3 - MAX_FRAME_BUFS)) +cam-frame_state.tribufs--; } cam-vb_bufs[frame] = buf; -mcam_reg_write(cam, frame == 0 ? REG_Y0BAR : REG_Y1BAR, buf-yuv_p.y); +mcam_reg_write(cam, REG_Y0BAR + (frame 2), buf-yuv_p.y); if (mcam_fmt_is_planar(fmt-pixelformat)) { -mcam_reg_write(cam, frame == 0 ? -REG_U0BAR : REG_U1BAR, buf-yuv_p.u); -mcam_reg_write(cam, frame == 0 ? -REG_V0BAR : REG_V1BAR, buf-yuv_p.v); +mcam_reg_write(cam, REG_U0BAR + (frame 2), buf-yuv_p.u); +mcam_reg_write(cam, REG_V0BAR + (frame 2), buf-yuv_p.v); } } @@ -433,10 +452,14 @@ static void mcam_set_contig_buffer(struct mcam_camera *cam, unsigned int frame) */ void mcam_ctlr_dma_contig(struct mcam_camera *cam) { -mcam_reg_set_bit(cam, REG_CTRL1, C1_TWOBUFS); -cam-nbufs = 2; -mcam_set_contig_buffer(cam, 0); -mcam_set_contig_buffer(cam, 1); +unsigned int frame; + +cam-nbufs = MAX_FRAME_BUFS; +for (frame = 0; frame cam-nbufs; frame++) +mcam_set_contig_buffer(cam, frame); + +if (cam-nbufs == 2) +mcam_reg_set_bit(cam,
Re: need help debugging ISP problem
Hi Adam, On Monday 05 November 2012 16:02:08 Adam Wozniak wrote: I'm working with a custom board based on an Overo WaterStorm com. The processor is a DM3730. The kernel is 2.6.32 based. 2.6.32 very probably means you're using the old TI driver. Please don't. That's buggy and totally unsupported. I advice upgrading to the latest mainline kernel. I'm trying to stress test the camera ISP by rapidly opening and closing the video device with ( while true; do gst-launch v4l2src device=/dev/video0 ! video/x-raw-yuv,width=320,height=240 ! ffmpegcolorspace ! pngenc snapshot=true ! fakesink; done ) After many iterations, I will see the kernel spit out: [ 2502.802795] Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa0bce04 [ 2502.810516] Internal error: : 1028 [#1] [ ... ] [ 2502.846893] PC is at isp_reg_readl+0x18/0x20 [ 2502.851196] LR is at isp_reg_readl+0x10/0x20 [ ... ] [ 2503.296447] [c02954a0] (isp_reg_readl+0x18/0x20) from [c02954f8] (isp_reg_and_or+0x1c/0x38) [ 2503.305206] [c02954f8] (isp_reg_and_or+0x1c/0x38) from [c029bad0] (isppreview_config_cfa+0x38/0x90) [ 2503.314666] [c029bad0] (isppreview_config_cfa+0x38/0x90) from [c029bc5c] (isppreview_config_datapath+0x134/0x330) [ 2503.325347] [c029bc5c] (isppreview_config_datapath+0x134/0x330) from [c029be68] (isppreview_s_pipeline+0x10/0xd0) [ 2503.336029] [c029be68] (isppreview_s_pipeline+0x10/0xd0) from [c0296e98] (isp_s_pipeline+0x1d8/0x280) [ 2503.345672] [c0296e98] (isp_s_pipeline+0x1d8/0x280) from [bf036b98] (cammux_streamon+0x218/0xa28 [cammux]) [ ... ] The register we're trying to access here is the ISP PRV_PCR. If I try to add debug code to read ISP_CTRL right before the fault, the ISP_CTRL access faults in the same way (i.e. the whole ISP is borked, not just the previewer). At first I thought the clocks were being disabled somehow, but tracking them seems to indicate that's not the case. Adding an early return in arch/arm/mach/omap2/clock.c omap2_dflt_clk_disable() (i.e. to disable disabling of clocks) does NOT help. What else might I be missing? What is necessary to be able to read the ISP registers? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 11/15] [media] marvell-ccic: add soc_camera support in mcam core
On Tue, 27 Nov 2012, Albert Wang wrote: [snip] +static int mcam_camera_set_fmt(struct soc_camera_device *icd, +struct v4l2_format *f) +{ +struct soc_camera_host *ici = to_soc_camera_host(icd-parent); +struct mcam_camera *mcam = ici-priv; +const struct soc_camera_format_xlate *xlate = NULL; +struct v4l2_mbus_framefmt mf; +struct v4l2_pix_format *pix = f-fmt.pix; +int ret = 0; No need to initialise ret. Yes, but it looks there is no bad impact if we initialize it. :) I just want to keep the rule: initialize it before use it. :) No, please, don't. Firstly, it adds bloat. Secondly, such blind initialisation can hide real bugs: the variable is initialised, so the compiler doesn't complain, then you use it in your code, but in reality, the value, that you used is meaningless in your context and you get a hidden bug. [snip] +static int mcam_camera_try_fmt(struct soc_camera_device *icd, +struct v4l2_format *f) +{ +struct soc_camera_host *ici = to_soc_camera_host(icd-parent); +struct mcam_camera *mcam = ici-priv; +const struct soc_camera_format_xlate *xlate; +struct v4l2_pix_format *pix = f-fmt.pix; +struct v4l2_mbus_framefmt mf; +__u32 pixfmt = pix-pixelformat; +int ret = 0; No need to initialise ret. + +xlate = soc_camera_xlate_by_fourcc(icd, pixfmt); +if (!xlate) { +cam_err(mcam, camera: format: %c not found\n, +pix-pixelformat); +return -EINVAL; You shouldn't fail .try_fmt() (unless something really bad happens). Just pick up a default supported format. Do you means we just need pick up the default supported format when try_fmt()? If you don't find the requested format - yes, just pick up any format, that you can support. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 10/15] [media] marvell-ccic: split mcam core into 2 parts for soc_camera support
Hi, Guennadi -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Wednesday, 28 November, 2012 00:39 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: RE: [PATCH 10/15] [media] marvell-ccic: split mcam core into 2 parts for soc_camera support On Tue, 27 Nov 2012, Albert Wang wrote: [snip] you did change a couple of things - like replaced printk() with cam_err(), and actually here: + cam_err(cam, marvell-cam: Cafe can't do S/G I/O, \ + attempting vmalloc mode instead\n); and here + cam_warn(cam, Unable to alloc DMA buffers at load \ + will try again later\n); the backslashes are not needed... Also in these declarations: Sorry, I have to clarify it. :) I replaced printk() and add backslashes just because the tool scripts/checkpatch.pl. It will report error when remove the blackslash and report warning when using printk(). But these errors and warnings will be reported only in latest kernel code. :) If you think we can ignore these errors and warnings, I'm OK to get back to the original code. :) Replacing printk() with cam_*() is ok, just please remove the backslashes. Actually, there are also spaces missing in above strings - when they'll be pasted together. As for checkpatch, I would ignore this its warning, because this is not new code, this has been there also in the original driver, you're just moving the code around. OK, I will follow up your suggestion. :) Thanks a lot for pointing out so many improvements in our patches. :) Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 11/15] [media] marvell-ccic: add soc_camera support in mcam core
Thanks Albert Wang 86-21-61092656 -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Wednesday, 28 November, 2012 00:50 To: Albert Wang Cc: cor...@lwn.net; Linux Media Mailing List; Libin Yang Subject: RE: [PATCH 11/15] [media] marvell-ccic: add soc_camera support in mcam core On Tue, 27 Nov 2012, Albert Wang wrote: [snip] +static int mcam_camera_set_fmt(struct soc_camera_device *icd, +struct v4l2_format *f) { +struct soc_camera_host *ici = to_soc_camera_host(icd-parent); +struct mcam_camera *mcam = ici-priv; +const struct soc_camera_format_xlate *xlate = NULL; +struct v4l2_mbus_framefmt mf; +struct v4l2_pix_format *pix = f-fmt.pix; +int ret = 0; No need to initialise ret. Yes, but it looks there is no bad impact if we initialize it. :) I just want to keep the rule: initialize it before use it. :) No, please, don't. Firstly, it adds bloat. Secondly, such blind initialisation can hide real bugs: the variable is initialised, so the compiler doesn't complain, then you use it in your code, but in reality, the value, that you used is meaningless in your context and you get a hidden bug. I have to agree with you at the second reason. So, I will double check them in our patches and remove them in the next version. [snip] +static int mcam_camera_try_fmt(struct soc_camera_device *icd, +struct v4l2_format *f) { +struct soc_camera_host *ici = to_soc_camera_host(icd-parent); +struct mcam_camera *mcam = ici-priv; +const struct soc_camera_format_xlate *xlate; +struct v4l2_pix_format *pix = f-fmt.pix; +struct v4l2_mbus_framefmt mf; +__u32 pixfmt = pix-pixelformat; +int ret = 0; No need to initialise ret. + +xlate = soc_camera_xlate_by_fourcc(icd, pixfmt); +if (!xlate) { +cam_err(mcam, camera: format: %c not found\n, +pix-pixelformat); +return -EINVAL; You shouldn't fail .try_fmt() (unless something really bad happens). Just pick up a default supported format. Do you means we just need pick up the default supported format when try_fmt()? If you don't find the requested format - yes, just pick up any format, that you can support. OK. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 0/15] [media] marvell-ccic: add soc camera support on marvell-ccic
Hi Laxman Just a general comment: this patch series is a huge improvement over the previous versions, now it is actually already reviewable! :-) Thanks for keeping on with this work! Best regards Guennadi On Fri, 23 Nov 2012, Albert Wang wrote: The following patches series will add soc camera support on marvell-ccic Change log v2: - remove register definition patch - split big patch to some small patches - split mcam-core.c to mcam-core.c and mcam-core-standard.c - add mcam-core-soc.c for soc camera support - split 3 frame buffers support patch into 2 patches [PATCH 01/15] [media] marvell-ccic: use internal variable replace [PATCH 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver [PATCH 03/15] [media] marvell-ccic: add clock tree support for marvell-ccic driver [PATCH 04/15] [media] marvell-ccic: reset ccic phy when stop streaming for stability [PATCH 05/15] [media] marvell-ccic: refine mcam_set_contig_buffer function [PATCH 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver [PATCH 07/15] [media] marvell-ccic: add SOF / EOF pair check for marvell-ccic driver [PATCH 08/15] [media] marvell-ccic: switch to resource managed allocation and request [PATCH 09/15] [media] marvell-ccic: refine vb2_ops for marvell-ccic driver [PATCH 10/15] [media] marvell-ccic: split mcam core into 2 parts for soc_camera support [PATCH 11/15] [media] marvell-ccic: add soc_camera support in mcam core [PATCH 12/15] [media] marvell-ccic: add soc_camera support in mmp driver [PATCH 13/15] [media] marvell-ccic: add dma burst mode support in marvell-ccic driver [PATCH 14/15] [media] marvell-ccic: use unsigned int type replace int type [PATCH 15/15] [media] marvell-ccic: add 3 frame buffers support in DMA_CONTIG mode v1: [PATCH 1/4] [media] mmp: add register definition for marvell ccic [PATCH 2/4] [media] marvell-ccic: core: add soc camera support on marvell-ccic mcam-core [PATCH 3/4] [media] marvell-ccic: mmp: add soc camera support on marvell-ccic mmp-driver [PATCH 4/4] [media] marvell-ccic: core: add 3 frame buffers support in DMA_CONTIG mode Thanks Albert Wang --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 0/15] [media] marvell-ccic: add soc camera support on marvell-ccic
Hi, Geunnadi Thank you very much for your review! Your help is very valuable for us. We will work out the next version based on your suggestion! I hope to get back to you on the review of the version 3 patch series next week. :) Have a nice day! Thanks Albert Wang 86-21-61092656 -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Wednesday, 28 November, 2012 01:16 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 0/15] [media] marvell-ccic: add soc camera support on marvell-ccic Hi Laxman Just a general comment: this patch series is a huge improvement over the previous versions, now it is actually already reviewable! :-) Thanks for keeping on with this work! Best regards Guennadi On Fri, 23 Nov 2012, Albert Wang wrote: The following patches series will add soc camera support on marvell-ccic Change log v2: - remove register definition patch - split big patch to some small patches - split mcam-core.c to mcam-core.c and mcam-core-standard.c - add mcam-core-soc.c for soc camera support - split 3 frame buffers support patch into 2 patches [PATCH 01/15] [media] marvell-ccic: use internal variable replace [PATCH 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver [PATCH 03/15] [media] marvell-ccic: add clock tree support for marvell-ccic driver [PATCH 04/15] [media] marvell-ccic: reset ccic phy when stop streaming for stability [PATCH 05/15] [media] marvell-ccic: refine mcam_set_contig_buffer function [PATCH 06/15] [media] marvell-ccic: add new formats support for marvell-ccic driver [PATCH 07/15] [media] marvell-ccic: add SOF / EOF pair check for marvell-ccic driver [PATCH 08/15] [media] marvell-ccic: switch to resource managed allocation and request [PATCH 09/15] [media] marvell-ccic: refine vb2_ops for marvell-ccic driver [PATCH 10/15] [media] marvell-ccic: split mcam core into 2 parts for soc_camera support [PATCH 11/15] [media] marvell-ccic: add soc_camera support in mcam core [PATCH 12/15] [media] marvell-ccic: add soc_camera support in mmp driver [PATCH 13/15] [media] marvell-ccic: add dma burst mode support in marvell-ccic driver [PATCH 14/15] [media] marvell-ccic: use unsigned int type replace int type [PATCH 15/15] [media] marvell-ccic: add 3 frame buffers support in DMA_CONTIG mode v1: [PATCH 1/4] [media] mmp: add register definition for marvell ccic [PATCH 2/4] [media] marvell-ccic: core: add soc camera support on marvell-ccic mcam-core [PATCH 3/4] [media] marvell-ccic: mmp: add soc camera support on marvell-ccic mmp-driver [PATCH 4/4] [media] marvell-ccic: core: add 3 frame buffers support in DMA_CONTIG mode Thanks Albert Wang --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/2] [media] rc: unlock on error in show_protocols()
We recently introduced a new return -ENODEV in this function but we need to unlock before returning. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index 601d1ac1..d593bc6 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -789,8 +789,10 @@ static ssize_t show_protocols(struct device *device, } else if (dev-raw) { enabled = dev-raw-enabled_protocols; allowed = ir_raw_get_allowed_protocols(); - } else + } else { + mutex_unlock(dev-lock); return -ENODEV; + } IR_dprintk(1, allowed - 0x%llx, enabled - 0x%llx\n, (long long)allowed, -- 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] [media] rc: unlock on error in store_protocols()
This error path is missing the unlock. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index 601d1ac1..759a40a 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -890,7 +892,8 @@ static ssize_t store_protocols(struct device *device, if (i == ARRAY_SIZE(proto_names)) { IR_dprintk(1, Unknown protocol: '%s'\n, tmp); - return -EINVAL; + ret = -EINVAL; + goto out; } count++; -- 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: linux-next: Tree for Nov 27 (media/v4l2-core/videobuf2-dma-contig.c)
On 11/26/2012 10:25 PM, Stephen Rothwell wrote: Hi all, Changes since 20121126: on i386: drivers/media/v4l2-core/videobuf2-dma-contig.c:743:16: error: 'vb2_dc_get_dmabuf' undeclared here (not in a function) Full randconfig file is attached. -- ~Randy # # Automatically generated file; DO NOT EDIT. # Linux/i386 3.7.0-rc7 Kernel Configuration # # CONFIG_64BIT is not set CONFIG_X86_32=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_OUTPUT_FORMAT=elf32-i386 CONFIG_ARCH_DEFCONFIG=arch/x86/configs/i386_defconfig CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_HAVE_LATENCYTOP_SUPPORT=y CONFIG_MMU=y CONFIG_NEED_DMA_MAP_STATE=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_BUG=y CONFIG_GENERIC_HWEIGHT=y CONFIG_GENERIC_GPIO=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_ARCH_HAS_CPU_RELAX=y CONFIG_ARCH_HAS_DEFAULT_IDLE=y CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y CONFIG_ARCH_HAS_CPU_AUTOPROBE=y CONFIG_HAVE_SETUP_PER_CPU_AREA=y CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y CONFIG_ARCH_HIBERNATION_POSSIBLE=y CONFIG_ARCH_SUSPEND_POSSIBLE=y # CONFIG_ZONE_DMA32 is not set # CONFIG_AUDIT_ARCH is not set CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y CONFIG_X86_32_SMP=y CONFIG_X86_HT=y CONFIG_X86_32_LAZY_GS=y CONFIG_ARCH_HWEIGHT_CFLAGS=-fcall-saved-ecx -fcall-saved-edx CONFIG_ARCH_SUPPORTS_UPROBES=y CONFIG_DEFCONFIG_LIST=/lib/modules/$UNAME_RELEASE/.config CONFIG_HAVE_IRQ_WORK=y CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y # # General setup # CONFIG_EXPERIMENTAL=y CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_CROSS_COMPILE= CONFIG_LOCALVERSION= # CONFIG_LOCALVERSION_AUTO is not set CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y # CONFIG_KERNEL_GZIP is not set # CONFIG_KERNEL_BZIP2 is not set CONFIG_KERNEL_LZMA=y # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set CONFIG_DEFAULT_HOSTNAME=(none) # CONFIG_SWAP is not set CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y CONFIG_FHANDLE=y CONFIG_HAVE_GENERIC_HARDIRQS=y # # IRQ subsystem # CONFIG_GENERIC_HARDIRQS=y CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_GENERIC_PENDING_IRQ=y CONFIG_GENERIC_IRQ_CHIP=y CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_FORCED_THREADING=y CONFIG_SPARSE_IRQ=y CONFIG_CLOCKSOURCE_WATCHDOG=y CONFIG_KTIME_SCALAR=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BUILD=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y CONFIG_GENERIC_CMOS_UPDATE=y # # Timers subsystem # CONFIG_TICK_ONESHOT=y # CONFIG_NO_HZ is not set CONFIG_HIGH_RES_TIMERS=y # # CPU/Task time and stats accounting # # CONFIG_TICK_CPU_ACCOUNTING is not set CONFIG_IRQ_TIME_ACCOUNTING=y # CONFIG_BSD_PROCESS_ACCT is not set # # RCU Subsystem # CONFIG_TREE_RCU=y # CONFIG_PREEMPT_RCU is not set CONFIG_RCU_FANOUT=32 CONFIG_RCU_FANOUT_LEAF=16 # CONFIG_RCU_FANOUT_EXACT is not set # CONFIG_TREE_RCU_TRACE is not set CONFIG_RCU_NOCB_CPU=y CONFIG_IKCONFIG=y # CONFIG_IKCONFIG_PROC is not set CONFIG_LOG_BUF_SHIFT=17 CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y CONFIG_ARCH_WANTS_NUMA_GENERIC_PGPROT=y # CONFIG_CGROUPS is not set # CONFIG_CHECKPOINT_RESTORE is not set CONFIG_NAMESPACES=y CONFIG_UTS_NS=y # CONFIG_IPC_NS is not set # CONFIG_USER_NS is not set CONFIG_PID_NS=y CONFIG_UIDGID_CONVERTED=y CONFIG_UIDGID_STRICT_TYPE_CHECKS=y # CONFIG_SCHED_AUTOGROUP is not set CONFIG_SYSFS_DEPRECATED=y CONFIG_SYSFS_DEPRECATED_V2=y CONFIG_RELAY=y # CONFIG_BLK_DEV_INITRD is not set CONFIG_CC_OPTIMIZE_FOR_SIZE=y CONFIG_SYSCTL=y CONFIG_ANON_INODES=y # CONFIG_EXPERT is not set CONFIG_HAVE_UID16=y CONFIG_UID16=y # CONFIG_SYSCTL_SYSCALL is not set CONFIG_SYSCTL_EXCEPTION_TRACE=y CONFIG_KALLSYMS=y CONFIG_HOTPLUG=y CONFIG_PRINTK=y CONFIG_BUG=y CONFIG_ELF_CORE=y CONFIG_PCSPKR_PLATFORM=y CONFIG_HAVE_PCSPKR_PLATFORM=y CONFIG_BASE_FULL=y CONFIG_FUTEX=y CONFIG_EPOLL=y CONFIG_SIGNALFD=y CONFIG_TIMERFD=y CONFIG_EVENTFD=y CONFIG_SHMEM=y CONFIG_AIO=y # CONFIG_EMBEDDED is not set CONFIG_HAVE_PERF_EVENTS=y # # Kernel Performance Events And Counters # CONFIG_PERF_EVENTS=y CONFIG_VM_EVENT_COUNTERS=y CONFIG_PCI_QUIRKS=y CONFIG_SLUB_DEBUG=y CONFIG_COMPAT_BRK=y # CONFIG_SLAB is not set CONFIG_SLUB=y CONFIG_PROFILING=y # CONFIG_OPROFILE is not set CONFIG_HAVE_OPROFILE=y CONFIG_OPROFILE_NMI_TIMER=y # CONFIG_JUMP_LABEL is not set CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y CONFIG_HAVE_IOREMAP_PROT=y CONFIG_HAVE_KPROBES=y CONFIG_HAVE_KRETPROBES=y CONFIG_HAVE_OPTPROBES=y CONFIG_HAVE_ARCH_TRACEHOOK=y CONFIG_HAVE_DMA_ATTRS=y CONFIG_HAVE_DMA_CONTIGUOUS=y CONFIG_USE_GENERIC_SMP_HELPERS=y CONFIG_GENERIC_SMP_IDLE_THREAD=y CONFIG_HAVE_REGS_AND_STACK_ACCESS_API=y CONFIG_HAVE_DMA_API_DEBUG=y CONFIG_HAVE_HW_BREAKPOINT=y CONFIG_HAVE_MIXED_BREAKPOINTS_REGS=y CONFIG_HAVE_USER_RETURN_NOTIFIER=y CONFIG_HAVE_PERF_EVENTS_NMI=y CONFIG_HAVE_PERF_REGS=y
cron job: media_tree daily build: ERRORS
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:Tue Nov 27 19:00:24 CET 2012 git hash:c6c22955f80f2db9614b01fe5a3d1cfcd8b3d848 gcc version: i686-linux-gcc (GCC) 4.7.1 host hardware:x86_64 host os: 3.4.07-marune linux-git-arm-eabi-davinci: WARNINGS linux-git-arm-eabi-exynos: WARNINGS linux-git-arm-eabi-omap: WARNINGS linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: WARNINGS linux-git-x86_64: OK linux-2.6.31.12-i686: ERRORS linux-2.6.32.6-i686: ERRORS linux-2.6.33-i686: ERRORS linux-2.6.34-i686: ERRORS linux-2.6.35.3-i686: ERRORS linux-2.6.36-i686: ERRORS linux-2.6.37-i686: ERRORS linux-2.6.38.2-i686: ERRORS linux-2.6.39.1-i686: ERRORS linux-3.0-i686: ERRORS linux-3.1-i686: ERRORS linux-3.2.1-i686: ERRORS linux-3.3-i686: WARNINGS linux-3.4-i686: ERRORS linux-3.5-i686: ERRORS linux-3.6-i686: WARNINGS linux-3.7-rc1-i686: WARNINGS linux-2.6.31.12-x86_64: ERRORS linux-2.6.32.6-x86_64: ERRORS linux-2.6.33-x86_64: ERRORS linux-2.6.34-x86_64: ERRORS linux-2.6.35.3-x86_64: ERRORS linux-2.6.36-x86_64: ERRORS linux-2.6.37-x86_64: ERRORS linux-2.6.38.2-x86_64: ERRORS linux-2.6.39.1-x86_64: ERRORS linux-3.0-x86_64: ERRORS linux-3.1-x86_64: ERRORS linux-3.2.1-x86_64: ERRORS linux-3.3-x86_64: WARNINGS linux-3.4-x86_64: ERRORS linux-3.5-x86_64: ERRORS linux-3.6-x86_64: WARNINGS linux-3.7-rc1-x86_64: WARNINGS apps: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/2] [media] rc: unlock on error in show_protocols()
On 28/11/12 06:35, Dan Carpenter wrote: We recently introduced a new return -ENODEV in this function but we need to unlock before returning. There is an identical patch here (scroll down): https://patchwork.kernel.org/patch/1284081/ I'm not sure what happened to it. Douglas Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index 601d1ac1..d593bc6 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -789,8 +789,10 @@ static ssize_t show_protocols(struct device *device, } else if (dev-raw) { enabled = dev-raw-enabled_protocols; allowed = ir_raw_get_allowed_protocols(); - } else + } else { + mutex_unlock(dev-lock); return -ENODEV; + } IR_dprintk(1, allowed - 0x%llx, enabled - 0x%llx\n, (long long)allowed, -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] ngene: fix dvb_pll_attach failure
Before dvb_pll_attch call, be sure that drxd demodulator was initialized, otherwise, dvb_pll_attach() will always failed. In dvb_pll_attach(), first thing done is to enable the I2C gate control in order to probe the pll by performing a read access. As demodulator was not initialized, every i2c access failed. Reported-by: frederic.mantega...@gbiloba.org Signed-off-by: Patrice Chotard patricechot...@free.fr --- drivers/media/pci/ngene/ngene-cards.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/pci/ngene/ngene-cards.c b/drivers/media/pci/ngene/ngene-cards.c index 96a13ed..e2192db 100644 --- a/drivers/media/pci/ngene/ngene-cards.c +++ b/drivers/media/pci/ngene/ngene-cards.c @@ -328,6 +328,8 @@ static int demod_attach_drxd(struct ngene_channel *chan) return -ENODEV; } + /* initialized the DRXD demodulator */ + chan-fe-ops.init(chan-fe); if (!dvb_attach(dvb_pll_attach, chan-fe, feconf-pll_address, chan-i2c_adapter, feconf-pll_type)) { -- 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: Tuning problems with em28xx-dvb tda10071 on MIPS-based router board
Thanks for the info. Then I'll try to fix it by myself and isolate the error by comparing the driver behavior on the PC and my router. I hope I can provide a patch for that afterwards. Are there any hints where I might look first. Since it only works for very few transponders I suppose the error in the frontend or not? Regards Ingo 2012/11/26, Antti Palosaari cr...@iki.fi: On 11/26/2012 07:50 PM, Ingo Kofler wrote: Hi, I am trying to get my PCTV DVB-S2 stick running on my TP-Link TL-WR1043ND that runs OpenWrt (Attitude Adjustment Beta, Kernel 3.3.8). I have cross-compiled the corresponding kernel modules and deployed them on the router. I have also deployed the firmware on the device. After loading the corresponding modules the /dev/dvb/... devices show up and the dmesg output seems to be fine. Then I tried to test the device using szap and a channels.conf file. Unfortunately, the device cannot tune to most of the transponders except of two. Both are located in the vertical high band of the Astra 19E. For all other transponders I do not get a lock of the frontend. Tuning works fine on my PC using kernel verions 3.2 and 3.5 (the ones that ship with Ubuntu) and using the same channels.conf file and stick. So I conclude that both the stick, the satellite dish and the channels.conf is working. I've also tested it on the router board with an external powered USB Hub (I though that maybe the power of the router's USB port wasn't good enough). Now I have no further ideas. Before I start to debug the C code and try to figure out the difference between the PC and the router - Are there any known issues with this driver? Does it work on MIPS and different endianess? No idea if it works or not any other than AMD64 (and i386). I use AMD64 Kernel on my computer and I cannot test easily any other arch's as I don't have suitable hardware. i386 is so common which means bug reports are got very quickly and fixed. Generally speaking I am a little bit surprised these drivers seems to just work from arch to arch quite often. 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: Tuning problems with em28xx-dvb tda10071 on MIPS-based router board
On 11/27/2012 10:38 PM, Ingo Kofler wrote: Thanks for the info. Then I'll try to fix it by myself and isolate the error by comparing the driver behavior on the PC and my router. I hope I can provide a patch for that afterwards. Are there any hints where I might look first. Since it only works for very few transponders I suppose the error in the frontend or not? Your help is very much welcome! Try to found out some difference between working and non-working muxes. Like you supposed, the error must be somewhere at the frontend side, which means TDA10071 or A8293 drivers. Error descriptions sounds like em28xx driver is not involved. regards Antti Regards Ingo 2012/11/26, Antti Palosaari cr...@iki.fi: On 11/26/2012 07:50 PM, Ingo Kofler wrote: Hi, I am trying to get my PCTV DVB-S2 stick running on my TP-Link TL-WR1043ND that runs OpenWrt (Attitude Adjustment Beta, Kernel 3.3.8). I have cross-compiled the corresponding kernel modules and deployed them on the router. I have also deployed the firmware on the device. After loading the corresponding modules the /dev/dvb/... devices show up and the dmesg output seems to be fine. Then I tried to test the device using szap and a channels.conf file. Unfortunately, the device cannot tune to most of the transponders except of two. Both are located in the vertical high band of the Astra 19E. For all other transponders I do not get a lock of the frontend. Tuning works fine on my PC using kernel verions 3.2 and 3.5 (the ones that ship with Ubuntu) and using the same channels.conf file and stick. So I conclude that both the stick, the satellite dish and the channels.conf is working. I've also tested it on the router board with an external powered USB Hub (I though that maybe the power of the router's USB port wasn't good enough). Now I have no further ideas. Before I start to debug the C code and try to figure out the difference between the PC and the router - Are there any known issues with this driver? Does it work on MIPS and different endianess? No idea if it works or not any other than AMD64 (and i386). I use AMD64 Kernel on my computer and I cannot test easily any other arch's as I don't have suitable hardware. i386 is so common which means bug reports are got very quickly and fixed. Generally speaking I am a little bit surprised these drivers seems to just work from arch to arch quite often. 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 -- 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
[yavta PATCH v1.1 2/2] Print v4l2_buffer timestamp type
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi --- Fixed. yavta.c | 14 +- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/yavta.c b/yavta.c index bf3e096..b4d1df6 100644 --- a/yavta.c +++ b/yavta.c @@ -464,6 +464,7 @@ static int video_alloc_buffers(struct device *dev, int nbufs, /* Map the buffers. */ for (i = 0; i rb.count; ++i) { + const char *ts_type; memset(buf, 0, sizeof buf); buf.index = i; buf.type = dev-type; @@ -474,7 +475,18 @@ static int video_alloc_buffers(struct device *dev, int nbufs, strerror(errno), errno); return ret; } - printf(length: %u offset: %u\n, buf.length, buf.m.offset); + switch (buf.flags V4L2_BUF_FLAG_TIMESTAMP_MASK) { + case V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN: + ts_type = unknown; + break; + case V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC: + ts_type = monotonic; + break; + default: + ts_type = invalid; + } + printf(length: %u offset: %u timestamp type: %s\n, + buf.length, buf.m.offset, ts_type); switch (dev-memtype) { case V4L2_MEMORY_MMAP: -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] v4l: Reset subdev v4l2_dev field to NULL if registration fails
Hi Laurent, On 11/25/2012 01:41 AM, Laurent Pinchart wrote: When subdev registration fails the subdev v4l2_dev field is left to a non-NULL value. Later calls to v4l2_device_unregister_subdev() will consider the subdev as registered and will module_put() the subdev module without any matching module_get(). Fix this by setting the subdev v4l2_dev field to NULL in v4l2_device_register_subdev() when the function fails. Signed-off-by: Laurent Pinchartlaurent.pinch...@ideasonboard.com Acked-by: Sylwester Nawrocki s.nawro...@samsung.com I'm just wondering whether including this patch in stable kernel releases could potentially break anything. --- drivers/media/v4l2-core/v4l2-device.c | 30 ++ 1 files changed, 14 insertions(+), 16 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c index 513969f..98a7f5e 100644 --- a/drivers/media/v4l2-core/v4l2-device.c +++ b/drivers/media/v4l2-core/v4l2-device.c @@ -159,31 +159,21 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev, sd-v4l2_dev = v4l2_dev; if (sd-internal_ops sd-internal_ops-registered) { err = sd-internal_ops-registered(sd); - if (err) { - module_put(sd-owner); - return err; - } + if (err) + goto error_module; } /* This just returns 0 if either of the two args is NULL */ err = v4l2_ctrl_add_handler(v4l2_dev-ctrl_handler, sd-ctrl_handler, NULL); - if (err) { - if (sd-internal_ops sd-internal_ops-unregistered) - sd-internal_ops-unregistered(sd); - module_put(sd-owner); - return err; - } + if (err) + goto error_unregister; #if defined(CONFIG_MEDIA_CONTROLLER) /* Register the entity. */ if (v4l2_dev-mdev) { err = media_device_register_entity(v4l2_dev-mdev, entity); - if (err 0) { - if (sd-internal_ops sd-internal_ops-unregistered) - sd-internal_ops-unregistered(sd); - module_put(sd-owner); - return err; - } + if (err 0) + goto error_unregister; } #endif @@ -192,6 +182,14 @@ int v4l2_device_register_subdev(struct v4l2_device *v4l2_dev, spin_unlock(v4l2_dev-lock); return 0; + +error_unregister: + if (sd-internal_ops sd-internal_ops-unregistered) + sd-internal_ops-unregistered(sd); +error_module: + module_put(sd-owner); + sd-v4l2_dev = NULL; + return err; } EXPORT_SYMBOL_GPL(v4l2_device_register_subdev); -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] [media] exynos-gsc: Some fixes and updates
Hi Sachin, On 11/26/2012 07:20 AM, Sachin Kamat wrote: I have re-organised this series as per your suggestion and included your patch exynos-gsc: Correct clock handling. However, I have created 3 patches as I found making them into 2 a little cumbersome. Hope they look good. This series is based on samsung/for_v3.8 branch of git://linuxtv.org/snawrocki/media.git Thanks, I've put together all exynos-gsc patches into this branch http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/exynos_gsc_v3.8 Please let me know if there is anything missing there. The other patches are here http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/media_for_v3.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
[patch 00/03 v2] driver for Masterkit MA901 usb radio
Hi all, This is second version of small patch series for ma901 usb radio driver. Initial letter about this usb radio was sent on October 29 and can be found here: http://www.spinics.net/lists/linux-media/msg55779.html Changes: - removed f-type check and set in vidioc_g_frequency() - added maintainers entry patch Best regards, Alexey Klimov -- 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 01/03 v2] media: add driver for Masterkit MA901 usb radio
This patch creates a new usb-radio driver, radio-ma901.c, that supports Masterkit MA 901 USB FM radio devices. This device plugs into both the USB and an analog audio input or headphones, so this thing only deals with initialization and frequency setting. Signed-off-by: Alexey Klimov klimov.li...@gmail.com diff --git a/drivers/media/radio/Kconfig b/drivers/media/radio/Kconfig index 8090b87..ead9928 100644 --- a/drivers/media/radio/Kconfig +++ b/drivers/media/radio/Kconfig @@ -124,6 +124,18 @@ config USB_KEENE To compile this driver as a module, choose M here: the module will be called radio-keene. +config USB_MA901 + tristate Masterkit MA901 USB FM radio support + depends on USB VIDEO_V4L2 + ---help--- + Say Y here if you want to connect this type of radio to your + computer's USB port. Note that the audio is not digital, and + you must connect the line out connector to a sound card or a + set of speakers or headphones. + + To compile this driver as a module, choose M here: the + module will be called radio-ma901. + config RADIO_TEA5764 tristate TEA5764 I2C FM radio support depends on I2C VIDEO_V4L2 diff --git a/drivers/media/radio/Makefile b/drivers/media/radio/Makefile index c03ce4f..303eaeb 100644 --- a/drivers/media/radio/Makefile +++ b/drivers/media/radio/Makefile @@ -24,6 +24,7 @@ obj-$(CONFIG_USB_DSBR) += dsbr100.o obj-$(CONFIG_RADIO_SI470X) += si470x/ obj-$(CONFIG_USB_MR800) += radio-mr800.o obj-$(CONFIG_USB_KEENE) += radio-keene.o +obj-$(CONFIG_USB_MA901) += radio-ma901.o obj-$(CONFIG_RADIO_TEA5764) += radio-tea5764.o obj-$(CONFIG_RADIO_SAA7706H) += saa7706h.o obj-$(CONFIG_RADIO_TEF6862) += tef6862.o diff --git a/drivers/media/radio/radio-ma901.c b/drivers/media/radio/radio-ma901.c new file mode 100644 index 000..20e2880 --- /dev/null +++ b/drivers/media/radio/radio-ma901.c @@ -0,0 +1,460 @@ +/* + * Driver for the MasterKit MA901 USB FM radio. This device plugs + * into the USB port and an analog audio input or headphones, so this thing + * only deals with initialization, frequency setting, volume. + * + * Copyright (c) 2012 Alexey Klimov klimov.li...@gmail.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include linux/kernel.h +#include linux/module.h +#include linux/init.h +#include linux/slab.h +#include linux/input.h +#include linux/videodev2.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h +#include media/v4l2-ctrls.h +#include media/v4l2-event.h +#include linux/usb.h +#include linux/mutex.h + +#define DRIVER_AUTHOR Alexey Klimov klimov.li...@gmail.com +#define DRIVER_DESC Masterkit MA901 USB FM radio driver +#define DRIVER_VERSION 0.0.1 + +MODULE_AUTHOR(DRIVER_AUTHOR); +MODULE_DESCRIPTION(DRIVER_DESC); +MODULE_LICENSE(GPL); +MODULE_VERSION(DRIVER_VERSION); + +#define USB_MA901_VENDOR 0x16c0 +#define USB_MA901_PRODUCT 0x05df + +/* dev_warn macro with driver name */ +#define MA901_DRIVER_NAME radio-ma901 +#define ma901radio_dev_warn(dev, fmt, arg...) \ + dev_warn(dev, MA901_DRIVER_NAME - fmt, ##arg) + +#define ma901radio_dev_err(dev, fmt, arg...) \ + dev_err(dev, MA901_DRIVER_NAME - fmt, ##arg) + +/* Probably USB_TIMEOUT should be modified in module parameter */ +#define BUFFER_LENGTH 8 +#define USB_TIMEOUT 500 + +#define FREQ_MIN 87.5 +#define FREQ_MAX 108.0 +#define FREQ_MUL 16000 + +#define MA901_VOLUME_MAX 16 +#define MA901_VOLUME_MIN 0 + +/* Commands that device should understand + * List isn't full and will be updated with implementation of new functions + */ +#define MA901_RADIO_SET_FREQ 0x03 +#define MA901_RADIO_SET_VOLUME 0x04 +#define MA901_RADIO_SET_MONO_STEREO0x05 + +/* Comfortable defines for ma901radio_set_stereo */ +#define MA901_WANT_STEREO 0x50 +#define MA901_WANT_MONO0xd0 + +/* module parameter */ +static int radio_nr = -1; +module_param(radio_nr, int, 0); +MODULE_PARM_DESC(radio_nr, Radio file number); + +/* Data for one (physical) device */ +struct ma901radio_device { + /* reference to USB and video device */ + struct usb_device *usbdev; + struct usb_interface *intf; + struct video_device vdev; + struct v4l2_device
[patch 02/03 v2] usb hid quirks for Masterkit MA901 usb radio
Don't let Masterkit MA901 USB radio be handled by usb hid drivers. This device will be handled by radio-ma901.c driver. Signed-off-by: Alexey Klimov klimov.li...@gmail.com diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 5de3bb3..8e06569 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -2025,6 +2025,7 @@ static const struct hid_device_id hid_ignore_list[] = { { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HYBRID) }, { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HEATCONTROL) }, { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_BEATPAD) }, + { HID_USB_DEVICE(USB_VENDOR_ID_MASTERKIT, USB_DEVICE_ID_MASTERKIT_MA901RADIO) }, { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1024LS) }, { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1208LS) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICKIT1) }, diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 1dcb76f..17aa4f6 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -533,6 +533,9 @@ #define USB_VENDOR_ID_MADCATZ 0x0738 #define USB_DEVICE_ID_MADCATZ_BEATPAD 0x4540 +#define USB_VENDOR_ID_MASTERKIT0x16c0 +#define USB_DEVICE_ID_MASTERKIT_MA901RADIO 0x05df + #define USB_VENDOR_ID_MCC 0x09db #define USB_DEVICE_ID_MCC_PMD1024LS0x0076 #define USB_DEVICE_ID_MCC_PMD1208LS0x007a -- 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 03/03 v2] MAINTAINERS: add entry for radio-ma901 driver
This patch adds MAINTAINERS entry for radio-ma901 usb radio driver. Signed-off-by: Alexey Klimov klimov.li...@gmail.com diff --git a/MAINTAINERS b/MAINTAINERS index b623679..a36b29c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4723,6 +4723,13 @@ Q: http://patchwork.linuxtv.org/project/linux-media/list/ S: Maintained F: drivers/media/dvb-frontends/m88rs2000* +MA901 MASTERKIT USB FM RADIO DRIVER +M: Alexey Klimov klimov.li...@gmail.com +L: linux-media@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +S: Maintained +F: drivers/media/radio/radio-ma901.c + MAC80211 M: Johannes Berg johan...@sipsolutions.net L: linux-wirel...@vger.kernel.org -- 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 0/9] [media] s5p-tv: Checkpatch Fixes and cleanup
On 11/26/2012 05:48 AM, Sachin Kamat wrote: Build tested based on samsung/for_v3.8 branch of git://linuxtv.org/snawrocki/media.git tree. How about testing it on Origen board ? Tomasz, are you OK with this patch series ? As a side note, for v3.9, when common clock framework support for the Exynos platforms is merged this driver will need to have clk_(un)prepare added. It will fail to initialize otherwise. Sachin Kamat (9): [media] s5p-tv: Add missing braces around sizeof in sdo_drv.c [media] s5p-tv: Add missing braces around sizeof in mixer_video.c [media] s5p-tv: Add missing braces around sizeof in mixer_reg.c [media] s5p-tv: Add missing braces around sizeof in mixer_drv.c [media] s5p-tv: Add missing braces around sizeof in hdmiphy_drv.c [media] s5p-tv: Add missing braces around sizeof in hdmi_drv.c [media] s5p-tv: Use devm_clk_get APIs in sdo_drv.c [media] s5p-tv: Use devm_* APIs in mixer_drv.c [media] s5p-tv: Use devm_clk_get APIs in hdmi_drv drivers/media/platform/s5p-tv/hdmi_drv.c| 28 +++-- drivers/media/platform/s5p-tv/hdmiphy_drv.c |2 +- drivers/media/platform/s5p-tv/mixer_drv.c | 87 +++ drivers/media/platform/s5p-tv/mixer_reg.c |6 +- drivers/media/platform/s5p-tv/mixer_video.c | 18 +++--- drivers/media/platform/s5p-tv/sdo_drv.c | 43 - 6 files changed, 57 insertions(+), 127 deletions(-) -- Thanks, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: Tree for Nov 27 (media/v4l2-core/videobuf2-dma-contig.c)
Hi, It's fixed already and you will check it at Nov 28 tree http://www.spinics.net/lists/linux-media/msg56830.html Thank you, Kyungmin Park On 11/28/12, Stephen Rothwell s...@canb.auug.org.au wrote: [Just adding some cc's] On Tue, 27 Nov 2012 10:32:15 -0800 Randy Dunlap rdun...@infradead.org wrote: On 11/26/2012 10:25 PM, Stephen Rothwell wrote: Hi all, Changes since 20121126: on i386: drivers/media/v4l2-core/videobuf2-dma-contig.c:743:16: error: 'vb2_dc_get_dmabuf' undeclared here (not in a function) Full randconfig file is attached. -- ~Randy -- Cheers, Stephen Rothwells...@canb.auug.org.au -- 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 03/15] [media] marvell-ccic: add clock tree support for marvell-ccic driver
Hello Guennadi, Thanks for your suggestion, please see my comments below. Best Regards, Libin -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 18:50 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 03/15] [media] marvell-ccic: add clock tree support for marvell-ccic driver + mcam-clk_num = pdata-clk_num; + } else { + for (i = 0; i pdata-clk_num; i++) { + if (mcam-clk[i]) { + clk_put(mcam-clk[i]); + mcam-clk[i] = NULL; + } + } + mcam-clk_num = 0; + } +} Don't think I like this. IIUC, your driver should only try to use clocks, that it knows about, not some random clocks, passed from the platform data. So, you should be using explicit clock names. In your platform data you can set whether a specific clock should be used or not, but not pass clock names down. Also you might want to consider using devm_clk_get() and be more careful with error handling. OK, we will try to enhance it. [Libin] Because there are some boards using mmp chip, and the clock names on different board may be totally different. And also this is why the clock number is not definite. To support more boards, the dynamic names are used instead of the static names. static int mmpcam_probe(struct platform_device *pdev) { @@ -290,6 -- 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
em28xx: msi Digivox ATSC board id [0db0:8810]
Hi, I recently acquired a msi Digivox ATSC tuner. I believe this card has an em28xx chip (the windows driver included is em28xx). Looking at the em28xx wiki page and digging around in the code it does not seem like the em28xx driver has support for this card. Based on my limit (extremely) amount of knowledge, it doesn't look like it would be terribly difficult to add support for this card. I am a complete hardware newbie (looking and willing to learn) and I hoping someone will be willing to help me out. Following the bus snooping guide I was able to snoop the usb tuner (using usbsnoop 2.0) and collect some data from this card (Windows XP, using the ArcSoft TotalMedia software the card shipped with). $ php parse-usbsnoop.php UsbSnoop.log parsed-usbsnoop.txt http://pyther.net/a/digivox_atsc/parsed-usbsnoop.txt $ perl contrib_em28xx_parse_em28xx.pl parsed-usbsnoop.txt parse_em28xx.txt http://pyther.net/a/digivox_atsc/parse_em28xx.txt $ lsusb -vvv (snippet) http://pyther.net/a/digivox_atsc/lsusb.txt Note: If necessary I can provide the entire UsbSnoop.log (however its ~350MB) At this point I'm not really sure how to use the above information to add support for my tuner. For starters I have non-existent C skills. From what I've looked at, I figure I have to add the vendor id and product id and it looks like I need to create a struct defining the input devices on the tuner (just astc/dvb on this card). It also looks like I need to find out the reset codes? Any help would be greatly appreciated. Model: msi Digivox ATSC Vendor / Product Id: [0db0:8810] URL: http://www.msi.com/product/mm/DIGIVOX-ATSC.html 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
anyone here know anyone at ivtvdriver.org? it's been down a few days now
I wonder if anyone here has control over or knows anyone who has control over the ivtvdriver.org website and lists. They seem to be down and have been for a bit now. Does anyone know if there is any expectation that this stuff will come back or is headed for moribund-land? Cheers, b. signature.asc Description: OpenPGP digital signature
ivtv driver inputs randomly block
I have a machine with a PVR-150 and a PVR-500 in it on a 3.2.0-33-generic (Ubuntu kernel, based on 3.2.1 IIUC) kernel. I am having problems where at random times random /dev/video[0,1,2] inputs will just block on read. It's not the same input every time and it's not even the same card every time. This is all hardware which has worked without any such problems before. To remedy the hanging input I simply have to rmmod ivtv modprobe ivtv and all is back to normal again, until it happens again. Any ideas? b. signature.asc Description: OpenPGP digital signature
Re: [PATCH 0/9] [media] s5p-tv: Checkpatch Fixes and cleanup
On 28 November 2012 04:57, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote: On 11/26/2012 05:48 AM, Sachin Kamat wrote: Build tested based on samsung/for_v3.8 branch of git://linuxtv.org/snawrocki/media.git tree. How about testing it on Origen board ? I wanted to but could not due to hardware setup problem. I will see if I can get it up today (I am off for the rest of the week). Tomasz, are you OK with this patch series ? As a side note, for v3.9, when common clock framework support for the Exynos platforms is merged this driver will need to have clk_(un)prepare added. It will fail to initialize otherwise. Sachin Kamat (9): [media] s5p-tv: Add missing braces around sizeof in sdo_drv.c [media] s5p-tv: Add missing braces around sizeof in mixer_video.c [media] s5p-tv: Add missing braces around sizeof in mixer_reg.c [media] s5p-tv: Add missing braces around sizeof in mixer_drv.c [media] s5p-tv: Add missing braces around sizeof in hdmiphy_drv.c [media] s5p-tv: Add missing braces around sizeof in hdmi_drv.c [media] s5p-tv: Use devm_clk_get APIs in sdo_drv.c [media] s5p-tv: Use devm_* APIs in mixer_drv.c [media] s5p-tv: Use devm_clk_get APIs in hdmi_drv drivers/media/platform/s5p-tv/hdmi_drv.c| 28 +++-- drivers/media/platform/s5p-tv/hdmiphy_drv.c |2 +- drivers/media/platform/s5p-tv/mixer_drv.c | 87 +++ drivers/media/platform/s5p-tv/mixer_reg.c |6 +- drivers/media/platform/s5p-tv/mixer_video.c | 18 +++--- drivers/media/platform/s5p-tv/sdo_drv.c | 43 - 6 files changed, 57 insertions(+), 127 deletions(-) -- Thanks, Sylwester -- With warm regards, Sachin -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] [media] exynos-gsc: Some fixes and updates
Hi Sylwester, On 28 November 2012 04:41, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote: Hi Sachin, On 11/26/2012 07:20 AM, Sachin Kamat wrote: I have re-organised this series as per your suggestion and included your patch exynos-gsc: Correct clock handling. However, I have created 3 patches as I found making them into 2 a little cumbersome. Hope they look good. This series is based on samsung/for_v3.8 branch of git://linuxtv.org/snawrocki/media.git Thanks, I've put together all exynos-gsc patches into this branch http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/exynos_gsc_v3.8 Please let me know if there is anything missing there. The other patches are here http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/media_for_v3.8 Thanks Sylwester. All accepted patches are present in the above 2 links. Awaiting feedback from Kamil for patches related to MFC. G2D etc. in the previous series. -- Regards, Sylwester -- With warm regards, Sachin -- 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 0/9] [media] s5p-tv: Checkpatch Fixes and cleanup
Hi Sylwester, On 28 November 2012 09:04, Sachin Kamat sachin.ka...@linaro.org wrote: On 28 November 2012 04:57, Sylwester Nawrocki sylvester.nawro...@gmail.com wrote: On 11/26/2012 05:48 AM, Sachin Kamat wrote: Build tested based on samsung/for_v3.8 branch of git://linuxtv.org/snawrocki/media.git tree. How about testing it on Origen board ? I wanted to but could not due to hardware setup problem. I will see if I can get it up today (I am off for the rest of the week). Tested this series with test application on Origen. Works fine. Tomasz, are you OK with this patch series ? As a side note, for v3.9, when common clock framework support for the Exynos platforms is merged this driver will need to have clk_(un)prepare added. It will fail to initialize otherwise. Sachin Kamat (9): [media] s5p-tv: Add missing braces around sizeof in sdo_drv.c [media] s5p-tv: Add missing braces around sizeof in mixer_video.c [media] s5p-tv: Add missing braces around sizeof in mixer_reg.c [media] s5p-tv: Add missing braces around sizeof in mixer_drv.c [media] s5p-tv: Add missing braces around sizeof in hdmiphy_drv.c [media] s5p-tv: Add missing braces around sizeof in hdmi_drv.c [media] s5p-tv: Use devm_clk_get APIs in sdo_drv.c [media] s5p-tv: Use devm_* APIs in mixer_drv.c [media] s5p-tv: Use devm_clk_get APIs in hdmi_drv drivers/media/platform/s5p-tv/hdmi_drv.c| 28 +++-- drivers/media/platform/s5p-tv/hdmiphy_drv.c |2 +- drivers/media/platform/s5p-tv/mixer_drv.c | 87 +++ drivers/media/platform/s5p-tv/mixer_reg.c |6 +- drivers/media/platform/s5p-tv/mixer_video.c | 18 +++--- drivers/media/platform/s5p-tv/sdo_drv.c | 43 - 6 files changed, 57 insertions(+), 127 deletions(-) -- Thanks, Sylwester -- With warm regards, Sachin -- With warm regards, Sachin -- 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 12/15] [media] marvell-ccic: add soc_camera support in mmp driver
Hello Guennadi, Please see my comments below. Regards, Libin -Original Message- From: Albert Wang Sent: Wednesday, November 28, 2012 12:06 AM To: Guennadi Liakhovetski Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: RE: [PATCH 12/15] [media] marvell-ccic: add soc_camera support in mmp driver Hi, Guennadi -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 23:54 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 12/15] [media] marvell-ccic: add soc_camera support in mmp driver [snip] mcam = cam-mcam; + spin_lock_init(mcam-dev_lock); mcam-plat_power_up = mmpcam_power_up; mcam-plat_power_down = mmpcam_power_down; mcam-ctlr_reset = mcam_ctlr_reset; mcam-calc_dphy = mmpcam_calc_dphy; mcam-dev = pdev-dev; mcam-use_smbus = 0; + mcam-card_name = pdata-name; + mcam-mclk_min = pdata-mclk_min; + mcam-mclk_src = pdata-mclk_src; + mcam-mclk_div = pdata-mclk_div; Actually you don't really have to copy everything from platform data to your private driver object during probing. You can access your platform data also at run-time. So, maybe you can survive without adding these .mclk_* struct members? Yes, make sense. :) [Libin] We add such members because we need use these variables in the file mcam-core-soc.c. In the mcam-core-soc.c, the pdata is invisible. I think we can split the probe function and copy them in the file mcam-core-soc.c as you suggested. [snip] + int chip_id; /* * MIPI support */ -- 1.7.9.5 Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver
Hello Guennadi, Please see my comments below. Best Regards, Libin -Original Message- From: Albert Wang Sent: Tuesday, November 27, 2012 7:21 PM To: Guennadi Liakhovetski Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: RE: [PATCH 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver Hi, Guennadi We will update the patch by following your good suggestion! :) [snip] + pll1 = clk_get(dev, pll1); + if (IS_ERR(pll1)) { + dev_err(dev, Could not get pll1 clock\n); + return; + } + + tx_clk_esc = clk_get_rate(pll1) / 100 / 12; + clk_put(pll1); Once you release your clock per clk_put() its rate can be changed by some other user, so, your tx_clk_esc becomes useless. Better keep the reference to the clock until clean up. Maybe you can also use devm_clk_get() to simplify the clean up. That's a good suggestion. [Libin] In our code design, the pll1 will never be changed after the system boots up. Camera and other components can only get the clk without modifying it. -- 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 03/15] [media] marvell-ccic: add clock tree support for marvell-ccic driver
Hi Libin On Tue, 27 Nov 2012, Libin Yang wrote: Hello Guennadi, Thanks for your suggestion, please see my comments below. Best Regards, Libin -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Tuesday, 27 November, 2012 18:50 To: Albert Wang Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: Re: [PATCH 03/15] [media] marvell-ccic: add clock tree support for marvell-ccic driver + mcam-clk_num = pdata-clk_num; + } else { + for (i = 0; i pdata-clk_num; i++) { + if (mcam-clk[i]) { + clk_put(mcam-clk[i]); + mcam-clk[i] = NULL; + } + } + mcam-clk_num = 0; + } +} Don't think I like this. IIUC, your driver should only try to use clocks, that it knows about, not some random clocks, passed from the platform data. So, you should be using explicit clock names. In your platform data you can set whether a specific clock should be used or not, but not pass clock names down. Also you might want to consider using devm_clk_get() and be more careful with error handling. OK, we will try to enhance it. [Libin] Because there are some boards using mmp chip, and the clock names on different board may be totally different. And also this is why the clock number is not definite. To support more boards, the dynamic names are used instead of the static names. No, I don't think it's right. The clock connection ID is the ID of the clock _consumer_, not the clock provider. So, your camera IP block has several clock inputs, and your platforms should provide clock lookup entries with names of those clock _inputs_, not of their clock sources. BTW, I really doubt it your camera block has 4 clock inputs? If some of them are parents of the clocks, that really supply the block (which would also explain why you call it a tree), then you don't have to clk_get() them explicitly. The clock framework will refcount and enable those parent clocks for you. So, I think, you really should fix your platforms. This has been discussed multiple times on the mailing lists, feel free to do some research, here one link: http://thread.gmane.org/gmane.linux.ports.arm.kernel/131302/focus=37730 Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver
On Tue, 27 Nov 2012, Libin Yang wrote: Hello Guennadi, Please see my comments below. Best Regards, Libin -Original Message- From: Albert Wang Sent: Tuesday, November 27, 2012 7:21 PM To: Guennadi Liakhovetski Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: RE: [PATCH 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver Hi, Guennadi We will update the patch by following your good suggestion! :) [snip] + pll1 = clk_get(dev, pll1); + if (IS_ERR(pll1)) { + dev_err(dev, Could not get pll1 clock\n); + return; + } + + tx_clk_esc = clk_get_rate(pll1) / 100 / 12; + clk_put(pll1); Once you release your clock per clk_put() its rate can be changed by some other user, so, your tx_clk_esc becomes useless. Better keep the reference to the clock until clean up. Maybe you can also use devm_clk_get() to simplify the clean up. That's a good suggestion. [Libin] In our code design, the pll1 will never be changed after the system boots up. Camera and other components can only get the clk without modifying it. This doesn't matter. We have a standard API and we have to abide to its rules. Your driver can be reused or its code can be copied by others. I don't think it should be too difficult to just issue devm_clk_get() once and then just forget about it. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver
Hi Guennadi, -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Wednesday, November 28, 2012 3:14 PM To: Libin Yang Cc: Albert Wang; cor...@lwn.net; linux-media@vger.kernel.org Subject: RE: [PATCH 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver On Tue, 27 Nov 2012, Libin Yang wrote: Hello Guennadi, Please see my comments below. Best Regards, Libin -Original Message- From: Albert Wang Sent: Tuesday, November 27, 2012 7:21 PM To: Guennadi Liakhovetski Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: RE: [PATCH 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver Hi, Guennadi We will update the patch by following your good suggestion! :) [snip] +pll1 = clk_get(dev, pll1); +if (IS_ERR(pll1)) { +dev_err(dev, Could not get pll1 clock\n); +return; +} + +tx_clk_esc = clk_get_rate(pll1) / 100 / 12; +clk_put(pll1); Once you release your clock per clk_put() its rate can be changed by some other user, so, your tx_clk_esc becomes useless. Better keep the reference to the clock until clean up. Maybe you can also use devm_clk_get() to simplify the clean up. That's a good suggestion. [Libin] In our code design, the pll1 will never be changed after the system boots up. Camera and other components can only get the clk without modifying it. This doesn't matter. We have a standard API and we have to abide to its rules. Your driver can be reused or its code can be copied by others. I don't think it should be too difficult to just issue devm_clk_get() once and then just forget about it. [Libin] Yes, you are right. We should consider the driver may be reused. I didn't realize it. Another question is: If we use devm_clk_get(), what I understand, the clk will be put when the device is being released. It means the driver will hold the clk all the time the driver is in the kernel. What do you think if we get the clk when opening the camera, and put it in the close? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ Best Regard, Libin -- 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 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver
On Tue, 27 Nov 2012, Libin Yang wrote: Hi Guennadi, -Original Message- From: Guennadi Liakhovetski [mailto:g.liakhovet...@gmx.de] Sent: Wednesday, November 28, 2012 3:14 PM To: Libin Yang Cc: Albert Wang; cor...@lwn.net; linux-media@vger.kernel.org Subject: RE: [PATCH 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver On Tue, 27 Nov 2012, Libin Yang wrote: Hello Guennadi, Please see my comments below. Best Regards, Libin -Original Message- From: Albert Wang Sent: Tuesday, November 27, 2012 7:21 PM To: Guennadi Liakhovetski Cc: cor...@lwn.net; linux-media@vger.kernel.org; Libin Yang Subject: RE: [PATCH 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver Hi, Guennadi We will update the patch by following your good suggestion! :) [snip] + pll1 = clk_get(dev, pll1); + if (IS_ERR(pll1)) { + dev_err(dev, Could not get pll1 clock\n); + return; + } + + tx_clk_esc = clk_get_rate(pll1) / 100 / 12; + clk_put(pll1); Once you release your clock per clk_put() its rate can be changed by some other user, so, your tx_clk_esc becomes useless. Better keep the reference to the clock until clean up. Maybe you can also use devm_clk_get() to simplify the clean up. That's a good suggestion. [Libin] In our code design, the pll1 will never be changed after the system boots up. Camera and other components can only get the clk without modifying it. This doesn't matter. We have a standard API and we have to abide to its rules. Your driver can be reused or its code can be copied by others. I don't think it should be too difficult to just issue devm_clk_get() once and then just forget about it. [Libin] Yes, you are right. We should consider the driver may be reused. I didn't realize it. Another question is: If we use devm_clk_get(), what I understand, the clk will be put when the device is being released. It means the driver will hold the clk all the time the driver is in the kernel. What do you think if we get the clk when opening the camera, and put it in the close? Sure, that's fine too. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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 02/15] [media] marvell-ccic: add MIPI support for marvell-ccic driver
Hi Guennadi, [snip] [Libin] Yes, you are right. We should consider the driver may be reused. I didn't realize it. Another question is: If we use devm_clk_get(), what I understand, the clk will be put when the device is being released. It means the driver will hold the clk all the time the driver is in the kernel. What do you think if we get the clk when opening the camera, and put it in the close? Sure, that's fine too. OK, I see. Thanks. Regards, Libin -- 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