* [PATCH 2/3] media: i2c: isl7998x: Use 0xf page to program all four pages
2024-07-13 15:41 [PATCH 1/3] media: i2c: isl7998x: Enable VACTIVE and HDELAY shadow registers Marek Vasut
@ 2024-07-13 15:41 ` Marek Vasut
2024-07-13 15:41 ` [PATCH 3/3] media: i2c: isl7998x: Implement get_mbus_config Marek Vasut
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Marek Vasut @ 2024-07-13 15:41 UTC (permalink / raw)
To: linux-media
Cc: Marek Vasut, Mauro Carvalho Chehab, Michael Tretter,
Pengutronix Kernel Team
Use page 0xf to program the same value of SDT and short detection
settings into all four pages 1..4 . This reduces the I2C traffic
slightly.
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Michael Tretter <m.tretter@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: linux-media@vger.kernel.org
---
drivers/media/i2c/isl7998x.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/media/i2c/isl7998x.c b/drivers/media/i2c/isl7998x.c
index 5f8f11f7a2c1c..7d2799e498520 100644
--- a/drivers/media/i2c/isl7998x.c
+++ b/drivers/media/i2c/isl7998x.c
@@ -312,14 +312,8 @@
static const struct reg_sequence isl7998x_init_seq_1[] = {
{ ISL7998X_REG_P0_SHORT_DIAG_IRQ_EN, 0xff },
- { ISL7998X_REG_PX_DEC_SDT(0x1), 0x07 },
- { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x1), 0x03 },
- { ISL7998X_REG_PX_DEC_SDT(0x2), 0x07 },
- { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x2), 0x03 },
- { ISL7998X_REG_PX_DEC_SDT(0x3), 0x07 },
- { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x3), 0x03 },
- { ISL7998X_REG_PX_DEC_SDT(0x4), 0x07 },
- { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x4), 0x03 },
+ { ISL7998X_REG_PX_DEC_SDT(0xf), 0x07 },
+ { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0xf), 0x03 },
{ ISL7998X_REG_P5_LI_ENGINE_CTL, 0x00 },
{ ISL7998X_REG_P0_SW_RESET_CTL, 0x1f, 10 },
{ ISL7998X_REG_P0_IO_BUFFER_CTL, 0x00 },
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/3] media: i2c: isl7998x: Implement get_mbus_config
2024-07-13 15:41 [PATCH 1/3] media: i2c: isl7998x: Enable VACTIVE and HDELAY shadow registers Marek Vasut
2024-07-13 15:41 ` [PATCH 2/3] media: i2c: isl7998x: Use 0xf page to program all four pages Marek Vasut
@ 2024-07-13 15:41 ` Marek Vasut
2024-07-14 14:33 ` Francesco Dolcini
2024-09-24 22:44 ` [PATCH 1/3] media: i2c: isl7998x: Enable VACTIVE and HDELAY shadow registers Marek Vasut
2024-10-01 14:56 ` Michael Tretter
3 siblings, 1 reply; 9+ messages in thread
From: Marek Vasut @ 2024-07-13 15:41 UTC (permalink / raw)
To: linux-media
Cc: Marek Vasut, Mauro Carvalho Chehab, Michael Tretter,
Pengutronix Kernel Team
This is used e.g. by imx6-mipi-csi2.c to determine the CSI2 lane count.
Implement the callback in isl7998x driver so it can be used with i.MX6
CSI2 receiver.
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Michael Tretter <m.tretter@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: linux-media@vger.kernel.org
---
drivers/media/i2c/isl7998x.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/media/i2c/isl7998x.c b/drivers/media/i2c/isl7998x.c
index 7d2799e498520..560f4845b5e62 100644
--- a/drivers/media/i2c/isl7998x.c
+++ b/drivers/media/i2c/isl7998x.c
@@ -1045,6 +1045,17 @@ static int isl7998x_set_fmt(struct v4l2_subdev *sd,
return 0;
}
+static int isl7998x_get_mbus_config(struct v4l2_subdev *sd, unsigned int pad,
+ struct v4l2_mbus_config *cfg)
+{
+ struct isl7998x *isl7998x = sd_to_isl7998x(sd);
+
+ cfg->type = V4L2_MBUS_CSI2_DPHY;
+ cfg->bus.mipi_csi2.num_data_lanes = isl7998x->nr_mipi_lanes;
+
+ return 0;
+}
+
static int isl7998x_set_ctrl(struct v4l2_ctrl *ctrl)
{
struct isl7998x *isl7998x = container_of(ctrl->handler,
@@ -1104,6 +1115,7 @@ static const struct v4l2_subdev_pad_ops isl7998x_subdev_pad_ops = {
.enum_frame_size = isl7998x_enum_frame_size,
.get_fmt = isl7998x_get_fmt,
.set_fmt = isl7998x_set_fmt,
+ .get_mbus_config = isl7998x_get_mbus_config,
};
static const struct v4l2_subdev_ops isl7998x_subdev_ops = {
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] media: i2c: isl7998x: Implement get_mbus_config
2024-07-13 15:41 ` [PATCH 3/3] media: i2c: isl7998x: Implement get_mbus_config Marek Vasut
@ 2024-07-14 14:33 ` Francesco Dolcini
2024-07-14 15:37 ` Marek Vasut
0 siblings, 1 reply; 9+ messages in thread
From: Francesco Dolcini @ 2024-07-14 14:33 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-media, Mauro Carvalho Chehab, Michael Tretter,
Pengutronix Kernel Team
Hello Marek,
On Sat, Jul 13, 2024 at 05:41:56PM +0200, Marek Vasut wrote:
> This is used e.g. by imx6-mipi-csi2.c to determine the CSI2 lane count.
> Implement the callback in isl7998x driver so it can be used with i.MX6
> CSI2 receiver.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
We had a similar need and we did send a similar patch for ov5640 [1],
however it was rejected. I did not follow-up if anything changed that
makes this patch not required, or maybe I am misunderstanding all of it?
Any comment?
Francesco
[1] https://lore.kernel.org/all/20230306063649.7387-1-marcel@ziswiler.com/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] media: i2c: isl7998x: Implement get_mbus_config
2024-07-14 14:33 ` Francesco Dolcini
@ 2024-07-14 15:37 ` Marek Vasut
2024-07-14 20:36 ` Francesco Dolcini
0 siblings, 1 reply; 9+ messages in thread
From: Marek Vasut @ 2024-07-14 15:37 UTC (permalink / raw)
To: Francesco Dolcini
Cc: linux-media, Mauro Carvalho Chehab, Michael Tretter,
Pengutronix Kernel Team
On 7/14/24 4:33 PM, Francesco Dolcini wrote:
> Hello Marek,
Hi,
> On Sat, Jul 13, 2024 at 05:41:56PM +0200, Marek Vasut wrote:
>> This is used e.g. by imx6-mipi-csi2.c to determine the CSI2 lane count.
>> Implement the callback in isl7998x driver so it can be used with i.MX6
>> CSI2 receiver.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>
> We had a similar need and we did send a similar patch for ov5640 [1],
> however it was rejected. I did not follow-up if anything changed that
> makes this patch not required, or maybe I am misunderstanding all of it?
> Any comment?
I am not sure what to make of that discussion, there was a lot of
discussion, but no conclusion or hint how to move forward . So, what now?
> Francesco
>
> [1] https://lore.kernel.org/all/20230306063649.7387-1-marcel@ziswiler.com/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] media: i2c: isl7998x: Implement get_mbus_config
2024-07-14 15:37 ` Marek Vasut
@ 2024-07-14 20:36 ` Francesco Dolcini
0 siblings, 0 replies; 9+ messages in thread
From: Francesco Dolcini @ 2024-07-14 20:36 UTC (permalink / raw)
To: Marek Vasut
Cc: Francesco Dolcini, linux-media, Mauro Carvalho Chehab,
Michael Tretter, Pengutronix Kernel Team
On Sun, Jul 14, 2024 at 05:37:10PM +0200, Marek Vasut wrote:
> On 7/14/24 4:33 PM, Francesco Dolcini wrote:
> > On Sat, Jul 13, 2024 at 05:41:56PM +0200, Marek Vasut wrote:
> > > This is used e.g. by imx6-mipi-csi2.c to determine the CSI2 lane count.
> > > Implement the callback in isl7998x driver so it can be used with i.MX6
> > > CSI2 receiver.
> > >
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> >
> > We had a similar need and we did send a similar patch for ov5640 [1],
> > however it was rejected. I did not follow-up if anything changed that
> > makes this patch not required, or maybe I am misunderstanding all of it?
> > Any comment?
>
> I am not sure what to make of that discussion, there was a lot of
> discussion, but no conclusion or hint how to move forward . So, what now?
That discusion was very confusing to me, but given I know little on the
topic I was hoping you could understand better than me ...
BTW, I am not even sure this patch is about the same issue.
Francesco
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] media: i2c: isl7998x: Enable VACTIVE and HDELAY shadow registers
2024-07-13 15:41 [PATCH 1/3] media: i2c: isl7998x: Enable VACTIVE and HDELAY shadow registers Marek Vasut
2024-07-13 15:41 ` [PATCH 2/3] media: i2c: isl7998x: Use 0xf page to program all four pages Marek Vasut
2024-07-13 15:41 ` [PATCH 3/3] media: i2c: isl7998x: Implement get_mbus_config Marek Vasut
@ 2024-09-24 22:44 ` Marek Vasut
2024-10-01 14:56 ` Michael Tretter
3 siblings, 0 replies; 9+ messages in thread
From: Marek Vasut @ 2024-09-24 22:44 UTC (permalink / raw)
To: linux-media
Cc: Mauro Carvalho Chehab, Michael Tretter, Pengutronix Kernel Team
On 7/13/24 5:41 PM, Marek Vasut wrote:
> The VACTIVE and HDELAY shadow registers must be enabled for per channel
> NTSC/PAL detection to work, enable them.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Michael Tretter <m.tretter@pengutronix.de>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: linux-media@vger.kernel.org
> ---
> drivers/media/i2c/isl7998x.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/isl7998x.c b/drivers/media/i2c/isl7998x.c
> index c7089035bbc10..5f8f11f7a2c1c 100644
> --- a/drivers/media/i2c/isl7998x.c
> +++ b/drivers/media/i2c/isl7998x.c
> @@ -312,13 +312,13 @@
>
> static const struct reg_sequence isl7998x_init_seq_1[] = {
> { ISL7998X_REG_P0_SHORT_DIAG_IRQ_EN, 0xff },
> - { ISL7998X_REG_PX_DEC_SDT(0x1), 0x00 },
> + { ISL7998X_REG_PX_DEC_SDT(0x1), 0x07 },
> { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x1), 0x03 },
> - { ISL7998X_REG_PX_DEC_SDT(0x2), 0x00 },
> + { ISL7998X_REG_PX_DEC_SDT(0x2), 0x07 },
> { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x2), 0x03 },
> - { ISL7998X_REG_PX_DEC_SDT(0x3), 0x00 },
> + { ISL7998X_REG_PX_DEC_SDT(0x3), 0x07 },
> { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x3), 0x03 },
> - { ISL7998X_REG_PX_DEC_SDT(0x4), 0x00 },
> + { ISL7998X_REG_PX_DEC_SDT(0x4), 0x07 },
> { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x4), 0x03 },
> { ISL7998X_REG_P5_LI_ENGINE_CTL, 0x00 },
> { ISL7998X_REG_P0_SW_RESET_CTL, 0x1f, 10 },
Any news on these patches ?
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] media: i2c: isl7998x: Enable VACTIVE and HDELAY shadow registers
2024-07-13 15:41 [PATCH 1/3] media: i2c: isl7998x: Enable VACTIVE and HDELAY shadow registers Marek Vasut
` (2 preceding siblings ...)
2024-09-24 22:44 ` [PATCH 1/3] media: i2c: isl7998x: Enable VACTIVE and HDELAY shadow registers Marek Vasut
@ 2024-10-01 14:56 ` Michael Tretter
2024-10-03 16:03 ` Marek Vasut
3 siblings, 1 reply; 9+ messages in thread
From: Michael Tretter @ 2024-10-01 14:56 UTC (permalink / raw)
To: Marek Vasut; +Cc: linux-media, Mauro Carvalho Chehab, Pengutronix Kernel Team
On Sat, 13 Jul 2024 17:41:54 +0200, Marek Vasut wrote:
> The VACTIVE and HDELAY shadow registers must be enabled for per channel
> NTSC/PAL detection to work, enable them.
I think the description is wrong as the change doesn't affect the
VACTIVE and HDELAY registers. The change sets the standard selection
register to auto detection to always trigger a standard detection during
initialization instead of defaulting to NTSC. querystd explicitly
triggers this behavior.
If the driver always runs standard detection during init, the driver
should also wait for the detection to be finished and update
isl7998x->norm accordingly, because otherwise the standard will be reset
to NTSC.
Maybe we can also get rid of this part from the init sequence and run
the standard detection during the initialization in code?
Michael
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Michael Tretter <m.tretter@pengutronix.de>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: linux-media@vger.kernel.org
> ---
> drivers/media/i2c/isl7998x.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/isl7998x.c b/drivers/media/i2c/isl7998x.c
> index c7089035bbc10..5f8f11f7a2c1c 100644
> --- a/drivers/media/i2c/isl7998x.c
> +++ b/drivers/media/i2c/isl7998x.c
> @@ -312,13 +312,13 @@
>
> static const struct reg_sequence isl7998x_init_seq_1[] = {
> { ISL7998X_REG_P0_SHORT_DIAG_IRQ_EN, 0xff },
> - { ISL7998X_REG_PX_DEC_SDT(0x1), 0x00 },
> + { ISL7998X_REG_PX_DEC_SDT(0x1), 0x07 },
> { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x1), 0x03 },
> - { ISL7998X_REG_PX_DEC_SDT(0x2), 0x00 },
> + { ISL7998X_REG_PX_DEC_SDT(0x2), 0x07 },
> { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x2), 0x03 },
> - { ISL7998X_REG_PX_DEC_SDT(0x3), 0x00 },
> + { ISL7998X_REG_PX_DEC_SDT(0x3), 0x07 },
> { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x3), 0x03 },
> - { ISL7998X_REG_PX_DEC_SDT(0x4), 0x00 },
> + { ISL7998X_REG_PX_DEC_SDT(0x4), 0x07 },
> { ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x4), 0x03 },
> { ISL7998X_REG_P5_LI_ENGINE_CTL, 0x00 },
> { ISL7998X_REG_P0_SW_RESET_CTL, 0x1f, 10 },
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] media: i2c: isl7998x: Enable VACTIVE and HDELAY shadow registers
2024-10-01 14:56 ` Michael Tretter
@ 2024-10-03 16:03 ` Marek Vasut
0 siblings, 0 replies; 9+ messages in thread
From: Marek Vasut @ 2024-10-03 16:03 UTC (permalink / raw)
To: Michael Tretter, linux-media, Mauro Carvalho Chehab,
Pengutronix Kernel Team
On 10/1/24 4:56 PM, Michael Tretter wrote:
> On Sat, 13 Jul 2024 17:41:54 +0200, Marek Vasut wrote:
>> The VACTIVE and HDELAY shadow registers must be enabled for per channel
>> NTSC/PAL detection to work, enable them.
>
> I think the description is wrong as the change doesn't affect the
> VACTIVE and HDELAY registers. The change sets the standard selection
> register to auto detection to always trigger a standard detection during
> initialization instead of defaulting to NTSC. querystd explicitly
> triggers this behavior.
It does, commit message fixed.
> If the driver always runs standard detection during init, the driver
> should also wait for the detection to be finished and update
> isl7998x->norm accordingly, because otherwise the standard will be reset
> to NTSC.
>
> Maybe we can also get rid of this part from the init sequence and run
> the standard detection during the initialization in code?
Maybe the init sequence is OK as-is and this and the follow up patches
should be discarded ? Users can run QUERYSTD to trigger the standard
detection when needed ?
^ permalink raw reply [flat|nested] 9+ messages in thread