| Message ID | 20230318234014.29506-1-dan.scally@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Dan On Sat, Mar 18, 2023 at 11:40:03PM +0000, Daniel Scally via libcamera-devel wrote: > Hello all > > This series is an attempt at adding support for the OV7251 camera on Microsoft > Surface platforms to libcamera. The sensor outputs data in MEDIA_BUS_FMT_Y10_1X10 > format, which is packed by the CIO2 CSI-2 reciever into V4L2_PIX_FMT_IPU3_Y10. As > far as I know, these data are not intended to run through the Imgu at all. Certainly Aren't they ? How can an application generically display Y10 if it receives data packed in a platform-specific format ? > the kernel's ipu3-imgu driver does not support that format, but given the simplicity Maybe it is still just missing from the driver ? Should we ask the maintainers of the driver if it is just missing or it is not supported at all ? From a quick glance I have't find a trivial way to add Y10 support to imgu_css_formats[] > of the format we can simply use the CIO2 device's capture-capable devnode as the > source of the image data. > > To facilitate that, I'm identifying sensors which don't have a format that needs > the Imgu and simply skipping the Imgu/IPA processing in the IPU3 pipeline for > them. > > What I don't particularly like about the way that I've done things here is that > the stream only works for the Raw role, which feels wrong. The advantage is that > it's contained within the existing IPU3 pipeline without too much disruption, > but I do wonder whether a separate CIO2-only pipeline might not be better. In > that case PipelineHandlerIPU3::match() would need amending to check whether the > sensor for each "ipu3-csi-2 n" instance needed hooking into the Imgu and not adding > those to the device match if it found that they didn't need it, so that a new > CIO2-only pipeline could claim the media devices. I'd love to hear other people's > thoughts about the best way to handle this. > > Thanks > Dan > > Daniel Scally (11): > pipeline: ipu3: Check if sensor supports test pattern control > include: drm_fourcc: Add Y10 format > libcamera/formats: Add IPU3_Y10 format > pipeline: ipu3: Identify sensors that do not need the Imgu > pipeline: ipu3: Add needsImgu flag to IPU3Frames > pipeline: ipu3: Support sensors using only CIO2 > pipeline: ipu3: Allow raw-only streams in IPU3 pipeline > pipeline: ipu3: Support IPU3_Y10 format > apps: qcam: Add support for IPU3_Y10 > apps: qcam: Remove restriction on raw-only streams > pipeline: ipu3: Increase maximum connected cameras > > include/linux/drm_fourcc.h | 3 + > src/apps/qcam/format_converter.cpp | 50 +++++ > src/apps/qcam/format_converter.h | 2 + > src/apps/qcam/main_window.cpp | 6 - > src/libcamera/formats.cpp | 10 + > src/libcamera/formats.yaml | 3 + > src/libcamera/pipeline/ipu3/cio2.cpp | 21 +++ > src/libcamera/pipeline/ipu3/cio2.h | 4 + > src/libcamera/pipeline/ipu3/frames.cpp | 45 +++-- > src/libcamera/pipeline/ipu3/frames.h | 3 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 242 ++++++++++++++----------- > 11 files changed, 255 insertions(+), 134 deletions(-) > > -- > 2.34.1 >
On Mon, Mar 20, 2023 at 08:56:00AM +0100, Jacopo Mondi via libcamera-devel wrote: > Hi Dan > > On Sat, Mar 18, 2023 at 11:40:03PM +0000, Daniel Scally via libcamera-devel wrote: > > Hello all > > > > This series is an attempt at adding support for the OV7251 camera on Microsoft > > Surface platforms to libcamera. The sensor outputs data in MEDIA_BUS_FMT_Y10_1X10 > > format, which is packed by the CIO2 CSI-2 reciever into V4L2_PIX_FMT_IPU3_Y10. As > > far as I know, these data are not intended to run through the Imgu at all. Certainly > > Aren't they ? How can an application generically display Y10 if it > receives data packed in a platform-specific format ? This will require platform-specific applications :-( > > the kernel's ipu3-imgu driver does not support that format, but given the simplicity > > Maybe it is still just missing from the driver ? Should we ask the > maintainers of the driver if it is just missing or it is not supported > at all ? From a quick glance I have't find a trivial way to add Y10 > support to imgu_css_formats[] It's most likely possible at the hardware level, but the information we have is that at least the firmware doesn't support skipping the CFA interpolation. > > of the format we can simply use the CIO2 device's capture-capable devnode as the > > source of the image data. > > > > To facilitate that, I'm identifying sensors which don't have a format that needs > > the Imgu and simply skipping the Imgu/IPA processing in the IPU3 pipeline for > > them. > > > > What I don't particularly like about the way that I've done things here is that > > the stream only works for the Raw role, which feels wrong. The advantage is that > > it's contained within the existing IPU3 pipeline without too much disruption, > > but I do wonder whether a separate CIO2-only pipeline might not be better. In > > that case PipelineHandlerIPU3::match() would need amending to check whether the > > sensor for each "ipu3-csi-2 n" instance needed hooking into the Imgu and not adding > > those to the device match if it found that they didn't need it, so that a new > > CIO2-only pipeline could claim the media devices. I'd love to hear other people's > > thoughts about the best way to handle this. > > > > Thanks > > Dan > > > > Daniel Scally (11): > > pipeline: ipu3: Check if sensor supports test pattern control > > include: drm_fourcc: Add Y10 format > > libcamera/formats: Add IPU3_Y10 format > > pipeline: ipu3: Identify sensors that do not need the Imgu > > pipeline: ipu3: Add needsImgu flag to IPU3Frames > > pipeline: ipu3: Support sensors using only CIO2 > > pipeline: ipu3: Allow raw-only streams in IPU3 pipeline > > pipeline: ipu3: Support IPU3_Y10 format > > apps: qcam: Add support for IPU3_Y10 > > apps: qcam: Remove restriction on raw-only streams > > pipeline: ipu3: Increase maximum connected cameras > > > > include/linux/drm_fourcc.h | 3 + > > src/apps/qcam/format_converter.cpp | 50 +++++ > > src/apps/qcam/format_converter.h | 2 + > > src/apps/qcam/main_window.cpp | 6 - > > src/libcamera/formats.cpp | 10 + > > src/libcamera/formats.yaml | 3 + > > src/libcamera/pipeline/ipu3/cio2.cpp | 21 +++ > > src/libcamera/pipeline/ipu3/cio2.h | 4 + > > src/libcamera/pipeline/ipu3/frames.cpp | 45 +++-- > > src/libcamera/pipeline/ipu3/frames.h | 3 +- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 242 ++++++++++++++----------- > > 11 files changed, 255 insertions(+), 134 deletions(-)