On 6/6/19 9:09 AM, Boris Brezillon wrote:
> On Thu, 6 Jun 2019 08:51:59 +0200
> Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote:
>
>> On Thu, 6 Jun 2019 12:53:57 +0900
>> Tomasz Figa <tfiga@xxxxxxxxxxxx> wrote:
>>
>>> On Thu, Jun 6, 2019 at 1:46 AM Boris Brezillon
>>> <boris.brezillon@xxxxxxxxxxxxx> wrote:
>>>>
>>>> CAP_M2M_MPLANE means the device supports _MPLANE formats for both
>>>> capture and output. Adjust the check to avoid EINVAL errors on
>>>> such devices.
>>>>
>>>> Fixes: 366c719d6479 ("media: v4l2: Get rid of ->vidioc_enum_fmt_vid_{cap,out}_mplane")
>>>> Reported-by: Maxime Jourdan <mjourdan@xxxxxxxxxxxx>
>>>> Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
>>>> ---
>>>> drivers/media/v4l2-core/v4l2-ioctl.c | 9 +++++++--
>>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> index b4c73e8f23c5..ace9b9761bed 100644
>>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> @@ -1385,6 +1385,7 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
>>>> struct video_device *vdev = video_devdata(file);
>>>> struct v4l2_fmtdesc *p = arg;
>>>> int ret = check_fmt(file, p->type);
>>>> + u32 cap_mask;
>>>>
>>>> if (ret)
>>>> return ret;
>>>> @@ -1393,7 +1394,9 @@ 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) !=
>>>> + cap_mask = V4L2_CAP_VIDEO_CAPTURE_MPLANE |
>>>> + V4L2_CAP_VIDEO_M2M_MPLANE;
>>>> + if (!!(vdev->device_caps & cap_mask) !=
>>>
>>> Shouldn't devices that report V4L2_CAP_VIDEO_M2M_MPLANE have also
>>> V4L2_CAP_VIDEO_CAPTURE_MPLANE and V4L2_CAP_VIDEO_OUTPUT_MPLANE
>>> reported anyway?
>>
>> That's the other option, force drivers that set
>> V4L2_CAP_VIDEO_M2M_MPLANE to also set
>> V4L2_CAP_VIDEO_{CAPTURE,OUTPUT}_MPLANE (or we can let the core do it).
>
> That would basically look like this. Hans, let me know if you want me
> to send a new version with the below diff.
No, I don't want that :-)
See my earlier reply for the reason.
Regards,
Hans
>
> --->8---
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 29946a2b2752..9cb782c62f69 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -933,6 +933,14 @@ int __video_register_device(struct video_device *vdev,
> }
> #endif
>
> + /*
> + * Adjust ->device_caps: CAP_M2M_MPLANE implies CAP_CAPTURE_MPLANES and
> + * CAP_OUTPUT_MLANES.
> + */
> + if (vdev->device_caps & V4L2_CAP_VIDEO_M2M_MPLANE)
> + vdev->device_caps |= V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> + V4L2_CAP_VIDEO_OUTPUT_MPLANE;
> +
> /* Pick a device node number */
> mutex_lock(&videodev_lock);
> nr = devnode_find(vdev, nr == -1 ? 0 : nr, minor_cnt);
>
