[U-Boot] [PATCH V4 3/4] video: Modify exynos_fimd driver to support LCD console

[Minkyu Kang <mk7.kang@samsung.com>]

On 08/01/13 14:31, Ajay kumar wrote:
> Hi  Minkyu,
> 
> On Tue, Jan 8, 2013 at 8:05 AM, Minkyu Kang <mk7.kang@samsung.com> wrote:
>> Dear Ajay Kumar,
>>
>> On 21/12/12 19:35, Ajay Kumar wrote:
>>> Currently, exynos FIMD driver is being used to support only TIZEN LOGOs.
>>> In order to get LCD console, we need to enable half word swap feature
>>> of FIMD and use 16 BPP.
>>> LCD console and proprietary Logo cannot be used simultaneously.
>>> We use logo_on field inside vidinfo_t structure to decide whether
>>> user wants Logo or Console.
>>>
>>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>>> ---
>>
>> Please add changelog here about what you changed since last version.
> I will send V5 soon. Please see my explanation below.
>>>  drivers/video/exynos_fb.c   |    7 +++++++
>>>  drivers/video/exynos_fimd.c |   12 ++++++++----
>>>  2 files changed, 15 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c
>>> index d9a3f9a..ee916be 100644
>>> --- a/drivers/video/exynos_fb.c
>>> +++ b/drivers/video/exynos_fb.c
>>> @@ -45,6 +45,13 @@ short console_row;
>>>
>>>  static unsigned int panel_width, panel_height;
>>>
>>> +#ifndef CONFIG_CMD_BMP
>>> +int bmp_display(ulong addr, int x, int y)
>>> +{
>>> +     return 0;
>>> +}
>>> +#endif
> 
> 
>> It's a common function of bmp command.
>> Please do not redefine such a function.
> 
> I am using CONFIG_CMD_BMP to differentiate between LCD Logo and LCD console.
> I select CONFIG_CMD_BMP only when I need Logo, and
> I will not select CONFIG_CMD_BMP when I need console.
> Lets consider that we want console. So we don't define CONFIG_CMD_BMP now.
> And, exynos_fb.c has linking time dependency on "bmp_display", which
> is not defined!
> In such a case, I am left with 2 options:
> 1) Place an #ifdef CONFIG_CMD_BMP inside exynos_fb, where it makes a
> call to "bmp_display".
> 2) Redefine "bmp_display" to do nothing inside a header file(That's
> how it is done in this patch!)
> Kindly let me know if you have a better way to do this.
> 

I think, 1) is better.

Thanks.
Minkyu Kang.