diff mbox

[V2] Origen: Set FIMD as the default display path

Message ID 1376560411-2798-1-git-send-email-tushar.behera@linaro.org
State New
Headers show

Commit Message

Tushar Behera Aug. 15, 2013, 9:53 a.m. UTC
On EXYNOS4210, there are three paths for display data to be processed,
namely MIE, MDNIE and FIMD. On Origen board, FIMD display controller
is used.

Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
Changes for V2:
* Updated review comments from Ajay Kumar, reusing the code from
arch/arm/cpu/armv7/exynos/system.c.

 board/samsung/origen/origen.c |    3 +++
 1 file changed, 3 insertions(+)

Comments

Ajay kumar Aug. 16, 2013, 7:31 a.m. UTC | #1
On Thu, Aug 15, 2013 at 6:53 PM, Tushar Behera <tushar.behera@linaro.org>wrote:

> On EXYNOS4210, there are three paths for display data to be processed,
> namely MIE, MDNIE and FIMD. On Origen board, FIMD display controller
> is used.
>
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
> Changes for V2:
> * Updated review comments from Ajay Kumar, reusing the code from
> arch/arm/cpu/armv7/exynos/system.c.
>
>  board/samsung/origen/origen.c |    3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/board/samsung/origen/origen.c b/board/samsung/origen/origen.c
> index 15f77ca..bb16699 100644
> --- a/board/samsung/origen/origen.c
> +++ b/board/samsung/origen/origen.c
> @@ -22,6 +22,9 @@ int board_init(void)
>         gpio2 = (struct exynos4_gpio_part2 *) EXYNOS4_GPIO_PART2_BASE;
>
>         gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);
> +
> +       set_system_display_ctrl();
> +
>
Ok. You are trying to get the display up in the kernel,
without enabling the display in u-boot.
And, thats why you are making an explicit call here?

        return 0;
>  }
>
> --
> 1.7.9.5
>
>
Donghwa Lee Aug. 19, 2013, 1:52 a.m. UTC | #2
On Fri, Aug 16, 2013 at 16:31, Ajay kumar wrote:
> On Thu, Aug 15, 2013 at 6:53 PM, Tushar Behera <tushar.behera@linaro.org <mailto:tushar.behera@linaro.org>> wrote:
>
>     On EXYNOS4210, there are three paths for display data to be processed,
>     namely MIE, MDNIE and FIMD. On Origen board, FIMD display controller
>     is used.
>
>     Signed-off-by: Tushar Behera <tushar.behera@linaro.org <mailto:tushar.behera@linaro.org>>
>     ---
>     Changes for V2:
>     * Updated review comments from Ajay Kumar, reusing the code from
>     arch/arm/cpu/armv7/exynos/system.c.
>
>      board/samsung/origen/origen.c |    3 +++
>      1 file changed, 3 insertions(+)
>
>     diff --git a/board/samsung/origen/origen.c b/board/samsung/origen/origen.c
>     index 15f77ca..bb16699 100644
>     --- a/board/samsung/origen/origen.c
>     +++ b/board/samsung/origen/origen.c
>     @@ -22,6 +22,9 @@ int board_init(void)
>             gpio2 = (struct exynos4_gpio_part2 *) EXYNOS4_GPIO_PART2_BASE;
>
>             gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);
>     +
>     +       set_system_display_ctrl();
>     +
>
> Ok. You are trying to get the display up in the kernel,
> without enabling the display in u-boot.
> And, thats why you are making an explicit call here?
>
Previously, Ajay kumar said set_system_display_ctrl() function is already called by
lcd_ctrl_init() of exynos_fb.c.
If you need to use FIMD path, you'd better use the existing one.
>
>             return 0;
>      }
>
>     --
>     1.7.9.5
>
>
Tushar Behera Aug. 19, 2013, 3:06 a.m. UTC | #3
On 19 August 2013 07:22, Donghwa Lee <dh09.lee@samsung.com> wrote:
> On Fri, Aug 16, 2013 at 16:31, Ajay kumar wrote:
>
> On Thu, Aug 15, 2013 at 6:53 PM, Tushar Behera <tushar.behera@linaro.org>
> wrote:
>>
>> On EXYNOS4210, there are three paths for display data to be processed,
>> namely MIE, MDNIE and FIMD. On Origen board, FIMD display controller
>> is used.
>>
>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>> ---
>> Changes for V2:
>> * Updated review comments from Ajay Kumar, reusing the code from
>> arch/arm/cpu/armv7/exynos/system.c.
>>
>>  board/samsung/origen/origen.c |    3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/board/samsung/origen/origen.c b/board/samsung/origen/origen.c
>> index 15f77ca..bb16699 100644
>> --- a/board/samsung/origen/origen.c
>> +++ b/board/samsung/origen/origen.c
>> @@ -22,6 +22,9 @@ int board_init(void)
>>         gpio2 = (struct exynos4_gpio_part2 *) EXYNOS4_GPIO_PART2_BASE;
>>
>>         gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);
>> +
>> +       set_system_display_ctrl();
>> +
>
> Ok. You are trying to get the display up in the kernel,
> without enabling the display in u-boot.
> And, thats why you are making an explicit call here?
>

Yes, as of now, I don't want LCD support in u-boot, but I want to just
enable the display path.

> Previously, Ajay kumar said set_system_display_ctrl() function is already
> called by
> lcd_ctrl_init() of exynos_fb.c.
> If you need to use FIMD path, you'd better use the existing one.

I tried enabling CONFIG_LCD and related config options so that
existing display path in u-boot is called. That somehow stalls the
u-boot and I didn't want to pursue that much as calling this function
fixes my issue.

>>
>>         return 0;
>>  }
>>
>> --
>> 1.7.9.5
>>
>
>
Donghwa Lee Aug. 19, 2013, 4:31 a.m. UTC | #4
On 2013년 08월 19일 12:06, Tushar Behera wrote:
> On 19 August 2013 07:22, Donghwa Lee <dh09.lee@samsung.com> wrote:
>> On Fri, Aug 16, 2013 at 16:31, Ajay kumar wrote:
>>
>> On Thu, Aug 15, 2013 at 6:53 PM, Tushar Behera <tushar.behera@linaro.org>
>> wrote:
>>> On EXYNOS4210, there are three paths for display data to be processed,
>>> namely MIE, MDNIE and FIMD. On Origen board, FIMD display controller
>>> is used.
>>>
>>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>>> ---
>>> Changes for V2:
>>> * Updated review comments from Ajay Kumar, reusing the code from
>>> arch/arm/cpu/armv7/exynos/system.c.
>>>
>>>  board/samsung/origen/origen.c |    3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/board/samsung/origen/origen.c b/board/samsung/origen/origen.c
>>> index 15f77ca..bb16699 100644
>>> --- a/board/samsung/origen/origen.c
>>> +++ b/board/samsung/origen/origen.c
>>> @@ -22,6 +22,9 @@ int board_init(void)
>>>         gpio2 = (struct exynos4_gpio_part2 *) EXYNOS4_GPIO_PART2_BASE;
>>>
>>>         gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);
>>> +
>>> +       set_system_display_ctrl();
>>> +
>> Ok. You are trying to get the display up in the kernel,
>> without enabling the display in u-boot.
>> And, thats why you are making an explicit call here?
>>
> Yes, as of now, I don't want LCD support in u-boot, but I want to just
> enable the display path.
>

If you don't need to LCD support in u-boot, which case do you need to enable the display path to FIMD?

>> Previously, Ajay kumar said set_system_display_ctrl() function is already
>> called by
>> lcd_ctrl_init() of exynos_fb.c.
>> If you need to use FIMD path, you'd better use the existing one.
> I tried enabling CONFIG_LCD and related config options so that
> existing display path in u-boot is called. That somehow stalls the
> u-boot and I didn't want to pursue that much as calling this function
> fixes my issue.
>
>>>         return 0;
>>>  }
>>>
>>> --
>>> 1.7.9.5
>>>
>>
>
>
Tushar Behera Aug. 19, 2013, 6 a.m. UTC | #5
On 19 August 2013 10:01, Donghwa Lee <dh09.lee@samsung.com> wrote:
> On 2013년 08월 19일 12:06, Tushar Behera wrote:
>> On 19 August 2013 07:22, Donghwa Lee <dh09.lee@samsung.com> wrote:
>>> On Fri, Aug 16, 2013 at 16:31, Ajay kumar wrote:
>>>
>>> On Thu, Aug 15, 2013 at 6:53 PM, Tushar Behera <tushar.behera@linaro.org>
>>> wrote:
>>>> On EXYNOS4210, there are three paths for display data to be processed,
>>>> namely MIE, MDNIE and FIMD. On Origen board, FIMD display controller
>>>> is used.
>>>>
>>>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>>>> ---
>>>> Changes for V2:
>>>> * Updated review comments from Ajay Kumar, reusing the code from
>>>> arch/arm/cpu/armv7/exynos/system.c.
>>>>
>>>>  board/samsung/origen/origen.c |    3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/board/samsung/origen/origen.c b/board/samsung/origen/origen.c
>>>> index 15f77ca..bb16699 100644
>>>> --- a/board/samsung/origen/origen.c
>>>> +++ b/board/samsung/origen/origen.c
>>>> @@ -22,6 +22,9 @@ int board_init(void)
>>>>         gpio2 = (struct exynos4_gpio_part2 *) EXYNOS4_GPIO_PART2_BASE;
>>>>
>>>>         gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);
>>>> +
>>>> +       set_system_display_ctrl();
>>>> +
>>> Ok. You are trying to get the display up in the kernel,
>>> without enabling the display in u-boot.
>>> And, thats why you are making an explicit call here?
>>>
>> Yes, as of now, I don't want LCD support in u-boot, but I want to just
>> enable the display path.
>>
>
> If you don't need to LCD support in u-boot, which case do you need to enable the display path to FIMD?
>

This modification is essential for enabling LCD support in kernel.
While working with DT kernel, we couldn't any suitable place in kernel
where we can place this code. That is the reason why we thought of
having this bit setup in u-boot.

>>> Previously, Ajay kumar said set_system_display_ctrl() function is already
>>> called by
>>> lcd_ctrl_init() of exynos_fb.c.
>>> If you need to use FIMD path, you'd better use the existing one.
>> I tried enabling CONFIG_LCD and related config options so that
>> existing display path in u-boot is called. That somehow stalls the
>> u-boot and I didn't want to pursue that much as calling this function
>> fixes my issue.
>>
>>>>         return 0;
>>>>  }
>>>>
>>>> --
>>>> 1.7.9.5
>>>>
>>>
>>
>>
>
Tushar Behera Aug. 28, 2013, 11:47 a.m. UTC | #6
On 19 August 2013 11:30, Tushar Behera <tushar.behera@linaro.org> wrote:
> On 19 August 2013 10:01, Donghwa Lee <dh09.lee@samsung.com> wrote:
>> On 2013년 08월 19일 12:06, Tushar Behera wrote:
>>> On 19 August 2013 07:22, Donghwa Lee <dh09.lee@samsung.com> wrote:
>>>> On Fri, Aug 16, 2013 at 16:31, Ajay kumar wrote:
>>>>
>>>> On Thu, Aug 15, 2013 at 6:53 PM, Tushar Behera <tushar.behera@linaro.org>
>>>> wrote:
>>>>> On EXYNOS4210, there are three paths for display data to be processed,
>>>>> namely MIE, MDNIE and FIMD. On Origen board, FIMD display controller
>>>>> is used.
>>>>>
>>>>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>>>>> ---
>>>>> Changes for V2:
>>>>> * Updated review comments from Ajay Kumar, reusing the code from
>>>>> arch/arm/cpu/armv7/exynos/system.c.
>>>>>
>>>>>  board/samsung/origen/origen.c |    3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/board/samsung/origen/origen.c b/board/samsung/origen/origen.c
>>>>> index 15f77ca..bb16699 100644
>>>>> --- a/board/samsung/origen/origen.c
>>>>> +++ b/board/samsung/origen/origen.c
>>>>> @@ -22,6 +22,9 @@ int board_init(void)
>>>>>         gpio2 = (struct exynos4_gpio_part2 *) EXYNOS4_GPIO_PART2_BASE;
>>>>>
>>>>>         gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);
>>>>> +
>>>>> +       set_system_display_ctrl();
>>>>> +
>>>> Ok. You are trying to get the display up in the kernel,
>>>> without enabling the display in u-boot.
>>>> And, thats why you are making an explicit call here?
>>>>
>>> Yes, as of now, I don't want LCD support in u-boot, but I want to just
>>> enable the display path.
>>>
>>
>> If you don't need to LCD support in u-boot, which case do you need to enable the display path to FIMD?
>>
>
> This modification is essential for enabling LCD support in kernel.
> While working with DT kernel, we couldn't any suitable place in kernel
> where we can place this code. That is the reason why we thought of
> having this bit setup in u-boot.
>

Can we go ahead with this patch?
Minkyu Kang Aug. 30, 2013, 5:07 a.m. UTC | #7
On 28/08/13 20:47, Tushar Behera wrote:
> On 19 August 2013 11:30, Tushar Behera <tushar.behera@linaro.org> wrote:
>> On 19 August 2013 10:01, Donghwa Lee <dh09.lee@samsung.com> wrote:
>>> On 2013년 08월 19일 12:06, Tushar Behera wrote:
>>>> On 19 August 2013 07:22, Donghwa Lee <dh09.lee@samsung.com> wrote:
>>>>> On Fri, Aug 16, 2013 at 16:31, Ajay kumar wrote:
>>>>>
>>>>> On Thu, Aug 15, 2013 at 6:53 PM, Tushar Behera <tushar.behera@linaro.org>
>>>>> wrote:
>>>>>> On EXYNOS4210, there are three paths for display data to be processed,
>>>>>> namely MIE, MDNIE and FIMD. On Origen board, FIMD display controller
>>>>>> is used.
>>>>>>
>>>>>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>>>>>> ---
>>>>>> Changes for V2:
>>>>>> * Updated review comments from Ajay Kumar, reusing the code from
>>>>>> arch/arm/cpu/armv7/exynos/system.c.
>>>>>>
>>>>>>  board/samsung/origen/origen.c |    3 +++
>>>>>>  1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/board/samsung/origen/origen.c b/board/samsung/origen/origen.c
>>>>>> index 15f77ca..bb16699 100644
>>>>>> --- a/board/samsung/origen/origen.c
>>>>>> +++ b/board/samsung/origen/origen.c
>>>>>> @@ -22,6 +22,9 @@ int board_init(void)
>>>>>>         gpio2 = (struct exynos4_gpio_part2 *) EXYNOS4_GPIO_PART2_BASE;
>>>>>>
>>>>>>         gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);
>>>>>> +
>>>>>> +       set_system_display_ctrl();
>>>>>> +
>>>>> Ok. You are trying to get the display up in the kernel,
>>>>> without enabling the display in u-boot.
>>>>> And, thats why you are making an explicit call here?
>>>>>
>>>> Yes, as of now, I don't want LCD support in u-boot, but I want to just
>>>> enable the display path.
>>>>
>>>
>>> If you don't need to LCD support in u-boot, which case do you need to enable the display path to FIMD?
>>>
>>
>> This modification is essential for enabling LCD support in kernel.
>> While working with DT kernel, we couldn't any suitable place in kernel
>> where we can place this code. That is the reason why we thought of
>> having this bit setup in u-boot.
>>

I understood your concept.
but I feel, it's a workaround.

you said.
"
I tried enabling CONFIG_LCD and related config options so that
existing display path in u-boot is called. That somehow stalls the
u-boot and I didn't want to pursue that much as calling this function
fixes my issue.
"

Why don't you try to find the reason why stalls the u-boot.

Thanks,
Minkyu Kang.
Tushar Behera Aug. 30, 2013, 6:34 a.m. UTC | #8
On 30 August 2013 10:37, Minkyu Kang <mk7.kang@samsung.com> wrote:
> On 28/08/13 20:47, Tushar Behera wrote:
>> On 19 August 2013 11:30, Tushar Behera <tushar.behera@linaro.org> wrote:
>>> On 19 August 2013 10:01, Donghwa Lee <dh09.lee@samsung.com> wrote:
>>>> On 2013년 08월 19일 12:06, Tushar Behera wrote:
>>>>> On 19 August 2013 07:22, Donghwa Lee <dh09.lee@samsung.com> wrote:
>>>>>> On Fri, Aug 16, 2013 at 16:31, Ajay kumar wrote:
>>>>>>
>>>>>> On Thu, Aug 15, 2013 at 6:53 PM, Tushar Behera <tushar.behera@linaro.org>
>>>>>> wrote:
>>>>>>> On EXYNOS4210, there are three paths for display data to be processed,
>>>>>>> namely MIE, MDNIE and FIMD. On Origen board, FIMD display controller
>>>>>>> is used.
>>>>>>>
>>>>>>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>>>>>>> ---
>>>>>>> Changes for V2:
>>>>>>> * Updated review comments from Ajay Kumar, reusing the code from
>>>>>>> arch/arm/cpu/armv7/exynos/system.c.
>>>>>>>
>>>>>>>  board/samsung/origen/origen.c |    3 +++
>>>>>>>  1 file changed, 3 insertions(+)
>>>>>>>
>>>>>>> diff --git a/board/samsung/origen/origen.c b/board/samsung/origen/origen.c
>>>>>>> index 15f77ca..bb16699 100644
>>>>>>> --- a/board/samsung/origen/origen.c
>>>>>>> +++ b/board/samsung/origen/origen.c
>>>>>>> @@ -22,6 +22,9 @@ int board_init(void)
>>>>>>>         gpio2 = (struct exynos4_gpio_part2 *) EXYNOS4_GPIO_PART2_BASE;
>>>>>>>
>>>>>>>         gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);
>>>>>>> +
>>>>>>> +       set_system_display_ctrl();
>>>>>>> +
>>>>>> Ok. You are trying to get the display up in the kernel,
>>>>>> without enabling the display in u-boot.
>>>>>> And, thats why you are making an explicit call here?
>>>>>>
>>>>> Yes, as of now, I don't want LCD support in u-boot, but I want to just
>>>>> enable the display path.
>>>>>
>>>>
>>>> If you don't need to LCD support in u-boot, which case do you need to enable the display path to FIMD?
>>>>
>>>
>>> This modification is essential for enabling LCD support in kernel.
>>> While working with DT kernel, we couldn't any suitable place in kernel
>>> where we can place this code. That is the reason why we thought of
>>> having this bit setup in u-boot.
>>>
>
> I understood your concept.
> but I feel, it's a workaround.
>
> you said.
> "
> I tried enabling CONFIG_LCD and related config options so that
> existing display path in u-boot is called. That somehow stalls the
> u-boot and I didn't want to pursue that much as calling this function
> fixes my issue.
> "
>
> Why don't you try to find the reason why stalls the u-boot.
>

I spent some time on fixing this while I was working on this patch,
but couldn't debug the issue within the available time. If I can find
sometime for this, I will surely look into fixing this.

We have already got display support merged with upstream kernel. After
applying this patch to u-boot, developers can get display up with
upstream stuff. That was the motivation for having this patch.
diff mbox

Patch

diff --git a/board/samsung/origen/origen.c b/board/samsung/origen/origen.c
index 15f77ca..bb16699 100644
--- a/board/samsung/origen/origen.c
+++ b/board/samsung/origen/origen.c
@@ -22,6 +22,9 @@  int board_init(void)
 	gpio2 = (struct exynos4_gpio_part2 *) EXYNOS4_GPIO_PART2_BASE;
 
 	gd->bd->bi_boot_params = (PHYS_SDRAM_1 + 0x100UL);
+
+	set_system_display_ctrl();
+
 	return 0;
 }