[v8,22/38] media: ccs: Support frame descriptors

Message ID	20240313072516.241106-23-sakari.ailus@linux.intel.com (mailing list archive)
State	New, archived
Headers	show Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.8]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A800E171BA for <linux-media@vger.kernel.org>; Wed, 13 Mar 2024 07:26:01 +0000 (UTC) From: Sakari Ailus <sakari.ailus@linux.intel.com> To: linux-media@vger.kernel.org Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>, tomi.valkeinen@ideasonboard.com, bingbu.cao@intel.com, hongju.wang@intel.com, hverkuil@xs4all.nl, Andrey Konovalov <andrey.konovalov@linaro.org>, Jacopo Mondi <jacopo.mondi@ideasonboard.com>, Dmitry Perchanov <dmitry.perchanov@intel.com>, "Ng, Khai Wen" <khai.wen.ng@intel.com>, Alain Volmat <alain.volmat@foss.st.com> Subject: [PATCH v8 22/38] media: ccs: Support frame descriptors Date: Wed, 13 Mar 2024 09:25:00 +0200 Message-Id: <20240313072516.241106-23-sakari.ailus@linux.intel.com> In-Reply-To: <20240313072516.241106-1-sakari.ailus@linux.intel.com> References: <20240313072516.241106-1-sakari.ailus@linux.intel.com> Precedence: bulk MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
Series	Generic line based metadata support, internal pads \| expand [v8,00/38] Generic line based metadata support, internal pads [v8,01/38] media: mc: Add INTERNAL pad flag [v8,02/38] media: Documentation: Add "stream" into glossary [v8,03/38] media: uapi: Add generic serial metadata mbus formats [v8,04/38] media: uapi: Document which mbus format fields are valid for metadata [v8,05/38] media: uapi: Add generic 8-bit metadata format definitions [v8,06/38] media: v4l: Support line-based metadata capture [v8,07/38] media: Documentation: Additional streams generally don't harm capture [v8,08/38] media: Documentation: Document embedded data guidelines for camera sensors [v8,09/38] media: Documentation: v4l: Document internal source pads [v8,10/38] media: Documentation: Document S_ROUTING behaviour [v8,11/38] media: v4l: subdev: Add a function to lock two sub-device states, use it [v8,12/38] media: v4l: subdev: Move G_ROUTING handling below S_ROUTING [v8,13/38] media: v4l: subdev: Copy argument back to user also for S_ROUTING [v8,14/38] media: v4l: subdev: Add len_routes field to struct v4l2_subdev_routing [v8,15/38] media: v4l: subdev: Return routes set using S_ROUTING [v8,16/38] media: v4l: subdev: Allow a larger number of routes than there's room for [v8,17/38] media: v4l: subdev: Add trivial set_routing support [v8,18/38] media: ccs: No need to set streaming to false in power off [v8,19/38] media: ccs: Use {enable,disable}_streams operations [v8,20/38] media: ccs: Track streaming state [v8,21/38] media: ccs: Move ccs_validate_csi_data_format up [v8,22/38] media: ccs: Support frame descriptors [v8,23/38] media: uapi: ccs: Add media bus code for MIPI CCS embedded data [v8,24/38] media: ccs: Add support for embedded data stream [v8,25/38] media: ccs: Remove ccs_get_crop_compose helper [v8,26/38] media: ccs: Rely on sub-device state locking [v8,27/38] media: ccs: Compute binning configuration from sub-device state [v8,28/38] media: ccs: Compute scaling configuration from sub-device state [v8,29/38] media: ccs: Remove which parameter from ccs_propagate [v8,30/38] media: Documentation: ccs: Document routing [v8,31/38] media: uapi: v4l: subdev: Enable streams API [v8,32/38] media: uapi: Add media bus code for ov2740 embedded data [v8,33/38] media: ov2740: Switch to {enable,disable}_streams [v8,34/38] media: ov2740: Track streaming state [v8,35/38] media: ov2740: Add support for embedded data [v8,36/38] media: v4l: Add V4L2_SUBDEV_ROUTE_FL_IMMUTABLE sub-device routing flag [v8,37/38] media: ccs: Add IMMUTABLE route flag [v8,38/38] media: ov2740: Add IMMUTABLE route flag

Message ID

20240313072516.241106-23-sakari.ailus@linux.intel.com (mailing list archive)

State

New, archived

Headers

From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: linux-media@vger.kernel.org
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	tomi.valkeinen@ideasonboard.com,
	bingbu.cao@intel.com,
	hongju.wang@intel.com,
	hverkuil@xs4all.nl,
	Andrey Konovalov <andrey.konovalov@linaro.org>,
	Jacopo Mondi <jacopo.mondi@ideasonboard.com>,
	Dmitry Perchanov <dmitry.perchanov@intel.com>,
	"Ng, Khai Wen" <khai.wen.ng@intel.com>,
	Alain Volmat <alain.volmat@foss.st.com>
Subject: [PATCH v8 22/38] media: ccs: Support frame descriptors
Date: Wed, 13 Mar 2024 09:25:00 +0200
Message-Id: <20240313072516.241106-23-sakari.ailus@linux.intel.com>
In-Reply-To: <20240313072516.241106-1-sakari.ailus@linux.intel.com>
References: <20240313072516.241106-1-sakari.ailus@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Series

Generic line based metadata support, internal pads | expand

Commit Message

Sakari Ailus March 13, 2024, 7:25 a.m. UTC

Provide information on the frame layout using frame descriptors.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c  | 60 +++++++++++++++++++++++++++++++
 drivers/media/i2c/ccs/ccs-quirk.h |  7 ++++
 drivers/media/i2c/ccs/ccs.h       |  4 +++
 3 files changed, 71 insertions(+)

Comments

Julien Massot March 15, 2024, 4:02 p.m. UTC | #1

On 3/13/24 08:25, Sakari Ailus wrote:
> Provide information on the frame layout using frame descriptors.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Julien Massot <julien.massot@collabora.com>
> ---
>   drivers/media/i2c/ccs/ccs-core.c  | 60 +++++++++++++++++++++++++++++++
>   drivers/media/i2c/ccs/ccs-quirk.h |  7 ++++
>   drivers/media/i2c/ccs/ccs.h       |  4 +++
>   3 files changed, 71 insertions(+)
> 
> diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
> index 0efbc63534bc..9cc2080b73ec 100644
> --- a/drivers/media/i2c/ccs/ccs-core.c
> +++ b/drivers/media/i2c/ccs/ccs-core.c
> @@ -25,6 +25,7 @@
>   #include <linux/slab.h>
>   #include <linux/smiapp.h>
>   #include <linux/v4l2-mediabus.h>
> +#include <media/mipi-csi2.h>
>   #include <media/v4l2-cci.h>
>   #include <media/v4l2-device.h>
>   #include <media/v4l2-fwnode.h>
> @@ -245,6 +246,33 @@ static int ccs_read_all_limits(struct ccs_sensor *sensor)
>   	return ret;
>   }
>   
> +static u8 ccs_mipi_csi2_data_type(unsigned int bpp)
> +{
> +	switch (bpp) {
> +	case 6:
> +		return MIPI_CSI2_DT_RAW6;
> +	case 7:
> +		return MIPI_CSI2_DT_RAW7;
> +	case 8:
> +		return MIPI_CSI2_DT_RAW8;
> +	case 10:
> +		return MIPI_CSI2_DT_RAW10;
> +	case 12:
> +		return MIPI_CSI2_DT_RAW12;
> +	case 14:
> +		return MIPI_CSI2_DT_RAW14;
> +	case 16:
> +		return MIPI_CSI2_DT_RAW16;
> +	case 20:
> +		return MIPI_CSI2_DT_RAW20;
> +	case 24:
> +		return MIPI_CSI2_DT_RAW24;
> +	default:
> +		WARN_ON(1);
> +		return 0;
> +	}
> +}
> +
>   static int ccs_read_frame_fmt(struct ccs_sensor *sensor)
>   {
>   	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
> @@ -2632,6 +2660,37 @@ static int ccs_set_selection(struct v4l2_subdev *subdev,
>   	return ret;
>   }
>   
> +static int ccs_get_frame_desc(struct v4l2_subdev *subdev, unsigned int pad,
> +				 struct v4l2_mbus_frame_desc *desc)
> +{
> +	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
> +	struct v4l2_mbus_frame_desc_entry *entry = desc->entry;
> +
> +	if (ccs_has_quirk(sensor, frame_desc))
> +		return ccs_call_quirk(sensor, frame_desc, desc);
> +
> +	switch (sensor->hwcfg.csi_signalling_mode) {
> +	case CCS_CSI_SIGNALING_MODE_CSI_2_DPHY:
> +	case CCS_CSI_SIGNALING_MODE_CSI_2_CPHY:
> +		desc->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +		break;
> +	default:
> +		/* FIXME: CCP2 support */
> +		return -EINVAL;
> +	}
> +
> +	entry->pixelcode = sensor->csi_format->code;
> +	entry->stream = CCS_STREAM_PIXEL;
> +	entry->bus.csi2.dt =
> +		sensor->csi_format->width == sensor->csi_format->compressed ?
> +		ccs_mipi_csi2_data_type(sensor->csi_format->compressed) :
> +		CCS_DEFAULT_COMPRESSED_DT;
> +	entry++;
> +	desc->num_entries++;
> +
> +	return 0;
> +}
> +
>   static int ccs_get_skip_frames(struct v4l2_subdev *subdev, u32 *frames)
>   {
>   	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
> @@ -3054,6 +3113,7 @@ static const struct v4l2_subdev_pad_ops ccs_pad_ops = {
>   	.set_selection = ccs_set_selection,
>   	.enable_streams = ccs_enable_streams,
>   	.disable_streams = ccs_disable_streams,
> +	.get_frame_desc = ccs_get_frame_desc,
>   };
>   
>   static const struct v4l2_subdev_sensor_ops ccs_sensor_ops = {
> diff --git a/drivers/media/i2c/ccs/ccs-quirk.h b/drivers/media/i2c/ccs/ccs-quirk.h
> index 392c97109617..3e1d9eaa33fa 100644
> --- a/drivers/media/i2c/ccs/ccs-quirk.h
> +++ b/drivers/media/i2c/ccs/ccs-quirk.h
> @@ -36,6 +36,7 @@ struct ccs_sensor;
>    *			 access may be done by the caller (default read
>    *			 value is zero), else negative error code on error
>    * @flags: Quirk flags
> + * @frame_desc: Obtain the frame descriptor
>    */
>   struct ccs_quirk {
>   	int (*limits)(struct ccs_sensor *sensor);
> @@ -46,6 +47,8 @@ struct ccs_quirk {
>   	int (*init)(struct ccs_sensor *sensor);
>   	int (*reg_access)(struct ccs_sensor *sensor, bool write, u32 *reg,
>   			  u32 *val);
> +	int (*frame_desc)(struct ccs_sensor *sensor,
> +			  struct v4l2_mbus_frame_desc *desc);
>   	unsigned long flags;
>   };
>   
> @@ -62,6 +65,10 @@ struct ccs_reg_8 {
>   		.val = _val,		\
>   	}
>   
> +#define ccs_has_quirk(sensor, _quirk)					\
> +	((sensor)->minfo.quirk &&					\
> +	 (sensor)->minfo.quirk->_quirk)
> +
>   #define ccs_call_quirk(sensor, _quirk, ...)				\
>   	((sensor)->minfo.quirk &&					\
>   	 (sensor)->minfo.quirk->_quirk ?				\
> diff --git a/drivers/media/i2c/ccs/ccs.h b/drivers/media/i2c/ccs/ccs.h
> index 4725e6eca8d0..adb152366ea2 100644
> --- a/drivers/media/i2c/ccs/ccs.h
> +++ b/drivers/media/i2c/ccs/ccs.h
> @@ -46,6 +46,8 @@
>   
>   #define CCS_COLOUR_COMPONENTS		4
>   
> +#define CCS_DEFAULT_COMPRESSED_DT	0x30
> +
>   #define SMIAPP_NAME			"smiapp"
>   #define CCS_NAME			"ccs"
>   
> @@ -175,6 +177,8 @@ struct ccs_csi_data_format {
>   #define CCS_PAD_SRC			1
>   #define CCS_PADS			2
>   
> +#define CCS_STREAM_PIXEL		0
> +
>   struct ccs_binning_subtype {
>   	u8 horizontal:4;
>   	u8 vertical:4;

Laurent Pinchart March 21, 2024, 4:44 p.m. UTC | #2

Hi Sakari,

Thank you for the patch.

On Wed, Mar 13, 2024 at 09:25:00AM +0200, Sakari Ailus wrote:
> Provide information on the frame layout using frame descriptors.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/ccs/ccs-core.c  | 60 +++++++++++++++++++++++++++++++
>  drivers/media/i2c/ccs/ccs-quirk.h |  7 ++++
>  drivers/media/i2c/ccs/ccs.h       |  4 +++
>  3 files changed, 71 insertions(+)
> 
> diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
> index 0efbc63534bc..9cc2080b73ec 100644
> --- a/drivers/media/i2c/ccs/ccs-core.c
> +++ b/drivers/media/i2c/ccs/ccs-core.c
> @@ -25,6 +25,7 @@
>  #include <linux/slab.h>
>  #include <linux/smiapp.h>
>  #include <linux/v4l2-mediabus.h>
> +#include <media/mipi-csi2.h>
>  #include <media/v4l2-cci.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-fwnode.h>
> @@ -245,6 +246,33 @@ static int ccs_read_all_limits(struct ccs_sensor *sensor)
>  	return ret;
>  }
>  
> +static u8 ccs_mipi_csi2_data_type(unsigned int bpp)
> +{
> +	switch (bpp) {
> +	case 6:
> +		return MIPI_CSI2_DT_RAW6;
> +	case 7:
> +		return MIPI_CSI2_DT_RAW7;
> +	case 8:
> +		return MIPI_CSI2_DT_RAW8;
> +	case 10:
> +		return MIPI_CSI2_DT_RAW10;
> +	case 12:
> +		return MIPI_CSI2_DT_RAW12;
> +	case 14:
> +		return MIPI_CSI2_DT_RAW14;
> +	case 16:
> +		return MIPI_CSI2_DT_RAW16;
> +	case 20:
> +		return MIPI_CSI2_DT_RAW20;
> +	case 24:
> +		return MIPI_CSI2_DT_RAW24;
> +	default:
> +		WARN_ON(1);
> +		return 0;
> +	}
> +}
> +
>  static int ccs_read_frame_fmt(struct ccs_sensor *sensor)
>  {
>  	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
> @@ -2632,6 +2660,37 @@ static int ccs_set_selection(struct v4l2_subdev *subdev,
>  	return ret;
>  }
>  
> +static int ccs_get_frame_desc(struct v4l2_subdev *subdev, unsigned int pad,
> +				 struct v4l2_mbus_frame_desc *desc)
> +{
> +	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
> +	struct v4l2_mbus_frame_desc_entry *entry = desc->entry;
> +
> +	if (ccs_has_quirk(sensor, frame_desc))
> +		return ccs_call_quirk(sensor, frame_desc, desc);

I would introduce the quirk later, along with the patch that will use
it.

> +
> +	switch (sensor->hwcfg.csi_signalling_mode) {
> +	case CCS_CSI_SIGNALING_MODE_CSI_2_DPHY:
> +	case CCS_CSI_SIGNALING_MODE_CSI_2_CPHY:
> +		desc->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> +		break;
> +	default:
> +		/* FIXME: CCP2 support */
> +		return -EINVAL;
> +	}
> +
> +	entry->pixelcode = sensor->csi_format->code;
> +	entry->stream = CCS_STREAM_PIXEL;
> +	entry->bus.csi2.dt =
> +		sensor->csi_format->width == sensor->csi_format->compressed ?
> +		ccs_mipi_csi2_data_type(sensor->csi_format->compressed) :

Functionally equivalent,

		ccs_mipi_csi2_data_type(sensor->csi_format->width) :

would be clearer I think. The way it's written today made me wonder why
you want the DT for the compressed format, which is not what you're
doing.

> +		CCS_DEFAULT_COMPRESSED_DT;
> +	entry++;
> +	desc->num_entries++;
> +
> +	return 0;
> +}
> +
>  static int ccs_get_skip_frames(struct v4l2_subdev *subdev, u32 *frames)
>  {
>  	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
> @@ -3054,6 +3113,7 @@ static const struct v4l2_subdev_pad_ops ccs_pad_ops = {
>  	.set_selection = ccs_set_selection,
>  	.enable_streams = ccs_enable_streams,
>  	.disable_streams = ccs_disable_streams,
> +	.get_frame_desc = ccs_get_frame_desc,
>  };
>  
>  static const struct v4l2_subdev_sensor_ops ccs_sensor_ops = {
> diff --git a/drivers/media/i2c/ccs/ccs-quirk.h b/drivers/media/i2c/ccs/ccs-quirk.h
> index 392c97109617..3e1d9eaa33fa 100644
> --- a/drivers/media/i2c/ccs/ccs-quirk.h
> +++ b/drivers/media/i2c/ccs/ccs-quirk.h
> @@ -36,6 +36,7 @@ struct ccs_sensor;
>   *			 access may be done by the caller (default read
>   *			 value is zero), else negative error code on error
>   * @flags: Quirk flags
> + * @frame_desc: Obtain the frame descriptor
>   */
>  struct ccs_quirk {
>  	int (*limits)(struct ccs_sensor *sensor);
> @@ -46,6 +47,8 @@ struct ccs_quirk {
>  	int (*init)(struct ccs_sensor *sensor);
>  	int (*reg_access)(struct ccs_sensor *sensor, bool write, u32 *reg,
>  			  u32 *val);
> +	int (*frame_desc)(struct ccs_sensor *sensor,
> +			  struct v4l2_mbus_frame_desc *desc);
>  	unsigned long flags;
>  };
>  
> @@ -62,6 +65,10 @@ struct ccs_reg_8 {
>  		.val = _val,		\
>  	}
>  
> +#define ccs_has_quirk(sensor, _quirk)					\
> +	((sensor)->minfo.quirk &&					\
> +	 (sensor)->minfo.quirk->_quirk)
> +
>  #define ccs_call_quirk(sensor, _quirk, ...)				\
>  	((sensor)->minfo.quirk &&					\
>  	 (sensor)->minfo.quirk->_quirk ?				\
> diff --git a/drivers/media/i2c/ccs/ccs.h b/drivers/media/i2c/ccs/ccs.h
> index 4725e6eca8d0..adb152366ea2 100644
> --- a/drivers/media/i2c/ccs/ccs.h
> +++ b/drivers/media/i2c/ccs/ccs.h
> @@ -46,6 +46,8 @@
>  
>  #define CCS_COLOUR_COMPONENTS		4
>  
> +#define CCS_DEFAULT_COMPRESSED_DT	0x30

I'd write

#define CCS_DEFAULT_COMPRESSED_DT	MIPI_CSI2_DT_USER_DEFINED(0)

> +
>  #define SMIAPP_NAME			"smiapp"
>  #define CCS_NAME			"ccs"
>  
> @@ -175,6 +177,8 @@ struct ccs_csi_data_format {
>  #define CCS_PAD_SRC			1
>  #define CCS_PADS			2
>  
> +#define CCS_STREAM_PIXEL		0
> +
>  struct ccs_binning_subtype {
>  	u8 horizontal:4;
>  	u8 vertical:4;

Sakari Ailus April 11, 2024, 8:33 a.m. UTC | #3

Hi Laurent,

On Thu, Mar 21, 2024 at 06:44:48PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.

Thanks for the review!

> 
> On Wed, Mar 13, 2024 at 09:25:00AM +0200, Sakari Ailus wrote:
> > Provide information on the frame layout using frame descriptors.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/i2c/ccs/ccs-core.c  | 60 +++++++++++++++++++++++++++++++
> >  drivers/media/i2c/ccs/ccs-quirk.h |  7 ++++
> >  drivers/media/i2c/ccs/ccs.h       |  4 +++
> >  3 files changed, 71 insertions(+)
> > 
> > diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
> > index 0efbc63534bc..9cc2080b73ec 100644
> > --- a/drivers/media/i2c/ccs/ccs-core.c
> > +++ b/drivers/media/i2c/ccs/ccs-core.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/smiapp.h>
> >  #include <linux/v4l2-mediabus.h>
> > +#include <media/mipi-csi2.h>
> >  #include <media/v4l2-cci.h>
> >  #include <media/v4l2-device.h>
> >  #include <media/v4l2-fwnode.h>
> > @@ -245,6 +246,33 @@ static int ccs_read_all_limits(struct ccs_sensor *sensor)
> >  	return ret;
> >  }
> >  
> > +static u8 ccs_mipi_csi2_data_type(unsigned int bpp)
> > +{
> > +	switch (bpp) {
> > +	case 6:
> > +		return MIPI_CSI2_DT_RAW6;
> > +	case 7:
> > +		return MIPI_CSI2_DT_RAW7;
> > +	case 8:
> > +		return MIPI_CSI2_DT_RAW8;
> > +	case 10:
> > +		return MIPI_CSI2_DT_RAW10;
> > +	case 12:
> > +		return MIPI_CSI2_DT_RAW12;
> > +	case 14:
> > +		return MIPI_CSI2_DT_RAW14;
> > +	case 16:
> > +		return MIPI_CSI2_DT_RAW16;
> > +	case 20:
> > +		return MIPI_CSI2_DT_RAW20;
> > +	case 24:
> > +		return MIPI_CSI2_DT_RAW24;
> > +	default:
> > +		WARN_ON(1);
> > +		return 0;
> > +	}
> > +}
> > +
> >  static int ccs_read_frame_fmt(struct ccs_sensor *sensor)
> >  {
> >  	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
> > @@ -2632,6 +2660,37 @@ static int ccs_set_selection(struct v4l2_subdev *subdev,
> >  	return ret;
> >  }
> >  
> > +static int ccs_get_frame_desc(struct v4l2_subdev *subdev, unsigned int pad,
> > +				 struct v4l2_mbus_frame_desc *desc)
> > +{
> > +	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
> > +	struct v4l2_mbus_frame_desc_entry *entry = desc->entry;
> > +
> > +	if (ccs_has_quirk(sensor, frame_desc))
> > +		return ccs_call_quirk(sensor, frame_desc, desc);
> 
> I would introduce the quirk later, along with the patch that will use
> it.

There is no user for this quirk in the series. It's been added for quirk
infrastructure completeness, I guess I can move it in a separate patch and
postpone that.

> 
> > +
> > +	switch (sensor->hwcfg.csi_signalling_mode) {
> > +	case CCS_CSI_SIGNALING_MODE_CSI_2_DPHY:
> > +	case CCS_CSI_SIGNALING_MODE_CSI_2_CPHY:
> > +		desc->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
> > +		break;
> > +	default:
> > +		/* FIXME: CCP2 support */
> > +		return -EINVAL;
> > +	}
> > +
> > +	entry->pixelcode = sensor->csi_format->code;
> > +	entry->stream = CCS_STREAM_PIXEL;
> > +	entry->bus.csi2.dt =
> > +		sensor->csi_format->width == sensor->csi_format->compressed ?
> > +		ccs_mipi_csi2_data_type(sensor->csi_format->compressed) :
> 
> Functionally equivalent,
> 
> 		ccs_mipi_csi2_data_type(sensor->csi_format->width) :
> 
> would be clearer I think. The way it's written today made me wonder why
> you want the DT for the compressed format, which is not what you're
> doing.

Sounds good.

> 
> > +		CCS_DEFAULT_COMPRESSED_DT;
> > +	entry++;
> > +	desc->num_entries++;
> > +
> > +	return 0;
> > +}
> > +
> >  static int ccs_get_skip_frames(struct v4l2_subdev *subdev, u32 *frames)
> >  {
> >  	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
> > @@ -3054,6 +3113,7 @@ static const struct v4l2_subdev_pad_ops ccs_pad_ops = {
> >  	.set_selection = ccs_set_selection,
> >  	.enable_streams = ccs_enable_streams,
> >  	.disable_streams = ccs_disable_streams,
> > +	.get_frame_desc = ccs_get_frame_desc,
> >  };
> >  
> >  static const struct v4l2_subdev_sensor_ops ccs_sensor_ops = {
> > diff --git a/drivers/media/i2c/ccs/ccs-quirk.h b/drivers/media/i2c/ccs/ccs-quirk.h
> > index 392c97109617..3e1d9eaa33fa 100644
> > --- a/drivers/media/i2c/ccs/ccs-quirk.h
> > +++ b/drivers/media/i2c/ccs/ccs-quirk.h
> > @@ -36,6 +36,7 @@ struct ccs_sensor;
> >   *			 access may be done by the caller (default read
> >   *			 value is zero), else negative error code on error
> >   * @flags: Quirk flags
> > + * @frame_desc: Obtain the frame descriptor
> >   */
> >  struct ccs_quirk {
> >  	int (*limits)(struct ccs_sensor *sensor);
> > @@ -46,6 +47,8 @@ struct ccs_quirk {
> >  	int (*init)(struct ccs_sensor *sensor);
> >  	int (*reg_access)(struct ccs_sensor *sensor, bool write, u32 *reg,
> >  			  u32 *val);
> > +	int (*frame_desc)(struct ccs_sensor *sensor,
> > +			  struct v4l2_mbus_frame_desc *desc);
> >  	unsigned long flags;
> >  };
> >  
> > @@ -62,6 +65,10 @@ struct ccs_reg_8 {
> >  		.val = _val,		\
> >  	}
> >  
> > +#define ccs_has_quirk(sensor, _quirk)					\
> > +	((sensor)->minfo.quirk &&					\
> > +	 (sensor)->minfo.quirk->_quirk)
> > +
> >  #define ccs_call_quirk(sensor, _quirk, ...)				\
> >  	((sensor)->minfo.quirk &&					\
> >  	 (sensor)->minfo.quirk->_quirk ?				\
> > diff --git a/drivers/media/i2c/ccs/ccs.h b/drivers/media/i2c/ccs/ccs.h
> > index 4725e6eca8d0..adb152366ea2 100644
> > --- a/drivers/media/i2c/ccs/ccs.h
> > +++ b/drivers/media/i2c/ccs/ccs.h
> > @@ -46,6 +46,8 @@
> >  
> >  #define CCS_COLOUR_COMPONENTS		4
> >  
> > +#define CCS_DEFAULT_COMPRESSED_DT	0x30
> 
> I'd write
> 
> #define CCS_DEFAULT_COMPRESSED_DT	MIPI_CSI2_DT_USER_DEFINED(0)

Yes.

> 
> > +
> >  #define SMIAPP_NAME			"smiapp"
> >  #define CCS_NAME			"ccs"
> >  
> > @@ -175,6 +177,8 @@ struct ccs_csi_data_format {
> >  #define CCS_PAD_SRC			1
> >  #define CCS_PADS			2
> >  
> > +#define CCS_STREAM_PIXEL		0
> > +
> >  struct ccs_binning_subtype {
> >  	u8 horizontal:4;
> >  	u8 vertical:4;
>

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 0efbc63534bc..9cc2080b73ec 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -25,6 +25,7 @@ 
 #include <linux/slab.h>
 #include <linux/smiapp.h>
 #include <linux/v4l2-mediabus.h>
+#include <media/mipi-csi2.h>
 #include <media/v4l2-cci.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-fwnode.h>
@@ -245,6 +246,33 @@  static int ccs_read_all_limits(struct ccs_sensor *sensor)
 	return ret;
 }
 
+static u8 ccs_mipi_csi2_data_type(unsigned int bpp)
+{
+	switch (bpp) {
+	case 6:
+		return MIPI_CSI2_DT_RAW6;
+	case 7:
+		return MIPI_CSI2_DT_RAW7;
+	case 8:
+		return MIPI_CSI2_DT_RAW8;
+	case 10:
+		return MIPI_CSI2_DT_RAW10;
+	case 12:
+		return MIPI_CSI2_DT_RAW12;
+	case 14:
+		return MIPI_CSI2_DT_RAW14;
+	case 16:
+		return MIPI_CSI2_DT_RAW16;
+	case 20:
+		return MIPI_CSI2_DT_RAW20;
+	case 24:
+		return MIPI_CSI2_DT_RAW24;
+	default:
+		WARN_ON(1);
+		return 0;
+	}
+}
+
 static int ccs_read_frame_fmt(struct ccs_sensor *sensor)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
@@ -2632,6 +2660,37 @@  static int ccs_set_selection(struct v4l2_subdev *subdev,
 	return ret;
 }
 
+static int ccs_get_frame_desc(struct v4l2_subdev *subdev, unsigned int pad,
+				 struct v4l2_mbus_frame_desc *desc)
+{
+	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
+	struct v4l2_mbus_frame_desc_entry *entry = desc->entry;
+
+	if (ccs_has_quirk(sensor, frame_desc))
+		return ccs_call_quirk(sensor, frame_desc, desc);
+
+	switch (sensor->hwcfg.csi_signalling_mode) {
+	case CCS_CSI_SIGNALING_MODE_CSI_2_DPHY:
+	case CCS_CSI_SIGNALING_MODE_CSI_2_CPHY:
+		desc->type = V4L2_MBUS_FRAME_DESC_TYPE_CSI2;
+		break;
+	default:
+		/* FIXME: CCP2 support */
+		return -EINVAL;
+	}
+
+	entry->pixelcode = sensor->csi_format->code;
+	entry->stream = CCS_STREAM_PIXEL;
+	entry->bus.csi2.dt =
+		sensor->csi_format->width == sensor->csi_format->compressed ?
+		ccs_mipi_csi2_data_type(sensor->csi_format->compressed) :
+		CCS_DEFAULT_COMPRESSED_DT;
+	entry++;
+	desc->num_entries++;
+
+	return 0;
+}
+
 static int ccs_get_skip_frames(struct v4l2_subdev *subdev, u32 *frames)
 {
 	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
@@ -3054,6 +3113,7 @@  static const struct v4l2_subdev_pad_ops ccs_pad_ops = {
 	.set_selection = ccs_set_selection,
 	.enable_streams = ccs_enable_streams,
 	.disable_streams = ccs_disable_streams,
+	.get_frame_desc = ccs_get_frame_desc,
 };
 
 static const struct v4l2_subdev_sensor_ops ccs_sensor_ops = {
diff --git a/drivers/media/i2c/ccs/ccs-quirk.h b/drivers/media/i2c/ccs/ccs-quirk.h
index 392c97109617..3e1d9eaa33fa 100644
--- a/drivers/media/i2c/ccs/ccs-quirk.h
+++ b/drivers/media/i2c/ccs/ccs-quirk.h
@@ -36,6 +36,7 @@  struct ccs_sensor;
  *			 access may be done by the caller (default read
  *			 value is zero), else negative error code on error
  * @flags: Quirk flags
+ * @frame_desc: Obtain the frame descriptor
  */
 struct ccs_quirk {
 	int (*limits)(struct ccs_sensor *sensor);
@@ -46,6 +47,8 @@  struct ccs_quirk {
 	int (*init)(struct ccs_sensor *sensor);
 	int (*reg_access)(struct ccs_sensor *sensor, bool write, u32 *reg,
 			  u32 *val);
+	int (*frame_desc)(struct ccs_sensor *sensor,
+			  struct v4l2_mbus_frame_desc *desc);
 	unsigned long flags;
 };
 
@@ -62,6 +65,10 @@  struct ccs_reg_8 {
 		.val = _val,		\
 	}
 
+#define ccs_has_quirk(sensor, _quirk)					\
+	((sensor)->minfo.quirk &&					\
+	 (sensor)->minfo.quirk->_quirk)
+
 #define ccs_call_quirk(sensor, _quirk, ...)				\
 	((sensor)->minfo.quirk &&					\
 	 (sensor)->minfo.quirk->_quirk ?				\
diff --git a/drivers/media/i2c/ccs/ccs.h b/drivers/media/i2c/ccs/ccs.h
index 4725e6eca8d0..adb152366ea2 100644
--- a/drivers/media/i2c/ccs/ccs.h
+++ b/drivers/media/i2c/ccs/ccs.h
@@ -46,6 +46,8 @@ 
 
 #define CCS_COLOUR_COMPONENTS		4
 
+#define CCS_DEFAULT_COMPRESSED_DT	0x30
+
 #define SMIAPP_NAME			"smiapp"
 #define CCS_NAME			"ccs"
 
@@ -175,6 +177,8 @@  struct ccs_csi_data_format {
 #define CCS_PAD_SRC			1
 #define CCS_PADS			2
 
+#define CCS_STREAM_PIXEL		0
+
 struct ccs_binning_subtype {
 	u8 horizontal:4;
 	u8 vertical:4;

[v8,22/38] media: ccs: Support frame descriptors

Commit Message

Comments

Patch