[libcamera-devel,v2,5/6] include: linux: intel-ipu3: Force alignement to 32 bytes

Message ID 20190527090559.26549-6-jacopo@jmondi.org
State Superseded
Delegated to: Jacopo Mondi
Headers show
Series
  • META_OUTPUT support + IPU3 parameters tuning
Related show

Commit Message

Jacopo Mondi May 27, 2019, 9:05 a.m. UTC
Fix compilation error on GCC 8.3.0

include/linux/intel-ipu3.h:2475:35: error: ‘ipu3_uapi_acc_param::awb_fr’
offset 36756 in ‘ipu3_uapi_acc_param’ isn’t aligned to 32
[-Werror=packed-not-aligned]

by forcing alignment to 32 bytes for struct ipu3_uapi_awb_fr_config_s.
The structure is itself defined as 32 bytes aligned, so adding another
alignment directive to the field definition should not change the
structure memory layout.

The error is reported by gcc8.3.0 only and not by other gcc compiler
versions (5.4.0) or clang.

As the header is exported from Linux v5.1 this is a workaround and
should probably be fixed in the kernel headers themselves if the error
is consistently reported in future compiler versions.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/linux/intel-ipu3.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Laurent Pinchart May 28, 2019, 2:41 p.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Mon, May 27, 2019 at 11:05:58AM +0200, Jacopo Mondi wrote:
> Fix compilation error on GCC 8.3.0
> 
> include/linux/intel-ipu3.h:2475:35: error: ‘ipu3_uapi_acc_param::awb_fr’
> offset 36756 in ‘ipu3_uapi_acc_param’ isn’t aligned to 32
> [-Werror=packed-not-aligned]
> 
> by forcing alignment to 32 bytes for struct ipu3_uapi_awb_fr_config_s.
> The structure is itself defined as 32 bytes aligned, so adding another
> alignment directive to the field definition should not change the
> structure memory layout.

It won't change the layout of the fields inside the structure, but it
will change the position of the structure inside the outer structure. As
Sakari proposed the same fix for upstream I assume this is correct, and
the firmware expect this, so

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

If we later find out that the ImgU doesn't work correctly, we should
remember this patch and investigate.

> The error is reported by gcc8.3.0 only and not by other gcc compiler
> versions (5.4.0) or clang.
> 
> As the header is exported from Linux v5.1 this is a workaround and
> should probably be fixed in the kernel headers themselves if the error
> is consistently reported in future compiler versions.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/linux/intel-ipu3.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/intel-ipu3.h b/include/linux/intel-ipu3.h
> index f758c9ba230b..fe6e8ed15eb1 100644
> --- a/include/linux/intel-ipu3.h
> +++ b/include/linux/intel-ipu3.h
> @@ -2472,7 +2472,7 @@ struct ipu3_uapi_acc_param {
>  	struct ipu3_uapi_yuvp1_yds_config yds2 __attribute__((aligned(32)));
>  	struct ipu3_uapi_yuvp2_tcc_static_config tcc __attribute__((aligned(32)));
>  	struct ipu3_uapi_anr_config anr;
> -	struct ipu3_uapi_awb_fr_config_s awb_fr;
> +	struct ipu3_uapi_awb_fr_config_s awb_fr __attribute__((aligned(32)));
>  	struct ipu3_uapi_ae_config ae;
>  	struct ipu3_uapi_af_config_s af;
>  	struct ipu3_uapi_awb_config awb;

Patch

diff --git a/include/linux/intel-ipu3.h b/include/linux/intel-ipu3.h
index f758c9ba230b..fe6e8ed15eb1 100644
--- a/include/linux/intel-ipu3.h
+++ b/include/linux/intel-ipu3.h
@@ -2472,7 +2472,7 @@  struct ipu3_uapi_acc_param {
 	struct ipu3_uapi_yuvp1_yds_config yds2 __attribute__((aligned(32)));
 	struct ipu3_uapi_yuvp2_tcc_static_config tcc __attribute__((aligned(32)));
 	struct ipu3_uapi_anr_config anr;
-	struct ipu3_uapi_awb_fr_config_s awb_fr;
+	struct ipu3_uapi_awb_fr_config_s awb_fr __attribute__((aligned(32)));
 	struct ipu3_uapi_ae_config ae;
 	struct ipu3_uapi_af_config_s af;
 	struct ipu3_uapi_awb_config awb;