Alderalke branch to development. by dmipx · Pull Request #12060 · realsenseai/librealsense

dmipx · 2023-08-01T10:43:35Z

All ipu6 & jetson enhancements to backend.
Support for multiple DFU devices.

backend-v4l2: MPLANE supported for 1 plane IPU6 currently supports only one [1] plane backend-v4l2: Metadata for IPU6 with MPLANE [1] support. IPU6 currently supports only MPLANE streaming metadata as video stream resolution

…rt [LRS-572]

[ADL-P] LRS V4L2 Backend Enumerate video nodes by name [LRS-573] rs-enum.sh: - Create symbolic links for video nodes and for metadata nodes - /dev/video-rs-[<sensor>|<sensor>-md]-[camera-index] - Enable ipu6 link enumeration backend-v4l2.cpp: - scan for rs-enum links Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

rs-enum: add comments, change rgb to color. backend-v4l2: change rgb to color, comments, try/catch for throwing module. Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

rs_sensor: RS2_CAMERA_INFO_DFU_DEVICE_PATH Tracked-by: [DSO-18802] [ADL-P] D4xx DFU support for LRS Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

IPU6 d4xx udev rules for binding and enumeration [DSO-18804] Tracking by Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

Metadata structure will be changed in future kernel releases, expected from version > 6.4. This patch helps avoid system header file change with Sakari metadata patch. Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

… devices.

remibettan · 2023-08-01T12:07:53Z

scripts/rs-enum.sh

@@ -16,14 +16,30 @@
 #
 # Alderlake:
 #$ ./rs-enum.sh 


Please add some comment with explanation of the commands sent in this script

you are not commenting c++ code for every magic you do.
This is simple filtering, nothing specific can be commented. No magic involved.

I think Remi meant command line parameters
i.e.

# parse command line parameters # for '-i' parameter, print links only

remibettan · 2023-08-01T12:08:21Z

scripts/rs_ipu6_d457_bind.sh

@@ -0,0 +1,78 @@
+#!/bin/bash
+# Dependency: v4l-utils


Please add some comment with explanation of the commands sent in this script

src/ds/d400/d400-color.cpp

remibettan · 2023-08-01T12:15:23Z

src/linux/backend-v4l2.cpp

+            _offset = buf.m.offset;
+            if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+                _original_length = buf.m.planes[0].length;
+                _offset = buf.m.planes[0].m.mem_offset;


could a const value be used here instead of planes[0]?

as soon we support only one plane in multi-plane buffer we have to leave that like so.
We had no use case for multiple planes and no system to verify on.

remibettan · 2023-08-01T12:16:00Z

src/linux/backend-v4l2.cpp

-
+            if (_type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+                buf.m.planes = planes;
+                buf.length = 1;


please consider replacing by some const, and add some comment to explain

That is for single buffer on multiplane, we have no other cases to verify what can be passsed to it for now.

remibettan · 2023-08-01T12:17:24Z

src/linux/backend-v4l2.cpp

                                bool partial_frame = (!compressed_format && (buf.bytesused < buffer->get_full_length() - MAX_META_DATA_SIZE));
                                bool overflow_frame = (buf.bytesused ==  buffer->get_length_frame_only() + MAX_META_DATA_SIZE);
+                                if (_dev.buf_type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+                                    /* metadata size is one line of profile, temporary disable validation */


Could you implement a better solution for that, that will still detect partial frame and overflow frame?

in ipu6 case we will always have full frame as it padding it by itself.
We can work on this in future

remibettan · 2023-08-01T12:18:43Z

src/linux/backend-v4l2.cpp

                // Configure metadata format - try d4xx, then fallback to currently retrieve UVC default header of 12 bytes
-                memcpy(fmt.fmt.raw_data,&request,sizeof(request));
+                memcpy(fmt.fmt.raw_data, &request, sizeof(request));
+                // use only for IPU6?


Please correct the comment - "?" should be solved

I will append line for submitted kernel patch

remibettan · 2023-08-01T12:19:26Z

src/linux/backend-v4l2.h

+                enum v4l2_buf_type buf_type;
+                unsigned char num_planes;
+                struct v4l2_capability cap;
+                struct v4l2_cropcap cropcap;


please add some comment to explain the meaning of cropcap

there is no function for LRS but it was polled this device capability in past. This is "crop capability" for frame cropping.

remibettan

Few comments - impressive work!

Nir-Az · 2023-08-01T19:27:04Z

config/99-realsense-d4xx-mipi-dfu.rules

+# DFU rules
+SUBSYSTEM=="d4xx-class",KERNEL=="d4xx-dfu*", GROUP="video", MODE="0660"
+# video links for SDK, binding for ipu6
+SUBSYSTEM=="video4linux", ATTR{name}=="DS5 mux *", RUN+="/bin/bash -c 'rs_ipu6_d457_bind.sh > /dev/kmsg; rs-enum.sh -q > /dev/kmsg'"


Is it possible to identify we are running on ADL-P and only run this script if needed?
Not sure jetson users want to see IPU6 script call..

after looking on debian package for udev rules, we have no way to get that information on debian installation stage.
I decided to merge all paltforms to one script. If it is so crucial - we can add post installation step and remove unnecessary files and modify rules but it will be a mess..

scripts/rs-enum.sh

Nir-Az · 2023-08-01T19:30:53Z

scripts/setup_udev_rules.sh

+    if [[ -n "${is_ipu6}" ]] || [[ -n "${is_tegra}" ]]; then
+        sudo cp config/99-realsense-d4xx-mipi-dfu.rules /etc/udev/rules.d/
+        sudo cp scripts/rs-enum.sh /usr/local/bin/rs-enum.sh
+        sudo cp scripts/rs_ipu6_d457_bind.sh /usr/local/bin/rs_ipu6_d457_bind.sh


Why not copy only relevant script based on is_tegra/is_ipu6?

trying to keep similarity for debian package (udev-rules)

Nir-Az · 2023-08-01T19:35:39Z

src/linux/backend-v4l2.cpp

            // assign unique id for mipi by appending camera id to bus_info (bus_info is same for each mipi port)
            // Note - jetson can use only bus_info, as card is different for each sensor and metadata node.
            info.unique_id = bus_info + "-" + std::to_string(cam_id); // use bus_info as per camera unique id for mipi
+            info.dfu_device_path = "/dev/d4xx-dfu504"; // Use legacy DFU device node used in firmware_update_manager


Does this hard coded path say we dont support multi camera DFU?

it comes form fw-update-helper.cpp

Nir-Az · 2023-08-01T19:39:14Z

src/linux/backend-v4l2.cpp

+                if ((_dev.cap.capabilities & V4L2_CAP_VIDEO_CAPTURE_MPLANE) && !is_platform_jetson())
+                {
+                    /* Sakari patch for videodev2.h. This structure will be within kernel > 6.4 */
+                    struct v4l2_meta_format {


Can we share the patch link instead of writing "Sakari patch"?

I will append line for submitted kernel patch

Nir-Az

Great work overall..
A few comment.

Since this goes public to all users, lets clean it up according to the comments and run CI builds to see no reggression on USB / Mipi sku's
Thanks
P.S any change need ls to be in debian repo? regarding udev rules / rs_enum?

common/fw-update-helper.cpp

Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

Dmitry Perchanov and others added 30 commits January 5, 2023 17:04

[ADL-P] LRS V4L2 Backend MPLANE streaming support [LRS-572]

986096b

backend-v4l2: MPLANE supported for 1 plane IPU6 currently supports only one [1] plane backend-v4l2: Metadata for IPU6 with MPLANE [1] support. IPU6 currently supports only MPLANE streaming metadata as video stream resolution

PR #11295 from dmipx: [ADL-P] LRS V4L2 Backend MPLANE streaming suppo…

21acbcb

…rt [LRS-572]

color processing block acc to platform

68d5f5e

using gev_dev_capabilities method

4084c7c

cr

abd95d8

PR #11294 from remibettan: color processing block acc to platform

a416a09

Following code review.

3e6b496

rs-enum: add comments, change rgb to color. backend-v4l2: change rgb to color, comments, try/catch for throwing module. Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

PR #11319 from dmipx: Enumerate video nodes by name

996d7b6

merge development

f00007b

PR #11435 from Nir-Az: Align ADL-P to Development

ed4e624

Merge remote-tracking branch 'origin/development' into adl-p

38dfbbf

PR #11485 from Nir-Az: align adl-p to dev

5e26e12

backend: dfu - add support for multiple cameras

06644ee

rs_sensor: RS2_CAMERA_INFO_DFU_DEVICE_PATH Tracked-by: [DSO-18802] [ADL-P] D4xx DFU support for LRS Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

scripts: rs-enum: cleanup

058726d

PR #11472 from dmipx: backend: dfu - add support for multiple cameras

0c69294

[ADL-P] Automate rs-enum script to udev rules

64dd453

IPU6 d4xx udev rules for binding and enumeration [DSO-18804] Tracking by Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

PR #11550 from dmipx: Automate rs-enum script to udev rules

022bad7

rs-enum: support for metadata nodes format updates

0e613b5

Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

PR #11592 from dmipx: rs-enum: support for metadata nodes format updates

a58c7df

Merge remote-tracking branch 'origin/development' into adl-p

875270f

PR #11605 from Nir-Az: Align ADL-P to development (23/3)

71da796

adl-p temporary disable metadata node

95a7572

Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

PR #11618 from dmipx: adl-p temporary disable metadata node

5f1f209

Merge remote-tracking branch 'origin/development' into adl-p

32b7038

PR #11832 from Nir-Az: align adl-p to development (23/5/23)

f0939d4

backend-v4l: ipu6 integration

fd1f272

Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

udev rules: update dfu filter for all d4xx dfu node names

7498df3

Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

backend-v4l2: use custom structure for metadata.

154b658

Metadata structure will be changed in future kernel releases, expected from version > 6.4. This patch helps avoid system header file change with Sakari metadata patch. Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

scripts: enable metadata bindings.

8786dc5

Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

PR #12023 from dmipx: scripts: rs-enum: update enumeration for jetson…

4c61dea

… devices.

dmipx requested review from Nir-Az and remibettan August 1, 2023 10:43

remibettan reviewed Aug 1, 2023

View reviewed changes

src/ds/d400/d400-color.cpp Show resolved Hide resolved

remibettan reviewed Aug 1, 2023

View reviewed changes

Nir-Az reviewed Aug 1, 2023

View reviewed changes

scripts/rs-enum.sh Show resolved Hide resolved

Nir-Az reviewed Aug 1, 2023

View reviewed changes

dmipx commented Aug 2, 2023

View reviewed changes

common/fw-update-helper.cpp Show resolved Hide resolved

Dmitry Perchanov and others added 5 commits August 4, 2023 09:42

scripts: ipu6 aggregated camera support.

4612539

Signed-off-by: Dmitry Perchanov <dmitry.perchanov@intel.com>

PR #12070 from dmipx: scripts: ipu6 aggregated camera support.

439c5a0

-sscripts: ipu6 bind commentary

e9e57bf

PR #12102 from dmipx: scripts: ipu6 bind commentary

0abacd1

PR #12108 from IntelRealSense: adl-p align to development

812cf43

dmipx mentioned this pull request Aug 28, 2023

Connecting Intel realsense to Xilinx KV260 #12136

Closed

Nir-Az closed this Aug 29, 2023

Nir-Az reopened this Aug 29, 2023

Nir-Az approved these changes Aug 29, 2023

View reviewed changes

Nir-Az merged commit 6514916 into development Aug 29, 2023

Nir-Az deleted the adl-p branch January 9, 2024 12:20

Conversation

dmipx commented Aug 1, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

remibettan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nir-Az left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants