* [PATCH 0/2] change fields of uapi struct rkisp1_cif_isp_ctk_config
@ 2020-07-07 17:08 Dafna Hirschfeld
2020-07-07 17:08 ` [PATCH 1/2] media: staging: rkisp1: replace 9 coeff* fields with a 3x3 array Dafna Hirschfeld
2020-07-07 17:08 ` [PATCH 2/2] media: staging: rkisp1: replace 3 fields 'ct_offset_*' with one array Dafna Hirschfeld
0 siblings, 2 replies; 7+ messages in thread
From: Dafna Hirschfeld @ 2020-07-07 17:08 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
dafna3, sakari.ailus, linux-rockchip, mchehab, tfiga
The struct rkisp1_cif_isp_ctk_config represents a 3x3 matrix
and an offset vector for the color correction. It is nicer
to represent those with a 3x3 array and a 3-array fields.
- First patch repalces 9 fields 'coeff*' with one field coeff[3][3]
- Second patch replaces 3 fields 'ct_offset_{rgb}' with one field 'ct_offset[3]'
Dafna Hirschfeld (2):
media: staging: rkisp1:: replace 9 coeff* fields with a 3x3 array
media: staging: rkisp1: replace 3 fields 'ct_offset_*' with one array
drivers/staging/media/rkisp1/rkisp1-params.c | 24 +++++++------------
.../staging/media/rkisp1/uapi/rkisp1-config.h | 20 +++++-----------
2 files changed, 15 insertions(+), 29 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] media: staging: rkisp1: replace 9 coeff* fields with a 3x3 array
2020-07-07 17:08 [PATCH 0/2] change fields of uapi struct rkisp1_cif_isp_ctk_config Dafna Hirschfeld
@ 2020-07-07 17:08 ` Dafna Hirschfeld
2020-07-27 17:50 ` Helen Koike
2020-07-07 17:08 ` [PATCH 2/2] media: staging: rkisp1: replace 3 fields 'ct_offset_*' with one array Dafna Hirschfeld
1 sibling, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2020-07-07 17:08 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
dafna3, sakari.ailus, linux-rockchip, mchehab, tfiga
The struct rkisp1_cif_isp_ctk_config has 9 fields 'coeff*' for the
3x3 color correction matrix. Replace these fields with one 3x3
array coeff[3][3] and document the field.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-params.c | 15 ++++++---------
drivers/staging/media/rkisp1/uapi/rkisp1-config.h | 14 ++++----------
2 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
index 797e79de659c..00b102eb67b6 100644
--- a/drivers/staging/media/rkisp1/rkisp1-params.c
+++ b/drivers/staging/media/rkisp1/rkisp1-params.c
@@ -402,15 +402,12 @@ static void rkisp1_goc_config(struct rkisp1_params *params,
static void rkisp1_ctk_config(struct rkisp1_params *params,
const struct rkisp1_cif_isp_ctk_config *arg)
{
- rkisp1_write(params->rkisp1, arg->coeff0, RKISP1_CIF_ISP_CT_COEFF_0);
- rkisp1_write(params->rkisp1, arg->coeff1, RKISP1_CIF_ISP_CT_COEFF_1);
- rkisp1_write(params->rkisp1, arg->coeff2, RKISP1_CIF_ISP_CT_COEFF_2);
- rkisp1_write(params->rkisp1, arg->coeff3, RKISP1_CIF_ISP_CT_COEFF_3);
- rkisp1_write(params->rkisp1, arg->coeff4, RKISP1_CIF_ISP_CT_COEFF_4);
- rkisp1_write(params->rkisp1, arg->coeff5, RKISP1_CIF_ISP_CT_COEFF_5);
- rkisp1_write(params->rkisp1, arg->coeff6, RKISP1_CIF_ISP_CT_COEFF_6);
- rkisp1_write(params->rkisp1, arg->coeff7, RKISP1_CIF_ISP_CT_COEFF_7);
- rkisp1_write(params->rkisp1, arg->coeff8, RKISP1_CIF_ISP_CT_COEFF_8);
+ unsigned int i, j, k = 0;
+
+ for (i = 0; i < 3; i++)
+ for (j = 0; j < 3; j++)
+ rkisp1_write(params->rkisp1, arg->coeff[i][j],
+ RKISP1_CIF_ISP_CT_COEFF_0 + 4 * k++);
rkisp1_write(params->rkisp1, arg->ct_offset_r,
RKISP1_CIF_ISP_CT_OFFSET_R);
rkisp1_write(params->rkisp1, arg->ct_offset_g,
diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
index ca0d031b14ac..a01711a668da 100644
--- a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
+++ b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
@@ -417,19 +417,13 @@ struct rkisp1_cif_isp_bdm_config {
/**
* struct rkisp1_cif_isp_ctk_config - Configuration used by Cross Talk correction
*
- * @coeff: color correction matrix
+ * @coeff: color correction matrix. Values are 11-bit signed fixed-point numbers with 4 bit integer
+ * and 7 bit fractional part, ranging from -8 (0x400) to +7.992 (0x3FF). 0 is
+ * represented by 0x000 and a coefficient value of 1 as 0x080.
* @ct_offset_b: offset for the crosstalk correction matrix
*/
struct rkisp1_cif_isp_ctk_config {
- __u16 coeff0;
- __u16 coeff1;
- __u16 coeff2;
- __u16 coeff3;
- __u16 coeff4;
- __u16 coeff5;
- __u16 coeff6;
- __u16 coeff7;
- __u16 coeff8;
+ __u16 coeff[3][3];
__u16 ct_offset_r;
__u16 ct_offset_g;
__u16 ct_offset_b;
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] media: staging: rkisp1: replace 3 fields 'ct_offset_*' with one array
2020-07-07 17:08 [PATCH 0/2] change fields of uapi struct rkisp1_cif_isp_ctk_config Dafna Hirschfeld
2020-07-07 17:08 ` [PATCH 1/2] media: staging: rkisp1: replace 9 coeff* fields with a 3x3 array Dafna Hirschfeld
@ 2020-07-07 17:08 ` Dafna Hirschfeld
2020-08-03 19:38 ` Helen Koike
1 sibling, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2020-07-07 17:08 UTC (permalink / raw)
To: linux-media, laurent.pinchart
Cc: dafna.hirschfeld, helen.koike, ezequiel, hverkuil, kernel,
dafna3, sakari.ailus, linux-rockchip, mchehab, tfiga
The struct rkisp1_cif_isp_ctk_config contains 3 fields
ct_offset_{rgb}. Replace them with one array field 'ct_offset[3].
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
drivers/staging/media/rkisp1/rkisp1-params.c | 9 +++------
drivers/staging/media/rkisp1/uapi/rkisp1-config.h | 6 ++----
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
index 00b102eb67b6..8596ad87a860 100644
--- a/drivers/staging/media/rkisp1/rkisp1-params.c
+++ b/drivers/staging/media/rkisp1/rkisp1-params.c
@@ -408,12 +408,9 @@ static void rkisp1_ctk_config(struct rkisp1_params *params,
for (j = 0; j < 3; j++)
rkisp1_write(params->rkisp1, arg->coeff[i][j],
RKISP1_CIF_ISP_CT_COEFF_0 + 4 * k++);
- rkisp1_write(params->rkisp1, arg->ct_offset_r,
- RKISP1_CIF_ISP_CT_OFFSET_R);
- rkisp1_write(params->rkisp1, arg->ct_offset_g,
- RKISP1_CIF_ISP_CT_OFFSET_G);
- rkisp1_write(params->rkisp1, arg->ct_offset_b,
- RKISP1_CIF_ISP_CT_OFFSET_B);
+ for (i = 0; i < 3; i++)
+ rkisp1_write(params->rkisp1, arg->ct_offset[i],
+ RKISP1_CIF_ISP_CT_OFFSET_R + i * 4);
}
static void rkisp1_ctk_enable(struct rkisp1_params *params, bool en)
diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
index a01711a668da..ec624f2579cd 100644
--- a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
+++ b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
@@ -420,13 +420,11 @@ struct rkisp1_cif_isp_bdm_config {
* @coeff: color correction matrix. Values are 11-bit signed fixed-point numbers with 4 bit integer
* and 7 bit fractional part, ranging from -8 (0x400) to +7.992 (0x3FF). 0 is
* represented by 0x000 and a coefficient value of 1 as 0x080.
- * @ct_offset_b: offset for the crosstalk correction matrix
+ * @ct_offset: Red, Green, Blue offsets for the crosstalk correction matrix
*/
struct rkisp1_cif_isp_ctk_config {
__u16 coeff[3][3];
- __u16 ct_offset_r;
- __u16 ct_offset_g;
- __u16 ct_offset_b;
+ __u16 ct_offset[3];
} __packed;
enum rkisp1_cif_isp_goc_mode {
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: staging: rkisp1: replace 9 coeff* fields with a 3x3 array
2020-07-07 17:08 ` [PATCH 1/2] media: staging: rkisp1: replace 9 coeff* fields with a 3x3 array Dafna Hirschfeld
@ 2020-07-27 17:50 ` Helen Koike
2020-07-31 9:33 ` Dafna Hirschfeld
0 siblings, 1 reply; 7+ messages in thread
From: Helen Koike @ 2020-07-27 17:50 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media, laurent.pinchart
Cc: ezequiel, hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip,
mchehab, tfiga
Hi Dafna,
On 7/7/20 2:08 PM, Dafna Hirschfeld wrote:
> The struct rkisp1_cif_isp_ctk_config has 9 fields 'coeff*' for the
> 3x3 color correction matrix. Replace these fields with one 3x3
> array coeff[3][3] and document the field.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> drivers/staging/media/rkisp1/rkisp1-params.c | 15 ++++++---------
> drivers/staging/media/rkisp1/uapi/rkisp1-config.h | 14 ++++----------
> 2 files changed, 10 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
> index 797e79de659c..00b102eb67b6 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-params.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-params.c
> @@ -402,15 +402,12 @@ static void rkisp1_goc_config(struct rkisp1_params *params,
> static void rkisp1_ctk_config(struct rkisp1_params *params,
> const struct rkisp1_cif_isp_ctk_config *arg)
> {
> - rkisp1_write(params->rkisp1, arg->coeff0, RKISP1_CIF_ISP_CT_COEFF_0);
> - rkisp1_write(params->rkisp1, arg->coeff1, RKISP1_CIF_ISP_CT_COEFF_1);
> - rkisp1_write(params->rkisp1, arg->coeff2, RKISP1_CIF_ISP_CT_COEFF_2);
> - rkisp1_write(params->rkisp1, arg->coeff3, RKISP1_CIF_ISP_CT_COEFF_3);
> - rkisp1_write(params->rkisp1, arg->coeff4, RKISP1_CIF_ISP_CT_COEFF_4);
> - rkisp1_write(params->rkisp1, arg->coeff5, RKISP1_CIF_ISP_CT_COEFF_5);
> - rkisp1_write(params->rkisp1, arg->coeff6, RKISP1_CIF_ISP_CT_COEFF_6);
> - rkisp1_write(params->rkisp1, arg->coeff7, RKISP1_CIF_ISP_CT_COEFF_7);
> - rkisp1_write(params->rkisp1, arg->coeff8, RKISP1_CIF_ISP_CT_COEFF_8);
> + unsigned int i, j, k = 0;
> +
> + for (i = 0; i < 3; i++)
> + for (j = 0; j < 3; j++)
> + rkisp1_write(params->rkisp1, arg->coeff[i][j],
> + RKISP1_CIF_ISP_CT_COEFF_0 + 4 * k++);
> rkisp1_write(params->rkisp1, arg->ct_offset_r,
> RKISP1_CIF_ISP_CT_OFFSET_R);
> rkisp1_write(params->rkisp1, arg->ct_offset_g,
> diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
> index ca0d031b14ac..a01711a668da 100644
> --- a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
> +++ b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
> @@ -417,19 +417,13 @@ struct rkisp1_cif_isp_bdm_config {
> /**
> * struct rkisp1_cif_isp_ctk_config - Configuration used by Cross Talk correction
> *
> - * @coeff: color correction matrix
> + * @coeff: color correction matrix. Values are 11-bit signed fixed-point numbers with 4 bit integer
> + * and 7 bit fractional part, ranging from -8 (0x400) to +7.992 (0x3FF). 0 is
> + * represented by 0x000 and a coefficient value of 1 as 0x080.
> * @ct_offset_b: offset for the crosstalk correction matrix
> */
> struct rkisp1_cif_isp_ctk_config {
> - __u16 coeff0;
> - __u16 coeff1;
> - __u16 coeff2;
> - __u16 coeff3;
> - __u16 coeff4;
> - __u16 coeff5;
> - __u16 coeff6;
> - __u16 coeff7;
> - __u16 coeff8;
> + __u16 coeff[3][3];
Why not:
__u16 coeff[9];
?
Thanks,
Helen
> __u16 ct_offset_r;
> __u16 ct_offset_g;
> __u16 ct_offset_b;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: staging: rkisp1: replace 9 coeff* fields with a 3x3 array
2020-07-27 17:50 ` Helen Koike
@ 2020-07-31 9:33 ` Dafna Hirschfeld
2020-08-03 19:38 ` Helen Koike
0 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2020-07-31 9:33 UTC (permalink / raw)
To: Helen Koike, linux-media, laurent.pinchart
Cc: ezequiel, hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip,
mchehab, tfiga
On 27.07.20 19:50, Helen Koike wrote:
> Hi Dafna,
>
> On 7/7/20 2:08 PM, Dafna Hirschfeld wrote:
>> The struct rkisp1_cif_isp_ctk_config has 9 fields 'coeff*' for the
>> 3x3 color correction matrix. Replace these fields with one 3x3
>> array coeff[3][3] and document the field.
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> ---
>> drivers/staging/media/rkisp1/rkisp1-params.c | 15 ++++++---------
>> drivers/staging/media/rkisp1/uapi/rkisp1-config.h | 14 ++++----------
>> 2 files changed, 10 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
>> index 797e79de659c..00b102eb67b6 100644
>> --- a/drivers/staging/media/rkisp1/rkisp1-params.c
>> +++ b/drivers/staging/media/rkisp1/rkisp1-params.c
>> @@ -402,15 +402,12 @@ static void rkisp1_goc_config(struct rkisp1_params *params,
>> static void rkisp1_ctk_config(struct rkisp1_params *params,
>> const struct rkisp1_cif_isp_ctk_config *arg)
>> {
>> - rkisp1_write(params->rkisp1, arg->coeff0, RKISP1_CIF_ISP_CT_COEFF_0);
>> - rkisp1_write(params->rkisp1, arg->coeff1, RKISP1_CIF_ISP_CT_COEFF_1);
>> - rkisp1_write(params->rkisp1, arg->coeff2, RKISP1_CIF_ISP_CT_COEFF_2);
>> - rkisp1_write(params->rkisp1, arg->coeff3, RKISP1_CIF_ISP_CT_COEFF_3);
>> - rkisp1_write(params->rkisp1, arg->coeff4, RKISP1_CIF_ISP_CT_COEFF_4);
>> - rkisp1_write(params->rkisp1, arg->coeff5, RKISP1_CIF_ISP_CT_COEFF_5);
>> - rkisp1_write(params->rkisp1, arg->coeff6, RKISP1_CIF_ISP_CT_COEFF_6);
>> - rkisp1_write(params->rkisp1, arg->coeff7, RKISP1_CIF_ISP_CT_COEFF_7);
>> - rkisp1_write(params->rkisp1, arg->coeff8, RKISP1_CIF_ISP_CT_COEFF_8);
>> + unsigned int i, j, k = 0;
>> +
>> + for (i = 0; i < 3; i++)
>> + for (j = 0; j < 3; j++)
>> + rkisp1_write(params->rkisp1, arg->coeff[i][j],
>> + RKISP1_CIF_ISP_CT_COEFF_0 + 4 * k++);
>> rkisp1_write(params->rkisp1, arg->ct_offset_r,
>> RKISP1_CIF_ISP_CT_OFFSET_R);
>> rkisp1_write(params->rkisp1, arg->ct_offset_g,
>> diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
>> index ca0d031b14ac..a01711a668da 100644
>> --- a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
>> +++ b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
>> @@ -417,19 +417,13 @@ struct rkisp1_cif_isp_bdm_config {
>> /**
>> * struct rkisp1_cif_isp_ctk_config - Configuration used by Cross Talk correction
>> *
>> - * @coeff: color correction matrix
>> + * @coeff: color correction matrix. Values are 11-bit signed fixed-point numbers with 4 bit integer
>> + * and 7 bit fractional part, ranging from -8 (0x400) to +7.992 (0x3FF). 0 is
>> + * represented by 0x000 and a coefficient value of 1 as 0x080.
>> * @ct_offset_b: offset for the crosstalk correction matrix
>> */
>> struct rkisp1_cif_isp_ctk_config {
>> - __u16 coeff0;
>> - __u16 coeff1;
>> - __u16 coeff2;
>> - __u16 coeff3;
>> - __u16 coeff4;
>> - __u16 coeff5;
>> - __u16 coeff6;
>> - __u16 coeff7;
>> - __u16 coeff8;
>> + __u16 coeff[3][3];
>
> Why not:
>
> __u16 coeff[9];
Hi, since it represents a 3x3 matrix I think it is nicer as a 3x3 array.
Thanks,
Dafna
>
> ?
>
> Thanks,
> Helen
>
>> __u16 ct_offset_r;
>> __u16 ct_offset_g;
>> __u16 ct_offset_b;
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: staging: rkisp1: replace 9 coeff* fields with a 3x3 array
2020-07-31 9:33 ` Dafna Hirschfeld
@ 2020-08-03 19:38 ` Helen Koike
0 siblings, 0 replies; 7+ messages in thread
From: Helen Koike @ 2020-08-03 19:38 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media, laurent.pinchart
Cc: ezequiel, hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip,
mchehab, tfiga
On 7/31/20 6:33 AM, Dafna Hirschfeld wrote:
>
>
> On 27.07.20 19:50, Helen Koike wrote:
>> Hi Dafna,
>>
>> On 7/7/20 2:08 PM, Dafna Hirschfeld wrote:
>>> The struct rkisp1_cif_isp_ctk_config has 9 fields 'coeff*' for the
>>> 3x3 color correction matrix. Replace these fields with one 3x3
>>> array coeff[3][3] and document the field.
>>>
>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>> ---
>>> drivers/staging/media/rkisp1/rkisp1-params.c | 15 ++++++---------
>>> drivers/staging/media/rkisp1/uapi/rkisp1-config.h | 14 ++++----------
>>> 2 files changed, 10 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
>>> index 797e79de659c..00b102eb67b6 100644
>>> --- a/drivers/staging/media/rkisp1/rkisp1-params.c
>>> +++ b/drivers/staging/media/rkisp1/rkisp1-params.c
>>> @@ -402,15 +402,12 @@ static void rkisp1_goc_config(struct rkisp1_params *params,
>>> static void rkisp1_ctk_config(struct rkisp1_params *params,
>>> const struct rkisp1_cif_isp_ctk_config *arg)
>>> {
>>> - rkisp1_write(params->rkisp1, arg->coeff0, RKISP1_CIF_ISP_CT_COEFF_0);
>>> - rkisp1_write(params->rkisp1, arg->coeff1, RKISP1_CIF_ISP_CT_COEFF_1);
>>> - rkisp1_write(params->rkisp1, arg->coeff2, RKISP1_CIF_ISP_CT_COEFF_2);
>>> - rkisp1_write(params->rkisp1, arg->coeff3, RKISP1_CIF_ISP_CT_COEFF_3);
>>> - rkisp1_write(params->rkisp1, arg->coeff4, RKISP1_CIF_ISP_CT_COEFF_4);
>>> - rkisp1_write(params->rkisp1, arg->coeff5, RKISP1_CIF_ISP_CT_COEFF_5);
>>> - rkisp1_write(params->rkisp1, arg->coeff6, RKISP1_CIF_ISP_CT_COEFF_6);
>>> - rkisp1_write(params->rkisp1, arg->coeff7, RKISP1_CIF_ISP_CT_COEFF_7);
>>> - rkisp1_write(params->rkisp1, arg->coeff8, RKISP1_CIF_ISP_CT_COEFF_8);
>>> + unsigned int i, j, k = 0;
>>> +
>>> + for (i = 0; i < 3; i++)
>>> + for (j = 0; j < 3; j++)
>>> + rkisp1_write(params->rkisp1, arg->coeff[i][j],
>>> + RKISP1_CIF_ISP_CT_COEFF_0 + 4 * k++);
>>> rkisp1_write(params->rkisp1, arg->ct_offset_r,
>>> RKISP1_CIF_ISP_CT_OFFSET_R);
>>> rkisp1_write(params->rkisp1, arg->ct_offset_g,
>>> diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
>>> index ca0d031b14ac..a01711a668da 100644
>>> --- a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
>>> +++ b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
>>> @@ -417,19 +417,13 @@ struct rkisp1_cif_isp_bdm_config {
>>> /**
>>> * struct rkisp1_cif_isp_ctk_config - Configuration used by Cross Talk correction
>>> *
>>> - * @coeff: color correction matrix
>>> + * @coeff: color correction matrix. Values are 11-bit signed fixed-point numbers with 4 bit integer
>>> + * and 7 bit fractional part, ranging from -8 (0x400) to +7.992 (0x3FF). 0 is
>>> + * represented by 0x000 and a coefficient value of 1 as 0x080.
>>> * @ct_offset_b: offset for the crosstalk correction matrix
>>> */
>>> struct rkisp1_cif_isp_ctk_config {
>>> - __u16 coeff0;
>>> - __u16 coeff1;
>>> - __u16 coeff2;
>>> - __u16 coeff3;
>>> - __u16 coeff4;
>>> - __u16 coeff5;
>>> - __u16 coeff6;
>>> - __u16 coeff7;
>>> - __u16 coeff8;
>>> + __u16 coeff[3][3];
>>
>> Why not:
>>
>> __u16 coeff[9];
>
> Hi, since it represents a 3x3 matrix I think it is nicer as a 3x3 array.
Fair enough, let's make this clear in the uAPI then.
Acked-by: Helen Koike <helen.koike@collabora.com>
Regards,
Helen
>
> Thanks,
> Dafna
>
>>
>> ?
>>
>> Thanks,
>> Helen
>>
>>> __u16 ct_offset_r;
>>> __u16 ct_offset_g;
>>> __u16 ct_offset_b;
>>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: staging: rkisp1: replace 3 fields 'ct_offset_*' with one array
2020-07-07 17:08 ` [PATCH 2/2] media: staging: rkisp1: replace 3 fields 'ct_offset_*' with one array Dafna Hirschfeld
@ 2020-08-03 19:38 ` Helen Koike
0 siblings, 0 replies; 7+ messages in thread
From: Helen Koike @ 2020-08-03 19:38 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media, laurent.pinchart
Cc: ezequiel, hverkuil, kernel, dafna3, sakari.ailus, linux-rockchip,
mchehab, tfiga
On 7/7/20 2:08 PM, Dafna Hirschfeld wrote:
> The struct rkisp1_cif_isp_ctk_config contains 3 fields
> ct_offset_{rgb}. Replace them with one array field 'ct_offset[3].
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
Thanks
Helen
> ---
> drivers/staging/media/rkisp1/rkisp1-params.c | 9 +++------
> drivers/staging/media/rkisp1/uapi/rkisp1-config.h | 6 ++----
> 2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
> index 00b102eb67b6..8596ad87a860 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-params.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-params.c
> @@ -408,12 +408,9 @@ static void rkisp1_ctk_config(struct rkisp1_params *params,
> for (j = 0; j < 3; j++)
> rkisp1_write(params->rkisp1, arg->coeff[i][j],
> RKISP1_CIF_ISP_CT_COEFF_0 + 4 * k++);
> - rkisp1_write(params->rkisp1, arg->ct_offset_r,
> - RKISP1_CIF_ISP_CT_OFFSET_R);
> - rkisp1_write(params->rkisp1, arg->ct_offset_g,
> - RKISP1_CIF_ISP_CT_OFFSET_G);
> - rkisp1_write(params->rkisp1, arg->ct_offset_b,
> - RKISP1_CIF_ISP_CT_OFFSET_B);
> + for (i = 0; i < 3; i++)
> + rkisp1_write(params->rkisp1, arg->ct_offset[i],
> + RKISP1_CIF_ISP_CT_OFFSET_R + i * 4);
> }
>
> static void rkisp1_ctk_enable(struct rkisp1_params *params, bool en)
> diff --git a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
> index a01711a668da..ec624f2579cd 100644
> --- a/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
> +++ b/drivers/staging/media/rkisp1/uapi/rkisp1-config.h
> @@ -420,13 +420,11 @@ struct rkisp1_cif_isp_bdm_config {
> * @coeff: color correction matrix. Values are 11-bit signed fixed-point numbers with 4 bit integer
> * and 7 bit fractional part, ranging from -8 (0x400) to +7.992 (0x3FF). 0 is
> * represented by 0x000 and a coefficient value of 1 as 0x080.
> - * @ct_offset_b: offset for the crosstalk correction matrix
> + * @ct_offset: Red, Green, Blue offsets for the crosstalk correction matrix
> */
> struct rkisp1_cif_isp_ctk_config {
> __u16 coeff[3][3];
> - __u16 ct_offset_r;
> - __u16 ct_offset_g;
> - __u16 ct_offset_b;
> + __u16 ct_offset[3];
> } __packed;
>
> enum rkisp1_cif_isp_goc_mode {
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-08-03 19:39 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-07 17:08 [PATCH 0/2] change fields of uapi struct rkisp1_cif_isp_ctk_config Dafna Hirschfeld
2020-07-07 17:08 ` [PATCH 1/2] media: staging: rkisp1: replace 9 coeff* fields with a 3x3 array Dafna Hirschfeld
2020-07-27 17:50 ` Helen Koike
2020-07-31 9:33 ` Dafna Hirschfeld
2020-08-03 19:38 ` Helen Koike
2020-07-07 17:08 ` [PATCH 2/2] media: staging: rkisp1: replace 3 fields 'ct_offset_*' with one array Dafna Hirschfeld
2020-08-03 19:38 ` Helen Koike
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).