[PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers

[PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers

[Rahul Sharma]

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>
---
v5:
        1) Added pm_runtime_get_sync to enable the Display power domain.
v4:
        1) Added clk_disable for prev clock when clk_enable fails.
v3:
        1) Added checks for clk_enable.
v2:
        Rebase.
 drivers/gpu/drm/exynos/exynos_drm_fimd.c |   35 +++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index bd30d0c..6a30415 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -898,18 +898,51 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
 {
 	struct fimd_context *ctx = fimd_manager.ctx;
 	struct drm_device *drm_dev = data;
-	int win;
+	int win, ret;
 
 	fimd_mgr_initialize(&fimd_manager, drm_dev);
 	exynos_drm_crtc_create(&fimd_manager);
 	if (ctx->display)
 		exynos_drm_create_enc_conn(drm_dev, ctx->display);
 
+	ret = pm_runtime_get_sync(ctx->dev);
+	if (ret) {
+		dev_err(dev, "pm runtime get has failed.\n");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(ctx->bus_clk);
+	if (ret) {
+		dev_err(dev, "bus clock enable failed.\n");
+		goto bus_clk_err;
+	}
+
+	ret = clk_prepare_enable(ctx->lcd_clk);
+	if (ret) {
+		dev_err(dev, "lcd clock enable failed.\n");
+		goto lcd_clk_err;
+	}
+
 	for (win = 0; win < WINDOWS_NR; win++)
 		fimd_clear_win(ctx, win);
 
+	clk_disable_unprepare(ctx->lcd_clk);
+	clk_disable_unprepare(ctx->bus_clk);
+
+	ret = pm_runtime_put_sync(ctx->dev);
+	if (ret) {
+		dev_err(dev, "pm runtime put has failed.\n");
+		goto pm_put_err;
+	}
+
 	return 0;
 
+lcd_clk_err:
+	clk_disable_unprepare(ctx->bus_clk);
+bus_clk_err:
+	pm_runtime_put_sync(ctx->dev);
+pm_put_err:
+	return ret;
 }
 
 static void fimd_unbind(struct device *dev, struct device *master,
-- 
1.7.9.5

Re: [PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers

[Rahul Sharma]

Hi Inki,

Please review this patch.

Regards,
Rahul Sharma

On 23 May 2014 17:17, 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>
> ---
> v5:
>         1) Added pm_runtime_get_sync to enable the Display power domain.
> v4:
>         1) Added clk_disable for prev clock when clk_enable fails.
> v3:
>         1) Added checks for clk_enable.
> v2:
>         Rebase.
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   35 +++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index bd30d0c..6a30415 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -898,18 +898,51 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
>  {
>         struct fimd_context *ctx = fimd_manager.ctx;
>         struct drm_device *drm_dev = data;
> -       int win;
> +       int win, ret;
>
>         fimd_mgr_initialize(&fimd_manager, drm_dev);
>         exynos_drm_crtc_create(&fimd_manager);
>         if (ctx->display)
>                 exynos_drm_create_enc_conn(drm_dev, ctx->display);
>
> +       ret = pm_runtime_get_sync(ctx->dev);
> +       if (ret) {
> +               dev_err(dev, "pm runtime get has failed.\n");
> +               return ret;
> +       }
> +
> +       ret = clk_prepare_enable(ctx->bus_clk);
> +       if (ret) {
> +               dev_err(dev, "bus clock enable failed.\n");
> +               goto bus_clk_err;
> +       }
> +
> +       ret = clk_prepare_enable(ctx->lcd_clk);
> +       if (ret) {
> +               dev_err(dev, "lcd clock enable failed.\n");
> +               goto lcd_clk_err;
> +       }
> +
>         for (win = 0; win < WINDOWS_NR; win++)
>                 fimd_clear_win(ctx, win);
>
> +       clk_disable_unprepare(ctx->lcd_clk);
> +       clk_disable_unprepare(ctx->bus_clk);
> +
> +       ret = pm_runtime_put_sync(ctx->dev);
> +       if (ret) {
> +               dev_err(dev, "pm runtime put has failed.\n");
> +               goto pm_put_err;
> +       }
> +
>         return 0;
>
> +lcd_clk_err:
> +       clk_disable_unprepare(ctx->bus_clk);
> +bus_clk_err:
> +       pm_runtime_put_sync(ctx->dev);
> +pm_put_err:
> +       return ret;
>  }
>
>  static void fimd_unbind(struct device *dev, struct device *master,
> --
> 1.7.9.5
>

Re: [PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers

[Daniel Kurtz]

On Mon, May 26, 2014 at 2:59 PM, Rahul Sharma <rahul.sharma@samsung.com> wrote:
>
> Hi Inki,
>
> Please review this patch.
>
> Regards,
> Rahul Sharma
>
> On 23 May 2014 17:17, 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>
> > ---
> > v5:
> >         1) Added pm_runtime_get_sync to enable the Display power domain.
> > v4:
> >         1) Added clk_disable for prev clock when clk_enable fails.
> > v3:
> >         1) Added checks for clk_enable.
> > v2:
> >         Rebase.
> >  drivers/gpu/drm/exynos/exynos_drm_fimd.c |   35 +++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> > index bd30d0c..6a30415 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> > @@ -898,18 +898,51 @@ static int fimd_bind(struct device *dev, struct device *master, void *data)
> >  {
> >         struct fimd_context *ctx = fimd_manager.ctx;
> >         struct drm_device *drm_dev = data;
> > -       int win;
> > +       int win, ret;
> >
> >         fimd_mgr_initialize(&fimd_manager, drm_dev);
> >         exynos_drm_crtc_create(&fimd_manager);
> >         if (ctx->display)
> >                 exynos_drm_create_enc_conn(drm_dev, ctx->display);
> >
> > +       ret = pm_runtime_get_sync(ctx->dev);
> > +       if (ret) {
> > +               dev_err(dev, "pm runtime get has failed.\n");
> > +               return ret;
> > +       }
> > +
> > +       ret = clk_prepare_enable(ctx->bus_clk);
> > +       if (ret) {
> > +               dev_err(dev, "bus clock enable failed.\n");
> > +               goto bus_clk_err;
> > +       }
> > +
> > +       ret = clk_prepare_enable(ctx->lcd_clk);

Hi Rahul,

Can you explain why exactly we are "clearing windows" here in probe(), anyway?

IIUC, bus_clk is the clock that enables FIMD register access, and
lcd_clk clocks the scan out engine.
Therefore, if we only need to read/write some registers, we only need
the bus_clk, not lcd_clk, right?

However, fimd_clear_win() actually clears per-window registers.
Writes to per-window registers typically do not take effect until the
next vblank.
Therefore we do would need to enable lcd_clk to ensure that these
changes take effect.
Furthermore, to ensure the window clear completes during probe(), we
would also need to synchronously wait for the next vblank here - but
only if FIMD scanout is actually enabled already, otherwise there will
never be a next scanout, so we must check for that first.
Lastly, waiting around for a vblank could take a while.  Doing so in
probe() is not very friendly to boot up time, so the waiting should
probably be moved out of the main probe() thread into some sort of
asynchronous handler, which could then signal back when the clear is
complete.

Do you agree, or am I missing something?

Thanks,
-djk

>
> > +       if (ret) {
> > +               dev_err(dev, "lcd clock enable failed.\n");
> > +               goto lcd_clk_err;
> > +       }
> > +
> >         for (win = 0; win < WINDOWS_NR; win++)
> >                 fimd_clear_win(ctx, win);
> >
> > +       clk_disable_unprepare(ctx->lcd_clk);
> > +       clk_disable_unprepare(ctx->bus_clk);
> > +
> > +       ret = pm_runtime_put_sync(ctx->dev);
> > +       if (ret) {
> > +               dev_err(dev, "pm runtime put has failed.\n");
> > +               goto pm_put_err;
> > +       }
> > +
> >         return 0;
> >
> > +lcd_clk_err:
> > +       clk_disable_unprepare(ctx->bus_clk);
> > +bus_clk_err:
> > +       pm_runtime_put_sync(ctx->dev);
> > +pm_put_err:
> > +       return ret;
> >  }
> >
> >  static void fimd_unbind(struct device *dev, struct device *master,
> > --
> > 1.7.9.5
> >
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers

[Rahul Sharma]

Hi Daniel,

On 26 May 2014 13:11, Daniel Kurtz <djkurtz@chromium.org> wrote:
> On Mon, May 26, 2014 at 2:59 PM, Rahul Sharma <rahul.sharma@samsung.com> wrote:
>>
>> Hi Inki,
>>
>> Please review this patch.
[snip]
>> > +
>> > +       ret = clk_prepare_enable(ctx->lcd_clk);
>
> Hi Rahul,
>
> Can you explain why exactly we are "clearing windows" here in probe(), anyway?

I am not sure why it was added there but it is present since the first
version of this
file. Probably Inki can explain about this :). I can see the change
coming from his
first patch for adding drm fimd driver.

>
> IIUC, bus_clk is the clock that enables FIMD register access, and
> lcd_clk clocks the scan out engine.
> Therefore, if we only need to read/write some registers, we only need
> the bus_clk, not lcd_clk, right?
>

Correct, bus_clk should be sufficient to access the registers. But unless we
are confident about all implicit clock requirements in all SoCs, it is
safer to follow
the power_on/off sequence. This implementation is as good as DPMS on -> perform
reg operation -> DPMS Off. It was same in the original version but
later clock enables
were moved out of the probe.

> However, fimd_clear_win() actually clears per-window registers.
> Writes to per-window registers typically do not take effect until the
> next vblank.
> Therefore we do would need to enable lcd_clk to ensure that these
> changes take effect.
> Furthermore, to ensure the window clear completes during probe(), we
> would also need to synchronously wait for the next vblank here - but
> only if FIMD scanout is actually enabled already, otherwise there will
> never be a next scanout, so we must check for that first.
> Lastly, waiting around for a vblank could take a while.  Doing so in
> probe() is not very friendly to boot up time, so the waiting should
> probably be moved out of the main probe() thread into some sort of
> asynchronous handler, which could then signal back when the clear is
> complete.
>
> Do you agree, or am I missing something?

I agree. There seems a room for improvement. But at present we have two options,
either fix the current implementation and try to improve it as you mentioned
above. OR remove fimd_clear_win from probe if it is just a legacy code which
is no more required.

@Inki, need your inputs here.

Regards,
Rahul Sharma.

>
> Thanks,
> -djk
>
>>
>> > +       if (ret) {
[snip]
>> > +pm_put_err:
>> > +       return ret;
>> >  }
>> >
>> >  static void fimd_unbind(struct device *dev, struct device *master,
>> > --
>> > 1.7.9.5
>> >
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers

[Rahul Sharma]

<Gentle Reminder>

On 26 May 2014 14:21, Rahul Sharma <rahul.sharma@samsung.com> wrote:
> Hi Daniel,
>
> On 26 May 2014 13:11, Daniel Kurtz <djkurtz@chromium.org> wrote:
>> On Mon, May 26, 2014 at 2:59 PM, Rahul Sharma <rahul.sharma@samsung.com> wrote:
>>>
>>> Hi Inki,
>>>
>>> Please review this patch.
> [snip]
>>> > +
>>> > +       ret = clk_prepare_enable(ctx->lcd_clk);
>>
>> Hi Rahul,
>>
>> Can you explain why exactly we are "clearing windows" here in probe(), anyway?
>
> I am not sure why it was added there but it is present since the first
> version of this
> file. Probably Inki can explain about this :). I can see the change
> coming from his
> first patch for adding drm fimd driver.
>
>>
>> IIUC, bus_clk is the clock that enables FIMD register access, and
>> lcd_clk clocks the scan out engine.
>> Therefore, if we only need to read/write some registers, we only need
>> the bus_clk, not lcd_clk, right?
>>
>
> Correct, bus_clk should be sufficient to access the registers. But unless we
> are confident about all implicit clock requirements in all SoCs, it is
> safer to follow
> the power_on/off sequence. This implementation is as good as DPMS on -> perform
> reg operation -> DPMS Off. It was same in the original version but
> later clock enables
> were moved out of the probe.
>
>> However, fimd_clear_win() actually clears per-window registers.
>> Writes to per-window registers typically do not take effect until the
>> next vblank.
>> Therefore we do would need to enable lcd_clk to ensure that these
>> changes take effect.
>> Furthermore, to ensure the window clear completes during probe(), we
>> would also need to synchronously wait for the next vblank here - but
>> only if FIMD scanout is actually enabled already, otherwise there will
>> never be a next scanout, so we must check for that first.
>> Lastly, waiting around for a vblank could take a while.  Doing so in
>> probe() is not very friendly to boot up time, so the waiting should
>> probably be moved out of the main probe() thread into some sort of
>> asynchronous handler, which could then signal back when the clear is
>> complete.
>>
>> Do you agree, or am I missing something?
>
> I agree. There seems a room for improvement. But at present we have two options,
> either fix the current implementation and try to improve it as you mentioned
> above. OR remove fimd_clear_win from probe if it is just a legacy code which
> is no more required.
>
> @Inki, need your inputs here.
>
> Regards,
> Rahul Sharma.
>
>>
>> Thanks,
>> -djk
>>
>>>
>>> > +       if (ret) {
> [snip]
>>> > +pm_put_err:
>>> > +       return ret;
>>> >  }
>>> >
>>> >  static void fimd_unbind(struct device *dev, struct device *master,
>>> > --
>>> > 1.7.9.5
>>> >
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers

[Inki Dae]

On 2014년 05월 27일 18:55, Rahul Sharma wrote:
> <Gentle Reminder>
> 
> On 26 May 2014 14:21, Rahul Sharma <rahul.sharma@samsung.com> wrote:
>> Hi Daniel,
>>
>> On 26 May 2014 13:11, Daniel Kurtz <djkurtz@chromium.org> wrote:
>>> On Mon, May 26, 2014 at 2:59 PM, Rahul Sharma <rahul.sharma@samsung.com> wrote:
>>>>
>>>> Hi Inki,
>>>>
>>>> Please review this patch.
>> [snip]
>>>>> +
>>>>> +       ret = clk_prepare_enable(ctx->lcd_clk);
>>>
>>> Hi Rahul,
>>>
>>> Can you explain why exactly we are "clearing windows" here in probe(), anyway?
>>
>> I am not sure why it was added there but it is present since the first
>> version of this
>> file. Probably Inki can explain about this :). I can see the change
>> coming from his
>> first patch for adding drm fimd driver.
>>
>>>
>>> IIUC, bus_clk is the clock that enables FIMD register access, and
>>> lcd_clk clocks the scan out engine.
>>> Therefore, if we only need to read/write some registers, we only need
>>> the bus_clk, not lcd_clk, right?
>>>
>>
>> Correct, bus_clk should be sufficient to access the registers. But unless we
>> are confident about all implicit clock requirements in all SoCs, it is
>> safer to follow
>> the power_on/off sequence. This implementation is as good as DPMS on -> perform
>> reg operation -> DPMS Off. It was same in the original version but
>> later clock enables
>> were moved out of the probe.
>>
>>> However, fimd_clear_win() actually clears per-window registers.
>>> Writes to per-window registers typically do not take effect until the
>>> next vblank.
>>> Therefore we do would need to enable lcd_clk to ensure that these
>>> changes take effect.
>>> Furthermore, to ensure the window clear completes during probe(), we
>>> would also need to synchronously wait for the next vblank here - but
>>> only if FIMD scanout is actually enabled already, otherwise there will
>>> never be a next scanout, so we must check for that first.
>>> Lastly, waiting around for a vblank could take a while.  Doing so in
>>> probe() is not very friendly to boot up time, so the waiting should
>>> probably be moved out of the main probe() thread into some sort of
>>> asynchronous handler, which could then signal back when the clear is
>>> complete.
>>>
>>> Do you agree, or am I missing something?
>>
>> I agree. There seems a room for improvement. But at present we have two options,
>> either fix the current implementation and try to improve it as you mentioned
>> above. OR remove fimd_clear_win from probe if it is just a legacy code which

Just let's remove fimd_clear_win. it wouldn't need to disable all
hardware overlays at probe.

Thanks,
Inki Dae

>> is no more required.
>>
>> @Inki, need your inputs here.
>>
>> Regards,
>> Rahul Sharma.
>>
>>>
>>> Thanks,
>>> -djk
>>>
>>>>
>>>>> +       if (ret) {
>> [snip]
>>>>> +pm_put_err:
>>>>> +       return ret;
>>>>>  }
>>>>>
>>>>>  static void fimd_unbind(struct device *dev, struct device *master,
>>>>> --
>>>>> 1.7.9.5
>>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [PATCH v5] drm/exynos: enable fimd clocks in probe before accessing fimd registers

[Rahul Sharma]

Thanks Inki,

I removed fimd_clear_win from fimd_probe and verified this change in chrome
setup. Not seeing any noticeable difference.

I have posted another patch at:
http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31629.html

Thanks everybody for your review effort.

Regards,
Rahul Sharma.


On 27 May 2014 18:28, Inki Dae <inki.dae@samsung.com> wrote:
> On 2014년 05월 27일 18:55, Rahul Sharma wrote:
>> <Gentle Reminder>
>>
>> On 26 May 2014 14:21, Rahul Sharma <rahul.sharma@samsung.com> wrote:
>>> Hi Daniel,
>>>
>>> On 26 May 2014 13:11, Daniel Kurtz <djkurtz@chromium.org> wrote:
>>>> On Mon, May 26, 2014 at 2:59 PM, Rahul Sharma <rahul.sharma@samsung.com> wrote:
>>>>>
>>>>> Hi Inki,
>>>>>
>>>>> Please review this patch.
>>> [snip]
>>>>>> +
>>>>>> +       ret = clk_prepare_enable(ctx->lcd_clk);
>>>>
>>>> Hi Rahul,
>>>>
>>>> Can you explain why exactly we are "clearing windows" here in probe(), anyway?
>>>
>>> I am not sure why it was added there but it is present since the first
>>> version of this
>>> file. Probably Inki can explain about this :). I can see the change
>>> coming from his
>>> first patch for adding drm fimd driver.
>>>
>>>>
>>>> IIUC, bus_clk is the clock that enables FIMD register access, and
>>>> lcd_clk clocks the scan out engine.
>>>> Therefore, if we only need to read/write some registers, we only need
>>>> the bus_clk, not lcd_clk, right?
>>>>
>>>
>>> Correct, bus_clk should be sufficient to access the registers. But unless we
>>> are confident about all implicit clock requirements in all SoCs, it is
>>> safer to follow
>>> the power_on/off sequence. This implementation is as good as DPMS on -> perform
>>> reg operation -> DPMS Off. It was same in the original version but
>>> later clock enables
>>> were moved out of the probe.
>>>
>>>> However, fimd_clear_win() actually clears per-window registers.
>>>> Writes to per-window registers typically do not take effect until the
>>>> next vblank.
>>>> Therefore we do would need to enable lcd_clk to ensure that these
>>>> changes take effect.
>>>> Furthermore, to ensure the window clear completes during probe(), we
>>>> would also need to synchronously wait for the next vblank here - but
>>>> only if FIMD scanout is actually enabled already, otherwise there will
>>>> never be a next scanout, so we must check for that first.
>>>> Lastly, waiting around for a vblank could take a while.  Doing so in
>>>> probe() is not very friendly to boot up time, so the waiting should
>>>> probably be moved out of the main probe() thread into some sort of
>>>> asynchronous handler, which could then signal back when the clear is
>>>> complete.
>>>>
>>>> Do you agree, or am I missing something?
>>>
>>> I agree. There seems a room for improvement. But at present we have two options,
>>> either fix the current implementation and try to improve it as you mentioned
>>> above. OR remove fimd_clear_win from probe if it is just a legacy code which
>
> Just let's remove fimd_clear_win. it wouldn't need to disable all
> hardware overlays at probe.
>
> Thanks,
> Inki Dae
>
>>> is no more required.
>>>
>>> @Inki, need your inputs here.
>>>
>>> Regards,
>>> Rahul Sharma.
>>>
>>>>
>>>> Thanks,
>>>> -djk
>>>>
>>>>>
>>>>>> +       if (ret) {
>>> [snip]
>>>>>> +pm_put_err:
>>>>>> +       return ret;
>>>>>>  }
>>>>>>
>>>>>>  static void fimd_unbind(struct device *dev, struct device *master,
>>>>>> --
>>>>>> 1.7.9.5
>>>>>>
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel