[RFC,1/3] media: tw9910: Allow to probe from device tree

  Currently the driver only probes via platform data passed from
board file.

Allow to probe from device tree too.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
Hi,

I am currently sending this series as RFC because I was
not able to get the TW9990 to work on a imx6sx board yet.

# media-ctl -p
Media controller API version 5.14.0

Media device information
------------------------
driver          imx7-csi
model           imx-media
serial          
bus info        
hw revision     0x0
driver version  5.14.0

Device topology
- entity 1: csi (2 pads, 2 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev0
	pad0: Sink
		[fmt:UYVY8_2X8/800x600 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
		<- "tw9910 2-0044":0 [ENABLED,IMMUTABLE]
	pad1: Source
		[fmt:UYVY8_2X8/800x600 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
		-> "csi capture":0 [ENABLED,IMMUTABLE]

- entity 4: csi capture (1 pad, 1 link)
            type Node subtype V4L flags 0
            device node name /dev/video0
	pad0: Sink
		<- "csi":1 [ENABLED,IMMUTABLE]

- entity 10: tw9910 2-0044 (1 pad, 1 link)
             type V4L2 subdev subtype Decoder flags 0
             device node name /dev/v4l-subdev1
	pad0: Source
		[fmt:UYVY8_2X8/640x480 field:interlaced-bt colorspace:smpte170m
		 crop.bounds:(0,0)/640x480
		 crop:(0,0)/640x480]
		-> "csi":0 [ENABLED,IMMUTABLE]

I get the following error when setting up the pipeline:

media-ctl -l "'tw9910 2-0044':0 -> 'csi':0[1]"
media-ctl -l "'csi':1 -> 'csi capture':0[1]"
media-ctl -v -V "'tw9910 2-0044':0 [fmt:UYVY8_2X8/800x480 field:any]"

Opening media device /dev/media0
Enumerating entities
Found 3 entities
Enumerating pads and links
Setting up format UYVY8_2X8 800x480 on pad tw9910 2-0044/0
Unable to set format: No such device or address (-6)
Unable to setup formats: No such device or address (6)

This -6 (ENXIO) error comes from:

tw9910_set_frame() ---> tw9910_mask_set() ---> i2c_smbus_read_byte_data():


static int tw9910_mask_set(struct i2c_client *client, u8 command,
			   u8 mask, u8 set)
{
	s32 val = i2c_smbus_read_byte_data(client, command);

I am able to dump TW9990 registers via i2cdetect and also via the probe
function, so I2C access is OK.

Not sure why I am getting these i2c_smbus_read_byte_data() errors.


Any ideas?

Thanks!


 drivers/media/i2c/tw9910.c | 47 ++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 9 deletions(-)
  

On Fri, Aug 27, 2021 at 10:02 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Currently the driver only probes via platform data passed from
> board file.
>
> Allow to probe from device tree too.
>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
> Hi,
>
> I am currently sending this series as RFC because I was
> not able to get the TW9990 to work on a imx6sx board yet.
>
> # media-ctl -p
> Media controller API version 5.14.0
>
> Media device information
> ------------------------
> driver          imx7-csi
> model           imx-media
> serial
> bus info
> hw revision     0x0
> driver version  5.14.0
>
> Device topology
> - entity 1: csi (2 pads, 2 links)
>             type V4L2 subdev subtype Unknown flags 0
>             device node name /dev/v4l-subdev0
>         pad0: Sink
>                 [fmt:UYVY8_2X8/800x600 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
>                 <- "tw9910 2-0044":0 [ENABLED,IMMUTABLE]
>         pad1: Source
>                 [fmt:UYVY8_2X8/800x600 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
>                 -> "csi capture":0 [ENABLED,IMMUTABLE]
>
> - entity 4: csi capture (1 pad, 1 link)
>             type Node subtype V4L flags 0
>             device node name /dev/video0
>         pad0: Sink
>                 <- "csi":1 [ENABLED,IMMUTABLE]
>
> - entity 10: tw9910 2-0044 (1 pad, 1 link)
>              type V4L2 subdev subtype Decoder flags 0
>              device node name /dev/v4l-subdev1
>         pad0: Source
>                 [fmt:UYVY8_2X8/640x480 field:interlaced-bt colorspace:smpte170m
>                  crop.bounds:(0,0)/640x480
>                  crop:(0,0)/640x480]
>                 -> "csi":0 [ENABLED,IMMUTABLE]
>
> I get the following error when setting up the pipeline:
>
> media-ctl -l "'tw9910 2-0044':0 -> 'csi':0[1]"
> media-ctl -l "'csi':1 -> 'csi capture':0[1]"
> media-ctl -v -V "'tw9910 2-0044':0 [fmt:UYVY8_2X8/800x480 field:any]"
>
> Opening media device /dev/media0
> Enumerating entities
> Found 3 entities
> Enumerating pads and links
> Setting up format UYVY8_2X8 800x480 on pad tw9910 2-0044/0
> Unable to set format: No such device or address (-6)
> Unable to setup formats: No such device or address (6)
>
> This -6 (ENXIO) error comes from:
>
> tw9910_set_frame() ---> tw9910_mask_set() ---> i2c_smbus_read_byte_data():
>
>
> static int tw9910_mask_set(struct i2c_client *client, u8 command,
>                            u8 mask, u8 set)
> {
>         s32 val = i2c_smbus_read_byte_data(client, command);
>
> I am able to dump TW9990 registers via i2cdetect and also via the probe
> function, so I2C access is OK.
>
> Not sure why I am getting these i2c_smbus_read_byte_data() errors.

I found the reason: the tw9910 driver was getting unpowered in the
incorrect place.
This problem was present prior to the DT conversion.

I will send a fix for it soon.

Thanks
  

Hi Fabio,

On Fri, Aug 27, 2021 at 10:01:48AM -0300, Fabio Estevam wrote:
> Currently the driver only probes via platform data passed from
> board file.
>
> Allow to probe from device tree too.
>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
> Hi,
>
> I am currently sending this series as RFC because I was
> not able to get the TW9990 to work on a imx6sx board yet.

I'm sorry but I've no access to any TW99xx device at the moment to
test with

>
> # media-ctl -p
> Media controller API version 5.14.0
>
> Media device information
> ------------------------
> driver          imx7-csi
> model           imx-media
> serial
> bus info
> hw revision     0x0
> driver version  5.14.0
>
> Device topology
> - entity 1: csi (2 pads, 2 links)
>             type V4L2 subdev subtype Unknown flags 0
>             device node name /dev/v4l-subdev0
> 	pad0: Sink
> 		[fmt:UYVY8_2X8/800x600 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
> 		<- "tw9910 2-0044":0 [ENABLED,IMMUTABLE]
> 	pad1: Source
> 		[fmt:UYVY8_2X8/800x600 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range]
> 		-> "csi capture":0 [ENABLED,IMMUTABLE]
>
> - entity 4: csi capture (1 pad, 1 link)
>             type Node subtype V4L flags 0
>             device node name /dev/video0
> 	pad0: Sink
> 		<- "csi":1 [ENABLED,IMMUTABLE]
>
> - entity 10: tw9910 2-0044 (1 pad, 1 link)
>              type V4L2 subdev subtype Decoder flags 0
>              device node name /dev/v4l-subdev1
> 	pad0: Source
> 		[fmt:UYVY8_2X8/640x480 field:interlaced-bt colorspace:smpte170m
> 		 crop.bounds:(0,0)/640x480
> 		 crop:(0,0)/640x480]
> 		-> "csi":0 [ENABLED,IMMUTABLE]
>
> I get the following error when setting up the pipeline:
>
> media-ctl -l "'tw9910 2-0044':0 -> 'csi':0[1]"
> media-ctl -l "'csi':1 -> 'csi capture':0[1]"
> media-ctl -v -V "'tw9910 2-0044':0 [fmt:UYVY8_2X8/800x480 field:any]"
>
> Opening media device /dev/media0
> Enumerating entities
> Found 3 entities
> Enumerating pads and links
> Setting up format UYVY8_2X8 800x480 on pad tw9910 2-0044/0
> Unable to set format: No such device or address (-6)
> Unable to setup formats: No such device or address (6)
>
> This -6 (ENXIO) error comes from:
>
> tw9910_set_frame() ---> tw9910_mask_set() ---> i2c_smbus_read_byte_data():
>
>
> static int tw9910_mask_set(struct i2c_client *client, u8 command,
> 			   u8 mask, u8 set)
> {
> 	s32 val = i2c_smbus_read_byte_data(client, command);
>
> I am able to dump TW9990 registers via i2cdetect and also via the probe
> function, so I2C access is OK.
>
> Not sure why I am getting these i2c_smbus_read_byte_data() errors.

There's an

	tw9910_s_power(&priv->subdev, 0);

at the end of the video_probe() function.

The driver handles power management with the legacy s_power() call
chain, and the receiver driver needs to v4l2_pipeline_pm_get() which
the imx driver does when the capture node is open.

Just an hint, you might have noticed already

>
>
> Any ideas?
>
> Thanks!
>
>
>  drivers/media/i2c/tw9910.c | 47 ++++++++++++++++++++++++++++++--------
>  1 file changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
> index 09f5b3986928..04f3c2dbc1cc 100644
> --- a/drivers/media/i2c/tw9910.c
> +++ b/drivers/media/i2c/tw9910.c
> @@ -22,6 +22,7 @@
>  #include <linux/init.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/of.h>

If you need this for struct of_device_id then you should include
<linux/mod_devicetable.h>

>  #include <linux/slab.h>
>  #include <linux/v4l2-mediabus.h>
>  #include <linux/videodev2.h>
> @@ -928,22 +929,27 @@ static const struct v4l2_subdev_ops tw9910_subdev_ops = {
>   * i2c_driver function
>   */
>
> +static int tw9910_parse_dt(struct i2c_client *client, struct tw9910_priv *priv)
> +{
> +	priv->info = devm_kzalloc(&client->dev, sizeof(*priv->info), GFP_KERNEL);
> +	if (!priv->info)

IS_ERR_OR_NULL() ?

> +		return -ENOMEM;

PTR_ERR() ?

> +
> +	/* Use default for now. Will retrieve from dt later */
> +	priv->info->mpout = 0;
> +	priv->info->buswidth = 8;
> +
> +	return 0;
> +}
> +
>  static int tw9910_probe(struct i2c_client *client,
>  			const struct i2c_device_id *did)
>
>  {
>  	struct tw9910_priv		*priv;
> -	struct tw9910_video_info	*info;
>  	struct i2c_adapter		*adapter = client->adapter;
>  	int ret;
>
> -	if (!client->dev.platform_data) {
> -		dev_err(&client->dev, "TW9910: missing platform data!\n");
> -		return -EINVAL;
> -	}
> -
> -	info = client->dev.platform_data;
> -
>  	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
>  		dev_err(&client->dev,
>  			"I2C-Adapter doesn't support I2C_FUNC_SMBUS_BYTE_DATA\n");
> @@ -954,7 +960,18 @@ static int tw9910_probe(struct i2c_client *client,
>  	if (!priv)
>  		return -ENOMEM;
>
> -	priv->info = info;
> +	if (IS_ENABLED(CONFIG_OF) && client->dev.of_node) {
> +		ret = tw9910_parse_dt(client, priv);
> +		if (ret < 0) {
> +			v4l_err(client, "DT parsing error\n");
> +			return ret;
> +		}
> +	} else if (client->dev.platform_data) {
> +		priv->info = client->dev.platform_data;
> +	} else {
> +		v4l_err(client, "No platform data!\n");
> +		return -ENODEV;
> +	}
>
>  	v4l2_i2c_subdev_init(&priv->subdev, client, &tw9910_subdev_ops);
>
> @@ -1007,13 +1024,25 @@ static int tw9910_remove(struct i2c_client *client)
>
>  static const struct i2c_device_id tw9910_id[] = {
>  	{ "tw9910", 0 },
> +	{ "tw9990", 0 },

Unrelated to this patch maybe ?

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, tw9910_id);
>
> +#ifdef CONFIG_OF

I think this can be dropped ?

> +static const struct of_device_id tw9910_of_id[] = {
> +	{ .compatible = "renesas,tw9910", },
> +	{ .compatible = "renesas,tw9990", },


TW9910 was originally manufactured by Techwell, then bought by
Intersil, then bought by Renesas. Not sure what I would use as the
chip is still marketed as "Techwell" and the public documentation
released by "Intersil", according to my web searches..


> +	{ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, tw9910_of_id);
> +#endif
> +
>  static struct i2c_driver tw9910_i2c_driver = {
>  	.driver = {
>  		.name = "tw9910",
> +		.of_match_table = of_match_ptr(tw9910_of_id),
>  	},
>  	.probe    = tw9910_probe,
>  	.remove   = tw9910_remove,
> --
> 2.25.1
>
  

Hi Jacopo,

On Mon, Sep 13, 2021 at 5:58 AM Jacopo Mondi <jacopo@jmondi.org> wrote:

> There's an
>
>         tw9910_s_power(&priv->subdev, 0);
>
> at the end of the video_probe() function.
>
> The driver handles power management with the legacy s_power() call
> chain, and the receiver driver needs to v4l2_pipeline_pm_get() which
> the imx driver does when the capture node is open.
>
> Just an hint, you might have noticed already

Thanks for your comments and review. Yes, I have fixed the I2C errors.

I plan to re-submit the entire series after I get the TW9990 to work
on my imx6sx-based board.

The capture driver is drivers/staging/media/imx/imx7-media-csi.c.

Currently, I am not able to get the stream to start.

This is the configuration I am using:

media-ctl -l "'tw9910 2-0044':0 -> 'csi':0[1]"
media-ctl -l "'csi':1 -> 'csi capture':0[1]"
media-ctl -v -V "'tw9910 2-0044':0 [fmt:UYVY8_2X8/720x480 field:any]"
Opening media device /dev/media0
Enumerating entities
Found 3 entities
Enumerating pads and links
Setting up format UYVY8_2X8 720x480 on pad tw9910 2-0044/0
Format set: UYVY8_2X8 720x480
Setting up format UYVY8_2X8 720x480 on pad csi/0
Format set: UYVY8_2X8 720x480

Then I launch the capture stream command:

v4l2-ctl --stream-mmap -d /dev/video1

but nothing happens here, no >>>>> frame indication progress is shown.

If I hit CTRL + C then I get:
[  715.467623] csi: wait last EOF timeout

Any suggestions?

Thanks
  

On Mon, Sep 13, 2021 at 9:53 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Jacopo,
>
> On Mon, Sep 13, 2021 at 5:58 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> > There's an
> >
> >         tw9910_s_power(&priv->subdev, 0);
> >
> > at the end of the video_probe() function.
> >
> > The driver handles power management with the legacy s_power() call
> > chain, and the receiver driver needs to v4l2_pipeline_pm_get() which
> > the imx driver does when the capture node is open.
> >
> > Just an hint, you might have noticed already
>
> Thanks for your comments and review. Yes, I have fixed the I2C errors.
>
> I plan to re-submit the entire series after I get the TW9990 to work
> on my imx6sx-based board.
>
> The capture driver is drivers/staging/media/imx/imx7-media-csi.c.
>
> Currently, I am not able to get the stream to start.
>
> This is the configuration I am using:
>
> media-ctl -l "'tw9910 2-0044':0 -> 'csi':0[1]"
> media-ctl -l "'csi':1 -> 'csi capture':0[1]"
> media-ctl -v -V "'tw9910 2-0044':0 [fmt:UYVY8_2X8/720x480 field:any]"
> Opening media device /dev/media0
> Enumerating entities
> Found 3 entities
> Enumerating pads and links
> Setting up format UYVY8_2X8 720x480 on pad tw9910 2-0044/0
> Format set: UYVY8_2X8 720x480
> Setting up format UYVY8_2X8 720x480 on pad csi/0
> Format set: UYVY8_2X8 720x480
>
> Then I launch the capture stream command:
>
> v4l2-ctl --stream-mmap -d /dev/video1
>
> but nothing happens here, no >>>>> frame indication progress is shown.
>
> If I hit CTRL + C then I get:
> [  715.467623] csi: wait last EOF timeout

I also need to set the video standard to PAL:

# v4l2-ctl --device /dev/v4l-subdev1 --set-standard PAL
Standard set to 000000ff

# media-ctl -l "'tw9910 2-0044':0 -> 'csi':0[1]"
# media-ctl -l "'csi':1 -> 'csi capture':0[1]"
# media-ctl -v -V "'tw9910 2-0044':0 [fmt:UYVY8_2X8/720x576
field:interlaced-bt]"
Opening media device /dev/media0
Enumerating entities
Found 3 entities
Enumerating pads and links
Setting up format UYVY8_2X8 720x576 on pad tw9910 2-0044/0
Format set: UYVY8_2X8 720x576
Setting up format UYVY8_2X8 720x576 on pad csi/0
Format set: UYVY8_2X8 720x576

# v4l2-ctl --stream-mmap -d /dev/video1
[   38.554032] priv->vdev.fmt.fmt.pix.width is 800
[   38.561444] f.fmt.pix.width is 720
[   38.567607] priv->vdev.fmt.fmt.pix.height is 600
[   38.574973] f.fmt.pix.height is 576
[   38.580992] priv->vdev.cc->cs is 1
[   38.586958] cc->cs is 1
[   38.591761] priv->vdev.compose.width is 800
[   38.598420] compose.width is 720
[   38.604048] priv->vdev.compose.height is 600
[   38.610681] compose.height is 576
[   38.616347] csi: capture format not valid
VIDIOC_STREAMON returned -1 (Broken pipe)

I added some printk debug lines to show the mismatch that prevents
the pipeline to start in capture_validate_fmt():

diff --git a/drivers/staging/media/imx/imx-media-capture.c
b/drivers/staging/media/imx/imx-media-capture.c
index b2f2cb3d6a60..511625981e93 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -552,6 +552,21 @@ static int capture_validate_fmt(struct capture_priv *priv)
        ret = __capture_try_fmt_vid_cap(priv, &fmt_src, &f, &cc, &compose);
        if (ret)
                return ret;
+
+       pr_err("priv->vdev.fmt.fmt.pix.width is %d\n",
priv->vdev.fmt.fmt.pix.width);
+       pr_err("f.fmt.pix.width is %d\n", f.fmt.pix.width);
+
+       pr_err("priv->vdev.fmt.fmt.pix.height is %d\n",
priv->vdev.fmt.fmt.pix.height);
+       pr_err("f.fmt.pix.height is %d\n", f.fmt.pix.height );
+
+       pr_err("priv->vdev.cc->cs is %d\n", priv->vdev.cc->cs);
+       pr_err("cc->cs is %d\n", cc->cs);
+
+       pr_err("priv->vdev.compose.width is %d\n", priv->vdev.compose.width);
+       pr_err("compose.width is %d\n", compose.width);
+
+       pr_err("priv->vdev.compose.height is %d\n", priv->vdev.compose.height);
+       pr_err("compose.height is %d\n", compose.height);

        return (priv->vdev.fmt.fmt.pix.width != f.fmt.pix.width ||
                priv->vdev.fmt.fmt.pix.height != f.fmt.pix.height ||

Any ideas as to whu such mismatch happens?

Thanks
  

Hi Fabio,
On Mon Sep 13, 2021 at 9:03 PM WEST, Fabio Estevam wrote:

> On Mon, Sep 13, 2021 at 9:53 AM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Jacopo,
> >
> > On Mon, Sep 13, 2021 at 5:58 AM Jacopo Mondi <jacopo@jmondi.org> wrote:
> >
> > > There's an
> > >
> > >         tw9910_s_power(&priv->subdev, 0);
> > >
> > > at the end of the video_probe() function.
> > >
> > > The driver handles power management with the legacy s_power() call
> > > chain, and the receiver driver needs to v4l2_pipeline_pm_get() which
> > > the imx driver does when the capture node is open.
> > >
> > > Just an hint, you might have noticed already
> >
> > Thanks for your comments and review. Yes, I have fixed the I2C errors.
> >
> > I plan to re-submit the entire series after I get the TW9990 to work
> > on my imx6sx-based board.
> >
> > The capture driver is drivers/staging/media/imx/imx7-media-csi.c.
> >
> > Currently, I am not able to get the stream to start.
> >
> > This is the configuration I am using:
> >
> > media-ctl -l "'tw9910 2-0044':0 -> 'csi':0[1]"
> > media-ctl -l "'csi':1 -> 'csi capture':0[1]"
> > media-ctl -v -V "'tw9910 2-0044':0 [fmt:UYVY8_2X8/720x480 field:any]"
> > Opening media device /dev/media0
> > Enumerating entities
> > Found 3 entities
> > Enumerating pads and links
> > Setting up format UYVY8_2X8 720x480 on pad tw9910 2-0044/0
> > Format set: UYVY8_2X8 720x480
> > Setting up format UYVY8_2X8 720x480 on pad csi/0
> > Format set: UYVY8_2X8 720x480
> >
> > Then I launch the capture stream command:
> >
> > v4l2-ctl --stream-mmap -d /dev/video1
> >
> > but nothing happens here, no >>>>> frame indication progress is shown.
> >
> > If I hit CTRL + C then I get:
> > [  715.467623] csi: wait last EOF timeout
>
> I also need to set the video standard to PAL:
>
> # v4l2-ctl --device /dev/v4l-subdev1 --set-standard PAL
> Standard set to 000000ff
>
> # media-ctl -l "'tw9910 2-0044':0 -> 'csi':0[1]"
> # media-ctl -l "'csi':1 -> 'csi capture':0[1]"
> # media-ctl -v -V "'tw9910 2-0044':0 [fmt:UYVY8_2X8/720x576
> field:interlaced-bt]"
> Opening media device /dev/media0
> Enumerating entities
> Found 3 entities
> Enumerating pads and links
> Setting up format UYVY8_2X8 720x576 on pad tw9910 2-0044/0
> Format set: UYVY8_2X8 720x576
> Setting up format UYVY8_2X8 720x576 on pad csi/0
> Format set: UYVY8_2X8 720x576
>
> # v4l2-ctl --stream-mmap -d /dev/video1
> [   38.554032] priv->vdev.fmt.fmt.pix.width is 800
> [   38.561444] f.fmt.pix.width is 720
> [   38.567607] priv->vdev.fmt.fmt.pix.height is 600
> [   38.574973] f.fmt.pix.height is 576
> [   38.580992] priv->vdev.cc->cs is 1
> [   38.586958] cc->cs is 1
> [   38.591761] priv->vdev.compose.width is 800
> [   38.598420] compose.width is 720
> [   38.604048] priv->vdev.compose.height is 600
> [   38.610681] compose.height is 576
> [   38.616347] csi: capture format not valid
> VIDIOC_STREAMON returned -1 (Broken pipe)
>
> I added some printk debug lines to show the mismatch that prevents
> the pipeline to start in capture_validate_fmt():
>
> diff --git a/drivers/staging/media/imx/imx-media-capture.c
> b/drivers/staging/media/imx/imx-media-capture.c
> index b2f2cb3d6a60..511625981e93 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -552,6 +552,21 @@ static int capture_validate_fmt(struct capture_priv *priv)
>         ret = __capture_try_fmt_vid_cap(priv, &fmt_src, &f, &cc, &compose);
>         if (ret)
>                 return ret;
> +
> +       pr_err("priv->vdev.fmt.fmt.pix.width is %d\n",
> priv->vdev.fmt.fmt.pix.width);
> +       pr_err("f.fmt.pix.width is %d\n", f.fmt.pix.width);
> +
> +       pr_err("priv->vdev.fmt.fmt.pix.height is %d\n",
> priv->vdev.fmt.fmt.pix.height);
> +       pr_err("f.fmt.pix.height is %d\n", f.fmt.pix.height );
> +
> +       pr_err("priv->vdev.cc->cs is %d\n", priv->vdev.cc->cs);
> +       pr_err("cc->cs is %d\n", cc->cs);
> +
> +       pr_err("priv->vdev.compose.width is %d\n", priv->vdev.compose.width);
> +       pr_err("compose.width is %d\n", compose.width);
> +
> +       pr_err("priv->vdev.compose.height is %d\n", priv->vdev.compose.height);
> +       pr_err("compose.height is %d\n", compose.height);
>
>         return (priv->vdev.fmt.fmt.pix.width != f.fmt.pix.width ||
>                 priv->vdev.fmt.fmt.pix.height != f.fmt.pix.height ||
>
> Any ideas as to whu such mismatch happens?

Looking  at this code it looks like you are using an old tree, Laurent
meanwhile have sent a patch that change this function:

a9512b261afd ("media: imx: capture: Simplify capture_validate_fmt() implementation")

can you test with the latest code as see if you have the same
problems.

------
Cheers,
     Rui
  

Hi Rui,

On Mon, Sep 13, 2021 at 7:12 PM Rui Miguel Silva <rui.silva@linaro.org> wrote:

> Looking  at this code it looks like you are using an old tree, Laurent
> meanwhile have sent a patch that change this function:
>
> a9512b261afd ("media: imx: capture: Simplify capture_validate_fmt() implementation")
>
> can you test with the latest code as see if you have the same
> problems.

I have just tested on 5.15-rc1 and this is what I get:

# v4l2-ctl --device /dev/v4l-subdev1 --set-standard PAL
Standard set to 000000ff
# media-ctl -l "'tw9910 2-0044':0 -> 'csi':0[1]"
# media-ctl -l "'csi':1 -> 'csi capture':0[1]"
# media-ctl -v -V "'tw9910 2-0044':0 [fmt:UYVY8_2X8/720x576
field:interlaced-bt]"
Opening media device /dev/media0
Enumerating entities
Found 3 entities
Enumerating pads and links
Setting up format UYVY8_2X8 720x576 on pad tw9910 2-0044/0
Format set: UYVY8_2X8 720x576
Setting up format UYVY8_2X8 720x576 on pad csi/0
Format set: UYVY8_2X8 720x576

# v4l2-ctl --stream-mmap -d /dev/video1
[   26.860654] priv->vdev.compose.width is 640
[   26.865419] fmt_src.format.width is 720
[   26.869390] priv->vdev.compose.height is 480
[   26.874268] compose.height is 576
[   26.877716] imx7-csi 2214000.csi: capture format not valid
VIDIOC_STREAMON returned -1 (Broken pipe)

Do you know why I am getting such a width/height mismatch?

Thanks,

Fabio Estevam
  

Oi Fabio,
On Thu Sep 16, 2021 at 9:52 PM WEST, Fabio Estevam wrote:

> Hi Rui,
>
> On Mon, Sep 13, 2021 at 7:12 PM Rui Miguel Silva <rui.silva@linaro.org> wrote:
>
> > Looking  at this code it looks like you are using an old tree, Laurent
> > meanwhile have sent a patch that change this function:
> >
> > a9512b261afd ("media: imx: capture: Simplify capture_validate_fmt() implementation")
> >
> > can you test with the latest code as see if you have the same
> > problems.
>
> I have just tested on 5.15-rc1 and this is what I get:
>
> # v4l2-ctl --device /dev/v4l-subdev1 --set-standard PAL
> Standard set to 000000ff
> # media-ctl -l "'tw9910 2-0044':0 -> 'csi':0[1]"
> # media-ctl -l "'csi':1 -> 'csi capture':0[1]"
> # media-ctl -v -V "'tw9910 2-0044':0 [fmt:UYVY8_2X8/720x576
> field:interlaced-bt]"
> Opening media device /dev/media0
> Enumerating entities
> Found 3 entities
> Enumerating pads and links
> Setting up format UYVY8_2X8 720x576 on pad tw9910 2-0044/0
> Format set: UYVY8_2X8 720x576
> Setting up format UYVY8_2X8 720x576 on pad csi/0
> Format set: UYVY8_2X8 720x576
>
> # v4l2-ctl --stream-mmap -d /dev/video1
> [   26.860654] priv->vdev.compose.width is 640

This looks like the default pix width (IMX_MEDIA_DEF_PIX_WIDTH)

> [   26.865419] fmt_src.format.width is 720
> [   26.869390] priv->vdev.compose.height is 480

and this the default pix height (IMX_MEDIA_DEF_PIX_HEIGHT)

> [   26.874268] compose.height is 576
> [   26.877716] imx7-csi 2214000.csi: capture format not valid
> VIDIOC_STREAMON returned -1 (Broken pipe)
> Do you know why I am getting such a width/height mismatch?

So, it seems like it is not configuring the pads, don't you need to do
a media-ctl -V in the csi:0?

or maybe dump the output of media-ctl -p and check the links and pads
configurations.

Hope this helps.

Cheers,
   Rui
  

Hi Rui,

On Thu, Sep 16, 2021 at 7:05 PM Rui Miguel Silva <rui.silva@linaro.org> wrote:

> So, it seems like it is not configuring the pads, don't you need to do
> a media-ctl -V in the csi:0?

Thanks for the suggestion. I tried passing media-ctl -V to csi:0:

# v4l2-ctl --device /dev/v4l-subdev1 --set-standard PAL
Standard set to 000000ff
# media-ctl -l "'tw9910 2-0044':0 -> 'csi':0[1]"
# media-ctl -l "'csi':1 -> 'csi capture':0[1]"
# media-ctl -v -V "'tw9910 2-0044':0 [fmt:UYVY8_2X8/720x576
field:interlaced-bt]"
# media-ctl -v -V "'csi':0 [fmt:UYVY8_2X8/720x576 field:interlaced-bt]"

Does this look correct?

Opening media device /dev/media0
Enumerating entities
Found 3 entities
Enumerating pads and links
Setting up format UYVY8_2X8 720x576 on pad tw9910 2-0044/0
Format set: UYVY8_2X8 720x576
Setting up format UYVY8_2X8 720x576 on pad csi/0
Format set: UYVY8_2X8 720x576
Opening media device /dev/media0
Enumerating entities
Found 3 entities
Enumerating pads and links
Setting up format UYVY8_2X8 720x576 on pad csi/0
Format set: UYVY8_2X8 720x576

# v4l2-ctl --stream-mmap -d /dev/video1
[   50.969910] priv->vdev.compose.width is 640
[   50.974685] fmt_src.format.width is 720
[   50.978659] priv->vdev.compose.height is 480
[   50.983208] compose.height is 576
[   50.986646] imx7-csi 2214000.csi: capture format not valid
VIDIOC_STREAMON returned -1 (Broken pipe)

but still getting the width/height mismatch.

# media-ctl -p
Media controller API version 5.15.0

Media device information
------------------------
driver          imx7-csi
model           imx-media
serial
bus info
hw revision     0x0
driver version  5.15.0

Device topology
- entity 1: csi (2 pads, 2 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev0
pad0: Sink
[fmt:UYVY8_2X8/720x576 field:none colorspace:srgb xfer:srgb ycbcr:601
quantization:lim-range]
<- "tw9910 2-0044":0 [ENABLED,IMMUTABLE]
pad1: Source
[fmt:UYVY8_2X8/720x576 field:none colorspace:srgb xfer:srgb ycbcr:601
quantization:lim-range]
-> "csi capture":0 [ENABLED,IMMUTABLE]

- entity 4: csi capture (1 pad, 1 link)
            type Node subtype V4L flags 0
            device node name /dev/video1
pad0: Sink
<- "csi":1 [ENABLED,IMMUTABLE]

- entity 10: tw9910 2-0044 (1 pad, 1 link)
             type V4L2 subdev subtype Decoder flags 0
             device node name /dev/v4l-subdev1
pad0: Source
[fmt:UYVY8_2X8/720x576 field:interlaced-bt colorspace:smpte170m
crop.bounds:(0,0)/768x576
crop:(0,0)/768x576]
-> "csi":0 [ENABLED,IMMUTABLE]

Thanks



> or maybe dump the output of media-ctl -p and check the links and pads
> configurations.
>
> Hope this helps.
>
> Cheers,
>    Rui