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

Cfe improvements #5632

Merged
merged 14 commits into from
Dec 7, 2023
Merged

Cfe improvements #5632

merged 14 commits into from
Dec 7, 2023

Conversation

tomba
Copy link
Contributor

@tomba tomba commented Oct 4, 2023

More CFE improvements and fixes. These may need more testing, as they start to change things that affect the userspace APIs and behavior.

These require raspberrypi/libcamera#82 to work (or more specifically, the last 3 patches require).

tomba added 3 commits November 1, 2023 12:30
Switch a debug print from cfe_dbg() to cfe_dbg_verbose() as it will be
printed often while streaming.

Signed-off-by: Tomi Valkeinen <[email protected]>
Make find_format_by_pix() accessible to other files in the driver.

Signed-off-by: Tomi Valkeinen <[email protected]>
8-bit bayer formats are missing remap definitions. Add them.

Signed-off-by: Tomi Valkeinen <[email protected]>
@tomba
Copy link
Contributor Author

tomba commented Nov 2, 2023

Cc: @naushir

Copy link
Contributor

@naushir naushir left a comment

Choose a reason for hiding this comment

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

Sorry for the late comments, but this slipped from my radar.

tomba added 11 commits December 7, 2023 12:14
16-bit bayer formats are missing compressed remap definitions. Add them.

Signed-off-by: Tomi Valkeinen <[email protected]>
…de()

Add helper functions which, given an mbus code, return the 16-bit
remapped mbus code or the compressed mbus code.

Signed-off-by: Tomi Valkeinen <[email protected]>
The CSI-2 subdev's set_fmt currently allows setting the source and sink
pad formats quite freely. This is not right, as the CSI-2 block can only
do one of the following when processing the stream: 1) pass through as
is, 2) expand to 16-bits, 3) compress.

The csi2_pad_set_fmt() should take this into account, and only allow
changing the source side mbus code, compared to the sink side format.

Signed-off-by: Tomi Valkeinen <[email protected]>
pisp_fe_pad_set_fmt() allows setting the pad formats quite freely. This
is not correct, and the function should only allow formats as supported
by the hardware. Fix this by:

Allow no format changes for FE_CONFIG_PAD and FE_STATS_PAD. They should
always be the hardcoded initial ones.

Allow setting FE_STREAM_PAD freely (but the mbus code must be
supported), and propagate the format to the FE_OUTPUT0_PAD and
FE_OUTPUT1_PAD pads.

Allow changing the mbus code for FE_OUTPUT0_PAD and FE_OUTPUT1_PAD pads
only if the mbus code is the compressed version of the sink side code.

TODO: FE supports scaling and cropping. This should be represented here
too?

Signed-off-by: Tomi Valkeinen <[email protected]>
Use get_frame_desc pad op for asking the CSI-2 VC and DT from the source
device driver, instead of hardcoding to VC 0, and getting the DT from a
formats table. To keep backward compatibility with sources that do not
implement get_frame_desc, implement a fallback mechanism that always
uses VC 0, and gets the DT from the formats table, based on the CSI2's
sink pad's format.

Signed-off-by: Tomi Valkeinen <[email protected]>
The hardware supports streaming from memory (in addition to streaming
from the CSI-2 RX), but the driver does not support this at the moment.

There are multiple places in the driver which uses
is_image_output_node(), even if the "output" part is not relevant. Thus,
in a minor preparation for the possible support for streaming from
memory, and to make it more obvious that the pieces of code are not
about the "output", add is_image_node() which will return true for both
input and output video nodes.

While at it, reformat also the metadata related macros to fit inside 80
columns.

Signed-off-by: Tomi Valkeinen <[email protected]>
The RP1 CSI-2 DMA can capture both video and metadata just fine, but at
the moment the video nodes are only set to support either video or
metadata.

Make the changes to support both video and metadata. This mostly means
tracking both video format and metadata format separately for each video
node, and using vb2_queue_change_type() to change the vb2 queue type
when needed.

Briefly, this means that the user can get/set both video and meta
formats to a single video node. The vb2 queue buffer type will be
changed when the user calls REQBUFS or CREATE_BUFS ioctls. This buffer
type will be then used as the "mode" for the video node when the user
starts the streaming, and based on that either the video or the meta
format will be used.

A bunch of macros are added (node_supports_xxx()), which tell if a node
can support a particular mode, whereas the existing macros
(is_xxx_node()) will tell if the node is currently in a particular mode.
Note that the latter will only work correctly between the start of the
streaming and the end of the streaming, and thus should be only used in
those code paths.

However, as the userspace (libcamera) does not support dual purpose
video nodes, for the time being let's keep the second video node as
V4L2_CAP_META_CAPTURE only to keep the userspace working.

Signed-off-by: Tomi Valkeinen <[email protected]>
The driver registers for line-end interrupts, but never uses them. This
just causes extra interrupt load, with more complexity in the driver.

Drop the LE handling. It can easily be added back if later needed.

Signed-off-by: Tomi Valkeinen <[email protected]>
The current csi2_link_validate() skips some important checks. Let's
rather use the standard v4l2_subdev_link_validate_default() as the
link_validate hook.

Signed-off-by: Tomi Valkeinen <[email protected]>
The current pisp_fe_link_validate() skips some important checks. Let's
rather use the standard v4l2_subdev_link_validate_default() as the
link_validate hook.

Signed-off-by: Tomi Valkeinen <[email protected]>
Improve the link validation for metadata by:
- Allowing capture buffers that are larger than the incoming frame
  (instead of requiring exact match).

- Instead of assuming that a metadata unit ("pixel") is 8 bits, use
  find_format_by_code() to get the format and use the bit depth from
  there. E.g. bit depth for RAW10 metadata will be 10 bits, when we
  move to the upstream metadata formats.

Signed-off-by: Tomi Valkeinen <[email protected]>
@naushir
Copy link
Contributor

naushir commented Dec 7, 2023

All tests seem to pass fine. Nothing else to do but merge now...

@naushir naushir merged commit e0f52cc into raspberrypi:rpi-6.1.y Dec 7, 2023
12 checks passed
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Dec 11, 2023
kernel: configs: rpi: Compile TSC2007 as module
See: raspberrypi/linux#5776

kernel: Cfe improvements
See: raspberrypi/linux#5632

kernel: Add support for Arducam OV64A40 64Mpx camera module
See: raspberrypi/linux#5708

kernel: 2712 MOP/MOPlet fixes
See: raspberrypi/linux#5773

kernel: dts: bcm2712: put usb under /axi not /soc
See: raspberrypi/linux#5772
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request Dec 11, 2023
kernel: configs: rpi: Compile TSC2007 as module
See: raspberrypi/linux#5776

kernel: Cfe improvements
See: raspberrypi/linux#5632

kernel: Add support for Arducam OV64A40 64Mpx camera module
See: raspberrypi/linux#5708

kernel: 2712 MOP/MOPlet fixes
See: raspberrypi/linux#5773

kernel: dts: bcm2712: put usb under /axi not /soc
See: raspberrypi/linux#5772
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.

2 participants