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.
>
> 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?
regards,
Darius
--
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
