media: vimc: scaler: Use subdev active state

Store the active formats and crop rectangle in the subdevice active
state. This simplifies implementation of the format and selection
accessors, and allows using the v4l2_subdev_get_fmt() helper to
implement the .get_fmt() operation.

The active configuration that is used in the .process_frame() handler is
still stored in the vimc_scaler_device structure. The driver could
instead access the active state in the .process_frame() handler, but the
required locking could interfere with the real time constraints of the
frame processing. This data would be stored in registers in the
.s_stream() handler for real hardware, storing it in dedicated storage
thus mimics a real driver. To differentiate them from the rest of the
device private data, move the corresponding fields to a sub-structure of
vimc_scaler_device named hw.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

drivers/media/test-drivers/vimc/vimc-scaler.c

@@ -26,13 +26,20 @@ enum vimc_scaler_pad {
 struct vimc_scaler_device {
 	struct vimc_ent_device ved;
 	struct v4l2_subdev sd;
-	struct v4l2_rect crop_rect;
-	/* Frame format for both sink and src pad */
-	struct v4l2_mbus_framefmt fmt[2];
-	/* Values calculated when the stream starts */
-	u8 *src_frame;
-	unsigned int bpp;
 	struct media_pad pads[2];
+
+	u8 *src_frame;
+
+	/*
+	 * Virtual "hardware" configuration, filled when the stream starts or
+	 * when controls are set.
+	 */
+	struct {
+		struct v4l2_mbus_framefmt sink_fmt;
+		struct v4l2_mbus_framefmt src_fmt;
+		struct v4l2_rect sink_crop;
+		unsigned int bpp;
+	} hw;
 };
 
 static const struct v4l2_mbus_framefmt fmt_default = {
@@ -132,39 +139,6 @@ static int vimc_scaler_enum_frame_size(struct v4l2_subdev *sd,
 	return 0;
 }
 
-static struct v4l2_mbus_framefmt *
-vimc_scaler_pad_format(struct vimc_scaler_device *vscaler,
-		    struct v4l2_subdev_state *sd_state, u32 pad,
-		    enum v4l2_subdev_format_whence which)
-{
-	if (which == V4L2_SUBDEV_FORMAT_TRY)
-		return v4l2_subdev_state_get_format(sd_state, pad);
-	else
-		return &vscaler->fmt[pad];
-}
-
-static struct v4l2_rect *
-vimc_scaler_pad_crop(struct vimc_scaler_device *vscaler,
-		  struct v4l2_subdev_state *sd_state,
-		  enum v4l2_subdev_format_whence which)
-{
-	if (which == V4L2_SUBDEV_FORMAT_TRY)
-		return v4l2_subdev_state_get_crop(sd_state, VIMC_SCALER_SINK);
-	else
-		return &vscaler->crop_rect;
-}
-
-static int vimc_scaler_get_fmt(struct v4l2_subdev *sd,
-			    struct v4l2_subdev_state *sd_state,
-			    struct v4l2_subdev_format *format)
-{
-	struct vimc_scaler_device *vscaler = v4l2_get_subdevdata(sd);
-
-	format->format = *vimc_scaler_pad_format(vscaler, sd_state, format->pad,
-					      format->which);
-	return 0;
-}
-
 static int vimc_scaler_set_fmt(struct v4l2_subdev *sd,
 			    struct v4l2_subdev_state *sd_state,
 			    struct v4l2_subdev_format *format)
@@ -176,7 +150,7 @@ static int vimc_scaler_set_fmt(struct v4l2_subdev *sd,
 	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE && vscaler->src_frame)
 		return -EBUSY;
 
-	fmt = vimc_scaler_pad_format(vscaler, sd_state, format->pad, format->which);
+	fmt = v4l2_subdev_state_get_format(sd_state, format->pad);
 
 	/*
 	 * The media bus code and colorspace can only be changed on the sink
@@ -214,14 +188,13 @@ static int vimc_scaler_set_fmt(struct v4l2_subdev *sd,
 		struct v4l2_mbus_framefmt *src_fmt;
 		struct v4l2_rect *crop;
 
-		crop = vimc_scaler_pad_crop(vscaler, sd_state, format->which);
+		crop = v4l2_subdev_state_get_crop(sd_state, VIMC_SCALER_SINK);
 		crop->width = fmt->width;
 		crop->height = fmt->height;
 		crop->top = 0;
 		crop->left = 0;
 
-		src_fmt = vimc_scaler_pad_format(vscaler, sd_state, VIMC_SCALER_SRC,
-					      format->which);
+		src_fmt = v4l2_subdev_state_get_format(sd_state, VIMC_SCALER_SRC);
 		*src_fmt = *fmt;
 	}
 
@@ -234,7 +207,6 @@ static int vimc_scaler_get_selection(struct v4l2_subdev *sd,
 				  struct v4l2_subdev_state *sd_state,
 				  struct v4l2_subdev_selection *sel)
 {
-	struct vimc_scaler_device *vscaler = v4l2_get_subdevdata(sd);
 	struct v4l2_mbus_framefmt *sink_fmt;
 
 	if (VIMC_IS_SRC(sel->pad))
@@ -242,11 +214,10 @@ static int vimc_scaler_get_selection(struct v4l2_subdev *sd,
 
 	switch (sel->target) {
 	case V4L2_SEL_TGT_CROP:
-		sel->r = *vimc_scaler_pad_crop(vscaler, sd_state, sel->which);
+		sel->r = *v4l2_subdev_state_get_crop(sd_state, VIMC_SCALER_SINK);
 		break;
 	case V4L2_SEL_TGT_CROP_BOUNDS:
-		sink_fmt = vimc_scaler_pad_format(vscaler, sd_state, VIMC_SCALER_SINK,
-					       sel->which);
+		sink_fmt = v4l2_subdev_state_get_format(sd_state, VIMC_SCALER_SINK);
 		sel->r = vimc_scaler_get_crop_bound_sink(sink_fmt);
 		break;
 	default:
@@ -282,9 +253,8 @@ static int vimc_scaler_set_selection(struct v4l2_subdev *sd,
 	if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE && vscaler->src_frame)
 		return -EBUSY;
 
-	crop_rect = vimc_scaler_pad_crop(vscaler, sd_state, sel->which);
-	sink_fmt = vimc_scaler_pad_format(vscaler, sd_state, VIMC_SCALER_SINK,
-				       sel->which);
+	crop_rect = v4l2_subdev_state_get_crop(sd_state, VIMC_SCALER_SINK);
+	sink_fmt = v4l2_subdev_state_get_format(sd_state, VIMC_SCALER_SINK);
 	vimc_scaler_adjust_sink_crop(&sel->r, sink_fmt);
 	*crop_rect = sel->r;
 
@@ -294,7 +264,7 @@ static int vimc_scaler_set_selection(struct v4l2_subdev *sd,
 static const struct v4l2_subdev_pad_ops vimc_scaler_pad_ops = {
 	.enum_mbus_code		= vimc_scaler_enum_mbus_code,
 	.enum_frame_size	= vimc_scaler_enum_frame_size,
-	.get_fmt		= vimc_scaler_get_fmt,
+	.get_fmt		= v4l2_subdev_get_fmt,
 	.set_fmt		= vimc_scaler_set_fmt,
 	.get_selection		= vimc_scaler_get_selection,
 	.set_selection		= vimc_scaler_set_selection,
@@ -305,27 +275,38 @@ static int vimc_scaler_s_stream(struct v4l2_subdev *sd, int enable)
 	struct vimc_scaler_device *vscaler = v4l2_get_subdevdata(sd);
 
 	if (enable) {
-		const struct vimc_pix_map *vpix;
+		struct v4l2_subdev_state *state;
+		const struct v4l2_mbus_framefmt *format;
+		const struct v4l2_rect *rect;
 		unsigned int frame_size;
 
 		if (vscaler->src_frame)
 			return 0;
 
-		/* Save the bytes per pixel of the sink */
-		vpix = vimc_pix_map_by_code(vscaler->fmt[VIMC_SCALER_SINK].code);
-		vscaler->bpp = vpix->bpp;
+		state = v4l2_subdev_lock_and_get_active_state(sd);
+
+		/* Save the bytes per pixel of the sink. */
+		format = v4l2_subdev_state_get_format(state, VIMC_SCALER_SINK);
+		vscaler->hw.sink_fmt = *format;
+		vscaler->hw.bpp = vimc_pix_map_by_code(format->code)->bpp;
+
+		/* Calculate the frame size of the source pad. */
+		format = v4l2_subdev_state_get_format(state, VIMC_SCALER_SRC);
+		vscaler->hw.src_fmt = *format;
+		frame_size = format->width * format->height * vscaler->hw.bpp;
+
+		rect = v4l2_subdev_state_get_crop(state, VIMC_SCALER_SINK);
+		vscaler->hw.sink_crop = *rect;
 
-		/* Calculate the frame size of the source pad */
-		frame_size = vscaler->fmt[VIMC_SCALER_SRC].width
-			   * vscaler->fmt[VIMC_SCALER_SRC].height * vscaler->bpp;
+		v4l2_subdev_unlock_state(state);
 
-		/* Allocate the frame buffer. Use vmalloc to be able to
-		 * allocate a large amount of memory
+		/*
+		 * Allocate the frame buffer. Use vmalloc to be able to allocate
+		 * a large amount of memory.
 		 */
 		vscaler->src_frame = vmalloc(frame_size);
 		if (!vscaler->src_frame)
 			return -ENOMEM;
-
 	} else {
 		if (!vscaler->src_frame)
 			return 0;
@@ -353,9 +334,9 @@ static const struct v4l2_subdev_internal_ops vimc_scaler_internal_ops = {
 static void vimc_scaler_fill_src_frame(const struct vimc_scaler_device *const vscaler,
 				    const u8 *const sink_frame)
 {
-	const struct v4l2_mbus_framefmt *src_fmt = &vscaler->fmt[VIMC_SCALER_SRC];
-	const struct v4l2_rect *r = &vscaler->crop_rect;
-	unsigned int snk_width = vscaler->fmt[VIMC_SCALER_SINK].width;
+	const struct v4l2_mbus_framefmt *sink_fmt = &vscaler->hw.sink_fmt;
+	const struct v4l2_mbus_framefmt *src_fmt = &vscaler->hw.src_fmt;
+	const struct v4l2_rect *r = &vscaler->hw.sink_crop;
 	unsigned int src_x, src_y;
 	u8 *walker = vscaler->src_frame;
 
@@ -364,16 +345,16 @@ static void vimc_scaler_fill_src_frame(const struct vimc_scaler_device *const vs
 		unsigned int snk_y, y_offset;
 
 		snk_y = (src_y * r->height) / src_fmt->height + r->top;
-		y_offset = snk_y * snk_width * vscaler->bpp;
+		y_offset = snk_y * sink_fmt->width * vscaler->hw.bpp;
 
 		for (src_x = 0; src_x < src_fmt->width; src_x++) {
 			unsigned int snk_x, x_offset, index;
 
 			snk_x = (src_x * r->width) / src_fmt->width + r->left;
-			x_offset = snk_x * vscaler->bpp;
+			x_offset = snk_x * vscaler->hw.bpp;
 			index = y_offset + x_offset;
-			memcpy(walker, &sink_frame[index], vscaler->bpp);
-			walker += vscaler->bpp;
+			memcpy(walker, &sink_frame[index], vscaler->hw.bpp);
+			walker += vscaler->hw.bpp;
 		}
 	}
 }
@@ -432,13 +413,6 @@ static struct vimc_ent_device *vimc_scaler_add(struct vimc_device *vimc,
 	vscaler->ved.process_frame = vimc_scaler_process_frame;
 	vscaler->ved.dev = vimc->mdev.dev;
 
-	/* Initialize the frame format */
-	vscaler->fmt[VIMC_SCALER_SINK] = fmt_default;
-	vscaler->fmt[VIMC_SCALER_SRC] = fmt_default;
-
-	/* Initialize the crop selection */
-	vscaler->crop_rect = crop_rect_default;
-
 	return &vscaler->ved;
 }