[TESTING] kbingham/rpi 6.16.y/imx283 fixes #7066
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
I haven't posted these upstream yet - sharing here to get them tested by known IMX283 users on RPi ... but no specific desire to merge this into the RPi tree until reviewed upstream. But they could be if testers find the series beneficial. |
|
SRGGB12_CSI2P,5592x3710 mode doesn't seems right, but all other modes are working fine: 5496x3672: |
|
Ah it looks like SRGGB10_CSI2P,5472x3648/0 this mode isn't working also, there is no error message but the frame is empty. |
|
Here are the jpeg images for all the modes =========10-bit Mode ============ |
Contributor
Author
|
Thank you - I hadn't noticed 10 bit was broken - but thanks to your test I can see the bug. I'll fix that. |
Contributor
Author
I think this is almost expected behaviour - I was expecting 'pink' because the bayer order ends up being mis-interpreted/mis-aligned - though I don't know why it goes quite so psychedelic through the ISP ... |
popcornmix
force-pushed
the
rpi-6.16.y
branch
2 times, most recently
from
32bcefd to
cfc6d51
Compare
popcornmix
force-pushed
the
rpi-6.16.y
branch
from
923a71e to
999cdc6
Compare
The target crop rectangle is initialized with the crop of the default sensor mode. This is incorrect when a different sensor mode gets selected. Fix that by updating the crop rectangle when changing the sensor mode. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
When the code requested by imx283_set_pad_format() is not supported, a kernel exception occurs due to dereferencing the mode variable which is null. Fix that by correcting the code to a valid value before getting the mode table. While at it, remove the cases for the other unsupported codes in get_mode_table. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Move the struct imx283_mode further down in the compilation unit so that it can make reference of the scan out mode structures which are presently defined after. No functional change intended in this commit. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Move the common data structures to a new scanout table and allow v4l2 output modes to reference their scanout. This removes duplication from the mode definitions. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The horizontal optical black values are not used as we do not enable HOB output - and instead directly use HTRIMMING to request the desired horizontal cropping position. Remove the unused reference to simplify. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The Vertical Optical Black region is a property of the selected scan mode. Move the storage of this property to the scan mode table so it does not get duplicated when adding new output modes. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The vertical cropping parameters are specific to the readout mode selected and do not need to be duplicated on v4l2 mode choices. Move them to the imx283_scanout definitions. This also fixes the 10bit mode which had not yet defined the veff correctly and worked by chance. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The IMX283 has different vertical offsets when applying binning modes. To provide consistent framing in each mode - ensure that the offsets measured are accounted for. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Provide a common reference for the recommended recording area to use in each of the binning modes. No functional change intended in this commit. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Adapt the Horiztonal configuration into its own scope to improve readability of these two associated register configurations. No functional change intended in this commit. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reduce the scope of the 5 registers used for vertical positioning to make it easier to maintain and calculate the exact vertical configuration. No functional changes intended in this patch, which simplifies later development. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Refactor the v_pos to separate out the vflip handling from the top coordinate. No functional change is intended in this commit. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The binning factors are determined by the chosen scan mode. Move the definition of the binning ratio to the scan mode strutures and remove from the v4l2 output mode definitions. The horizontal binning ratio is not used and therefore is dropped. This also fixes the 10-bit mode handling which previously had an undefined vbin_ratio for MODE1. V4L2 does not currently expose an API to support the differences between binning and skipping, so while the mode capabilities are kept for the skipping modes - there is no definition to use them yet. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The minimum exposure is a factor of the scan mode. Move the definitions of the minimum exposure timeto the scan mode structures and clean up the duplication from the v4l2 output mode definitions. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The vertical handling cuts unused lines to restrict the output to the desired height. It can be valid to set y_outsize larger than the veff to include optical black regions in the visible image. Ensure that the cut is clamped to the veff to prevent underflow. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The user clamp region is included in the pixel native area coordinates but is not included in the sensor vertical coordinates when configuring the VWINPOS. Remove the 16 line offset from the top position to account for this and in the event that a crop position requested vertical optical black, reduce the OB_SIZE_V register which causes those lines to be output in the main image data type instead. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The IMX283 introduces an extra line on the all-pixel scan out modes to prevent bayer re-ordering on flip handling. This is undesireable as it introduces the line in all crop configurations when the image is not flipped. The OB_SIZE_V register determines how many lines from the output will be directed into the custom data type for optical black region. To overcome the extra line which is forcefully added, utilise the vertical optical black region to redirect the extra line. An additional line also needs to be redirected to once again retain the bayer order. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The size should represent the amount of lines to cut vertically and exclude from the pixel array for the image data. Keeping this at the expected value causes corruption on the last line. Reduce by increasing the defined size of the image by a single line to ensure that the data is valid for the final output. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Provide a mode that outputs all pixels from the full native array. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Add a mode to support output of the full illuminated area of the pixel array. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Provide a mode that includes all effective pixels which includes a 12 pixel margin for colour processing on all edges. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The exposure control on the imx283 is handled through the SHR register. This value is calculated based upon the hmax and vmax registers as a property of the total line and frame length. Ensure that the SHR is updated whenever the blankings update and adjust the frame intervals to ensure the correct exposure is configured on the sensor. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The IMX283 supports binning modes which combine multiple measured pixels into a single output pixel. The minimum timings for this must account for the operations on all measured pixels, not the output pixel sizes. Determine and calculate all hmax and vmax values in respect of the HBLANK and VBLANK controls against the native resolution of the mode as specified in the mode crop rectangle as opposed to the output mode width and height. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The exposure ranges are updated currently whenever the VBLANK control is updated. However, the total exposure limits are a factor of both the HBLANK and the VBLANK durations. Determine the hmax value any time the control is set, including if the device is not yet streaming and update the limits accordingly. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The VFLIP control is configured through the MDVREV bit of the HTRIMMING register. Simplify the conditional branch from 5 lines to 3 by determining the trim value which must always include setting the HTRIMMING_EN bit, and conditionally set the MDVREV before applying it to the hardware. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
kbingham
force-pushed
the
kbingham/rpi-6.16.y/imx283-fixes
branch
from
35ce301 to
3d804a8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some redesign of the imx283 mode configuration that allows us to add modes and configurations more easily. Ultimately - I hope to progress this sometime to support free arbitrary cropping on the sensor side with the driver handling all the alignment and restrictions correctly - but that will take some more time still yet.
Crucially in this series is some changes to the handling of the user clamp area which contains the VOB but isn't actually part of the sensor vertical position coordinates. Previously - this means that a 16 line offset has been occuring in the output image.
Another artifact is that the sensor introduces an extra line at the top of the image which we believe is to support or prevent bayer reordering on vflip. But the additional line produces an undesirable artifact in the output. This series uses the ability to adjust the clamp area/VOB to extend over the additional line (and one more to maintain bayer order) to remove the artifact. To prevent this causing the image to be offset - an extra two lines are requested on top of the mode first.
Finally - three clear new modes are added
I'd still like to get the binning and timing values out of the crop mode structure - but that will take some more work - and we don't yet have an api to control binning / skipping which makes it difficult to convey in the scanout mode structures.