-
Notifications
You must be signed in to change notification settings - Fork 8.3k
drivers: video: hm01b0 - reset/pwdn more formats #98154
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
base: main
Are you sure you want to change the base?
Conversation
e39c928 to
503e7b1
Compare
josuah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these improvements!
Here are a few suggestions to try to reduce the code size and some help for implementing frame rate control.
Let me know if you have questions!
|
@josuah @mjs513 - What my gut tells me, is there are some wrong settings for 324x324 versus 244x324 So it showed the 324x324, however if I look at the Logic Analyzer output It shows 244 rows output: But if I configure by 324x244: But logic analyzer output shows: |
909b1aa to
53af77b
Compare
|
Quick note: with the help of @mjs513 found the format issue and fixed. I am going to mark as ready for review again. Also unless I hear otherwise, I will |
|
@KurtE, thanks for the check. So, should I understand that with DCMI fix applied behavior is now correct regarding the error you raised previously ? |
|
@avolmat-st - @KurtE
|
What I have seen with these cameras on different processors and the like, is they feel very temperamental. Back earlier when @mjs513 and I were trying them out on Teensy Micromod, I felt like the HM0360 was more stable. But the 45fps at 324x324 on the Vision shield with 8 bit data bus I have now received about 4000 * 32 frames and |
70f4bb1 to
4c7fb95
Compare
edf2143 to
b3971fb
Compare
|
I decided to follow through on my thought that the get_frmival should compute the actual current value from the |
avolmat-st
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Looks good to me. Only had few last very minor points.
Also could you rebase on latest head since currently the branch is not mergable ?
| /* give time for the camera to startup before checking it */ | ||
| k_msleep(CHECK_CONNECTION_DELAY_MS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a guess but, I feel like this extra delay is only meaningful if there there is either a reset or powerdown gpio involved, otherwise it is adding an extra delay on a device which has already been in that state for already a while probably, in which case it should be done within the
#if DT_ANY_INST_HAS_PROP_STATUS_OKAY(pwdn_gpios) || DT_ANY_INST_HAS_PROP_STATUS_OKAY(reset_gpios)
Adding some of the missing pieces to the HM01b0 video class. changed the VIDEO define to match all of the other video drivers that I have tried: That is CONFIG_VIDEO_HIMAX_HM01B0 to CONFIG_VIDEO_HM01B0 Added PWDN and RESET pins, to yaml, plus code. Added a few more format sizes: 324x324 to 324x244 164x122 match the spec. Added BAYER format for the color camera (VIDEO_PIX_FMT_SBGGR8) Only for Full, and QVGA, as cameras with BAYER filter do not support the binning mode. Added 4 data bit support, using new format: VIDEO_PIX_FMT_Y4 Support for hflip/vflip controls. Added the frmival set/get/enum functions, to allow us better control of how many frames are generated per second. Used the reset register enumeration similar to other implementions, such as Arduino library, Teensy Library, and OpenME Signed-off-by: Kurt Eckhardt <kurte@rockisland.com>
|
@avolmat-st |
avolmat-st
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @KurtE, sorry (again), one last small wording in the migration-guide tp integrate it better in the generated documentation.
Rename the config CONFIG_VIDEO_HIMAX_HM01B0 to CONFIG_VIDEO_HM01B0 Signed-off-by: Kurt Eckhardt <kurte@rockisland.com> Co-Authored-By: Alain Volmat <58030053+avolmat-st@users.noreply.github.com>
|
avolmat-st
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @KurtE. LGTM.









Now that the @NinoSc PR has been merged:
Adding some of the missing pieces to the HM01b0 video class.
Note: not maybe necessary, but changed the VIDEO define to match all of the other video drivers that I have tried:
That is CONFIG_VIDEO_HIMAX_HM01B0 to
CONFIG_VIDEO_HM01B0
Also keep wondering if the default data-bits should be 8 as to be more likely compatible with more camera drivers. But
left it as 1.
Added PWDN and RESET pins, to yaml, plus code.
Added a few more format sizes: 326x326 to 326x244 match the spec. Verified again from earlier
tests, and Logic Analyzer.
I have also added some of the other components that are in other related drivers, like the
ctrl for HFLIP/VFlip
Added 4 bit mode using the format @avolmat-st suggested in the related issue.
Added format for cameras with Bayer filter.
Questions are:
Should I remove the current resolutions like: 320x320 as the camera does not actually return that?
The images are not as good as I would like or saw from the cameras on other setups. I believe a lot of
this is the other setups (Arducam, Linux, Teensy) do a lot settings of the registers than this currently does.
Things like Auto Exposure and the like.
Main question is: what granularity of change would you prefer to see?
Continue on to refine the images? Or incremental steps? Should I squash all of these?
Do you agree with the cahnge from CONFIG_VIDEO_HIMAX_HM01B0 -> CONFIG_VIDEO_HM01B0 or should I revert it?
Thanks
Kurt