| Message ID | 20240313072516.241106-9-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Generic line based metadata support, internal pads | expand |
On 3/13/24 08:24, Sakari Ailus wrote: > Document how embedded data support should be implemented for camera > sensors, and when and how CCS embedded data format should be referenced. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > .../media/drivers/camera-sensor.rst | 29 +++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/Documentation/userspace-api/media/drivers/camera-sensor.rst b/Documentation/userspace-api/media/drivers/camera-sensor.rst > index 919a50e8b9d9..92545538b855 100644 > --- a/Documentation/userspace-api/media/drivers/camera-sensor.rst > +++ b/Documentation/userspace-api/media/drivers/camera-sensor.rst > @@ -102,3 +102,32 @@ register programming sequences shall initialize the :ref:`V4L2_CID_HFLIP > values programmed by the register sequences. The default values of these > controls shall be 0 (disabled). Especially these controls shall not be inverted, > independently of the sensor's mounting rotation. > + > +Embedded data > +------------- > + > +Many sensors, mostly raw sensors, support embedded data which is used to convey > +the sensor configuration for the captured frame back to the host. While CSI-2 is > +the most common bus used by such sensors, embedded data can be available on > +other bus types as well. > + > +Embedded data support shall use an internal source pad (a pad that has > +``MEDIA_PAD_FL_SINK <MEDIA-PAD-FL-SINK>`` and ``MEDIA_PAD_FL_INTERNAL > +<MEDIA-PAD-FL-INTERNAL>`` flags set) and route to the external pad. If embedded > +data output can be disabled in hardware, it should be possible to disable the > +embedded data route via ``VIDIOC_SUBDEV_S_ROUTING`` IOCTL. > + > +In general, changing the embedded data format from the driver-configured values > +is not supported. The height of the metadata is hardware specific and the width > +is that (or less of that) of the image width, as configured on the pixel data > +stream. > + > +CCS and non-CCS embedded data > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + > +Embedded data which is compliant with CCS definitions shall use ``CCS embedded > +data format <MEDIA-BUS-FMT-CCS-EMBEDDED>``. Device specific embedded data which > +is compliant up to MIPI CCS embedded data levels 1 or 2 only shall refer to CCS > +embedded data formats and document the level of conformance. The rest of the > +device specific embedded data format shall be documented in the context of the > +data format itself. Reviewed-by: Julien Massot <julien.massot@collabora.com> Julien
Hi Sakari, Thank you for the patch. On Wed, Mar 13, 2024 at 09:24:46AM +0200, Sakari Ailus wrote: > Document how embedded data support should be implemented for camera > sensors, and when and how CCS embedded data format should be referenced. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > .../media/drivers/camera-sensor.rst | 29 +++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/Documentation/userspace-api/media/drivers/camera-sensor.rst b/Documentation/userspace-api/media/drivers/camera-sensor.rst > index 919a50e8b9d9..92545538b855 100644 > --- a/Documentation/userspace-api/media/drivers/camera-sensor.rst > +++ b/Documentation/userspace-api/media/drivers/camera-sensor.rst > @@ -102,3 +102,32 @@ register programming sequences shall initialize the :ref:`V4L2_CID_HFLIP > values programmed by the register sequences. The default values of these > controls shall be 0 (disabled). Especially these controls shall not be inverted, > independently of the sensor's mounting rotation. > + > +Embedded data > +------------- > + > +Many sensors, mostly raw sensors, support embedded data which is used to convey > +the sensor configuration for the captured frame back to the host. While CSI-2 is > +the most common bus used by such sensors, embedded data can be available on > +other bus types as well. > + > +Embedded data support shall use an internal source pad (a pad that has s/source pad/sink pad/ Or do we call those "internal source pad" ? As this is uAPI documentation, it is mainly targetting (in my opinion) userspace developers. I would write "Embedded data support uses ..." to describe the behaviour seen from userspace, instead of using "shall" to describe the requirements towards drivers. > +``MEDIA_PAD_FL_SINK <MEDIA-PAD-FL-SINK>`` and ``MEDIA_PAD_FL_INTERNAL > +<MEDIA-PAD-FL-INTERNAL>`` flags set) and route to the external pad. If embedded > +data output can be disabled in hardware, it should be possible to disable the > +embedded data route via ``VIDIOC_SUBDEV_S_ROUTING`` IOCTL. You should mention the image pad here too: Such sensors expose two internal sink pads (pads that have both the ``MEDIA_PAD_FL_SINK <MEDIA-PAD-FL-SINK>`` and ``MEDIA_PAD_FL_INTERNAL <MEDIA-PAD-FL-INTERNAL>`` flags set) to model the source of the image and embedded data streams. Each of those pads produces a single stream, and the sub-device routes those streams to the external (source) pad. If embedded data output can be disabled in hardware, the sub-device allows disabling the embedded data route via the ``VIDIOC_SUBDEV_S_ROUTING`` IOCTL. > + > +In general, changing the embedded data format from the driver-configured values > +is not supported. The height of the metadata is hardware specific and the width s/hardware specific/device-specific/ > +is that (or less of that) of the image width, as configured on the pixel data > +stream. > + > +CCS and non-CCS embedded data > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + > +Embedded data which is compliant with CCS definitions shall use ``CCS embedded > +data format <MEDIA-BUS-FMT-CCS-EMBEDDED>``. Device specific embedded data which You should clarify here that you're talking about the internal sink pad. s/Device specific/Only device-specific/ > +is compliant up to MIPI CCS embedded data levels 1 or 2 only shall refer to CCS s/up to/with the/ s/only shall/may/ > +embedded data formats and document the level of conformance. The rest of the > +device specific embedded data format shall be documented in the context of the s/device specific/device-specific/ > +data format itself. Not sure what you mean by the context in the last sentence.
Hi Laurent, On Wed, Mar 20, 2024 at 02:03:00AM +0200, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Wed, Mar 13, 2024 at 09:24:46AM +0200, Sakari Ailus wrote: > > Document how embedded data support should be implemented for camera > > sensors, and when and how CCS embedded data format should be referenced. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > .../media/drivers/camera-sensor.rst | 29 +++++++++++++++++++ > > 1 file changed, 29 insertions(+) > > > > diff --git a/Documentation/userspace-api/media/drivers/camera-sensor.rst b/Documentation/userspace-api/media/drivers/camera-sensor.rst > > index 919a50e8b9d9..92545538b855 100644 > > --- a/Documentation/userspace-api/media/drivers/camera-sensor.rst > > +++ b/Documentation/userspace-api/media/drivers/camera-sensor.rst > > @@ -102,3 +102,32 @@ register programming sequences shall initialize the :ref:`V4L2_CID_HFLIP > > values programmed by the register sequences. The default values of these > > controls shall be 0 (disabled). Especially these controls shall not be inverted, > > independently of the sensor's mounting rotation. > > + > > +Embedded data > > +------------- > > + > > +Many sensors, mostly raw sensors, support embedded data which is used to convey > > +the sensor configuration for the captured frame back to the host. While CSI-2 is > > +the most common bus used by such sensors, embedded data can be available on > > +other bus types as well. > > + > > +Embedded data support shall use an internal source pad (a pad that has > > s/source pad/sink pad/ > > Or do we call those "internal source pad" ? As I wrote in the earlier e-mail, I'm quite unhappy with the "internal source pad" term, mainly because of the conflict with the term itself and the flags that indicate it. I'd just call them "internal sink pads" and then explain they're actually a source of data. > > As this is uAPI documentation, it is mainly targetting (in my opinion) > userspace developers. I would write "Embedded data support uses ..." to > describe the behaviour seen from userspace, instead of using "shall" to > describe the requirements towards drivers. Sounds good. > > > +``MEDIA_PAD_FL_SINK <MEDIA-PAD-FL-SINK>`` and ``MEDIA_PAD_FL_INTERNAL > > +<MEDIA-PAD-FL-INTERNAL>`` flags set) and route to the external pad. If embedded > > +data output can be disabled in hardware, it should be possible to disable the > > +embedded data route via ``VIDIOC_SUBDEV_S_ROUTING`` IOCTL. > > You should mention the image pad here too: > > Such sensors expose two internal sink pads (pads that have both the > ``MEDIA_PAD_FL_SINK <MEDIA-PAD-FL-SINK>`` and ``MEDIA_PAD_FL_INTERNAL > <MEDIA-PAD-FL-INTERNAL>`` flags set) to model the source of the image and > embedded data streams. Each of those pads produces a single stream, and the s/Each of those/Both of these/ > sub-device routes those streams to the external (source) pad. If embedded data > output can be disabled in hardware, the sub-device allows disabling the > embedded data route via the ``VIDIOC_SUBDEV_S_ROUTING`` IOCTL. In the last sentence, we need to refer to the driver. But generally I agree. I'll use: "If the sub-device driver supports disabling embedded data, this can be done by disabling the embedded data route via the ``VIDIOC_SUBDEV_S_ROUTING`` IOCTL." > > > + > > +In general, changing the embedded data format from the driver-configured values > > +is not supported. The height of the metadata is hardware specific and the width > > s/hardware specific/device-specific/ Yes. > > > +is that (or less of that) of the image width, as configured on the pixel data > > +stream. > > + > > +CCS and non-CCS embedded data > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > + > > +Embedded data which is compliant with CCS definitions shall use ``CCS embedded > > +data format <MEDIA-BUS-FMT-CCS-EMBEDDED>``. Device specific embedded data which > > You should clarify here that you're talking about the internal sink pad. > > s/Device specific/Only device-specific/ > > > +is compliant up to MIPI CCS embedded data levels 1 or 2 only shall refer to CCS > > s/up to/with the/ > s/only shall/may/ > > > +embedded data formats and document the level of conformance. The rest of the > > +device specific embedded data format shall be documented in the context of the > > s/device specific/device-specific/ > > > +data format itself. > > Not sure what you mean by the context in the last sentence. This refers to CCS embedded data support levels which appears to be documented in a later patch in the series ("media: uapi: ccs: Add media bus code for MIPI CCS embedded data"). I'll add this paragraph after that patch. The paragraph became finally: Embedded data which is fully compliant with CCS definitions uses ``CCS embedded data format <MEDIA-BUS-FMT-CCS-EMBEDDED>`` media bus code (level 3). Device-specific embedded data compliant with either MIPI CCS embedded data levels 1 or 2 only shall not use CCS embedded data mbus code, but may refer to CCS embedded data documentation with the level of conformance specified and omit documenting these aspects of the format. The rest of the device-specific embedded data format is documented in the context of the data format itself.
diff --git a/Documentation/userspace-api/media/drivers/camera-sensor.rst b/Documentation/userspace-api/media/drivers/camera-sensor.rst index 919a50e8b9d9..92545538b855 100644 --- a/Documentation/userspace-api/media/drivers/camera-sensor.rst +++ b/Documentation/userspace-api/media/drivers/camera-sensor.rst @@ -102,3 +102,32 @@ register programming sequences shall initialize the :ref:`V4L2_CID_HFLIP values programmed by the register sequences. The default values of these controls shall be 0 (disabled). Especially these controls shall not be inverted, independently of the sensor's mounting rotation. + +Embedded data +------------- + +Many sensors, mostly raw sensors, support embedded data which is used to convey +the sensor configuration for the captured frame back to the host. While CSI-2 is +the most common bus used by such sensors, embedded data can be available on +other bus types as well. + +Embedded data support shall use an internal source pad (a pad that has +``MEDIA_PAD_FL_SINK <MEDIA-PAD-FL-SINK>`` and ``MEDIA_PAD_FL_INTERNAL +<MEDIA-PAD-FL-INTERNAL>`` flags set) and route to the external pad. If embedded +data output can be disabled in hardware, it should be possible to disable the +embedded data route via ``VIDIOC_SUBDEV_S_ROUTING`` IOCTL. + +In general, changing the embedded data format from the driver-configured values +is not supported. The height of the metadata is hardware specific and the width +is that (or less of that) of the image width, as configured on the pixel data +stream. + +CCS and non-CCS embedded data +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Embedded data which is compliant with CCS definitions shall use ``CCS embedded +data format <MEDIA-BUS-FMT-CCS-EMBEDDED>``. Device specific embedded data which +is compliant up to MIPI CCS embedded data levels 1 or 2 only shall refer to CCS +embedded data formats and document the level of conformance. The rest of the +device specific embedded data format shall be documented in the context of the +data format itself.
Document how embedded data support should be implemented for camera sensors, and when and how CCS embedded data format should be referenced. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- .../media/drivers/camera-sensor.rst | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+)