dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/rockchip: vop: Correct enabled clocks during setup
@ 2015-09-29  9:28 Sjoerd Simons
  2015-09-29  9:55 ` Yakir Yang
  2015-09-29 10:14 ` Mark yao
  0 siblings, 2 replies; 6+ messages in thread
From: Sjoerd Simons @ 2015-09-29  9:28 UTC (permalink / raw)
  To: Mark Yao
  Cc: Heiko Stuebner, linux-kernel, dri-devel, linux-rockchip,
	David Airlie, linux-arm-kernel

When doing the initial setup both the hclk and the aclk need to be
enabled otherwise the board will simply hang. This only occurs when
building the vop driver as a module, when its built-in the initial setup
happens to run before the clock framework shuts of unused clocks
(including the aclk).

While there also switch to doing prepare and enable in one step rather
then separate steps to reduce the amount of code required.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

---

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36 +++++++++++------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 5d8ae5e..48719df 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1575,32 +1575,25 @@ static int vop_initial(struct vop *vop)
 		return PTR_ERR(vop->dclk);
 	}
 
-	ret = clk_prepare(vop->hclk);
-	if (ret < 0) {
-		dev_err(vop->dev, "failed to prepare hclk\n");
-		return ret;
-	}
-
 	ret = clk_prepare(vop->dclk);
 	if (ret < 0) {
 		dev_err(vop->dev, "failed to prepare dclk\n");
-		goto err_unprepare_hclk;
+		return ret;
 	}
 
-	ret = clk_prepare(vop->aclk);
+	/* Enable both the hclk and aclk to setup the vop */
+	ret = clk_prepare_enable(vop->hclk);
 	if (ret < 0) {
-		dev_err(vop->dev, "failed to prepare aclk\n");
+		dev_err(vop->dev, "failed to prepare/enable hclk\n");
 		goto err_unprepare_dclk;
 	}
 
-	/*
-	 * enable hclk, so that we can config vop register.
-	 */
-	ret = clk_enable(vop->hclk);
+	ret = clk_prepare_enable(vop->aclk);
 	if (ret < 0) {
-		dev_err(vop->dev, "failed to prepare aclk\n");
-		goto err_unprepare_aclk;
+		dev_err(vop->dev, "failed to prepare/enable aclk\n");
+		goto err_disable_hclk;
 	}
+
 	/*
 	 * do hclk_reset, reset all vop registers.
 	 */
@@ -1608,7 +1601,7 @@ static int vop_initial(struct vop *vop)
 	if (IS_ERR(ahb_rst)) {
 		dev_err(vop->dev, "failed to get ahb reset\n");
 		ret = PTR_ERR(ahb_rst);
-		goto err_disable_hclk;
+		goto err_disable_aclk;
 	}
 	reset_control_assert(ahb_rst);
 	usleep_range(10, 20);
@@ -1634,26 +1627,25 @@ static int vop_initial(struct vop *vop)
 	if (IS_ERR(vop->dclk_rst)) {
 		dev_err(vop->dev, "failed to get dclk reset\n");
 		ret = PTR_ERR(vop->dclk_rst);
-		goto err_unprepare_aclk;
+		goto err_disable_aclk;
 	}
 	reset_control_assert(vop->dclk_rst);
 	usleep_range(10, 20);
 	reset_control_deassert(vop->dclk_rst);
 
 	clk_disable(vop->hclk);
+	clk_disable(vop->aclk);
 
 	vop->is_enabled = false;
 
 	return 0;
 
+err_disable_aclk:
+	clk_disable_unprepare(vop->aclk);
 err_disable_hclk:
-	clk_disable(vop->hclk);
-err_unprepare_aclk:
-	clk_unprepare(vop->aclk);
+	clk_disable_unprepare(vop->hclk);
 err_unprepare_dclk:
 	clk_unprepare(vop->dclk);
-err_unprepare_hclk:
-	clk_unprepare(vop->hclk);
 	return ret;
 }
 
-- 
2.5.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/rockchip: vop: Correct enabled clocks during setup
  2015-09-29  9:28 [PATCH] drm/rockchip: vop: Correct enabled clocks during setup Sjoerd Simons
@ 2015-09-29  9:55 ` Yakir Yang
  2015-09-29 10:58   ` Yakir Yang
  2015-09-29 10:14 ` Mark yao
  1 sibling, 1 reply; 6+ messages in thread
From: Yakir Yang @ 2015-09-29  9:55 UTC (permalink / raw)
  To: Sjoerd Simons, Mark Yao
  Cc: linux-kernel, dri-devel, linux-rockchip, linux-arm-kernel



On 09/29/2015 05:28 PM, Sjoerd Simons wrote:
> When doing the initial setup both the hclk and the aclk need to be
> enabled otherwise the board will simply hang. This only occurs when
> building the vop driver as a module, when its built-in the initial setup
> happens to run before the clock framework shuts of unused clocks
> (including the aclk).
>
> While there also switch to doing prepare and enable in one step rather
> then separate steps to reduce the amount of code required.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

Looks good and test on chromeos-3.14 tree, no problem, so

Tested-by: Yakir Yang <ykk@rock-chips.com>

> ---
>
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36 +++++++++++------------------
>   1 file changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 5d8ae5e..48719df 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1575,32 +1575,25 @@ static int vop_initial(struct vop *vop)
>   		return PTR_ERR(vop->dclk);
>   	}
>   
> -	ret = clk_prepare(vop->hclk);
> -	if (ret < 0) {
> -		dev_err(vop->dev, "failed to prepare hclk\n");
> -		return ret;
> -	}
> -
>   	ret = clk_prepare(vop->dclk);
>   	if (ret < 0) {
>   		dev_err(vop->dev, "failed to prepare dclk\n");
> -		goto err_unprepare_hclk;
> +		return ret;
>   	}
>   
> -	ret = clk_prepare(vop->aclk);
> +	/* Enable both the hclk and aclk to setup the vop */
> +	ret = clk_prepare_enable(vop->hclk);
>   	if (ret < 0) {
> -		dev_err(vop->dev, "failed to prepare aclk\n");
> +		dev_err(vop->dev, "failed to prepare/enable hclk\n");
>   		goto err_unprepare_dclk;
>   	}
>   
> -	/*
> -	 * enable hclk, so that we can config vop register.
> -	 */
> -	ret = clk_enable(vop->hclk);
> +	ret = clk_prepare_enable(vop->aclk);
>   	if (ret < 0) {
> -		dev_err(vop->dev, "failed to prepare aclk\n");
> -		goto err_unprepare_aclk;
> +		dev_err(vop->dev, "failed to prepare/enable aclk\n");
> +		goto err_disable_hclk;
>   	}
> +
>   	/*
>   	 * do hclk_reset, reset all vop registers.
>   	 */
> @@ -1608,7 +1601,7 @@ static int vop_initial(struct vop *vop)
>   	if (IS_ERR(ahb_rst)) {
>   		dev_err(vop->dev, "failed to get ahb reset\n");
>   		ret = PTR_ERR(ahb_rst);
> -		goto err_disable_hclk;
> +		goto err_disable_aclk;
>   	}
>   	reset_control_assert(ahb_rst);
>   	usleep_range(10, 20);
> @@ -1634,26 +1627,25 @@ static int vop_initial(struct vop *vop)
>   	if (IS_ERR(vop->dclk_rst)) {
>   		dev_err(vop->dev, "failed to get dclk reset\n");
>   		ret = PTR_ERR(vop->dclk_rst);
> -		goto err_unprepare_aclk;
> +		goto err_disable_aclk;
>   	}
>   	reset_control_assert(vop->dclk_rst);
>   	usleep_range(10, 20);
>   	reset_control_deassert(vop->dclk_rst);
>   
>   	clk_disable(vop->hclk);
> +	clk_disable(vop->aclk);
>   
>   	vop->is_enabled = false;
>   
>   	return 0;
>   
> +err_disable_aclk:
> +	clk_disable_unprepare(vop->aclk);
>   err_disable_hclk:
> -	clk_disable(vop->hclk);
> -err_unprepare_aclk:
> -	clk_unprepare(vop->aclk);
> +	clk_disable_unprepare(vop->hclk);
>   err_unprepare_dclk:
>   	clk_unprepare(vop->dclk);
> -err_unprepare_hclk:
> -	clk_unprepare(vop->hclk);
>   	return ret;
>   }
>   


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/rockchip: vop: Correct enabled clocks during setup
  2015-09-29  9:28 [PATCH] drm/rockchip: vop: Correct enabled clocks during setup Sjoerd Simons
  2015-09-29  9:55 ` Yakir Yang
@ 2015-09-29 10:14 ` Mark yao
  1 sibling, 0 replies; 6+ messages in thread
From: Mark yao @ 2015-09-29 10:14 UTC (permalink / raw)
  To: Sjoerd Simons; +Cc: linux-kernel, dri-devel, linux-rockchip, linux-arm-kernel

Hi Sjoerd
     We double check this problem, yes, board will hang if aclk is disabled
when setting vop register.

     Acked-by: Mark Yao <mark.yao@rock-chips.com>

Thanks for this fix.

On 2015年09月29日 17:28, Sjoerd Simons wrote:
> When doing the initial setup both the hclk and the aclk need to be
> enabled otherwise the board will simply hang. This only occurs when
> building the vop driver as a module, when its built-in the initial setup
> happens to run before the clock framework shuts of unused clocks
> (including the aclk).
>
> While there also switch to doing prepare and enable in one step rather
> then separate steps to reduce the amount of code required.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
>
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36 +++++++++++------------------
>   1 file changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 5d8ae5e..48719df 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1575,32 +1575,25 @@ static int vop_initial(struct vop *vop)
>   		return PTR_ERR(vop->dclk);
>   	}
>   
> -	ret = clk_prepare(vop->hclk);
> -	if (ret < 0) {
> -		dev_err(vop->dev, "failed to prepare hclk\n");
> -		return ret;
> -	}
> -
>   	ret = clk_prepare(vop->dclk);
>   	if (ret < 0) {
>   		dev_err(vop->dev, "failed to prepare dclk\n");
> -		goto err_unprepare_hclk;
> +		return ret;
>   	}
>   
> -	ret = clk_prepare(vop->aclk);
> +	/* Enable both the hclk and aclk to setup the vop */
> +	ret = clk_prepare_enable(vop->hclk);
>   	if (ret < 0) {
> -		dev_err(vop->dev, "failed to prepare aclk\n");
> +		dev_err(vop->dev, "failed to prepare/enable hclk\n");
>   		goto err_unprepare_dclk;
>   	}
>   
> -	/*
> -	 * enable hclk, so that we can config vop register.
> -	 */
> -	ret = clk_enable(vop->hclk);
> +	ret = clk_prepare_enable(vop->aclk);
>   	if (ret < 0) {
> -		dev_err(vop->dev, "failed to prepare aclk\n");
> -		goto err_unprepare_aclk;
> +		dev_err(vop->dev, "failed to prepare/enable aclk\n");
> +		goto err_disable_hclk;
>   	}
> +
>   	/*
>   	 * do hclk_reset, reset all vop registers.
>   	 */
> @@ -1608,7 +1601,7 @@ static int vop_initial(struct vop *vop)
>   	if (IS_ERR(ahb_rst)) {
>   		dev_err(vop->dev, "failed to get ahb reset\n");
>   		ret = PTR_ERR(ahb_rst);
> -		goto err_disable_hclk;
> +		goto err_disable_aclk;
>   	}
>   	reset_control_assert(ahb_rst);
>   	usleep_range(10, 20);
> @@ -1634,26 +1627,25 @@ static int vop_initial(struct vop *vop)
>   	if (IS_ERR(vop->dclk_rst)) {
>   		dev_err(vop->dev, "failed to get dclk reset\n");
>   		ret = PTR_ERR(vop->dclk_rst);
> -		goto err_unprepare_aclk;
> +		goto err_disable_aclk;
>   	}
>   	reset_control_assert(vop->dclk_rst);
>   	usleep_range(10, 20);
>   	reset_control_deassert(vop->dclk_rst);
>   
>   	clk_disable(vop->hclk);
> +	clk_disable(vop->aclk);
>   
>   	vop->is_enabled = false;
>   
>   	return 0;
>   
> +err_disable_aclk:
> +	clk_disable_unprepare(vop->aclk);
>   err_disable_hclk:
> -	clk_disable(vop->hclk);
> -err_unprepare_aclk:
> -	clk_unprepare(vop->aclk);
> +	clk_disable_unprepare(vop->hclk);
>   err_unprepare_dclk:
>   	clk_unprepare(vop->dclk);
> -err_unprepare_hclk:
> -	clk_unprepare(vop->hclk);
>   	return ret;
>   }
>   


-- 
Mark Yao


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/rockchip: vop: Correct enabled clocks during setup
  2015-09-29  9:55 ` Yakir Yang
@ 2015-09-29 10:58   ` Yakir Yang
  2015-09-29 11:58     ` Sjoerd Simons
  0 siblings, 1 reply; 6+ messages in thread
From: Yakir Yang @ 2015-09-29 10:58 UTC (permalink / raw)
  To: Sjoerd Simons, Mark Yao
  Cc: linux-kernel, dri-devel, linux-rockchip, linux-arm-kernel



On 09/29/2015 05:55 PM, Yakir Yang wrote:
>
>
> On 09/29/2015 05:28 PM, Sjoerd Simons wrote:
>> When doing the initial setup both the hclk and the aclk need to be
>> enabled otherwise the board will simply hang. This only occurs when
>> building the vop driver as a module, when its built-in the initial setup

Hmm... My previous test was built-in the vop driver, and just notice that
you say problem only occurred when building the vop driver as module.
That's to say my test was wrong, so I try to do the right things.

But I found that vop driver module and rockchipdrm driver module in
dependency cycles, here are the build message:
     depmod: ERROR: Found 2 modules in dependency cycles!
     depmod: ERROR: Cycle detected: rockchip_drm_vop -> rockchipdrm -> 
rockchip_drm_vop
     Makefile:1054: recipe for target '_modinst_post' failed

And past my Makefile:
vop-y := rockchip_drm_drv.o rockchip_drm_fb.o rockchip_drm_fbdev.o 
rockchip_drm_gem.o
obj-m = rockchip_drm_vop.o vop.o

Very like to know how you handle this dependency cycles :)

Thanks,
- Yakir

>> happens to run before the clock framework shuts of unused clocks
>> (including the aclk).
>>
>> While there also switch to doing prepare and enable in one step rather
>> then separate steps to reduce the amount of code required.
>>
>> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> Looks good and test on chromeos-3.14 tree, no problem, so
>
> Tested-by: Yakir Yang <ykk@rock-chips.com>
>
>> ---
>>
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36 
>> +++++++++++------------------
>>   1 file changed, 14 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
>> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> index 5d8ae5e..48719df 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -1575,32 +1575,25 @@ static int vop_initial(struct vop *vop)
>>           return PTR_ERR(vop->dclk);
>>       }
>>   -    ret = clk_prepare(vop->hclk);
>> -    if (ret < 0) {
>> -        dev_err(vop->dev, "failed to prepare hclk\n");
>> -        return ret;
>> -    }
>> -
>>       ret = clk_prepare(vop->dclk);
>>       if (ret < 0) {
>>           dev_err(vop->dev, "failed to prepare dclk\n");
>> -        goto err_unprepare_hclk;
>> +        return ret;
>>       }
>>   -    ret = clk_prepare(vop->aclk);
>> +    /* Enable both the hclk and aclk to setup the vop */
>> +    ret = clk_prepare_enable(vop->hclk);
>>       if (ret < 0) {
>> -        dev_err(vop->dev, "failed to prepare aclk\n");
>> +        dev_err(vop->dev, "failed to prepare/enable hclk\n");
>>           goto err_unprepare_dclk;
>>       }
>>   -    /*
>> -     * enable hclk, so that we can config vop register.
>> -     */
>> -    ret = clk_enable(vop->hclk);
>> +    ret = clk_prepare_enable(vop->aclk);
>>       if (ret < 0) {
>> -        dev_err(vop->dev, "failed to prepare aclk\n");
>> -        goto err_unprepare_aclk;
>> +        dev_err(vop->dev, "failed to prepare/enable aclk\n");
>> +        goto err_disable_hclk;
>>       }
>> +
>>       /*
>>        * do hclk_reset, reset all vop registers.
>>        */
>> @@ -1608,7 +1601,7 @@ static int vop_initial(struct vop *vop)
>>       if (IS_ERR(ahb_rst)) {
>>           dev_err(vop->dev, "failed to get ahb reset\n");
>>           ret = PTR_ERR(ahb_rst);
>> -        goto err_disable_hclk;
>> +        goto err_disable_aclk;
>>       }
>>       reset_control_assert(ahb_rst);
>>       usleep_range(10, 20);
>> @@ -1634,26 +1627,25 @@ static int vop_initial(struct vop *vop)
>>       if (IS_ERR(vop->dclk_rst)) {
>>           dev_err(vop->dev, "failed to get dclk reset\n");
>>           ret = PTR_ERR(vop->dclk_rst);
>> -        goto err_unprepare_aclk;
>> +        goto err_disable_aclk;
>>       }
>>       reset_control_assert(vop->dclk_rst);
>>       usleep_range(10, 20);
>>       reset_control_deassert(vop->dclk_rst);
>>         clk_disable(vop->hclk);
>> +    clk_disable(vop->aclk);
>>         vop->is_enabled = false;
>>         return 0;
>>   +err_disable_aclk:
>> +    clk_disable_unprepare(vop->aclk);
>>   err_disable_hclk:
>> -    clk_disable(vop->hclk);
>> -err_unprepare_aclk:
>> -    clk_unprepare(vop->aclk);
>> +    clk_disable_unprepare(vop->hclk);
>>   err_unprepare_dclk:
>>       clk_unprepare(vop->dclk);
>> -err_unprepare_hclk:
>> -    clk_unprepare(vop->hclk);
>>       return ret;
>>   }
>


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/rockchip: vop: Correct enabled clocks during setup
  2015-09-29 10:58   ` Yakir Yang
@ 2015-09-29 11:58     ` Sjoerd Simons
  2015-09-30  9:51       ` Romain Perier
  0 siblings, 1 reply; 6+ messages in thread
From: Sjoerd Simons @ 2015-09-29 11:58 UTC (permalink / raw)
  To: Yakir Yang, Mark Yao
  Cc: linux-kernel, dri-devel, linux-rockchip, linux-arm-kernel

On Tue, 2015-09-29 at 18:58 +0800, Yakir Yang wrote:
> 
> On 09/29/2015 05:55 PM, Yakir Yang wrote:
> > 
> > 
> > On 09/29/2015 05:28 PM, Sjoerd Simons wrote:
> > > When doing the initial setup both the hclk and the aclk need to
> > > be
> > > enabled otherwise the board will simply hang. This only occurs
> > > when
> > > building the vop driver as a module, when its built-in the
> > > initial setup
> 
> Hmm... My previous test was built-in the vop driver, and just notice
> that
> you say problem only occurred when building the vop driver as module.
> That's to say my test was wrong, so I try to do the right things.
> 
> But I found that vop driver module and rockchipdrm driver module in
> dependency cycles, here are the build message:
>      depmod: ERROR: Found 2 modules in dependency cycles!


I've only tested with mainline which doesn't seem to have that issue?
So can't easily help you there unfortunately.


> Thanks,
> - Yakir
> 
> > > happens to run before the clock framework shuts of unused clocks
> > > (including the aclk).
> > > 
> > > While there also switch to doing prepare and enable in one step
> > > rather
> > > then separate steps to reduce the amount of code required.
> > > 
> > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> > 
> > Looks good and test on chromeos-3.14 tree, no problem, so
> > 
> > Tested-by: Yakir Yang <ykk@rock-chips.com>
> > 
> > > ---
> > > 
> > >   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36 
> > > +++++++++++------------------
> > >   1 file changed, 14 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
> > > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > index 5d8ae5e..48719df 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > @@ -1575,32 +1575,25 @@ static int vop_initial(struct vop *vop)
> > >           return PTR_ERR(vop->dclk);
> > >       }
> > >   -    ret = clk_prepare(vop->hclk);
> > > -    if (ret < 0) {
> > > -        dev_err(vop->dev, "failed to prepare hclk\n");
> > > -        return ret;
> > > -    }
> > > -
> > >       ret = clk_prepare(vop->dclk);
> > >       if (ret < 0) {
> > >           dev_err(vop->dev, "failed to prepare dclk\n");
> > > -        goto err_unprepare_hclk;
> > > +        return ret;
> > >       }
> > >   -    ret = clk_prepare(vop->aclk);
> > > +    /* Enable both the hclk and aclk to setup the vop */
> > > +    ret = clk_prepare_enable(vop->hclk);
> > >       if (ret < 0) {
> > > -        dev_err(vop->dev, "failed to prepare aclk\n");
> > > +        dev_err(vop->dev, "failed to prepare/enable hclk\n");
> > >           goto err_unprepare_dclk;
> > >       }
> > >   -    /*
> > > -     * enable hclk, so that we can config vop register.
> > > -     */
> > > -    ret = clk_enable(vop->hclk);
> > > +    ret = clk_prepare_enable(vop->aclk);
> > >       if (ret < 0) {
> > > -        dev_err(vop->dev, "failed to prepare aclk\n");
> > > -        goto err_unprepare_aclk;
> > > +        dev_err(vop->dev, "failed to prepare/enable aclk\n");
> > > +        goto err_disable_hclk;
> > >       }
> > > +
> > >       /*
> > >        * do hclk_reset, reset all vop registers.
> > >        */
> > > @@ -1608,7 +1601,7 @@ static int vop_initial(struct vop *vop)
> > >       if (IS_ERR(ahb_rst)) {
> > >           dev_err(vop->dev, "failed to get ahb reset\n");
> > >           ret = PTR_ERR(ahb_rst);
> > > -        goto err_disable_hclk;
> > > +        goto err_disable_aclk;
> > >       }
> > >       reset_control_assert(ahb_rst);
> > >       usleep_range(10, 20);
> > > @@ -1634,26 +1627,25 @@ static int vop_initial(struct vop *vop)
> > >       if (IS_ERR(vop->dclk_rst)) {
> > >           dev_err(vop->dev, "failed to get dclk reset\n");
> > >           ret = PTR_ERR(vop->dclk_rst);
> > > -        goto err_unprepare_aclk;
> > > +        goto err_disable_aclk;
> > >       }
> > >       reset_control_assert(vop->dclk_rst);
> > >       usleep_range(10, 20);
> > >       reset_control_deassert(vop->dclk_rst);
> > >         clk_disable(vop->hclk);
> > > +    clk_disable(vop->aclk);
> > >         vop->is_enabled = false;
> > >         return 0;
> > >   +err_disable_aclk:
> > > +    clk_disable_unprepare(vop->aclk);
> > >   err_disable_hclk:
> > > -    clk_disable(vop->hclk);
> > > -err_unprepare_aclk:
> > > -    clk_unprepare(vop->aclk);
> > > +    clk_disable_unprepare(vop->hclk);
> > >   err_unprepare_dclk:
> > >       clk_unprepare(vop->dclk);
> > > -err_unprepare_hclk:
> > > -    clk_unprepare(vop->hclk);
> > >       return ret;
> > >   }
> > 
> 
> 

-- 
Sjoerd Simons
Collabora Ltd.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/rockchip: vop: Correct enabled clocks during setup
  2015-09-29 11:58     ` Sjoerd Simons
@ 2015-09-30  9:51       ` Romain Perier
  0 siblings, 0 replies; 6+ messages in thread
From: Romain Perier @ 2015-09-30  9:51 UTC (permalink / raw)
  To: Sjoerd Simons
  Cc: Yakir Yang, Mark Yao, Heiko Stuebner, David Airlie,
	Linux Kernel Mailing List, dri-devel,
	open list:ARM/Rockchip SoC...,
	linux-arm-kernel

Hi all,

I had an issue with the drm driver, when I start the kernel via kexec,
the drm driver freezes the platform. it is caused by the function
vop_initial (when copying registers via ahb). This issue happens with
the chromeos kernel or with the mainline kernel. When I investigated a
bit, I found that it had something to do with the hclk clock.

This patch fixes the issue. Everything works like a charm now.

Tested-by: Romain Perier <romain.perier@gmail.com>

2015-09-29 13:58 GMT+02:00 Sjoerd Simons <sjoerd.simons@collabora.co.uk>:
> On Tue, 2015-09-29 at 18:58 +0800, Yakir Yang wrote:
>>
>> On 09/29/2015 05:55 PM, Yakir Yang wrote:
>> >
>> >
>> > On 09/29/2015 05:28 PM, Sjoerd Simons wrote:
>> > > When doing the initial setup both the hclk and the aclk need to
>> > > be
>> > > enabled otherwise the board will simply hang. This only occurs
>> > > when
>> > > building the vop driver as a module, when its built-in the
>> > > initial setup
>>
>> Hmm... My previous test was built-in the vop driver, and just notice
>> that
>> you say problem only occurred when building the vop driver as module.
>> That's to say my test was wrong, so I try to do the right things.
>>
>> But I found that vop driver module and rockchipdrm driver module in
>> dependency cycles, here are the build message:
>>      depmod: ERROR: Found 2 modules in dependency cycles!
>
>
> I've only tested with mainline which doesn't seem to have that issue?
> So can't easily help you there unfortunately.
>
>
>> Thanks,
>> - Yakir
>>
>> > > happens to run before the clock framework shuts of unused clocks
>> > > (including the aclk).
>> > >
>> > > While there also switch to doing prepare and enable in one step
>> > > rather
>> > > then separate steps to reduce the amount of code required.
>> > >
>> > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>> >
>> > Looks good and test on chromeos-3.14 tree, no problem, so
>> >
>> > Tested-by: Yakir Yang <ykk@rock-chips.com>
>> >
>> > > ---
>> > >
>> > >   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36
>> > > +++++++++++------------------
>> > >   1 file changed, 14 insertions(+), 22 deletions(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> > > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> > > index 5d8ae5e..48719df 100644
>> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> > > @@ -1575,32 +1575,25 @@ static int vop_initial(struct vop *vop)
>> > >           return PTR_ERR(vop->dclk);
>> > >       }
>> > >   -    ret = clk_prepare(vop->hclk);
>> > > -    if (ret < 0) {
>> > > -        dev_err(vop->dev, "failed to prepare hclk\n");
>> > > -        return ret;
>> > > -    }
>> > > -
>> > >       ret = clk_prepare(vop->dclk);
>> > >       if (ret < 0) {
>> > >           dev_err(vop->dev, "failed to prepare dclk\n");
>> > > -        goto err_unprepare_hclk;
>> > > +        return ret;
>> > >       }
>> > >   -    ret = clk_prepare(vop->aclk);
>> > > +    /* Enable both the hclk and aclk to setup the vop */
>> > > +    ret = clk_prepare_enable(vop->hclk);
>> > >       if (ret < 0) {
>> > > -        dev_err(vop->dev, "failed to prepare aclk\n");
>> > > +        dev_err(vop->dev, "failed to prepare/enable hclk\n");
>> > >           goto err_unprepare_dclk;
>> > >       }
>> > >   -    /*
>> > > -     * enable hclk, so that we can config vop register.
>> > > -     */
>> > > -    ret = clk_enable(vop->hclk);
>> > > +    ret = clk_prepare_enable(vop->aclk);
>> > >       if (ret < 0) {
>> > > -        dev_err(vop->dev, "failed to prepare aclk\n");
>> > > -        goto err_unprepare_aclk;
>> > > +        dev_err(vop->dev, "failed to prepare/enable aclk\n");
>> > > +        goto err_disable_hclk;
>> > >       }
>> > > +
>> > >       /*
>> > >        * do hclk_reset, reset all vop registers.
>> > >        */
>> > > @@ -1608,7 +1601,7 @@ static int vop_initial(struct vop *vop)
>> > >       if (IS_ERR(ahb_rst)) {
>> > >           dev_err(vop->dev, "failed to get ahb reset\n");
>> > >           ret = PTR_ERR(ahb_rst);
>> > > -        goto err_disable_hclk;
>> > > +        goto err_disable_aclk;
>> > >       }
>> > >       reset_control_assert(ahb_rst);
>> > >       usleep_range(10, 20);
>> > > @@ -1634,26 +1627,25 @@ static int vop_initial(struct vop *vop)
>> > >       if (IS_ERR(vop->dclk_rst)) {
>> > >           dev_err(vop->dev, "failed to get dclk reset\n");
>> > >           ret = PTR_ERR(vop->dclk_rst);
>> > > -        goto err_unprepare_aclk;
>> > > +        goto err_disable_aclk;
>> > >       }
>> > >       reset_control_assert(vop->dclk_rst);
>> > >       usleep_range(10, 20);
>> > >       reset_control_deassert(vop->dclk_rst);
>> > >         clk_disable(vop->hclk);
>> > > +    clk_disable(vop->aclk);
>> > >         vop->is_enabled = false;
>> > >         return 0;
>> > >   +err_disable_aclk:
>> > > +    clk_disable_unprepare(vop->aclk);
>> > >   err_disable_hclk:
>> > > -    clk_disable(vop->hclk);
>> > > -err_unprepare_aclk:
>> > > -    clk_unprepare(vop->aclk);
>> > > +    clk_disable_unprepare(vop->hclk);
>> > >   err_unprepare_dclk:
>> > >       clk_unprepare(vop->dclk);
>> > > -err_unprepare_hclk:
>> > > -    clk_unprepare(vop->hclk);
>> > >       return ret;
>> > >   }
>> >
>>
>>
>
> --
> Sjoerd Simons
> Collabora Ltd.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-09-30  9:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-29  9:28 [PATCH] drm/rockchip: vop: Correct enabled clocks during setup Sjoerd Simons
2015-09-29  9:55 ` Yakir Yang
2015-09-29 10:58   ` Yakir Yang
2015-09-29 11:58     ` Sjoerd Simons
2015-09-30  9:51       ` Romain Perier
2015-09-29 10:14 ` Mark yao

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).