Hi Dan,
On Mon, Jul 29, 2024 at 11:51:43AM +0100, Daniel Scally wrote:
> On 23/07/2024 00:48, Laurent Pinchart wrote:
> > On Tue, Jul 09, 2024 at 02:29:04PM +0100, Daniel Scally wrote:
> >> Add structures describing the ISP parameters to mali-c55-config.h
> >>
> >> Acked-by: Nayden Kanchev <nayden.kanchev@xxxxxxx>
> >> Co-developed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> >> Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> >> Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
> >> ---
> >> Changes in v6:
> >>
> >> - Flagged which struct goes with which enum value from
> >> enum mali_c55_param_block_type
> >> - Used only types with well defined sizes in the structs
> >> - Expanded the documentation for skipping in the AEXP histogram config
> >> - Aligned the header struct to 64 bits
> >> - Added a new union type to hold pointers to the parameter structs
> >>
> >> Changes in v5:
> >>
> >> - New patch
> >>
> >> .../uapi/linux/media/arm/mali-c55-config.h | 763 ++++++++++++++++++
> >> 1 file changed, 763 insertions(+)
> >>
> >> diff --git a/include/uapi/linux/media/arm/mali-c55-config.h b/include/uapi/linux/media/arm/mali-c55-config.h
> >> index 21b453bdee5d..ea9872e9076f 100644
> >> --- a/include/uapi/linux/media/arm/mali-c55-config.h
> >> +++ b/include/uapi/linux/media/arm/mali-c55-config.h
> >> @@ -179,4 +179,767 @@ struct mali_c55_stats_buffer {
> >> __u32 reserved3[15];
> >> } __attribute__((packed));
> >>
> >> +/**
> >> + * enum mali_c55_param_buffer_version - Mali-C55 parameters block versioning
> >> + *
> >> + * @MALI_C55_PARAM_BUFFER_V1: First version of Mali-C55 parameters block
> >> + */
> >> +enum mali_c55_param_buffer_version {
> >> + MALI_C55_PARAM_BUFFER_V1,
> >> +};
> >> +
> >> +/**
> >> + * enum mali_c55_param_block_type - Enumeration of Mali-C55 parameter blocks
> >> + *
> >> + * This enumeration defines the types of Mali-C55 parameters block. Each block
> >> + * configures a specific processing block of the Mali-C55 ISP. The block
> >> + * type allows the driver to correctly interpret the parameters block data.
> >> + *
> >> + * It is the responsibility of userspace to correctly set the type of each
> >> + * parameters block.
> >> + *
> >> + * @MALI_C55_PARAM_BLOCK_SENSOR_OFFS: Sensor pre-shading black level offset
> >> + * @MALI_C55_PARAM_BLOCK_AEXP_HIST: Auto-exposure 1024-bin histogram
> >> + * configuration
> >> + * @MALI_C55_PARAM_BLOCK_AEXP_IHIST: Post-Iridix auto-exposure 1024-bin
> >> + * histogram configuration
> >> + * @MALI_C55_PARAM_BLOCK_AEXP_HIST_WEIGHTS: Auto-exposure 1024-bin histogram
> >> + * weighting
> >> + * @MALI_C55_PARAM_BLOCK_AEXP_IHIST_WEIGHTS: Post-Iridix auto-exposure 1024-bin
> >> + * histogram weighting
> >> + * @MALI_C55_PARAM_BLOCK_DIGITAL_GAIN: Digital gain
> >> + * @MALI_C55_PARAM_BLOCK_AWB_GAINS: Auto-white balance gains
> >> + * @MALI_C55_PARAM_BLOCK_AWB_CONFIG: Auto-white balance statistics config
> >> + * @MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP: Auto-white balance gains for AEXP-0 tap
> >> + * @MALI_C55_PARAM_MESH_SHADING_CONFIG : Mesh shading tables configuration
> >> + * @MALI_C55_PARAM_MESH_SHADING_SELECTION: Mesh shading table selection
> >> + * @MALI_C55_PARAM_BLOCK_SENTINEL: First non-valid block index
> >
> > The sentinel value will change when adding new types. That may affect
> > userspace. We managed to get rid of the sentinel in the rkisp1 driver,
> > it would be nice to do the same here.
>
> Ack - I'm sure it'll be possible.
>
> >> + */
> >> +enum mali_c55_param_block_type {
> >> + MALI_C55_PARAM_BLOCK_SENSOR_OFFS,
> >> + MALI_C55_PARAM_BLOCK_AEXP_HIST,
> >> + MALI_C55_PARAM_BLOCK_AEXP_IHIST,
> >> + MALI_C55_PARAM_BLOCK_AEXP_HIST_WEIGHTS,
> >> + MALI_C55_PARAM_BLOCK_AEXP_IHIST_WEIGHTS,
> >> + MALI_C55_PARAM_BLOCK_DIGITAL_GAIN,
> >> + MALI_C55_PARAM_BLOCK_AWB_GAINS,
> >> + MALI_C55_PARAM_BLOCK_AWB_CONFIG,
> >> + MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP,
> >> + MALI_C55_PARAM_MESH_SHADING_CONFIG,
> >> + MALI_C55_PARAM_MESH_SHADING_SELECTION,
> >> + MALI_C55_PARAM_BLOCK_SENTINEL,
> >> +};
> >> +
> >> +/**
> >> + * struct mali_c55_params_block_header - Mali-C55 parameter block header
> >> + *
> >> + * This structure represents the common part of all the ISP configuration
> >> + * blocks. Each parameters block embeds an instance of this structure type
> >> + * as its first member, followed by the block-specific configuration data. The
> >> + * driver inspects this common header to discern the block type and its size and
> >> + * properly handle the block content by casting it to the correct block-specific
> >> + * type.
> >> + *
> >> + * The @type field is one of the values enumerated by
> >> + * :c:type:`mali_c55_param_block_type` and specifies how the data should be
> >> + * interpreted by the driver. The @size field specifies the size of the
> >> + * parameters block and is used by the driver for validation purposes. The
> >> + * @enabled field specifies if the ISP block should be enabled (and configured
> >> + * according to the provided parameters) or disabled.
> >> + *
> >> + * Userspace is responsible for correctly populating the parameters block header
> >> + * fields (@type, @enabled and @size) and correctly populate the block-specific
> >> + * parameters.
> >> + *
> >> + * For example:
> >> + *
> >> + * .. code-block:: c
> >> + *
> >> + * void populate_sensor_offs(struct mali_c55_params_block_header *block) {
> >> + * block->type = MALI_C55_PARAM_BLOCK_SENSOR_OFFS;
> >> + * block->enabled = true;
> >> + * block->size = sizeof(struct mali_c55_params_sensor_off_preshading);
> >> + *
> >> + * struct mali_c55_params_sensor_off_preshading *sensor_offs =
> >> + * (struct mali_c55_params_sensor_off_preshading *)block;
> >> + *
> >> + * sensor_offs->chan00 = offset00;
> >> + * sensor_offs->chan01 = offset01;
> >> + * sensor_offs->chan10 = offset10;
> >> + * sensor_offs->chan11 = offset11;
> >> + * }
> >> + *
> >> + * @type: The parameters block type from :c:type:`mali_c55_param_block_type`
> >> + * @enabled: Block enabled/disabled flag
> >> + * @size: Size (in bytes) of the parameters block
> >> + */
> >> +struct mali_c55_params_block_header {
> >> + __u8 type;
> >> + __u8 enabled;
> >> + __u32 size;
> >> +} __attribute__((aligned(8)));
> >> +
> >> +/**
> >> + * struct mali_c55_params_sensor_off_preshading - offset subtraction for each
> >> + * color channel
> >> + *
> >> + * Provides removal of the sensor black level from the sensor data. Separate
> >> + * offsets are provided for each of the four Bayer component color channels
> >> + * which are defaulted to R, Gr, Gb, B.
> >> + *
> >> + * header.type should be set to MALI_C55_PARAM_BLOCK_SENSOR_OFFS from
> >> + * :c:type:`mali_c55_param_block_type` for this block.
> >> + *
> >> + * @header: The Mali-C55 parameters block header
> >> + * @chan00: Offset for color channel 00 (default: R)
> >> + * @chan01: Offset for color channel 01 (default: Gr)
> >> + * @chan10: Offset for color channel 10 (default: Gb)
> >> + * @chan11: Offset for color channel 11 (default: B)
> >> + */
> >> +struct mali_c55_params_sensor_off_preshading {
> >> + struct mali_c55_params_block_header header;
> >> + __u32 chan00;
> >> + __u32 chan01;
> >> + __u32 chan10;
> >> + __u32 chan11;
> >> +};
> >> +
> >> +/**
> >> + * enum mali_c55_aexp_hist_tap_points - Tap points for the AEXP histogram
> >> + * @MALI_C55_AEXP_HIST_TAP_WB: After static white balance
> >> + * @MALI_C55_AEXP_HIST_TAP_FS: After WDR Frame Stitch
> >> + * @MALI_C55_AEXP_HIST_TAP_TPG: After the test pattern generator
> >> + */
> >> +enum mali_c55_aexp_hist_tap_points {
> >> + MALI_C55_AEXP_HIST_TAP_WB = 0,
> >> + MALI_C55_AEXP_HIST_TAP_FS,
> >> + MALI_C55_AEXP_HIST_TAP_TPG,
> >> +};
> >> +
> >> +/**
> >> + * enum mali_c55_aexp_skip_x - Horizontal pixel skipping
> >> + * @MALI_C55_AEXP_SKIP_X_EVERY_2ND: Collect every 2nd pixel horizontally
> >> + * @MALI_C55_AEXP_SKIP_X_EVERY_3RD: Collect every 3rd pixel horizontally
> >> + * @MALI_C55_AEXP_SKIP_X_EVERY_4TH: Collect every 4th pixel horizontally
> >> + * @MALI_C55_AEXP_SKIP_X_EVERY_5TH: Collect every 5th pixel horizontally
> >> + * @MALI_C55_AEXP_SKIP_X_EVERY_8TH: Collect every 8th pixel horizontally
> >> + * @MALI_C55_AEXP_SKIP_X_EVERY_9TH: Collect every 9th pixel horizontally
> >> + */
> >> +enum mali_c55_aexp_skip_x {
> >> + MALI_C55_AEXP_SKIP_X_EVERY_2ND,
> >> + MALI_C55_AEXP_SKIP_X_EVERY_3RD,
> >> + MALI_C55_AEXP_SKIP_X_EVERY_4TH,
> >> + MALI_C55_AEXP_SKIP_X_EVERY_5TH,
> >> + MALI_C55_AEXP_SKIP_X_EVERY_8TH,
> >> + MALI_C55_AEXP_SKIP_X_EVERY_9TH
> >> +};
> >> +
> >> +/**
> >> + * enum mali_c55_aexp_skip_y - Vertical pixel skipping
> >> + * @MALI_C55_AEXP_SKIP_Y_ALL: Collect every single pixel vertically
> >> + * @MALI_C55_AEXP_SKIP_Y_EVERY_2ND: Collect every 2nd pixel vertically
> >> + * @MALI_C55_AEXP_SKIP_Y_EVERY_3RD: Collect every 3rd pixel vertically
> >> + * @MALI_C55_AEXP_SKIP_Y_EVERY_4TH: Collect every 4th pixel vertically
> >> + * @MALI_C55_AEXP_SKIP_Y_EVERY_5TH: Collect every 5th pixel vertically
> >> + * @MALI_C55_AEXP_SKIP_Y_EVERY_8TH: Collect every 8th pixel vertically
> >> + * @MALI_C55_AEXP_SKIP_Y_EVERY_9TH: Collect every 9th pixel vertically
> >> + */
> >> +enum mali_c55_aexp_skip_y {
> >> + MALI_C55_AEXP_SKIP_Y_ALL,
> >> + MALI_C55_AEXP_SKIP_Y_EVERY_2ND,
> >> + MALI_C55_AEXP_SKIP_Y_EVERY_3RD,
> >> + MALI_C55_AEXP_SKIP_Y_EVERY_4TH,
> >> + MALI_C55_AEXP_SKIP_Y_EVERY_5TH,
> >> + MALI_C55_AEXP_SKIP_Y_EVERY_8TH,
> >> + MALI_C55_AEXP_SKIP_Y_EVERY_9TH
> >> +};
> >> +
> >> +/**
> >> + * enum mali_c55_aexp_row_column_offset - Start from the first or second row or
> >> + * column
> >> + * @MALI_C55_AEXP_FIRST_ROW_OR_COL: Start from the first row / column
> >> + * @MALI_C55_AEXP_SECOND_ROW_OR_COL: Start from the second row / column
> >> + */
> >> +enum mali_c55_aexp_row_column_offset {
> >> + MALI_C55_AEXP_FIRST_ROW_OR_COL = 1,
> >> + MALI_C55_AEXP_SECOND_ROW_OR_COL = 2,
> >> +};
> >> +
> >> +/**
> >> + * enum mali_c55_aexp_hist_plane_mode - Mode for the AEXP Histograms
> >> + * @MALI_C55_AEXP_HIST_COMBINED: All color planes in one 1024-bin histogram
> >> + * @MALI_C55_AEXP_HIST_SEPARATE: Each color plane in one 256-bin histogram with a bin width of 16
> >> + * @MALI_C55_AEXP_HIST_FOCUS_00: Top left plane in the first bank, rest in second bank
> >> + * @MALI_C55_AEXP_HIST_FOCUS_01: Top right plane in the first bank, rest in second bank
> >> + * @MALI_C55_AEXP_HIST_FOCUS_10: Bottom left plane in the first bank, rest in second bank
> >> + * @MALI_C55_AEXP_HIST_FOCUS_11: Bottom right plane in the first bank, rest in second bank
> >> + *
> >> + * In the "focus" modes statistics are collected into two 512-bin histograms
> >> + * with a bin width of 8. One colour plane is in the first histogram with the
> >> + * remainder combined into the second. The four options represent which of the
> >> + * four positions in a bayer pattern are the focused plane.
> >> + */
> >> +enum mali_c55_aexp_hist_plane_mode {
> >> + MALI_C55_AEXP_HIST_COMBINED = 0,
> >> + MALI_C55_AEXP_HIST_SEPARATE = 1,
> >> + MALI_C55_AEXP_HIST_FOCUS_00 = 4,
> >> + MALI_C55_AEXP_HIST_FOCUS_01 = 5,
> >> + MALI_C55_AEXP_HIST_FOCUS_10 = 6,
> >> + MALI_C55_AEXP_HIST_FOCUS_11 = 7,
> >> +};
> >> +
> >> +/**
> >> + * struct mali_c55_params_aexp_hist - configuration for AEXP metering hists
> >> + *
> >> + * This struct allows users to configure the 1024-bin AEXP histograms. Broadly
> >> + * speaking the parameters allow you to mask particular regions of the image and
> >> + * to select different kinds of histogram.
> >> + *
> >> + * The skip_x, offset_x, skip_y and offset_y fields allow users to ignore or
> >> + * mask pixels in the frame by their position relative to the top left pixel.
> >> + * First, the skip_y, offset_x and offset_y fields define a 2x2 pixel pattern of
> >> + * which pixels covered by the pattern will be counted in the statistics.
> >
> > I have a really hard time parsing the second sentence. Let me try to
> > rephrase it.
> >
> > * First, the skip_y, offset_x and offset_y fields define which pixels will be
> > * counted in the statistics within a 2x2 pixels region.
> >
> > Is that what you meant ?
>
> Yes, although I feel it's a bit iffy...perhaps "...which of the pixels within each 2x2 region will
> be counted in the statistics"?
Sounds good to me.
> >> + *
> >> + * If skip_y == 0 then two pixels from each coveredregion will be counted. If
> >
> > s/coveredregion/region/
> >
> >> + * both offset_x and offset_y are zero, then the two left-most pixels in each
> >> + * 2x2 pixel region covered by the pattern will be counted. Setting offset_x = 1
> >
> > I'd drop "covered by the pattern" as I'm not sure what that means.
>
> OK, I think it reads fine that way.
>
> >> + * will discount the top left pixel and count the top right pixel. Setting
> >> + * offset_y = 1 will discount the bottom left pixel and count the bottom right
> >> + * pixel.
> >
> > That sounds strange, I would have expected offset_y to control something
> > vertically, not control the horizontal position on the second line. If
> > that's how the hardware operates, why not.
>
> Yes it is strange, but that's what the documentation says.
>
> >> + *
> >> + * If skip_y != 0 then only a single pixel from each region covered by the
> >> + * pattern will be counted. In this case offset_x controls whether the pixel
> >> + * that's counted is in the left (if offset_x == 0) or right (if offset_x == 1)
> >> + * column and offset_y controls whether the pixel that's counted is in the top
> >> + * (if offset_y == 0) or bottom (if offset_y == 1) row.
> >> + *
> >> + * The skip_x and skip_y fields control how the pixel masking pattern is
> >> + * repeated across the image data. The first instance of the pattern is always
> >
> > And here maybe "control how the 2x2 pixel region is repeated ...".
> > s/pattern/region/ in the rest.
>
> Okedokey
>
> > This is otherwise a great explanation, thank you.
>
> Thanks!
>
> >> + * in the top left of the image data. The skip_x field controls how many pixels
> >> + * are ignored in the x direction before the pixel masking pattern is repeated.
> >> + * The skip_y field controls how many pixels are ignored in the y direction
> >> + * before the pixel masking pattern is repeated.
> >> + *
> >> + * These fields can be used to reduce the number of pixels counted for the
> >> + * statistics, but it's important to be careful to configure them correctly.
> >> + * Some combinations of values will result in colour components from the input
> >> + * data being ignored entirely, for example in the following configuration:
> >> + *
> >> + * skip_x = 0
> >> + * offset_x = 0
> >> + * skip_y = 0
> >> + * offset_y = 0
> >> + *
> >> + * Only the R and Gb components of RGGB data that was input would be collected.
> >> + * Similarly in the following configuration:
> >> + *
> >> + * skip_x = 0
> >> + * offset_x = 0
> >> + * skip_y = 1
> >> + * offset_y = 1
> >> + *
> >> + * Only the Gb component of RGGB data that was input would be collected. To
> >> + * correct things such that all 4 colour components were included it would be
> >> + * necessary to set the skip_x and skip_y fields in a way that resulted in all
> >> + * four colour components being collected:
> >> + *
> >> + * skip_x = 1
> >> + * offset_x = 0
> >> + * skip_y = 1
> >> + * offset_y = 1
> >> + *
> >> + * header.type should be set to one of either MALI_C55_PARAM_BLOCK_AEXP_HIST or
> >> + * MALI_C55_PARAM_BLOCK_AEXP_IHIST from :c:type:`mali_c55_param_block_type`.
> >> + *
> >> + * @header: The Mali-C55 parameters block header
> >> + * @skip_x: Horizontal decimation. See enum mali_c55_aexp_skip_x
> >> + * @offset_x: Skip the first column, or not. See enum mali_c55_aexp_row_column_offset
> >> + * @skip_y: Vertical decimation. See enum mali_c55_aexp_skip_y
> >> + * @offset_y: Skip the first row, or not. See enum mali_c55_aexp_row_column_offset
> >> + * @scale_bottom: Scale pixels in bottom half of intensity range: 0=1x ,1=2x, 2=4x, 4=8x, 4=16x
> >> + * @scale_top: scale pixels in top half of intensity range: 0=1x ,1=2x, 2=4x, 4=8x, 4=16x
> >> + * @plane_mode: Plane separation mode. See enum mali_c55_aexp_hist_plane_mode
> >> + * @tap_point: Tap point for histogram from enum mali_c55_aexp_hist_tap_points.
> >> + * This parameter is unused for the post-Iridix Histogram
> >> + */
> >> +struct mali_c55_params_aexp_hist {
> >> + struct mali_c55_params_block_header header;
> >> + __u8 skip_x;
> >> + __u8 offset_x;
> >> + __u8 skip_y;
> >> + __u8 offset_y;
> >> + __u8 scale_bottom;
> >> + __u8 scale_top;
> >> + __u8 plane_mode;
> >> + __u8 tap_point;
> >> +};
> >> +
> >> +/**
> >> + * struct mali_c55_params_aexp_weights - Array of weights for AEXP metering
> >> + *
> >> + * This struct allows users to configure the weighting for both of the 1024-bin
> >> + * AEXP histograms. The pixel data collected for each zone is multiplied by the
> >> + * corresponding weight from this array, which may be zero if the intention is
> >> + * to mask off the zone entirely.
> >> + *
> >> + * header.type should be set to one of either MALI_C55_PARAM_BLOCK_AEXP_HIST_WEIGHTS
> >> + * or MALI_C55_PARAM_BLOCK_AEXP_IHIST_WEIGHTS from :c:type:`mali_c55_param_block_type`.
> >> + *
> >> + * @header: The Mali-C55 parameters block header
> >> + * @nodes_used_horiz: Number of active zones horizontally [0..15]
> >> + * @nodes_used_vert: Number of active zones vertically [0..15]
> >> + * @zone_weights: Zone weighting. Index is row*col where 0,0 is the top
> >> + * left zone continuing in raster order. Each zone can be
> >> + * weighted in the range [0..15]. The number of rows and
> >> + * columns is defined by @nodes_used_vert and
> >> + * @nodes_used_horiz
> >> + */
> >> +struct mali_c55_params_aexp_weights {
> >> + struct mali_c55_params_block_header header;
> >> + __u8 nodes_used_horiz;
> >> + __u8 nodes_used_vert;
> >> + __u8 zone_weights[MALI_C55_MAX_ZONES];
> >> +};
> >> +
> >> +/**
> >> + * struct mali_c55_params_digital_gain - Digital gain value
> >> + *
> >> + * This struct carries a digital gain value to set in the ISP.
> >> + *
> >> + * header.type should be set to MALI_C55_PARAM_BLOCK_DIGITAL_GAIN from
> >> + * :c:type:`mali_c55_param_block_type` for this block.
> >> + *
> >> + * @header: The Mali-C55 parameters block header
> >> + * @gain: The digital gain value to apply, in Q5.8 format.
> >> + */
> >> +struct mali_c55_params_digital_gain {
> >> + struct mali_c55_params_block_header header;
> >> + __u16 gain;
> >> +};
> >> +
> >> +/**
> >> + * enum mali_c55_awb_stats_mode - Statistics mode for AWB
> >> + * @MALI_C55_AWB_MODE_GRBR: Statistics collected as Green/Red and Blue/Red ratios
> >> + * @MALI_C55_AWB_MODE_RGBG: Statistics collected as Red/Green and Blue/Green ratios
> >> + */
> >> +enum mali_c55_awb_stats_mode {
> >> + MALI_C55_AWB_MODE_GRBR = 0,
> >> + MALI_C55_AWB_MODE_RGBG,
> >> +};
> >> +
> >> +/**
> >> + * struct mali_c55_params_awb_gains - Gain settings for auto white balance
> >> + *
> >> + * This struct allows users to configure the gains for auto-white balance. There
> >> + * are four gain settings corresponding to each colour channel in the bayer
> >> + * domain. Although named generically, the association between the gain applied
> >> + * and the colour channel is done automatically within the ISP depending on the
> >> + * input format, and so the following mapping always holds true::
> >> + *
> >> + * gain00 = R
> >> + * gain01 = Gr
> >> + * gain10 = Gb
> >> + * gain11 = B
> >> + *
> >> + * All of the gains are stored in Q4.8 format.
> >> + *
> >> + * header.type should be set to one of either MALI_C55_PARAM_BLOCK_AWB_GAINS or
> >> + * MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP from :c:type:`mali_c55_param_block_type`.
> >> + *
> >> + * @header: The Mali-C55 parameters block header
> >> + * @gain00: Multiplier for colour channel 00
> >> + * @gain01: Multiplier for colour channel 01
> >> + * @gain10: Multiplier for colour channel 10
> >> + * @gain11: Multiplier for colour channel 11
> >> + */
> >> +struct mali_c55_params_awb_gains {
> >> + struct mali_c55_params_block_header header;
> >> + __u16 gain00;
> >> + __u16 gain01;
> >> + __u16 gain10;
> >> + __u16 gain11;
> >> +};
> >> +
> >> +/**
> >> + * enum mali_c55_params_awb_tap_points - Tap points for the AWB statistics
> >> + * @MALI_C55_AWB_STATS_TAP_PF: Immediately after the Purple Fringe block
> >> + * @MALI_C55_AWB_STATS_TAP_CNR: Immediately after the CNR block
> >> + */
> >> +enum mali_c55_params_awb_tap_points {
> >> + MALI_C55_AWB_STATS_TAP_PF = 0,
> >> + MALI_C55_AWB_STATS_TAP_CNR,
> >> +};
> >> +
> >> +/**
> >> + * struct mali_c55_params_awb_config - Stats settings for auto-white balance
> >> + *
> >> + * This struct allows the configuration of the statistics generated for auto
> >> + * white balance. Pixel intensity limits can be set to exclude overly bright or
> >> + * dark regions of an image from the statistics entirely. Colour ratio minima
> >> + * and maxima can be set to discount pixels who's ratios fall outside the
> >> + * defined boundaries; there are two sets of registers to do this - the
> >> + * "min/max" ratios which bound a region and the "high/low" ratios which further
> >> + * trim the upper and lower ratios. For example with the boundaries configured
> >> + * as follows, only pixels whos colour ratios falls into the region marked "A"
> >> + * would be counted::
> >> + *
> >> + * cr_high
> >> + * 2.0 | |
> >> + * | cb_max --> _________________________v_____
> >> + * 1.8 | | \ |
> >> + * | | \ |
> >> + * 1.6 | | \ |
> >> + * | | \ |
> >> + * c 1.4 | cb_low -->|\ A \|<-- cb_high
> >> + * b | | \ |
> >> + * 1.2 | | \ |
> >> + * r | | \ |
> >> + * a 1.0 | cb_min --> |____\_________________________|
> >> + * t | ^ ^ ^
> >> + * i 0.8 | | | |
> >> + * o | cr_min | cr_max
> >> + * s 0.6 | |
> >> + * | cr_low
> >> + * 0.4 |
> >> + * |
> >> + * 0.2 |
> >> + * |
> >> + * 0.0 |_______________________________________________________________
> >> + * 0.0 0.2 0.4 0.6 0.8 1.0 1.2 1.4 1.6 1.8 2.0
> >> + * cr ratios
> >> + *
> >> + * header.type should be set to MALI_C55_PARAM_BLOCK_AWB_CONFIG from
> >> + * :c:type:`mali_c55_param_block_type` for this block.
> >> + *
> >> + * @header: The Mali-C55 parameters block header
> >> + * @tap_point: The tap point from enum mali_c55_params_awb_tap_points
> >> + * @stats_mode: AWB statistics collection mode, see :c:type:`mali_c55_awb_stats_mode`
> >> + * @white_level: Upper pixel intensity (I.E. raw pixel values) limit
> >> + * @black_level: Lower pixel intensity (I.E. raw pixel values) limit
> >> + * @cr_max: Maximum R/G ratio (Q4.8 format)
> >> + * @cr_min: Minimum R/G ratio (Q4.8 format)
> >> + * @cb_max: Maximum B/G ratio (Q4.8 format)
> >> + * @cb_min: Minimum B/G ratio (Q4.8 format)
> >> + * @nodes_used_horiz: Number of active zones horizontally [0..15]
> >> + * @nodes_used_vert: Number of active zones vertically [0..15]
> >> + * @cr_high: R/G ratio trim high (Q4.8 format)
> >> + * @cr_low: R/G ratio trim low (Q4.8 format)
> >> + * @cb_high: B/G ratio trim high (Q4.8 format)
> >> + * @cb_low: B/G ratio trim low (Q4.8 format)
> >> + */
> >> +struct mali_c55_params_awb_config {
> >> + struct mali_c55_params_block_header header;
> >> + __u8 tap_point;
> >> + __u8 stats_mode;
> >> + __u16 white_level;
> >> + __u16 black_level;
> >> + __u16 cr_max;
> >> + __u16 cr_min;
> >> + __u16 cb_max;
> >> + __u16 cb_min;
> >> + __u8 nodes_used_horiz;
> >> + __u8 nodes_used_vert;
> >> + __u16 cr_high;
> >> + __u16 cr_low;
> >> + __u16 cb_high;
> >> + __u16 cb_low;
> >> +};
> >> +
> >> +#define MALI_C55_NUM_MESH_SHADING_ELEMENTS 3072
> >> +
> >> +/**
> >> + * struct mali_c55_params_mesh_shading_config - Mesh shading configuration
> >> + *
> >> + * The mesh shading correction module allows programming a separate table of
> >> + * either 16x16 or 32x32 node coefficients for 3 different light sources. The
> >> + * final correction coefficients applied are computed by blending the
> >> + * coefficients from two tables together.
> >> + *
> >> + * A page of 1024 32-bit integers is associated to each colour channel, with
> >> + * pages stored consecutively in memory. Each 32-bit integer packs 3 8-bit
> >> + * correction coefficients for a single node, one for each of the three light
> >> + * sources. The 8 most significant bits are unused. The following table
> >> + * describes the layout::
> >> + *
> >> + * +----------- Page (Colour Plane) 0 -------------+
> >> + * | @mesh[i] | Mesh Point | Bits | Light Source |
> >> + * +-----------+------------+-------+--------------+
> >> + * | 0 | 0,0 | 16,23 | LS2 |
> >> + * | | | 08-15 | LS1 |
> >> + * | | | 00-07 | LS0 |
> >> + * +-----------+------------+-------+--------------+
> >> + * | 1 | 0,1 | 16,23 | LS2 |
> >> + * | | | 08-15 | LS1 |
> >> + * | | | 00-07 | LS0 |
> >> + * +-----------+------------+-------+--------------+
> >> + * | ... | ... | ... | ... |
> >> + * +-----------+------------+-------+--------------+
> >> + * | 1023 | 31,31 | 16,23 | LS2 |
> >> + * | | | 08-15 | LS1 |
> >> + * | | | 00-07 | LS0 |
> >> + * +----------- Page (Colour Plane) 1 -------------+
> >> + * | @mesh[i] | Mesh Point | Bits | Light Source |
> >> + * +-----------+------------+-------+--------------+
> >> + * | 1024 | 0,0 | 16,23 | LS2 |
> >> + * | | | 08-15 | LS1 |
> >> + * | | | 00-07 | LS0 |
> >> + * +-----------+------------+-------+--------------+
> >> + * | 1025 | 0,1 | 16,23 | LS2 |
> >> + * | | | 08-15 | LS1 |
> >> + * | | | 00-07 | LS0 |
> >> + * +-----------+------------+-------+--------------+
> >> + * | ... | ... | ... | ... |
> >> + * +-----------+------------+-------+--------------+
> >> + * | 2047 | 31,31 | 16,23 | LS2 |
> >> + * | | | 08-15 | LS1 |
> >> + * | | | 00-07 | LS0 |
> >> + * +----------- Page (Colour Plane) 2 -------------+
> >> + * | @mesh[i] | Mesh Point | Bits | Light Source |
> >> + * +-----------+------------+-------+--------------+
> >> + * | 2048 | 0,0 | 16,23 | LS2 |
> >> + * | | | 08-15 | LS1 |
> >> + * | | | 00-07 | LS0 |
> >> + * +-----------+------------+-------+--------------+
> >> + * | 2049 | 0,1 | 16,23 | LS2 |
> >> + * | | | 08-15 | LS1 |
> >> + * | | | 00-07 | LS0 |
> >> + * +-----------+------------+-------+--------------+
> >> + * | ... | ... | ... | ... |
> >> + * +-----------+------------+-------+--------------+
> >> + * | 3071 | 31,31 | 16,23 | LS2 |
> >> + * | | | 08-15 | LS1 |
> >> + * | | | 00-07 | LS0 |
> >> + * +-----------+------------+-------+--------------+
> >> + *
> >> + * The @mesh_scale member determines the precision and minimum and maximum gain.
> >> + * For example if @mesh_scale is 0 and therefore selects 0 - 2x gain, a value of
> >> + * 0 in a coefficient means 0.0 gain, a value of 128 means 1.0 gain and 255
> >> + * means 2.0 gain.
> >> + *
> >> + * header.type should be set to MALI_C55_PARAM_MESH_SHADING_CONFIG from
> >> + * :c:type:`mali_c55_param_block_type` for this block.
> >> + *
> >> + * @header: The Mali-C55 parameters block header
> >> + * @mesh_show: Output the mesh data rather than image data
> >> + * @mesh_scale: Set the precision and maximum gain range of mesh shading
> >> + * - 0 = 0-2x gain
> >> + * - 1 = 0-4x gain
> >> + * - 2 = 0-8x gain
> >> + * - 3 = 0-16x gain
> >> + * - 4 = 1-2x gain
> >> + * - 5 = 1-3x gain
> >> + * - 6 = 1-5x gain
> >> + * - 7 = 1-9x gain
> >> + * @mesh_page_r: Mesh page select for red colour plane [0..2]
> >> + * @mesh_page_g: Mesh page select for green colour plane [0..2]
> >> + * @mesh_page_b: Mesh page select for blue colour plane [0..2]
> >> + * @mesh_width: Number of horizontal nodes minus 1 [15,31]
> >> + * @mesh_height: Number of vertical nodes minus 1 [15,31]
> >> + * @mesh: Mesh shading correction tables
> >> + */
> >> +struct mali_c55_params_mesh_shading_config {
> >> + struct mali_c55_params_block_header header;
> >> + __u8 mesh_show;
> >> + __u8 mesh_scale;
> >> + __u8 mesh_page_r;
> >> + __u8 mesh_page_g;
> >> + __u8 mesh_page_b;
> >> + __u8 mesh_width;
> >> + __u8 mesh_height;
> >> + __u32 mesh[MALI_C55_NUM_MESH_SHADING_ELEMENTS];
> >> +};
> >> +
> >> +/** enum mali_c55_params_mesh_alpha_bank - Mesh shading table bank selection
> >> + * @MALI_C55_MESH_ALPHA_BANK_LS0_AND_LS1 - Select Light Sources 0 and 1
> >> + * @MALI_C55_MESH_ALPHA_BANK_LS1_AND_LS2 - Select Light Sources 1 and 2
> >> + * @MALI_C55_MESH_ALPHA_BANK_LS0_AND_LS2 - Select Light Sources 0 and 2
> >> + */
> >> +enum mali_c55_params_mesh_alpha_bank {
> >> + MALI_C55_MESH_ALPHA_BANK_LS0_AND_LS1 = 0,
> >> + MALI_C55_MESH_ALPHA_BANK_LS1_AND_LS2 = 1,
> >> + MALI_C55_MESH_ALPHA_BANK_LS0_AND_LS2 = 4
> >> +};
> >> +
> >> +/**
> >> + * struct mali_c55_params_mesh_shading_selection - Mesh table selection
> >> + *
> >> + * The module computes the final correction coefficients by blending the ones
> >> + * from two light source tables, which are selected (independently for each
> >> + * colour channel) by the @mesh_alpha_bank_r/g/b fields.
> >> + *
> >> + * The final blended coefficients for each node are calculated using the
> >> + * following equation:
> >> + *
> >> + * Final coefficient = (a * LS\ :sub:`b`\ + (256 - a) * LS\ :sub:`a`\) / 256
> >> + *
> >> + * Where a is the @mesh_alpha_r/g/b value, and LS\ :sub:`a`\ and LS\ :sub:`b`\
> >> + * are the node cofficients for the two tables selected by the
> >> + * @mesh_alpha_bank_r/g/b value.
> >> + *
> >> + * The scale of the applied correction may also be controlled by tuning the
> >> + * @mesh_strength member. This is a modifier to the final coefficients which can
> >> + * be used to globally reduce the gains applied.
> >> + *
> >> + * header.type should be set to MALI_C55_PARAM_MESH_SHADING_SELECTION from
> >> + * :c:type:`mali_c55_param_block_type` for this block.
> >> + *
> >> + * @header: The Mali-C55 parameters block header
> >> + * @mesh_alpha_bank_r: Red mesh table select (c:type:`enum mali_c55_params_mesh_alpha_bank`)
> >> + * @mesh_alpha_bank_g: Green mesh table select (c:type:`enum mali_c55_params_mesh_alpha_bank`)
> >> + * @mesh_alpha_bank_b: Blue mesh table select (c:type:`enum mali_c55_params_mesh_alpha_bank`)
> >> + * @mesh_alpha_r: Blend coefficient for R [0..255]
> >> + * @mesh_alpha_g: Blend coefficient for G [0..255]
> >> + * @mesh_alpha_b: Blend coefficient for B [0..255]
> >> + * @mesh_strength: Mesh strength in Q4.12 format [0..4096]
> >> + */
> >> +struct mali_c55_params_mesh_shading_selection {
> >> + struct mali_c55_params_block_header header;
> >> + __u8 mesh_alpha_bank_r;
> >> + __u8 mesh_alpha_bank_g;
> >> + __u8 mesh_alpha_bank_b;
> >> + __u8 mesh_alpha_r;
> >> + __u8 mesh_alpha_g;
> >> + __u8 mesh_alpha_b;
> >> + __u16 mesh_strength;
> >> +};
> >> +
> >> +/**
> >> + * union mali_c55_params_block - Generalisation of a parameter block
> >> + *
> >> + * This union allows the driver to treat a block as a generic pointer to this
> >> + * union and safely access the header and block-specific struct without having
> >> + * to resort to casting. The header member is accessed first, and the type field
> >
> > If this is for driver usage only, I would move the structure to the
> > driver. Let's minimize the UAPI surface.
>
> I have actually used the union in libcamera too, to clean up the code there handling filling the
> blocks. We could redefine it within libcamera of course but given it's useful in userspace too, I
> thought it made sense to keep it in the header...what do you think?
I would prefer minimizing the area of the UAPI. Have you looked at the
parameters helper I implemented for the rkisp1 IPA module ? Maybe a
similar approach could be used for the C55, and that wouldn't require
the union.
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> >
> >> + * checked which allows the driver to determine which of the other members
> >> + * should be used. The data member at the end allows a pointer to an address
> >> + * within the data member of :c:type:`mali_c55_params_buffer` to initialise a
> >> + * union variable.
> >> + *
> >> + * @header: Pointer to the shared header struct embedded as the
> >> + * first member of all the possible other members (except
> >> + * @data). This member would be accessed first and the type
> >> + * field checked to determine which of the other members
> >> + * should be accessed.
> >> + * @sensor_offs: For header->type == MALI_C55_PARAM_BLOCK_SENSOR_OFFS
> >> + * @aexp_hist: For header->type == MALI_C55_PARAM_BLOCK_AEXP_HIST and
> >> + * header->type == MALI_C55_PARAM_BLOCK_AEXP_IHIST
> >> + * @aexp_weights: For header->type == MALI_C55_PARAM_BLOCK_AEXP_HIST_WEIGHTS
> >> + * and header->type = MALI_C55_PARAM_BLOCK_AEXP_IHIST_WEIGHTS
> >> + * @digital_gain: For header->type == MALI_C55_PARAM_BLOCK_DIGITAL_GAIN
> >> + * @awb_gains: For header->type == MALI_C55_PARAM_BLOCK_AWB_GAINS and
> >> + * header->type = MALI_C55_PARAM_BLOCK_AWB_GAINS_AEXP
> >> + * @awb_config: For header->type == MALI_C55_PARAM_MESH_SHADING_CONFIG
> >> + * @shading_config: For header->type == MALI_C55_PARAM_MESH_SHADING_SELECTION
> >> + * @shading_selection: For header->type == MALI_C55_PARAM_BLOCK_SENSOR_OFFS
> >> + * @data: Allows easy initialisation of a union variable with a
> >> + * pointer into a __u8 array.
> >> + */
> >> +union mali_c55_params_block {
> >> + struct mali_c55_params_block_header *header;
> >> + struct mali_c55_params_sensor_off_preshading *sensor_offs;
> >> + struct mali_c55_params_aexp_hist *aexp_hist;
> >> + struct mali_c55_params_aexp_weights *aexp_weights;
> >> + struct mali_c55_params_digital_gain *digital_gain;
> >> + struct mali_c55_params_awb_gains *awb_gains;
> >> + struct mali_c55_params_awb_config *awb_config;
> >> + struct mali_c55_params_mesh_shading_config *shading_config;
> >> + struct mali_c55_params_mesh_shading_selection *shading_selection;
> >> + __u8 *data;
> >> +};
> >> +
> >> +/**
> >> + * define MALI_C55_PARAMS_MAX_SIZE - Maximum size of all Mali C55 Parameters
> >> + *
> >> + * Though the parameters for the Mali-C55 are passed as optional blocks, the
> >> + * driver still needs to know the absolute maximum size so that it can allocate
> >> + * a buffer sized appropriately to accommodate userspace attempting to set all
> >> + * possible parameters in a single frame.
> >> + *
> >> + * Some structs are in this list multiple times. Where that's the case, it just
> >> + * reflects the fact that the same struct can be used with multiple different
> >> + * header types from :c:type:`mali_c55_param_block_type`.
> >> + */
> >> +#define MALI_C55_PARAMS_MAX_SIZE \
> >> + (sizeof(struct mali_c55_params_sensor_off_preshading) + \
> >> + sizeof(struct mali_c55_params_aexp_hist) + \
> >> + sizeof(struct mali_c55_params_aexp_weights) + \
> >> + sizeof(struct mali_c55_params_aexp_hist) + \
> >> + sizeof(struct mali_c55_params_aexp_weights) + \
> >> + sizeof(struct mali_c55_params_digital_gain) + \
> >> + sizeof(struct mali_c55_params_awb_gains) + \
> >> + sizeof(struct mali_c55_params_awb_config) + \
> >> + sizeof(struct mali_c55_params_awb_gains) + \
> >> + sizeof(struct mali_c55_params_mesh_shading_config) + \
> >> + sizeof(struct mali_c55_params_mesh_shading_selection))
> >> +
> >> +/**
> >> + * struct mali_c55_params_buffer - 3A configuration parameters
> >> + *
> >> + * This struct contains the configuration parameters of the Mali-C55 ISP
> >> + * algorithms, serialized by userspace into a data buffer. Each configuration
> >> + * parameter block is represented by a block-specific structure which contains a
> >> + * :c:type:`mali_c55_params_block_header` entry as first member. Userspace
> >> + * populates the @data buffer with configuration parameters for the blocks that
> >> + * it intends to configure. As a consequence, the data buffer effective size
> >> + * changes according to the number of ISP blocks that userspace intends to
> >> + * configure.
> >> + *
> >> + * The parameters buffer is versioned by the @version field to allow modifying
> >> + * and extending its definition. Userspace shall populate the @version field to
> >> + * inform the driver about the version it intends to use. The driver will parse
> >> + * and handle the @data buffer according to the data layout specific to the
> >> + * indicated version and return an error if the desired version is not
> >> + * supported.
> >> + *
> >> + * For each ISP block that userspace wants to configure, a block-specific
> >> + * structure is appended to the @data buffer, one after the other without gaps
> >> + * in between nor overlaps. Userspace shall populate the @total_size field with
> >> + * the effective size, in bytes, of the @data buffer.
> >> + *
> >> + * The expected memory layout of the parameters buffer is::
> >> + *
> >> + * +-------------------- struct mali_c55_params_buffer ------------------+
> >> + * | version = MALI_C55_PARAM_BUFFER_V1; |
> >> + * | total_size = sizeof(struct mali_c55_params_sensor_off_preshading) |
> >> + * | sizeof(struct mali_c55_params_aexp_hist); |
> >> + * | +------------------------- data ---------------------------------+ |
> >> + * | | +--------- struct mali_c55_params_sensor_off_preshading ------+ | |
> >> + * | | | +-------- struct mali_c55_params_block_header header -----+ | | |
> >> + * | | | | type = MALI_C55_PARAM_BLOCK_SENSOR_OFFS; | | | |
> >> + * | | | | enabled = 1; | | | |
> >> + * | | | | size = | | | |
> >> + * | | | | sizeof(struct mali_c55_params_sensor_off_preshading);| | | |
> >> + * | | | +---------------------------------------------------------+ | | |
> >> + * | | | chan00 = ...; | | |
> >> + * | | | chan01 = ...; | | |
> >> + * | | | chan10 = ...; | | |
> >> + * | | | chan11 = ...; | | |
> >> + * | | +------------ struct mali_c55_params_aexp_hist ---------------+ | |
> >> + * | | | +-------- struct mali_c55_params_block_header header -----+ | | |
> >> + * | | | | type = MALI_C55_PARAM_BLOCK_AEXP_HIST; | | | |
> >> + * | | | | enabled = 1; | | | |
> >> + * | | | | size = sizeof(struct mali_c55_params_aexp_hist); | | | |
> >> + * | | | +---------------------------------------------------------+ | | |
> >> + * | | | skip_x = ...; | | |
> >> + * | | | offset_x = ...; | | |
> >> + * | | | skip_y = ...; | | |
> >> + * | | | offset_y = ...; | | |
> >> + * | | | scale_bottom = ...; | | |
> >> + * | | | scale_top = ...; | | |
> >> + * | | | plane_mode = ...; | | |
> >> + * | | | tap_point = ...; | | |
> >> + * | | +-------------------------------------------------------------+ | |
> >> + * | +-----------------------------------------------------------------+ |
> >> + * +---------------------------------------------------------------------+
> >> + *
> >> + * @version: The version from :c:type:`mali_c55_param_buffer_version`
> >> + * @total_size: The Mali-C55 configuration data effective size, excluding this
> >> + * header
> >> + * @data: The Mali-C55 configuration blocks data
> >> + */
> >> +struct mali_c55_params_buffer {
> >> + __u8 version;
> >> + __u32 total_size;
> >> + __u8 data[MALI_C55_PARAMS_MAX_SIZE];
> >> +};
> >> +
> >> #endif /* __UAPI_MALI_C55_CONFIG_H */
--
Regards,
Laurent Pinchart
