Re: [PATCH] media: fix a typo CONFIG_HAVE_GENERIC_DMA_COHERENT in videobuf2-dma-contig.c

2012-11-27 Thread Prabhakar Lad
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Mauro Carvalho Chehab
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Albert Wang


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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Shaik Ameer Basha
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Tomi Valkeinen
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.

2012-11-27 Thread Fabio Estevam
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

2012-11-27 Thread Laurent Pinchart
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

2012-11-27 Thread Tomi Valkeinen
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Laurent Pinchart
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Laurent Pinchart
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Laurent Pinchart
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Laurent Pinchart
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

2012-11-27 Thread Laurent Pinchart
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Laurent Pinchart
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Albert Wang
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

2012-11-27 Thread Albert Wang


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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Albert Wang
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()

2012-11-27 Thread Dan Carpenter
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()

2012-11-27 Thread Dan Carpenter
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)

2012-11-27 Thread Randy Dunlap
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

2012-11-27 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date: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()

2012-11-27 Thread Douglas Bagnall
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

2012-11-27 Thread Patrice Chotard
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

2012-11-27 Thread Ingo Kofler
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

2012-11-27 Thread Antti Palosaari

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

2012-11-27 Thread Sakari Ailus
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

2012-11-27 Thread Sylwester Nawrocki

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

2012-11-27 Thread Sylwester Nawrocki

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

2012-11-27 Thread Alexey Klimov
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

2012-11-27 Thread Alexey Klimov
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

2012-11-27 Thread Alexey Klimov
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

2012-11-27 Thread Alexey Klimov
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

2012-11-27 Thread Sylwester Nawrocki

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)

2012-11-27 Thread Kyungmin Park
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

2012-11-27 Thread Libin Yang
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]

2012-11-27 Thread Matthew Gyurgyik

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

2012-11-27 Thread Brian J. Murrell
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

2012-11-27 Thread Brian J. Murrell
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

2012-11-27 Thread Sachin Kamat
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

2012-11-27 Thread Sachin Kamat
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

2012-11-27 Thread Sachin Kamat
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

2012-11-27 Thread Libin Yang
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

2012-11-27 Thread Libin Yang
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Libin Yang
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

2012-11-27 Thread Guennadi Liakhovetski
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

2012-11-27 Thread Libin Yang
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