Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
* [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	[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	[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, back to index

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

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git