From 0d346d2a6f54f06f36b224fd27cd6eafe8c83be9 Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Thu, 10 Jun 2021 17:55:58 +0300 Subject: media: v4l2-subdev: add subdev-wide state 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_state' (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_state. The changes to drivers/media/v4l2-core/v4l2-subdev.c and include/media/v4l2-subdev.h were written by hand, and all the driver changes were done with the semantic patch below. The spatch needs to be applied to a select list of directories. I used the following shell commands to apply the spatch: dirs="drivers/media/i2c drivers/media/platform drivers/media/usb drivers/media/test-drivers/vimc drivers/media/pci drivers/staging/media" for dir in $dirs; do spatch -j8 --dir --include-headers --no-show-diff --in-place --sp-file v4l2-subdev-state.cocci $dir; done Note that Coccinelle chokes on a few drivers (gcc extensions?). With minor changes we can make Coccinelle run fine, and these changes can be reverted after spatch. The diff for these changes is: For drivers/media/i2c/s5k5baf.c: @@ -1481,7 +1481,7 @@ static int s5k5baf_set_selection(struct v4l2_subdev *sd, &s5k5baf_cis_rect, v4l2_subdev_get_try_crop(sd, cfg, PAD_CIS), v4l2_subdev_get_try_compose(sd, cfg, PAD_CIS), - v4l2_subdev_get_try_crop(sd, cfg, PAD_OUT) + v4l2_subdev_get_try_crop(sd, cfg, PAD_OUT), }; s5k5baf_set_rect_and_adjust(rects, rtype, &sel->r); return 0; For drivers/media/platform/s3c-camif/camif-capture.c: @@ -1230,7 +1230,7 @@ static int s3c_camif_subdev_get_fmt(struct v4l2_subdev *sd, *mf = camif->mbus_fmt; break; - case CAMIF_SD_PAD_SOURCE_C...CAMIF_SD_PAD_SOURCE_P: + case CAMIF_SD_PAD_SOURCE_C: /* crop rectangle at camera interface input */ mf->width = camif->camif_crop.width; mf->height = camif->camif_crop.height; @@ -1332,7 +1332,7 @@ static int s3c_camif_subdev_set_fmt(struct v4l2_subdev *sd, } break; - case CAMIF_SD_PAD_SOURCE_C...CAMIF_SD_PAD_SOURCE_P: + case CAMIF_SD_PAD_SOURCE_C: /* Pixel format can be only changed on the sink pad. */ mf->code = camif->mbus_fmt.code; mf->width = crop->width; The semantic patch is: // // Change function parameter @@ identifier func; identifier cfg; @@ func(..., - struct v4l2_subdev_pad_config *cfg + struct v4l2_subdev_state *sd_state , ...) { <... - cfg + sd_state ...> } // Change function declaration parameter @@ identifier func; identifier cfg; type T; @@ T func(..., - struct v4l2_subdev_pad_config *cfg + struct v4l2_subdev_state *sd_state , ...); // Change function return value @@ identifier func; @@ - struct v4l2_subdev_pad_config + struct v4l2_subdev_state *func(...) { ... } // Change function declaration return value @@ identifier func; @@ - struct v4l2_subdev_pad_config + struct v4l2_subdev_state *func(...); // Some drivers pass a local pad_cfg for a single pad to a called function. Wrap it // inside a pad_state. @@ identifier func; identifier pad_cfg; @@ func(...) { ... struct v4l2_subdev_pad_config pad_cfg; + struct v4l2_subdev_state pad_state = { .pads = &pad_cfg }; <+... ( v4l2_subdev_call | sensor_call | isi_try_fse | isc_try_fse | saa_call_all ) (..., - &pad_cfg + &pad_state ,...) ...+> } // If the function uses fields from pad_config, access via state->pads @@ identifier func; identifier state; @@ func(..., struct v4l2_subdev_state *state , ...) { <... ( - state->try_fmt + state->pads->try_fmt | - state->try_crop + state->pads->try_crop | - state->try_compose + state->pads->try_compose ) ...> } // If the function accesses the filehandle, use fh->state instead @@ struct v4l2_subdev_fh *fh; @@ - fh->pad + fh->state @@ struct v4l2_subdev_fh fh; @@ - fh.pad + fh.state // Start of vsp1 specific @@ @@ struct vsp1_entity { ... - struct v4l2_subdev_pad_config *config; + struct v4l2_subdev_state *config; ... }; @@ symbol entity; @@ vsp1_entity_init(...) { ... entity->config = - v4l2_subdev_alloc_pad_config + v4l2_subdev_alloc_state (&entity->subdev); ... } @@ symbol entity; @@ vsp1_entity_destroy(...) { ... - v4l2_subdev_free_pad_config + v4l2_subdev_free_state (entity->config); ... } @exists@ identifier func =~ "(^vsp1.*)|(hsit_set_format)|(sru_enum_frame_size)|(sru_set_format)|(uif_get_selection)|(uif_set_selection)|(uds_enum_frame_size)|(uds_set_format)|(brx_set_format)|(brx_get_selection)|(histo_get_selection)|(histo_set_selection)|(brx_set_selection)"; symbol config; @@ func(...) { ... - struct v4l2_subdev_pad_config *config; + struct v4l2_subdev_state *config; ... } // End of vsp1 specific // Start of rcar specific @@ identifier sd; identifier pad_cfg; @@ rvin_try_format(...) { ... - struct v4l2_subdev_pad_config *pad_cfg; + struct v4l2_subdev_state *sd_state; ... - pad_cfg = v4l2_subdev_alloc_pad_config(sd); + sd_state = v4l2_subdev_alloc_state(sd); <... - pad_cfg + sd_state ...> - v4l2_subdev_free_pad_config(pad_cfg); + v4l2_subdev_free_state(sd_state); ... } // End of rcar specific // Start of rockchip specific @@ identifier func =~ "(rkisp1_rsz_get_pad_fmt)|(rkisp1_rsz_get_pad_crop)|(rkisp1_rsz_register)"; symbol rsz; symbol pad_cfg; @@ func(...) { + struct v4l2_subdev_state state = { .pads = rsz->pad_cfg }; ... - rsz->pad_cfg + &state ... } @@ identifier func =~ "(rkisp1_isp_get_pad_fmt)|(rkisp1_isp_get_pad_crop)"; symbol isp; symbol pad_cfg; @@ func(...) { + struct v4l2_subdev_state state = { .pads = isp->pad_cfg }; ... - isp->pad_cfg + &state ... } @@ symbol rkisp1; symbol isp; symbol pad_cfg; @@ rkisp1_isp_register(...) { + struct v4l2_subdev_state state = { .pads = rkisp1->isp.pad_cfg }; ... - rkisp1->isp.pad_cfg + &state ... } // End of rockchip specific // Start of tegra-video specific @@ identifier sd; identifier pad_cfg; @@ __tegra_channel_try_format(...) { ... - struct v4l2_subdev_pad_config *pad_cfg; + struct v4l2_subdev_state *sd_state; ... - pad_cfg = v4l2_subdev_alloc_pad_config(sd); + sd_state = v4l2_subdev_alloc_state(sd); <... - pad_cfg + sd_state ...> - v4l2_subdev_free_pad_config(pad_cfg); + v4l2_subdev_free_state(sd_state); ... } @@ identifier sd_state; @@ __tegra_channel_try_format(...) { ... struct v4l2_subdev_state *sd_state; <... - sd_state->try_crop + sd_state->pads->try_crop ...> } // End of tegra-video specific // Signed-off-by: Tomi Valkeinen Acked-by: Laurent Pinchart Acked-by: Sakari Ailus Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- include/media/v4l2-subdev.h | 74 +++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 30 deletions(-) (limited to 'include/media/v4l2-subdev.h') diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index d0e9a5bdb08b..89115ba4c0f2 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -623,6 +623,19 @@ struct v4l2_subdev_pad_config { struct v4l2_rect try_compose; }; +/** + * struct v4l2_subdev_state - Used for storing subdev state information. + * + * @pads: &struct v4l2_subdev_pad_config array + * + * This structure only needs to be passed to the pad op if the 'which' field + * of the main argument is set to %V4L2_SUBDEV_FORMAT_TRY. For + * %V4L2_SUBDEV_FORMAT_ACTIVE it is safe to pass %NULL. + */ +struct v4l2_subdev_state { + struct v4l2_subdev_pad_config *pads; +}; + /** * struct v4l2_subdev_pad_ops - v4l2-subdev pad level operations * @@ -687,27 +700,27 @@ struct v4l2_subdev_pad_config { */ struct v4l2_subdev_pad_ops { int (*init_cfg)(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg); + struct v4l2_subdev_state *state); int (*enum_mbus_code)(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_state *state, struct v4l2_subdev_mbus_code_enum *code); int (*enum_frame_size)(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_state *state, struct v4l2_subdev_frame_size_enum *fse); int (*enum_frame_interval)(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_state *state, struct v4l2_subdev_frame_interval_enum *fie); int (*get_fmt)(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_state *state, struct v4l2_subdev_format *format); int (*set_fmt)(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_state *state, struct v4l2_subdev_format *format); int (*get_selection)(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_state *state, struct v4l2_subdev_selection *sel); int (*set_selection)(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_state *state, struct v4l2_subdev_selection *sel); int (*get_edid)(struct v4l2_subdev *sd, struct v4l2_edid *edid); int (*set_edid)(struct v4l2_subdev *sd, struct v4l2_edid *edid); @@ -918,14 +931,14 @@ struct v4l2_subdev { * struct v4l2_subdev_fh - Used for storing subdev information per file handle * * @vfh: pointer to &struct v4l2_fh - * @pad: pointer to &struct v4l2_subdev_pad_config + * @state: pointer to &struct v4l2_subdev_state * @owner: module pointer to the owner of this file handle */ struct v4l2_subdev_fh { struct v4l2_fh vfh; struct module *owner; #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) - struct v4l2_subdev_pad_config *pad; + struct v4l2_subdev_state *state; #endif }; @@ -945,17 +958,17 @@ struct v4l2_subdev_fh { * &struct v4l2_subdev_pad_config->try_fmt * * @sd: pointer to &struct v4l2_subdev - * @cfg: pointer to &struct v4l2_subdev_pad_config array. - * @pad: index of the pad in the @cfg array. + * @state: pointer to &struct v4l2_subdev_state + * @pad: index of the pad in the &struct v4l2_subdev_state->pads array */ static inline struct v4l2_mbus_framefmt * v4l2_subdev_get_try_format(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_state *state, unsigned int pad) { if (WARN_ON(pad >= sd->entity.num_pads)) pad = 0; - return &cfg[pad].try_fmt; + return &state->pads[pad].try_fmt; } /** @@ -963,17 +976,17 @@ v4l2_subdev_get_try_format(struct v4l2_subdev *sd, * &struct v4l2_subdev_pad_config->try_crop * * @sd: pointer to &struct v4l2_subdev - * @cfg: pointer to &struct v4l2_subdev_pad_config array. - * @pad: index of the pad in the @cfg array. + * @state: pointer to &struct v4l2_subdev_state. + * @pad: index of the pad in the &struct v4l2_subdev_state->pads array. */ static inline struct v4l2_rect * v4l2_subdev_get_try_crop(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_state *state, unsigned int pad) { if (WARN_ON(pad >= sd->entity.num_pads)) pad = 0; - return &cfg[pad].try_crop; + return &state->pads[pad].try_crop; } /** @@ -981,17 +994,17 @@ v4l2_subdev_get_try_crop(struct v4l2_subdev *sd, * &struct v4l2_subdev_pad_config->try_compose * * @sd: pointer to &struct v4l2_subdev - * @cfg: pointer to &struct v4l2_subdev_pad_config array. - * @pad: index of the pad in the @cfg array. + * @state: pointer to &struct v4l2_subdev_state. + * @pad: index of the pad in the &struct v4l2_subdev_state->pads array. */ static inline struct v4l2_rect * v4l2_subdev_get_try_compose(struct v4l2_subdev *sd, - struct v4l2_subdev_pad_config *cfg, + struct v4l2_subdev_state *state, unsigned int pad) { if (WARN_ON(pad >= sd->entity.num_pads)) pad = 0; - return &cfg[pad].try_compose; + return &state->pads[pad].try_compose; } #endif @@ -1093,20 +1106,21 @@ int v4l2_subdev_link_validate_default(struct v4l2_subdev *sd, int v4l2_subdev_link_validate(struct media_link *link); /** - * v4l2_subdev_alloc_pad_config - Allocates memory for pad config + * v4l2_subdev_alloc_state - allocate v4l2_subdev_state * - * @sd: pointer to struct v4l2_subdev + * @sd: pointer to &struct v4l2_subdev for which the state is being allocated. + * + * Must call v4l2_subdev_free_state() when state is no longer needed. */ -struct -v4l2_subdev_pad_config *v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd); +struct v4l2_subdev_state *v4l2_subdev_alloc_state(struct v4l2_subdev *sd); /** - * v4l2_subdev_free_pad_config - Frees memory allocated by - * v4l2_subdev_alloc_pad_config(). + * v4l2_subdev_free_state - free a v4l2_subdev_state * - * @cfg: pointer to &struct v4l2_subdev_pad_config + * @state: v4l2_subdev_state to be freed. */ -void v4l2_subdev_free_pad_config(struct v4l2_subdev_pad_config *cfg); +void v4l2_subdev_free_state(struct v4l2_subdev_state *state); + #endif /* CONFIG_MEDIA_CONTROLLER */ /** -- cgit v1.2.3