Re: [PATCH V2 6/7] ARM: EXYNOS5: Add the bus clock for FIMD

[Jingoo Han <jg1.han@samsung.com>]

On Tuesday, July 24, 2012 12:06 PM, Joonyoung Shim Wrote:
> 
> On 07/24/2012 11:15 AM, Jingoo Han wrote:
> > On Tuesday, July 24, 2012 10:56 AM, Joonyoung Shim Wrote:
> >> On 07/24/2012 08:55 AM, Jingoo Han wrote:
> >>> On Tuesday, July 24, 2012 8:46 AM, Joonyoung Shim Wrote:
> >>>> On 07/24/2012 08:14 AM, Jingoo Han wrote:
> >>>>> On Monday, July 23, 2012 6:55 PM, Joonyoung Shim wrote:
> >>>>>> Hi, Jingoo.
> >>>>>>
> >>>>>> On 07/23/2012 05:34 PM, Joonyoung Shim wrote:
> >>>>>>> On 07/18/2012 02:57 PM, Leela Krishna Amudala wrote:
> >>>>>>>> This patch adds the bus clock for FIMD and changes
> >>>>>>>> the device name for lcd clock
> >>>>>>> Please refer below patch for exynos4.
> >>>>>>>
> >>>>>>> http://lists.linaro.org/pipermail/linaro-dev/2011-December/008872.html
> >>>>>>>
> >>>>>>>> Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
> >>>>>>>> ---
> >>>>>>>>      arch/arm/mach-exynos/clock-exynos5.c |    7 ++++++-
> >>>>>>>>      1 files changed, 6 insertions(+), 1 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/arch/arm/mach-exynos/clock-exynos5.c
> >>>>>>>> b/arch/arm/mach-exynos/clock-exynos5.c
> >>>>>>>> index 774533c..f001876 100644
> >>>>>>>> --- a/arch/arm/mach-exynos/clock-exynos5.c
> >>>>>>>> +++ b/arch/arm/mach-exynos/clock-exynos5.c
> >>>>>>>> @@ -634,6 +634,11 @@ static struct clk exynos5_init_clocks_off[] = {
> >>>>>>>>              .enable        = exynos5_clk_ip_disp1_ctrl,
> >>>>>>>>              .ctrlbit    = (1 << 3),
> >>>>>>>>          }, {
> >>>>>>>> +        .name           = "fimd",
> >>>>>>>> +        .devname        = "exynos5-fb",
> >>>>>>>> +        .enable         = exynos5_clk_ip_disp1_ctrl,
> >>>>>>>> +        .ctrlbit        = (1 << 0),
> >>>>>>>> +    }, {
> >>>>>> With this patch, it causes below error at the DP driver because fimd
> >>>>>> clock is disabled.
> >>>>>>
> >>>>>> [    0.210000] exynos-dp exynos-dp: Timeout of video streamclk ok
> >>>>>> [    0.210000] exynos-dp exynos-dp: unable to config video
> >>>>>> [    0.210000] exynos-dp: probe of exynos-dp failed with error -110
> >>>>>>
> >>>>>> I wonder fimd clock has any dependency with DP
> >>>>> FIMD pixel clock is necessary to enable DP.
> >>>> So then, i think DP driver also should control FIMD pixel clock.
> >>>> Do you have any patch or plan for it?
> >>> Um, I don't think so.
> >>> Because, DP cannot work by itself.
> >>> In order to use DP, FIMD should be enabled.
> >>>
> >>> If FIMD is enabled, FIMD pixel clock is enabled;
> >>> therefore, DP driver does not need to control FIMD pixel clock.
> >> Why does DP driver have FIMD driver dependency? Also for this, it needs
> >> FIMD driver is probed earlier then DP driver.  We cannot decide driver
> >> probe order if they are same level drivers and itself is weird
> >> condition.  Although there is hardware dependency, DP and FIMD driver
> >> don't have any code relations. They are each other drivers. But DP
> >> needs FIMD pixel clock and because the clock can be control at the
> >> several drivers and the clock framework exists for that, then i think
> >> it's better DP driver also control FIMD pixel clock.
> >>
> >>> In my opinion, adding config dependency would be better, such as FB_S3C or DRM_EXYNOS_FIMD.
> >> I think this is not solution. How do you ensure FIMD driver is probed
> >> earlier than DP driver? Even if it's possible, when FIMD driver only
> >> controls pixel clock, DP driver will execute any operations regardless
> >> status of FIMD pixel clock, so if FIMD driver turns off pixel clock,
> >> then DP will occur any error.
> > late_initcall can ensure DP driver is probed later.
> 
> I'm not sure late_initcall solution is good. It must choose at the last
> if there isn't other way really.


late_initcall is already used by many drivers.
If you have any good idea, please, suggest it.


> 
> >
> > As you mentioned, DP controller does not work without FIMD controller.
> > Because FIMD controller should provide video data and video clock to DP controller.
> > In this case, adding config dependency would be good.
> >
> > If FIMD driver turns off pixel clock, DP should be turned off too.
> 
> This means DP driver should know FIMD status.
> 
> > Currently, FB FIMD driver turns off pixel clock in remove() and suspend().
> 
> You should also consider blank operation and runtime suspend / resume.


Um, other people can submit this patch, then I will review it.
Please don't urge me to do what.
That's it.


> 
> > So, FIMD driver is enabled, DP will not occur any error on pixel clock.
> >
> 
> Anyway i want to be solved early.
> 
> Thanks.
> --
> 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