From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Sakari Ailus <sakari.ailus@iki.fi>
Cc: linux-media@vger.kernel.org
Subject: Re: [PATCH v2.1 16/18] smiapp: Clean up smiapp_set_format()
Date: Thu, 02 Oct 2014 16:53:02 +0300 [thread overview]
Message-ID: <2345405.asIYuPqanQ@avalon> (raw)
In-Reply-To: <1412251754-9061-1-git-send-email-sakari.ailus@iki.fi>
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.ailus@linux.intel.com>
For the whole series,
Acked-by: Laurent Pinchart <laurent.pinchart@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
next prev parent reply other threads:[~2014-10-02 13:53 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-02 8:45 [PATCH v2] smiapp and smiapp-pll: more robust parameter handling, cleanups Sakari Ailus
2014-10-02 8:45 ` [PATCH v2 01/18] smiapp: Take mutex during PLL update in sensor initialisation Sakari Ailus
2014-10-02 8:45 ` [PATCH v2 02/18] smiapp-pll: Correct clock debug prints Sakari Ailus
2014-10-02 8:45 ` [PATCH v2 03/18] smiapp-pll: The clock tree values are unsigned --- fix " Sakari Ailus
2014-10-02 8:45 ` [PATCH v2 04/18] smiapp-pll: Separate bounds checking into a separate function Sakari Ailus
2014-10-02 8:45 ` [PATCH v2 05/18] smiapp-pll: External clock frequency isn't an output value Sakari Ailus
2014-10-02 8:45 ` [PATCH v2 06/18] smiapp-pll: Unify OP and VT PLL structs Sakari Ailus
2014-10-02 8:45 ` [PATCH v2 07/18] smiapp-pll: Calculate OP clocks only for sensors that have them Sakari Ailus
2014-10-02 8:45 ` [PATCH v2 08/18] smiapp-pll: Don't validate OP clocks if there are none Sakari Ailus
2014-10-02 8:45 ` [PATCH v2 09/18] smiapp: The PLL calculator handles sensors with VT clocks only Sakari Ailus
2014-10-02 8:46 ` [PATCH v2 10/18] smiapp: Remove validation of op_pix_clk_div Sakari Ailus
2014-10-02 8:46 ` [PATCH v2 11/18] smiapp-pll: Add pixel rate in pixel array as output parameters Sakari Ailus
2014-10-02 8:46 ` [PATCH v2 12/18] smiapp: Use actual pixel rate calculated by the PLL calculator Sakari Ailus
2014-10-02 8:46 ` [PATCH v2 13/18] smiapp: Split calculating PLL with sensor's limits from updating it Sakari Ailus
2014-10-02 8:46 ` [PATCH v2 14/18] smiapp: Gather information on valid link rate and BPP combinations Sakari Ailus
2014-10-02 8:46 ` [PATCH v2 15/18] smiapp: Take valid link frequencies into account in supported mbus codes Sakari Ailus
2014-10-02 8:46 ` [PATCH v2 16/18] smiapp: Clean up smiapp_set_format() Sakari Ailus
2014-10-02 12:09 ` [PATCH v2.1 " Sakari Ailus
2014-10-02 13:53 ` Laurent Pinchart [this message]
2014-10-02 8:46 ` [PATCH v2 17/18] smiapp: Set valid link frequency range Sakari Ailus
2014-10-02 8:46 ` [PATCH v2 18/18] smiapp: Update PLL when setting format Sakari Ailus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2345405.asIYuPqanQ@avalon \
--to=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=sakari.ailus@iki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).