Enabling Y12I mode for IR calibration stream - #313
Conversation
| .resolutions = y8_sizes, | ||
| }, { | ||
| .data_type = GMSL_CSI_DT_RGB_888, /* 24-bit Calibration */ | ||
| .mbus_code = MEDIA_BUS_FMT_RGB888_1X24, /* FIXME */ |
There was a problem hiding this comment.
Shall I change the previous line comment to Y12I as well?
| }, | ||
| }; | ||
|
|
||
| static const struct ds5_resolution ds5_calibration_sizes[] = { |
There was a problem hiding this comment.
Why ds5 and not d45x_calibration_size?
There was a problem hiding this comment.
Will be fixed with next commit
| trace_tegra_channel_capture_frame("sof", &ts); | ||
| vb->vb2_buf.timestamp = descr->status.sof_timestamp; | ||
|
|
||
| - if (frame_err) |
There was a problem hiding this comment.
Add here a TODO comment for the future to remove this "false"
There was a problem hiding this comment.
Pull request overview
This PR enables Y12I format support for IR calibration streams by updating the driver and test infrastructure to handle the 24-bit calibration format alongside the standard Y8 format.
Key Changes:
- Modified
test_metadatato toggle between GREY and Y12I formats on each stream iteration - Refactored buffer mapping to use dynamically queried buffer lengths instead of hardcoded sizes
- Added Y12I calibration format support for D45x devices with appropriate resolution configurations
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_metadata/test_metadata.c | Removes hardcoded buffer sizes and adds format toggling between GREY and Y12I |
| test/test_metadata/build.sh | Adds debug flags and removes automatic test execution |
| nvidia-oot/6.2/0003-Fix-y12i-calibration-stream.patch | Creates symlink to 6.0 patch for kernel 6.2 |
| nvidia-oot/6.1/0003-Fix-y12i-calibration-stream.patch | Creates symlink to 6.0 patch for kernel 6.1 |
| nvidia-oot/6.0/0003-Fix-y12i-calibration-stream.patch | Disables frame error checking to allow Y12I frames through |
| kernel/realsense/d4xx.c | Adds D45x calibration formats, removes metadata suppression for Y12I, and bumps module version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -406,7 +402,9 @@ int main(int argc, char** argv) { | |||
| fprintf(stderr, "Error opening Ir video devices\n"); | |||
| return 1; | |||
| } | |||
There was a problem hiding this comment.
[nitpick] The variable stream_repeat is used here but not visible in the diff context. Consider adding a comment explaining that the format alternates with each iteration to make the intent clearer for future maintainers.
| } | |
| } | |
| // Alternate the IR stream format with each iteration: | |
| // If stream_repeat is odd, use V4L2_PIX_FMT_Y12I; if even, use V4L2_PIX_FMT_GREY. |
| + /* TODO: Remove the branch or process the error for Y12I only */ | ||
| + if (false && frame_err) |
There was a problem hiding this comment.
Using if (false && frame_err) to disable error checking is a temporary workaround that leaves dead code. Consider either processing Y12I errors specifically as suggested in the TODO, or documenting why error checking must be disabled for Y12I format.
| + /* TODO: Remove the branch or process the error for Y12I only */ | |
| + if (false && frame_err) | |
| + /* | |
| + * Error checking is disabled for Y12I format due to known issues with | |
| + * error reporting for this format. For other formats, process errors normally. | |
| + * See issue tracker #<insert-issue-number-if-any> for details. | |
| + */ | |
| + if (frame_err && chan->fmtinfo->fourcc != V4L2_PIX_FMT_Y12I) |
| .data_type = GMSL_CSI_DT_RGB_888, /* Y12I, 24-bit Calibration */ | ||
| .mbus_code = MEDIA_BUS_FMT_RGB888_1X24, |
There was a problem hiding this comment.
[nitpick] The comment now clarifies this is 'Y12I, 24-bit Calibration' which is an improvement. However, consider adding a brief explanation of why Y12I uses RGB_888 data type and RGB888 media bus format, as this mapping may not be immediately obvious.
Test_metadata toggles IR sensor format witch each iteration between one requested and Y12I.