On 19 March 2012 13:10, Darius Augulis <augulis.darius@xxxxxxxxx> wrote:
> Hi,
>
> On Mon, Mar 19, 2012 at 9:29 AM, Thomas Abraham
> <thomas.abraham@xxxxxxxxxx> wrote:
>> Hi Darius,
>>
>> On 19 March 2012 05:16, Jingoo Han <jg1.han@xxxxxxxxxxx> wrote:
>> [...]
>>
>>>
>>> Because there are not multiple windows and driver sees only single window as you mentiond,
>>> Only 'struct s3c_fb_pd_win mini6410_fb_win[0]' should be used and
>>> 'struct s3c_fb_pd_win mini6410_fb_win[1]' should not be used.
>>> s3c_fb_pd_win[0] means window 0 of FIMD IP, s3c_fb_pd_win[1] means window 1 of FIMD IP.
>>>
>>> Therefore, if mini6410_fb_win[0] and mini6410_fb_win[1] are included, it means that
>>> 2 windows of FIMD IP are used in single LCD.
>>>
>>> If you want to support multiple LCDs, you should use struct s3c_fb_platdata[0], struct s3c_fb_platdata[1],
>>> because 'struct s3c_fb_platdata' means LCD panel.
>>>
>>> If you understand the purpose of s3c_fb_platdata and s3c_fb_pd_win, you should do, at least, as below.
>>>
>>> static struct s3c_fb_pd_win mini6410_fb_lcd0_win[] = {
>>> {
>>> .win_mode = { /* 4.3" 480x272 */
>>> .left_margin = 3,
>>> .right_margin = 2,
>>> .upper_margin = 1,
>>> .lower_margin = 1,
>>> .hsync_len = 40,
>>> .vsync_len = 1,
>>> .xres = 480,
>>> .yres = 272,
>>> },
>>> .max_bpp = 32,
>>> .default_bpp = 16,
>>> },
>>> };
>>>
>>> static struct s3c_fb_platdata mini6410_lcd_pdata[0] __initdata = {
>>> .setup_gpio = s3c64xx_fb_gpio_setup_24bpp,
>>> .win[0] = &mini6410_fb_lcd0_win[0],
>>> .vidcon0 = VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB,
>>> .vidcon1 = VIDCON1_INV_HSYNC | VIDCON1_INV_VSYNC,
>>> };
>>>
>>> static struct s3c_fb_pd_win mini6410_fb_lcd1_win[] = {
>>> {
>>> .win_mode = { /* 7.0" 800x480 */
>>> .left_margin = 8,
>>> .right_margin = 13,
>>> .upper_margin = 7,
>>> .lower_margin = 5,
>>> .hsync_len = 3,
>>> .vsync_len = 1,
>>> .xres = 800,
>>> .yres = 480,
>>> },
>>> .max_bpp = 32,
>>> .default_bpp = 16,
>>> },
>>> };
>>>
>>> static struct s3c_fb_platdata mini6410_lcd_pdata[1] __initdata = {
>>> .setup_gpio = s3c64xx_fb_gpio_setup_24bpp,
>>> .win[0] = &mini6410_fb_lcd1_win[0],
>>> .vidcon0 = VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB,
>>> .vidcon1 = VIDCON1_INV_HSYNC | VIDCON1_INV_VSYNC,
>>> };
>>>
>>> Each struct means:
>>> mini6410_lcd_pdata[0]: 4.3" 480x272 LCD
>>> mini6410_fb_lcd0_win[0]: window 0 used for 4.3" 480x272 LCD
>>>
>>> mini6410_lcd_pdata[1]: 7.0" 800x480 LCD
>>> mini6410_fb_lcd1_win[0]: window 0 used for 7.0" 800x480 LCD
>>>
>>> In this case, 4.3" or 7.0" LCD can be selected using mini6410_lcd_pdata[], instead of selecting mini6410_fb_win[0] or
>>> mini6410_fb_win[1].
>>>
>>> My point is that 's3c_fb_platdata and s3c_fb_pd_win should be used exactly'.
>>
>> I agree with solution which Jingoo Han has proposed. Instead of
>> selecting one instance of s3c_fb_pd_win over the other at runtime, the
>> features->lcd_index can be used to select one of the instances of
>> platform data for s3c-fb driver.
>
> I agree with it too. Would you make such patch based on your s3c-fb patch set?
> Somebody should make it and definitely not to remove multiple LCD
> suport for these boards.
Sure. I will make a patch for that. I did not notice the way two lcd's
were supported in your code while preparing this patchset. Thanks for
bringing up this point.
>
>>
>> s3c_fb_pd_win is used to describe a window supported by the fimd
>> controller. It is not meant to carry lcd timing information. The lcd
>> panel timing remains the same irrespective of the window sizes. And
>> this patchset is a step towards untangling window information and
>> video timing.
>
> Right now s3c_fb_pd_win contains exactly LCD timing values.
> Your patch will move these timing values to struct fb_videomode.
>
> I could make necessary patch for that too, but I think you will spend
> less time doing it by yourself than syncing with me.
> What do you think?
Yes, I will prepare an additional patch in this series that will
maintain backward compatibility for real6410 and mini6410 with the
approach suggested by Jingoo Han. As I do not have either of these
boards, an ack from you would be very helpful when the next version of
this patchset is posted.
Thanks,
Thomas.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
