> -----Original Message----- > From: Thomas Abraham [mailto:thomas.abraham@xxxxxxxxxx] > Sent: Tuesday, March 06, 2012 3:35 PM > To: Jingoo Han > Cc: linux-fbdev@xxxxxxxxxxxxxxx; FlorianSchandinat@xxxxxx; linux-samsung-soc@xxxxxxxxxxxxxxx; > kgene.kim@xxxxxxxxxxx; ben-linux@xxxxxxxxx; patches@xxxxxxxxxx > Subject: Re: [PATCH 1/3] video: s3c-fb: move video interface timing out of window setup data > > On 6 March 2012 11:52, Jingoo Han <jg1.han@xxxxxxxxxxx> wrote: > > >> -----Original Message----- > >> From: Thomas Abraham [mailto:thomas.abraham@xxxxxxxxxx] > >> Sent: Tuesday, March 06, 2012 2:26 PM > >> To: Jingoo Han > >> Cc: linux-fbdev@xxxxxxxxxxxxxxx; FlorianSchandinat@xxxxxx; linux-samsung-soc@xxxxxxxxxxxxxxx; > >> kgene.kim@xxxxxxxxxxx; ben-linux@xxxxxxxxx; patches@xxxxxxxxxx > >> Subject: Re: [PATCH 1/3] video: s3c-fb: move video interface timing out of window setup data > >> > >> On 6 March 2012 10:15, Jingoo Han <jg1.han@xxxxxxxxxxx> wrote: > >> > >> >> -----Original Message----- > >> >> From: Thomas Abraham [mailto:thomas.abraham@xxxxxxxxxx] > >> >> Sent: Sunday, March 04, 2012 12:50 AM > >> >> To: linux-fbdev@xxxxxxxxxxxxxxx > >> >> Cc: FlorianSchandinat@xxxxxx; linux-samsung-soc@xxxxxxxxxxxxxxx; kgene.kim@xxxxxxxxxxx; > >> >> jg1.han@xxxxxxxxxxx; ben-linux@xxxxxxxxx; patches@xxxxxxxxxx > >> >> 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-linux@xxxxxxxxx> > >> >> Cc: Jingoo Han <jg1.han@xxxxxxxxxxx> > >> >> Signed-off-by: Thomas Abraham <thomas.abraham@xxxxxxxxxx> > >> >> --- > >> >> 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 ? > > > > No, you're right. The i80 is not supported. > > Please ignore my comment. > > > >> > >> > > >> >> * @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. > > > > My point is that variable type 'initmode' can be not changed by using pointer type as follows: > > > > @@ -1243,11 +1243,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; > > In this case, initmode is not pointing to the 'constant' lcd panel > timing values. > > > > > 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,6 +1286,8 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb *sfb, unsigned int win_no, > > } > > > > /* setup the initial video mode from the window */ > > + initmode->xres = windata->xres; > > + initmode->yres = windata->yres; > > And then here, the xres and yres of the 'constant' lcd panel timing > value will be changed. > > But, we don't want to change the actual lcd panel timing value. That > is the reason to first copy the lcd panel timing value into a local > copy and then update the xres and yres and then pass this local copy > of the timing value to fb_videomode_to_var() function. OK. I see. You're right. If you add add it in the resume path in the resume path, you can use my acked-by to this 1st patch. Thank you for sending the patch. Best regards, Jingoo Han > > 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
 |