Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1730242 > unrolled thread
| Started by | Sylwester Nawrocki <s.nawrocki@samsung.com> |
|---|---|
| First post | 2017-09-11 11:40 +0200 |
| Last post | 2017-09-13 11:30 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits Sylwester Nawrocki <s.nawrocki@samsung.com> - 2017-09-11 11:40 +0200
Re: [PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits Hoegeun Kwon <hoegeun.kwon@samsung.com> - 2017-09-13 04:40 +0200
Re: [PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits Sylwester Nawrocki <s.nawrocki@samsung.com> - 2017-09-13 11:20 +0200
Re: [PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits Hoegeun Kwon <hoegeun.kwon@samsung.com> - 2017-09-13 11:30 +0200
| From | Sylwester Nawrocki <s.nawrocki@samsung.com> |
|---|---|
| Date | 2017-09-11 11:40 +0200 |
| Subject | Re: [PATCH v3 4/6] [media] exynos-gsc: Add hardware rotation limits |
| Message-ID | <uotmc-15R-39@gated-at.bofh.it> |
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,
> },
--
Regards,
Sylwester
[toc] | [next] | [standalone]
| From | Hoegeun Kwon <hoegeun.kwon@samsung.com> |
|---|---|
| Date | 2017-09-13 04:40 +0200 |
| Message-ID | <up5KT-1mw-13@gated-at.bofh.it> |
| In reply to | #1730242 |
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,
>> },
>
[toc] | [prev] | [next] | [standalone]
| From | Sylwester Nawrocki <s.nawrocki@samsung.com> |
|---|---|
| Date | 2017-09-13 11:20 +0200 |
| Message-ID | <upbZV-5r0-37@gated-at.bofh.it> |
| In reply to | #1731341 |
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.
--
Regards,
Sylwester
[toc] | [prev] | [next] | [standalone]
| From | Hoegeun Kwon <hoegeun.kwon@samsung.com> |
|---|---|
| Date | 2017-09-13 11:30 +0200 |
| Message-ID | <upc9B-5vh-25@gated-at.bofh.it> |
| In reply to | #1731500 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web