diff mbox

media: video-mux: fix compliance failures

Message ID 20180522162925.16854-1-p.zabel@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Philipp Zabel May 22, 2018, 4:29 p.m. UTC 
Limit frame sizes to the [1, UINT_MAX-1] interval, media bus formats to
the available list of formats, and initialize pad and try formats.

Reported-by: Rui Miguel Silva <rui.silva@linaro.org>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/video-mux.c | 110 +++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

Comments

Hans Verkuil May 22, 2018, 5:47 p.m. UTC | #1 
On 22/05/18 18:29, Philipp Zabel wrote:
> Limit frame sizes to the [1, UINT_MAX-1] interval, media bus formats to
> the available list of formats, and initialize pad and try formats.
> 
> Reported-by: Rui Miguel Silva <rui.silva@linaro.org>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/platform/video-mux.c | 110 +++++++++++++++++++++++++++++
>  1 file changed, 110 insertions(+)
> 
> diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
> index 1fb887293337..ade1dae706aa 100644
> --- a/drivers/media/platform/video-mux.c
> +++ b/drivers/media/platform/video-mux.c
> @@ -180,6 +180,87 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
>  	if (!source_mbusformat)
>  		return -EINVAL;
>  
> +	/* No size limitations except V4L2 compliance requirements */
> +	v4l_bound_align_image(&sdformat->format.width, 1, UINT_MAX - 1, 0,
> +			      &sdformat->format.height, 1, UINT_MAX - 1, 0, 0);

This is a bit dubious. I would pick more realistic min/max values like 16 and
65536. UINT_MAX - 1 will overflow whenever code increments/multiplies it for some
reason, which can cause all sorts of weird issues.

> +
> +	/* All formats except LVDS and vendor specific formats are acceptable */
> +	switch (sdformat->format.code) {
> +	case MEDIA_BUS_FMT_RGB444_1X12:
> +	case MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE:
> +	case MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE:
> +	case MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE:
> +	case MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE:
> +	case MEDIA_BUS_FMT_RGB565_1X16:
> +	case MEDIA_BUS_FMT_BGR565_2X8_BE:
> +	case MEDIA_BUS_FMT_BGR565_2X8_LE:
> +	case MEDIA_BUS_FMT_RGB565_2X8_BE:
> +	case MEDIA_BUS_FMT_RGB565_2X8_LE:
> +	case MEDIA_BUS_FMT_RGB666_1X18:
> +	case MEDIA_BUS_FMT_RBG888_1X24:
> +	case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
> +	case MEDIA_BUS_FMT_BGR888_1X24:
> +	case MEDIA_BUS_FMT_GBR888_1X24:
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +	case MEDIA_BUS_FMT_RGB888_2X12_BE:
> +	case MEDIA_BUS_FMT_RGB888_2X12_LE:
> +	case MEDIA_BUS_FMT_ARGB8888_1X32:
> +	case MEDIA_BUS_FMT_RGB888_1X32_PADHI:
> +	case MEDIA_BUS_FMT_RGB101010_1X30:
> +	case MEDIA_BUS_FMT_RGB121212_1X36:
> +	case MEDIA_BUS_FMT_RGB161616_1X48:
> +	case MEDIA_BUS_FMT_Y8_1X8:
> +	case MEDIA_BUS_FMT_UV8_1X8:
> +	case MEDIA_BUS_FMT_UYVY8_1_5X8:
> +	case MEDIA_BUS_FMT_VYUY8_1_5X8:
> +	case MEDIA_BUS_FMT_YUYV8_1_5X8:
> +	case MEDIA_BUS_FMT_YVYU8_1_5X8:
> +	case MEDIA_BUS_FMT_UYVY8_2X8:
> +	case MEDIA_BUS_FMT_VYUY8_2X8:
> +	case MEDIA_BUS_FMT_YUYV8_2X8:
> +	case MEDIA_BUS_FMT_YVYU8_2X8:
> +	case MEDIA_BUS_FMT_Y10_1X10:
> +	case MEDIA_BUS_FMT_UYVY10_2X10:
> +	case MEDIA_BUS_FMT_VYUY10_2X10:
> +	case MEDIA_BUS_FMT_YUYV10_2X10:
> +	case MEDIA_BUS_FMT_YVYU10_2X10:
> +	case MEDIA_BUS_FMT_Y12_1X12:
> +	case MEDIA_BUS_FMT_UYVY12_2X12:
> +	case MEDIA_BUS_FMT_VYUY12_2X12:
> +	case MEDIA_BUS_FMT_YUYV12_2X12:
> +	case MEDIA_BUS_FMT_YVYU12_2X12:
> +	case MEDIA_BUS_FMT_UYVY8_1X16:
> +	case MEDIA_BUS_FMT_VYUY8_1X16:
> +	case MEDIA_BUS_FMT_YUYV8_1X16:
> +	case MEDIA_BUS_FMT_YVYU8_1X16:
> +	case MEDIA_BUS_FMT_YDYUYDYV8_1X16:
> +	case MEDIA_BUS_FMT_UYVY10_1X20:
> +	case MEDIA_BUS_FMT_VYUY10_1X20:
> +	case MEDIA_BUS_FMT_YUYV10_1X20:
> +	case MEDIA_BUS_FMT_YVYU10_1X20:
> +	case MEDIA_BUS_FMT_VUY8_1X24:
> +	case MEDIA_BUS_FMT_YUV8_1X24:
> +	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
> +	case MEDIA_BUS_FMT_UYVY12_1X24:
> +	case MEDIA_BUS_FMT_VYUY12_1X24:
> +	case MEDIA_BUS_FMT_YUYV12_1X24:
> +	case MEDIA_BUS_FMT_YVYU12_1X24:
> +	case MEDIA_BUS_FMT_YUV10_1X30:
> +	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
> +	case MEDIA_BUS_FMT_AYUV8_1X32:
> +	case MEDIA_BUS_FMT_UYYVYY12_0_5X36:
> +	case MEDIA_BUS_FMT_YUV12_1X36:
> +	case MEDIA_BUS_FMT_YUV16_1X48:
> +	case MEDIA_BUS_FMT_UYYVYY16_0_5X48:
> +	case MEDIA_BUS_FMT_JPEG_1X8:
> +	case MEDIA_BUS_FMT_AHSV8888_1X32:
> +		break;
> +	default:
> +		sdformat->format.code = MEDIA_BUS_FMT_Y8_1X8;

Add a break here.

> +	}
> +	if (sdformat->format.field == V4L2_FIELD_ANY)
> +		sdformat->format.field = V4L2_FIELD_NONE;
> +
>  	mutex_lock(&vmux->lock);
>  
>  	/* Source pad mirrors active sink pad, no limitations on sink pads */
> @@ -197,11 +278,33 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int video_mux_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> +	struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
> +	struct v4l2_mbus_framefmt *mbusformat;
> +	int i;
> +
> +	mutex_lock(&vmux->lock);
> +
> +	for (i = 0; i < sd->entity.num_pads; i++) {
> +		mbusformat = v4l2_subdev_get_try_format(sd, fh->pad, i);
> +		*mbusformat = vmux->format_mbus[i];
> +	}
> +
> +	mutex_unlock(&vmux->lock);
> +
> +	return 0;
> +}

This isn't the right approach. Instead implement the init_cfg pad op.

> +
>  static const struct v4l2_subdev_pad_ops video_mux_pad_ops = {
>  	.get_fmt = video_mux_get_format,
>  	.set_fmt = video_mux_set_format,
>  };
>  
> +static const struct v4l2_subdev_internal_ops video_mux_internal_ops = {
> +	.open = video_mux_open,
> +};

So this can be dropped.

> +
>  static const struct v4l2_subdev_ops video_mux_subdev_ops = {
>  	.pad = &video_mux_pad_ops,
>  	.video = &video_mux_subdev_video_ops,
> @@ -226,6 +329,7 @@ static int video_mux_probe(struct platform_device *pdev)
>  	v4l2_subdev_init(&vmux->subdev, &video_mux_subdev_ops);
>  	snprintf(vmux->subdev.name, sizeof(vmux->subdev.name), "%s", np->name);
>  	vmux->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +	vmux->subdev.internal_ops = &video_mux_internal_ops;
>  	vmux->subdev.dev = dev;
>  
>  	/*
> @@ -263,6 +367,12 @@ static int video_mux_probe(struct platform_device *pdev)
>  	for (i = 0; i < num_pads - 1; i++)
>  		vmux->pads[i].flags = MEDIA_PAD_FL_SINK;
>  	vmux->pads[num_pads - 1].flags = MEDIA_PAD_FL_SOURCE;
> +	for (i = 0; i < num_pads; i++) {
> +		vmux->format_mbus[i].width = 1;
> +		vmux->format_mbus[i].height = 1;
> +		vmux->format_mbus[i].code = MEDIA_BUS_FMT_Y8_1X8;
> +		vmux->format_mbus[i].field = V4L2_FIELD_NONE;
> +	}
>  
>  	vmux->subdev.entity.function = MEDIA_ENT_F_VID_MUX;
>  	ret = media_entity_pads_init(&vmux->subdev.entity, num_pads,
> 

Regards,

	Hans
Philipp Zabel May 23, 2018, 8:47 a.m. UTC | #2 
Hi Hans,

thank you for the review comments.

On Tue, 2018-05-22 at 19:47 +0200, Hans Verkuil wrote:
> On 22/05/18 18:29, Philipp Zabel wrote:
> > Limit frame sizes to the [1, UINT_MAX-1] interval, media bus formats to
> > the available list of formats, and initialize pad and try formats.
> > 
> > Reported-by: Rui Miguel Silva <rui.silva@linaro.org>
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  drivers/media/platform/video-mux.c | 110 +++++++++++++++++++++++++++++
> >  1 file changed, 110 insertions(+)
> > 
> > diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
> > index 1fb887293337..ade1dae706aa 100644
> > --- a/drivers/media/platform/video-mux.c
> > +++ b/drivers/media/platform/video-mux.c
> > @@ -180,6 +180,87 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
> >  	if (!source_mbusformat)
> >  		return -EINVAL;
> >  
> > +	/* No size limitations except V4L2 compliance requirements */
> > +	v4l_bound_align_image(&sdformat->format.width, 1, UINT_MAX - 1, 0,
> > +			      &sdformat->format.height, 1, UINT_MAX - 1, 0, 0);
> 
> This is a bit dubious. I would pick more realistic min/max values like 16 and

Why 16? A grayscale or RGB sensor could crop down to 1x1, see mt9v032
for example.

> 65536. UINT_MAX - 1 will overflow whenever code increments/multiplies it for some
> reason, which can cause all sorts of weird issues.

Ok. Should v4l2-compliance check for > 65536 then, instead of (or
additionally to) UINT_MAX?

> > +
> > +	/* All formats except LVDS and vendor specific formats are acceptable */
> > +	switch (sdformat->format.code) {
> > +	case MEDIA_BUS_FMT_RGB444_1X12:
> > +	case MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE:
[...]
> > +	case MEDIA_BUS_FMT_JPEG_1X8:
> > +	case MEDIA_BUS_FMT_AHSV8888_1X32:
> > +		break;
> > +	default:
> > +		sdformat->format.code = MEDIA_BUS_FMT_Y8_1X8;
> 
> Add a break here.

Will do.

> > +	}
> > +	if (sdformat->format.field == V4L2_FIELD_ANY)
> > +		sdformat->format.field = V4L2_FIELD_NONE;
> > +
> >  	mutex_lock(&vmux->lock);
> >  
> >  	/* Source pad mirrors active sink pad, no limitations on sink pads */
> > @@ -197,11 +278,33 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
> >  	return 0;
> >  }
> >  
> > +static int video_mux_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > +{
> > +	struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
> > +	struct v4l2_mbus_framefmt *mbusformat;
> > +	int i;
> > +
> > +	mutex_lock(&vmux->lock);
> > +
> > +	for (i = 0; i < sd->entity.num_pads; i++) {
> > +		mbusformat = v4l2_subdev_get_try_format(sd, fh->pad, i);
> > +		*mbusformat = vmux->format_mbus[i];
> > +	}
> > +
> > +	mutex_unlock(&vmux->lock);
> > +
> > +	return 0;
> > +}
> 
> This isn't the right approach. Instead implement the init_cfg pad op.

How embarrassing, yes.

> > +
> >  static const struct v4l2_subdev_pad_ops video_mux_pad_ops = {
> >  	.get_fmt = video_mux_get_format,
> >  	.set_fmt = video_mux_set_format,
> >  };
> >  
> > +static const struct v4l2_subdev_internal_ops video_mux_internal_ops = {
> > +	.open = video_mux_open,
> > +};
> 
> So this can be dropped.

Ok, thanks!

regards
Philipp
Hans Verkuil May 23, 2018, 8:53 a.m. UTC | #3 
On 23/05/18 10:47, Philipp Zabel wrote:
> Hi Hans,
> 
> thank you for the review comments.
> 
> On Tue, 2018-05-22 at 19:47 +0200, Hans Verkuil wrote:
>> On 22/05/18 18:29, Philipp Zabel wrote:
>>> Limit frame sizes to the [1, UINT_MAX-1] interval, media bus formats to
>>> the available list of formats, and initialize pad and try formats.
>>>
>>> Reported-by: Rui Miguel Silva <rui.silva@linaro.org>
>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>>> ---
>>>  drivers/media/platform/video-mux.c | 110 +++++++++++++++++++++++++++++
>>>  1 file changed, 110 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
>>> index 1fb887293337..ade1dae706aa 100644
>>> --- a/drivers/media/platform/video-mux.c
>>> +++ b/drivers/media/platform/video-mux.c
>>> @@ -180,6 +180,87 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
>>>  	if (!source_mbusformat)
>>>  		return -EINVAL;
>>>  
>>> +	/* No size limitations except V4L2 compliance requirements */
>>> +	v4l_bound_align_image(&sdformat->format.width, 1, UINT_MAX - 1, 0,
>>> +			      &sdformat->format.height, 1, UINT_MAX - 1, 0, 0);
>>
>> This is a bit dubious. I would pick more realistic min/max values like 16 and
> 
> Why 16? A grayscale or RGB sensor could crop down to 1x1, see mt9v032
> for example.

Was that ever tested? Just because the software allows it, doesn't mean it actually
works.

> 
>> 65536. UINT_MAX - 1 will overflow whenever code increments/multiplies it for some
>> reason, which can cause all sorts of weird issues.
> 
> Ok. Should v4l2-compliance check for > 65536 then, instead of (or
> additionally to) UINT_MAX?

I think so, yes.

> 
>>> +
>>> +	/* All formats except LVDS and vendor specific formats are acceptable */
>>> +	switch (sdformat->format.code) {
>>> +	case MEDIA_BUS_FMT_RGB444_1X12:
>>> +	case MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE:
> [...]
>>> +	case MEDIA_BUS_FMT_JPEG_1X8:
>>> +	case MEDIA_BUS_FMT_AHSV8888_1X32:
>>> +		break;
>>> +	default:
>>> +		sdformat->format.code = MEDIA_BUS_FMT_Y8_1X8;
>>
>> Add a break here.
> 
> Will do.
> 
>>> +	}
>>> +	if (sdformat->format.field == V4L2_FIELD_ANY)
>>> +		sdformat->format.field = V4L2_FIELD_NONE;
>>> +
>>>  	mutex_lock(&vmux->lock);
>>>  
>>>  	/* Source pad mirrors active sink pad, no limitations on sink pads */
>>> @@ -197,11 +278,33 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
>>>  	return 0;
>>>  }
>>>  
>>> +static int video_mux_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>>> +{
>>> +	struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
>>> +	struct v4l2_mbus_framefmt *mbusformat;
>>> +	int i;
>>> +
>>> +	mutex_lock(&vmux->lock);
>>> +
>>> +	for (i = 0; i < sd->entity.num_pads; i++) {
>>> +		mbusformat = v4l2_subdev_get_try_format(sd, fh->pad, i);
>>> +		*mbusformat = vmux->format_mbus[i];
>>> +	}
>>> +
>>> +	mutex_unlock(&vmux->lock);
>>> +
>>> +	return 0;
>>> +}
>>
>> This isn't the right approach. Instead implement the init_cfg pad op.
> 
> How embarrassing, yes.
> 
>>> +
>>>  static const struct v4l2_subdev_pad_ops video_mux_pad_ops = {
>>>  	.get_fmt = video_mux_get_format,
>>>  	.set_fmt = video_mux_set_format,
>>>  };
>>>  
>>> +static const struct v4l2_subdev_internal_ops video_mux_internal_ops = {
>>> +	.open = video_mux_open,
>>> +};
>>
>> So this can be dropped.
> 
> Ok, thanks!
> 
> regards
> Philipp
> 

Regards,

	Hans
Philipp Zabel May 23, 2018, 9:30 a.m. UTC | #4 
On Wed, 2018-05-23 at 10:53 +0200, Hans Verkuil wrote:
> On 23/05/18 10:47, Philipp Zabel wrote:
> > Hi Hans,
> > 
> > thank you for the review comments.
> > 
> > On Tue, 2018-05-22 at 19:47 +0200, Hans Verkuil wrote:
> > > On 22/05/18 18:29, Philipp Zabel wrote:
> > > > Limit frame sizes to the [1, UINT_MAX-1] interval, media bus formats to
> > > > the available list of formats, and initialize pad and try formats.
> > > > 
> > > > Reported-by: Rui Miguel Silva <rui.silva@linaro.org>
> > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > > ---
> > > >  drivers/media/platform/video-mux.c | 110 +++++++++++++++++++++++++++++
> > > >  1 file changed, 110 insertions(+)
> > > > 
> > > > diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
> > > > index 1fb887293337..ade1dae706aa 100644
> > > > --- a/drivers/media/platform/video-mux.c
> > > > +++ b/drivers/media/platform/video-mux.c
> > > > @@ -180,6 +180,87 @@ static int video_mux_set_format(struct v4l2_subdev *sd,
> > > >  	if (!source_mbusformat)
> > > >  		return -EINVAL;
> > > >  
> > > > +	/* No size limitations except V4L2 compliance requirements */
> > > > +	v4l_bound_align_image(&sdformat->format.width, 1, UINT_MAX - 1, 0,
> > > > +			      &sdformat->format.height, 1, UINT_MAX - 1, 0, 0);
> > > 
> > > This is a bit dubious. I would pick more realistic min/max values like 16 and
> > 
> > Why 16? A grayscale or RGB sensor could crop down to 1x1, see mt9v032
> > for example.
> 
> Was that ever tested? Just because the software allows it, doesn't mean it actually
> works.

I don't know. I'll test this when I get access to a sensor that could
support such low cropping.

I'd just prefer this artificial limit not to be imposed by the generic
video mux driver, as a mux doesn't care about framing.

For example the i.MX media driver currently has an (also artificial)
limit to 16 pixel aligned frame sizes in the CSI subdev anyway.

regards
Philipp
diff mbox

Patch

diff --git a/drivers/media/platform/video-mux.c b/drivers/media/platform/video-mux.c
index 1fb887293337..ade1dae706aa 100644
--- a/drivers/media/platform/video-mux.c
+++ b/drivers/media/platform/video-mux.c
@@ -180,6 +180,87 @@  static int video_mux_set_format(struct v4l2_subdev *sd,
 	if (!source_mbusformat)
 		return -EINVAL;
 
+	/* No size limitations except V4L2 compliance requirements */
+	v4l_bound_align_image(&sdformat->format.width, 1, UINT_MAX - 1, 0,
+			      &sdformat->format.height, 1, UINT_MAX - 1, 0, 0);
+
+	/* All formats except LVDS and vendor specific formats are acceptable */
+	switch (sdformat->format.code) {
+	case MEDIA_BUS_FMT_RGB444_1X12:
+	case MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE:
+	case MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE:
+	case MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE:
+	case MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE:
+	case MEDIA_BUS_FMT_RGB565_1X16:
+	case MEDIA_BUS_FMT_BGR565_2X8_BE:
+	case MEDIA_BUS_FMT_BGR565_2X8_LE:
+	case MEDIA_BUS_FMT_RGB565_2X8_BE:
+	case MEDIA_BUS_FMT_RGB565_2X8_LE:
+	case MEDIA_BUS_FMT_RGB666_1X18:
+	case MEDIA_BUS_FMT_RBG888_1X24:
+	case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
+	case MEDIA_BUS_FMT_BGR888_1X24:
+	case MEDIA_BUS_FMT_GBR888_1X24:
+	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_RGB888_2X12_BE:
+	case MEDIA_BUS_FMT_RGB888_2X12_LE:
+	case MEDIA_BUS_FMT_ARGB8888_1X32:
+	case MEDIA_BUS_FMT_RGB888_1X32_PADHI:
+	case MEDIA_BUS_FMT_RGB101010_1X30:
+	case MEDIA_BUS_FMT_RGB121212_1X36:
+	case MEDIA_BUS_FMT_RGB161616_1X48:
+	case MEDIA_BUS_FMT_Y8_1X8:
+	case MEDIA_BUS_FMT_UV8_1X8:
+	case MEDIA_BUS_FMT_UYVY8_1_5X8:
+	case MEDIA_BUS_FMT_VYUY8_1_5X8:
+	case MEDIA_BUS_FMT_YUYV8_1_5X8:
+	case MEDIA_BUS_FMT_YVYU8_1_5X8:
+	case MEDIA_BUS_FMT_UYVY8_2X8:
+	case MEDIA_BUS_FMT_VYUY8_2X8:
+	case MEDIA_BUS_FMT_YUYV8_2X8:
+	case MEDIA_BUS_FMT_YVYU8_2X8:
+	case MEDIA_BUS_FMT_Y10_1X10:
+	case MEDIA_BUS_FMT_UYVY10_2X10:
+	case MEDIA_BUS_FMT_VYUY10_2X10:
+	case MEDIA_BUS_FMT_YUYV10_2X10:
+	case MEDIA_BUS_FMT_YVYU10_2X10:
+	case MEDIA_BUS_FMT_Y12_1X12:
+	case MEDIA_BUS_FMT_UYVY12_2X12:
+	case MEDIA_BUS_FMT_VYUY12_2X12:
+	case MEDIA_BUS_FMT_YUYV12_2X12:
+	case MEDIA_BUS_FMT_YVYU12_2X12:
+	case MEDIA_BUS_FMT_UYVY8_1X16:
+	case MEDIA_BUS_FMT_VYUY8_1X16:
+	case MEDIA_BUS_FMT_YUYV8_1X16:
+	case MEDIA_BUS_FMT_YVYU8_1X16:
+	case MEDIA_BUS_FMT_YDYUYDYV8_1X16:
+	case MEDIA_BUS_FMT_UYVY10_1X20:
+	case MEDIA_BUS_FMT_VYUY10_1X20:
+	case MEDIA_BUS_FMT_YUYV10_1X20:
+	case MEDIA_BUS_FMT_YVYU10_1X20:
+	case MEDIA_BUS_FMT_VUY8_1X24:
+	case MEDIA_BUS_FMT_YUV8_1X24:
+	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
+	case MEDIA_BUS_FMT_UYVY12_1X24:
+	case MEDIA_BUS_FMT_VYUY12_1X24:
+	case MEDIA_BUS_FMT_YUYV12_1X24:
+	case MEDIA_BUS_FMT_YVYU12_1X24:
+	case MEDIA_BUS_FMT_YUV10_1X30:
+	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
+	case MEDIA_BUS_FMT_AYUV8_1X32:
+	case MEDIA_BUS_FMT_UYYVYY12_0_5X36:
+	case MEDIA_BUS_FMT_YUV12_1X36:
+	case MEDIA_BUS_FMT_YUV16_1X48:
+	case MEDIA_BUS_FMT_UYYVYY16_0_5X48:
+	case MEDIA_BUS_FMT_JPEG_1X8:
+	case MEDIA_BUS_FMT_AHSV8888_1X32:
+		break;
+	default:
+		sdformat->format.code = MEDIA_BUS_FMT_Y8_1X8;
+	}
+	if (sdformat->format.field == V4L2_FIELD_ANY)
+		sdformat->format.field = V4L2_FIELD_NONE;
+
 	mutex_lock(&vmux->lock);
 
 	/* Source pad mirrors active sink pad, no limitations on sink pads */
@@ -197,11 +278,33 @@  static int video_mux_set_format(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int video_mux_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	struct video_mux *vmux = v4l2_subdev_to_video_mux(sd);
+	struct v4l2_mbus_framefmt *mbusformat;
+	int i;
+
+	mutex_lock(&vmux->lock);
+
+	for (i = 0; i < sd->entity.num_pads; i++) {
+		mbusformat = v4l2_subdev_get_try_format(sd, fh->pad, i);
+		*mbusformat = vmux->format_mbus[i];
+	}
+
+	mutex_unlock(&vmux->lock);
+
+	return 0;
+}
+
 static const struct v4l2_subdev_pad_ops video_mux_pad_ops = {
 	.get_fmt = video_mux_get_format,
 	.set_fmt = video_mux_set_format,
 };
 
+static const struct v4l2_subdev_internal_ops video_mux_internal_ops = {
+	.open = video_mux_open,
+};
+
 static const struct v4l2_subdev_ops video_mux_subdev_ops = {
 	.pad = &video_mux_pad_ops,
 	.video = &video_mux_subdev_video_ops,
@@ -226,6 +329,7 @@  static int video_mux_probe(struct platform_device *pdev)
 	v4l2_subdev_init(&vmux->subdev, &video_mux_subdev_ops);
 	snprintf(vmux->subdev.name, sizeof(vmux->subdev.name), "%s", np->name);
 	vmux->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	vmux->subdev.internal_ops = &video_mux_internal_ops;
 	vmux->subdev.dev = dev;
 
 	/*
@@ -263,6 +367,12 @@  static int video_mux_probe(struct platform_device *pdev)
 	for (i = 0; i < num_pads - 1; i++)
 		vmux->pads[i].flags = MEDIA_PAD_FL_SINK;
 	vmux->pads[num_pads - 1].flags = MEDIA_PAD_FL_SOURCE;
+	for (i = 0; i < num_pads; i++) {
+		vmux->format_mbus[i].width = 1;
+		vmux->format_mbus[i].height = 1;
+		vmux->format_mbus[i].code = MEDIA_BUS_FMT_Y8_1X8;
+		vmux->format_mbus[i].field = V4L2_FIELD_NONE;
+	}
 
 	vmux->subdev.entity.function = MEDIA_ENT_F_VID_MUX;
 	ret = media_entity_pads_init(&vmux->subdev.entity, num_pads,