[PATCH 4/4] staging: atomisp: Replace ia_css_region with v4l2_rect

[PATCH 4/4] staging: atomisp: Replace ia_css_region with v4l2_rect

[Karthikey Kadati]

Replace custom struct ia_css_region with standard struct v4l2_rect

to align with V4L2 API.

Signed-off-by: Unix Antigravity <unix.antigravity@test.com>
Signed-off-by: Karthikey Kadati <karthikey3608@gmail.com>
---
 drivers/staging/media/atomisp/pci/ia_css_types.h | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/ia_css_types.h b/drivers/staging/media/atomisp/pci/ia_css_types.h
index 5c21a5415..7cfb355c8 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_types.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_types.h
@@ -18,6 +18,7 @@
 #include <type_support.h>
 
 #include "ia_css_frac.h"
+#include <linux/videodev2.h>
 
 #include "isp/kernels/aa/aa_2/ia_css_aa2_types.h"
 #include "isp/kernels/anr/anr_1.0/ia_css_anr_types.h"
@@ -430,12 +431,10 @@ struct ia_css_point {
 /**
  * This specifies the region
  */
-struct ia_css_region {
-	s32 left; /** Starting point coordinates for the region */
-	s32 top;
-	s32 width; /** Region resolution */
-	s32 height;
-};
+/**
+ * This specifies the region
+ */
+/* struct v4l2_rect has been replaced by struct v4l2_rect */
 
 /**
  * Digital zoom:
@@ -444,7 +443,7 @@ struct ia_css_region {
  * Set the digital zoom factor, this is a logarithmic scale. The actual zoom
  * factor will be 64/x.
  * Setting dx or dy to 0 disables digital zoom for that direction.
- * New API change for Digital zoom:(added struct ia_css_region zoom_region)
+ * New API change for Digital zoom:(added struct v4l2_rect zoom_region)
  * zoom_region specifies the origin of the zoom region and width and
  * height of that region.
  * origin : This is the coordinate (x,y) within the effective input resolution
@@ -457,7 +456,7 @@ struct ia_css_region {
 struct ia_css_dz_config {
 	u32 dx; /** Horizontal zoom factor */
 	u32 dy; /** Vertical zoom factor */
-	struct ia_css_region zoom_region; /** region for zoom */
+	struct v4l2_rect zoom_region; /** region for zoom */
 };
 
 /* The still capture mode, this can be RAW (simply copy sensor input to DDR),
-- 
2.43.0

Re: [PATCH 4/4] staging: atomisp: Replace ia_css_region with v4l2_rect

[Dan Carpenter]

On Mon, Jan 12, 2026 at 03:55:18PM +0530, Karthikey Kadati wrote:
> Replace custom struct ia_css_region with standard struct v4l2_rect
> 
> to align with V4L2 API.
> 
> Signed-off-by: Unix Antigravity <unix.antigravity@test.com>
> Signed-off-by: Karthikey Kadati <karthikey3608@gmail.com>
> ---
>  drivers/staging/media/atomisp/pci/ia_css_types.h | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/ia_css_types.h b/drivers/staging/media/atomisp/pci/ia_css_types.h
> index 5c21a5415..7cfb355c8 100644
> --- a/drivers/staging/media/atomisp/pci/ia_css_types.h
> +++ b/drivers/staging/media/atomisp/pci/ia_css_types.h
> @@ -18,6 +18,7 @@
>  #include <type_support.h>
>  
>  #include "ia_css_frac.h"
> +#include <linux/videodev2.h>
>  
>  #include "isp/kernels/aa/aa_2/ia_css_aa2_types.h"
>  #include "isp/kernels/anr/anr_1.0/ia_css_anr_types.h"
> @@ -430,12 +431,10 @@ struct ia_css_point {
>  /**
>   * This specifies the region
>   */
> -struct ia_css_region {
> -	s32 left; /** Starting point coordinates for the region */
> -	s32 top;
> -	s32 width; /** Region resolution */
> -	s32 height;
> -};
> +/**
> + * This specifies the region
> + */
> +/* struct v4l2_rect has been replaced by struct v4l2_rect */

This historical artifact should be in the commit message instead of in
the code.  In other words, delete both comments.

regards,
dan carpenter

Re: [PATCH 4/4] staging: atomisp: Replace ia_css_region with v4l2_rect

[Sakari Ailus]

Hi Karthikey,

Thanks for the patches.

On Mon, Jan 12, 2026 at 03:55:18PM +0530, Karthikey Kadati wrote:
> Replace custom struct ia_css_region with standard struct v4l2_rect
> 
> to align with V4L2 API.

You seem to systematically have an extra newline between the lines of text
in the commit messages. Please fix that.

Please try to send patches in consistent sets to the relevant recipients
and avoid sending new versions before people have time to comment on the
previous ones. Also please think before sending whether there's still
something to fix and detail what changes there were from the previous
version (below the '---' line).

> 
> Signed-off-by: Unix Antigravity <unix.antigravity@test.com>

I guess this is possible but it looks like bogus Sob: line.

Who wrote the patch, should there be Co-developed-by: tag?

> Signed-off-by: Karthikey Kadati <karthikey3608@gmail.com>
> ---
>  drivers/staging/media/atomisp/pci/ia_css_types.h | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/ia_css_types.h b/drivers/staging/media/atomisp/pci/ia_css_types.h
> index 5c21a5415..7cfb355c8 100644
> --- a/drivers/staging/media/atomisp/pci/ia_css_types.h
> +++ b/drivers/staging/media/atomisp/pci/ia_css_types.h
> @@ -18,6 +18,7 @@
>  #include <type_support.h>
>  
>  #include "ia_css_frac.h"
> +#include <linux/videodev2.h>
>  
>  #include "isp/kernels/aa/aa_2/ia_css_aa2_types.h"
>  #include "isp/kernels/anr/anr_1.0/ia_css_anr_types.h"
> @@ -430,12 +431,10 @@ struct ia_css_point {
>  /**
>   * This specifies the region
>   */
> -struct ia_css_region {
> -	s32 left; /** Starting point coordinates for the region */
> -	s32 top;
> -	s32 width; /** Region resolution */
> -	s32 height;
> -};
> +/**
> + * This specifies the region
> + */
> +/* struct v4l2_rect has been replaced by struct v4l2_rect */
>  
>  /**
>   * Digital zoom:
> @@ -444,7 +443,7 @@ struct ia_css_region {
>   * Set the digital zoom factor, this is a logarithmic scale. The actual zoom
>   * factor will be 64/x.
>   * Setting dx or dy to 0 disables digital zoom for that direction.
> - * New API change for Digital zoom:(added struct ia_css_region zoom_region)
> + * New API change for Digital zoom:(added struct v4l2_rect zoom_region)
>   * zoom_region specifies the origin of the zoom region and width and
>   * height of that region.
>   * origin : This is the coordinate (x,y) within the effective input resolution
> @@ -457,7 +456,7 @@ struct ia_css_region {
>  struct ia_css_dz_config {
>  	u32 dx; /** Horizontal zoom factor */
>  	u32 dy; /** Vertical zoom factor */
> -	struct ia_css_region zoom_region; /** region for zoom */
> +	struct v4l2_rect zoom_region; /** region for zoom */
>  };
>  
>  /* The still capture mode, this can be RAW (simply copy sensor input to DDR),

-- 
Regards,

Sakari Ailus

Re: [PATCH 4/4] staging: atomisp: Replace ia_css_region with v4l2_rect

[Andy Shevchenko]

On Mon, Jan 12, 2026 at 03:55:18PM +0530, Karthikey Kadati wrote:
> Replace custom struct ia_css_region with standard struct v4l2_rect
> 
> to align with V4L2 API.

Your series is split (no common thread in email), please be sure you supplied
`git format-patch --thread ...` when created the series for sending.

...

>  #include <type_support.h>
>  
>  #include "ia_css_frac.h"
> +#include <linux/videodev2.h>

Linux headers should go first before other (there are some more generic headers
like dt-bindings, but it's not the case here).

>  #include "isp/kernels/aa/aa_2/ia_css_aa2_types.h"
>  #include "isp/kernels/anr/anr_1.0/ia_css_anr_types.h"

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/4] staging: atomisp: Replace ia_css_region with v4l2_rect

[Andy Shevchenko]

On Mon, Jan 12, 2026 at 03:55:18PM +0530, Karthikey Kadati wrote:
> Replace custom struct ia_css_region with standard struct v4l2_rect
> 
> to align with V4L2 API.

...

> -struct ia_css_region {
> -	s32 left; /** Starting point coordinates for the region */
> -	s32 top;

> -	s32 width; /** Region resolution */
> -	s32 height;

This is not equivalent change as the last two members are unsigned in
the generic data structure. Have you been able to test this on real HW?
Independently, you can audit the code for the multiplications and
divisions when the result put in these fields or the values from this
fields are in the calculations for others.

> -};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/4] staging: atomisp: Replace ia_css_region with v4l2_rect

[Karthikey Kadati]

Hi Andy,

I checked the code as you suggested regarding the signedness change.

The 'left' and 'top' members remain signed (s32) in v4l2_rect, so
there is no change there.

For 'width' and 'height' (which became u32), I audited the usage:
1. In atomisp_cmd.c, the multiplications fit safely within integer limits.
2. In sh_css_params.c, the logic assigns these to 'unsigned int'. The
existing bounds checks (e.g., x0 > x1) correctly handle any edge
cases.

I verified this by compiling the driver, and the v2 patch I just sent
reflects this.

Thanks,
Karthikey


On Mon, 12 Jan 2026 at 16:37, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Mon, Jan 12, 2026 at 03:55:18PM +0530, Karthikey Kadati wrote:
> > Replace custom struct ia_css_region with standard struct v4l2_rect
> >
> > to align with V4L2 API.
>
> ...
>
> > -struct ia_css_region {
> > -     s32 left; /** Starting point coordinates for the region */
> > -     s32 top;
>
> > -     s32 width; /** Region resolution */
> > -     s32 height;
>
> This is not equivalent change as the last two members are unsigned in
> the generic data structure. Have you been able to test this on real HW?
> Independently, you can audit the code for the multiplications and
> divisions when the result put in these fields or the values from this
> fields are in the calculations for others.
>
> > -};
>
> --
> With Best Regards,
> Andy Shevchenko
>
>