Hi Laurent On Tue, Jul 04, 2023 at 05:56:16PM +0300, Laurent Pinchart wrote: > 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. Ah right! Should I state that for this reasons sensor's that are affected by color pattern reordering should disable changing the value of flips during streaming ? Thanks j > > 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
