OV5645 can operate in virtual channel 0-3 in CSI2 interfaces, this patch
adds support for module parameter virtual_channel to select the required
channel. By default OV5645 operates in virtual channel 0.
Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/media/i2c/ov5645.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
index a6c17d15d754..0a0671164623 100644
--- a/drivers/media/i2c/ov5645.c
+++ b/drivers/media/i2c/ov5645.c
@@ -54,6 +54,7 @@
#define OV5645_TIMING_TC_REG21 0x3821
#define OV5645_SENSOR_MIRROR BIT(1)
#define OV5645_MIPI_CTRL00 0x4800
+#define OV5645_REG_DEBUG_MODE 0x4814
#define OV5645_PRE_ISP_TEST_SETTING_1 0x503d
#define OV5645_TEST_PATTERN_MASK 0x3
#define OV5645_SET_TEST_PATTERN(x) ((x) & OV5645_TEST_PATTERN_MASK)
@@ -61,6 +62,11 @@
#define OV5645_SDE_SAT_U 0x5583
#define OV5645_SDE_SAT_V 0x5584
+static u8 virtual_channel;
+module_param(virtual_channel, byte, 0644);
+MODULE_PARM_DESC(virtual_channel,
+ "MIPI CSI-2 virtual channel (0..3), default 0");
+
/* regulator supplies */
static const char * const ov5645_supply_name[] = {
"vdddo", /* Digital I/O (1.8V) supply */
@@ -983,12 +989,34 @@ static int ov5645_get_selection(struct v4l2_subdev *sd,
return 0;
}
+static int ov5645_set_virtual_channel(struct ov5645 *ov5645)
+{
+ u8 temp, channel = virtual_channel;
+ int ret;
+
+ if (channel > 3)
+ return -EINVAL;
+
+ ret = ov5645_read_reg(ov5645, OV5645_REG_DEBUG_MODE, &temp);
+ if (ret)
+ return ret;
+
+ temp &= ~(3 << 6);
+ temp |= (channel << 6);
+
+ return ov5645_write_reg(ov5645, OV5645_REG_DEBUG_MODE, temp);
+}
+
static int ov5645_s_stream(struct v4l2_subdev *subdev, int enable)
{
struct ov5645 *ov5645 = to_ov5645(subdev);
int ret;
if (enable) {
+ ret = ov5645_set_virtual_channel(ov5645);
+ if (ret < 0)
+ return ret;
+
ret = ov5645_set_register_array(ov5645,
ov5645->current_mode->data,
ov5645->current_mode->data_size);
--
2.20.1
Hi Lad,
On Fri, Feb 28, 2020 at 1:41 PM Lad Prabhakar
<[email protected]> wrote:
>
> OV5645 can operate in virtual channel 0-3 in CSI2 interfaces, this patch
> adds support for module parameter virtual_channel to select the required
> channel. By default OV5645 operates in virtual channel 0.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> drivers/media/i2c/ov5645.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> index a6c17d15d754..0a0671164623 100644
> --- a/drivers/media/i2c/ov5645.c
> +++ b/drivers/media/i2c/ov5645.c
> @@ -54,6 +54,7 @@
> #define OV5645_TIMING_TC_REG21 0x3821
> #define OV5645_SENSOR_MIRROR BIT(1)
> #define OV5645_MIPI_CTRL00 0x4800
> +#define OV5645_REG_DEBUG_MODE 0x4814
> #define OV5645_PRE_ISP_TEST_SETTING_1 0x503d
> #define OV5645_TEST_PATTERN_MASK 0x3
> #define OV5645_SET_TEST_PATTERN(x) ((x) & OV5645_TEST_PATTERN_MASK)
> @@ -61,6 +62,11 @@
> #define OV5645_SDE_SAT_U 0x5583
> #define OV5645_SDE_SAT_V 0x5584
>
> +static u8 virtual_channel;
> +module_param(virtual_channel, byte, 0644);
> +MODULE_PARM_DESC(virtual_channel,
> + "MIPI CSI-2 virtual channel (0..3), default 0");
Should this be a device tree property instead?
Hi Fabio,
Thank you for the review.
On Fri, Feb 28, 2020 at 5:31 PM Fabio Estevam <[email protected]> wrote:
>
> Hi Lad,
>
> On Fri, Feb 28, 2020 at 1:41 PM Lad Prabhakar
> <[email protected]> wrote:
> >
> > OV5645 can operate in virtual channel 0-3 in CSI2 interfaces, this patch
> > adds support for module parameter virtual_channel to select the required
> > channel. By default OV5645 operates in virtual channel 0.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > drivers/media/i2c/ov5645.c | 28 ++++++++++++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> > index a6c17d15d754..0a0671164623 100644
> > --- a/drivers/media/i2c/ov5645.c
> > +++ b/drivers/media/i2c/ov5645.c
> > @@ -54,6 +54,7 @@
> > #define OV5645_TIMING_TC_REG21 0x3821
> > #define OV5645_SENSOR_MIRROR BIT(1)
> > #define OV5645_MIPI_CTRL00 0x4800
> > +#define OV5645_REG_DEBUG_MODE 0x4814
> > #define OV5645_PRE_ISP_TEST_SETTING_1 0x503d
> > #define OV5645_TEST_PATTERN_MASK 0x3
> > #define OV5645_SET_TEST_PATTERN(x) ((x) & OV5645_TEST_PATTERN_MASK)
> > @@ -61,6 +62,11 @@
> > #define OV5645_SDE_SAT_U 0x5583
> > #define OV5645_SDE_SAT_V 0x5584
> >
> > +static u8 virtual_channel;
> > +module_param(virtual_channel, byte, 0644);
> > +MODULE_PARM_DESC(virtual_channel,
> > + "MIPI CSI-2 virtual channel (0..3), default 0");
>
> Should this be a device tree property instead?
I did give a thought about it, but making this as DT property would
make it more stiff.
Cheers,
--Prabhakar
Hi Prabhakar,
On Mon, Mar 2, 2020 at 4:19 AM Lad, Prabhakar
<[email protected]> wrote:
> > Should this be a device tree property instead?
> I did give a thought about it, but making this as DT property would
> make it more stiff.
In case a system has two OV5645 and we want to operate each OV5645
with a different virtual channel, it will not be possible with the
module_param approach.
Using a device tree property would make it possible though, so I think
it makes more sense to use a device tree property for this.
Thanks
Adding Niklas and Jacopo,
On Mon, Mar 2, 2020, 12:33 PM Fabio Estevam <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Mon, Mar 2, 2020 at 4:19 AM Lad, Prabhakar
> <[email protected]> wrote:
>
> > > Should this be a device tree property instead?
> > I did give a thought about it, but making this as DT property would
> > make it more stiff.
>
> In case a system has two OV5645 and we want to operate each OV5645
> with a different virtual channel, it will not be possible with the
> module_param approach.
>
> Using a device tree property would make it possible though, so I think
> it makes more sense to use a device tree property for this.
>
As often happens, driver parameter is probably the easiest and less
invasive way to customize a driver, so I can imagine myself carrying
something like this downstream if needed. Haven't we all?
It's definitely not suitable upstream, as Fabio points out, but
I don't think a devicetree approach is either.
It seems Niklas and Jacopo have been working on adding
proper support to route this, via some new ioctls.
https://patchwork.linuxtv.org/patch/55300/
Not sure what's the status of it.
Hope it helps,
Ezequiel
Hi Ezequiel,
Thank you for the review.
On Tue, Mar 3, 2020 at 3:01 AM Ezequiel Garcia
<[email protected]> wrote:
>
> Adding Niklas and Jacopo,
>
> On Mon, Mar 2, 2020, 12:33 PM Fabio Estevam <[email protected]> wrote:
> >
> > Hi Prabhakar,
> >
> > On Mon, Mar 2, 2020 at 4:19 AM Lad, Prabhakar
> > <[email protected]> wrote:
> >
> > > > Should this be a device tree property instead?
> > > I did give a thought about it, but making this as DT property would
> > > make it more stiff.
> >
> > In case a system has two OV5645 and we want to operate each OV5645
> > with a different virtual channel, it will not be possible with the
> > module_param approach.
> >
> > Using a device tree property would make it possible though, so I think
> > it makes more sense to use a device tree property for this.
> >
>
> As often happens, driver parameter is probably the easiest and less
> invasive way to customize a driver, so I can imagine myself carrying
> something like this downstream if needed. Haven't we all?
>
> It's definitely not suitable upstream, as Fabio points out, but
> I don't think a devicetree approach is either.
>
Agreed. I was suggesting maybe v4l2-ctl instead ?
> It seems Niklas and Jacopo have been working on adding
> proper support to route this, via some new ioctls.
>
> https://patchwork.linuxtv.org/patch/55300/
>
> Not sure what's the status of it.
>
something similar needs to be implemented for ov5645 driver.
Cheers,
--Prabhakar
> Hope it helps,
> Ezequiel
Hi Prabhakar,
On Fri, Feb 28, 2020 at 04:41:26PM +0000, Lad Prabhakar wrote:
> OV5645 can operate in virtual channel 0-3 in CSI2 interfaces, this patch
> adds support for module parameter virtual_channel to select the required
> channel. By default OV5645 operates in virtual channel 0.
What's your use case for different virtual channels?
--
Regards,
Sakari Ailus
Hi Sakari,
On Thu, Mar 5, 2020 at 11:43 AM Sakari Ailus
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Fri, Feb 28, 2020 at 04:41:26PM +0000, Lad Prabhakar wrote:
> > OV5645 can operate in virtual channel 0-3 in CSI2 interfaces, this patch
> > adds support for module parameter virtual_channel to select the required
> > channel. By default OV5645 operates in virtual channel 0.
>
> What's your use case for different virtual channels?
>
Just ability to switch to different virtual channel, based on ov5640
driver. The rcar-csi2
has capability to capture from multiple channels so that we can
capture simultaneously
from two sensors.
Cheers,
--Prabhakar
> --
> Regards,
>
> Sakari Ailus
Hi Sakari,
On Fri, Mar 6, 2020 at 10:18 AM Lad, Prabhakar
<[email protected]> wrote:
>
> Hi Sakari,
>
> On Thu, Mar 5, 2020 at 11:43 AM Sakari Ailus
> <[email protected]> wrote:
> >
> > Hi Prabhakar,
> >
> > On Fri, Feb 28, 2020 at 04:41:26PM +0000, Lad Prabhakar wrote:
> > > OV5645 can operate in virtual channel 0-3 in CSI2 interfaces, this patch
> > > adds support for module parameter virtual_channel to select the required
> > > channel. By default OV5645 operates in virtual channel 0.
> >
> > What's your use case for different virtual channels?
> >
> Just ability to switch to different virtual channel, based on ov5640
> driver. The rcar-csi2
> has capability to capture from multiple channels so that we can
> capture simultaneously
> from two sensors.
>
Any thoughts on how this could be handled ?
Cheers,
--Prabhakar
On Tue, Mar 10, 2020 at 11:17:10AM +0000, Lad, Prabhakar wrote:
> Hi Sakari,
>
> On Fri, Mar 6, 2020 at 10:18 AM Lad, Prabhakar
> <[email protected]> wrote:
> >
> > Hi Sakari,
> >
> > On Thu, Mar 5, 2020 at 11:43 AM Sakari Ailus
> > <[email protected]> wrote:
> > >
> > > Hi Prabhakar,
> > >
> > > On Fri, Feb 28, 2020 at 04:41:26PM +0000, Lad Prabhakar wrote:
> > > > OV5645 can operate in virtual channel 0-3 in CSI2 interfaces, this patch
> > > > adds support for module parameter virtual_channel to select the required
> > > > channel. By default OV5645 operates in virtual channel 0.
> > >
> > > What's your use case for different virtual channels?
> > >
> > Just ability to switch to different virtual channel, based on ov5640
> > driver. The rcar-csi2
> > has capability to capture from multiple channels so that we can
> > capture simultaneously
> > from two sensors.
> >
> Any thoughts on how this could be handled ?
A module parameter to support sending the pixel data on virtual channels is
certainly a hack. You couldn't support the same kind of sensors with
different virtual channel configuration in a deterministic way nor the
receiver would have an ability to verify the hardware is properly
configured.
The multiplexed streams patchset (subject "[PATCH v3 00/31] v4l: add
support for multiplexed streams" on LMML) adds support for CSI-2 virtual
channels and data types. It's been a while since a version of that was
posted though.
Jacopo, what are your plans regarding that set?
--
Kind regards,
Sakari Ailus