Re: [PATCH v9 09/13] [media] exynos5-fimc-is: Add the hardware pipeline control

[Hans Verkuil]

On 09/27/2013 12:59 PM, Arun Kumar K wrote:
> This patch adds the crucial hardware pipeline control for the
> fimc-is driver. All the subdev nodes will call this pipeline
> interfaces to reach the hardware. Responsibilities of this module
> involves configuring and maintaining the hardware pipeline involving
> multiple sub-ips like ISP, DRC, Scalers, ODC, 3DNR, FD etc.
> 
> Signed-off-by: Arun Kumar K <arun...@samsung.com>
> Signed-off-by: Kilyeon Im <kilyeon...@samsung.com>
> Reviewed-by: Sylwester Nawrocki <s.nawro...@samsung.com>
> ---
>  .../media/platform/exynos5-is/fimc-is-pipeline.c   | 1708 
> ++++++++++++++++++++
>  .../media/platform/exynos5-is/fimc-is-pipeline.h   |  129 ++
>  2 files changed, 1837 insertions(+)
>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-pipeline.c
>  create mode 100644 drivers/media/platform/exynos5-is/fimc-is-pipeline.h
> 
> diff --git a/drivers/media/platform/exynos5-is/fimc-is-pipeline.c 
> b/drivers/media/platform/exynos5-is/fimc-is-pipeline.c
> new file mode 100644
> index 0000000..a73d952
> --- /dev/null
> +++ b/drivers/media/platform/exynos5-is/fimc-is-pipeline.c

<snip>

> +int fimc_is_pipeline_open(struct fimc_is_pipeline *pipeline,
> +                     struct fimc_is_sensor *sensor)
> +{
> +     struct fimc_is *is = pipeline->is;
> +     struct is_region *region;
> +     unsigned long index[2] = {0};
> +     int ret;
> +
> +     if (!sensor)
> +             return -EINVAL;
> +
> +     mutex_lock(&pipeline->pipe_lock);
> +
> +     if (test_bit(PIPELINE_OPEN, &pipeline->state)) {
> +             dev_err(pipeline->dev, "Pipeline already open\n");
> +             ret = -EINVAL;
> +             goto err_exit;
> +     }
> +
> +     pipeline->fcount = 0;
> +     pipeline->sensor = sensor;
> +
> +     if (is->num_pipelines == 0) {
> +             /* Init memory */
> +             ret = fimc_is_pipeline_initmem(pipeline);
> +             if (ret) {
> +                     dev_err(pipeline->dev, "Pipeline memory init failed\n");
> +                     goto err_exit;
> +             }
> +
> +             /* Load firmware */
> +             ret = fimc_is_pipeline_load_firmware(pipeline);
> +             if (ret) {
> +                     dev_err(pipeline->dev, "Firmware load failed\n");
> +                     goto err_fw;
> +             }
> +
> +             /* Power ON */
> +             ret = fimc_is_pipeline_power(pipeline, 1);
> +             if (ret) {
> +                     dev_err(pipeline->dev, "A5 power on failed\n");
> +                     goto err_fw;
> +             }
> +
> +             /* Wait for FW Init to complete */
> +             ret = fimc_is_itf_wait_init_state(&is->interface);
> +             if (ret) {
> +                     dev_err(pipeline->dev, "FW init failed\n");
> +                     goto err_fw;
> +             }
> +     }
> +
> +     /* Open Sensor */
> +     region = pipeline->is_region;
> +     ret = fimc_is_itf_open_sensor(&is->interface,
> +                     pipeline->instance,
> +                     sensor->drvdata->id,
> +                     sensor->i2c_bus,
> +                     pipeline->minfo->shared.paddr);
> +     if (ret) {
> +             dev_err(pipeline->dev, "Open sensor failed\n");
> +             goto err_exit;
> +     }
> +
> +     /* Load setfile */
> +     ret = fimc_is_pipeline_setfile(pipeline);
> +     if (ret)
> +             goto err_exit;
> +
> +     /* Stream off */
> +     ret = fimc_is_itf_stream_off(&is->interface, pipeline->instance);
> +     if (ret)
> +             goto err_exit;
> +
> +     /* Process off */
> +     ret = fimc_is_itf_process_off(&is->interface, pipeline->instance);
> +     if (ret)
> +             goto err_exit;
> +
> +     if (is->num_pipelines == 0) {
> +             /* Copy init params to FW region */
> +             memset(&region->parameter, 0x0, sizeof(struct is_param_region));
> +
> +             memcpy(&region->parameter.sensor, &init_sensor_param,
> +                             sizeof(struct sensor_param));

How about:

                region->parameter.sensor = init_sensor_param;

Shorter and type-safe.

Ditto for the memcpy's below.

> +             memcpy(&region->parameter.isp, &init_isp_param,
> +                             sizeof(struct isp_param));
> +             memcpy(&region->parameter.drc, &init_drc_param,
> +                             sizeof(struct drc_param));
> +             memcpy(&region->parameter.scalerc, &init_scalerc_param,
> +                             sizeof(struct scalerc_param));
> +             memcpy(&region->parameter.odc, &init_odc_param,
> +                             sizeof(struct odc_param));
> +             memcpy(&region->parameter.dis, &init_dis_param,
> +                             sizeof(struct dis_param));
> +             memcpy(&region->parameter.tdnr, &init_tdnr_param,
> +                             sizeof(struct tdnr_param));
> +             memcpy(&region->parameter.scalerp, &init_scalerp_param,
> +                             sizeof(struct scalerp_param));
> +             memcpy(&region->parameter.fd, &init_fd_param,
> +                             sizeof(struct fd_param));
> +             wmb();
> +
> +             /* Set all init params to FW */
> +             index[0] = 0xffffffff;
> +             index[1] = 0xffffffff;
> +             ret = fimc_is_itf_set_param(&is->interface, pipeline->instance,
> +                             index[0], index[1]);
> +             if (ret) {
> +                     dev_err(pipeline->dev, "%s failed\n", __func__);
> +                     return -EINVAL;
> +             }
> +     }
> +
> +     /* Set state to OPEN */
> +     set_bit(PIPELINE_OPEN, &pipeline->state);
> +     is->num_pipelines++;
> +
> +     mutex_unlock(&pipeline->pipe_lock);
> +     return 0;
> +
> +err_fw:
> +     fimc_is_pipeline_freemem(pipeline);
> +err_exit:
> +     mutex_unlock(&pipeline->pipe_lock);
> +     return ret;
> +}

Regards,

        Hans

--
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