Hello again, On Mon, Jun 24, 2024 at 03:52:41PM GMT, Hans Verkuil wrote: > Hi Jacopo, > > On 31/05/2024 10:07, Jacopo Mondi wrote: > > Add the Raspberry Pi PiSP Back End uAPI header. > > > > The header defines the data type used to configure the PiSP Back End > > ISP. > > > > The detailed description of the types and of the ISP configuration > > procedure is available at > > https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> > > --- > > MAINTAINERS | 7 + > > .../linux/media/raspberrypi/pisp_be_config.h | 927 ++++++++++++++++++ > > .../linux/media/raspberrypi/pisp_common.h | 199 ++++ > > 3 files changed, 1133 insertions(+) > > create mode 100644 include/uapi/linux/media/raspberrypi/pisp_be_config.h > > create mode 100644 include/uapi/linux/media/raspberrypi/pisp_common.h > > > > <snip> > > > diff --git a/include/uapi/linux/media/raspberrypi/pisp_be_config.h b/include/uapi/linux/media/raspberrypi/pisp_be_config.h > > new file mode 100644 > > index 000000000000..3eb4be03c5f4 > > --- /dev/null > > +++ b/include/uapi/linux/media/raspberrypi/pisp_be_config.h > > @@ -0,0 +1,927 @@ > > <snip> > > > +/** > > + * struct pisp_be_tiles_config - Raspberry Pi PiSP Back End configuration > > + * @tiles: Tile descriptors > > + * @num_tiles: Number of tiles > > + * @config: PiSP Back End configuration > > + */ > > +struct pisp_be_tiles_config { > > + struct pisp_tile tiles[PISP_BACK_END_NUM_TILES]; > > + int num_tiles; > > Everything else is nicely __u8/16/32, and then there is suddenly an 'int' > where I would expect to see a __u32. > > I think a v10 is needed anyway (see next review), so it would be nice to > pick up this change for v10. Sure I can change it. While at it I passed ' struct pisp_be_tiles_config' through pahole. struct pisp_be_tiles_config { struct pisp_tile tiles[64]; /* 0 10240 */ /* --- cacheline 160 boundary (10240 bytes) --- */ __u32 num_tiles; /* 10240 4 */ struct pisp_be_config config; /* 10244 6276 */ /* size: 16520, cachelines: 259, members: 3 */ /* last cacheline: 8 bytes */ }; if 'config' gets accessed by pointer on aarch64 it will result in an unaligned access ? Do we need to insert a 32 bits padding between 'num_tiles' and 'config' ? > > Regards, > > Hans > > > + struct pisp_be_config config; > > +} __attribute__((packed)); > > + > > +#endif /* _UAPI_PISP_BE_CONFIG_H_ */ >
