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

drivers: media: cfe: Add non-continuous CSI-2 clock mode #6705

Draft
wants to merge 1 commit into
base: rpi-6.12.y
Choose a base branch
from

Conversation

naushir
Copy link
Contributor

@naushir naushir commented Mar 6, 2025

Set the DPHY block to manual clock lane termination enable and HS mode i V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK is set in the bus flags.

@naushir
Copy link
Contributor Author

naushir commented Mar 6, 2025

@6by9 very speculative change that sets up the DPHY for non-continuous clock.

Copy link
Contributor

@6by9 6by9 left a comment

Choose a reason for hiding this comment

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

Thanks - I'll give it a test with OV9281.

@naushir naushir force-pushed the rpi-6.12.y branch 3 times, most recently from 0bee9c6 to 6945aa8 Compare March 6, 2025 13:25
@pelwell
Copy link
Contributor

pelwell commented Mar 6, 2025

I would enable warnings-as-errors by default, but some don't like it. It's enabled in the auto-builds after the choice of defconfig by this step:

scripts/config --file $ROOT/.config --set-val CONFIG_WERROR y

where $ROOT may be . in your case.

@naushir
Copy link
Contributor Author

naushir commented Mar 6, 2025

This change will need adjusting so that we can switch between auto/manual termination since it's possible this is mode specific.

cfe->csi2.dphy.noncontinuous_clock =
(!ret ? mbus_config.bus.mipi_csi2.flags : cfe->csi2.bus_flags) &
V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;

Copy link
Contributor

Choose a reason for hiding this comment

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

That'll do.

I'd have fixed up the active_lanes value too with

if (!ret) {
   cfe->csi2.dphy.noncontinuous_clock = mbus_config.bus.mipi_csi2.flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
   cfe->csi2.dphy.active_lanes = mbus_config.bus.mipi_csi2.num_data_lanes;
	if (cfe->csi2.dphy.active_lanes > cfe->csi2.dphy.max_lanes) {
		cfe_err("Device has requested %u data lanes, which is >%u configured in DT\n",
			cfe->csi2.dphy.active_lanes, cfe->csi2.dphy.max_lanes);
		ret = -EINVAL;
		goto err_disable_cfe;
}} else {
  cfe->csi2.dphy.noncontinuous_clock = cfe->csi2.bus_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
   cfe->csi2.dphy.active_lanes = cfe->csi2.dphy.max_lanes;
}

but that can wait for another day.

I still haven't managed to get the failure case to happen that we were discussing this for though....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still haven't managed to get the failure case to happen that we were discussing this for though....

Even with this commit reverted?

Copy link
Contributor

Choose a reason for hiding this comment

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

I still haven't managed to get the failure case to happen that we were discussing this for though....

Even with this commit reverted?

Reproduced now without this commit.

6.12 doesn't have #6700 to add the option for selecting clock mode. Adding that makes testing far easier.
Continuous clock mode fails. Non-continuous is OK. That's similar to Pi3 which won't stream in continuous clock mode.

With the latest version of this patch (guess who tried testing the version that only looked at the get_mbus_config result version) I get timeouts in continuous mode, and black images non-continuous.

So we look to be in the right sort of area, but not quite right. I'll give it a few more minutes poking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's quite likely the register configuration I've used is also incorrect. The datasheet is very convoluted.

@naushir
Copy link
Contributor Author

naushir commented Mar 7, 2025

After staring a bit more at the databook, I suspect that while the register configuration may be right, the way we (now) write multiple config register through the test interface is likely wrong.

@popcornmix popcornmix force-pushed the rpi-6.12.y branch 2 times, most recently from 28d1e43 to de1afd2 Compare March 13, 2025 14:14
Calling this function without holding the mmap_read_lock causes the
kernel to throw an error message, spamming the dmesg logs when running
the Hailo hardware.

Fix it by adding the approprite lock/unlock functions around find_vdma().

Signed-off-by: Naushir Patuck <[email protected]>
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.

3 participants