diff mbox

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

Message ID 1342591053-7092-7-git-send-email-l.krishna@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Leela Krishna Amudala July 18, 2012, 5:57 a.m. UTC
This patch adds the bus clock for FIMD and changes
the device name for lcd clock

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

Comments

Joonyoung Shim July 23, 2012, 8:34 a.m. UTC | #1
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),
> +	}, {
>   		.name		= "iis",
>   		.devname	= "samsung-i2s.1",
>   		.enable		= exynos5_clk_ip_peric_ctrl,
> @@ -1132,7 +1137,7 @@ static struct clksrc_clk exynos5_clksrcs[] = {
>   	}, {
>   		.clk	= {
>   			.name		= "sclk_fimd",
> -			.devname	= "s3cfb.1",
> +			.devname	= "exynos5-fb",
>   			.enable		= exynos5_clksrc_mask_disp1_0_ctrl,
>   			.ctrlbit	= (1 << 0),
>   		},
Joonyoung Shim July 23, 2012, 9:54 a.m. UTC | #2
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

Thanks.
Jingoo Han July 23, 2012, 11:14 p.m. UTC | #3
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.

Best regards,
Jingoo Han

> 
> Thanks.
Joonyoung Shim July 23, 2012, 11:45 p.m. UTC | #4
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?
Jingoo Han July 23, 2012, 11:48 p.m. UTC | #5
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?

I have no plan.

Best regards,
Jingoo Han

> --
> 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
Jingoo Han July 23, 2012, 11:55 p.m. UTC | #6
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.

In my opinion, adding config dependency would be better, such as FB_S3C or DRM_EXYNOS_FIMD.

> --
> 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
Joonyoung Shim July 24, 2012, 1:55 a.m. UTC | #7
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.
Jingoo Han July 24, 2012, 2:15 a.m. UTC | #8
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.

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.
Currently, FB FIMD driver turns off pixel clock in remove() and suspend().
So, FIMD driver is enabled, DP will not occur any error on pixel clock.
Joonyoung Shim July 24, 2012, 3:06 a.m. UTC | #9
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.

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

> So, FIMD driver is enabled, DP will not occur any error on pixel clock.
>

Anyway i want to be solved early.

Thanks.
Jingoo Han July 24, 2012, 4:02 a.m. UTC | #10
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
Hi,

On 07/24/2012 06:02 AM, Jingoo Han wrote:
>>>>>>>> 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.

I agree with Mr. Shim here. using initcall doesn't look like a solution
to this issue. Neither compile time dependency is of much help.

The S3C framebuffer driver can be used as a loadable module, how
would initcalls be supposed to work then ?

Some notify mechanisms or deferred probing could be used, to make sure one
of drivers is initialized before the other.

--

Thanks,
Sylwester
diff mbox

Patch

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),
+	}, {
 		.name		= "iis",
 		.devname	= "samsung-i2s.1",
 		.enable		= exynos5_clk_ip_peric_ctrl,
@@ -1132,7 +1137,7 @@  static struct clksrc_clk exynos5_clksrcs[] = {
 	}, {
 		.clk	= {
 			.name		= "sclk_fimd",
-			.devname	= "s3cfb.1",
+			.devname	= "exynos5-fb",
 			.enable		= exynos5_clksrc_mask_disp1_0_ctrl,
 			.ctrlbit	= (1 << 0),
 		},