media: i2c: imx500: input tensor injection#7040
Conversation
pelwell
left a comment
pelwell
left a comment
There was a problem hiding this comment.
I don't have much to say about this, except that you could comply with checkpatch's implicit suggestion to remove those elses.
| if (hndsk_reg.val == IMX500_DD_DAT_INJECTION_HNDSK_TRANS_COMP) { | ||
| break; | ||
| } else if ( | ||
| hndsk_reg.val == |
There was a problem hiding this comment.
Move this up onto the same line as the if (
| /* Check if we've reached the final completion state */ | ||
| if (hndsk_reg.val == IMX500_DD_DAT_INJECTION_HNDSK_TRANS_COMP) { | ||
| break; | ||
| } else if ( |
There was a problem hiding this comment.
As checkpatch and Phil have said, no need for the else on any of these as you've jumped out of the loop or returned in the error handling of all of them.
| ret = cci_write(imx500->regmap, IMX500_REG_STRM_MODE_SEL, 4, NULL); | ||
| if (ret) { | ||
| dev_err(&client->dev, "Failed to enable DNN processing\n"); | ||
| return ret; |
There was a problem hiding this comment.
Remove this return ret and braces, and just return ret; at line 2113.
| } | ||
|
|
||
| const u8 *tensor_data; | ||
| size_t tensor_data_size; |
There was a problem hiding this comment.
Don't we get a warning for variables being defined in the middle of a function?
| break; | ||
| case V4L2_CID_USER_IMX500_INJECTION_CMP_FRM: | ||
| ctrl->val = imx500->current_injection_cmp_frm; | ||
| dev_info(&client->dev, "Injection comparison frame: %u\n", |
There was a problem hiding this comment.
This could be pretty chatty. dev_dbg?
naushir
left a comment
naushir
left a comment
There was a problem hiding this comment.
Apart from the styling things mentioned, I see no problems with the logic of the code and state machine itself.
Input tensor injection is a debug feature that allows a user-controlled input to be passed directly to IMX500's inference engine (bypassing the in-built ISP). Three new custom controls are added to ENABLE_INJECTION before streaming begins, to provide appropriate input tensors via an INPUT_TENSOR_FD, and to provide notification of DNN results in the sensor output via INJECTION_CMP_FRM. Signed-off-by: Richard Oliver <richard.oliver@raspberrypi.com>
roliver-rpi
force-pushed
the
dev/roliver/imx500_it_injection
branch
from
f4da5c4 to
a016921
Compare
|
I believe all PR comments have been addressed. Are we happy to merge @6by9 ? |
|
LGTM! |
6by9
left a comment
6by9
left a comment
There was a problem hiding this comment.
I'm happy for it to be merged as is despite these comments.
| imx500->input_tensor_fd = | ||
| v4l2_ctrl_new_custom(ctrl_hdlr, &input_tensor_fd, NULL); | ||
| imx500->injection_cmp_frm = | ||
| v4l2_ctrl_new_custom(ctrl_hdlr, &injection_cmp_frm, NULL); |
There was a problem hiding this comment.
Added 3 controls, but not increased the hint passed in ret = v4l2_ctrl_handler_init(ctrl_hdlr, 16); at line 3374.
I make it 17 controls total (two added by v4l2_ctrl_new_fwnode_properties), which implies that the number was wrong before.
| static int __must_check imx500_spi_write(struct imx500 *state, const u8 *data, | ||
| size_t size) | ||
| { | ||
| if (size % 4 || size > ONE_MIB) |
There was a problem hiding this comment.
nit: size % 4 would be the same as size & 3. The compiler may well do that anyway.
| return ret; | ||
| } | ||
|
|
||
| if (tensor_data_size % 4 != 0) { |
There was a problem hiding this comment.
nit: if (tensor_data_size & 3)
kernel: Enable interrupts on CM5 ethernet phy (again) See: raspberrypi/linux#6963 kernel: media: i2c: imx500: input tensor injection See: raspberrypi/linux#7040
kernel: Enable interrupts on CM5 ethernet phy (again) See: raspberrypi/linux#6963 kernel: media: i2c: imx500: input tensor injection See: raspberrypi/linux#7040
4 participants
Input tensor injection is a debug feature that allows a user-controlled input to be passed directly to IMX500's inference engine (bypassing the in-built ISP).
Three new custom controls are added to ENABLE_INJECTION before streaming begins, to provide appropriate input tensors via an INPUT_TENSOR_FD, and to provide notification of DNN results in the sensor output via INJECTION_CMP_FRM.