| Message ID | 20231017105630.558089-1-sakari.ailus@linux.intel.com |
|---|---|
| Headers | show |
| Series | Document link-frequencies better, small fixes | expand |
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 > ~~~~~~~~~~~~~~~~~~ >
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
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 >