[PATCH v4 3/5] media: mali-c55: Add Mali-C55 ISP driver

[Dan Scally]

Hello

On 23/05/2024 10:49, Laurent Pinchart wrote:
> On Thu, May 23, 2024 at 11:48:02AM +0200, Jacopo Mondi wrote:
>> On Thu, May 23, 2024 at 08:03:49AM GMT, Sakari Ailus wrote:
>>> Hi Daniel,
>> [snip]
>>
>>>> +
>>>> +static int mali_c55_vb2_start_streaming(struct vb2_queue *q, unsigned int count)
>>>> +{
>>>> +	struct mali_c55_cap_dev *cap_dev = q->drv_priv;
>>>> +	struct mali_c55 *mali_c55 = cap_dev->mali_c55;
>>>> +	struct mali_c55_resizer *rzr = cap_dev->rzr;
>>>> +	struct mali_c55_isp *isp = &mali_c55->isp;
>>>> +	int ret;
>>>> +
>>>> +	guard(mutex)(&isp->lock);
>>>> +
>>>> +	ret = pm_runtime_resume_and_get(mali_c55->dev);
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	ret = video_device_pipeline_start(&cap_dev->vdev,
>>>> +					  &cap_dev->mali_c55->pipe);
>>>> +	if (ret) {
>>>> +		dev_err(mali_c55->dev, "%s failed to start media pipeline\n",
>>>> +			mali_c55_cap_dev_to_name(cap_dev));
>>>> +		goto err_pm_put;
>>>> +	}
>>>> +
>>>> +	mali_c55_cap_dev_stream_enable(cap_dev);
>>>> +	mali_c55_rzr_start_stream(rzr);
>>>> +
>>>> +	/*
>>>> +	 * We only start the ISP if we're the only capture device that's
>>>> +	 * streaming. Otherwise, it'll already be active.
>>>> +	 */
>>>> +	if (mali_c55->pipe.start_count == 1) {
>>> Do you start streaming on the sensor when the first video node does?
>>>
>>> This means that frames may be lost. E.g. the IPU6 ISYS driver only starts
>>> streaming on the sensor once all video nodes of the pipeline have been
>>> started.
>> How would you ever know which nodes will be started ?
> That can be done with link setup. Any video device that has an active
> link to the ISP would need to be started.


So if you don't want to stream data from one of the two capture nodes, you'd need to disable the 
link between it and the resizer? That seems quite clunky. Does it matter if one of them starts a 
frame or two later? As opposed to both of them starting in sync a frame or two later?

>