* [PATCH] media: i2c: ov5645: Add virtual_channel module parameter
@ 2020-02-28 16:41 Lad Prabhakar
2020-02-28 17:31 ` Fabio Estevam
2020-03-05 11:43 ` Sakari Ailus
0 siblings, 2 replies; 10+ messages in thread
From: Lad Prabhakar @ 2020-02-28 16:41 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sakari Ailus, Fabio Estevam
Cc: linux-media, linux-kernel, Lad Prabhakar
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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] media: i2c: ov5645: Add virtual_channel module parameter
2020-02-28 16:41 [PATCH] media: i2c: ov5645: Add virtual_channel module parameter Lad Prabhakar
@ 2020-02-28 17:31 ` Fabio Estevam
2020-03-02 7:18 ` Lad, Prabhakar
2020-03-05 11:43 ` Sakari Ailus
1 sibling, 1 reply; 10+ messages in thread
From: Fabio Estevam @ 2020-02-28 17:31 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media, linux-kernel,
Lad Prabhakar
Hi Lad,
On Fri, Feb 28, 2020 at 1:41 PM Lad Prabhakar
<prabhakar.csengg@gmail.com> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> 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?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: i2c: ov5645: Add virtual_channel module parameter
2020-02-28 17:31 ` Fabio Estevam
@ 2020-03-02 7:18 ` Lad, Prabhakar
2020-03-02 15:33 ` Fabio Estevam
0 siblings, 1 reply; 10+ messages in thread
From: Lad, Prabhakar @ 2020-03-02 7:18 UTC (permalink / raw)
To: Fabio Estevam
Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media, linux-kernel,
Lad Prabhakar
Hi Fabio,
Thank you for the review.
On Fri, Feb 28, 2020 at 5:31 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Lad,
>
> On Fri, Feb 28, 2020 at 1:41 PM Lad Prabhakar
> <prabhakar.csengg@gmail.com> 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 <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: i2c: ov5645: Add virtual_channel module parameter
2020-03-02 7:18 ` Lad, Prabhakar
@ 2020-03-02 15:33 ` Fabio Estevam
2020-03-03 3:01 ` Ezequiel Garcia
0 siblings, 1 reply; 10+ messages in thread
From: Fabio Estevam @ 2020-03-02 15:33 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media, linux-kernel,
Lad Prabhakar
Hi Prabhakar,
On Mon, Mar 2, 2020 at 4:19 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: i2c: ov5645: Add virtual_channel module parameter
2020-03-02 15:33 ` Fabio Estevam
@ 2020-03-03 3:01 ` Ezequiel Garcia
2020-03-03 7:01 ` Lad, Prabhakar
0 siblings, 1 reply; 10+ messages in thread
From: Ezequiel Garcia @ 2020-03-03 3:01 UTC (permalink / raw)
To: Fabio Estevam
Cc: Lad, Prabhakar, Mauro Carvalho Chehab, Sakari Ailus, linux-media,
linux-kernel, Lad Prabhakar, Jacopo Mondi, Niklas Söderlund
Adding Niklas and Jacopo,
On Mon, Mar 2, 2020, 12:33 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Prabhakar,
>
> On Mon, Mar 2, 2020 at 4:19 AM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: i2c: ov5645: Add virtual_channel module parameter
2020-03-03 3:01 ` Ezequiel Garcia
@ 2020-03-03 7:01 ` Lad, Prabhakar
0 siblings, 0 replies; 10+ messages in thread
From: Lad, Prabhakar @ 2020-03-03 7:01 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Fabio Estevam, Mauro Carvalho Chehab, Sakari Ailus, linux-media,
linux-kernel, Lad Prabhakar, Jacopo Mondi, Niklas Söderlund
Hi Ezequiel,
Thank you for the review.
On Tue, Mar 3, 2020 at 3:01 AM Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
>
> Adding Niklas and Jacopo,
>
> On Mon, Mar 2, 2020, 12:33 PM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Prabhakar,
> >
> > On Mon, Mar 2, 2020 at 4:19 AM Lad, Prabhakar
> > <prabhakar.csengg@gmail.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: i2c: ov5645: Add virtual_channel module parameter
2020-02-28 16:41 [PATCH] media: i2c: ov5645: Add virtual_channel module parameter Lad Prabhakar
2020-02-28 17:31 ` Fabio Estevam
@ 2020-03-05 11:43 ` Sakari Ailus
2020-03-06 10:18 ` Lad, Prabhakar
1 sibling, 1 reply; 10+ messages in thread
From: Sakari Ailus @ 2020-03-05 11:43 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Mauro Carvalho Chehab, Fabio Estevam, linux-media, linux-kernel,
Lad Prabhakar, Jacopo Mondi
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: i2c: ov5645: Add virtual_channel module parameter
2020-03-05 11:43 ` Sakari Ailus
@ 2020-03-06 10:18 ` Lad, Prabhakar
2020-03-10 11:17 ` Lad, Prabhakar
0 siblings, 1 reply; 10+ messages in thread
From: Lad, Prabhakar @ 2020-03-06 10:18 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mauro Carvalho Chehab, Fabio Estevam, linux-media, LKML,
Lad Prabhakar, Jacopo Mondi
Hi Sakari,
On Thu, Mar 5, 2020 at 11:43 AM Sakari Ailus
<sakari.ailus@linux.intel.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: i2c: ov5645: Add virtual_channel module parameter
2020-03-06 10:18 ` Lad, Prabhakar
@ 2020-03-10 11:17 ` Lad, Prabhakar
2020-03-10 12:43 ` Sakari Ailus
0 siblings, 1 reply; 10+ messages in thread
From: Lad, Prabhakar @ 2020-03-10 11:17 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mauro Carvalho Chehab, Fabio Estevam, linux-media, LKML,
Lad Prabhakar, Jacopo Mondi
Hi Sakari,
On Fri, Mar 6, 2020 at 10:18 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> Hi Sakari,
>
> On Thu, Mar 5, 2020 at 11:43 AM Sakari Ailus
> <sakari.ailus@linux.intel.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] media: i2c: ov5645: Add virtual_channel module parameter
2020-03-10 11:17 ` Lad, Prabhakar
@ 2020-03-10 12:43 ` Sakari Ailus
0 siblings, 0 replies; 10+ messages in thread
From: Sakari Ailus @ 2020-03-10 12:43 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Mauro Carvalho Chehab, Fabio Estevam, linux-media, LKML,
Lad Prabhakar, Jacopo Mondi
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
> <prabhakar.csengg@gmail.com> wrote:
> >
> > Hi Sakari,
> >
> > On Thu, Mar 5, 2020 at 11:43 AM Sakari Ailus
> > <sakari.ailus@linux.intel.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-03-10 12:43 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28 16:41 [PATCH] media: i2c: ov5645: Add virtual_channel module parameter Lad Prabhakar
2020-02-28 17:31 ` Fabio Estevam
2020-03-02 7:18 ` Lad, Prabhakar
2020-03-02 15:33 ` Fabio Estevam
2020-03-03 3:01 ` Ezequiel Garcia
2020-03-03 7:01 ` Lad, Prabhakar
2020-03-05 11:43 ` Sakari Ailus
2020-03-06 10:18 ` Lad, Prabhakar
2020-03-10 11:17 ` Lad, Prabhakar
2020-03-10 12:43 ` Sakari Ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).