Hi Sakari,
Thank you for the patch.
On Thursday 02 October 2014 15:09:14 Sakari Ailus wrote:
> smiapp_set_format() has accumulated a fair amount of changes without a
> needed refactoring, do the cleanup now. There's also an unlocked version of
> v4l2_ctrl_range_changed(), using that fixes a small serialisation issue with
> the user space interface.
>
> __v4l2_ctrl_modify_range() is used instead of v4l2_ctrl_modify_range() in
> smiapp_set_format_source() since the mutex is now held during the function
> call.
>
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
For the whole series,
Acked-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> ---
> drivers/media/i2c/smiapp/smiapp-core.c | 73 ++++++++++++++++-------------
> 1 file changed, 43 insertions(+), 30 deletions(-)
>
> since v2:
>
> - Move comment on changed media bus codes to smiapp_set_format_source().
>
> - Add a comment to the patch description on the use of the unlocked variant
> of v4l2_ctrl_modify_range().
>
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
> b/drivers/media/i2c/smiapp/smiapp-core.c index 926f60c..416b7bd 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -1728,51 +1728,64 @@ static const struct smiapp_csi_data_format
> return csi_format;
> }
>
> -static int smiapp_set_format(struct v4l2_subdev *subdev,
> - struct v4l2_subdev_fh *fh,
> - struct v4l2_subdev_format *fmt)
> +static int smiapp_set_format_source(struct v4l2_subdev *subdev,
> + struct v4l2_subdev_fh *fh,
> + struct v4l2_subdev_format *fmt)
> {
> struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
> - struct smiapp_subdev *ssd = to_smiapp_subdev(subdev);
> - struct v4l2_rect *crops[SMIAPP_PADS];
> + const struct smiapp_csi_data_format *csi_format,
> + *old_csi_format = sensor->csi_format;
> + u32 code = fmt->format.code;
> + unsigned int i;
> + int rval;
>
> - mutex_lock(&sensor->mutex);
> + rval = __smiapp_get_format(subdev, fh, fmt);
> + if (rval)
> + return rval;
>
> /*
> * Media bus code is changeable on src subdev's source pad. On
> * other source pads we just get format here.
> */
> - if (fmt->pad == ssd->source_pad) {
> - u32 code = fmt->format.code;
> - int rval = __smiapp_get_format(subdev, fh, fmt);
> - bool range_changed = false;
> - unsigned int i;
> -
> - if (!rval && subdev == &sensor->src->sd) {
> - const struct smiapp_csi_data_format *csi_format =
> - smiapp_validate_csi_data_format(sensor, code);
> + if (subdev != &sensor->src->sd)
> + return 0;
>
> - if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> - if (csi_format->width !=
> - sensor->csi_format->width)
> - range_changed = true;
> + csi_format = smiapp_validate_csi_data_format(sensor, code);
>
> - sensor->csi_format = csi_format;
> - }
> + fmt->format.code = csi_format->code;
>
> - fmt->format.code = csi_format->code;
> - }
> + if (fmt->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> + return 0;
>
> - mutex_unlock(&sensor->mutex);
> - if (rval || !range_changed)
> - return rval;
> + sensor->csi_format = csi_format;
>
> + if (csi_format->width != old_csi_format->width)
> for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++)
> - v4l2_ctrl_modify_range(
> - sensor->test_data[i],
> - 0, (1 << sensor->csi_format->width) - 1, 1, 0);
> + __v4l2_ctrl_modify_range(
> + sensor->test_data[i], 0,
> + (1 << csi_format->width) - 1, 1, 0);
>
> - return 0;
> + return 0;
> +}
> +
> +static int smiapp_set_format(struct v4l2_subdev *subdev,
> + struct v4l2_subdev_fh *fh,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct smiapp_sensor *sensor = to_smiapp_sensor(subdev);
> + struct smiapp_subdev *ssd = to_smiapp_subdev(subdev);
> + struct v4l2_rect *crops[SMIAPP_PADS];
> +
> + mutex_lock(&sensor->mutex);
> +
> + if (fmt->pad == ssd->source_pad) {
> + int rval;
> +
> + rval = smiapp_set_format_source(subdev, fh, fmt);
> +
> + mutex_unlock(&sensor->mutex);
> +
> + return rval;
> }
>
> /* Sink pad. Width and height are changeable here. */
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
