On Tue, Jul 04, 2023 at 12:03:00PM +0100, Dave Stevenson wrote: > On Tue, 4 Jul 2023 at 11:24, Jacopo Mondi wrote: > > On Tue, Jul 04, 2023 at 10:58:11AM +0100, Dave Stevenson wrote: > > > Hi Jacopo > > > > > > Thanks for adding documentation. > > > Sorry I couldn't be at your presentation, but I'll find the slides > > > (and recording if there is one). > > > > videos and slides should be already available for attendees. If not I > > can send you the slide deck, but trust me, there is nothing new for > > you there. > > > > > On Mon, 3 Jul 2023 at 21:29, Jacopo Mondi wrote: > > > > > > > > Document the requirement of notifying to userspace the possible > > > > re-ordering of the color sample components when a vertical or horizontal > > > > flip is applied to a RAW camera sensor. > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> > > > > --- > > > > Documentation/driver-api/media/camera-sensor.rst | 16 ++++++++++++++++ > > > > 1 file changed, 16 insertions(+) > > > > > > > > diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst > > > > index 93f4f2536c25..ee4a7fe5f72a 100644 > > > > --- a/Documentation/driver-api/media/camera-sensor.rst > > > > +++ b/Documentation/driver-api/media/camera-sensor.rst > > > > @@ -173,3 +173,19 @@ V4L2_CID_VFLIP controls with the 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. > > > > + > > > > +Flip handling for raw camera sensors > > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > Possibly "for colour raw camera sensors". > > > Mono sensors are still raw in that they need processing for black > > > level, lens shading, etc, but they won't change the colour ordering. > > > > Thanks, good point > > > > > > + > > > > +Applying vertical and horizontal flips on raw camera sensors inverts the color > > > > +sample reading direction on the sensor's pixel array. This causes the > > > > +re-ordering of the color samples on the sensor's output frame. > > > > > > This *may* cause the re-ordering.... > > > > > > Not all sensors do. Some shift the readout by one line/column to keep > > > the Bayer order the same, and technically should update the selection. > > > OnSemi sensors in particular seem to do this, as do the Sony > > > IMX327/290/462 family. > > > > Is it the driver doing the shift or is it the sensor automatically > > adjusting ? > > The sensor does it. Both cases exist, some drivers do the adjustments internally. That's not something I would recommend, but it was implemented to allow changing the flip controls during streaming as we lacked (and still lack) a solution to this problem. There are also sensors that make this behaviour configurable, with a register bit to indicate if the crop rectangle should be adjusted automatically. While moving the crop rectangle by one pixel in a large resolution sensor could seem difficult to notice, it can have very noticeable effects. For instance, defective pixel correction that operates on a list of known bad pixels will require adjustement. > For IMX290, the array is defined horizontally as: > 4 ignored pixels > 8 pixels margin for colour processing > 1920 recording pixel area > 9 pixels margin for colour processing > 4 ignored pixels > (3 dummy pixels) Do you know what those dummy pixels are, and how they differ from the ignored pixels that are not supposed to be read out either ? > Ignoring the dummy ones, that gives a width of 1945 pixels. > > Vertically is similar with 1109 pixels total (9 colour margin / 1080 > active / 8 colour margin). > > That means it is a red pixel in all 4 corners of the full 1945x1109 > array, and it keeps the 8 pixels margin on the left of the image read > out in all circumstances, and 8 at the top too so that readout always > follows that. > It's slightly weird as a sensor in that it has presets for 1080p and > 720p, as well as a windowed mode where you can arbitrarily crop from > the whole array. The driver only uses the two presets, so exactly what > is going on is partly reverse engineering. > > With regard OnSemi, we asked them about it several years ago, and the > response was that there was no way to change that behaviour for the > sensor we were looking at. > From that datasheet: > "While the sensor’s format is W × H, the additional active columns and > active rows are included for use when horizontal or vertical mirrored > readout is enabled, to allow readout to start on the same pixel. The > pixel adjustment is always performed for monochrome or color versions" > Definition of the VERT_FLIP bit > "1: Readout is flipped (mirrored) vertically so that the row specified > by y_addr_end_ (+1) is read out of the sensor first" > and HORIZ_MIRROR bit > "1: Readout is mirrored horizontally so that the column specified by > x_addr_end_ (+1) is read out of the sensor first." > > Having just checked a different OnSemi datasheet, that doesn't state > that it alters the pixel read out first when flipped. It possibly > depends on the target market for the sensor, and how configurable they > anticipate the ISPs to be. > > > > > As an example, a > > > > +raw camera sensor with a Bayer pattern color filter array and a native RGGB > > > > +Bayer order will produce frames with GRBG component ordering when a vertical > > > > +flip is applied. > > > > > > Vertical flip of RGGB would be GBRG as the RG and GB get swapped, not > > > GRBG (which would be horizontal flip). > > > > I clearly mean horizontal sorry. I'll fix. > > > > > > Camera sensor drivers where inverting the reading order > > > > +direction causes a re-ordering of the color components are requested to register > > > > +the ``V4L2_CID_VFLIP`` and ``V4L2_CID_HFLIP`` controls with the > > > > +``V4L2_CTRL_FLAG_MODIFY_LAYOUT`` flag enabled to notify userspace that enabling > > > > +a flip can potentially change the output buffer content layout. Flips should > > > > +also be taken into account when enumerating and handling media bus formats > > > > +on the camera sensor source pads. -- Regards, Laurent Pinchart
