Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-19 Thread Eric Anholt 
Stephen Boyd  writes:

> On 05/08, Eric Anholt wrote:
>> This is required for the panel to work on bcm911360, where CLCDCLK is
>> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
>> CLCDCLK, for platforms that have a settable rate on that one.
>> 
>> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
>> COMMON_CLK.
>> 
>> Signed-off-by: Eric Anholt 
>
> Reviewed-by: Stephen Boyd 
>
> One minor comment below
>
>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c 
>> b/drivers/gpu/drm/pl111/pl111_display.c
>> index 39a5c33bce7d..2d924a6bf43c 100644
>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>> @@ -288,6 +296,126 @@ const struct drm_simple_display_pipe_funcs 
>> pl111_display_funcs = {
> [...]
>> +
>> +return 0;
>> +}
>> +
>> +const struct clk_ops pl111_clk_div_ops = {
>
> static?

Fixed, thanks.


signature.asc
Description: PGP signature

Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-19 Thread Eric Anholt 
Stephen Boyd  writes:

> On 05/08, Eric Anholt wrote:
>> This is required for the panel to work on bcm911360, where CLCDCLK is
>> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
>> CLCDCLK, for platforms that have a settable rate on that one.
>> 
>> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
>> COMMON_CLK.
>> 
>> Signed-off-by: Eric Anholt 
>
> Reviewed-by: Stephen Boyd 
>
> One minor comment below
>
>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c 
>> b/drivers/gpu/drm/pl111/pl111_display.c
>> index 39a5c33bce7d..2d924a6bf43c 100644
>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>> @@ -288,6 +296,126 @@ const struct drm_simple_display_pipe_funcs 
>> pl111_display_funcs = {
> [...]
>> +
>> +return 0;
>> +}
>> +
>> +const struct clk_ops pl111_clk_div_ops = {
>
> static?

Fixed, thanks.


signature.asc
Description: PGP signature

Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-18 Thread Stephen Boyd 
On 05/08, Eric Anholt wrote:
> This is required for the panel to work on bcm911360, where CLCDCLK is
> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
> CLCDCLK, for platforms that have a settable rate on that one.
> 
> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
> COMMON_CLK.
> 
> Signed-off-by: Eric Anholt 

Reviewed-by: Stephen Boyd 

One minor comment below

> diff --git a/drivers/gpu/drm/pl111/pl111_display.c 
> b/drivers/gpu/drm/pl111/pl111_display.c
> index 39a5c33bce7d..2d924a6bf43c 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -288,6 +296,126 @@ const struct drm_simple_display_pipe_funcs 
> pl111_display_funcs = {
[...]
> +
> + return 0;
> +}
> +
> +const struct clk_ops pl111_clk_div_ops = {

static?

> + .recalc_rate = pl111_clk_div_recalc_rate,
> + .round_rate = pl111_clk_div_round_rate,
> + .set_rate = pl111_clk_div_set_rate,
> +};
> +
> +static int

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-18 Thread Stephen Boyd 
On 05/08, Eric Anholt wrote:
> This is required for the panel to work on bcm911360, where CLCDCLK is
> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
> CLCDCLK, for platforms that have a settable rate on that one.
> 
> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
> COMMON_CLK.
> 
> Signed-off-by: Eric Anholt 

Reviewed-by: Stephen Boyd 

One minor comment below

> diff --git a/drivers/gpu/drm/pl111/pl111_display.c 
> b/drivers/gpu/drm/pl111/pl111_display.c
> index 39a5c33bce7d..2d924a6bf43c 100644
> --- a/drivers/gpu/drm/pl111/pl111_display.c
> +++ b/drivers/gpu/drm/pl111/pl111_display.c
> @@ -288,6 +296,126 @@ const struct drm_simple_display_pipe_funcs 
> pl111_display_funcs = {
[...]
> +
> + return 0;
> +}
> +
> +const struct clk_ops pl111_clk_div_ops = {

static?

> + .recalc_rate = pl111_clk_div_recalc_rate,
> + .round_rate = pl111_clk_div_round_rate,
> + .set_rate = pl111_clk_div_set_rate,
> +};
> +
> +static int

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-10 Thread Linus Walleij 
On Tue, May 9, 2017 at 8:18 PM, Eric Anholt  wrote:
> Linus Walleij  writes:
>
>> On Mon, May 8, 2017 at 9:33 PM, Eric Anholt  wrote:
>>
>>> This is required for the panel to work on bcm911360, where CLCDCLK is
>>> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
>>> CLCDCLK, for platforms that have a settable rate on that one.
>>>
>>> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
>>> COMMON_CLK.
>>>
>>> Signed-off-by: Eric Anholt 
>>
>> Reviewed-by: Linus Walleij 
>
> Thanks.  Waiting on an ack from clock folks, then we'll be ready to go,
> I think.

Mike/Stephen?

It would be nice to have this queued in -next quite early in the v4.13
development cycle, so I can start looking into migrating the rest of the
old fbdev users to this.

BCM911360 seems like some oddity, there is not even a pictur of
it online, but I guess it's a cool little gadget :)

Yours,
Linus Walleij

Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-10 Thread Linus Walleij 
On Tue, May 9, 2017 at 8:18 PM, Eric Anholt  wrote:
> Linus Walleij  writes:
>
>> On Mon, May 8, 2017 at 9:33 PM, Eric Anholt  wrote:
>>
>>> This is required for the panel to work on bcm911360, where CLCDCLK is
>>> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
>>> CLCDCLK, for platforms that have a settable rate on that one.
>>>
>>> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
>>> COMMON_CLK.
>>>
>>> Signed-off-by: Eric Anholt 
>>
>> Reviewed-by: Linus Walleij 
>
> Thanks.  Waiting on an ack from clock folks, then we'll be ready to go,
> I think.

Mike/Stephen?

It would be nice to have this queued in -next quite early in the v4.13
development cycle, so I can start looking into migrating the rest of the
old fbdev users to this.

BCM911360 seems like some oddity, there is not even a pictur of
it online, but I guess it's a cool little gadget :)

Yours,
Linus Walleij

Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-09 Thread Eric Anholt 
Linus Walleij  writes:

> On Mon, May 8, 2017 at 9:33 PM, Eric Anholt  wrote:
>
>> This is required for the panel to work on bcm911360, where CLCDCLK is
>> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
>> CLCDCLK, for platforms that have a settable rate on that one.
>>
>> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
>> COMMON_CLK.
>>
>> Signed-off-by: Eric Anholt 
>
> Reviewed-by: Linus Walleij 

Thanks.  Waiting on an ack from clock folks, then we'll be ready to go,
I think.


signature.asc
Description: PGP signature

Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-09 Thread Eric Anholt 
Linus Walleij  writes:

> On Mon, May 8, 2017 at 9:33 PM, Eric Anholt  wrote:
>
>> This is required for the panel to work on bcm911360, where CLCDCLK is
>> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
>> CLCDCLK, for platforms that have a settable rate on that one.
>>
>> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
>> COMMON_CLK.
>>
>> Signed-off-by: Eric Anholt 
>
> Reviewed-by: Linus Walleij 

Thanks.  Waiting on an ack from clock folks, then we'll be ready to go,
I think.


signature.asc
Description: PGP signature

Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-08 Thread Linus Walleij 
On Mon, May 8, 2017 at 9:33 PM, Eric Anholt  wrote:

> This is required for the panel to work on bcm911360, where CLCDCLK is
> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
> CLCDCLK, for platforms that have a settable rate on that one.
>
> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
> COMMON_CLK.
>
> Signed-off-by: Eric Anholt 

Reviewed-by: Linus Walleij 

Yours,
Linus Walleij

Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-08 Thread Linus Walleij 
On Mon, May 8, 2017 at 9:33 PM, Eric Anholt  wrote:

> This is required for the panel to work on bcm911360, where CLCDCLK is
> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
> CLCDCLK, for platforms that have a settable rate on that one.
>
> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
> COMMON_CLK.
>
> Signed-off-by: Eric Anholt 

Reviewed-by: Linus Walleij 

Yours,
Linus Walleij

[PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-08 Thread Eric Anholt 
This is required for the panel to work on bcm911360, where CLCDCLK is
the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
CLCDCLK, for platforms that have a settable rate on that one.

v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
COMMON_CLK.

Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/pl111/Kconfig |   1 +
 drivers/gpu/drm/pl111/pl111_display.c | 162 ++
 drivers/gpu/drm/pl111/pl111_drm.h |   8 ++
 drivers/gpu/drm/pl111/pl111_drv.c |  11 +--
 include/linux/amba/clcd-regs.h|   5 ++
 5 files changed, 163 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
index ede49efd531f..309f4fd52de7 100644
--- a/drivers/gpu/drm/pl111/Kconfig
+++ b/drivers/gpu/drm/pl111/Kconfig
@@ -2,6 +2,7 @@ config DRM_PL111
tristate "DRM Support for PL111 CLCD Controller"
depends on DRM
depends on ARM || ARM64 || COMPILE_TEST
+   depends on COMMON_CLK
select DRM_KMS_HELPER
select DRM_KMS_CMA_HELPER
select DRM_GEM_CMA_HELPER
diff --git a/drivers/gpu/drm/pl111/pl111_display.c 
b/drivers/gpu/drm/pl111/pl111_display.c
index 39a5c33bce7d..2d924a6bf43c 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -108,7 +108,7 @@ static void pl111_display_enable(struct 
drm_simple_display_pipe *pipe,
u32 cntl;
u32 ppl, hsw, hfp, hbp;
u32 lpp, vsw, vfp, vbp;
-   u32 cpl;
+   u32 cpl, tim2;
int ret;
 
ret = clk_set_rate(priv->clk, mode->clock * 1000);
@@ -142,20 +142,28 @@ static void pl111_display_enable(struct 
drm_simple_display_pipe *pipe,
   (vfp << 16) |
   (vbp << 24),
   priv->regs + CLCD_TIM1);
-   /* XXX: We currently always use CLCDCLK with no divisor.  We
-* could probably reduce power consumption by using HCLK
-* (apb_pclk) with a divisor when it gets us near our target
-* pixel clock.
-*/
-   writel(((mode->flags & DRM_MODE_FLAG_NHSYNC) ? TIM2_IHS : 0) |
-  ((mode->flags & DRM_MODE_FLAG_NVSYNC) ? TIM2_IVS : 0) |
-  ((connector->display_info.bus_flags &
-DRM_BUS_FLAG_DE_LOW) ? TIM2_IOE : 0) |
-  ((connector->display_info.bus_flags &
-DRM_BUS_FLAG_PIXDATA_NEGEDGE) ? TIM2_IPC : 0) |
-  TIM2_BCD |
-  (cpl << 16),
-  priv->regs + CLCD_TIM2);
+
+   spin_lock(>tim2_lock);
+
+   tim2 = readl(priv->regs + CLCD_TIM2);
+   tim2 &= (TIM2_BCD | TIM2_PCD_LO_MASK | TIM2_PCD_HI_MASK);
+
+   if (mode->flags & DRM_MODE_FLAG_NHSYNC)
+   tim2 |= TIM2_IHS;
+
+   if (mode->flags & DRM_MODE_FLAG_NVSYNC)
+   tim2 |= TIM2_IVS;
+
+   if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW)
+   tim2 |= TIM2_IOE;
+
+   if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+   tim2 |= TIM2_IPC;
+
+   tim2 |= cpl << 16;
+   writel(tim2, priv->regs + CLCD_TIM2);
+   spin_unlock(>tim2_lock);
+
writel(0, priv->regs + CLCD_TIM3);
 
drm_panel_prepare(priv->connector.panel);
@@ -288,6 +296,126 @@ const struct drm_simple_display_pipe_funcs 
pl111_display_funcs = {
.prepare_fb = pl111_display_prepare_fb,
 };
 
+static int pl111_clk_div_choose_div(struct clk_hw *hw, unsigned long rate,
+   unsigned long *prate, bool set_parent)
+{
+   int best_div = 1, div;
+   struct clk_hw *parent = clk_hw_get_parent(hw);
+   unsigned long best_prate = 0;
+   unsigned long best_diff = ~0ul;
+   int max_div = (1 << (TIM2_PCD_LO_BITS + TIM2_PCD_HI_BITS)) - 1;
+
+   for (div = 1; div < max_div; div++) {
+   unsigned long this_prate, div_rate, diff;
+
+   if (set_parent)
+   this_prate = clk_hw_round_rate(parent, rate * div);
+   else
+   this_prate = *prate;
+   div_rate = DIV_ROUND_UP_ULL(this_prate, div);
+   diff = abs(rate - div_rate);
+
+   if (diff < best_diff) {
+   best_div = div;
+   best_diff = diff;
+   best_prate = this_prate;
+   }
+   }
+
+   *prate = best_prate;
+   return best_div;
+}
+
+static long pl111_clk_div_round_rate(struct clk_hw *hw, unsigned long rate,
+unsigned long *prate)
+{
+   int div = pl111_clk_div_choose_div(hw, rate, prate, true);
+
+   return DIV_ROUND_UP_ULL(*prate, div);
+}
+
+static unsigned long pl111_clk_div_recalc_rate(struct clk_hw *hw,
+  unsigned long prate)
+{
+   struct pl111_drm_dev_private *priv =
+   container_of(hw, struct pl111_drm_dev_private, clk_div);
+   u32 tim2 =

[PATCH v2] drm/pl111: Register the clock divider and use it.

2017-05-08 Thread Eric Anholt 
This is required for the panel to work on bcm911360, where CLCDCLK is
the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
CLCDCLK, for platforms that have a settable rate on that one.

v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
COMMON_CLK.

Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/pl111/Kconfig |   1 +
 drivers/gpu/drm/pl111/pl111_display.c | 162 ++
 drivers/gpu/drm/pl111/pl111_drm.h |   8 ++
 drivers/gpu/drm/pl111/pl111_drv.c |  11 +--
 include/linux/amba/clcd-regs.h|   5 ++
 5 files changed, 163 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig
index ede49efd531f..309f4fd52de7 100644
--- a/drivers/gpu/drm/pl111/Kconfig
+++ b/drivers/gpu/drm/pl111/Kconfig
@@ -2,6 +2,7 @@ config DRM_PL111
tristate "DRM Support for PL111 CLCD Controller"
depends on DRM
depends on ARM || ARM64 || COMPILE_TEST
+   depends on COMMON_CLK
select DRM_KMS_HELPER
select DRM_KMS_CMA_HELPER
select DRM_GEM_CMA_HELPER
diff --git a/drivers/gpu/drm/pl111/pl111_display.c 
b/drivers/gpu/drm/pl111/pl111_display.c
index 39a5c33bce7d..2d924a6bf43c 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -108,7 +108,7 @@ static void pl111_display_enable(struct 
drm_simple_display_pipe *pipe,
u32 cntl;
u32 ppl, hsw, hfp, hbp;
u32 lpp, vsw, vfp, vbp;
-   u32 cpl;
+   u32 cpl, tim2;
int ret;
 
ret = clk_set_rate(priv->clk, mode->clock * 1000);
@@ -142,20 +142,28 @@ static void pl111_display_enable(struct 
drm_simple_display_pipe *pipe,
   (vfp << 16) |
   (vbp << 24),
   priv->regs + CLCD_TIM1);
-   /* XXX: We currently always use CLCDCLK with no divisor.  We
-* could probably reduce power consumption by using HCLK
-* (apb_pclk) with a divisor when it gets us near our target
-* pixel clock.
-*/
-   writel(((mode->flags & DRM_MODE_FLAG_NHSYNC) ? TIM2_IHS : 0) |
-  ((mode->flags & DRM_MODE_FLAG_NVSYNC) ? TIM2_IVS : 0) |
-  ((connector->display_info.bus_flags &
-DRM_BUS_FLAG_DE_LOW) ? TIM2_IOE : 0) |
-  ((connector->display_info.bus_flags &
-DRM_BUS_FLAG_PIXDATA_NEGEDGE) ? TIM2_IPC : 0) |
-  TIM2_BCD |
-  (cpl << 16),
-  priv->regs + CLCD_TIM2);
+
+   spin_lock(>tim2_lock);
+
+   tim2 = readl(priv->regs + CLCD_TIM2);
+   tim2 &= (TIM2_BCD | TIM2_PCD_LO_MASK | TIM2_PCD_HI_MASK);
+
+   if (mode->flags & DRM_MODE_FLAG_NHSYNC)
+   tim2 |= TIM2_IHS;
+
+   if (mode->flags & DRM_MODE_FLAG_NVSYNC)
+   tim2 |= TIM2_IVS;
+
+   if (connector->display_info.bus_flags & DRM_BUS_FLAG_DE_LOW)
+   tim2 |= TIM2_IOE;
+
+   if (connector->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_NEGEDGE)
+   tim2 |= TIM2_IPC;
+
+   tim2 |= cpl << 16;
+   writel(tim2, priv->regs + CLCD_TIM2);
+   spin_unlock(>tim2_lock);
+
writel(0, priv->regs + CLCD_TIM3);
 
drm_panel_prepare(priv->connector.panel);
@@ -288,6 +296,126 @@ const struct drm_simple_display_pipe_funcs 
pl111_display_funcs = {
.prepare_fb = pl111_display_prepare_fb,
 };
 
+static int pl111_clk_div_choose_div(struct clk_hw *hw, unsigned long rate,
+   unsigned long *prate, bool set_parent)
+{
+   int best_div = 1, div;
+   struct clk_hw *parent = clk_hw_get_parent(hw);
+   unsigned long best_prate = 0;
+   unsigned long best_diff = ~0ul;
+   int max_div = (1 << (TIM2_PCD_LO_BITS + TIM2_PCD_HI_BITS)) - 1;
+
+   for (div = 1; div < max_div; div++) {
+   unsigned long this_prate, div_rate, diff;
+
+   if (set_parent)
+   this_prate = clk_hw_round_rate(parent, rate * div);
+   else
+   this_prate = *prate;
+   div_rate = DIV_ROUND_UP_ULL(this_prate, div);
+   diff = abs(rate - div_rate);
+
+   if (diff < best_diff) {
+   best_div = div;
+   best_diff = diff;
+   best_prate = this_prate;
+   }
+   }
+
+   *prate = best_prate;
+   return best_div;
+}
+
+static long pl111_clk_div_round_rate(struct clk_hw *hw, unsigned long rate,
+unsigned long *prate)
+{
+   int div = pl111_clk_div_choose_div(hw, rate, prate, true);
+
+   return DIV_ROUND_UP_ULL(*prate, div);
+}
+
+static unsigned long pl111_clk_div_recalc_rate(struct clk_hw *hw,
+  unsigned long prate)
+{
+   struct pl111_drm_dev_private *priv =
+   container_of(hw, struct pl111_drm_dev_private, clk_div);
+   u32 tim2 = readl(priv->regs +