Re: [PATCH v3 2/5] media: qcom: camss: vfe-17x: Fix write master selection for RDI lines

[Loic Poulain]

On Mon, Sep 28, 2026 at 8:43 AM Hitesh Patel <hitesh@xxxxxxxxxxxxxx> wrote:
>
> On the gen2 VFE bus there is no crossbar between the RDI paths and
> the bus write masters: RDI n is served by write master n. The common
> vfe_get_output_v2() reflects this by mapping line->id to write master
> line->id, and every gen2 VFE except 17x uses it.
>
> The 17x code kept its own vfe_get_output(), which reserves whichever
> write master is free first. That happens to be the right one as long
> as a single line streams and it is RDI0. As soon as two lines of the
> same VFE stream, for instance two virtual channels demultiplexed by
> the CSID to RDI0 and RDI1, the second line to start is given the
> write master of the other RDI. Both write masters are then programmed
> with the buffer address and frame size of the wrong line. The frames
> are truncated to the smaller of the two buffers and the SMMU faults
> on the larger one.
>
> Use vfe_get_output_v2() on 17x like the other gen2 VFEs and drop the
> duplicate. This also removes an error path that released
> output->wm_idx[0] before it had been assigned.
>
> The gen1 VFEs are not affected: they have a bus crossbar and any write
> master can serve any line, so vfe_reserve_wm() is correct there.
>
> Fixes: 7319cdf189bb ("media: camss: Add support for VFE hardware version Titan 170")
> Signed-off-by: Hitesh Patel <hitesh@xxxxxxxxxxxxxx>

Reviewed-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>

> ---
>  .../media/platform/qcom/camss/camss-vfe-17x.c | 41 +------------------
>  1 file changed, 1 insertion(+), 40 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> index c011f64f63..62c2405359 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
> @@ -382,45 +382,6 @@ static int vfe_halt(struct vfe_device *vfe)
>         return 0;
>  }
>
> -static int vfe_get_output(struct vfe_line *line)
> -{
> -       struct vfe_device *vfe = to_vfe(line);
> -       struct vfe_output *output;
> -       unsigned long flags;
> -       int wm_idx;
> -
> -       spin_lock_irqsave(&vfe->output_lock, flags);
> -
> -       output = &line->output;
> -       if (output->state > VFE_OUTPUT_RESERVED) {
> -               dev_err(vfe->camss->dev, "Output is running\n");
> -               goto error;
> -       }
> -
> -       output->wm_num = 1;
> -
> -       wm_idx = vfe_reserve_wm(vfe, line->id);
> -       if (wm_idx < 0) {
> -               dev_err(vfe->camss->dev, "Can not reserve wm\n");
> -               goto error_get_wm;
> -       }
> -       output->wm_idx[0] = wm_idx;
> -
> -       output->drop_update_idx = 0;
> -
> -       spin_unlock_irqrestore(&vfe->output_lock, flags);
> -
> -       return 0;
> -
> -error_get_wm:
> -       vfe_release_wm(vfe, output->wm_idx[0]);
> -       output->state = VFE_OUTPUT_OFF;
> -error:
> -       spin_unlock_irqrestore(&vfe->output_lock, flags);
> -
> -       return -EINVAL;
> -}
> -
>  /*
>   * vfe_enable - Enable streaming on VFE line
>   * @line: VFE line
> @@ -441,7 +402,7 @@ static int vfe_enable(struct vfe_line *line)
>
>         mutex_unlock(&vfe->stream_lock);
>
> -       ret = vfe_get_output(line);
> +       ret = vfe_get_output_v2(line);
>         if (ret < 0)
>                 goto error_get_output;
>
> --
> 2.43.0
>