On 4/12/19 10:25 AM, Boris Brezillon wrote:
> On Thu, 11 Apr 2019 12:38:06 +0200
> Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>
>>>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>> index f6d663934648..97ba365218fb 100644
>>>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>> @@ -1380,6 +1380,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>>>>> static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
>>>>> struct file *file, void *fh, void *arg)
>>>>> {
>>>>> + struct video_device *vdev = video_devdata(file);
>>>>> struct v4l2_fmtdesc *p = arg;
>>>>> int ret = check_fmt(file, p->type);
>>>>>
>>>>> @@ -1389,30 +1390,30 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
>>>>>
>>>>> switch (p->type) {
>>>>> case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>>>>> + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>>>>> + if (!!(vdev->device_caps & V4L2_CAP_VIDEO_CAPTURE_MPLANE) !=
>>>>
>>>> Hmm. I am not sure if all drivers that set V4L2_CAP_VIDEO_CAPTURE_MPLANE also
>>>> fill in vdev->device_caps. While filling in vdev->device_caps is required for
>>>> new drivers, older drivers often don't do this.
>>>>
>>>> You would have to check all drivers that set V4L2_CAP_VIDEO_CAPTURE/OUTPUT_MPLANE
>>>> to verify that they also set vdev->device_caps. I'm fairly certain the Samsung
>>>> drivers don't fill this in.
>>>
>>> I'll check that and fix those that don't set the flag. Or do you have
>>> another solution to handle that case?
>>>
>>
>> There might be other solutions as well, but regardless of that it would be a very
>> good idea for other reasons as well if all MPLANE drivers would fill in vdev->device_caps.
>
> Looks like all drivers except fimc-isp-video.c have the _MPLANE caps
> properly set/reported.
>
That's not enough, they need to fill in vdev->device_caps as well. I.e., they shouldn't
fill in device_caps and capabilities in their querycap implementation, instead they
should just set device_caps in the struct video_device. That way the V4L2 core will
know the video device capabilities since it is now stored in an internal structure.
Regards,
Hans
