On Fri, Aug 04, 2023 at 01:32:43PM +0300, Laurent Pinchart wrote: > On Fri, Aug 04, 2023 at 06:20:36AM +0000, Sakari Ailus wrote: > > On Fri, Aug 04, 2023 at 01:08:40AM +0300, Laurent Pinchart wrote: > > > > > > +static int mei_csi_set_fmt(struct v4l2_subdev *sd, > > > > > > + struct v4l2_subdev_state *sd_state, > > > > > > + struct v4l2_subdev_format *format) > > > > > > +{ > > > > > > + struct v4l2_mbus_framefmt *source_mbusformat; > > > > > > + struct v4l2_mbus_framefmt *mbusformat; > > > > > > + struct mei_csi *csi = sd_to_csi(sd); > > > > > > + struct media_pad *pad; > > > > > > + > > > > > > + mbusformat = mei_csi_get_pad_format(sd, sd_state, format->pad, > > > > > > + format->which); > > > > > > + if (!mbusformat) > > > > > > + return -EINVAL; > > > > > > + > > > > > > + source_mbusformat = mei_csi_get_pad_format(sd, sd_state, CSI_PAD_SOURCE, > > > > > > + format->which); > > > > > > + if (!source_mbusformat) > > > > > > + return -EINVAL; > > > > > > + > > > > > > + v4l_bound_align_image(&format->format.width, 1, 65536, 0, > > > > > > + &format->format.height, 1, 65536, 0, 0); > > > > > > + > > > > > > + switch (format->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: > > > > > > + case MEDIA_BUS_FMT_SBGGR8_1X8: > > > > > > + case MEDIA_BUS_FMT_SGBRG8_1X8: > > > > > > + case MEDIA_BUS_FMT_SGRBG8_1X8: > > > > > > + case MEDIA_BUS_FMT_SRGGB8_1X8: > > > > > > + case MEDIA_BUS_FMT_SBGGR10_1X10: > > > > > > + case MEDIA_BUS_FMT_SGBRG10_1X10: > > > > > > + case MEDIA_BUS_FMT_SGRBG10_1X10: > > > > > > + case MEDIA_BUS_FMT_SRGGB10_1X10: > > > > > > + case MEDIA_BUS_FMT_SBGGR12_1X12: > > > > > > + case MEDIA_BUS_FMT_SGBRG12_1X12: > > > > > > + case MEDIA_BUS_FMT_SGRBG12_1X12: > > > > > > + case MEDIA_BUS_FMT_SRGGB12_1X12: > > > > > > + case MEDIA_BUS_FMT_SBGGR14_1X14: > > > > > > + case MEDIA_BUS_FMT_SGBRG14_1X14: > > > > > > + case MEDIA_BUS_FMT_SGRBG14_1X14: > > > > > > + case MEDIA_BUS_FMT_SRGGB14_1X14: > > > > > > + case MEDIA_BUS_FMT_SBGGR16_1X16: > > > > > > + case MEDIA_BUS_FMT_SGBRG16_1X16: > > > > > > + case MEDIA_BUS_FMT_SGRBG16_1X16: > > > > > > + case MEDIA_BUS_FMT_SRGGB16_1X16: > > > > > > + break; > > > > > > + default: > > > > > > + format->format.code = MEDIA_BUS_FMT_Y8_1X8; > > > > > > + break; > > > > > > + } > > > > > > > > > > I wonder if we should simply accept all formats. The IVSC doesn't seem > > > > > to cara. > > > > > > > > The video mux needs something similar. I was thinking of adding a generic > > > > helper for such functions, perhaps with flags to suggest which formats to > > > > accept. > > > > > > What would be the drawbacks of accepting all formats ? > > > > Also the ones that aren't defined at the moment? > > > > They're not valid, although in practice there might not be issues, as they > > are compared across links in any case. > > That was my reasoning too. > > > I guess there should also be support for enum_mbus_codes, and for that we > > need a list of some sort. > > Hmmm... If a subdev really implements pass-through for video data, I > wonder if we could lift the requirement of implementing format > enumeration. v4l2-compliance tests for that. There are also other drivers that don't implement it but they're mostly old. I guess libcamera wouldn't need this, is that right? -- Regards, Sakari Ailus