* [PATCH 0/4] media: i2c: st-vgxy61 Add support for embedded metadata
@ 2024-03-15 8:51 Julien Massot
2024-03-15 8:51 ` [PATCH 1/4] media: i2c: st-vgxy61: Use sub-device active state Julien Massot
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Julien Massot @ 2024-03-15 8:51 UTC (permalink / raw
To: mchehab, sakari.ailus, benjamin.mugnier, sylvain.petinot
Cc: linux-media, kernel, Julien Massot
The ST-VGXY61 sensors are transmitting Intelligent Status Line (ISL).
The ISL contains some values of the user interface registers.
ISL data are transmitted as RAW8 data packing and follow the
SMIA CSI-2, simplified, 2-byte, tagged data with MIPI_CSI2_DT_EMBEDDED_8B
datatype.
According to the documentation, the firmware preconfigures two status lines
of 256 bytes sent every frame, hence the format 256x2.
ISL data can be disabled in SW_STDBY state, so the set_routing callback is
added to disable the ISL data transmission.
This patchset depends on the "Generic line based metadata support, internal pads"
v8 patchset version.
Julien Massot (4):
media: i2c: st-vgxy61: Use sub-device active state
media: i2c: st-vgxy61: Add support for embedded data
media: i2c: st-vgxy61: Switch to {enable,disable}_streams
media: i2c: st-vgxy61: Implement set_routing callback
drivers/media/i2c/st-vgxy61.c | 345 +++++++++++++++++++++++++---------
1 file changed, 260 insertions(+), 85 deletions(-)
--
2.44.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/4] media: i2c: st-vgxy61: Use sub-device active state
2024-03-15 8:51 [PATCH 0/4] media: i2c: st-vgxy61 Add support for embedded metadata Julien Massot
@ 2024-03-15 8:51 ` Julien Massot
2024-04-03 12:26 ` Benjamin Mugnier
2024-03-15 8:51 ` [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data Julien Massot
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Julien Massot @ 2024-03-15 8:51 UTC (permalink / raw
To: mchehab, sakari.ailus, benjamin.mugnier, sylvain.petinot
Cc: linux-media, kernel, Julien Massot
Use sub-device active state. Rely on control handler lock to serialize
access to the active state.
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
drivers/media/i2c/st-vgxy61.c | 109 ++++++++++++----------------------
1 file changed, 39 insertions(+), 70 deletions(-)
diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
index b9e7c57027b1..733713f909cf 100644
--- a/drivers/media/i2c/st-vgxy61.c
+++ b/drivers/media/i2c/st-vgxy61.c
@@ -397,8 +397,6 @@ struct vgxy61_dev {
u16 line_length;
u16 rot_term;
bool gpios_polarity;
- /* Lock to protect all members below */
- struct mutex lock;
struct v4l2_ctrl_handler ctrl_handler;
struct v4l2_ctrl *pixel_rate_ctrl;
struct v4l2_ctrl *expo_ctrl;
@@ -686,27 +684,6 @@ static int vgxy61_enum_mbus_code(struct v4l2_subdev *sd,
return 0;
}
-static int vgxy61_get_fmt(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state,
- struct v4l2_subdev_format *format)
-{
- struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
- struct v4l2_mbus_framefmt *fmt;
-
- mutex_lock(&sensor->lock);
-
- if (format->which == V4L2_SUBDEV_FORMAT_TRY)
- fmt = v4l2_subdev_state_get_format(sd_state, format->pad);
- else
- fmt = &sensor->fmt;
-
- format->format = *fmt;
-
- mutex_unlock(&sensor->lock);
-
- return 0;
-}
-
static u16 vgxy61_get_vblank_min(struct vgxy61_dev *sensor,
enum vgxy61_hdr_mode hdr)
{
@@ -1167,16 +1144,17 @@ static int vgxy61_stream_disable(struct vgxy61_dev *sensor)
static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable)
{
struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
+ struct v4l2_subdev_state *sd_state;
int ret = 0;
- mutex_lock(&sensor->lock);
+ sd_state = v4l2_subdev_lock_and_get_active_state(&sensor->sd);
ret = enable ? vgxy61_stream_enable(sensor) :
vgxy61_stream_disable(sensor);
if (!ret)
sensor->streaming = enable;
- mutex_unlock(&sensor->lock);
+ v4l2_subdev_unlock_state(sd_state);
return ret;
}
@@ -1187,51 +1165,39 @@ static int vgxy61_set_fmt(struct v4l2_subdev *sd,
{
struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
const struct vgxy61_mode_info *new_mode;
- struct v4l2_mbus_framefmt *fmt;
int ret;
- mutex_lock(&sensor->lock);
-
- if (sensor->streaming) {
- ret = -EBUSY;
- goto out;
- }
+ if (sensor->streaming)
+ return -EBUSY;
ret = vgxy61_try_fmt_internal(sd, &format->format, &new_mode);
if (ret)
- goto out;
-
- if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
- fmt = v4l2_subdev_state_get_format(sd_state, 0);
- *fmt = format->format;
- } else if (sensor->current_mode != new_mode ||
- sensor->fmt.code != format->format.code) {
- fmt = &sensor->fmt;
- *fmt = format->format;
-
- sensor->current_mode = new_mode;
-
- /* Reset vblank and framelength to default */
- ret = vgxy61_update_vblank(sensor,
- VGXY61_FRAME_LENGTH_DEF -
- new_mode->crop.height,
- sensor->hdr);
-
- /* Update controls to reflect new mode */
- __v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate_ctrl,
- get_pixel_rate(sensor));
- __v4l2_ctrl_modify_range(sensor->vblank_ctrl,
- sensor->vblank_min,
- 0xffff - new_mode->crop.height,
- 1, sensor->vblank);
- __v4l2_ctrl_s_ctrl(sensor->vblank_ctrl, sensor->vblank);
- __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min,
- sensor->expo_max, 1,
- sensor->expo_long);
- }
+ return ret;
+
+ *v4l2_subdev_state_get_format(sd_state, format->pad) = format->format;
-out:
- mutex_unlock(&sensor->lock);
+ if (format->which == V4L2_SUBDEV_FORMAT_TRY)
+ return 0;
+
+ sensor->current_mode = new_mode;
+
+ /* Reset vblank and framelength to default */
+ ret = vgxy61_update_vblank(sensor,
+ VGXY61_FRAME_LENGTH_DEF -
+ new_mode->crop.height,
+ sensor->hdr);
+
+ /* Update controls to reflect new mode */
+ __v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate_ctrl,
+ get_pixel_rate(sensor));
+ __v4l2_ctrl_modify_range(sensor->vblank_ctrl,
+ sensor->vblank_min,
+ 0xffff - new_mode->crop.height,
+ 1, sensor->vblank);
+ __v4l2_ctrl_s_ctrl(sensor->vblank_ctrl, sensor->vblank);
+ __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min,
+ sensor->expo_max, 1,
+ sensor->expo_long);
return ret;
}
@@ -1321,8 +1287,6 @@ static int vgxy61_init_controls(struct vgxy61_dev *sensor)
int ret;
v4l2_ctrl_handler_init(hdl, 16);
- /* We can use our own mutex for the ctrl lock */
- hdl->lock = &sensor->lock;
v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN, 0, 0x1c, 1,
sensor->analog_gain);
v4l2_ctrl_new_std(hdl, ops, V4L2_CID_DIGITAL_GAIN, 0, 0xfff, 1,
@@ -1398,7 +1362,7 @@ static const struct v4l2_subdev_video_ops vgxy61_video_ops = {
static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
.enum_mbus_code = vgxy61_enum_mbus_code,
- .get_fmt = vgxy61_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = vgxy61_set_fmt,
.get_selection = vgxy61_get_selection,
.enum_frame_size = vgxy61_enum_frame_size,
@@ -1801,7 +1765,7 @@ static int vgxy61_probe(struct i2c_client *client)
vgxy61_fill_framefmt(sensor, sensor->current_mode, &sensor->fmt,
VGXY61_MEDIA_BUS_FMT_DEF);
- mutex_init(&sensor->lock);
+ sensor->sd.state_lock = sensor->ctrl_handler.lock;
ret = vgxy61_update_hdr(sensor, sensor->hdr);
if (ret)
@@ -1820,6 +1784,10 @@ static int vgxy61_probe(struct i2c_client *client)
goto error_handler_free;
}
+ ret = v4l2_subdev_init_finalize(&sensor->sd);
+ if (ret)
+ goto error_media_entity_cleanup;
+
/* Enable runtime PM and turn off the device */
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
@@ -1841,11 +1809,12 @@ static int vgxy61_probe(struct i2c_client *client)
error_pm_runtime:
pm_runtime_disable(&client->dev);
pm_runtime_set_suspended(&client->dev);
+ v4l2_subdev_cleanup(&sensor->sd);
+error_media_entity_cleanup:
media_entity_cleanup(&sensor->sd.entity);
error_handler_free:
v4l2_ctrl_handler_free(sensor->sd.ctrl_handler);
error_power_off:
- mutex_destroy(&sensor->lock);
vgxy61_power_off(dev);
return ret;
@@ -1857,7 +1826,7 @@ static void vgxy61_remove(struct i2c_client *client)
struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
v4l2_async_unregister_subdev(&sensor->sd);
- mutex_destroy(&sensor->lock);
+ v4l2_subdev_cleanup(&sensor->sd);
media_entity_cleanup(&sensor->sd.entity);
pm_runtime_disable(&client->dev);
--
2.44.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data
2024-03-15 8:51 [PATCH 0/4] media: i2c: st-vgxy61 Add support for embedded metadata Julien Massot
2024-03-15 8:51 ` [PATCH 1/4] media: i2c: st-vgxy61: Use sub-device active state Julien Massot
@ 2024-03-15 8:51 ` Julien Massot
2024-03-15 15:43 ` kernel test robot
` (2 more replies)
2024-03-15 8:51 ` [PATCH 3/4] media: i2c: st-vgxy61: Switch to {enable,disable}_streams Julien Massot
2024-03-15 8:51 ` [PATCH 4/4] media: i2c: st-vgxy61: Implement set_routing callback Julien Massot
3 siblings, 3 replies; 12+ messages in thread
From: Julien Massot @ 2024-03-15 8:51 UTC (permalink / raw
To: mchehab, sakari.ailus, benjamin.mugnier, sylvain.petinot
Cc: linux-media, kernel, Julien Massot
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(-)
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;
--
2.44.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/4] media: i2c: st-vgxy61: Switch to {enable,disable}_streams
2024-03-15 8:51 [PATCH 0/4] media: i2c: st-vgxy61 Add support for embedded metadata Julien Massot
2024-03-15 8:51 ` [PATCH 1/4] media: i2c: st-vgxy61: Use sub-device active state Julien Massot
2024-03-15 8:51 ` [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data Julien Massot
@ 2024-03-15 8:51 ` Julien Massot
2024-04-03 12:26 ` Benjamin Mugnier
2024-03-15 8:51 ` [PATCH 4/4] media: i2c: st-vgxy61: Implement set_routing callback Julien Massot
3 siblings, 1 reply; 12+ messages in thread
From: Julien Massot @ 2024-03-15 8:51 UTC (permalink / raw
To: mchehab, sakari.ailus, benjamin.mugnier, sylvain.petinot
Cc: linux-media, kernel, Julien Massot
Switch from s_stream to enable_streams and disable_streams callbacks.
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
drivers/media/i2c/st-vgxy61.c | 37 +++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
index e8302456a8d9..754853378ee6 100644
--- a/drivers/media/i2c/st-vgxy61.c
+++ b/drivers/media/i2c/st-vgxy61.c
@@ -420,7 +420,7 @@ struct vgxy61_dev {
struct v4l2_ctrl *vblank_ctrl;
struct v4l2_ctrl *vflip_ctrl;
struct v4l2_ctrl *hflip_ctrl;
- bool streaming;
+ u8 streaming;
struct v4l2_mbus_framefmt fmt;
const struct vgxy61_mode_info *sensor_modes;
unsigned int sensor_modes_nb;
@@ -1188,20 +1188,35 @@ static int vgxy61_stream_disable(struct vgxy61_dev *sensor)
return ret;
}
-static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable)
+static int vgxy61_enable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state, u32 pad,
+ u64 streams_mask)
{
struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
- struct v4l2_subdev_state *sd_state;
int ret = 0;
- sd_state = v4l2_subdev_lock_and_get_active_state(&sensor->sd);
-
- ret = enable ? vgxy61_stream_enable(sensor) :
- vgxy61_stream_disable(sensor);
+ if (!sensor->streaming)
+ ret = vgxy61_stream_enable(sensor);
if (!ret)
- sensor->streaming = enable;
+ sensor->streaming |= streams_mask;
- v4l2_subdev_unlock_state(sd_state);
+ return ret;
+}
+
+static int vgxy61_disable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state, u32 pad,
+ u64 streams_mask)
+{
+ struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
+ int ret;
+
+ sensor->streaming &= ~streams_mask;
+ if (sensor->streaming)
+ return 0;
+
+ ret = vgxy61_stream_disable(sensor);
+ if (!ret)
+ sensor->streaming = false;
return ret;
}
@@ -1496,7 +1511,7 @@ static const struct v4l2_subdev_core_ops vgxy61_core_ops = {
};
static const struct v4l2_subdev_video_ops vgxy61_video_ops = {
- .s_stream = vgxy61_s_stream,
+ .s_stream = v4l2_subdev_s_stream_helper,
};
static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
@@ -1506,6 +1521,8 @@ static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
.get_frame_desc = vgxy61_get_frame_desc,
.get_selection = vgxy61_get_selection,
.enum_frame_size = vgxy61_enum_frame_size,
+ .enable_streams = vgxy61_enable_streams,
+ .disable_streams = vgxy61_disable_streams,
};
static const struct v4l2_subdev_ops vgxy61_subdev_ops = {
--
2.44.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] media: i2c: st-vgxy61: Implement set_routing callback
2024-03-15 8:51 [PATCH 0/4] media: i2c: st-vgxy61 Add support for embedded metadata Julien Massot
` (2 preceding siblings ...)
2024-03-15 8:51 ` [PATCH 3/4] media: i2c: st-vgxy61: Switch to {enable,disable}_streams Julien Massot
@ 2024-03-15 8:51 ` Julien Massot
3 siblings, 0 replies; 12+ messages in thread
From: Julien Massot @ 2024-03-15 8:51 UTC (permalink / raw
To: mchehab, sakari.ailus, benjamin.mugnier, sylvain.petinot
Cc: linux-media, kernel, Julien Massot
It's possible to bypass the Intelligent Status Line, through the
bypass register. Allows to remove the internal route from the meta
pad to the source pad.
Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
drivers/media/i2c/st-vgxy61.c | 41 +++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
index 754853378ee6..f931c51fddbb 100644
--- a/drivers/media/i2c/st-vgxy61.c
+++ b/drivers/media/i2c/st-vgxy61.c
@@ -1221,6 +1221,46 @@ static int vgxy61_disable_streams(struct v4l2_subdev *sd,
return ret;
}
+static int vgxy61_set_routing(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state,
+ enum v4l2_subdev_format_whence which,
+ struct v4l2_subdev_krouting *routing)
+{
+ struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
+ struct device *dev = &sensor->i2c_client->dev;
+ unsigned int isl_bypass = VGXY61_ISL_BYPASS;
+ struct v4l2_subdev_route *route;
+ int ret;
+
+ if (sensor->streaming)
+ return -EBUSY;
+
+ if (routing->num_routes != 1 && routing->num_routes != 2)
+ return -EINVAL;
+
+ route = &routing->routes[0];
+ if (route->sink_pad != VGXY61_PAD_PIXEL ||
+ route->source_stream != VGXY61_STREAM_PIXEL) {
+ dev_warn(dev, "Can't disable pixel route\n");
+ return -EINVAL;
+ }
+
+ if (routing->num_routes == 2) {
+ route = &routing->routes[1];
+ if (route->sink_pad != VGXY61_PAD_META ||
+ route->source_stream != VGXY61_STREAM_META)
+ return -EINVAL;
+ isl_bypass = 0;
+ }
+
+ ret = cci_update_bits(sensor->regmap, VGXY61_REG_BYPASS_CTRL,
+ VGXY61_ISL_BYPASS, isl_bypass, NULL);
+ if (ret)
+ return ret;
+
+ return v4l2_subdev_set_routing(sd, state, routing);
+}
+
static int __vgxy61_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_mbus_framefmt *format,
@@ -1518,6 +1558,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,
+ .set_routing = vgxy61_set_routing,
.get_frame_desc = vgxy61_get_frame_desc,
.get_selection = vgxy61_get_selection,
.enum_frame_size = vgxy61_enum_frame_size,
--
2.44.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data
2024-03-15 8:51 ` [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data Julien Massot
@ 2024-03-15 15:43 ` kernel test robot
2024-03-15 22:35 ` kernel test robot
2024-04-03 12:26 ` Benjamin Mugnier
2 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2024-03-15 15:43 UTC (permalink / raw
To: Julien Massot, mchehab, sakari.ailus, benjamin.mugnier,
sylvain.petinot
Cc: llvm, oe-kbuild-all, linux-media, kernel, Julien Massot
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data
2024-03-15 8:51 ` [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data Julien Massot
2024-03-15 15:43 ` kernel test robot
@ 2024-03-15 22:35 ` kernel test robot
2024-04-03 12:26 ` Benjamin Mugnier
2 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2024-03-15 22:35 UTC (permalink / raw
To: Julien Massot, mchehab, sakari.ailus, benjamin.mugnier,
sylvain.petinot
Cc: oe-kbuild-all, linux-media, kernel, Julien Massot
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] media: i2c: st-vgxy61: Use sub-device active state
2024-03-15 8:51 ` [PATCH 1/4] media: i2c: st-vgxy61: Use sub-device active state Julien Massot
@ 2024-04-03 12:26 ` Benjamin Mugnier
0 siblings, 0 replies; 12+ messages in thread
From: Benjamin Mugnier @ 2024-04-03 12:26 UTC (permalink / raw
To: Julien Massot, mchehab, sakari.ailus, sylvain.petinot; +Cc: linux-media, kernel
Hi Julien,
Thank you for your patch.
First, sorry for the delay. I have a lot of stuff ongoing. I'll be more
available now.
Second, I don't currently have a setup that can handle the multi stream
serie. Therefore I'm not able to test your serie. Following your
patchset acquiring such a platform went up in my todo list.
On 3/15/24 09:51, Julien Massot wrote:
> Use sub-device active state. Rely on control handler lock to serialize
> access to the active state.
>
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
I have yet to dive deep into active states.
I find curious that the 'current_mode' field in vgxy61_dev is still
here. From my understanding it should be replaced by
'v4l2_subdev_state_get_format', and all the 'current_mode->crop'
replaced by 'v4l2_subdev_state_get_crop'.
Someone tell me if this is incorrect.
> ---
> drivers/media/i2c/st-vgxy61.c | 109 ++++++++++++----------------------
> 1 file changed, 39 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
> index b9e7c57027b1..733713f909cf 100644
> --- a/drivers/media/i2c/st-vgxy61.c
> +++ b/drivers/media/i2c/st-vgxy61.c
> @@ -397,8 +397,6 @@ struct vgxy61_dev {
> u16 line_length;
> u16 rot_term;
> bool gpios_polarity;
> - /* Lock to protect all members below */
> - struct mutex lock;
> struct v4l2_ctrl_handler ctrl_handler;
> struct v4l2_ctrl *pixel_rate_ctrl;
> struct v4l2_ctrl *expo_ctrl;
> @@ -686,27 +684,6 @@ static int vgxy61_enum_mbus_code(struct v4l2_subdev *sd,
> return 0;
> }
>
> -static int vgxy61_get_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_state *sd_state,
> - struct v4l2_subdev_format *format)
> -{
> - struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
> - struct v4l2_mbus_framefmt *fmt;
> -
> - mutex_lock(&sensor->lock);
> -
> - if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> - fmt = v4l2_subdev_state_get_format(sd_state, format->pad);
> - else
> - fmt = &sensor->fmt;
> -
> - format->format = *fmt;
> -
> - mutex_unlock(&sensor->lock);
> -
> - return 0;
> -}
> -
> static u16 vgxy61_get_vblank_min(struct vgxy61_dev *sensor,
> enum vgxy61_hdr_mode hdr)
> {
> @@ -1167,16 +1144,17 @@ static int vgxy61_stream_disable(struct vgxy61_dev *sensor)
> static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable)
> {
> struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
> + struct v4l2_subdev_state *sd_state;
> int ret = 0;
>
> - mutex_lock(&sensor->lock);
> + sd_state = v4l2_subdev_lock_and_get_active_state(&sensor->sd);
>
> ret = enable ? vgxy61_stream_enable(sensor) :
> vgxy61_stream_disable(sensor);
> if (!ret)
> sensor->streaming = enable;
>
> - mutex_unlock(&sensor->lock);
> + v4l2_subdev_unlock_state(sd_state);
>
> return ret;
> }
> @@ -1187,51 +1165,39 @@ static int vgxy61_set_fmt(struct v4l2_subdev *sd,
> {
> struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
> const struct vgxy61_mode_info *new_mode;
> - struct v4l2_mbus_framefmt *fmt;
> int ret;
>
> - mutex_lock(&sensor->lock);
> -
> - if (sensor->streaming) {
> - ret = -EBUSY;
> - goto out;
> - }
> + if (sensor->streaming)
> + return -EBUSY;
>
> ret = vgxy61_try_fmt_internal(sd, &format->format, &new_mode);
> if (ret)
> - goto out;
> -
> - if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> - fmt = v4l2_subdev_state_get_format(sd_state, 0);
> - *fmt = format->format;
> - } else if (sensor->current_mode != new_mode ||
> - sensor->fmt.code != format->format.code) {
> - fmt = &sensor->fmt;
> - *fmt = format->format;
> -
> - sensor->current_mode = new_mode;
> -
> - /* Reset vblank and framelength to default */
> - ret = vgxy61_update_vblank(sensor,
> - VGXY61_FRAME_LENGTH_DEF -
> - new_mode->crop.height,
> - sensor->hdr);
> -
> - /* Update controls to reflect new mode */
> - __v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate_ctrl,
> - get_pixel_rate(sensor));
> - __v4l2_ctrl_modify_range(sensor->vblank_ctrl,
> - sensor->vblank_min,
> - 0xffff - new_mode->crop.height,
> - 1, sensor->vblank);
> - __v4l2_ctrl_s_ctrl(sensor->vblank_ctrl, sensor->vblank);
> - __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min,
> - sensor->expo_max, 1,
> - sensor->expo_long);
> - }
> + return ret;
> +
> + *v4l2_subdev_state_get_format(sd_state, format->pad) = format->format;
>
> -out:
> - mutex_unlock(&sensor->lock);
> + if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> + return 0;
> +
> + sensor->current_mode = new_mode;
> +
> + /* Reset vblank and framelength to default */
> + ret = vgxy61_update_vblank(sensor,
> + VGXY61_FRAME_LENGTH_DEF -
> + new_mode->crop.height,
> + sensor->hdr);
> +
> + /* Update controls to reflect new mode */
> + __v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate_ctrl,
> + get_pixel_rate(sensor));
> + __v4l2_ctrl_modify_range(sensor->vblank_ctrl,
> + sensor->vblank_min,
> + 0xffff - new_mode->crop.height,
> + 1, sensor->vblank);
> + __v4l2_ctrl_s_ctrl(sensor->vblank_ctrl, sensor->vblank);
> + __v4l2_ctrl_modify_range(sensor->expo_ctrl, sensor->expo_min,
> + sensor->expo_max, 1,
> + sensor->expo_long);
>
> return ret;
> }
> @@ -1321,8 +1287,6 @@ static int vgxy61_init_controls(struct vgxy61_dev *sensor)
> int ret;
>
> v4l2_ctrl_handler_init(hdl, 16);
> - /* We can use our own mutex for the ctrl lock */
> - hdl->lock = &sensor->lock;
> v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN, 0, 0x1c, 1,
> sensor->analog_gain);
> v4l2_ctrl_new_std(hdl, ops, V4L2_CID_DIGITAL_GAIN, 0, 0xfff, 1,
> @@ -1398,7 +1362,7 @@ static const struct v4l2_subdev_video_ops vgxy61_video_ops = {
>
> static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
> .enum_mbus_code = vgxy61_enum_mbus_code,
> - .get_fmt = vgxy61_get_fmt,
> + .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = vgxy61_set_fmt,
> .get_selection = vgxy61_get_selection,
> .enum_frame_size = vgxy61_enum_frame_size,
> @@ -1801,7 +1765,7 @@ static int vgxy61_probe(struct i2c_client *client)
> vgxy61_fill_framefmt(sensor, sensor->current_mode, &sensor->fmt,
> VGXY61_MEDIA_BUS_FMT_DEF);
>
> - mutex_init(&sensor->lock);
> + sensor->sd.state_lock = sensor->ctrl_handler.lock;
>
> ret = vgxy61_update_hdr(sensor, sensor->hdr);
> if (ret)
> @@ -1820,6 +1784,10 @@ static int vgxy61_probe(struct i2c_client *client)
> goto error_handler_free;
> }
>
> + ret = v4l2_subdev_init_finalize(&sensor->sd);
> + if (ret)
> + goto error_media_entity_cleanup;
> +
> /* Enable runtime PM and turn off the device */
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
> @@ -1841,11 +1809,12 @@ static int vgxy61_probe(struct i2c_client *client)
> error_pm_runtime:
> pm_runtime_disable(&client->dev);
> pm_runtime_set_suspended(&client->dev);
> + v4l2_subdev_cleanup(&sensor->sd);
> +error_media_entity_cleanup:
> media_entity_cleanup(&sensor->sd.entity);
> error_handler_free:
> v4l2_ctrl_handler_free(sensor->sd.ctrl_handler);
> error_power_off:
> - mutex_destroy(&sensor->lock);
> vgxy61_power_off(dev);
>
> return ret;
> @@ -1857,7 +1826,7 @@ static void vgxy61_remove(struct i2c_client *client)
> struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
>
> v4l2_async_unregister_subdev(&sensor->sd);
> - mutex_destroy(&sensor->lock);
> + v4l2_subdev_cleanup(&sensor->sd);
> media_entity_cleanup(&sensor->sd.entity);
>
> pm_runtime_disable(&client->dev);
--
Regards,
Benjamin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data
2024-03-15 8:51 ` [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data Julien Massot
2024-03-15 15:43 ` kernel test robot
2024-03-15 22:35 ` kernel test robot
@ 2024-04-03 12:26 ` Benjamin Mugnier
2 siblings, 0 replies; 12+ messages in thread
From: Benjamin Mugnier @ 2024-04-03 12:26 UTC (permalink / raw
To: Julien Massot, mchehab, sakari.ailus, sylvain.petinot; +Cc: linux-media, kernel
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;
--
Regards,
Benjamin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] media: i2c: st-vgxy61: Switch to {enable,disable}_streams
2024-03-15 8:51 ` [PATCH 3/4] media: i2c: st-vgxy61: Switch to {enable,disable}_streams Julien Massot
@ 2024-04-03 12:26 ` Benjamin Mugnier
2024-04-03 12:38 ` Sakari Ailus
0 siblings, 1 reply; 12+ messages in thread
From: Benjamin Mugnier @ 2024-04-03 12:26 UTC (permalink / raw
To: Julien Massot, mchehab, sakari.ailus, sylvain.petinot; +Cc: linux-media, kernel
Hi Julien,
On 3/15/24 09:51, Julien Massot wrote:
> Switch from s_stream to enable_streams and disable_streams callbacks.
>
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> ---
> drivers/media/i2c/st-vgxy61.c | 37 +++++++++++++++++++++++++----------
> 1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/i2c/st-vgxy61.c b/drivers/media/i2c/st-vgxy61.c
> index e8302456a8d9..754853378ee6 100644
> --- a/drivers/media/i2c/st-vgxy61.c
> +++ b/drivers/media/i2c/st-vgxy61.c
> @@ -420,7 +420,7 @@ struct vgxy61_dev {
> struct v4l2_ctrl *vblank_ctrl;
> struct v4l2_ctrl *vflip_ctrl;
> struct v4l2_ctrl *hflip_ctrl;
> - bool streaming;
> + u8 streaming;
> struct v4l2_mbus_framefmt fmt;
> const struct vgxy61_mode_info *sensor_modes;
> unsigned int sensor_modes_nb;
> @@ -1188,20 +1188,35 @@ static int vgxy61_stream_disable(struct vgxy61_dev *sensor)
> return ret;
> }
>
> -static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable)
> +static int vgxy61_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> {
Should we also check that 'pad == 0' ? Or is it always so ?
> struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
> - struct v4l2_subdev_state *sd_state;
> int ret = 0;
>
> - sd_state = v4l2_subdev_lock_and_get_active_state(&sensor->sd);
> -
> - ret = enable ? vgxy61_stream_enable(sensor) :
> - vgxy61_stream_disable(sensor);
> + if (!sensor->streaming)
> + ret = vgxy61_stream_enable(sensor);
> if (!ret)
> - sensor->streaming = enable;
> + sensor->streaming |= streams_mask;
>
> - v4l2_subdev_unlock_state(sd_state);
> + return ret;
> +}
> +
> +static int vgxy61_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
Ditto.
> + struct vgxy61_dev *sensor = to_vgxy61_dev(sd);
> + int ret;
> +
> + sensor->streaming &= ~streams_mask;
> + if (sensor->streaming)
> + return 0;
> +
> + ret = vgxy61_stream_disable(sensor);
> + if (!ret)
> + sensor->streaming = false;
>
> return ret;
> }
> @@ -1496,7 +1511,7 @@ static const struct v4l2_subdev_core_ops vgxy61_core_ops = {
> };
>
> static const struct v4l2_subdev_video_ops vgxy61_video_ops = {
> - .s_stream = vgxy61_s_stream,
> + .s_stream = v4l2_subdev_s_stream_helper,
> };
>
> static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
> @@ -1506,6 +1521,8 @@ static const struct v4l2_subdev_pad_ops vgxy61_pad_ops = {
> .get_frame_desc = vgxy61_get_frame_desc,
> .get_selection = vgxy61_get_selection,
> .enum_frame_size = vgxy61_enum_frame_size,
> + .enable_streams = vgxy61_enable_streams,
> + .disable_streams = vgxy61_disable_streams,
> };
>
> static const struct v4l2_subdev_ops vgxy61_subdev_ops = {
--
Regards,
Benjamin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] media: i2c: st-vgxy61: Switch to {enable,disable}_streams
2024-04-03 12:26 ` Benjamin Mugnier
@ 2024-04-03 12:38 ` Sakari Ailus
2024-04-03 14:12 ` Benjamin Mugnier
0 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2024-04-03 12:38 UTC (permalink / raw
To: Benjamin Mugnier
Cc: Julien Massot, mchehab, sylvain.petinot, linux-media, kernel
On Wed, Apr 03, 2024 at 02:26:32PM +0200, Benjamin Mugnier wrote:
> > @@ -1188,20 +1188,35 @@ static int vgxy61_stream_disable(struct vgxy61_dev *sensor)
> > return ret;
> > }
> >
> > -static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable)
> > +static int vgxy61_enable_streams(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state, u32 pad,
> > + u64 streams_mask)
> > {
>
> Should we also check that 'pad == 0' ? Or is it always so ?
If the number of pads in the sub-device is 1, then the pad is always 0
here: this is checked in the framework.
--
Sakari Ailus
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] media: i2c: st-vgxy61: Switch to {enable,disable}_streams
2024-04-03 12:38 ` Sakari Ailus
@ 2024-04-03 14:12 ` Benjamin Mugnier
0 siblings, 0 replies; 12+ messages in thread
From: Benjamin Mugnier @ 2024-04-03 14:12 UTC (permalink / raw
To: Sakari Ailus; +Cc: Julien Massot, mchehab, sylvain.petinot, linux-media, kernel
On 4/3/24 14:38, Sakari Ailus wrote:
> On Wed, Apr 03, 2024 at 02:26:32PM +0200, Benjamin Mugnier wrote:
>>> @@ -1188,20 +1188,35 @@ static int vgxy61_stream_disable(struct vgxy61_dev *sensor)
>>> return ret;
>>> }
>>>
>>> -static int vgxy61_s_stream(struct v4l2_subdev *sd, int enable)
>>> +static int vgxy61_enable_streams(struct v4l2_subdev *sd,
>>> + struct v4l2_subdev_state *state, u32 pad,
>>> + u64 streams_mask)
>>> {
>>
>> Should we also check that 'pad == 0' ? Or is it always so ?
>
> If the number of pads in the sub-device is 1, then the pad is always 0
> here: this is checked in the framework.
>
Perfect. Thank you for the clarification.
--
Regards,
Benjamin
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-04-03 14:12 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-15 8:51 [PATCH 0/4] media: i2c: st-vgxy61 Add support for embedded metadata Julien Massot
2024-03-15 8:51 ` [PATCH 1/4] media: i2c: st-vgxy61: Use sub-device active state Julien Massot
2024-04-03 12:26 ` Benjamin Mugnier
2024-03-15 8:51 ` [PATCH 2/4] media: i2c: st-vgxy61: Add support for embedded data Julien Massot
2024-03-15 15:43 ` kernel test robot
2024-03-15 22:35 ` kernel test robot
2024-04-03 12:26 ` Benjamin Mugnier
2024-03-15 8:51 ` [PATCH 3/4] media: i2c: st-vgxy61: Switch to {enable,disable}_streams Julien Massot
2024-04-03 12:26 ` Benjamin Mugnier
2024-04-03 12:38 ` Sakari Ailus
2024-04-03 14:12 ` Benjamin Mugnier
2024-03-15 8:51 ` [PATCH 4/4] media: i2c: st-vgxy61: Implement set_routing callback Julien Massot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.