| Message ID | 20240313072516.241106-24-sakari.ailus@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v8,01/38] media: mc: Add INTERNAL pad flag | expand |
On 3/13/24 08:25, Sakari Ailus wrote: > Add new MIPI CCS embedded data media bus code > (MEDIA_BUS_FMT_CCS_EMBEDDED). > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Reviewed-by: Julien Massot <julien.massot@collabora.com> > --- > .../media/v4l/subdev-formats.rst | 28 +++++++++++++++++++ > include/uapi/linux/media-bus-format.h | 3 ++ > 2 files changed, 31 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst > index cbd475f7cae9..c8f982411e70 100644 > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst > @@ -8564,3 +8564,31 @@ and finally the bit number in subscript. "X" indicates a padding bit. > - X > - X > - X > + > +.. _MEDIA-BUS-FMT-CCS-EMBEDDED: > + > +MIPI CCS Embedded Data Formats > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > + > +`MIPI CCS <https://www.mipi.org/specifications/camera-command-set>`_ defines a > +metadata format for sensor embedded data, which is used to store the register > +configuration used for capturing a given frame. The format is defined in the CCS > +specification. The media bus code for this format is > +``MEDIA_BUS_FMT_CCS_EMBEDDED``. > + > +The CCS embedded data format definition includes three levels: > + > +1. Padding within CSI-2 bus :ref:`Data unit <media-glossary-data-unit>` as > + documented in the MIPI CCS specification. > + > +2. The tagged data format as documented in the MIPI CCS specification. > + > +3. Register addresses and register documentation as documented in the MIPI CCS > + specification. > + > +The format definition shall be used only by devices that fulfill all three > +levels above. > + > +This mbus code are only used for "2-byte simplified tagged data format" (code > +0xa) but their use may be extended further in the future, to cover other CCS > +embedded data format codes. > diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h > index d4c1d991014b..03f7e9ab517b 100644 > --- a/include/uapi/linux/media-bus-format.h > +++ b/include/uapi/linux/media-bus-format.h > @@ -183,4 +183,7 @@ > #define MEDIA_BUS_FMT_META_20 0x8006 > #define MEDIA_BUS_FMT_META_24 0x8007 > > +/* Specific metadata formats. Next is 0x9002. */ > +#define MEDIA_BUS_FMT_CCS_EMBEDDED 0x9001 > + > #endif /* __LINUX_MEDIA_BUS_FORMAT_H */
Hi Sakari, Thank you for the patch. On Wed, Mar 13, 2024 at 09:25:01AM +0200, Sakari Ailus wrote: > Add new MIPI CCS embedded data media bus code > (MEDIA_BUS_FMT_CCS_EMBEDDED). > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > .../media/v4l/subdev-formats.rst | 28 +++++++++++++++++++ > include/uapi/linux/media-bus-format.h | 3 ++ > 2 files changed, 31 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst > index cbd475f7cae9..c8f982411e70 100644 > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst > @@ -8564,3 +8564,31 @@ and finally the bit number in subscript. "X" indicates a padding bit. > - X > - X > - X > + > +.. _MEDIA-BUS-FMT-CCS-EMBEDDED: > + > +MIPI CCS Embedded Data Formats > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > + > +`MIPI CCS <https://www.mipi.org/specifications/camera-command-set>`_ defines a > +metadata format for sensor embedded data, which is used to store the register > +configuration used for capturing a given frame. The format is defined in the CCS > +specification. The media bus code for this format is > +``MEDIA_BUS_FMT_CCS_EMBEDDED``. > + > +The CCS embedded data format definition includes three levels: > + > +1. Padding within CSI-2 bus :ref:`Data unit <media-glossary-data-unit>` as s/Data unit/Data Unit/ > + documented in the MIPI CCS specification. > + > +2. The tagged data format as documented in the MIPI CCS specification. > + > +3. Register addresses and register documentation as documented in the MIPI CCS > + specification. > + > +The format definition shall be used only by devices that fulfill all three > +levels above. Hmmmm... Do we need to mandate level 3 ? There are lots of sensors that comply with the first two levels but have their own register set. Would you like a sensor-specific embedded data format for each of them ? If so, how would we document it without essentially copying the datasheet ? > + > +This mbus code are only used for "2-byte simplified tagged data format" (code > +0xa) but their use may be extended further in the future, to cover other CCS > +embedded data format codes. > diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h > index d4c1d991014b..03f7e9ab517b 100644 > --- a/include/uapi/linux/media-bus-format.h > +++ b/include/uapi/linux/media-bus-format.h > @@ -183,4 +183,7 @@ > #define MEDIA_BUS_FMT_META_20 0x8006 > #define MEDIA_BUS_FMT_META_24 0x8007 > > +/* Specific metadata formats. Next is 0x9002. */ > +#define MEDIA_BUS_FMT_CCS_EMBEDDED 0x9001 > + > #endif /* __LINUX_MEDIA_BUS_FORMAT_H */
Hi Laurent, On Thu, Mar 21, 2024 at 06:49:14PM +0200, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Wed, Mar 13, 2024 at 09:25:01AM +0200, Sakari Ailus wrote: > > Add new MIPI CCS embedded data media bus code > > (MEDIA_BUS_FMT_CCS_EMBEDDED). > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > .../media/v4l/subdev-formats.rst | 28 +++++++++++++++++++ > > include/uapi/linux/media-bus-format.h | 3 ++ > > 2 files changed, 31 insertions(+) > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst > > index cbd475f7cae9..c8f982411e70 100644 > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst > > @@ -8564,3 +8564,31 @@ and finally the bit number in subscript. "X" indicates a padding bit. > > - X > > - X > > - X > > + > > +.. _MEDIA-BUS-FMT-CCS-EMBEDDED: > > + > > +MIPI CCS Embedded Data Formats > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > + > > +`MIPI CCS <https://www.mipi.org/specifications/camera-command-set>`_ defines a > > +metadata format for sensor embedded data, which is used to store the register > > +configuration used for capturing a given frame. The format is defined in the CCS > > +specification. The media bus code for this format is > > +``MEDIA_BUS_FMT_CCS_EMBEDDED``. > > + > > +The CCS embedded data format definition includes three levels: > > + > > +1. Padding within CSI-2 bus :ref:`Data unit <media-glossary-data-unit>` as > > s/Data unit/Data Unit/ Yes. > > > + documented in the MIPI CCS specification. > > + > > +2. The tagged data format as documented in the MIPI CCS specification. > > + > > +3. Register addresses and register documentation as documented in the MIPI CCS > > + specification. > > + > > +The format definition shall be used only by devices that fulfill all three > > +levels above. > > Hmmmm... Do we need to mandate level 3 ? There are lots of sensors that > comply with the first two levels but have their own register set. Would > you like a sensor-specific embedded data format for each of them ? If > so, how would we document it without essentially copying the datasheet ? You could refer to the datasheet if it's publicly available. The CCS static data format is meaningful only for accessing (level 1) and parsing (level 2) the data without knowing its meaning, which is the most important aspect. The user space will need to know the meaning of the data and can additionally select the same data unpacker than for CCS and possibly also the CCS data parser if applicable. The user space should especially not have to have a list of CCS-compliant sensors which would become necessary, should we skip on the 3rd level requirement here. > > > + > > +This mbus code are only used for "2-byte simplified tagged data format" (code > > +0xa) but their use may be extended further in the future, to cover other CCS > > +embedded data format codes. > > diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h > > index d4c1d991014b..03f7e9ab517b 100644 > > --- a/include/uapi/linux/media-bus-format.h > > +++ b/include/uapi/linux/media-bus-format.h > > @@ -183,4 +183,7 @@ > > #define MEDIA_BUS_FMT_META_20 0x8006 > > #define MEDIA_BUS_FMT_META_24 0x8007 > > > > +/* Specific metadata formats. Next is 0x9002. */ > > +#define MEDIA_BUS_FMT_CCS_EMBEDDED 0x9001 > > + > > #endif /* __LINUX_MEDIA_BUS_FORMAT_H */ >
Hi Sakari, On Thu, Apr 11, 2024 at 09:04:47AM +0000, Sakari Ailus wrote: > On Thu, Mar 21, 2024 at 06:49:14PM +0200, Laurent Pinchart wrote: > > On Wed, Mar 13, 2024 at 09:25:01AM +0200, Sakari Ailus wrote: > > > Add new MIPI CCS embedded data media bus code > > > (MEDIA_BUS_FMT_CCS_EMBEDDED). > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > --- > > > .../media/v4l/subdev-formats.rst | 28 +++++++++++++++++++ > > > include/uapi/linux/media-bus-format.h | 3 ++ > > > 2 files changed, 31 insertions(+) > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst > > > index cbd475f7cae9..c8f982411e70 100644 > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst > > > @@ -8564,3 +8564,31 @@ and finally the bit number in subscript. "X" indicates a padding bit. > > > - X > > > - X > > > - X > > > + > > > +.. _MEDIA-BUS-FMT-CCS-EMBEDDED: > > > + > > > +MIPI CCS Embedded Data Formats > > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > + > > > +`MIPI CCS <https://www.mipi.org/specifications/camera-command-set>`_ defines a > > > +metadata format for sensor embedded data, which is used to store the register > > > +configuration used for capturing a given frame. The format is defined in the CCS > > > +specification. The media bus code for this format is > > > +``MEDIA_BUS_FMT_CCS_EMBEDDED``. > > > + > > > +The CCS embedded data format definition includes three levels: > > > + > > > +1. Padding within CSI-2 bus :ref:`Data unit <media-glossary-data-unit>` as > > > > s/Data unit/Data Unit/ > > Yes. > > > > + documented in the MIPI CCS specification. > > > + > > > +2. The tagged data format as documented in the MIPI CCS specification. > > > + > > > +3. Register addresses and register documentation as documented in the MIPI CCS > > > + specification. > > > + > > > +The format definition shall be used only by devices that fulfill all three > > > +levels above. > > > > Hmmmm... Do we need to mandate level 3 ? There are lots of sensors that > > comply with the first two levels but have their own register set. Would > > you like a sensor-specific embedded data format for each of them ? If > > so, how would we document it without essentially copying the datasheet ? > > You could refer to the datasheet if it's publicly available. What if it's not ? How would you expect documentation to be organized ? Will we then end up adding one media bus code per sensor ? > The CCS static data format is meaningful only for accessing (level 1) and > parsing (level 2) the data without knowing its meaning, which is the most > important aspect. The user space will need to know the meaning of the data > and can additionally select the same data unpacker than for CCS and > possibly also the CCS data parser if applicable. > > The user space should especially not have to have a list of CCS-compliant > sensors which would become necessary, should we skip on the 3rd level > requirement here. > > > > + > > > +This mbus code are only used for "2-byte simplified tagged data format" (code > > > +0xa) but their use may be extended further in the future, to cover other CCS > > > +embedded data format codes. > > > diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h > > > index d4c1d991014b..03f7e9ab517b 100644 > > > --- a/include/uapi/linux/media-bus-format.h > > > +++ b/include/uapi/linux/media-bus-format.h > > > @@ -183,4 +183,7 @@ > > > #define MEDIA_BUS_FMT_META_20 0x8006 > > > #define MEDIA_BUS_FMT_META_24 0x8007 > > > > > > +/* Specific metadata formats. Next is 0x9002. */ > > > +#define MEDIA_BUS_FMT_CCS_EMBEDDED 0x9001 > > > + > > > #endif /* __LINUX_MEDIA_BUS_FORMAT_H */
Hi Laurent, On Fri, Apr 12, 2024 at 10:07:46PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > On Thu, Apr 11, 2024 at 09:04:47AM +0000, Sakari Ailus wrote: > > On Thu, Mar 21, 2024 at 06:49:14PM +0200, Laurent Pinchart wrote: > > > On Wed, Mar 13, 2024 at 09:25:01AM +0200, Sakari Ailus wrote: > > > > Add new MIPI CCS embedded data media bus code > > > > (MEDIA_BUS_FMT_CCS_EMBEDDED). > > > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > --- > > > > .../media/v4l/subdev-formats.rst | 28 +++++++++++++++++++ > > > > include/uapi/linux/media-bus-format.h | 3 ++ > > > > 2 files changed, 31 insertions(+) > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst > > > > index cbd475f7cae9..c8f982411e70 100644 > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst > > > > @@ -8564,3 +8564,31 @@ and finally the bit number in subscript. "X" indicates a padding bit. > > > > - X > > > > - X > > > > - X > > > > + > > > > +.. _MEDIA-BUS-FMT-CCS-EMBEDDED: > > > > + > > > > +MIPI CCS Embedded Data Formats > > > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > + > > > > +`MIPI CCS <https://www.mipi.org/specifications/camera-command-set>`_ defines a > > > > +metadata format for sensor embedded data, which is used to store the register > > > > +configuration used for capturing a given frame. The format is defined in the CCS > > > > +specification. The media bus code for this format is > > > > +``MEDIA_BUS_FMT_CCS_EMBEDDED``. > > > > + > > > > +The CCS embedded data format definition includes three levels: > > > > + > > > > +1. Padding within CSI-2 bus :ref:`Data unit <media-glossary-data-unit>` as > > > > > > s/Data unit/Data Unit/ > > > > Yes. > > > > > > + documented in the MIPI CCS specification. > > > > + > > > > +2. The tagged data format as documented in the MIPI CCS specification. > > > > + > > > > +3. Register addresses and register documentation as documented in the MIPI CCS > > > > + specification. > > > > + > > > > +The format definition shall be used only by devices that fulfill all three > > > > +levels above. > > > > > > Hmmmm... Do we need to mandate level 3 ? There are lots of sensors that > > > comply with the first two levels but have their own register set. Would > > > you like a sensor-specific embedded data format for each of them ? If > > > so, how would we document it without essentially copying the datasheet ? > > > > You could refer to the datasheet if it's publicly available. > > What if it's not ? How would you expect documentation to be organized ? > Will we then end up adding one media bus code per sensor ? Probably. The user space needs a way to find out what to expect. If a sensor produces its own custom metadata format, it needs to be documented. There's also documentation for OV2740 embedded data in the set.
Hi Sakari, On Sun, Apr 14, 2024 at 10:48:49AM +0000, Sakari Ailus wrote: > On Fri, Apr 12, 2024 at 10:07:46PM +0300, Laurent Pinchart wrote: > > On Thu, Apr 11, 2024 at 09:04:47AM +0000, Sakari Ailus wrote: > > > On Thu, Mar 21, 2024 at 06:49:14PM +0200, Laurent Pinchart wrote: > > > > On Wed, Mar 13, 2024 at 09:25:01AM +0200, Sakari Ailus wrote: > > > > > Add new MIPI CCS embedded data media bus code > > > > > (MEDIA_BUS_FMT_CCS_EMBEDDED). > > > > > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > --- > > > > > .../media/v4l/subdev-formats.rst | 28 +++++++++++++++++++ > > > > > include/uapi/linux/media-bus-format.h | 3 ++ > > > > > 2 files changed, 31 insertions(+) > > > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst > > > > > index cbd475f7cae9..c8f982411e70 100644 > > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst > > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst > > > > > @@ -8564,3 +8564,31 @@ and finally the bit number in subscript. "X" indicates a padding bit. > > > > > - X > > > > > - X > > > > > - X > > > > > + > > > > > +.. _MEDIA-BUS-FMT-CCS-EMBEDDED: > > > > > + > > > > > +MIPI CCS Embedded Data Formats > > > > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > > + > > > > > +`MIPI CCS <https://www.mipi.org/specifications/camera-command-set>`_ defines a > > > > > +metadata format for sensor embedded data, which is used to store the register > > > > > +configuration used for capturing a given frame. The format is defined in the CCS > > > > > +specification. The media bus code for this format is > > > > > +``MEDIA_BUS_FMT_CCS_EMBEDDED``. > > > > > + > > > > > +The CCS embedded data format definition includes three levels: > > > > > + > > > > > +1. Padding within CSI-2 bus :ref:`Data unit <media-glossary-data-unit>` as > > > > > > > > s/Data unit/Data Unit/ > > > > > > Yes. > > > > > > > > + documented in the MIPI CCS specification. > > > > > + > > > > > +2. The tagged data format as documented in the MIPI CCS specification. > > > > > + > > > > > +3. Register addresses and register documentation as documented in the MIPI CCS > > > > > + specification. > > > > > + > > > > > +The format definition shall be used only by devices that fulfill all three > > > > > +levels above. > > > > > > > > Hmmmm... Do we need to mandate level 3 ? There are lots of sensors that > > > > comply with the first two levels but have their own register set. Would > > > > you like a sensor-specific embedded data format for each of them ? If > > > > so, how would we document it without essentially copying the datasheet ? > > > > > > You could refer to the datasheet if it's publicly available. > > > > What if it's not ? How would you expect documentation to be organized ? > > Will we then end up adding one media bus code per sensor ? > > Probably. The user space needs a way to find out what to expect. If a > sensor produces its own custom metadata format, it needs to be documented. > > There's also documentation for OV2740 embedded data in the set. The OV2740 is a different case than what I had in mind. It uses level 1 of the CCS embedded data only, without using the CCS tagged format. The documentation you've included in this patch series looks good to me. For sensors that are compatible with CCS embedded data level 2, I'm not sure how to proceed. Those sensors send (register, value) pairs in their embedded data, but the register set is not CCS-compatible. Even when documentation is available, it's hard to verify that the sensor will never output the value of undocumented registers, as the registers included in the embedded data may vary depending on the sensor configuration. How would you proceed in such cases ? I can try to write a sample documentation patch, but I don't know what you expect. I'm thinking about the IMX219 sensor for instance, but any sensor compatible with CCS embedded data level 2 without a public datasheet is a good example.
diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst index cbd475f7cae9..c8f982411e70 100644 --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst @@ -8564,3 +8564,31 @@ and finally the bit number in subscript. "X" indicates a padding bit. - X - X - X + +.. _MEDIA-BUS-FMT-CCS-EMBEDDED: + +MIPI CCS Embedded Data Formats +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +`MIPI CCS <https://www.mipi.org/specifications/camera-command-set>`_ defines a +metadata format for sensor embedded data, which is used to store the register +configuration used for capturing a given frame. The format is defined in the CCS +specification. The media bus code for this format is +``MEDIA_BUS_FMT_CCS_EMBEDDED``. + +The CCS embedded data format definition includes three levels: + +1. Padding within CSI-2 bus :ref:`Data unit <media-glossary-data-unit>` as + documented in the MIPI CCS specification. + +2. The tagged data format as documented in the MIPI CCS specification. + +3. Register addresses and register documentation as documented in the MIPI CCS + specification. + +The format definition shall be used only by devices that fulfill all three +levels above. + +This mbus code are only used for "2-byte simplified tagged data format" (code +0xa) but their use may be extended further in the future, to cover other CCS +embedded data format codes. diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h index d4c1d991014b..03f7e9ab517b 100644 --- a/include/uapi/linux/media-bus-format.h +++ b/include/uapi/linux/media-bus-format.h @@ -183,4 +183,7 @@ #define MEDIA_BUS_FMT_META_20 0x8006 #define MEDIA_BUS_FMT_META_24 0x8007 +/* Specific metadata formats. Next is 0x9002. */ +#define MEDIA_BUS_FMT_CCS_EMBEDDED 0x9001 + #endif /* __LINUX_MEDIA_BUS_FORMAT_H */
Add new MIPI CCS embedded data media bus code (MEDIA_BUS_FMT_CCS_EMBEDDED). Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- .../media/v4l/subdev-formats.rst | 28 +++++++++++++++++++ include/uapi/linux/media-bus-format.h | 3 ++ 2 files changed, 31 insertions(+)