Hi Sakari,
Thanks for the patch.
On Tuesday 06 March 2012 18:33:15 Sakari Ailus wrote:
> From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx>
>
> Calculate PLL configuration based on input data: sensor configuration, board
> properties and sensor-specific limits.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx>
[snip]
> diff --git a/drivers/media/video/smiapp-pll.c
> b/drivers/media/video/smiapp-pll.c new file mode 100644
> index 0000000..3a999c0
> --- /dev/null
> +++ b/drivers/media/video/smiapp-pll.c
[snip]
> +int smiapp_pll_calculate(struct device *dev, struct smiapp_pll_limits
> *limits,
> + struct smiapp_pll *pll)
> +{
[snip]
> + /*
> + * Some sensors perform analogue binning and some do this
> + * digitally. The ones doing this digitally can be roughly be
> + * found out using this formula. The ones doing this digitally
> + * should run at higher clock rate, so smaller divisor is used
> + * on video timing side.
> + */
> + if (limits->min_line_length_pck_bin > limits->min_line_length_pck
> + / pll->binning_horizontal)
> + vt_op_binning_div = pll->binning_horizontal;
> + else
> + vt_op_binning_div = 1;
> + dev_dbg(dev, "vt_op_binning_div: %d\n", vt_op_binning_div);
> +
> + /*
> + * Profile 2 supports vt_pix_clk_div E [4, 10]
> + *
> + * Horizontal binning can be used as a base for difference in
> + * divisors. One must make sure that horizontal blanking is
> + * enough to accommodate the CSI-2 sync codes.
> + *
> + * Take scaling factor into account as well.
> + *
> + * Find absolute limits for the factor of vt divider.
> + */
> + dev_dbg(dev, "scale_m: %d\n", pll->scale_m);
> + min_vt_div = DIV_ROUND_UP(pll->op_pix_clk_div * pll->op_sys_clk_div
> + * pll->scale_n,
I still need
if (pll->flags & SMIAPP_PLL_FLAG_DUAL_READOUT)
num_readout_paths = 2;
else
num_readout_paths = 1;
min_vt_div = DIV_ROUND_UP(pll->op_pix_clk_div * pll->op_sys_clk_div
* pll->scale_n * num_readout_paths,
for the AR0330 driver, but I can add that later.
> + lane_op_clock_ratio * vt_op_binning_div
> + * pll->scale_m);
[snip]
> + /*
> + * Find pix_div such that a legal pix_div * sys_div results
> + * into a value which is not smaller than div, the desired
> + * divisor.
> + */
> + for (vt_div = min_vt_div; vt_div <= max_vt_div;
> + vt_div += 2 - (vt_div & 1)) {
> + for (sys_div = min_sys_div;
> + sys_div <= max_sys_div;
> + sys_div += 2 - (sys_div & 1)) {
> + int pix_div = DIV_ROUND_UP(vt_div, sys_div);
> +
> + if (pix_div <
> + limits->min_vt_pix_clk_div
> + || pix_div
> + > limits->max_vt_pix_clk_div) {
if (pix_div < limits->min_vt_pix_clk_div ||
pix_div > limits->max_vt_pix_clk_div) {
> + dev_dbg(dev,
> + "pix_div %d too small or too big (%d--%d)\n",
> + pix_div,
> + limits->min_vt_pix_clk_div,
> + limits->max_vt_pix_clk_div);
> + continue;
> + }
> +
> + /* Check if this one is better. */
> + if (pix_div * sys_div
> + <= ALIGN(min_vt_div, best_pix_div))
> + best_pix_div = pix_div;
> + }
> + if (best_pix_div < INT_MAX >> 1)
> + break;
> + }
[snip]
> diff --git a/drivers/media/video/smiapp/smiapp-core.c
> b/drivers/media/video/smiapp/smiapp-core.c new file mode 100644
> index 0000000..879fb1e
> --- /dev/null
> +++ b/drivers/media/video/smiapp/smiapp-core.c
[snip]
> +static void smiapp_get_crop_compose(struct v4l2_subdev *subdev,
> + struct v4l2_subdev_fh *fh,
> + struct v4l2_rect **crops,
> + struct v4l2_rect **comps, int which)
> +{
> + struct smiapp_subdev *ssd = to_smiapp_subdev(subdev);
> + int i;
Can you guess ? :-)
> +
> + if (which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> + if (crops)
> + for (i = 0; i < subdev->entity.num_pads; i++)
> + crops[i] = &ssd->crop[i];
> + if (comps)
> + *comps = &ssd->compose;
> + } else {
> + if (crops) {
> + for (i = 0; i < subdev->entity.num_pads; i++) {
> + crops[i] = v4l2_subdev_get_try_crop(fh, i);
> + BUG_ON(!crops[i]);
> + }
> + }
> + if (comps) {
> + *comps = v4l2_subdev_get_try_compose(fh,
> + SMIAPP_PAD_SINK);
> + BUG_ON(!*comps);
> + }
> + }
> +}
[snip]
> +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];
> + const struct smiapp_csi_data_format *csi_format = sensor->csi_format;
> + int i;
:-D
> +
> + mutex_lock(&sensor->mutex);
> +
> + smiapp_get_crop_compose(subdev, fh, crops, NULL, fmt->which);
> +
> + if (subdev == &sensor->src->sd && fmt->pad == SMIAPP_PAD_SRC) {
> + for (i = 0; i < ARRAY_SIZE(smiapp_csi_data_formats); i++) {
> + if (sensor->mbus_frame_fmts & (1 << i) &&
> + smiapp_csi_data_formats[i].code
> + == fmt->format.code) {
> + csi_format = &smiapp_csi_data_formats[i];
> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> + sensor->csi_format = csi_format;
> + break;
> + }
> + }
> + }
> +
> + if (fmt->pad == ssd->source_pad) {
> + int rval;
> +
> + rval = __smiapp_get_format(subdev, fh, fmt);
> + fmt->format.code = csi_format->code;
> +
> + mutex_unlock(&sensor->mutex);
> + return rval;
> + }
> +
> + fmt->format.code = csi_format->code;
This will result in the format code at the output of the pixel array being
equal to the format code at the output of the binner/scaler. I don't think
you want that.
You probably need to take a small step back and rethink the code flow in this
function a bit. I'm pretty sure you can keep it simple.
> + fmt->format.width &= ~1;
> + fmt->format.height &= ~1;
> +
> + fmt->format.width =
> + clamp(fmt->format.width,
> + sensor->limits[SMIAPP_LIMIT_MIN_X_OUTPUT_SIZE],
> + sensor->limits[SMIAPP_LIMIT_MAX_X_OUTPUT_SIZE]);
> + fmt->format.height =
> + clamp(fmt->format.height,
> + sensor->limits[SMIAPP_LIMIT_MIN_Y_OUTPUT_SIZE],
> + sensor->limits[SMIAPP_LIMIT_MAX_Y_OUTPUT_SIZE]);
> +
> + crops[ssd->sink_pad]->left = 0;
> + crops[ssd->sink_pad]->top = 0;
> + crops[ssd->sink_pad]->width = fmt->format.width;
> + crops[ssd->sink_pad]->height = fmt->format.height;
> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> + ssd->sink_fmt = *crops[ssd->sink_pad];
> + smiapp_propagate(subdev, fh, fmt->which,
> + V4L2_SUBDEV_SEL_TGT_CROP_ACTIVE);
> +
> + mutex_unlock(&sensor->mutex);
> +
> + return 0;
> +}
[snip]
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
