cfe improvements/fixes #5630
Merged
cfe improvements/fixes #5630
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cfe_probe_complete() calls cfe_put() on both success and fail code paths. This works for the success path, but causes the cfe_device struct to be freed, even if it will be used later in the teardown code. Fix this by making the ref handling a bit saner: Let the video nodes have the refs as they do now, but also keep a ref in the "main" driver, released only at cfe_remove() time. This way the driver does not depend on the video nodes keeping the refs. Signed-off-by: Tomi Valkeinen <[email protected]>
The logic for handling width & height in cfe_start_channel() is somewhat
odd and, afaics, broken. The code reads:
bool start_fe = is_fe_enabled(cfe) &&
test_all_nodes(cfe, NODE_ENABLED, NODE_STREAMING);
if (start_fe || is_image_output_node(node)) {
width = node->fmt.fmt.pix.width;
height = node->fmt.fmt.pix.height;
}
cfe_start_channel() is called for all video nodes that will be used. So
this means that if, say, fe_stats is enabled as the last node, start_fe
will be true, and width and height will be taken from fe_stats' node.
The width and height will thus contain garbage, which then gets
programmed to the csi2 registers.
It seems that this often still works fine, though, probably if the width
& height are large enough.
Drop the above code, and instead get the width & height from the csi2
subdev's sink pad for the csi2 channel that is used. For metadata the
width & height will be 0 as before.
Signed-off-by: Tomi Valkeinen <[email protected]>
The driver has two places where it writes a register based on a condition, and when that condition is false, the driver presumes that the register has the reset value. This is not a good idea, so fix those places to always write the register. Signed-off-by: Tomi Valkeinen <[email protected]>
Use ~0, not -1, when working with unsigned values (-1 is not unsigned). Signed-off-by: Tomi Valkeinen <[email protected]>
The debug print in cfe_schedule_next_csi2_job() is printed every frame, and should thus use cfe_dbg_irq() to avoid spamming, rather than cfe_dbg(). Signed-off-by: Tomi Valkeinen <[email protected]>
Rename the xxx_dbg_irq() macros to xxx_dbg_verbose(), as they can be used to verbose debugs outside irq context too. Signed-off-by: Tomi Valkeinen <[email protected]>
Add back debug prints in csi2 and pisp_fe reg_write() functions, but use the 'irq' variants to avoid spamming in normal situation. Signed-off-by: Tomi Valkeinen <[email protected]>
Expose the verbose debug flag as a module parameter. Signed-off-by: Tomi Valkeinen <[email protected]>
Track the errors from the CSI-2 receiver: overflows and discards. These are recorded in a table which can be read by the userspace via debugfs. As tracking the errors may cause much more interrupt load, the tracking needs to be enabled with a module parameter. Note that the recording is not perfect: we only record the last discarded DT for each discard type, instead of recording all of them. This means that e.g. if the device is discarding two unmatched DTs, the debugfs file only shows the last one recorded. Recording all of them would need a more sophisticated recording system to avoid the need of a very large table, or dynamic allocation. Signed-off-by: Tomi Valkeinen <[email protected]>
Drop 'sensor_embedded_data' field, as it is unused. Signed-off-by: Tomi Valkeinen <[email protected]>
Set hardcoded values for enum csi2_mode, as the values will be programmed to HW registers. Signed-off-by: Tomi Valkeinen <[email protected]>
When pisp_fe_pad_set_fmt() is given an mbus code that CFE does not support, it currently defaults to MEDIA_BUS_FMT_SBGGR10_1X10. This is not correct, as FE does not support SBGGR10. Set the default to MEDIA_BUS_FMT_SRGGB16_1X16 instead. Signed-off-by: Tomi Valkeinen <[email protected]>
Set default meta format's field to V4L2_FIELD_NONE, instead of zeroing it which indicates V4L2_FIELD_ANY. Metadata doesn't have fields, so NONE makes sense, and furthermore the default v4l2 link validation will check for matching fields, or that the sink field is NONE. Signed-off-by: Tomi Valkeinen <[email protected]>
When the FE is enabled, ensure that the FE_CONFIG node is enabled. Otherwise fail cfe_start_streaming() entirely. Signed-off-by: Naushir Patuck <[email protected]>
|
@naushir looks good to me |
tomba
reviewed
Oct 4, 2023
| ret = -EINVAL; | ||
| goto err_streaming; | ||
| } | ||
|
|
There was a problem hiding this comment.
@naushir Ah, noticed only now. The above code block is on the wrong side of the pm_runtime_resume_and_get. It will miss a runtime put in error case.
popcornmix
added a commit
to raspberrypi/firmware
that referenced
this pull request
Oct 5, 2023
kernel: vc4/drm: Remove the clear of SCALER_DISPBKGND_FILL See: raspberrypi/linux#5633 kernel: media: i2c: Move Kconfig entry for IMX477 to the camera sensor section See: raspberrypi/linux#5631 kernel: cfe improvements/fixes See: raspberrypi/linux#5630 kernel: Add DT aliases to map to DRM card names See: raspberrypi/linux#5629 kernel: Replace hdmi-codec channel map fix with accepted version See: raspberrypi/linux#5628 kernel: overlays: Fix vc4-kms-dsi-7inch See: raspberrypi/linux#5622
popcornmix
added a commit
to raspberrypi/rpi-firmware
that referenced
this pull request
Oct 5, 2023
kernel: vc4/drm: Remove the clear of SCALER_DISPBKGND_FILL See: raspberrypi/linux#5633 kernel: media: i2c: Move Kconfig entry for IMX477 to the camera sensor section See: raspberrypi/linux#5631 kernel: cfe improvements/fixes See: raspberrypi/linux#5630 kernel: Add DT aliases to map to DRM card names See: raspberrypi/linux#5629 kernel: Replace hdmi-codec channel map fix with accepted version See: raspberrypi/linux#5628 kernel: overlays: Fix vc4-kms-dsi-7inch See: raspberrypi/linux#5622
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.