Hi Sakari
On 23/05/2024 18:53, Sakari Ailus wrote:
Hi Dan,
On Thu, May 23, 2024 at 02:44:06PM +0100, Dan Scally wrote:
Hi Sakari - thanks for the review. Snipping some bits for which I have no comment...
On 23/05/2024 09:03, Sakari Ailus wrote:
<snip>
+
+static unsigned int mali_c55_calculate_bank_num(struct mali_c55 *mali_c55,
+ unsigned int crop,
+ unsigned int scale)
+{
+ unsigned int tmp;
+ unsigned int i;
+
+ tmp = (scale * 1000) / crop;
This looks like something that can overflow. Can it?
Shouldn't be able to; maximum scale width is 8192.
Ok.
1000U in that case?
Done
+ for (i = 0; i < MALI_C55_RESIZER_COEFS_NUM_BANKS; i++) {
+ for (j = 0; j < MALI_C55_RESIZER_COEFS_NUM_ENTRIES; j++) {
+ mali_c55_write(mali_c55, haddr,
+ mali_c55_scaler_h_filter_coefficients[i][j]);
+ mali_c55_write(mali_c55, vaddr,
+ mali_c55_scaler_v_filter_coefficients[i][j]);
+
+ haddr += 4;
+ vaddr += 4;
sizeof(u32) ?
Up to you.
I think I'll keep it if it's all the same to you
Well, not the same but I'll let you decide. :-)
OK you're right, the sizeof() is better
...
+static int mali_c55_tpg_init_state(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state)
+{
+ struct v4l2_mbus_framefmt *fmt;
+
+ fmt = v4l2_subdev_state_get_format(sd_state, MALI_C55_TPG_SRC_PAD);
Can be assigned in the declaration.
How would you make it fit that way?
struct v4l2_mbus_framefmt *fmt =
v4l2_subdev_state_get_format(sd_state, MALI_C55_TPG_SRC_PAD);
Done - thank you!