Re: [PATCH RFC 3/3] s5p-fimc: improved pipeline try format routine

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

Hi Sakari,

Thank you for the comments.

On 12/05/2012 12:22 AM, Sakari Ailus wrote:
>> diff --git a/drivers/media/platform/s5p-fimc/fimc-capture.c b/drivers/media/platform/s5p-fimc/fimc-capture.c
>> index 3acbea3..39c4555 100644
>> --- a/drivers/media/platform/s5p-fimc/fimc-capture.c
>> +++ b/drivers/media/platform/s5p-fimc/fimc-capture.c
>> @@ -794,6 +794,21 @@ static int fimc_cap_enum_fmt_mplane(struct file *file, void *priv,
>>  	return 0;
>>  }
>>  
>> +static struct media_entity *fimc_pipeline_get_head(struct media_entity *me)
>> +{
>> +	struct media_pad *pad = &me->pads[0];
>> +
>> +	while (!(pad->flags & MEDIA_PAD_FL_SOURCE)) {
>> +		pad = media_entity_remote_source(pad);
>> +		if (!pad)
>> +			break;
> 
> Isn't it an error if a sink pad of the entity isn't connected?
> media_entity_remote_source(pad) returns NULL if the link is disabled. I'm
> just wondering if this is possible.

It is not possible to not have all links connected, from video device to
the sensor subdev entity, at the point when this function is called
(from within VIDIOC_TRY_FMT or VIDIOC_S_FMT ioctls). fimc_pipeline_prepare()
walks the graph in direction from video node to the sensor, also using
media_entity_remote_source(). If it doesn't reach the sensor entity and
save a pointer to it for further reference the video node open() will fail.
And there won't be even a chance to invoke VIDIOC_TRY_FMT/VIDIOC_S_FMT 
ioctls. It's true the above function takes some assumptions that could be
explained with a relevant comment.

It's worth to note that all this in-driver setting of format at the 
pipeline is for static default links created by the driver. If userspace 
reconfigures links it should not rely on it in first place. When we finally 
prepare a libv4l support for this driver it should just go away. Regular 
V4L2 applications must use libv4l2 anyway since the driver supports only 
multi-planar API.

--

Regards,
Sylwester