[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CABgxDo+NV7tnQpap-NA=yzJy0vEePFw0JSk2F2yGfhuTcEmtAA@mail.gmail.com>
Date: Wed, 30 Sep 2015 11:51:30 +0200
From: Romain Perier <romain.perier@...il.com>
To: Sjoerd Simons <sjoerd.simons@...labora.co.uk>
Cc: Yakir Yang <ykk@...k-chips.com>,
Mark Yao <mark.yao@...k-chips.com>,
Heiko Stuebner <heiko@...ech.de>,
David Airlie <airlied@...ux.ie>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
dri-devel@...ts.freedesktop.org,
"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] drm/rockchip: vop: Correct enabled clocks during setup
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@...il.com>
2015-09-29 13:58 GMT+02:00 Sjoerd Simons <sjoerd.simons@...labora.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@...labora.co.uk>
>> >
>> > Looks good and test on chromeos-3.14 tree, no problem, so
>> >
>> > Tested-by: Yakir Yang <ykk@...k-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@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists