Skip to content

Conversation

NinoSc
Copy link
Contributor

@NinoSc NinoSc commented Aug 23, 2025

Add Himax HM01B0 camera sensor driver.
It depends on I2C and it is required to configure the camera.
reference PR: #93688

@zephyrbot zephyrbot added the area: Video Video subsystem label Aug 23, 2025
@NinoSc NinoSc force-pushed the add_hm01b0_sensor branch 4 times, most recently from f16a06f to 63c60da Compare August 23, 2025 16:16
@josuah
Copy link
Contributor

josuah commented Aug 23, 2025

Thank you for splitting it out of #93688!

I think the last thing it needs is an entry in the test framework so that this driver is checked automatically for build errors.

You can insert a devicetree entry for it in this file:

Then this should be enough for the CI to build the driver on every pull request.

@NinoSc NinoSc force-pushed the add_hm01b0_sensor branch from 63c60da to 42a6083 Compare August 24, 2025 12:21
@NinoSc
Copy link
Contributor Author

NinoSc commented Aug 24, 2025

Thank you for splitting it out of #93688!

I think the last thing it needs is an entry in the test framework so that this driver is checked automatically for build errors.

You can insert a devicetree entry for it in this file:

Then this should be enough for the CI to build the driver on every pull request.

I hope it is ok https://github.com/NinoSc/zephyr/blob/42a6083cf50e3e4580ed8d265084a3a896667cc3/tests/drivers/build_all/video/app.overlay#L136
Let me know please

@KurtE
Copy link
Contributor

KurtE commented Aug 24, 2025

Quick questions, observations: As mentioned in #93688

Wondering if you have it running on any specific hardware and have it displaying or saving any frames?
Other than running a quick test with 1 bit data to see how it was differed in the data generated (using Saleae Logic Analyzer)
I have generated valid frames for 4 bit and 8 bit mode, so far I have only tried 320x240

Example on GIGA 4 bit mode (Arducam Camera for GIGA) There are 244 rows of data generated.
image

Each row generates 652 bytes of data. Two bytes per pixel so 356 pixels per row:
image

For the heck of it, I just now tried 160x120 and:
image
Looks like it generated 122 columns
image
By maybe 328 clocks or 164 pixels... Will try that out...
Added that resolution using the 160x120 init... and verified that it works on the giga 4 bit mode.
I scaled the pixels up when I displayed them.
image

But all of this was on STM32H7... processor And working around issues of 4 bits of data per clock / DMA transfer and how do you
combine the two nibbles that come in separate bytes back into one byte. Some STM32 specific issues/ideas I tried to
mention in a new issue: #94907

However the number for rows/columns generated per the settings I would assume is the same for most processors.

Will be fun to experiment once this is merged...

#define HM01B0_INIT(inst) \
const struct hm01b0_config hm01b0_config_##inst = { \
.i2c = I2C_DT_SPEC_INST_GET(inst), \
.data_bits = DT_INST_PROP(inst, data_bits), \
Copy link
Member

Choose a reason for hiding this comment

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

Instead of the runtime if statement you could shrink the driver flash usage a small amount with:

Suggested change
.data_bits = DT_INST_PROP(inst, data_bits), \
.ctrl_val = HM01B0_CTRL_VAL(DT_INST_PROP(inst, data_bits)), \

And:

#define HM01B0_CTRL_VAL(data_bits) \
    ((data_bits) == 8 ? 0x02 : \
     (data_bits) == 4 ? 0x42 : \
     (data_bits) == 1 ? 0x22 : 0x00)

The Devicetree enum will ensure only 1, 4 or 8 is valid for the data-bits property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion!
I applied it

@NinoSc
Copy link
Contributor Author

NinoSc commented Aug 26, 2025

Hi @KurtE, thanks for the detailed information. Unfortunately I am away and I don't have the analyzer (I have only the board - rp2350 and the camera).
I did a test with resolution 320x240-4 data bits. You can find in attachment the image and the frame (maybe not full synchronized). Le me know please if you see something weird
Screenshot_4bits_320x240

frame_320x240.DMP

Add Himax HM01B0 camera sensor driver.
It depends on I2C and it is required to configure the camera.

Signed-off-by: Antonino Scarpaci <[email protected]>
@NinoSc NinoSc force-pushed the add_hm01b0_sensor branch from 42a6083 to 9df58ef Compare August 26, 2025 09:55
Copy link

@KurtE
Copy link
Contributor

KurtE commented Aug 26, 2025

Thanks @NinoSc - looks like you are getting a good image.

What I am wondering however is something that I am not sure if either of us can answer but more of a generic question
for others like the reviewers, including ones like: @josuah @avolmat-st @ngphibang
Some of this I am trying to ask in the issue: #94907

You are asking the: camera for an image that is 320x240:
HM01B0_VIDEO_FORMAT_CAP(320, 240, VIDEO_PIX_FMT_GREY),
I am pretty sure, that the camera is actually returning: 326x244

Wondering if this can be fixed by changing the initialization of the registers for the different resolutions:
For example: QVGA. Most of the other drivers I have played with, set additional registers. Many of them
look like the ones set in OpenMV, or Arducam_dvp, or ...

#define HIMAX_LINE_LEN_PCK_QVGA     0x178
#define HIMAX_FRAME_LENGTH_QVGA     0x104
...
static const uint16_t QVGA_regs[][2] = {
    {0x0383,                0x01},
    {0x0387,                0x01},
    {0x0390,                0x00},
    {QVGA_WIN_EN,           0x01},// Enable QVGA window readout
    {MAX_INTG_H,            (HIMAX_FRAME_LENGTH_QVGA - 2) >> 8},
    {MAX_INTG_L,            (HIMAX_FRAME_LENGTH_QVGA - 2) & 0xFF},
    {FRAME_LEN_LINES_H,     (HIMAX_FRAME_LENGTH_QVGA >> 8)},
    {FRAME_LEN_LINES_L,     (HIMAX_FRAME_LENGTH_QVGA & 0xFF)},
    {LINE_LEN_PCK_H,        (HIMAX_LINE_LEN_PCK_QVGA >> 8)},
    {LINE_LEN_PCK_L,        (HIMAX_LINE_LEN_PCK_QVGA & 0xFF)},
    {GRP_PARAM_HOLD,        0x01},
    //============= End of regs marker ==================
    {0x0000,            0x00},

};

I was in the process of playing with some of this. But I also was trying to bring in more of their
initial register settings... I should simply try extending yours to see if that works.

EDIT: - Their register set did not change anything that effected the Width/Height

Your combined PR works:

As your Video code: video_pio_dma.c is setup to manipulate the stream returned by the camera.
For example it has:

	if (config->data_bits == 8) {
		data->pclk_per_px = 1;
	} else if (config->data_bits == 4) {
		data->pclk_per_px = 2;
	} else if (config->data_bits == 1) {
		data->pclk_per_px = 8;
	} else {
		LOG_ERR("Invalid data bits!");
		return -ENODEV;
	}

	/* 2 pixels for borders */
	data->border_px = 2;
	data->pio_program_offset = -1;

Which I believe program code then skips the border bits. I don't know the RPI PIO stuff so have not walked through
all of it, but my guess, is per row it might look at the HSYNC/HREF pin, until it is on, then ignore so many pixels
and then read in N pixels and ignore any others until the start of the next horizontal row...

But this is all pretty specific to this camera and this setup. Should something more generic be done?
On my mods, I added:
HM01B0_VIDEO_FORMAT_CAP(326, 244, VIDEO_PIX_FMT_GREY),
Maybe what the PR stuff is sort of doing logically is:
set Format: 326, 244, VIDEO_PIX_FMT_GREY set Selection (TGT_COMPOSE) of 320x240

It is also unclear to me in a generic way: on how we should handle the config->data_bits for GRAY if the camera
is not configured to return 8 bits per PCLK? That is if I use the Arducam HM01b0 GIGA camera, it only is setup to
return up to 4 bits per clock (they did not route the other 4 data pins). So we need to be able to receive 2 transfers
per pixel? Who is responsible for potentially allocating enough memory to hold 2 bytes per pixel when the data
format says it is 8 bits? Who is responsible for then combining the nibbles?

Currently I have it hacked with another FORMAT type: VIDEO_PIX_FMT_GREY_44 which then says that the
data format size is 16 bits, and then my sketch combines them...

Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Video Video subsystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants