Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow refresh serdes configs during runtime #105

Merged
merged 1 commit into from
May 22, 2022

Conversation

xzhangxa
Copy link
Collaborator

@xzhangxa xzhangxa commented May 17, 2022

  • Add 2 attrs to refresh max9295/max9296 configs anytime by user.
  • When switching formats of IR stream, do not refresh all the serdes
    settings, so other streaming streams won't be interrupted. Only the
    necessary addresses are set now.

Test steps for using the new attributes:

  1. Boot the board and use D457 as usual;
  2. Power off the max9296 board then power on, to lose all settings of max9295/9296;
  3. Run commands below.
echo 1 | sudo tee /sys/bus/i2c/drivers/max9295/30-0040/refresh_setting
echo 1 | sudo tee /sys/bus/i2c/drivers/max9296/30-0048/refresh_setting
  1. Start streams of D457 as usual, still works. IR stream format will still be the last used one, matching the v4l2 node format.

And when switching formats of IR stream, the serdes settings being changed are reduced to only the necessary ones. So now if another stream is streaming, it won't be interrupted (on current master, the streaming could be seen stopped for 1 or 2 seconds then go on again), with the new changes the other stream won't be affected.

@xzhangxa xzhangxa requested review from dmipx, ev-mp and jnzw May 17, 2022 11:47
Copy link
Contributor

@dmipx dmipx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's great! Thank you!

@dmipx
Copy link
Contributor

dmipx commented May 18, 2022

isn't it better to have max9295/6 as modules and not compile it to kernel?
current config is
CONFIG_I2C_IOEXPANDER_SER_MAX9295=y
CONFIG_I2C_IOEXPANDER_DESER_MAX9296=y

- Add 2 attrs to refresh max9295/max9296 configs anytime by user.
- When switching formats of IR stream, do not refresh all the serdes
  settings, so other streaming streams won't be interrupted. Only the
  necessary addresses are set now.

Signed-off-by: Xin Zhang <xin.x.zhang@intel.com>
@xzhangxa
Copy link
Collaborator Author

isn't it better to have max9295/6 as modules and not compile it to kernel? current config is CONFIG_I2C_IOEXPANDER_SER_MAX9295=y CONFIG_I2C_IOEXPANDER_DESER_MAX9296=y

We didn't modify the default kernel config of Jetson, so it's just the original setting.

@xzhangxa
Copy link
Collaborator Author

This PR is ready to review/merge. Besides the refresh attributes, another fix and cleanup is added. Please see the updated first comment for details.

@xzhangxa xzhangxa marked this pull request as ready for review May 18, 2022 11:35
@xzhangxa xzhangxa mentioned this pull request May 19, 2022
Copy link
Contributor

@jnzw jnzw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

- {0x0331, 0x11}, // Write 0x33 for 4 lanes
- {0x0308, 0x6F}, // All pipes pull clock from port B
- {0x0311, 0xF0}, // All pipes pull data from port B
{0x0312, 0x0B}, // Double 8-bit data on pipe X, Y & U
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this part looks complicated, can you elaborate why we discard 4 registers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see originally we only have map_pipe_y8_opt and map_pipe_y12i_opt, they are used for y8_y8i and y12i separately, the first 4 addr/val pairs are same, only the last one is different.

Now it's split to a common map_pipe_opt with the 4 common addr/val, and map_pipe_y8_opt and map_pipe_y12i_opt for the last different pair. When switching only the last pair is refreshed. The patch file instead of the diff in code makes it less obvious :)

@ev-mp ev-mp merged commit 4139b1b into IntelRealSense:master May 22, 2022
@xzhangxa xzhangxa deleted the refresh branch October 13, 2022 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants