| Message ID | 1400735767-5905-1-git-send-email-rahul.sharma@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi Rahul, On 22 May 2014 10:46, Rahul Sharma <rahul.sharma@samsung.com> wrote: > From: Rahul Sharma <Rahul.Sharma@samsung.com> > > Fimd probe is accessing fimd Registers without enabling the fimd > gate clocks. If FIMD clocks are kept disabled in Uboot or disbaled > during kernel boottime, the system hangs during boottime. > > This issue got surfaced when verifying with sysmmu enabled. Probe of > fimd Sysmmu enables the master clock before accessing sysmmu regs and > then disables. Later fimd probe tries to read the register without > enabling the clock which is wrong and hangs the system. > > Signed-off-by: Rahul Sharma <Rahul.Sharma@samsung.com> > --- > Rebased on exynos-drm-next branch. > > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index 173ee97..a79ba0a 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -891,9 +891,15 @@ static int fimd_bind(struct device *dev, struct device *master, void *data) > if (ctx->display) > exynos_drm_create_enc_conn(drm_dev, ctx->display); > > + clk_prepare_enable(ctx->bus_clk); Probably a check for its success? > + clk_prepare_enable(ctx->lcd_clk); ditto.
On 22 May 2014 11:51, Sachin Kamat <sachin.kamat@linaro.org> wrote: > Hi Rahul, [snip] >> >> + clk_prepare_enable(ctx->bus_clk); > > Probably a check for its success? > >> + clk_prepare_enable(ctx->lcd_clk); > Generally we don't check this in any of the driver. It will be quite unnecessary. Regards, Rahul Sharma > ditto. > > -- > With warm regards, > Sachin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22 May 2014 12:06, Rahul Sharma <rahul.sharma@samsung.com> wrote: > On 22 May 2014 11:51, Sachin Kamat <sachin.kamat@linaro.org> wrote: >> Hi Rahul, > [snip] >>> >>> + clk_prepare_enable(ctx->bus_clk); >> >> Probably a check for its success? >> >>> + clk_prepare_enable(ctx->lcd_clk); >> > > Generally we don't check this in any of the driver. It will be > quite unnecessary. However in your case, since you mentioned if the clock is not enabled, it will hang the system when fimd probe tries to read the register, this check would ensure it doesn't happen (hang) even if clk_prepare_enable fails for whatever reasons.
On Thu, May 22, 2014 at 12:06:16PM +0530, Rahul Sharma wrote: > On 22 May 2014 11:51, Sachin Kamat <sachin.kamat@linaro.org> wrote: > > Hi Rahul, > [snip] > >> > >> + clk_prepare_enable(ctx->bus_clk); > > > > Probably a check for its success? > > > >> + clk_prepare_enable(ctx->lcd_clk); > > > > Generally we don't check this in any of the driver. It will be > quite unnecessary. Then those drivers are all buggy. There's a reason why this function returns an int rather than void. Just because you've never seen it fail doesn't mean it can't. Thierry
On 22 May 2014 13:33, Thierry Reding <thierry.reding@gmail.com> wrote: > On Thu, May 22, 2014 at 12:06:16PM +0530, Rahul Sharma wrote: >> On 22 May 2014 11:51, Sachin Kamat <sachin.kamat@linaro.org> wrote: >> > Hi Rahul, >> [snip] >> >> >> >> + clk_prepare_enable(ctx->bus_clk); >> > >> > Probably a check for its success? >> > >> >> + clk_prepare_enable(ctx->lcd_clk); >> > >> >> Generally we don't check this in any of the driver. It will be >> quite unnecessary. > > Then those drivers are all buggy. There's a reason why this function > returns an int rather than void. Just because you've never seen it fail > doesn't mean it can't. Okay... I don't mind putting extra checks. V3 is coming :). Best Regards, Rahul Sharma > > Thierry > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 173ee97..a79ba0a 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -891,9 +891,15 @@ static int fimd_bind(struct device *dev, struct device *master, void *data) if (ctx->display) exynos_drm_create_enc_conn(drm_dev, ctx->display); + clk_prepare_enable(ctx->bus_clk); + clk_prepare_enable(ctx->lcd_clk); + for (win = 0; win < WINDOWS_NR; win++) fimd_clear_win(ctx, win); + clk_disable_unprepare(ctx->lcd_clk); + clk_disable_unprepare(ctx->bus_clk); + return 0; }