Am 19.01.21 um 17:32 schrieb Hans Verkuil:
On 19/01/2021 16:32, Dafna Hirschfeld wrote:
Am 19.01.21 um 16:00 schrieb 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@xxxxxxxxxxxxx>
---
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!
Actually I don't know, I just copied the docs in the datasheet.
I can try figure it out. I can meanwhile send a patch without the doc until
we are sure. Is that OK?
No, this should be documented properly.
I see that indeed bits 0-3 are the fractional part.
The measurements are taken over 25 sub-windows and it is possible to
give a weight to each window. If I set all weights to 1 then I expect to get
an integer value and indeed in that case I see that the last 4 bits are always 0.
If I set the weights to other than 1 I get the last 4 bits not only 0.
The UAPI is generally not very good documented mainly because there is currently
no open source userspace that uses it.
Is this the only fractional type that is used in the rkisp1 uapi or are there
others? (Just checking).
There are other places as well.
I see that there are some values in rkisp1_cif_isp_lsc_config that are fractional
and not documented.
The two other fractional types in the uapi are already documented.
I can add a patch that extends the documentation of all the fields in the uapi using the datasheet.
But I think such a patch can be added after de-staging.
Thanks,
Dafna
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?
This seems to be a bug. I see that this cast is introduced in v12 of the patch
"media: staging: rkisp1: add capture device for statistics".
This cast does not exist in any of the downstream versions.
Clearly something that needs to be fixed as well.
Regards,
Hans
Thanks,
Dafna
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];
};
/**
