Re: [PATCH v4 3/5] media: mali-c55: Add Mali-C55 ISP driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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!






[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux