RE: How to support MIPI CSI-2 controller in soc-camera framework?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Guennadi:
	Sorry for the late!
	"I would leave the choice of a connection to the platform 
code."  I agree with you on this, since HW board will only support MIPI or parallel even the sensor
can support both.

	Please review ! thanks for your time.

Subject: [PATCH] V4L/DVB: V4L2: add MIPI bus flags

Signed-off-by: Kassey Lee <ygli@xxxxxxxxxxx>
Signed-off-by: Qing Xu <qingx@xxxxxxxxxxx>
---
 include/media/soc_camera.h |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 9386db8..480e3e9 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -95,7 +95,12 @@ struct soc_camera_host_ops {
 #define SOCAM_SENSOR_INVERT_HSYNC	(1 << 2)
 #define SOCAM_SENSOR_INVERT_VSYNC	(1 << 3)
 #define SOCAM_SENSOR_INVERT_DATA	(1 << 4)
-
+#define SOCAM_MIPI_1LANE	(1 << 17)
+#define SOCAM_MIPI_2LANE	(1 << 18)
+#define SOCAM_MIPI_3LANE	(1 << 19)
+#define SOCAM_MIPI_4LANE	(1 << 20)
+#define SOCAM_MIPI	(SOCAM_MIPI_1LANE | SOCAM_MIPI_2LANE | \
+			SOCAM_MIPI_3LANE | SOCAM_MIPI_4LANE)
 struct i2c_board_info;
 struct regulator_bulk_data;
 
@@ -259,7 +264,7 @@ static inline unsigned long soc_camera_bus_param_compatible(
 			unsigned long camera_flags, unsigned long bus_flags)
 {
 	unsigned long common_flags, hsync, vsync, pclk, data, buswidth, mode;
-
+	unsigned long mipi;
 	common_flags = camera_flags & bus_flags;
 
 	hsync = common_flags & (SOCAM_HSYNC_ACTIVE_HIGH | SOCAM_HSYNC_ACTIVE_LOW);
@@ -268,8 +273,8 @@ static inline unsigned long soc_camera_bus_param_compatible(
 	data = common_flags & (SOCAM_DATA_ACTIVE_HIGH | SOCAM_DATA_ACTIVE_LOW);
 	mode = common_flags & (SOCAM_MASTER | SOCAM_SLAVE);
 	buswidth = common_flags & SOCAM_DATAWIDTH_MASK;
-
-	return (!hsync || !vsync || !pclk || !data || !mode || !buswidth) ? 0 :
+	mipi = common_flags & SOCAM_MIPI;
+	return ((!hsync || !vsync || !pclk || !data || !mode || !buswidth) && (!mipi)) ? 0 :
 		common_flags;
 }


Best regards
Kassey 
Email: ygli@xxxxxxxxxxx
Application Processor Systems Engineering, Marvell Technology Group Ltd.
Shanghai, China.


-----Original Message-----
From: linux-media-owner@xxxxxxxxxxxxxxx [mailto:linux-media-owner@xxxxxxxxxxxxxxx] On Behalf Of Guennadi Liakhovetski
Sent: 2011å2æ4æ 15:45
To: Qing Xu
Cc: Laurent Pinchart; Linux Media Mailing List
Subject: RE: How to support MIPI CSI-2 controller in soc-camera framework?

Hi

Sorry for the delay first of all.

On Wed, 19 Jan 2011, Qing Xu wrote:

> Hi,
> 
> (a general request: could you please configure your mailer to wrap
> Lines at somewhere around 70 characters?)
> very sorry for the un-convenience!
> 
> Thanks for your description! I could verify and try your way on our
> CSI-2 driver.
> Also, our another chip's camera controller support both MIPI and
> traditional parallel(H_sync/V_sync) interface, we hope host can
> negotiate with sensor on MIPI configure, as the sensor could be
> parallel interface or MIPI interface, so I have a proposal as
> follow:
> 
> in soc_camera.h, SOCAM_XXX defines all HW connection properties,
> I thing MIPI(1/2/3/4 lanes) is also a kind of HW connection
> property, and it is mutex with parallel properties(if sensor
> support mipi connection, the HW signal has no parallel property
> any more), once host controller find subdev support MIPI, it will
> enable MIPI functional itself, and if subdev only support parallel,
> it will enable parallel functional itself.

I think, yes, we can add MIPI definitions to soc_camera.h, similar to your 
proposal below, but I don't think we need the "SOCAM_MIPI" macro itself, 
maybe define it as a mask

#define SOCAM_MIPI (SOCAM_MIPI_1LANE | SOCAM_MIPI_2LANE | SOCAM_MIPI_3LANE | SOCAM_MIPI_4LANE)

Also, the decision "if MIPI supported by the client always prefer it over 
the parallel connection" doesn't seem to be a meaningful thing to do in 
the driver to me. I would leave the choice of a connection to the platform 
code. In that case your addition to the soc_camera_apply_sensor_flags() 
function becomes unneeded.

Makes sense?

Thanks
Guennadi

> (you can find the proposal in the code which I have sent, refer to 
> pxa955_cam_set_bus_param() in pxa955_cam.c, ov5642_query_bus_param
> In ov5642.c)
> 
> --- a/drivers/media/video/soc_camera.c
> +++ b/drivers/media/video/soc_camera.c
> unsigned long soc_camera_apply_sensor_flags(struct soc_camera_link *icl,
>                 if (f == SOCAM_PCLK_SAMPLE_RISING || f == SOCAM_PCLK_SAMPLE_FALLING)
>                         flags ^= SOCAM_PCLK_SAMPLE_RISING | SOCAM_PCLK_SAMPLE_FALLING;
>         }
> +       if (icl->flags & SOCAM_MIPI) {
> +               flags &= SOCAM_MIPI | SOCAM_MIPI_1LANE | SOCAM_MIPI_2LANE
> +                                       | SOCAM_MIPI_3LANE | SOCAM_MIPI_4LANE;
> +       }
> 
>         return flags;
>  }
> 
> --- a/include/media/soc_camera.h
> +++ b/include/media/soc_camera.h
> 
>  #define SOCAM_DATA_ACTIVE_HIGH         (1 << 14)
>  #define SOCAM_DATA_ACTIVE_LOW          (1 << 15)
> 
> +#define SOCAM_MIPI             (1 << 16)
> +#define SOCAM_MIPI_1LANE               (1 << 17)
> +#define SOCAM_MIPI_2LANE               (1 << 18)
> +#define SOCAM_MIPI_3LANE               (1 << 19)
> +#define SOCAM_MIPI_4LANE               (1 << 20)
> +
> 
>  static inline unsigned long soc_camera_bus_param_compatible(
>                         unsigned long camera_flags, unsigned long bus_flags)
>  {
> -       unsigned long common_flags, hsync, vsync, pclk, data, buswidth, mode;
> +       unsigned long common_flags, hsync, vsync, pclk, data, buswidth, mode, mipi;
> 
>         common_flags = camera_flags & bus_flags;
> 
> @@ -261,8 +267,10 @@ static inline unsigned long soc_camera_bus_param_compatible(
>         data = common_flags & (SOCAM_DATA_ACTIVE_HIGH | SOCAM_DATA_ACTIVE_LOW);
>         mode = common_flags & (SOCAM_MASTER | SOCAM_SLAVE);
>         buswidth = common_flags & SOCAM_DATAWIDTH_MASK;
> +       mipi = common_flags & (SOCAM_MIPI | SOCAM_MIPI_1LANE | SOCAM_MIPI_2LANE
> +                                               | SOCAM_MIPI_3LANE | SOCAM_MIPI_4LANE);
> 
> -       return (!hsync || !vsync || !pclk || !data || !mode || !buswidth) ? 0 :
> +       return ((!hsync || !vsync || !pclk || !data || !mode || !buswidth) && (!mipi)) ? 0 :
>                 common_flags;
>  }
> 
> 
> -----Original Message-----
> From: Guennadi Liakhovetski [mailto:g.liakhovetski@xxxxxx]
> Sent: 2011ÃÃ1ÃÃ20ÃÃ 0:20
> To: Qing Xu
> Cc: Laurent Pinchart; Linux Media Mailing List
> Subject: Re: How to support MIPI CSI-2 controller in soc-camera framework?
> 
> (a general request: could you please configure your mailer to wrap lines
> at somewhere around 70 characters?)
> 
> On Tue, 18 Jan 2011, Qing Xu wrote:
> 
> > Hi,
> >
> > Our chip support both MIPI and parallel interface. The HW connection logic is
> > sensor(such as ov5642) -> our MIPI controller(handle DPHY timing/ CSI-2
> > things) -> our camera controller (handle DMA transmitting/ fmt/ size
> > things). Now, I find the driver of sh_mobile_csi2.c, it seems like a
> > CSI-2 driver, but I don't quite understand how it works:
> > 1) how the host controller call into this driver?
> 
> This is a normal v4l2-subdev driver. Platform data for the
> sh_mobile_ceu_camera driver provides a link to CSI2 driver data, then the
> host driver loads the CSI2 driver, which then links itself into the
> subdevice list. Look at arch/arm/mach-shmobile/board-ap4evb.c how the data
> is linked:
> 
> static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
>         .flags = SH_CEU_FLAG_USE_8BIT_BUS,
>         .csi2_dev = &csi2_device.dev,
> };
> 
> and in the hosz driver drivers/media/video/sh_mobile_ceu_camera.c look in
> the sh_mobile_ceu_probe function below the lines:
> 
>         csi2 = pcdev->pdata->csi2_dev;
>         if (csi2) {
> ...
> 
> 
> > 2) how the host controller/sensor negotiate MIPI variable with this
> > driver, such as D-PHY timing(hs_settle/hs_termen/clk_settle/clk_termen),
> > number of lanes...?
> 
> Since I only had a limited number of MIPI setups, I haven't implemented
> maximum flexibility. A part of the parameters is hard-coded, another part
> is provided in the platform driver, yet another part is calculated
> dynamically.
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 

---
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
ÿô.nlj·Ÿ®‰­†+%ŠË±é¥Šwÿº{.nlj·¥Š{±þg‰¯â^n‡r¡öë¨è&£ûz¹Þúzf£¢·hšˆ§~†­†Ûÿÿïÿ‘ê_èæ+v‰¨þ)ßø

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux