diff mbox

[v3,4/6,media] exynos-gsc: Add hardware rotation limits

Message ID 1504850560-27950-5-git-send-email-hoegeun.kwon@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hoegeun Kwon Sept. 8, 2017, 6:02 a.m. UTC
The hardware rotation limits of gsc depends on SOC (Exynos
5250/5420/5433). Distinguish them and add them to the driver data.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 96 ++++++++++++++++++++++++----
 1 file changed, 83 insertions(+), 13 deletions(-)

Comments

On 09/08/2017 08:02 AM, Hoegeun Kwon wrote:
> The hardware rotation limits of gsc depends on SOC (Exynos
> 5250/5420/5433). Distinguish them and add them to the driver data.
> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
>   drivers/media/platform/exynos-gsc/gsc-core.c | 96 ++++++++++++++++++++++++----
>   1 file changed, 83 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
> index 4380150..8f8636e 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
> @@ -943,7 +943,37 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
>   	return IRQ_HANDLED;
>   }
>   
> -static struct gsc_pix_max gsc_v_100_max = {
> +static struct gsc_pix_max gsc_v_5250_max = {
> +	.org_scaler_bypass_w	= 8192,
> +	.org_scaler_bypass_h	= 8192,
> +	.org_scaler_input_w	= 4800,
> +	.org_scaler_input_h	= 3344,
> +	.real_rot_dis_w		= 4800,
> +	.real_rot_dis_h		= 3344,
> +	.real_rot_en_w		= 2016,
> +	.real_rot_en_h		= 2016,
> +	.target_rot_dis_w	= 4800,
> +	.target_rot_dis_h	= 3344,
> +	.target_rot_en_w	= 2016,
> +	.target_rot_en_h	= 2016,
> +};
> +
> +static struct gsc_pix_max gsc_v_5420_max = {
> +	.org_scaler_bypass_w	= 8192,
> +	.org_scaler_bypass_h	= 8192,
> +	.org_scaler_input_w	= 4800,
> +	.org_scaler_input_h	= 3344,
> +	.real_rot_dis_w		= 4800,
> +	.real_rot_dis_h		= 3344,
> +	.real_rot_en_w		= 2048,
> +	.real_rot_en_h		= 2048,
> +	.target_rot_dis_w	= 4800,
> +	.target_rot_dis_h	= 3344,
> +	.target_rot_en_w	= 2016,
> +	.target_rot_en_h	= 2016,
> +};
> +
> +static struct gsc_pix_max gsc_v_5433_max = {
>   	.org_scaler_bypass_w	= 8192,
>   	.org_scaler_bypass_h	= 8192,
>   	.org_scaler_input_w	= 4800,
> @@ -979,8 +1009,8 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
>   	.target_h		= 2,  /* yuv420 : 2, others : 1 */
>   };
>   
> -static struct gsc_variant gsc_v_100_variant = {
> -	.pix_max		= &gsc_v_100_max,
> +static struct gsc_variant gsc_v_5250_variant = {
> +	.pix_max		= &gsc_v_5250_max,
>   	.pix_min		= &gsc_v_100_min,
>   	.pix_align		= &gsc_v_100_align,
>   	.in_buf_cnt		= 32,
> @@ -992,12 +1022,48 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
>   	.local_sc_down		= 2,
>   };
>   
> -static struct gsc_driverdata gsc_v_100_drvdata = {
> +static struct gsc_variant gsc_v_5420_variant = {
> +	.pix_max		= &gsc_v_5420_max,
> +	.pix_min		= &gsc_v_100_min,
> +	.pix_align		= &gsc_v_100_align,
> +	.in_buf_cnt		= 32,
> +	.out_buf_cnt		= 32,
> +	.sc_up_max		= 8,
> +	.sc_down_max		= 16,
> +	.poly_sc_down_max	= 4,
> +	.pre_sc_down_max	= 4,
> +	.local_sc_down		= 2,
> +};
> +
> +static struct gsc_variant gsc_v_5433_variant = {
> +	.pix_max		= &gsc_v_5433_max,
> +	.pix_min		= &gsc_v_100_min,
> +	.pix_align		= &gsc_v_100_align,
> +	.in_buf_cnt		= 32,
> +	.out_buf_cnt		= 32,
> +	.sc_up_max		= 8,
> +	.sc_down_max		= 16,
> +	.poly_sc_down_max	= 4,
> +	.pre_sc_down_max	= 4,
> +	.local_sc_down		= 2,
> +};
> +
> +static struct gsc_driverdata gsc_v_5250_drvdata = {
>   	.variant = {
> -		[0] = &gsc_v_100_variant,
> -		[1] = &gsc_v_100_variant,
> -		[2] = &gsc_v_100_variant,
> -		[3] = &gsc_v_100_variant,
> +		[0] = &gsc_v_5250_variant,
> +		[1] = &gsc_v_5250_variant,
> +		[2] = &gsc_v_5250_variant,
> +		[3] = &gsc_v_5250_variant,
> +	},
> +	.num_entities = 4,
> +	.clk_names = { "gscl" },
> +	.num_clocks = 1,
> +};
> +
> +static struct gsc_driverdata gsc_v_5420_drvdata = {
> +	.variant = {
> +		[0] = &gsc_v_5420_variant,
> +		[1] = &gsc_v_5420_variant,
>   	},
>   	.num_entities = 4,
>   	.clk_names = { "gscl" },
> @@ -1006,9 +1072,9 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
>   
>   static struct gsc_driverdata gsc_5433_drvdata = {
>   	.variant = {
> -		[0] = &gsc_v_100_variant,
> -		[1] = &gsc_v_100_variant,
> -		[2] = &gsc_v_100_variant,
> +		[0] = &gsc_v_5433_variant,
> +		[1] = &gsc_v_5433_variant,
> +		[2] = &gsc_v_5433_variant,
>   	},
>   	.num_entities = 3,
>   	.clk_names = { "pclk", "aclk", "aclk_xiu", "aclk_gsclbend" },
> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv)
>   
>   static const struct of_device_id exynos_gsc_match[] = {
>   	{
> -		.compatible = "samsung,exynos5-gsc",
> -		.data = &gsc_v_100_drvdata,

Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant
data, so that it can work with "samsung,exynos5-gsc" compatible in DT
on both exynos5250 and exynos5420 SoCs?

> +		.compatible = "samsung,exynos5250-gsc",
> +		.data = &gsc_v_5250_drvdata,
> +	},
> +	{
> +		.compatible = "samsung,exynos5420-gsc",
> +		.data = &gsc_v_5420_drvdata,
>   	},
Hoegeun Kwon Sept. 13, 2017, 2:33 a.m. UTC | #2
Hi Sylwester,

On 09/11/2017 06:35 PM, Sylwester Nawrocki wrote:
> On 09/08/2017 08:02 AM, Hoegeun Kwon wrote:
>> The hardware rotation limits of gsc depends on SOC (Exynos
>> 5250/5420/5433). Distinguish them and add them to the driver data.
>>
>> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
>> ---
>>   drivers/media/platform/exynos-gsc/gsc-core.c | 96 
>> ++++++++++++++++++++++++----
>>   1 file changed, 83 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
>> b/drivers/media/platform/exynos-gsc/gsc-core.c
>> index 4380150..8f8636e 100644
>> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
>> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
>> @@ -943,7 +943,37 @@ static irqreturn_t gsc_irq_handler(int irq, void 
>> *priv)
>>       return IRQ_HANDLED;
>>   }
>>   -static struct gsc_pix_max gsc_v_100_max = {
>> +static struct gsc_pix_max gsc_v_5250_max = {
>> +    .org_scaler_bypass_w    = 8192,
>> +    .org_scaler_bypass_h    = 8192,
>> +    .org_scaler_input_w    = 4800,
>> +    .org_scaler_input_h    = 3344,
>> +    .real_rot_dis_w        = 4800,
>> +    .real_rot_dis_h        = 3344,
>> +    .real_rot_en_w        = 2016,
>> +    .real_rot_en_h        = 2016,
>> +    .target_rot_dis_w    = 4800,
>> +    .target_rot_dis_h    = 3344,
>> +    .target_rot_en_w    = 2016,
>> +    .target_rot_en_h    = 2016,
>> +};
>> +
>> +static struct gsc_pix_max gsc_v_5420_max = {
>> +    .org_scaler_bypass_w    = 8192,
>> +    .org_scaler_bypass_h    = 8192,
>> +    .org_scaler_input_w    = 4800,
>> +    .org_scaler_input_h    = 3344,
>> +    .real_rot_dis_w        = 4800,
>> +    .real_rot_dis_h        = 3344,
>> +    .real_rot_en_w        = 2048,
>> +    .real_rot_en_h        = 2048,
>> +    .target_rot_dis_w    = 4800,
>> +    .target_rot_dis_h    = 3344,
>> +    .target_rot_en_w    = 2016,
>> +    .target_rot_en_h    = 2016,
>> +};
>> +
>> +static struct gsc_pix_max gsc_v_5433_max = {
>>       .org_scaler_bypass_w    = 8192,
>>       .org_scaler_bypass_h    = 8192,
>>       .org_scaler_input_w    = 4800,
>> @@ -979,8 +1009,8 @@ static irqreturn_t gsc_irq_handler(int irq, void 
>> *priv)
>>       .target_h        = 2,  /* yuv420 : 2, others : 1 */
>>   };
>>   -static struct gsc_variant gsc_v_100_variant = {
>> -    .pix_max        = &gsc_v_100_max,
>> +static struct gsc_variant gsc_v_5250_variant = {
>> +    .pix_max        = &gsc_v_5250_max,
>>       .pix_min        = &gsc_v_100_min,
>>       .pix_align        = &gsc_v_100_align,
>>       .in_buf_cnt        = 32,
>> @@ -992,12 +1022,48 @@ static irqreturn_t gsc_irq_handler(int irq, 
>> void *priv)
>>       .local_sc_down        = 2,
>>   };
>>   -static struct gsc_driverdata gsc_v_100_drvdata = {
>> +static struct gsc_variant gsc_v_5420_variant = {
>> +    .pix_max        = &gsc_v_5420_max,
>> +    .pix_min        = &gsc_v_100_min,
>> +    .pix_align        = &gsc_v_100_align,
>> +    .in_buf_cnt        = 32,
>> +    .out_buf_cnt        = 32,
>> +    .sc_up_max        = 8,
>> +    .sc_down_max        = 16,
>> +    .poly_sc_down_max    = 4,
>> +    .pre_sc_down_max    = 4,
>> +    .local_sc_down        = 2,
>> +};
>> +
>> +static struct gsc_variant gsc_v_5433_variant = {
>> +    .pix_max        = &gsc_v_5433_max,
>> +    .pix_min        = &gsc_v_100_min,
>> +    .pix_align        = &gsc_v_100_align,
>> +    .in_buf_cnt        = 32,
>> +    .out_buf_cnt        = 32,
>> +    .sc_up_max        = 8,
>> +    .sc_down_max        = 16,
>> +    .poly_sc_down_max    = 4,
>> +    .pre_sc_down_max    = 4,
>> +    .local_sc_down        = 2,
>> +};
>> +
>> +static struct gsc_driverdata gsc_v_5250_drvdata = {
>>       .variant = {
>> -        [0] = &gsc_v_100_variant,
>> -        [1] = &gsc_v_100_variant,
>> -        [2] = &gsc_v_100_variant,
>> -        [3] = &gsc_v_100_variant,
>> +        [0] = &gsc_v_5250_variant,
>> +        [1] = &gsc_v_5250_variant,
>> +        [2] = &gsc_v_5250_variant,
>> +        [3] = &gsc_v_5250_variant,
>> +    },
>> +    .num_entities = 4,
>> +    .clk_names = { "gscl" },
>> +    .num_clocks = 1,
>> +};
>> +
>> +static struct gsc_driverdata gsc_v_5420_drvdata = {
>> +    .variant = {
>> +        [0] = &gsc_v_5420_variant,
>> +        [1] = &gsc_v_5420_variant,
>>       },
>>       .num_entities = 4,
>>       .clk_names = { "gscl" },
>> @@ -1006,9 +1072,9 @@ static irqreturn_t gsc_irq_handler(int irq, 
>> void *priv)
>>     static struct gsc_driverdata gsc_5433_drvdata = {
>>       .variant = {
>> -        [0] = &gsc_v_100_variant,
>> -        [1] = &gsc_v_100_variant,
>> -        [2] = &gsc_v_100_variant,
>> +        [0] = &gsc_v_5433_variant,
>> +        [1] = &gsc_v_5433_variant,
>> +        [2] = &gsc_v_5433_variant,
>>       },
>>       .num_entities = 3,
>>       .clk_names = { "pclk", "aclk", "aclk_xiu", "aclk_gsclbend" },
>> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq, 
>> void *priv)
>>     static const struct of_device_id exynos_gsc_match[] = {
>>       {
>> -        .compatible = "samsung,exynos5-gsc",
>> -        .data = &gsc_v_100_drvdata,
>
> Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant
> data, so that it can work with "samsung,exynos5-gsc" compatible in DT
> on both exynos5250 and exynos5420 SoCs?
>

Thank you for your question.

Exynos 5250 and 5420 have different hardware rotation limits.
Exynos 5250 is '.real_rot_en_w/h = 2016' and 5420 is '.real_rot_en_w/h = 
2048'.

So my opinion they must have different compatible.

Best regards,
Hoegeun

>> +        .compatible = "samsung,exynos5250-gsc",
>> +        .data = &gsc_v_5250_drvdata,
>> +    },
>> +    {
>> +        .compatible = "samsung,exynos5420-gsc",
>> +        .data = &gsc_v_5420_drvdata,
>>       },
>
Hi Hoegeun,

On 09/13/2017 04:33 AM, Hoegeun Kwon wrote:
>>> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq,
>>> void *priv)
>>>      static const struct of_device_id exynos_gsc_match[] = {
>>>        {
>>> -        .compatible = "samsung,exynos5-gsc",
>>> -        .data = &gsc_v_100_drvdata,
>> Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant
>> data, so that it can work with "samsung,exynos5-gsc" compatible in DT
>> on both exynos5250 and exynos5420 SoCs?
>>
> Thank you for your question.
> 
> Exynos 5250 and 5420 have different hardware rotation limits.
> Exynos 5250 is '.real_rot_en_w/h = 2016' and 5420 is '.real_rot_en_w/h =
> 2048'.
> 
> So my opinion they must have different compatible.

I think there is some misunderstanding, mu suggestion was to keep the 
"samsung,exynos5-gsc" compatible entry in addition to the new introduced 
ones: "samsung,exynos5250-gsc" and "samsung,exynos5420-gsc". That's in
order to make your changes possibly backward compatible, in theory 
the updated driver should still work without changes in dts.
Hoegeun Kwon Sept. 13, 2017, 9:28 a.m. UTC | #4
On 09/13/2017 06:11 PM, Sylwester Nawrocki wrote:
> Hi Hoegeun,
>
> On 09/13/2017 04:33 AM, Hoegeun Kwon wrote:
>>>> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq,
>>>> void *priv)
>>>>       static const struct of_device_id exynos_gsc_match[] = {
>>>>         {
>>>> -        .compatible = "samsung,exynos5-gsc",
>>>> -        .data = &gsc_v_100_drvdata,
>>> Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant
>>> data, so that it can work with "samsung,exynos5-gsc" compatible in DT
>>> on both exynos5250 and exynos5420 SoCs?
>>>
>> Thank you for your question.
>>
>> Exynos 5250 and 5420 have different hardware rotation limits.
>> Exynos 5250 is '.real_rot_en_w/h = 2016' and 5420 is '.real_rot_en_w/h =
>> 2048'.
>>
>> So my opinion they must have different compatible.
> I think there is some misunderstanding, mu suggestion was to keep the
> "samsung,exynos5-gsc" compatible entry in addition to the new introduced
> ones: "samsung,exynos5250-gsc" and "samsung,exynos5420-gsc". That's in
> order to make your changes possibly backward compatible, in theory
> the updated driver should still work without changes in dts.


Thank you again for your explanation.

Yes, I understood.
I will keep "samsung,exynos5-gsc" compatible,
and add Exynos 5250/5420/5433 compatible.

Best regards,
Hoegeun
diff mbox

Patch

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
index 4380150..8f8636e 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -943,7 +943,37 @@  static irqreturn_t gsc_irq_handler(int irq, void *priv)
 	return IRQ_HANDLED;
 }
 
-static struct gsc_pix_max gsc_v_100_max = {
+static struct gsc_pix_max gsc_v_5250_max = {
+	.org_scaler_bypass_w	= 8192,
+	.org_scaler_bypass_h	= 8192,
+	.org_scaler_input_w	= 4800,
+	.org_scaler_input_h	= 3344,
+	.real_rot_dis_w		= 4800,
+	.real_rot_dis_h		= 3344,
+	.real_rot_en_w		= 2016,
+	.real_rot_en_h		= 2016,
+	.target_rot_dis_w	= 4800,
+	.target_rot_dis_h	= 3344,
+	.target_rot_en_w	= 2016,
+	.target_rot_en_h	= 2016,
+};
+
+static struct gsc_pix_max gsc_v_5420_max = {
+	.org_scaler_bypass_w	= 8192,
+	.org_scaler_bypass_h	= 8192,
+	.org_scaler_input_w	= 4800,
+	.org_scaler_input_h	= 3344,
+	.real_rot_dis_w		= 4800,
+	.real_rot_dis_h		= 3344,
+	.real_rot_en_w		= 2048,
+	.real_rot_en_h		= 2048,
+	.target_rot_dis_w	= 4800,
+	.target_rot_dis_h	= 3344,
+	.target_rot_en_w	= 2016,
+	.target_rot_en_h	= 2016,
+};
+
+static struct gsc_pix_max gsc_v_5433_max = {
 	.org_scaler_bypass_w	= 8192,
 	.org_scaler_bypass_h	= 8192,
 	.org_scaler_input_w	= 4800,
@@ -979,8 +1009,8 @@  static irqreturn_t gsc_irq_handler(int irq, void *priv)
 	.target_h		= 2,  /* yuv420 : 2, others : 1 */
 };
 
-static struct gsc_variant gsc_v_100_variant = {
-	.pix_max		= &gsc_v_100_max,
+static struct gsc_variant gsc_v_5250_variant = {
+	.pix_max		= &gsc_v_5250_max,
 	.pix_min		= &gsc_v_100_min,
 	.pix_align		= &gsc_v_100_align,
 	.in_buf_cnt		= 32,
@@ -992,12 +1022,48 @@  static irqreturn_t gsc_irq_handler(int irq, void *priv)
 	.local_sc_down		= 2,
 };
 
-static struct gsc_driverdata gsc_v_100_drvdata = {
+static struct gsc_variant gsc_v_5420_variant = {
+	.pix_max		= &gsc_v_5420_max,
+	.pix_min		= &gsc_v_100_min,
+	.pix_align		= &gsc_v_100_align,
+	.in_buf_cnt		= 32,
+	.out_buf_cnt		= 32,
+	.sc_up_max		= 8,
+	.sc_down_max		= 16,
+	.poly_sc_down_max	= 4,
+	.pre_sc_down_max	= 4,
+	.local_sc_down		= 2,
+};
+
+static struct gsc_variant gsc_v_5433_variant = {
+	.pix_max		= &gsc_v_5433_max,
+	.pix_min		= &gsc_v_100_min,
+	.pix_align		= &gsc_v_100_align,
+	.in_buf_cnt		= 32,
+	.out_buf_cnt		= 32,
+	.sc_up_max		= 8,
+	.sc_down_max		= 16,
+	.poly_sc_down_max	= 4,
+	.pre_sc_down_max	= 4,
+	.local_sc_down		= 2,
+};
+
+static struct gsc_driverdata gsc_v_5250_drvdata = {
 	.variant = {
-		[0] = &gsc_v_100_variant,
-		[1] = &gsc_v_100_variant,
-		[2] = &gsc_v_100_variant,
-		[3] = &gsc_v_100_variant,
+		[0] = &gsc_v_5250_variant,
+		[1] = &gsc_v_5250_variant,
+		[2] = &gsc_v_5250_variant,
+		[3] = &gsc_v_5250_variant,
+	},
+	.num_entities = 4,
+	.clk_names = { "gscl" },
+	.num_clocks = 1,
+};
+
+static struct gsc_driverdata gsc_v_5420_drvdata = {
+	.variant = {
+		[0] = &gsc_v_5420_variant,
+		[1] = &gsc_v_5420_variant,
 	},
 	.num_entities = 4,
 	.clk_names = { "gscl" },
@@ -1006,9 +1072,9 @@  static irqreturn_t gsc_irq_handler(int irq, void *priv)
 
 static struct gsc_driverdata gsc_5433_drvdata = {
 	.variant = {
-		[0] = &gsc_v_100_variant,
-		[1] = &gsc_v_100_variant,
-		[2] = &gsc_v_100_variant,
+		[0] = &gsc_v_5433_variant,
+		[1] = &gsc_v_5433_variant,
+		[2] = &gsc_v_5433_variant,
 	},
 	.num_entities = 3,
 	.clk_names = { "pclk", "aclk", "aclk_xiu", "aclk_gsclbend" },
@@ -1017,8 +1083,12 @@  static irqreturn_t gsc_irq_handler(int irq, void *priv)
 
 static const struct of_device_id exynos_gsc_match[] = {
 	{
-		.compatible = "samsung,exynos5-gsc",
-		.data = &gsc_v_100_drvdata,
+		.compatible = "samsung,exynos5250-gsc",
+		.data = &gsc_v_5250_drvdata,
+	},
+	{
+		.compatible = "samsung,exynos5420-gsc",
+		.data = &gsc_v_5420_drvdata,
 	},
 	{
 		.compatible = "samsung,exynos5433-gsc",