Re: [PATCH] media: imx283: drop CENTERED_RECTANGLE due to clang failure

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

 



Quoting Hans Verkuil (2024-06-13 16:19:08)
> The CENTERED_RECTANGLE define fails to compile on clang and old gcc
> versions. Just drop it and fill in the crop rectangles explicitly.

So back when I was playing around with this I thought it would have got
dropped during review / upstreaming before now - because I couldn't find
a way to make sure the macro args were guaranteed to be used only once,
by putting some locals in the macro (because of the initialisation).

So I'm not surprised that it needs to be removed, but I am surprised
that it wasn't for the reason I expected ;-)

Anyway - maybe later I'll experiement with more common helpers perhaps -
but not if it hits compile errors..



> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> ---
> diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c
> index 6428eb5394a9..8490618c5071 100644
> --- a/drivers/media/i2c/imx283.c
> +++ b/drivers/media/i2c/imx283.c
> @@ -407,14 +407,6 @@ static const struct imx283_reg_list link_freq_reglist[] = {
>         },
>  };
> 
> -#define CENTERED_RECTANGLE(rect, _width, _height)                      \
> -       {                                                               \
> -               .left = rect.left + ((rect.width - (_width)) / 2),      \
> -               .top = rect.top + ((rect.height - (_height)) / 2),      \
> -               .width = (_width),                                      \
> -               .height = (_height),                                    \
> -       }
> -
>  /* Mode configs */
>  static const struct imx283_mode supported_modes_12bit[] = {
>         {
> @@ -440,7 +432,12 @@ static const struct imx283_mode supported_modes_12bit[] = {
>                 .min_shr = 11,
>                 .horizontal_ob = 96,
>                 .vertical_ob = 16,
> -               .crop = CENTERED_RECTANGLE(imx283_active_area, 5472, 3648),
> +               .crop = {
> +                       .top = 40,
> +                       .left = 108,
> +                       .width = 5472,
> +                       .height = 3648,
> +               },

I do recall using v4l2_rects to define the active area so they could be
used collectively rather than initialising things as
	.width = WIDTH,
	.height = HEIGHT,

So - perhaps this could be (if it compiles):
	.crop = imx283_active_area,

but either way is fine with me if it unbreaks linux-next.


Reviewed-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>

>         },
>         {
>                 /*
> @@ -468,7 +465,12 @@ static const struct imx283_mode supported_modes_12bit[] = {
>                 .horizontal_ob = 48,
>                 .vertical_ob = 4,
> 
> -               .crop = CENTERED_RECTANGLE(imx283_active_area, 5472, 3648),
> +               .crop = {
> +                       .top = 40,
> +                       .left = 108,
> +                       .width = 5472,
> +                       .height = 3648,
> +               },
>         },
>  };
> 
> @@ -489,7 +491,12 @@ static const struct imx283_mode supported_modes_10bit[] = {
>                 .min_shr = 10,
>                 .horizontal_ob = 96,
>                 .vertical_ob = 16,
> -               .crop = CENTERED_RECTANGLE(imx283_active_area, 5472, 3648),
> +               .crop = {
> +                       .top = 40,
> +                       .left = 108,
> +                       .width = 5472,
> +                       .height = 3648,
> +               },
>         },
>  };
> 
>





[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