Skip to content
Permalink
Browse files
RFC: media: v4l2-subdev: add subdev-wide config struct
We have 'struct v4l2_subdev_pad_config' which contains configuration for
a single pad used for the TRY functionality, and an array of those
structs is passed to various v4l2_subdev_pad_ops.

I was working on subdev internal routing between pads, and realized that
there's no way to add TRY functionality for routes, which is not pad
specific configuration. Adding a separate struct for try-route config
wouldn't work either, as e.g. set-fmt needs to know the try-route
configuration to propagate the settings.

This patch adds a new struct, 'struct v4l2_subdev_config' (which at the
moment only contains the v4l2_subdev_pad_config array) and the new
struct is used in most of the places where v4l2_subdev_pad_config was
used. All v4l2_subdev_pad_ops functions taking v4l2_subdev_pad_config
are changed to instead take v4l2_subdev_config.

Two drivers are changed to work with the above changes (drivers for HW
which I have) as an example.

I worked on a semantic patch (included below, my first spatch...) to do
this change to all drivers, but hit lots of problems with non-trivial
uses of v4l2_subdev_pad_config.

As it looks like substantial amount of manual work is needed, I'm
posting this RFC to get an ack on the changes before continuing that
work.

@ v4l2_subdev_pad_ops @
identifier pad_ops;
identifier func;
@@

(
static const struct v4l2_subdev_pad_ops pad_ops = {
        ...,
        .enum_mbus_code = func,
        ...,
};
|
static const struct v4l2_subdev_pad_ops pad_ops = {
        ...,
        .enum_frame_size = func,
        ...,
};
|
static const struct v4l2_subdev_pad_ops pad_ops = {
        ...,
        .enum_frame_interval = func,
        ...,
};
|
static const struct v4l2_subdev_pad_ops pad_ops = {
        ...,
        .get_fmt = func,
        ...,
};
|
static const struct v4l2_subdev_pad_ops pad_ops = {
        ...,
        .set_fmt = func,
        ...,
};
|
static const struct v4l2_subdev_pad_ops pad_ops = {
        ...,
        .get_selection = func,
        ...,
};
|
static const struct v4l2_subdev_pad_ops pad_ops = {
        ...,
        .set_selection = func,
        ...,
};
|
static const struct v4l2_subdev_pad_ops pad_ops = {
        ...,
        .init_cfg = func,
        ...,
};
)

@@
identifier v4l2_subdev_pad_ops.func;
identifier sd;
identifier cfg;
@@

 func(struct v4l2_subdev *sd,
-	struct v4l2_subdev_pad_config *cfg,
+	struct v4l2_subdev_config *cfg,
	...
      )
 {
    ...
 }

@@
identifier v4l2_subdev_pad_ops.func;
identifier sd;
identifier cfg;
@@

 func(struct v4l2_subdev *sd,
-   struct v4l2_subdev_pad_config *cfg
+   struct v4l2_subdev_config *cfg
      )
 {
    ...
 }

@@
struct v4l2_subdev_fh *fh;
@@
-    fh->pad
+    &fh->cfg

@@
identifier func;
identifier cfg;
@@

func(...,
- struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_config *cfg,
 ...)
{
    ...
}

@@
struct v4l2_subdev_config *cfg;
@@
 {
    <...
(
-   cfg->try_fmt
+   cfg->pad_configs->try_fmt
|
-   cfg->try_crop
+   cfg->pad_configs->try_crop
|
-   cfg->try_compose
+   cfg->pad_configs->try_compose
)
    ...>
 }

@@
identifier pad_cfg;
@@
{
    ...
    struct v4l2_subdev_pad_config pad_cfg;
+   struct v4l2_subdev_config cfg = { .pad_configs = &pad_cfg };
    <...
-   &pad_cfg
+   &cfg
    ...>
}

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
  • Loading branch information
tomba authored and intel-lab-lkp committed Apr 9, 2021
1 parent 4f4e664 commit 4690ebdd6fd3180366038788f7df1fa2420f00c9
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 78 deletions.
@@ -2227,7 +2227,7 @@ static int ov5640_try_frame_interval(struct ov5640_dev *sensor,
}

static int ov5640_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_format *format)
{
struct ov5640_dev *sensor = to_ov5640_dev(sd);
@@ -2285,7 +2285,7 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
}

static int ov5640_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_format *format)
{
struct ov5640_dev *sensor = to_ov5640_dev(sd);
@@ -2818,7 +2818,7 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
}

static int ov5640_enum_frame_size(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_frame_size_enum *fse)
{
if (fse->pad != 0)
@@ -2838,7 +2838,7 @@ static int ov5640_enum_frame_size(struct v4l2_subdev *sd,

static int ov5640_enum_frame_interval(
struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_frame_interval_enum *fie)
{
struct ov5640_dev *sensor = to_ov5640_dev(sd);
@@ -2924,7 +2924,7 @@ static int ov5640_s_frame_interval(struct v4l2_subdev *sd,
}

static int ov5640_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
{
if (code->pad != 0)
@@ -586,7 +586,7 @@ static inline struct cal_camerarx *to_cal_camerarx(struct v4l2_subdev *sd)

static struct v4l2_mbus_framefmt *
cal_camerarx_get_pad_format(struct cal_camerarx *phy,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
unsigned int pad, u32 which)
{
switch (which) {
@@ -611,7 +611,7 @@ static int cal_camerarx_sd_s_stream(struct v4l2_subdev *sd, int enable)
}

static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
{
struct cal_camerarx *phy = to_cal_camerarx(sd);
@@ -639,7 +639,7 @@ static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
}

static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_frame_size_enum *fse)
{
struct cal_camerarx *phy = to_cal_camerarx(sd);
@@ -679,7 +679,7 @@ static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
}

static int cal_camerarx_sd_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_format *format)
{
struct cal_camerarx *phy = to_cal_camerarx(sd);
@@ -692,7 +692,7 @@ static int cal_camerarx_sd_get_fmt(struct v4l2_subdev *sd,
}

static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_format *format)
{
struct cal_camerarx *phy = to_cal_camerarx(sd);
@@ -742,7 +742,7 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
}

static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg)
struct v4l2_subdev_config *cfg)
{
struct v4l2_subdev_format format = {
.which = cfg ? V4L2_SUBDEV_FORMAT_TRY
@@ -26,19 +26,18 @@
#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)
{
if (sd->entity.num_pads) {
fh->pad = v4l2_subdev_alloc_pad_config(sd);
if (fh->pad == NULL)
return -ENOMEM;
}
int ret;

ret = v4l2_subdev_init_config(sd, &fh->cfg);
if (ret)
return ret;

return 0;
}

static void subdev_fh_free(struct v4l2_subdev_fh *fh)
{
v4l2_subdev_free_pad_config(fh->pad);
fh->pad = NULL;
v4l2_subdev_uninit_config(&fh->cfg);
}

static int subdev_open(struct file *file)
@@ -146,7 +145,7 @@ static inline int check_pad(struct v4l2_subdev *sd, u32 pad)
return 0;
}

static int check_cfg(u32 which, struct v4l2_subdev_pad_config *cfg)
static int check_cfg(u32 which, struct v4l2_subdev_config *cfg)
{
if (which == V4L2_SUBDEV_FORMAT_TRY && !cfg)
return -EINVAL;
@@ -155,7 +154,7 @@ static int check_cfg(u32 which, struct v4l2_subdev_pad_config *cfg)
}

static inline int check_format(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_format *format)
{
if (!format)
@@ -166,23 +165,23 @@ static inline int check_format(struct v4l2_subdev *sd,
}

static int call_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_format *format)
{
return check_format(sd, cfg, format) ? :
sd->ops->pad->get_fmt(sd, cfg, format);
}

static int call_set_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_format *format)
{
return check_format(sd, cfg, format) ? :
sd->ops->pad->set_fmt(sd, cfg, format);
}

static int call_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
{
if (!code)
@@ -194,7 +193,7 @@ static int call_enum_mbus_code(struct v4l2_subdev *sd,
}

static int call_enum_frame_size(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_frame_size_enum *fse)
{
if (!fse)
@@ -229,7 +228,7 @@ static int call_s_frame_interval(struct v4l2_subdev *sd,
}

static int call_enum_frame_interval(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_frame_interval_enum *fie)
{
if (!fie)
@@ -241,7 +240,7 @@ static int call_enum_frame_interval(struct v4l2_subdev *sd,
}

static inline int check_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_selection *sel)
{
if (!sel)
@@ -252,15 +251,15 @@ static inline int check_selection(struct v4l2_subdev *sd,
}

static int call_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_selection *sel)
{
return check_selection(sd, cfg, sel) ? :
sd->ops->pad->get_selection(sd, cfg, sel);
}

static int call_set_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_config *cfg,
struct v4l2_subdev_selection *sel)
{
return check_selection(sd, cfg, sel) ? :
@@ -506,7 +505,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)

memset(format->reserved, 0, sizeof(format->reserved));
memset(format->format.reserved, 0, sizeof(format->format.reserved));
return v4l2_subdev_call(sd, pad, get_fmt, subdev_fh->pad, format);
return v4l2_subdev_call(sd, pad, get_fmt, &subdev_fh->cfg, format);
}

case VIDIOC_SUBDEV_S_FMT: {
@@ -517,7 +516,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)

memset(format->reserved, 0, sizeof(format->reserved));
memset(format->format.reserved, 0, sizeof(format->format.reserved));
return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format);
return v4l2_subdev_call(sd, pad, set_fmt, &subdev_fh->cfg, format);
}

case VIDIOC_SUBDEV_G_CROP: {
@@ -531,7 +530,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
sel.target = V4L2_SEL_TGT_CROP;

rval = v4l2_subdev_call(
sd, pad, get_selection, subdev_fh->pad, &sel);
sd, pad, get_selection, &subdev_fh->cfg, &sel);

crop->rect = sel.r;

@@ -553,7 +552,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
sel.r = crop->rect;

rval = v4l2_subdev_call(
sd, pad, set_selection, subdev_fh->pad, &sel);
sd, pad, set_selection, &subdev_fh->cfg, &sel);

crop->rect = sel.r;

@@ -564,15 +563,15 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
struct v4l2_subdev_mbus_code_enum *code = arg;

memset(code->reserved, 0, sizeof(code->reserved));
return v4l2_subdev_call(sd, pad, enum_mbus_code, subdev_fh->pad,
return v4l2_subdev_call(sd, pad, enum_mbus_code, &subdev_fh->cfg,
code);
}

case VIDIOC_SUBDEV_ENUM_FRAME_SIZE: {
struct v4l2_subdev_frame_size_enum *fse = arg;

memset(fse->reserved, 0, sizeof(fse->reserved));
return v4l2_subdev_call(sd, pad, enum_frame_size, subdev_fh->pad,
return v4l2_subdev_call(sd, pad, enum_frame_size, &subdev_fh->cfg,
fse);
}

@@ -597,7 +596,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
struct v4l2_subdev_frame_interval_enum *fie = arg;

memset(fie->reserved, 0, sizeof(fie->reserved));
return v4l2_subdev_call(sd, pad, enum_frame_interval, subdev_fh->pad,
return v4l2_subdev_call(sd, pad, enum_frame_interval, &subdev_fh->cfg,
fie);
}

@@ -606,7 +605,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)

memset(sel->reserved, 0, sizeof(sel->reserved));
return v4l2_subdev_call(
sd, pad, get_selection, subdev_fh->pad, sel);
sd, pad, get_selection, &subdev_fh->cfg, sel);
}

case VIDIOC_SUBDEV_S_SELECTION: {
@@ -617,7 +616,7 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)

memset(sel->reserved, 0, sizeof(sel->reserved));
return v4l2_subdev_call(
sd, pad, set_selection, subdev_fh->pad, sel);
sd, pad, set_selection, &subdev_fh->cfg, sel);
}

case VIDIOC_G_EDID: {
@@ -892,35 +891,35 @@ int v4l2_subdev_link_validate(struct media_link *link)
}
EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate);

struct v4l2_subdev_pad_config *
v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd)
int v4l2_subdev_init_config(struct v4l2_subdev *sd, struct v4l2_subdev_config *cfg)
{
struct v4l2_subdev_pad_config *cfg;
int ret;

if (!sd->entity.num_pads)
return NULL;

cfg = kvmalloc_array(sd->entity.num_pads, sizeof(*cfg),
GFP_KERNEL | __GFP_ZERO);
if (!cfg)
return NULL;
if (sd->entity.num_pads) {
cfg->pad_configs = kvmalloc_array(sd->entity.num_pads, sizeof(*cfg->pad_configs),
GFP_KERNEL | __GFP_ZERO);
if (!cfg->pad_configs)
return -ENOMEM;
} else {
cfg->pad_configs = NULL;
}

ret = v4l2_subdev_call(sd, pad, init_cfg, cfg);
if (ret < 0 && ret != -ENOIOCTLCMD) {
kvfree(cfg);
return NULL;
kvfree(cfg->pad_configs);
return ret;
}

return cfg;
return 0;
}
EXPORT_SYMBOL_GPL(v4l2_subdev_alloc_pad_config);
EXPORT_SYMBOL_GPL(v4l2_subdev_init_config);

void v4l2_subdev_free_pad_config(struct v4l2_subdev_pad_config *cfg)
void v4l2_subdev_uninit_config(struct v4l2_subdev_config *cfg)
{
kvfree(cfg);
kvfree(cfg->pad_configs);
}
EXPORT_SYMBOL_GPL(v4l2_subdev_free_pad_config);
EXPORT_SYMBOL_GPL(v4l2_subdev_uninit_config);

#endif /* CONFIG_MEDIA_CONTROLLER */

void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)

0 comments on commit 4690ebd

Please sign in to comment.