[PATCH v2 0/3] Document link-frequencies better, small fixes

[PATCH v2 0/3] Document link-frequencies better, small fixes

[Sakari Ailus]

Hi folks,

Here are a few small documentation improvements.

since v1:

- Fix reference.

Sakari Ailus (3):
  media: Documentation: Document how link frequencies can be chosen
  media: Documentation: BT.601 is not a bus
  media: Documentation: LP-11 and LP-111 are states, not modes

 .../driver-api/media/camera-sensor.rst        | 18 ++++++++++++++++--
 Documentation/driver-api/media/tx-rx.rst      | 19 +++++++++----------
 .../media/drivers/camera-sensor.rst           |  2 ++
 3 files changed, 27 insertions(+), 12 deletions(-)

-- 
2.39.2

[PATCH v2 1/3] media: Documentation: Document how link frequencies can be chosen

[Sakari Ailus]

Document how link frequencies can be selected for the link-frequencies
property.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/driver-api/media/camera-sensor.rst   | 14 ++++++++++++++
 .../userspace-api/media/drivers/camera-sensor.rst  |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
index 6456145f96ed..0de5c86cbd1f 100644
--- a/Documentation/driver-api/media/camera-sensor.rst
+++ b/Documentation/driver-api/media/camera-sensor.rst
@@ -29,6 +29,20 @@ used in the system. Using another frequency may cause harmful effects
 elsewhere. Therefore only the pre-determined frequencies are configurable by the
 user.
 
+On choosing link frequencies
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Choosing link frequencies for a board is generally a part of the hardware design
+process as one needs to ensure an EMC-safe frequency the sensor supports with
+the given external clock frequency exists. On development systems this may be
+less than an immediate concern, so more or less anything that sensor and the
+rest of the applicable hardware supports can be used.
+
+If the sensor's PLL tree is not documented and all that is available are
+register lists, even knowing the frequency a driver uses may be difficult. This
+could still be :ref:`calculated from the number of lanes, sensor's output image
+size, blanking values and frame rate <media_camera_raw_frame_interval>`.
+
 ACPI
 ~~~~
 
diff --git a/Documentation/userspace-api/media/drivers/camera-sensor.rst b/Documentation/userspace-api/media/drivers/camera-sensor.rst
index 919a50e8b9d9..e0596b85e7ec 100644
--- a/Documentation/userspace-api/media/drivers/camera-sensor.rst
+++ b/Documentation/userspace-api/media/drivers/camera-sensor.rst
@@ -44,6 +44,8 @@ There are two different methods for obtaining possibilities for different frame
 intervals as well as configuring the frame interval. Which one to implement
 depends on the type of the device.
 
+.. _media_camera_raw_frame_interval:
+
 Raw camera sensors
 ~~~~~~~~~~~~~~~~~~
 
-- 
2.39.2

[PATCH v2 2/3] media: Documentation: BT.601 is not a bus

[Sakari Ailus]

BT.601 is not actually a bus specification, leaving parallel bus without a
specification to refer to. Fix this.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/driver-api/media/camera-sensor.rst | 4 ++--
 Documentation/driver-api/media/tx-rx.rst         | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
index 0de5c86cbd1f..19f2feeecc91 100644
--- a/Documentation/driver-api/media/camera-sensor.rst
+++ b/Documentation/driver-api/media/camera-sensor.rst
@@ -9,8 +9,8 @@ This document covers the in-kernel APIs only. For the best practices on
 userspace API implementation in camera sensor drivers, please see
 :ref:`media_using_camera_sensor_drivers`.
 
-CSI-2 and parallel (BT.601 and BT.656) busses
----------------------------------------------
+CSI-2 and parallel and BT.656 buses
+-----------------------------------
 
 Please see :ref:`transmitter-receiver`.
 
diff --git a/Documentation/driver-api/media/tx-rx.rst b/Documentation/driver-api/media/tx-rx.rst
index e1e9258dd862..7e115e3c4735 100644
--- a/Documentation/driver-api/media/tx-rx.rst
+++ b/Documentation/driver-api/media/tx-rx.rst
@@ -25,9 +25,8 @@ the host SoC. It is defined by the `MIPI alliance`_.
 Parallel
 ^^^^^^^^
 
-`BT.601`_ and `BT.656`_ are the most common parallel busses.
+The parallel bus and its `BT.656`_ variant are the most common parallel busses.
 
-.. _`BT.601`: https://en.wikipedia.org/wiki/Rec._601
 .. _`BT.656`: https://en.wikipedia.org/wiki/ITU-R_BT.656
 
 Transmitter drivers
-- 
2.39.2

[PATCH v2 3/3] media: Documentation: LP-11 and LP-111 are states, not modes

[Sakari Ailus]

LP-11 and LP-111 are CSI-2 bus states, not modes. Fix this.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/driver-api/media/tx-rx.rst | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/driver-api/media/tx-rx.rst b/Documentation/driver-api/media/tx-rx.rst
index 7e115e3c4735..bc1b94cffdd9 100644
--- a/Documentation/driver-api/media/tx-rx.rst
+++ b/Documentation/driver-api/media/tx-rx.rst
@@ -89,13 +89,13 @@ where
 	pixel rate on the camera sensor's pixel array which is indicated by the
 	:ref:`V4L2_CID_PIXEL_RATE <v4l2-cid-pixel-rate>` control.
 
-LP-11 and LP-111 modes
-^^^^^^^^^^^^^^^^^^^^^^
+LP-11 and LP-111 states
+^^^^^^^^^^^^^^^^^^^^^^^
 
-As part of transitioning to high speed mode, a CSI-2 transmitter typically
+As part of transitioning to high speed state, a CSI-2 transmitter typically
 briefly sets the bus to LP-11 or LP-111 state, depending on the PHY. This period
 may be as short as 100 µs, during which the receiver observes this state and
-proceeds its own part of high speed mode transition.
+proceeds its own part of high speed state transition.
 
 Most receivers are capable of autonomously handling this once the software has
 configured them to do so, but there are receivers which require software
@@ -104,7 +104,7 @@ in software, especially when there is no interrupt telling something is
 happening.
 
 One way to address this is to configure the transmitter side explicitly to LP-11
-or LP-111 mode, which requires support from the transmitter hardware. This is
+or LP-111 state, which requires support from the transmitter hardware. This is
 not universally available. Many devices return to this state once streaming is
 stopped while the state after power-on is LP-00 or LP-000.
 
@@ -115,11 +115,11 @@ transitioning to streaming state, but not yet start streaming. Similarly, the
 to call ``.post_streamoff()`` for each successful call of ``.pre_streamon()``.
 
 In the context of CSI-2, the ``.pre_streamon()`` callback is used to transition
-the transmitter to the LP-11 or LP-111 mode. This also requires powering on the
+the transmitter to the LP-11 or LP-111 state. This also requires powering on the
 device, so this should be only done when it is needed.
 
-Receiver drivers that do not need explicit LP-11 or LP-111 mode setup are waived
-from calling the two callbacks.
+Receiver drivers that do not need explicit LP-11 or LP-111 state setup are
+waived from calling the two callbacks.
 
 Stopping the transmitter
 ^^^^^^^^^^^^^^^^^^^^^^^^
-- 
2.39.2

Re: [PATCH v2 3/3] media: Documentation: LP-11 and LP-111 are states, not modes

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Tue, Oct 17, 2023 at 01:56:30PM +0300, Sakari Ailus wrote:
> LP-11 and LP-111 are CSI-2 bus states, not modes. Fix this.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  Documentation/driver-api/media/tx-rx.rst | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/driver-api/media/tx-rx.rst b/Documentation/driver-api/media/tx-rx.rst
> index 7e115e3c4735..bc1b94cffdd9 100644
> --- a/Documentation/driver-api/media/tx-rx.rst
> +++ b/Documentation/driver-api/media/tx-rx.rst
> @@ -89,13 +89,13 @@ where
>  	pixel rate on the camera sensor's pixel array which is indicated by the
>  	:ref:`V4L2_CID_PIXEL_RATE <v4l2-cid-pixel-rate>` control.
>  
> -LP-11 and LP-111 modes
> -^^^^^^^^^^^^^^^^^^^^^^
> +LP-11 and LP-111 states
> +^^^^^^^^^^^^^^^^^^^^^^^

Indeed.

>  
> -As part of transitioning to high speed mode, a CSI-2 transmitter typically
> +As part of transitioning to high speed state, a CSI-2 transmitter typically

"high speed" is a mode according to the D-PHY specification.

>  briefly sets the bus to LP-11 or LP-111 state, depending on the PHY. This period
>  may be as short as 100 µs, during which the receiver observes this state and
> -proceeds its own part of high speed mode transition.
> +proceeds its own part of high speed state transition.

Same here.

>  
>  Most receivers are capable of autonomously handling this once the software has
>  configured them to do so, but there are receivers which require software
> @@ -104,7 +104,7 @@ in software, especially when there is no interrupt telling something is
>  happening.
>  
>  One way to address this is to configure the transmitter side explicitly to LP-11
> -or LP-111 mode, which requires support from the transmitter hardware. This is
> +or LP-111 state, which requires support from the transmitter hardware. This is

Ack.

With the two changes referring to high speed state dropped,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  not universally available. Many devices return to this state once streaming is
>  stopped while the state after power-on is LP-00 or LP-000.
>  
> @@ -115,11 +115,11 @@ transitioning to streaming state, but not yet start streaming. Similarly, the
>  to call ``.post_streamoff()`` for each successful call of ``.pre_streamon()``.
>  
>  In the context of CSI-2, the ``.pre_streamon()`` callback is used to transition
> -the transmitter to the LP-11 or LP-111 mode. This also requires powering on the
> +the transmitter to the LP-11 or LP-111 state. This also requires powering on the
>  device, so this should be only done when it is needed.
>  
> -Receiver drivers that do not need explicit LP-11 or LP-111 mode setup are waived
> -from calling the two callbacks.
> +Receiver drivers that do not need explicit LP-11 or LP-111 state setup are
> +waived from calling the two callbacks.
>  
>  Stopping the transmitter
>  ^^^^^^^^^^^^^^^^^^^^^^^^

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/3] media: Documentation: BT.601 is not a bus

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Tue, Oct 17, 2023 at 01:56:29PM +0300, Sakari Ailus wrote:
> BT.601 is not actually a bus specification, leaving parallel bus without a
> specification to refer to. Fix this.

I'm really annoyed there's no standard name for parallel buses :-(

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  Documentation/driver-api/media/camera-sensor.rst | 4 ++--
>  Documentation/driver-api/media/tx-rx.rst         | 3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> index 0de5c86cbd1f..19f2feeecc91 100644
> --- a/Documentation/driver-api/media/camera-sensor.rst
> +++ b/Documentation/driver-api/media/camera-sensor.rst
> @@ -9,8 +9,8 @@ This document covers the in-kernel APIs only. For the best practices on
>  userspace API implementation in camera sensor drivers, please see
>  :ref:`media_using_camera_sensor_drivers`.
>  
> -CSI-2 and parallel (BT.601 and BT.656) busses
> ----------------------------------------------
> +CSI-2 and parallel and BT.656 buses

CSI-2, parallel and BT.656 buses

> +-----------------------------------
>  
>  Please see :ref:`transmitter-receiver`.
>  
> diff --git a/Documentation/driver-api/media/tx-rx.rst b/Documentation/driver-api/media/tx-rx.rst
> index e1e9258dd862..7e115e3c4735 100644
> --- a/Documentation/driver-api/media/tx-rx.rst
> +++ b/Documentation/driver-api/media/tx-rx.rst
> @@ -25,9 +25,8 @@ the host SoC. It is defined by the `MIPI alliance`_.
>  Parallel
>  ^^^^^^^^
>  
> -`BT.601`_ and `BT.656`_ are the most common parallel busses.
> +The parallel bus and its `BT.656`_ variant are the most common parallel busses.

We use "parallel" to mean explicit sync signals in many places
(including APIs), and here it covers BT.656 too :-( This sentence is
fairly bad.

>  
> -.. _`BT.601`: https://en.wikipedia.org/wiki/Rec._601
>  .. _`BT.656`: https://en.wikipedia.org/wiki/ITU-R_BT.656
>  
>  Transmitter drivers

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/3] media: Documentation: Document how link frequencies can be chosen

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Tue, Oct 17, 2023 at 01:56:28PM +0300, Sakari Ailus wrote:
> Document how link frequencies can be selected for the link-frequencies
> property.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  Documentation/driver-api/media/camera-sensor.rst   | 14 ++++++++++++++
>  .../userspace-api/media/drivers/camera-sensor.rst  |  2 ++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> index 6456145f96ed..0de5c86cbd1f 100644
> --- a/Documentation/driver-api/media/camera-sensor.rst
> +++ b/Documentation/driver-api/media/camera-sensor.rst
> @@ -29,6 +29,20 @@ used in the system. Using another frequency may cause harmful effects
>  elsewhere. Therefore only the pre-determined frequencies are configurable by the
>  user.
>  
> +On choosing link frequencies
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Choosing link frequencies for a board is generally a part of the hardware design
> +process as one needs to ensure an EMC-safe frequency the sensor supports with
> +the given external clock frequency exists.

This is a bit hard to parse. I would write

Choosing link frequencies for a board is generally a part of the hardware design
process as one needs to select EMC-safe frequencies that the sensor supports with
the given external clock frequency.

> On development systems this may be
> +less than an immediate concern, so more or less anything that sensor and the
> +rest of the applicable hardware supports can be used.

True, but it still doesn't say what to pick :-)

Q: What link frequency do I put in DT for a development board?
A: Any frequency will do.
Q: 1Hz?
A: No, it has to be supported by the sensor
Q: How do I figure that out?
A: ...

And once the range (or list) of frequencies the driver supports (for a
given input clock frequency) will be known, the selection process is
still not totally straightforward, as it will have implications on what
resolutions and frame rates the sensor will be able to output. This is
complicated even further if the sensor can support different number of
data lanes.

> +
> +If the sensor's PLL tree is not documented and all that is available are
> +register lists, even knowing the frequency a driver uses may be difficult. This
> +could still be :ref:`calculated from the number of lanes, sensor's output image
> +size, blanking values and frame rate <media_camera_raw_frame_interval>`.
> +
>  ACPI
>  ~~~~
>  
> diff --git a/Documentation/userspace-api/media/drivers/camera-sensor.rst b/Documentation/userspace-api/media/drivers/camera-sensor.rst
> index 919a50e8b9d9..e0596b85e7ec 100644
> --- a/Documentation/userspace-api/media/drivers/camera-sensor.rst
> +++ b/Documentation/userspace-api/media/drivers/camera-sensor.rst
> @@ -44,6 +44,8 @@ There are two different methods for obtaining possibilities for different frame
>  intervals as well as configuring the frame interval. Which one to implement
>  depends on the type of the device.
>  
> +.. _media_camera_raw_frame_interval:
> +
>  Raw camera sensors
>  ~~~~~~~~~~~~~~~~~~
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/3] media: Documentation: Document how link frequencies can be chosen

[Sakari Ailus]

Hi Laurent,

On Tue, Oct 17, 2023 at 02:52:21PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Tue, Oct 17, 2023 at 01:56:28PM +0300, Sakari Ailus wrote:
> > Document how link frequencies can be selected for the link-frequencies
> > property.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  Documentation/driver-api/media/camera-sensor.rst   | 14 ++++++++++++++
> >  .../userspace-api/media/drivers/camera-sensor.rst  |  2 ++
> >  2 files changed, 16 insertions(+)
> > 
> > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> > index 6456145f96ed..0de5c86cbd1f 100644
> > --- a/Documentation/driver-api/media/camera-sensor.rst
> > +++ b/Documentation/driver-api/media/camera-sensor.rst
> > @@ -29,6 +29,20 @@ used in the system. Using another frequency may cause harmful effects
> >  elsewhere. Therefore only the pre-determined frequencies are configurable by the
> >  user.
> >  
> > +On choosing link frequencies
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +Choosing link frequencies for a board is generally a part of the hardware design
> > +process as one needs to ensure an EMC-safe frequency the sensor supports with
> > +the given external clock frequency exists.
> 
> This is a bit hard to parse. I would write
> 
> Choosing link frequencies for a board is generally a part of the hardware design
> process as one needs to select EMC-safe frequencies that the sensor supports with
> the given external clock frequency.

I'll use this in v3.

> 
> > On development systems this may be
> > +less than an immediate concern, so more or less anything that sensor and the
> > +rest of the applicable hardware supports can be used.
> 
> True, but it still doesn't say what to pick :-)
> 
> Q: What link frequency do I put in DT for a development board?
> A: Any frequency will do.
> Q: 1Hz?
> A: No, it has to be supported by the sensor
> Q: How do I figure that out?
> A: ...
> 
> And once the range (or list) of frequencies the driver supports (for a
> given input clock frequency) will be known, the selection process is
> still not totally straightforward, as it will have implications on what
> resolutions and frame rates the sensor will be able to output. This is
> complicated even further if the sensor can support different number of
> data lanes.

Ultimately this is always sensor specific: the PLL tree documentation is
the key here if the frequencies are not known otherwise. I guess one
guidance we could give is look what the driver supports but that is what
virtually every developers can figure out by themselves.

I'd say teaching mathematics is out of scope of this documentation.

> 
> > +
> > +If the sensor's PLL tree is not documented and all that is available are
> > +register lists, even knowing the frequency a driver uses may be difficult. This
> > +could still be :ref:`calculated from the number of lanes, sensor's output image
> > +size, blanking values and frame rate <media_camera_raw_frame_interval>`.
> > +
> >  ACPI
> >  ~~~~
> >  
> > diff --git a/Documentation/userspace-api/media/drivers/camera-sensor.rst b/Documentation/userspace-api/media/drivers/camera-sensor.rst
> > index 919a50e8b9d9..e0596b85e7ec 100644
> > --- a/Documentation/userspace-api/media/drivers/camera-sensor.rst
> > +++ b/Documentation/userspace-api/media/drivers/camera-sensor.rst
> > @@ -44,6 +44,8 @@ There are two different methods for obtaining possibilities for different frame
> >  intervals as well as configuring the frame interval. Which one to implement
> >  depends on the type of the device.
> >  
> > +.. _media_camera_raw_frame_interval:
> > +
> >  Raw camera sensors
> >  ~~~~~~~~~~~~~~~~~~
> >  
> 

-- 
Sakari Ailus

Re: [PATCH v2 1/3] media: Documentation: Document how link frequencies can be chosen

[Laurent Pinchart]

Hi Sakari,

On Tue, Oct 17, 2023 at 01:07:38PM +0000, Sakari Ailus wrote:
> On Tue, Oct 17, 2023 at 02:52:21PM +0300, Laurent Pinchart wrote:
> > On Tue, Oct 17, 2023 at 01:56:28PM +0300, Sakari Ailus wrote:
> > > Document how link frequencies can be selected for the link-frequencies
> > > property.
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > ---
> > >  Documentation/driver-api/media/camera-sensor.rst   | 14 ++++++++++++++
> > >  .../userspace-api/media/drivers/camera-sensor.rst  |  2 ++
> > >  2 files changed, 16 insertions(+)
> > > 
> > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> > > index 6456145f96ed..0de5c86cbd1f 100644
> > > --- a/Documentation/driver-api/media/camera-sensor.rst
> > > +++ b/Documentation/driver-api/media/camera-sensor.rst
> > > @@ -29,6 +29,20 @@ used in the system. Using another frequency may cause harmful effects
> > >  elsewhere. Therefore only the pre-determined frequencies are configurable by the
> > >  user.
> > >  
> > > +On choosing link frequencies
> > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > +
> > > +Choosing link frequencies for a board is generally a part of the hardware design
> > > +process as one needs to ensure an EMC-safe frequency the sensor supports with
> > > +the given external clock frequency exists.
> > 
> > This is a bit hard to parse. I would write
> > 
> > Choosing link frequencies for a board is generally a part of the hardware design
> > process as one needs to select EMC-safe frequencies that the sensor supports with
> > the given external clock frequency.
> 
> I'll use this in v3.
> 
> > > On development systems this may be
> > > +less than an immediate concern, so more or less anything that sensor and the
> > > +rest of the applicable hardware supports can be used.
> > 
> > True, but it still doesn't say what to pick :-)
> > 
> > Q: What link frequency do I put in DT for a development board?
> > A: Any frequency will do.
> > Q: 1Hz?
> > A: No, it has to be supported by the sensor
> > Q: How do I figure that out?
> > A: ...
> > 
> > And once the range (or list) of frequencies the driver supports (for a
> > given input clock frequency) will be known, the selection process is
> > still not totally straightforward, as it will have implications on what
> > resolutions and frame rates the sensor will be able to output. This is
> > complicated even further if the sensor can support different number of
> > data lanes.
> 
> Ultimately this is always sensor specific: the PLL tree documentation is
> the key here if the frequencies are not known otherwise. I guess one
> guidance we could give is look what the driver supports but that is what
> virtually every developers can figure out by themselves.
> 
> I'd say teaching mathematics is out of scope of this documentation.

I don't know how to compute the information. Can you teach me ? And
let's record it in the documentation.

> > > +
> > > +If the sensor's PLL tree is not documented and all that is available are
> > > +register lists, even knowing the frequency a driver uses may be difficult. This
> > > +could still be :ref:`calculated from the number of lanes, sensor's output image
> > > +size, blanking values and frame rate <media_camera_raw_frame_interval>`.
> > > +
> > >  ACPI
> > >  ~~~~
> > >  
> > > diff --git a/Documentation/userspace-api/media/drivers/camera-sensor.rst b/Documentation/userspace-api/media/drivers/camera-sensor.rst
> > > index 919a50e8b9d9..e0596b85e7ec 100644
> > > --- a/Documentation/userspace-api/media/drivers/camera-sensor.rst
> > > +++ b/Documentation/userspace-api/media/drivers/camera-sensor.rst
> > > @@ -44,6 +44,8 @@ There are two different methods for obtaining possibilities for different frame
> > >  intervals as well as configuring the frame interval. Which one to implement
> > >  depends on the type of the device.
> > >  
> > > +.. _media_camera_raw_frame_interval:
> > > +
> > >  Raw camera sensors
> > >  ~~~~~~~~~~~~~~~~~~
> > >  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/3] media: Documentation: Document how link frequencies can be chosen

[Sakari Ailus]

Hi Laurent,

On Tue, Oct 17, 2023 at 04:34:59PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tue, Oct 17, 2023 at 01:07:38PM +0000, Sakari Ailus wrote:
> > On Tue, Oct 17, 2023 at 02:52:21PM +0300, Laurent Pinchart wrote:
> > > On Tue, Oct 17, 2023 at 01:56:28PM +0300, Sakari Ailus wrote:
> > > > Document how link frequencies can be selected for the link-frequencies
> > > > property.
> > > > 
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > ---
> > > >  Documentation/driver-api/media/camera-sensor.rst   | 14 ++++++++++++++
> > > >  .../userspace-api/media/drivers/camera-sensor.rst  |  2 ++
> > > >  2 files changed, 16 insertions(+)
> > > > 
> > > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> > > > index 6456145f96ed..0de5c86cbd1f 100644
> > > > --- a/Documentation/driver-api/media/camera-sensor.rst
> > > > +++ b/Documentation/driver-api/media/camera-sensor.rst
> > > > @@ -29,6 +29,20 @@ used in the system. Using another frequency may cause harmful effects
> > > >  elsewhere. Therefore only the pre-determined frequencies are configurable by the
> > > >  user.
> > > >  
> > > > +On choosing link frequencies
> > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > +
> > > > +Choosing link frequencies for a board is generally a part of the hardware design
> > > > +process as one needs to ensure an EMC-safe frequency the sensor supports with
> > > > +the given external clock frequency exists.
> > > 
> > > This is a bit hard to parse. I would write
> > > 
> > > Choosing link frequencies for a board is generally a part of the hardware design
> > > process as one needs to select EMC-safe frequencies that the sensor supports with
> > > the given external clock frequency.
> > 
> > I'll use this in v3.
> > 
> > > > On development systems this may be
> > > > +less than an immediate concern, so more or less anything that sensor and the
> > > > +rest of the applicable hardware supports can be used.
> > > 
> > > True, but it still doesn't say what to pick :-)
> > > 
> > > Q: What link frequency do I put in DT for a development board?
> > > A: Any frequency will do.
> > > Q: 1Hz?
> > > A: No, it has to be supported by the sensor
> > > Q: How do I figure that out?
> > > A: ...
> > > 
> > > And once the range (or list) of frequencies the driver supports (for a
> > > given input clock frequency) will be known, the selection process is
> > > still not totally straightforward, as it will have implications on what
> > > resolutions and frame rates the sensor will be able to output. This is
> > > complicated even further if the sensor can support different number of
> > > data lanes.
> > 
> > Ultimately this is always sensor specific: the PLL tree documentation is
> > the key here if the frequencies are not known otherwise. I guess one
> > guidance we could give is look what the driver supports but that is what
> > virtually every developers can figure out by themselves.
> > 
> > I'd say teaching mathematics is out of scope of this documentation.
> 
> I don't know how to compute the information. Can you teach me ? And
> let's record it in the documentation.

You're certainly not in a position to state this: you've written a PLL
calculator for an Aptina sensor. It's certainly easier to come up with a
single configuration than writing code that comes up with a reasonable
configuration when only the input and output parameters are known.

The Aptina PLL calculator is much more simple than the CCS PLL calculator,
perhaps we should use it as an example?

> 
> > > > +
> > > > +If the sensor's PLL tree is not documented and all that is available are
> > > > +register lists, even knowing the frequency a driver uses may be difficult. This
> > > > +could still be :ref:`calculated from the number of lanes, sensor's output image
> > > > +size, blanking values and frame rate <media_camera_raw_frame_interval>`.
> > > > +
> > > >  ACPI
> > > >  ~~~~
> > > >  
> > > > diff --git a/Documentation/userspace-api/media/drivers/camera-sensor.rst b/Documentation/userspace-api/media/drivers/camera-sensor.rst
> > > > index 919a50e8b9d9..e0596b85e7ec 100644
> > > > --- a/Documentation/userspace-api/media/drivers/camera-sensor.rst
> > > > +++ b/Documentation/userspace-api/media/drivers/camera-sensor.rst
> > > > @@ -44,6 +44,8 @@ There are two different methods for obtaining possibilities for different frame
> > > >  intervals as well as configuring the frame interval. Which one to implement
> > > >  depends on the type of the device.
> > > >  
> > > > +.. _media_camera_raw_frame_interval:
> > > > +
> > > >  Raw camera sensors
> > > >  ~~~~~~~~~~~~~~~~~~
> > > >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,

Sakari Ailus

Re: [PATCH v2 2/3] media: Documentation: BT.601 is not a bus

[Sakari Ailus]

Hi Laurent,

Thanks for the review.

On Tue, Oct 17, 2023 at 02:38:53PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Tue, Oct 17, 2023 at 01:56:29PM +0300, Sakari Ailus wrote:
> > BT.601 is not actually a bus specification, leaving parallel bus without a
> > specification to refer to. Fix this.
> 
> I'm really annoyed there's no standard name for parallel buses :-(
> 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  Documentation/driver-api/media/camera-sensor.rst | 4 ++--
> >  Documentation/driver-api/media/tx-rx.rst         | 3 +--
> >  2 files changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
> > index 0de5c86cbd1f..19f2feeecc91 100644
> > --- a/Documentation/driver-api/media/camera-sensor.rst
> > +++ b/Documentation/driver-api/media/camera-sensor.rst
> > @@ -9,8 +9,8 @@ This document covers the in-kernel APIs only. For the best practices on
> >  userspace API implementation in camera sensor drivers, please see
> >  :ref:`media_using_camera_sensor_drivers`.
> >  
> > -CSI-2 and parallel (BT.601 and BT.656) busses
> > ----------------------------------------------
> > +CSI-2 and parallel and BT.656 buses
> 
> CSI-2, parallel and BT.656 buses

Yes.

> 
> > +-----------------------------------
> >  
> >  Please see :ref:`transmitter-receiver`.
> >  
> > diff --git a/Documentation/driver-api/media/tx-rx.rst b/Documentation/driver-api/media/tx-rx.rst
> > index e1e9258dd862..7e115e3c4735 100644
> > --- a/Documentation/driver-api/media/tx-rx.rst
> > +++ b/Documentation/driver-api/media/tx-rx.rst
> > @@ -25,9 +25,8 @@ the host SoC. It is defined by the `MIPI alliance`_.
> >  Parallel
> >  ^^^^^^^^
> >  
> > -`BT.601`_ and `BT.656`_ are the most common parallel busses.
> > +The parallel bus and its `BT.656`_ variant are the most common parallel busses.
> 
> We use "parallel" to mean explicit sync signals in many places
> (including APIs), and here it covers BT.656 too :-( This sentence is
> fairly bad.

I'll mention parallel and BT.656 separately.

> 
> >  
> > -.. _`BT.601`: https://en.wikipedia.org/wiki/Rec._601
> >  .. _`BT.656`: https://en.wikipedia.org/wiki/ITU-R_BT.656
> >  
> >  Transmitter drivers
> 

-- 
Regards,

Sakari Ailus

Re: [PATCH v2 3/3] media: Documentation: LP-11 and LP-111 are states, not modes

[Sakari Ailus]

Hi Laurent,

On Tue, Oct 17, 2023 at 02:36:09PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Tue, Oct 17, 2023 at 01:56:30PM +0300, Sakari Ailus wrote:
> > LP-11 and LP-111 are CSI-2 bus states, not modes. Fix this.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  Documentation/driver-api/media/tx-rx.rst | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/media/tx-rx.rst b/Documentation/driver-api/media/tx-rx.rst
> > index 7e115e3c4735..bc1b94cffdd9 100644
> > --- a/Documentation/driver-api/media/tx-rx.rst
> > +++ b/Documentation/driver-api/media/tx-rx.rst
> > @@ -89,13 +89,13 @@ where
> >  	pixel rate on the camera sensor's pixel array which is indicated by the
> >  	:ref:`V4L2_CID_PIXEL_RATE <v4l2-cid-pixel-rate>` control.
> >  
> > -LP-11 and LP-111 modes
> > -^^^^^^^^^^^^^^^^^^^^^^
> > +LP-11 and LP-111 states
> > +^^^^^^^^^^^^^^^^^^^^^^^
> 
> Indeed.
> 
> >  
> > -As part of transitioning to high speed mode, a CSI-2 transmitter typically
> > +As part of transitioning to high speed state, a CSI-2 transmitter typically
> 
> "high speed" is a mode according to the D-PHY specification.

Yes, indeed. I'll address these in v3.

> 
> >  briefly sets the bus to LP-11 or LP-111 state, depending on the PHY. This period
> >  may be as short as 100 µs, during which the receiver observes this state and
> > -proceeds its own part of high speed mode transition.
> > +proceeds its own part of high speed state transition.
> 
> Same here.
> 
> >  
> >  Most receivers are capable of autonomously handling this once the software has
> >  configured them to do so, but there are receivers which require software
> > @@ -104,7 +104,7 @@ in software, especially when there is no interrupt telling something is
> >  happening.
> >  
> >  One way to address this is to configure the transmitter side explicitly to LP-11
> > -or LP-111 mode, which requires support from the transmitter hardware. This is
> > +or LP-111 state, which requires support from the transmitter hardware. This is
> 
> Ack.
> 
> With the two changes referring to high speed state dropped,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

-- 
Regards,

Sakari Ailus