RE: [PATCH v7 05/16] intel-ipu3: abi: Add structs
Hi Sakari,
> Subject: Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
>
> Hi Raj,
>
> On Mon, Nov 05, 2018 at 07:05:53PM +, Mani, Rajmohan wrote:
> > Hi Sakari,
> >
> > > Subject: Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
> > >
> > > Hi Yong,
> > >
> > > On Mon, Oct 29, 2018 at 03:22:59PM -0700, Yong Zhi wrote:
> > > > This add all the structs of IPU3 firmware ABI.
> > > >
> > > > Signed-off-by: Yong Zhi
> > > > Signed-off-by: Rajmohan Mani
> > >
> > > ...
> > >
> > > > +struct imgu_abi_shd_intra_frame_operations_data {
> > > > + struct imgu_abi_acc_operation
> > > > + operation_list[IMGU_ABI_SHD_MAX_OPERATIONS]
> > > __attribute__((aligned(32)));
> > > > + struct imgu_abi_acc_process_lines_cmd_data
> > > > + process_lines_data[IMGU_ABI_SHD_MAX_PROCESS_LINES]
> > > __attribute__((aligned(32)));
> > > > + struct imgu_abi_shd_transfer_luts_set_data
> > > > + transfer_data[IMGU_ABI_SHD_MAX_TRANSFERS]
> > > > +__attribute__((aligned(32)));
> > >
> > > Could you replace this wth __aligned(32), please? The same for the
> > > rest of the header.
> > >
> >
> > Using __aligned(32) in the uAPI header resulted in compilation errors
> > in user space / camera HAL code.
> >
> > e.g
> > ../../../../../../../../usr/include/linux/intel-ipu3.h:464:57: error:
> > expected ';'
> > at end of declaration list
> > __u8 bayer_table[IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE]
> > __aligned(32);
> >
> > So we ended up using __attribute__((aligned(32))) format in uAPI
> > header and to be consistent, we followed the same format in ABI header as
> well.
> >
> > Let us know if it's okay to deviate between uAPI and ABI header for
> > this alignment qualifier.
>
> There's a reason for using __attribute__((aligned(32))) in the uAPI header,
> but
> not in the in-kernel headers where __aligned(32) is preferred.
>
> I have a patch for addressing this for the uAPI headers as well so
> __aligned(32) could be used there, too; I'll submit it soon. Let's see...
> there are kerneldoc issues still in this area.
>
Great. Thanks for the patch to address this need.
> --
> Regards,
>
> Sakari Ailus
> sakari.ai...@linux.intel.com
Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
Hi Raj,
On Mon, Nov 05, 2018 at 07:05:53PM +, Mani, Rajmohan wrote:
> Hi Sakari,
>
> > -Original Message-
> > From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com]
> > Sent: Monday, November 05, 2018 12:28 AM
> > To: Zhi, Yong
> > Cc: linux-media@vger.kernel.org; tf...@chromium.org; mche...@kernel.org;
> > hans.verk...@cisco.com; laurent.pinch...@ideasonboard.com; Mani,
> > Rajmohan ; Zheng, Jian Xu
> > ; Hu, Jerry W ; Toivonen,
> > Tuukka ; Qiu, Tian Shu
> > ; Cao, Bingbu
> > Subject: Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
> >
> > Hi Yong,
> >
> > On Mon, Oct 29, 2018 at 03:22:59PM -0700, Yong Zhi wrote:
> > > This add all the structs of IPU3 firmware ABI.
> > >
> > > Signed-off-by: Yong Zhi
> > > Signed-off-by: Rajmohan Mani
> >
> > ...
> >
> > > +struct imgu_abi_shd_intra_frame_operations_data {
> > > + struct imgu_abi_acc_operation
> > > + operation_list[IMGU_ABI_SHD_MAX_OPERATIONS]
> > __attribute__((aligned(32)));
> > > + struct imgu_abi_acc_process_lines_cmd_data
> > > + process_lines_data[IMGU_ABI_SHD_MAX_PROCESS_LINES]
> > __attribute__((aligned(32)));
> > > + struct imgu_abi_shd_transfer_luts_set_data
> > > + transfer_data[IMGU_ABI_SHD_MAX_TRANSFERS]
> > > +__attribute__((aligned(32)));
> >
> > Could you replace this wth __aligned(32), please? The same for the rest of
> > the
> > header.
> >
>
> Using __aligned(32) in the uAPI header resulted in compilation errors in
> user space / camera HAL code.
>
> e.g
> ../../../../../../../../usr/include/linux/intel-ipu3.h:464:57: error:
> expected ';'
> at end of declaration list
> __u8 bayer_table[IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE] __aligned(32);
>
> So we ended up using __attribute__((aligned(32))) format in uAPI header and
> to be consistent, we followed the same format in ABI header as well.
>
> Let us know if it's okay to deviate between uAPI and ABI header for this
> alignment qualifier.
There's a reason for using __attribute__((aligned(32))) in the uAPI header,
but not in the in-kernel headers where __aligned(32) is preferred.
I have a patch for addressing this for the uAPI headers as well so
__aligned(32) could be used there, too; I'll submit it soon. Let's see...
there are kerneldoc issues still in this area.
--
Regards,
Sakari Ailus
sakari.ai...@linux.intel.com
RE: [PATCH v7 05/16] intel-ipu3: abi: Add structs
Hi Sakari,
> -Original Message-
> From: Sakari Ailus [mailto:sakari.ai...@linux.intel.com]
> Sent: Monday, November 05, 2018 12:28 AM
> To: Zhi, Yong
> Cc: linux-media@vger.kernel.org; tf...@chromium.org; mche...@kernel.org;
> hans.verk...@cisco.com; laurent.pinch...@ideasonboard.com; Mani,
> Rajmohan ; Zheng, Jian Xu
> ; Hu, Jerry W ; Toivonen,
> Tuukka ; Qiu, Tian Shu
> ; Cao, Bingbu
> Subject: Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
>
> Hi Yong,
>
> On Mon, Oct 29, 2018 at 03:22:59PM -0700, Yong Zhi wrote:
> > This add all the structs of IPU3 firmware ABI.
> >
> > Signed-off-by: Yong Zhi
> > Signed-off-by: Rajmohan Mani
>
> ...
>
> > +struct imgu_abi_shd_intra_frame_operations_data {
> > + struct imgu_abi_acc_operation
> > + operation_list[IMGU_ABI_SHD_MAX_OPERATIONS]
> __attribute__((aligned(32)));
> > + struct imgu_abi_acc_process_lines_cmd_data
> > + process_lines_data[IMGU_ABI_SHD_MAX_PROCESS_LINES]
> __attribute__((aligned(32)));
> > + struct imgu_abi_shd_transfer_luts_set_data
> > + transfer_data[IMGU_ABI_SHD_MAX_TRANSFERS]
> > +__attribute__((aligned(32)));
>
> Could you replace this wth __aligned(32), please? The same for the rest of the
> header.
>
Using __aligned(32) in the uAPI header resulted in compilation errors in
user space / camera HAL code.
e.g
../../../../../../../../usr/include/linux/intel-ipu3.h:464:57: error: expected
';'
at end of declaration list
__u8 bayer_table[IPU3_UAPI_AWB_FR_BAYER_TABLE_MAX_SIZE] __aligned(32);
So we ended up using __attribute__((aligned(32))) format in uAPI header and
to be consistent, we followed the same format in ABI header as well.
Let us know if it's okay to deviate between uAPI and ABI header for this
alignment qualifier.
> --
> Regards,
>
> Sakari Ailus
> sakari.ai...@linux.intel.com
Re: [PATCH v7 05/16] intel-ipu3: abi: Add structs
Hi Yong,
On Mon, Oct 29, 2018 at 03:22:59PM -0700, Yong Zhi wrote:
> This add all the structs of IPU3 firmware ABI.
>
> Signed-off-by: Yong Zhi
> Signed-off-by: Rajmohan Mani
...
> +struct imgu_abi_shd_intra_frame_operations_data {
> + struct imgu_abi_acc_operation
> + operation_list[IMGU_ABI_SHD_MAX_OPERATIONS]
> __attribute__((aligned(32)));
> + struct imgu_abi_acc_process_lines_cmd_data
> + process_lines_data[IMGU_ABI_SHD_MAX_PROCESS_LINES]
> __attribute__((aligned(32)));
> + struct imgu_abi_shd_transfer_luts_set_data
> + transfer_data[IMGU_ABI_SHD_MAX_TRANSFERS]
> __attribute__((aligned(32)));
Could you replace this wth __aligned(32), please? The same for the rest of
the header.
--
Regards,
Sakari Ailus
sakari.ai...@linux.intel.com
[PATCH v7 05/16] intel-ipu3: abi: Add structs
This add all the structs of IPU3 firmware ABI.
Signed-off-by: Yong Zhi
Signed-off-by: Rajmohan Mani
---
drivers/media/pci/intel/ipu3/ipu3-abi.h | 1350 +++
1 file changed, 1350 insertions(+)
diff --git a/drivers/media/pci/intel/ipu3/ipu3-abi.h
b/drivers/media/pci/intel/ipu3/ipu3-abi.h
index ac08ad3..21703da 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-abi.h
+++ b/drivers/media/pci/intel/ipu3/ipu3-abi.h
@@ -658,4 +658,1354 @@ enum imgu_abi_stage_type {
IMGU_ABI_STAGE_TYPE_ISP,
};
+struct imgu_abi_acc_operation {
+ /*
+* zero means on init,
+* others mean upon receiving an ack signal from the BC acc.
+*/
+ u8 op_indicator;
+ u8 op_type;
+} __packed;
+
+struct imgu_abi_acc_process_lines_cmd_data {
+ u16 lines;
+ u8 cfg_set;
+ u8 __reserved; /* Align to 4 bytes */
+} __packed;
+
+/* Bayer shading definitions */
+
+struct imgu_abi_shd_transfer_luts_set_data {
+ u8 set_number;
+ u8 padding[3];
+ imgu_addr_t rg_lut_ddr_addr;
+ imgu_addr_t bg_lut_ddr_addr;
+ u32 align_dummy;
+} __packed;
+
+struct imgu_abi_shd_grid_config {
+ /* reg 0 */
+ u32 grid_width:8;
+ u32 grid_height:8;
+ u32 block_width:3;
+ u32 __reserved0:1;
+ u32 block_height:3;
+ u32 __reserved1:1;
+ u32 grid_height_per_slice:8;
+ /* reg 1 */
+ s32 x_start:13;
+ s32 __reserved2:3;
+ s32 y_start:13;
+ s32 __reserved3:3;
+} __packed;
+
+struct imgu_abi_shd_general_config {
+ u32 init_set_vrt_offst_ul:8;
+ u32 shd_enable:1;
+ /* aka 'gf' */
+ u32 gain_factor:2;
+ u32 __reserved:21;
+} __packed;
+
+struct imgu_abi_shd_black_level_config {
+ /* reg 0 */
+ s32 bl_r:12;
+ s32 __reserved0:4;
+ s32 bl_gr:12;
+ u32 __reserved1:1;
+ /* aka 'nf' */
+ u32 normalization_shift:3;
+ /* reg 1 */
+ s32 bl_gb:12;
+ s32 __reserved2:4;
+ s32 bl_b:12;
+ s32 __reserved3:4;
+} __packed;
+
+struct imgu_abi_shd_intra_frame_operations_data {
+ struct imgu_abi_acc_operation
+ operation_list[IMGU_ABI_SHD_MAX_OPERATIONS]
__attribute__((aligned(32)));
+ struct imgu_abi_acc_process_lines_cmd_data
+ process_lines_data[IMGU_ABI_SHD_MAX_PROCESS_LINES]
__attribute__((aligned(32)));
+ struct imgu_abi_shd_transfer_luts_set_data
+ transfer_data[IMGU_ABI_SHD_MAX_TRANSFERS]
__attribute__((aligned(32)));
+} __packed;
+
+struct imgu_abi_shd_config {
+ struct ipu3_uapi_shd_config_static shd __attribute__((aligned(32)));
+ struct imgu_abi_shd_intra_frame_operations_data shd_ops
__attribute__((aligned(32)));
+ struct ipu3_uapi_shd_lut shd_lut __attribute__((aligned(32)));
+} __packed;
+
+struct imgu_abi_stripe_input_frame_resolution {
+ u16 width;
+ u16 height;
+ u32 bayer_order;/* enum ipu3_uapi_bayer_order */
+ u32 raw_bit_depth;
+} __packed;
+
+/* Stripe-based processing */
+
+struct imgu_abi_stripes {
+ /* offset from start of frame - measured in pixels */
+ u16 offset;
+ /* stripe width - measured in pixels */
+ u16 width;
+ /* stripe width - measured in pixels */
+ u16 height;
+} __packed;
+
+struct imgu_abi_stripe_data {
+ /*
+* number of stripes for current processing source
+* - VLIW binary parameter we currently support 1 or 2 stripes
+*/
+ u16 num_of_stripes;
+
+ u8 padding[2];
+
+ /*
+* the following data is derived from resolution-related
+* pipe config and from num_of_stripes
+*/
+
+ /*
+*'input-stripes' - before input cropping
+* used by input feeder
+*/
+ struct imgu_abi_stripe_input_frame_resolution input_frame;
+
+ /*'effective-stripes' - after input cropping used dpc, bds */
+ struct imgu_abi_stripes effective_stripes[IPU3_UAPI_MAX_STRIPES];
+
+ /* 'down-scaled-stripes' - after down-scaling ONLY. used by BDS */
+ struct imgu_abi_stripes down_scaled_stripes[IPU3_UAPI_MAX_STRIPES];
+
+ /*
+*'bds-out-stripes' - after bayer down-scaling and padding.
+* used by all algos starting with norm up to the ref-frame for GDC
+* (currently up to the output kernel)
+*/
+ struct imgu_abi_stripes bds_out_stripes[IPU3_UAPI_MAX_STRIPES];
+
+ /* 'bds-out-stripes (no overlap)' - used for ref kernel */
+ struct imgu_abi_stripes
+ bds_out_stripes_no_overlap[IPU3_UAPI_MAX_STRIPES];
+
+ /*
+* input resolution for output system (equal to bds_out - envelope)
+* output-system input frame width as configured by user
+*/
+ u16 output_system_in_frame_width;
+ /* output-system input frame height as configured by user */
+ u16 output_system_in_frame_height;
+
+ /*
+*
