Hi Niklas,
On Tue, Apr 13, 2021 at 08:02:53PM +0200, Niklas Söderlund wrote:
> Add support for the R-Car V3U (r8a779a0) to the driver. The V3U have the
has
> VIN modules connected to a ISP instead of directly to the R-Car CSI-2
> receivers. The reason being that the ISP performs channel selection
> based on CSI-2 VC/DT pairs and routes the video data to different VIN
> modules. Other SoC versions this filtering is done by the VIN modules
"In other SoC versions". Or "For"...
> themself.
>
> While the media graph very different from other SoCs the only difference
the media graph is
> in operating the VIN modules is that the VC/DT filtering should be
> skipped as that is performed by the ISP.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> ---
> drivers/media/platform/rcar-vin/rcar-core.c | 102 +++++++++++++++++++-
> drivers/media/platform/rcar-vin/rcar-dma.c | 20 ++--
> drivers/media/platform/rcar-vin/rcar-vin.h | 15 ++-
> 3 files changed, 124 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 763be02f507e6f3f..98326e80d2ed23d2 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -1006,6 +1006,87 @@ static int rvin_csi2_init(struct rvin_dev *vin)
> return ret;
> }
>
> +/* -----------------------------------------------------------------------------
> + * ISP
> + */
> +
> +static void rvin_isp_setup_links(struct rvin_dev *vin)
> +{
> + unsigned int i;
> +
> + /* Create all media device links between VINs and ISP's. */
> + mutex_lock(&vin->group->lock);
> + for (i = 0; i < RCAR_VIN_NUM; i++) {
> + struct media_pad *source_pad, *sink_pad;
> + struct media_entity *source, *sink;
> + unsigned int source_slot = i / 8;
> + unsigned int source_idx = i % 8 + 1;
> +
> + if (!vin->group->vin[i])
> + continue;
> +
> + /* Check that ISP is part of the group. */
> + if (!vin->group->remotes[source_slot].subdev)
> + continue;
> +
> + source = &vin->group->remotes[source_slot].subdev->entity;
> + source_pad = &source->pads[source_idx];
> +
> + sink = &vin->group->vin[i]->vdev.entity;
> + sink_pad = &sink->pads[0];
> +
> + /* Skip if link already exists. */
> + if (media_entity_find_link(source_pad, sink_pad))
> + continue;
> +
> + if (media_create_pad_link(source, source_idx, sink, 0,
> + MEDIA_LNK_FL_ENABLED |
> + MEDIA_LNK_FL_IMMUTABLE)) {
> + vin_err(vin, "Error adding link from %s to %s\n",
> + source->name, sink->name);
Same question about the CSI-2 version, is is ok not reporting errors ?
> + break;
> + }
> + }
> + mutex_unlock(&vin->group->lock);
> +}
> +
> +static void rvin_isp_cleanup(struct rvin_dev *vin)
> +{
> + rvin_group_notifier_cleanup(vin);
> + rvin_group_put(vin);
> + rvin_free_controls(vin);
> +}
> +
> +static int rvin_isp_init(struct rvin_dev *vin)
> +{
> + int ret;
> +
> + vin->pad.flags = MEDIA_PAD_FL_SINK;
> + ret = media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
> + if (ret)
> + return ret;
> +
> + ret = rvin_create_controls(vin, NULL);
> + if (ret < 0)
> + return ret;
> +
> + ret = rvin_group_get(vin, rvin_isp_setup_links, NULL);
> + if (ret)
> + goto err_controls;
> +
> + ret = rvin_group_notifier_init(vin, 2, RVIN_ISP_MAX);
> + if (ret)
> + goto err_group;
> +
> + return 0;
> +err_group:
> + rvin_group_put(vin);
> +err_controls:
> + rvin_free_controls(vin);
> +
> + return ret;
> +}
> +
> /* -----------------------------------------------------------------------------
> * Suspend / Resume
> */
> @@ -1378,6 +1459,15 @@ static const struct rvin_info rcar_info_r8a77995 = {
> .routes = rcar_info_r8a77995_routes,
> };
>
> +static const struct rvin_info rcar_info_r8a779a0 = {
> + .model = RCAR_GEN3,
> + .use_mc = true,
> + .use_isp = true,
> + .nv12 = true,
> + .max_width = 4096,
> + .max_height = 4096,
> +};
> +
> static const struct of_device_id rvin_of_id_table[] = {
> {
> .compatible = "renesas,vin-r8a774a1",
> @@ -1435,6 +1525,10 @@ static const struct of_device_id rvin_of_id_table[] = {
> .compatible = "renesas,vin-r8a77995",
> .data = &rcar_info_r8a77995,
> },
> + {
> + .compatible = "renesas,vin-r8a779a0",
> + .data = &rcar_info_r8a779a0,
> + },
> { /* Sentinel */ },
> };
> MODULE_DEVICE_TABLE(of, rvin_of_id_table);
> @@ -1483,7 +1577,9 @@ static int rcar_vin_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, vin);
>
> - if (vin->info->use_mc)
> + if (vin->info->use_isp)
> + ret = rvin_isp_init(vin);
> + else if (vin->info->use_mc)
> ret = rvin_csi2_init(vin);
> else
> ret = rvin_parallel_init(vin);
> @@ -1507,7 +1603,9 @@ static int rcar_vin_remove(struct platform_device *pdev)
>
> rvin_v4l2_unregister(vin);
>
> - if (vin->info->use_mc)
> + if (vin->info->use_isp)
> + rvin_isp_cleanup(vin);
> + else if (vin->info->use_mc)
> rvin_csi2_cleanup(vin);
> else
> rvin_parallel_cleanup(vin);
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index f30dafbdf61ca15f..24b3a51e524fcc72 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -783,16 +783,18 @@ static int rvin_setup(struct rvin_dev *vin)
> /* Always update on field change */
> vnmc |= VNMC_VUP;
>
> - /* If input and output use the same colorspace, use bypass mode */
> - if (input_is_yuv == output_is_yuv)
> - vnmc |= VNMC_BPS;
> + if (!vin->info->use_isp) {
> + /* If input and output use the same colorspace, use bypass mode */
> + if (input_is_yuv == output_is_yuv)
> + vnmc |= VNMC_BPS;
>
> - if (vin->info->model == RCAR_GEN3) {
> - /* Select between CSI-2 and parallel input */
> - if (vin->is_csi)
> - vnmc &= ~VNMC_DPINE;
> - else
> - vnmc |= VNMC_DPINE;
> + if (vin->info->model == RCAR_GEN3) {
> + /* Select between CSI-2 and parallel input */
> + if (vin->is_csi)
> + vnmc &= ~VNMC_DPINE;
> + else
> + vnmc |= VNMC_DPINE;
> + }
> }
>
> /* Progressive or interlaced mode */
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index f2c32d5ceb7f83d9..88728ebe470ecafd 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -29,7 +29,7 @@
> #define HW_BUFFER_MASK 0x7f
>
> /* Max number on VIN instances that can be in a system */
> -#define RCAR_VIN_NUM 8
> +#define RCAR_VIN_NUM 32
>
> struct rvin_group;
>
> @@ -48,7 +48,17 @@ enum rvin_csi_id {
> RVIN_CSI_MAX,
> };
>
> -#define RVIN_REMOTES_MAX RVIN_CSI_MAX
> +enum rvin_isp_id {
> + RVIN_ISP0,
> + RVIN_ISP1,
> + RVIN_ISP2,
> + RVIN_ISP4,
> + RVIN_ISP_MAX,
> +};
Be aware this seems not to be used
> +
> +#define RVIN_REMOTES_MAX \
> + (((unsigned int)RVIN_CSI_MAX) > ((unsigned int)RVIN_ISP_MAX) ? \
> + RVIN_CSI_MAX : RVIN_ISP_MAX)
And I assume this is here in order to prepare for the introduction of
new CSI-2 interfaces or new ISPs
The way how easy it looks plumbing ISP support is means you did a
great job paving the way for that to happen in the previous patches!
Really good job!
Reviewed-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
Thanks
j
>
> /**
> * enum rvin_dma_state - DMA states
> @@ -158,6 +168,7 @@ struct rvin_group_route {
> struct rvin_info {
> enum model_id model;
> bool use_mc;
> + bool use_isp;
> bool nv12;
>
> unsigned int max_width;
> --
> 2.31.1
>
