[PATCH] media: rkisp1: uapi: change hist_bins array type from __u16 to __u32

[Hans Verkuil]

On 19/01/2021 15:53, Dafna Hirschfeld wrote:
> Each entry in the array is a 20 bits value composed of 16
> bits unsigned integer and 4 bits fractional part. So the
> type should change to __u32.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>
> ---
> This patch is applied on top of v6 of the patchset
> "Fix the rkisp1 userspace API for later IP versions"
> 
>  include/uapi/linux/rkisp1-config.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/rkisp1-config.h b/include/uapi/linux/rkisp1-config.h
> index 57ca3eea985f..47f6b84d7c56 100644
> --- a/include/uapi/linux/rkisp1-config.h
> +++ b/include/uapi/linux/rkisp1-config.h
> @@ -895,7 +895,8 @@ struct rkisp1_cif_isp_af_stat {
>  /**
>   * struct rkisp1_cif_isp_hist_stat - statistics histogram data
>   *
> - * @hist_bins: measured bin counters
> + * @hist_bins: measured bin counters. Each bin is a 20 bits value
> + *	       composed of a 16-bit unsigned integer and 4 bits fractional part.

So bits 0-3 are the fractional part and bits 4-19 contain the integer part?
What goes where should be defined!

Looking at rkisp1_stats_get_hst_meas() I see this:

        for (i = 0; i < RKISP1_CIF_ISP_HIST_BIN_N_MAX; i++)
                pbuf->params.hist.hist_bins[i] =
                        (u8)rkisp1_read(rkisp1,
                                        RKISP1_CIF_ISP_HIST_BIN_0 + i * 4);

Here this is cast to a u8, so how does this work?

There is something fishy here...

Regards,

	Hans

>   *
>   * The histogram values divided into 16 bins for V10/V11 and 32 bins
>   * for V12/V13. It is configured within the struct rkisp1_cif_isp_hst_config.
> @@ -909,7 +910,7 @@ struct rkisp1_cif_isp_af_stat {
>   * RKISP1_CIF_ISP_HIST_BIN_N_MAX is equal to the maximum of the two.
>   */
>  struct rkisp1_cif_isp_hist_stat {
> -	__u16 hist_bins[RKISP1_CIF_ISP_HIST_BIN_N_MAX];
> +	__u32 hist_bins[RKISP1_CIF_ISP_HIST_BIN_N_MAX];
>  };
>  
>  /**
>