This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

vpfe_video bug, resulting in graph not starting

Hi,

I think this can be one of the nice posts on this forum: I found a bug and fixed it, in the linux-davinci.git repo from arago, which is at the moment equal to the PSP 03.21.00.04 kernel.

I ran into the problem after converting my sensor driver to work with the Media Controller. When you call streamon on the video device after setting up the graph, vpfe_video.c  (in vpfe_pipeline_enable) walks through the graph and calls S_STREAM on each entity, using:

		subdev = media_entity_to_v4l2_subdev(entity);

		ret = v4l2_subdev_call(subdev, video, s_stream, 1);
		if (ret < 0 && ret != -ENOIOCTLCMD)
			break;

There actually is logic to cover for the case where the entity doesn't implement this IOCTL - my case - where it is fine to just continue to the next entity. However, when this entity is the last one in the graph, the variable ret will retain the -ENOIOCTLCMD value (equaling -515). In that case, there is no actual failure, yet vpfe_pipeline_enable will return a negative value and this will cause vpfe_pipeline_set_stream, and in turn vpfe_start_capture, called by the STREAMON ioctl (vpfe_streamon) to return with failure. This results in the disability to succesfully start the video streaming, dequeing of the buffers and basically one big failure.

I propose the following fix, where the return value is reset to 0 in case of an ENOIOCTLCMD.

diff --git a/drivers/media/video/davinci/vpfe_video.c b/drivers/media/video/davinci/vpfe_video.c
index 9555c93..ddb5429 100644
--- a/drivers/media/video/davinci/vpfe_video.c
+++ b/drivers/media/video/davinci/vpfe_video.c
@@ -337,6 +337,8 @@ static int vpfe_pipeline_enable(struct vpfe_video_device *video,
                ret = v4l2_subdev_call(subdev, video, s_stream, 1);
                if (ret < 0 && ret != -ENOIOCTLCMD)
                        break;
+               else if (ret == -ENOIOCTLCMD)
+                   ret = 0;
 
        }
Is this the right place to post such proposals, and if so, can it be considered?
Thanks
Jasper
  • Jasper,

    With the above patch, there is no way to know whether stream has started on the subdev which returned -ENOIOCTLCMD. I think it is better to implement streamon(even if its dummy) operation in subdev than what is proposed in the above patch.