diff mbox series

[2/4] media: i2c: st-vgxy61: Add support for embedded data

Message ID 20240315085158.1213159-3-julien.massot@collabora.com (mailing list archive)
State New, archived
Headers show
Series media: i2c: st-vgxy61 Add support for embedded metadata | expand

Commit Message

Julien Massot March 15, 2024, 8:51 a.m. UTC
Add support for embedded data. This introduces two internal pads for pixel
and embedded data streams. The sensor can send ISL data at the begginning
of each frame.

The ISL data contains information related to the current frame such as:
ROI, cropping and orientation, gains, thermal sensors values,
frame counter..

The Intelligent Status Line follows the CCS embedded data format definition
regarding the tagged data but not for the registers address, therefore the
format code is MEDIA_BUS_FMT_META_8 and not MEDIA_BUS_FMT_CCS_EMBEDDED.

Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
 drivers/media/i2c/st-vgxy61.c | 172 +++++++++++++++++++++++++++++++---
 1 file changed, 160 insertions(+), 12 deletions(-)

Comments

kernel test robot March 15, 2024, 3:43 p.m. UTC | #1
Hi Julien,

kernel test robot noticed the following build errors:

[auto build test ERROR on media-tree/master]
[also build test ERROR on linuxtv-media-stage/master next-20240315]
[cannot apply to sailus-media-tree/streams linus/master v6.8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Julien-Massot/media-i2c-st-vgxy61-Use-sub-device-active-state/20240315-165346
base:   git://linuxtv.org/media_tree.git master
patch link:    https://lore.kernel.org/r/20240315085158.1213159-3-julien.massot%40collabora.com
patch subject: [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240315/202403152358.DM9q4q25-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 8f68022f8e6e54d1aeae4ed301f5a015963089b7)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240315/202403152358.DM9q4q25-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403152358.DM9q4q25-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/media/i2c/st-vgxy61.c:11:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/media/i2c/st-vgxy61.c:11:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/media/i2c/st-vgxy61.c:11:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   In file included from drivers/media/i2c/st-vgxy61.c:11:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:20:
   In file included from include/linux/mm.h:2188:
   include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     522 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/media/i2c/st-vgxy61.c:709:16: error: use of undeclared identifier 'MEDIA_BUS_FMT_META_8'
     709 |                 code->code = MEDIA_BUS_FMT_META_8;
         |                              ^
   drivers/media/i2c/st-vgxy61.c:1251:19: error: use of undeclared identifier 'MEDIA_BUS_FMT_META_8'
    1251 |         meta_fmt->code = MEDIA_BUS_FMT_META_8;
         |                          ^
   drivers/media/i2c/st-vgxy61.c:1309:29: error: use of undeclared identifier 'MEDIA_BUS_FMT_META_8'
    1309 |         desc->entry[1].pixelcode = MEDIA_BUS_FMT_META_8;
         |                                    ^
>> drivers/media/i2c/st-vgxy61.c:1326:6: error: use of undeclared identifier 'V4L2_SUBDEV_ROUTE_FL_IMMUTABLE'
    1326 |                                  V4L2_SUBDEV_ROUTE_FL_IMMUTABLE,
         |                                  ^
>> drivers/media/i2c/st-vgxy61.c:1336:17: error: invalid application of 'sizeof' to an incomplete type 'struct v4l2_subdev_route[]'
    1336 |                 .num_routes = ARRAY_SIZE(routes),
         |                               ^~~~~~~~~~~~~~~~~~
   include/linux/array_size.h:11:32: note: expanded from macro 'ARRAY_SIZE'
      11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                                ^~~~~
>> drivers/media/i2c/st-vgxy61.c:1891:23: error: use of undeclared identifier 'MEDIA_PAD_FL_INTERNAL'
    1891 |                 MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_INTERNAL;
         |                                     ^
   drivers/media/i2c/st-vgxy61.c:1893:23: error: use of undeclared identifier 'MEDIA_PAD_FL_INTERNAL'
    1893 |                 MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_INTERNAL;
         |                                     ^
   7 warnings and 7 errors generated.


vim +/MEDIA_BUS_FMT_META_8 +709 drivers/media/i2c/st-vgxy61.c

   698	
   699	static int vgxy61_enum_mbus_code(struct v4l2_subdev *sd,
   700					 struct v4l2_subdev_state *sd_state,
   701					 struct v4l2_subdev_mbus_code_enum *code)
   702	{
   703		if (code->pad == VGXY61_PAD_META ||
   704		    (code->pad == VGXY61_PAD_SOURCE &&
   705		     code->stream == VGXY61_STREAM_META)) {
   706			if (code->index > 0)
   707				return -EINVAL;
   708	
 > 709			code->code = MEDIA_BUS_FMT_META_8;
   710			return 0;
   711		}
   712	
   713		if (code->index >= ARRAY_SIZE(vgxy61_supported_codes))
   714			return -EINVAL;
   715	
   716		code->code = vgxy61_supported_codes[code->index].code;
   717	
   718		return 0;
   719	}
   720
kernel test robot March 15, 2024, 10:35 p.m. UTC | #2
Hi Julien,

kernel test robot noticed the following build errors:

[auto build test ERROR on media-tree/master]
[also build test ERROR on linuxtv-media-stage/master next-20240315]
[cannot apply to sailus-media-tree/streams linus/master v6.8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Julien-Massot/media-i2c-st-vgxy61-Use-sub-device-active-state/20240315-165346
base:   git://linuxtv.org/media_tree.git master
patch link:    https://lore.kernel.org/r/20240315085158.1213159-3-julien.massot%40collabora.com
patch subject: [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240316/202403160651.QB9YWHsu-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/202403160651.QB9YWHsu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403160651.QB9YWHsu-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/media/i2c/st-vgxy61.c: In function 'vgxy61_enum_mbus_code':
>> drivers/media/i2c/st-vgxy61.c:709:30: error: 'MEDIA_BUS_FMT_META_8' undeclared (first use in this function); did you mean 'MEDIA_BUS_FMT_Y8_1X8'?
     709 |                 code->code = MEDIA_BUS_FMT_META_8;
         |                              ^~~~~~~~~~~~~~~~~~~~
         |                              MEDIA_BUS_FMT_Y8_1X8
   drivers/media/i2c/st-vgxy61.c:709:30: note: each undeclared identifier is reported only once for each function it appears in
   drivers/media/i2c/st-vgxy61.c: In function '__vgxy61_set_fmt':
   drivers/media/i2c/st-vgxy61.c:1251:26: error: 'MEDIA_BUS_FMT_META_8' undeclared (first use in this function); did you mean 'MEDIA_BUS_FMT_Y8_1X8'?
    1251 |         meta_fmt->code = MEDIA_BUS_FMT_META_8;
         |                          ^~~~~~~~~~~~~~~~~~~~
         |                          MEDIA_BUS_FMT_Y8_1X8
   drivers/media/i2c/st-vgxy61.c: In function 'vgxy61_get_frame_desc':
   drivers/media/i2c/st-vgxy61.c:1309:36: error: 'MEDIA_BUS_FMT_META_8' undeclared (first use in this function); did you mean 'MEDIA_BUS_FMT_Y8_1X8'?
    1309 |         desc->entry[1].pixelcode = MEDIA_BUS_FMT_META_8;
         |                                    ^~~~~~~~~~~~~~~~~~~~
         |                                    MEDIA_BUS_FMT_Y8_1X8
   drivers/media/i2c/st-vgxy61.c: In function 'vgxy61_init_state':
>> drivers/media/i2c/st-vgxy61.c:1326:34: error: 'V4L2_SUBDEV_ROUTE_FL_IMMUTABLE' undeclared (first use in this function); did you mean 'V4L2_SUBDEV_ROUTE_FL_ACTIVE'?
    1326 |                                  V4L2_SUBDEV_ROUTE_FL_IMMUTABLE,
         |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                                  V4L2_SUBDEV_ROUTE_FL_ACTIVE
   drivers/media/i2c/st-vgxy61.c: In function 'vgxy61_probe':
>> drivers/media/i2c/st-vgxy61.c:1891:37: error: 'MEDIA_PAD_FL_INTERNAL' undeclared (first use in this function); did you mean 'MEDIA_PAD_FL_SOURCE'?
    1891 |                 MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_INTERNAL;
         |                                     ^~~~~~~~~~~~~~~~~~~~~
         |                                     MEDIA_PAD_FL_SOURCE


vim +709 drivers/media/i2c/st-vgxy61.c

   698	
   699	static int vgxy61_enum_mbus_code(struct v4l2_subdev *sd,
   700					 struct v4l2_subdev_state *sd_state,
   701					 struct v4l2_subdev_mbus_code_enum *code)
   702	{
   703		if (code->pad == VGXY61_PAD_META ||
   704		    (code->pad == VGXY61_PAD_SOURCE &&
   705		     code->stream == VGXY61_STREAM_META)) {
   706			if (code->index > 0)
   707				return -EINVAL;
   708	
 > 709			code->code = MEDIA_BUS_FMT_META_8;
   710			return 0;
   711		}
   712	
   713		if (code->index >= ARRAY_SIZE(vgxy61_supported_codes))
   714			return -EINVAL;
   715	
   716		code->code = vgxy61_supported_codes[code->index].code;
   717	
   718		return 0;
   719	}
   720
Benjamin Mugnier April 3, 2024, 12:26 p.m. UTC | #3
Hi Julien,

On 3/15/24 09:51, Julien Massot wrote:
> Add support for embedded data. This introduces two internal pads for pixel
> and embedded data streams. The sensor can send ISL data at the begginning

Typo 'beginning'.

> of each frame.
> 
> The ISL data contains information related to the current frame such as:
> ROI, cropping and orientation, gains, thermal sensors values,
> frame counter..
> 
> The Intelligent Status Line follows the CCS embedded data format definition
> regarding the tagged data but not for the registers address, therefore the
> format code is MEDIA_BUS_FMT_META_8 and not MEDIA_BUS_FMT_CCS_EMBEDDED.
> 
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> ---
>  drivers/media/i2c/st-vgxy61.c | 172 +++++++++++++++++++++++++++++++---
>  1 file changed, 160 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
> index 733713f909cf..e8302456a8d9 100644
> --- a/drivers/media/i2c/st-vgxy61.c
> +++ b/drivers/media/i2c/st-vgxy61.c
> @@ -88,11 +88,16 @@
>  #define VGXY61_REG_PATGEN_SHORT_DATA_B			CCI_REG16_LE(0x0954)
>  #define VGXY61_REG_PATGEN_SHORT_DATA_GB			CCI_REG16_LE(0x0956)
>  #define VGXY61_REG_BYPASS_CTRL				CCI_REG8(0x0a60)
> +#define VGXY61_ISL_BYPASS				BIT(3)
> +#define VGXY61_ASIL_BYPASS				BIT(2)
>  
>  #define VGX661_WIDTH					1464
>  #define VGX661_HEIGHT					1104
>  #define VGX761_WIDTH					1944
>  #define VGX761_HEIGHT					1204
> +/* two status lines (ISL), of 256 bytes each */
> +#define VGXY61_META_WIDTH				256
> +#define VGXY61_META_HEIGHT				2
>  #define VGX661_DEFAULT_MODE				1
>  #define VGX761_DEFAULT_MODE				1
>  #define VGX661_SHORT_ROT_TERM				93
> @@ -112,6 +117,18 @@
>  #define VGXY61_FWPATCH_REVISION_MINOR			0
>  #define VGXY61_FWPATCH_REVISION_MICRO			5
>  
> +enum {
> +	VGXY61_PAD_SOURCE,
> +	VGXY61_PAD_PIXEL,
> +	VGXY61_PAD_META,
> +	VGXY61_NUM_PADS,
> +};
> +
> +enum {
> +	VGXY61_STREAM_PIXEL,
> +	VGXY61_STREAM_META,
> +};
> +
>  static const u8 patch_array[] = {
>  	0xbf, 0x00, 0x05, 0x20, 0x06, 0x01, 0xe0, 0xe0, 0x04, 0x80, 0xe6, 0x45,
>  	0xed, 0x6f, 0xfe, 0xff, 0x14, 0x80, 0x1f, 0x84, 0x10, 0x42, 0x05, 0x7c,
> @@ -382,7 +399,7 @@ struct vgxy61_dev {
>  	struct i2c_client *i2c_client;
>  	struct regmap *regmap;
>  	struct v4l2_subdev sd;
> -	struct media_pad pad;
> +	struct media_pad pads[VGXY61_NUM_PADS];
>  	struct regulator_bulk_data supplies[ARRAY_SIZE(vgxy61_supply_name)];
>  	struct gpio_desc *reset_gpio;
>  	struct clk *xclk;
> @@ -655,6 +672,13 @@ static int vgxy61_get_selection(struct v4l2_subdev *sd,
>  {
>  	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
>  
> +	/*
> +	 * The embedded data stream doesn't support selection rectangles,
> +	 * neither on the embedded data pad nor on the source pad.
> +	 */
> +	if (sel->pad == VGXY61_PAD_META)
> +		return -EINVAL;
> +
>  	switch (sel->target) {
>  	case V4L2_SEL_TGT_CROP:
>  		sel->r = sensor->current_mode->crop;
> @@ -676,6 +700,16 @@ static int vgxy61_enum_mbus_code(struct v4l2_subdev *sd,
>  				 struct v4l2_subdev_state *sd_state,
>  				 struct v4l2_subdev_mbus_code_enum *code)
>  {
> +	if (code->pad == VGXY61_PAD_META ||
> +	    (code->pad == VGXY61_PAD_SOURCE &&
> +	     code->stream == VGXY61_STREAM_META)) {
> +		if (code->index > 0)
> +			return -EINVAL;
> +
> +		code->code = MEDIA_BUS_FMT_META_8;
> +		return 0;
> +	}
> +
>  	if (code->index >= ARRAY_SIZE(vgxy61_supported_codes))
>  		return -EINVAL;
>  
> @@ -703,6 +737,19 @@ static int vgxy61_enum_frame_size(struct v4l2_subdev *sd,
>  {
>  	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
>  
> +	if (fse->pad == VGXY61_PAD_META ||
> +	    (fse->pad == VGXY61_PAD_SOURCE &&
> +	     fse->stream == VGXY61_STREAM_META)) {
> +		if (fse->index > 0)
> +			return -EINVAL;
> +
> +		fse->min_width = VGXY61_META_WIDTH;
> +		fse->max_width = VGXY61_META_WIDTH;
> +		fse->min_height = VGXY61_META_HEIGHT;
> +		fse->max_height = VGXY61_META_HEIGHT;
> +		return 0;
> +	}
> +
>  	if (fse->index >= sensor->sensor_modes_nb)
>  		return -EINVAL;
>  
> @@ -1159,24 +1206,54 @@ static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable)
>  	return ret;
>  }
>  
> -static int vgxy61_set_fmt(struct v4l2_subdev *sd,
> -			  struct v4l2_subdev_state *sd_state,
> -			  struct v4l2_subdev_format *format)
> +static int __vgxy61_set_fmt(struct v4l2_subdev *sd,
> +			    struct v4l2_subdev_state *sd_state,
> +			    struct v4l2_mbus_framefmt *format,
> +			    enum v4l2_subdev_format_whence which,
> +			    unsigned int pad, unsigned int stream)
>  {
>  	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
> +	struct v4l2_mbus_framefmt *src_pix_fmt, *src_meta_fmt, *pix_fmt,
> +		*meta_fmt;
>  	const struct vgxy61_mode_info *new_mode;
>  	int ret;
>  
>  	if (sensor->streaming)
>  		return -EBUSY;
>  
> -	ret = vgxy61_try_fmt_internal(sd, &format->format, &new_mode);
> +	/*
> +	 * Allow setting format on internal pixel pad as well as the source
> +	 * pad's pixel stream (for compatibility).
> +	 */
> +	if ((pad == VGXY61_PAD_SOURCE && stream == VGXY61_STREAM_META) ||
> +	    pad == VGXY61_PAD_META) {
> +		*format = *v4l2_subdev_state_get_format(sd_state, pad, stream);
> +		return 0;
> +	}
> +
> +	pix_fmt = v4l2_subdev_state_get_format(sd_state, VGXY61_PAD_PIXEL, 0);
> +	meta_fmt = v4l2_subdev_state_get_format(sd_state, VGXY61_PAD_META, 0);
> +	src_pix_fmt = v4l2_subdev_state_get_format(sd_state, VGXY61_PAD_SOURCE,
> +						   VGXY61_STREAM_PIXEL);
> +	src_meta_fmt = v4l2_subdev_state_get_format(sd_state, VGXY61_PAD_SOURCE,
> +						    VGXY61_STREAM_META);
> +
> +	ret = vgxy61_try_fmt_internal(sd, format, &new_mode);
>  	if (ret)
>  		return ret;
>  
> -	*v4l2_subdev_state_get_format(sd_state, format->pad) = format->format;
> +	pix_fmt->width = format->width;
> +	pix_fmt->height = format->height;
> +	pix_fmt->code = format->code;
> +	pix_fmt->field = V4L2_FIELD_NONE;
> +
> +	*format = *src_pix_fmt = *pix_fmt;
> +	meta_fmt->code = MEDIA_BUS_FMT_META_8;
> +	meta_fmt->width = VGXY61_META_WIDTH;
> +	meta_fmt->height = VGXY61_META_HEIGHT;
> +	*src_meta_fmt = *meta_fmt;
>  
> -	if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> +	if (which == V4L2_SUBDEV_FORMAT_TRY)
>  		return 0;
>  
>  	sensor->current_mode = new_mode;
> @@ -1202,16 +1279,78 @@ static int vgxy61_set_fmt(struct v4l2_subdev *sd,
>  	return ret;
>  }
>  
> +static int vgxy61_set_fmt(struct v4l2_subdev *sd,
> +			  struct v4l2_subdev_state *sd_state,
> +			  struct v4l2_subdev_format *fmt)
> +{
> +	return __vgxy61_set_fmt(sd, sd_state, &fmt->format, fmt->which,
> +				fmt->pad, fmt->stream);
> +}
> +
> +static int vgxy61_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
> +				 struct v4l2_mbus_frame_desc *desc)
> +{
> +	struct v4l2_subdev_state *sd_state;
> +	struct v4l2_mbus_framefmt *fmt;
> +
> +	desc->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +	desc->num_entries = 2;
> +
> +	sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> +	fmt = v4l2_subdev_state_get_format(sd_state, VGXY61_PAD_SOURCE,
> +					   VGXY61_STREAM_PIXEL);
> +	v4l2_subdev_unlock_state(sd_state);
> +
> +	desc->entry[0].stream = VGXY61_STREAM_PIXEL;
> +	desc->entry[0].pixelcode = fmt->code;
> +	desc->entry[0].bus.csi2.dt = get_data_type_by_code(fmt->code);
> +
> +	desc->entry[1].stream = VGXY61_STREAM_META;
> +	desc->entry[1].pixelcode = MEDIA_BUS_FMT_META_8;
> +	desc->entry[1].bus.csi2.dt = MIPI_CSI2_DT_EMBEDDED_8B;
> +
> +	return 0;
> +}
> +
>  static int vgxy61_init_state(struct v4l2_subdev *sd,
>  			     struct v4l2_subdev_state *sd_state)
>  {
>  	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
>  	struct v4l2_subdev_format fmt = { 0 };
> +	struct v4l2_subdev_route routes[] = {
> +		{
> +			.sink_pad = VGXY61_PAD_PIXEL,
> +			.source_pad = VGXY61_PAD_SOURCE,
> +			.source_stream = VGXY61_STREAM_PIXEL,
> +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE |
> +				 V4L2_SUBDEV_ROUTE_FL_IMMUTABLE,
> +		}, {
> +			.sink_pad = VGXY61_PAD_META,
> +			.source_pad = VGXY61_PAD_SOURCE,
> +			.source_stream = VGXY61_STREAM_META,
> +			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> +		},
> +	};
> +	struct v4l2_subdev_krouting routing = {
> +		.routes = routes,
> +		.num_routes = ARRAY_SIZE(routes),
> +	};
> +	struct v4l2_subdev_state *active_state;
> +	int ret;
> +
> +	ret = v4l2_subdev_set_routing(sd, sd_state, &routing);
> +	if (ret)
> +		return ret;
> +
> +	active_state = v4l2_subdev_get_locked_active_state(sd);
>  
>  	vgxy61_fill_framefmt(sensor, sensor->current_mode, &fmt.format,
>  			     VGXY61_MEDIA_BUS_FMT_DEF);
>  
> -	return vgxy61_set_fmt(sd, sd_state, &fmt);
> +	return __vgxy61_set_fmt(sd, sd_state, &fmt.format,
> +				active_state == sd_state ?
> +				V4L2_SUBDEV_FORMAT_ACTIVE :
> +				V4L2_SUBDEV_FORMAT_TRY, VGXY61_PAD_PIXEL, 0);
>  }
>  
>  static int vgxy61_s_ctrl(struct v4l2_ctrl *ctrl)
> @@ -1364,6 +1503,7 @@ static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
>  	.enum_mbus_code = vgxy61_enum_mbus_code,
>  	.get_fmt = v4l2_subdev_get_fmt,
>  	.set_fmt = vgxy61_set_fmt,
> +	.get_frame_desc = vgxy61_get_frame_desc,
>  	.get_selection = vgxy61_get_selection,
>  	.enum_frame_size = vgxy61_enum_frame_size,
>  };
> @@ -1478,7 +1618,8 @@ static int vgxy61_configure(struct vgxy61_dev *sensor)
>  	cci_write(sensor->regmap, VGXY61_REG_CLK_SYS_PLL_MULT, mult, &ret);
>  	cci_write(sensor->regmap, VGXY61_REG_OIF_CTRL, sensor->oif_ctrl, &ret);
>  	cci_write(sensor->regmap, VGXY61_REG_FRAME_CONTENT_CTRL, 0, &ret);
> -	cci_write(sensor->regmap, VGXY61_REG_BYPASS_CTRL, 4, &ret);
> +	cci_write(sensor->regmap, VGXY61_REG_BYPASS_CTRL, VGXY61_ASIL_BYPASS,
> +		  &ret);
>  	if (ret)
>  		return ret;
>  	vgxy61_update_gpios_strobe_polarity(sensor, sensor->gpios_polarity);
> @@ -1743,8 +1884,14 @@ static int vgxy61_probe(struct i2c_client *client)
>  	v4l2_i2c_subdev_init(&sensor->sd, client, &vgxy61_subdev_ops);
>  	sensor->sd.internal_ops = &vgxy61_internal_ops;
>  	sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> -			    V4L2_SUBDEV_FL_HAS_EVENTS;
> -	sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
> +			    V4L2_SUBDEV_FL_HAS_EVENTS |
> +			    V4L2_SUBDEV_FL_STREAMS;
> +	sensor->pads[VGXY61_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
> +	sensor->pads[VGXY61_PAD_PIXEL].flags =
> +		MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_INTERNAL;
> +	sensor->pads[VGXY61_PAD_META].flags =
> +		MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_INTERNAL;
> +
>  	sensor->sd.entity.ops = &vgxy61_subdev_entity_ops;
>  	sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
>  
> @@ -1778,7 +1925,8 @@ static int vgxy61_probe(struct i2c_client *client)
>  		goto error_power_off;
>  	}
>  
> -	ret = media_entity_pads_init(&sensor->sd.entity, 1, &sensor->pad);
> +	ret = media_entity_pads_init(&sensor->sd.entity,
> +				     ARRAY_SIZE(sensor->pads), sensor->pads);
>  	if (ret) {
>  		dev_err(&client->dev, "pads init failed %d\n", ret);
>  		goto error_handler_free;
diff mbox series

Patch

diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
index 733713f909cf..e8302456a8d9 100644
--- a/drivers/media/i2c/st-vgxy61.c
+++ b/drivers/media/i2c/st-vgxy61.c
@@ -88,11 +88,16 @@ 
 #define VGXY61_REG_PATGEN_SHORT_DATA_B			CCI_REG16_LE(0x0954)
 #define VGXY61_REG_PATGEN_SHORT_DATA_GB			CCI_REG16_LE(0x0956)
 #define VGXY61_REG_BYPASS_CTRL				CCI_REG8(0x0a60)
+#define VGXY61_ISL_BYPASS				BIT(3)
+#define VGXY61_ASIL_BYPASS				BIT(2)
 
 #define VGX661_WIDTH					1464
 #define VGX661_HEIGHT					1104
 #define VGX761_WIDTH					1944
 #define VGX761_HEIGHT					1204
+/* two status lines (ISL), of 256 bytes each */
+#define VGXY61_META_WIDTH				256
+#define VGXY61_META_HEIGHT				2
 #define VGX661_DEFAULT_MODE				1
 #define VGX761_DEFAULT_MODE				1
 #define VGX661_SHORT_ROT_TERM				93
@@ -112,6 +117,18 @@ 
 #define VGXY61_FWPATCH_REVISION_MINOR			0
 #define VGXY61_FWPATCH_REVISION_MICRO			5
 
+enum {
+	VGXY61_PAD_SOURCE,
+	VGXY61_PAD_PIXEL,
+	VGXY61_PAD_META,
+	VGXY61_NUM_PADS,
+};
+
+enum {
+	VGXY61_STREAM_PIXEL,
+	VGXY61_STREAM_META,
+};
+
 static const u8 patch_array[] = {
 	0xbf, 0x00, 0x05, 0x20, 0x06, 0x01, 0xe0, 0xe0, 0x04, 0x80, 0xe6, 0x45,
 	0xed, 0x6f, 0xfe, 0xff, 0x14, 0x80, 0x1f, 0x84, 0x10, 0x42, 0x05, 0x7c,
@@ -382,7 +399,7 @@  struct vgxy61_dev {
 	struct i2c_client *i2c_client;
 	struct regmap *regmap;
 	struct v4l2_subdev sd;
-	struct media_pad pad;
+	struct media_pad pads[VGXY61_NUM_PADS];
 	struct regulator_bulk_data supplies[ARRAY_SIZE(vgxy61_supply_name)];
 	struct gpio_desc *reset_gpio;
 	struct clk *xclk;
@@ -655,6 +672,13 @@  static int vgxy61_get_selection(struct v4l2_subdev *sd,
 {
 	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
 
+	/*
+	 * The embedded data stream doesn't support selection rectangles,
+	 * neither on the embedded data pad nor on the source pad.
+	 */
+	if (sel->pad == VGXY61_PAD_META)
+		return -EINVAL;
+
 	switch (sel->target) {
 	case V4L2_SEL_TGT_CROP:
 		sel->r = sensor->current_mode->crop;
@@ -676,6 +700,16 @@  static int vgxy61_enum_mbus_code(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_state *sd_state,
 				 struct v4l2_subdev_mbus_code_enum *code)
 {
+	if (code->pad == VGXY61_PAD_META ||
+	    (code->pad == VGXY61_PAD_SOURCE &&
+	     code->stream == VGXY61_STREAM_META)) {
+		if (code->index > 0)
+			return -EINVAL;
+
+		code->code = MEDIA_BUS_FMT_META_8;
+		return 0;
+	}
+
 	if (code->index >= ARRAY_SIZE(vgxy61_supported_codes))
 		return -EINVAL;
 
@@ -703,6 +737,19 @@  static int vgxy61_enum_frame_size(struct v4l2_subdev *sd,
 {
 	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
 
+	if (fse->pad == VGXY61_PAD_META ||
+	    (fse->pad == VGXY61_PAD_SOURCE &&
+	     fse->stream == VGXY61_STREAM_META)) {
+		if (fse->index > 0)
+			return -EINVAL;
+
+		fse->min_width = VGXY61_META_WIDTH;
+		fse->max_width = VGXY61_META_WIDTH;
+		fse->min_height = VGXY61_META_HEIGHT;
+		fse->max_height = VGXY61_META_HEIGHT;
+		return 0;
+	}
+
 	if (fse->index >= sensor->sensor_modes_nb)
 		return -EINVAL;
 
@@ -1159,24 +1206,54 @@  static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
-static int vgxy61_set_fmt(struct v4l2_subdev *sd,
-			  struct v4l2_subdev_state *sd_state,
-			  struct v4l2_subdev_format *format)
+static int __vgxy61_set_fmt(struct v4l2_subdev *sd,
+			    struct v4l2_subdev_state *sd_state,
+			    struct v4l2_mbus_framefmt *format,
+			    enum v4l2_subdev_format_whence which,
+			    unsigned int pad, unsigned int stream)
 {
 	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
+	struct v4l2_mbus_framefmt *src_pix_fmt, *src_meta_fmt, *pix_fmt,
+		*meta_fmt;
 	const struct vgxy61_mode_info *new_mode;
 	int ret;
 
 	if (sensor->streaming)
 		return -EBUSY;
 
-	ret = vgxy61_try_fmt_internal(sd, &format->format, &new_mode);
+	/*
+	 * Allow setting format on internal pixel pad as well as the source
+	 * pad's pixel stream (for compatibility).
+	 */
+	if ((pad == VGXY61_PAD_SOURCE && stream == VGXY61_STREAM_META) ||
+	    pad == VGXY61_PAD_META) {
+		*format = *v4l2_subdev_state_get_format(sd_state, pad, stream);
+		return 0;
+	}
+
+	pix_fmt = v4l2_subdev_state_get_format(sd_state, VGXY61_PAD_PIXEL, 0);
+	meta_fmt = v4l2_subdev_state_get_format(sd_state, VGXY61_PAD_META, 0);
+	src_pix_fmt = v4l2_subdev_state_get_format(sd_state, VGXY61_PAD_SOURCE,
+						   VGXY61_STREAM_PIXEL);
+	src_meta_fmt = v4l2_subdev_state_get_format(sd_state, VGXY61_PAD_SOURCE,
+						    VGXY61_STREAM_META);
+
+	ret = vgxy61_try_fmt_internal(sd, format, &new_mode);
 	if (ret)
 		return ret;
 
-	*v4l2_subdev_state_get_format(sd_state, format->pad) = format->format;
+	pix_fmt->width = format->width;
+	pix_fmt->height = format->height;
+	pix_fmt->code = format->code;
+	pix_fmt->field = V4L2_FIELD_NONE;
+
+	*format = *src_pix_fmt = *pix_fmt;
+	meta_fmt->code = MEDIA_BUS_FMT_META_8;
+	meta_fmt->width = VGXY61_META_WIDTH;
+	meta_fmt->height = VGXY61_META_HEIGHT;
+	*src_meta_fmt = *meta_fmt;
 
-	if (format->which == V4L2_SUBDEV_FORMAT_TRY)
+	if (which == V4L2_SUBDEV_FORMAT_TRY)
 		return 0;
 
 	sensor->current_mode = new_mode;
@@ -1202,16 +1279,78 @@  static int vgxy61_set_fmt(struct v4l2_subdev *sd,
 	return ret;
 }
 
+static int vgxy61_set_fmt(struct v4l2_subdev *sd,
+			  struct v4l2_subdev_state *sd_state,
+			  struct v4l2_subdev_format *fmt)
+{
+	return __vgxy61_set_fmt(sd, sd_state, &fmt->format, fmt->which,
+				fmt->pad, fmt->stream);
+}
+
+static int vgxy61_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
+				 struct v4l2_mbus_frame_desc *desc)
+{
+	struct v4l2_subdev_state *sd_state;
+	struct v4l2_mbus_framefmt *fmt;
+
+	desc->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+	desc->num_entries = 2;
+
+	sd_state = v4l2_subdev_lock_and_get_active_state(sd);
+	fmt = v4l2_subdev_state_get_format(sd_state, VGXY61_PAD_SOURCE,
+					   VGXY61_STREAM_PIXEL);
+	v4l2_subdev_unlock_state(sd_state);
+
+	desc->entry[0].stream = VGXY61_STREAM_PIXEL;
+	desc->entry[0].pixelcode = fmt->code;
+	desc->entry[0].bus.csi2.dt = get_data_type_by_code(fmt->code);
+
+	desc->entry[1].stream = VGXY61_STREAM_META;
+	desc->entry[1].pixelcode = MEDIA_BUS_FMT_META_8;
+	desc->entry[1].bus.csi2.dt = MIPI_CSI2_DT_EMBEDDED_8B;
+
+	return 0;
+}
+
 static int vgxy61_init_state(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_state *sd_state)
 {
 	struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
 	struct v4l2_subdev_format fmt = { 0 };
+	struct v4l2_subdev_route routes[] = {
+		{
+			.sink_pad = VGXY61_PAD_PIXEL,
+			.source_pad = VGXY61_PAD_SOURCE,
+			.source_stream = VGXY61_STREAM_PIXEL,
+			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE |
+				 V4L2_SUBDEV_ROUTE_FL_IMMUTABLE,
+		}, {
+			.sink_pad = VGXY61_PAD_META,
+			.source_pad = VGXY61_PAD_SOURCE,
+			.source_stream = VGXY61_STREAM_META,
+			.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+		},
+	};
+	struct v4l2_subdev_krouting routing = {
+		.routes = routes,
+		.num_routes = ARRAY_SIZE(routes),
+	};
+	struct v4l2_subdev_state *active_state;
+	int ret;
+
+	ret = v4l2_subdev_set_routing(sd, sd_state, &routing);
+	if (ret)
+		return ret;
+
+	active_state = v4l2_subdev_get_locked_active_state(sd);
 
 	vgxy61_fill_framefmt(sensor, sensor->current_mode, &fmt.format,
 			     VGXY61_MEDIA_BUS_FMT_DEF);
 
-	return vgxy61_set_fmt(sd, sd_state, &fmt);
+	return __vgxy61_set_fmt(sd, sd_state, &fmt.format,
+				active_state == sd_state ?
+				V4L2_SUBDEV_FORMAT_ACTIVE :
+				V4L2_SUBDEV_FORMAT_TRY, VGXY61_PAD_PIXEL, 0);
 }
 
 static int vgxy61_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -1364,6 +1503,7 @@  static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
 	.enum_mbus_code = vgxy61_enum_mbus_code,
 	.get_fmt = v4l2_subdev_get_fmt,
 	.set_fmt = vgxy61_set_fmt,
+	.get_frame_desc = vgxy61_get_frame_desc,
 	.get_selection = vgxy61_get_selection,
 	.enum_frame_size = vgxy61_enum_frame_size,
 };
@@ -1478,7 +1618,8 @@  static int vgxy61_configure(struct vgxy61_dev *sensor)
 	cci_write(sensor->regmap, VGXY61_REG_CLK_SYS_PLL_MULT, mult, &ret);
 	cci_write(sensor->regmap, VGXY61_REG_OIF_CTRL, sensor->oif_ctrl, &ret);
 	cci_write(sensor->regmap, VGXY61_REG_FRAME_CONTENT_CTRL, 0, &ret);
-	cci_write(sensor->regmap, VGXY61_REG_BYPASS_CTRL, 4, &ret);
+	cci_write(sensor->regmap, VGXY61_REG_BYPASS_CTRL, VGXY61_ASIL_BYPASS,
+		  &ret);
 	if (ret)
 		return ret;
 	vgxy61_update_gpios_strobe_polarity(sensor, sensor->gpios_polarity);
@@ -1743,8 +1884,14 @@  static int vgxy61_probe(struct i2c_client *client)
 	v4l2_i2c_subdev_init(&sensor->sd, client, &vgxy61_subdev_ops);
 	sensor->sd.internal_ops = &vgxy61_internal_ops;
 	sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
-			    V4L2_SUBDEV_FL_HAS_EVENTS;
-	sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
+			    V4L2_SUBDEV_FL_HAS_EVENTS |
+			    V4L2_SUBDEV_FL_STREAMS;
+	sensor->pads[VGXY61_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE;
+	sensor->pads[VGXY61_PAD_PIXEL].flags =
+		MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_INTERNAL;
+	sensor->pads[VGXY61_PAD_META].flags =
+		MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_INTERNAL;
+
 	sensor->sd.entity.ops = &vgxy61_subdev_entity_ops;
 	sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
 
@@ -1778,7 +1925,8 @@  static int vgxy61_probe(struct i2c_client *client)
 		goto error_power_off;
 	}
 
-	ret = media_entity_pads_init(&sensor->sd.entity, 1, &sensor->pad);
+	ret = media_entity_pads_init(&sensor->sd.entity,
+				     ARRAY_SIZE(sensor->pads), sensor->pads);
 	if (ret) {
 		dev_err(&client->dev, "pads init failed %d\n", ret);
 		goto error_handler_free;