Hi,
On 1/11/24 07:55, bingbu.cao@xxxxxxxxx wrote:
> From: Bingbu Cao <bingbu.cao@xxxxxxxxx>
>
> Intel Image Processing Unit 6th Gen includes input and processing systems
> but the hardware presents itself as a single PCI device in system.
>
> IPU6 PCI device driver basically does PCI configurations and load
> the firmware binary, initialises IPU virtual bus, and sets up platform
> specific variants to support multiple IPU6 devices in single device
> driver.
>
> Signed-off-by: Bingbu Cao <bingbu.cao@xxxxxxxxx>
Just one small thing I noticed, not a full review.
<snip>
> +static u32 ipu6se_csi_offsets[] = {
> + IPU6_CSI_PORT_A_ADDR_OFFSET,
> + IPU6_CSI_PORT_B_ADDR_OFFSET,
> + IPU6_CSI_PORT_C_ADDR_OFFSET,
> + IPU6_CSI_PORT_D_ADDR_OFFSET
> +};
> +
> +/*
> + * IPU6 on TGL support maximum 8 csi2 ports
> + * IPU6SE on JSL and IPU6EP on ADL support maximum 4 csi2 ports
> + * IPU6EP on MTL support maximum 6 csi2 ports
> + */
> +static u32 ipu6_tgl_csi_offsets[] = {
> + IPU6_CSI_PORT_A_ADDR_OFFSET,
> + IPU6_CSI_PORT_B_ADDR_OFFSET,
> + IPU6_CSI_PORT_C_ADDR_OFFSET,
> + IPU6_CSI_PORT_D_ADDR_OFFSET,
> + IPU6_CSI_PORT_E_ADDR_OFFSET,
> + IPU6_CSI_PORT_F_ADDR_OFFSET,
> + IPU6_CSI_PORT_G_ADDR_OFFSET,
> + IPU6_CSI_PORT_H_ADDR_OFFSET
> +};
> +
> +static u32 ipu6ep_mtl_csi_offsets[] = {
> + IPU6_CSI_PORT_A_ADDR_OFFSET,
> + IPU6_CSI_PORT_B_ADDR_OFFSET,
> + IPU6_CSI_PORT_C_ADDR_OFFSET,
> + IPU6_CSI_PORT_D_ADDR_OFFSET,
> + IPU6_CSI_PORT_E_ADDR_OFFSET,
> + IPU6_CSI_PORT_F_ADDR_OFFSET
> +};
> +
> +static u32 ipu6_csi_offsets[] = {
> + IPU6_CSI_PORT_A_ADDR_OFFSET,
> + IPU6_CSI_PORT_B_ADDR_OFFSET,
> + IPU6_CSI_PORT_C_ADDR_OFFSET,
> + IPU6_CSI_PORT_D_ADDR_OFFSET
> +};
These 4 arrays all are the same, except for their lengths.
Please just use a single array wuth the full 8 A-H
entries and then just directly code the amount of ports
here:
> +static void ipu6_internal_pdata_init(struct ipu6_device *isp)
> +{
<snip>
> + isys_ipdata.csi2.nports = ARRAY_SIZE(ipu6_csi_offsets);
So put the default 4 here instead of ARRAY_SIZE(),
<snip>
> + if (is_ipu6_tgl(hw_ver)) {
> + isys_ipdata.csi2.nports = ARRAY_SIZE(ipu6_tgl_csi_offsets);
and then put 8 here instead of ARRAY_SIZE(), etc.
Then there no longer is a need to have 4 different arrays just
to have them a different lenght.
And this line:
> + isys_ipdata.csi2.offsets = ipu6_tgl_csi_offsets;
Can then be fully dropped as well as similar lines below.
> + }
> +
> + if (is_ipu6ep_mtl(hw_ver)) {
> + isys_ipdata.csi2.nports = ARRAY_SIZE(ipu6ep_mtl_csi_offsets);
> + isys_ipdata.csi2.offsets = ipu6ep_mtl_csi_offsets;
E.g. this one can also be dropped.
Regards,
Hans
