On 6 March 2012 10:15, Jingoo Han <jg1....@samsung.com> wrote:
>> -----Original Message-----
>> From: Thomas Abraham [mailto:thomas.abra...@linaro.org]
>> Sent: Sunday, March 04, 2012 12:50 AM
>> To: linux-fb...@vger.kernel.org
>> Cc: florianschandi...@gmx.de; linux-samsung-soc@vger.kernel.org;
>> kgene....@samsung.com;
>> jg1....@samsung.com; ben-li...@fluff.org; patc...@linaro.org
>> Subject: [PATCH 1/3] video: s3c-fb: move video interface timing out of
>> window setup data
>>
>> The video interface timing is independent of the window setup data.
>> The resolution of the window can be smaller than that of the lcd
>> panel to which the video data is output.
>>
>> So move the video timing data from the per-window setup data to the
>> platform specific section in the platform data. This also removes
>> the restriction that atleast one window should have the same
>> resolution as that of the panel attached.
>>
>> Cc: Ben Dooks <ben-li...@fluff.org>
>> Cc: Jingoo Han <jg1....@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.abra...@linaro.org>
>> ---
>> arch/arm/plat-samsung/include/plat/fb.h | 9 ++-
>> drivers/video/s3c-fb.c | 106
>> +++++++++++++++++--------------
>> 2 files changed, 63 insertions(+), 52 deletions(-)
>>
>> diff --git a/arch/arm/plat-samsung/include/plat/fb.h
>> b/arch/arm/plat-samsung/include/plat/fb.h
>> index 0fedf47..39d6bd7 100644
>> --- a/arch/arm/plat-samsung/include/plat/fb.h
>> +++ b/arch/arm/plat-samsung/include/plat/fb.h
>> @@ -24,15 +24,16 @@
>>
>> /**
>> * struct s3c_fb_pd_win - per window setup data
>> - * @win_mode: The display parameters to initialise (not for window 0)
>> + * @xres : The window X size.
>> + * @yres : The window Y size.
>> * @virtual_x: The virtual X size.
>> * @virtual_y: The virtual Y size.
>> */
>> struct s3c_fb_pd_win {
>> - struct fb_videomode win_mode;
>> -
>> unsigned short default_bpp;
>> unsigned short max_bpp;
>> + unsigned short xres;
>> + unsigned short yres;
>> unsigned short virtual_x;
>> unsigned short virtual_y;
>> };
>> @@ -45,6 +46,7 @@ struct s3c_fb_pd_win {
>> * @default_win: default window layer number to be used for UI layer.
>> * @vidcon0: The base vidcon0 values to control the panel data format.
>> * @vidcon1: The base vidcon1 values to control the panel data output.
>> + * @vtiming: Video timing when connected to a RGB type panel.
>
> fb_videomode can be set, even if it is not RGB type panel.
> In my opinion, it would be better.
> + * @vtiming: The video timing values to set the interface timing of the
> panel.
The other interface that is supported is the i80 interface. Can these
timing values be used for i80 interface as well ?
>
>> * @win: The setup data for each hardware window, or NULL for unused.
>> * @display_mode: The LCD output display mode.
>> *
>> @@ -58,6 +60,7 @@ struct s3c_fb_platdata {
>> void (*setup_gpio)(void);
>>
>> struct s3c_fb_pd_win *win[S3C_FB_MAX_WIN];
>> + struct fb_videomode *vtiming;
>>
>> u32 default_win;
>>
>> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
>> index 1fb7ddf..8e05d4d 100644
>> --- a/drivers/video/s3c-fb.c
>> +++ b/drivers/video/s3c-fb.c
>> @@ -495,7 +495,6 @@ static int s3c_fb_set_par(struct fb_info *info)
>> u32 alpha = 0;
>> u32 data;
>> u32 pagewidth;
>> - int clkdiv;
>>
>> dev_dbg(sfb->dev, "setting framebuffer parameters\n");
>>
>> @@ -532,46 +531,9 @@ static int s3c_fb_set_par(struct fb_info *info)
>> /* disable the window whilst we update it */
>> writel(0, regs + WINCON(win_no));
>>
>> - /* use platform specified window as the basis for the lcd timings */
>> -
>> - if (win_no == sfb->pdata->default_win) {
>> - clkdiv = s3c_fb_calc_pixclk(sfb, var->pixclock);
>> -
>> - data = sfb->pdata->vidcon0;
>> - data &= ~(VIDCON0_CLKVAL_F_MASK | VIDCON0_CLKDIR);
>> -
>> - if (clkdiv > 1)
>> - data |= VIDCON0_CLKVAL_F(clkdiv-1) | VIDCON0_CLKDIR;
>> - else
>> - data &= ~VIDCON0_CLKDIR; /* 1:1 clock */
>> -
>> - /* write the timing data to the panel */
>> -
>> - if (sfb->variant.is_2443)
>> - data |= (1 << 5);
>> -
>> - writel(data, regs + VIDCON0);
>> -
>> + if (win_no == sfb->pdata->default_win)
>> s3c_fb_enable(sfb, 1);
>>
>> - data = VIDTCON0_VBPD(var->upper_margin - 1) |
>> - VIDTCON0_VFPD(var->lower_margin - 1) |
>> - VIDTCON0_VSPW(var->vsync_len - 1);
>> -
>> - writel(data, regs + sfb->variant.vidtcon);
>> -
>> - data = VIDTCON1_HBPD(var->left_margin - 1) |
>> - VIDTCON1_HFPD(var->right_margin - 1) |
>> - VIDTCON1_HSPW(var->hsync_len - 1);
>> -
>> - /* VIDTCON1 */
>> - writel(data, regs + sfb->variant.vidtcon + 4);
>> -
>> - data = VIDTCON2_LINEVAL(var->yres - 1) |
>> - VIDTCON2_HOZVAL(var->xres - 1);
>> - writel(data, regs + sfb->variant.vidtcon + 8);
>> - }
>> -
>
> It looks good.
> VIDTCON registers don't need to be written whenever s3c_fb_set_par is called.
>
>> /* write the buffer address */
>>
>> /* start and end registers stride is 8 */
>> @@ -1136,11 +1098,11 @@ static int __devinit s3c_fb_alloc_memory(struct
>> s3c_fb *sfb,
>>
>> dev_dbg(sfb->dev, "allocating memory for display\n");
>>
>> - real_size = windata->win_mode.xres * windata->win_mode.yres;
>> + real_size = windata->xres * windata->yres;
>> virt_size = windata->virtual_x * windata->virtual_y;
>>
>> dev_dbg(sfb->dev, "real_size=%u (%u.%u), virt_size=%u (%u.%u)\n",
>> - real_size, windata->win_mode.xres, windata->win_mode.yres,
>> + real_size, windata->xres, windata->yres,
>> virt_size, windata->virtual_x, windata->virtual_y);
>>
>> size = (real_size > virt_size) ? real_size : virt_size;
>> @@ -1222,7 +1184,7 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb
>> *sfb, unsigned int win_no,
>> struct s3c_fb_win **res)
>> {
>> struct fb_var_screeninfo *var;
>> - struct fb_videomode *initmode;
>> + struct fb_videomode initmode;
>
> *initmode cannot be used???
> Can you tell me why pointer type should be changed?
>
The initmode is used to pass video timing to the fb_videomode_to_var()
function. Each window setup data in platform data included video
timing information. Since, the video timing data is now moved out of
per-window data, the xres and yres values have to be setup based on
the window for which the fb_videomode_to_var() is called. Hence, the
common timing values is first copied into initmode and then the window
specific xres and yres are set. If initmode is a maintained as a
pointer (to the video timing data in platform data), then any xres and
yres update to initmode would overwrite the 'constant' video timing.
>> struct s3c_fb_pd_win *windata;
>> struct s3c_fb_win *win;
>> struct fb_info *fbinfo;
>> @@ -1243,11 +1205,11 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb
>> *sfb, unsigned int win_no,
>> }
>>
>> windata = sfb->pdata->win[win_no];
>> - initmode = &windata->win_mode;
>> + initmode = *sfb->pdata->vtiming;
>>
>> WARN_ON(windata->max_bpp == 0);
>> - WARN_ON(windata->win_mode.xres == 0);
>> - WARN_ON(windata->win_mode.yres == 0);
>> + WARN_ON(windata->xres == 0);
>> + WARN_ON(windata->yres == 0);
>>
>> win = fbinfo->par;
>> *res = win;
>> @@ -1286,7 +1248,9 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb
>> *sfb, unsigned int win_no,
>> }
>>
>> /* setup the initial video mode from the window */
>> - fb_videomode_to_var(&fbinfo->var, initmode);
>> + initmode.xres = windata->xres;
>> + initmode.yres = windata->yres;
Here, the xres and yres values are copied to initmode and described above.
>> + fb_videomode_to_var(&fbinfo->var, &initmode);
>>
>> fbinfo->fix.type = FB_TYPE_PACKED_PIXELS;
>> fbinfo->fix.accel = FB_ACCEL_NONE;
>> @@ -1331,6 +1295,51 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb
>> *sfb, unsigned int win_no,
>> }
>>
>> /**
>> + * s3c_fb_set_rgb_timing() - set video timing for rgb interface.
>> + * @sfb: The base resources for the hardware.
>> + *
>> + * Set horizontal and vertical lcd rgb interface timing.
>> + */
>> +static void s3c_fb_set_rgb_timing(struct s3c_fb *sfb)
>> +{
>> + struct fb_videomode *vmode = sfb->pdata->vtiming;
>> + void __iomem *regs = sfb->regs;
>> + int clkdiv;
>> + u32 data;
>> +
>> + if (!vmode->pixclock)
>> + s3c_fb_missing_pixclock(vmode);
>> +
>> + clkdiv = s3c_fb_calc_pixclk(sfb, vmode->pixclock);
>> +
>> + data = sfb->pdata->vidcon0;
>> + data &= ~(VIDCON0_CLKVAL_F_MASK | VIDCON0_CLKDIR);
>> +
>> + if (clkdiv > 1)
>> + data |= VIDCON0_CLKVAL_F(clkdiv-1) | VIDCON0_CLKDIR;
>> + else
>> + data &= ~VIDCON0_CLKDIR; /* 1:1 clock */
>> +
>> + if (sfb->variant.is_2443)
>> + data |= (1 << 5);
>> + writel(data, regs + VIDCON0);
>> +
>> + data = VIDTCON0_VBPD(vmode->upper_margin - 1) |
>> + VIDTCON0_VFPD(vmode->lower_margin - 1) |
>> + VIDTCON0_VSPW(vmode->vsync_len - 1);
>> + writel(data, regs + sfb->variant.vidtcon);
>> +
>> + data = VIDTCON1_HBPD(vmode->left_margin - 1) |
>> + VIDTCON1_HFPD(vmode->right_margin - 1) |
>> + VIDTCON1_HSPW(vmode->hsync_len - 1);
>> + writel(data, regs + sfb->variant.vidtcon + 4);
>> +
>> + data = VIDTCON2_LINEVAL(vmode->yres - 1) |
>> + VIDTCON2_HOZVAL(vmode->xres - 1);
>> + writel(data, regs + sfb->variant.vidtcon + 8);
>> +}
>> +
>> +/**
>> * s3c_fb_clear_win() - clear hardware window registers.
>> * @sfb: The base resources for the hardware.
>> * @win: The window to process.
>> @@ -1473,15 +1482,14 @@ static int __devinit s3c_fb_probe(struct
>> platform_device *pdev)
>> writel(0xffffff, regs + WKEYCON1);
>> }
>>
>> + s3c_fb_set_rgb_timing(sfb);
>> +
>
> This s3c_fb_set_rgb_timing(sfb) should be added to s3c_fb_resume().
> The timing registers should be set when s3c_fb_resume() is called.
> If not, after resuming, timing registers such as VIDTCONx will not bet set.
Yes, I missed that. I will add it in the resume path.
Thanks for you comments. If you could let me know if i80 video timing
values can be passed with 'struct fb_videomode', I will do the changes
as you have suggested and quickly repost this patchset.
Thanks,
Thomas.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
