Re: [PATCH 30/74] media: qcom: camss: vfe: Add support for starting multiple write-masters in one output's group

[Gjorgji Rosikopulos (Consultant)]

Hi Bryan,

On 10/5/2026 8:14 PM, Bryan O'Donoghue wrote:
> Signed-off-by: Bryan O'Donoghue <bod@xxxxxxxxxx>
> ---
>  drivers/media/platform/qcom/camss/camss-vfe.c | 63 +++++++++++++++++----------
>  1 file changed, 40 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index 3fe0139f82d16..c078af53b0758 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -642,36 +642,16 @@ void vfe_buf_done(struct vfe_device *vfe, int wm)
>  	spin_unlock_irqrestore(&vfe->output_lock, flags);
>  }
>  
> -int vfe_enable_output_v2(struct vfe_line *line)
> +static int vfe_enable_one_output(struct vfe_line *line, struct vfe_output *output)
>  {
>  	struct vfe_device *vfe = to_vfe(line);
> -	struct vfe_output *output = &line->output[0];
>  	const struct vfe_hw_ops *ops = vfe->res->hw_ops;
> -	struct media_pad *sensor_pad;
> -	unsigned long flags;
> -	unsigned int frame_skip = 0;
>  	unsigned int i;
>  
> -	sensor_pad = camss_find_sensor_pad(&line->subdev.entity);
> -	if (sensor_pad) {
> -		struct v4l2_subdev *subdev =
> -			media_entity_to_v4l2_subdev(sensor_pad->entity);
> -
> -		v4l2_subdev_call(subdev, sensor, g_skip_frames, &frame_skip);
> -		/* Max frame skip is 29 frames */
> -		if (frame_skip > VFE_FRAME_DROP_VAL - 1)
> -			frame_skip = VFE_FRAME_DROP_VAL - 1;
> -	}
> -
> -	spin_lock_irqsave(&vfe->output_lock, flags);
> -
> -	ops->reg_update_clear(vfe, line->id);
> -
>  	if (output->state > VFE_OUTPUT_RESERVED) {
>  		dev_err(vfe->camss->dev,
>  			"Output is not in reserved state %d\n",
>  			output->state);
> -		spin_unlock_irqrestore(&vfe->output_lock, flags);
>  		return -EINVAL;
>  	}
>  
> @@ -683,7 +663,10 @@ int vfe_enable_output_v2(struct vfe_line *line)
>  	output->wait_reg_update = 0;
>  	reinit_completion(&output->reg_update);
>  
> -	ops->vfe_wm_start(vfe, output->wm[0].bus_client, line);
> +	if (ops->vfe_output_start)
> +		ops->vfe_output_start(vfe, output);
> +	else
> +		ops->vfe_wm_start(vfe, output->wm[0].bus_client, line);
>  

Is really confusing we have new API but just start_xx has guard whether that API is used,
Not to mention v2 and other vX versions of the functions which is difficult to track. Can we
abstract them to some kind of ops and use one or other API so will be easy for the new platforms
to be integrated. Honestly is really difficult for me to read and trace the code in this file,
but since you like it a lot and keep on updating i don not have match choice than to work with it :-).

~Gjorgji