# RE: \[PATCH 2/2\] video: s3c-fb: Modify s3c-fb driver to support window positioning - List: linux-fbdev - 2 messages, 2 authors - Page: https://ratatoskr.run/linux-fbdev/2011/08/9560260/t ## RE: \[PATCH 2/2\] video: s3c-fb: Modify s3c-fb driver to support window positioning - From: JinGoo Han \ - Date: 2011-08-26 00:44:10 - Message: https://ratatoskr.run/linux-fbdev/2011/08/9560260.md ```text Hi, Ajay. > -----Original Message----- > From: Ajay Kumar [mailto:] > Sent: Friday, August 26, 2011 4:52 AM > To: ; ; linux- > arm-kernel@lists.infradead.org > Cc: FlorianSchandinat@gmx.de; ; ; > m.szyprowski@samsung.com; ben-linux@fluff.org; banajit.g@samsung.com > Subject: [PATCH 2/2] video: s3c-fb: Modify s3c-fb driver to support window > positioning > > This patch modifies the existing s3c-fb driver to provide > flexibility to the user to reposition the framebuffer windows. > > Signed-off-by: Ajay Kumar > Signed-off-by: Banajit Goswami > --- > drivers/video/s3c-fb.c | 37 ++++++++++++++++++++++++++++++++----- > 1 files changed, 32 insertions(+), 5 deletions(-) > > diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c > index 0fda252..41179d7 100644 > --- a/drivers/video/s3c-fb.c > +++ b/drivers/video/s3c-fb.c > @@ -442,6 +442,7 @@ static int s3c_fb_set_par(struct fb_info *info) > struct fb_var_screeninfo *var = &info->var; > struct s3c_fb_win *win = info->par; > struct s3c_fb *sfb = win->parent; > + struct s3cfb_window_pos *winpos = &win->windata->winpos; > void __iomem *regs = sfb->regs; > void __iomem *buf = regs; > int win_no = win->index; > @@ -539,12 +540,13 @@ static int s3c_fb_set_par(struct fb_info *info) > > /* write 'OSD' registers to control position of framebuffer */ > > - data = VIDOSDxA_TOPLEFT_X(0) | VIDOSDxA_TOPLEFT_Y(0); > + data = VIDOSDxA_TOPLEFT_X(winpos->win_pos_x) | > + VIDOSDxA_TOPLEFT_Y(winpos->win_pos_y); > writel(data, regs + VIDOSD_A(win_no, sfb->variant)); > > - data = VIDOSDxB_BOTRIGHT_X(s3c_fb_align_word(var->bits_per_pixel, > - var->xres - 1)) | > - VIDOSDxB_BOTRIGHT_Y(var->yres - 1); > + data = VIDOSDxB_BOTRIGHT_X((s3c_fb_align_word(var->bits_per_pixel, > + (winpos->win_pos_x + var->xres - 1)))) | > + VIDOSDxB_BOTRIGHT_Y((winpos->win_pos_y + var->yres - 1)); > > writel(data, regs + VIDOSD_B(win_no, sfb->variant)); > > @@ -999,8 +1001,10 @@ static int s3c_fb_ioctl(struct fb_info *info, > unsigned int cmd, > { > struct s3c_fb_win *win = info->par; > struct s3c_fb *sfb = win->parent; > - int ret; > + struct s3cfb_window_pos *winpos = &win->windata->winpos; > + int ret = 0; > u32 crtc; > + u32 data; > > switch (cmd) { > case FBIO_WAITFORVSYNC: > @@ -1011,6 +1015,29 @@ static int s3c_fb_ioctl(struct fb_info *info, > unsigned int cmd, > > ret = s3c_fb_wait_for_vsync(sfb, crtc); > break; > + case S3CFB_WIN_POSITION: > + if (copy_from_user(winpos, (u32 __user *)arg, > + sizeof(struct s3cfb_window_pos))) { > + ret = -EFAULT; > + break; > + } > + > + shadow_protect_win(win, 1); > + > + /* write 'OSD' registers to set position of the window */ > + data = VIDOSDxA_TOPLEFT_X(winpos->win_pos_x) | > + VIDOSDxA_TOPLEFT_Y(winpos->win_pos_y); > + writel(data, sfb->regs + VIDOSD_A(win->index, sfb->variant)); > + > + data = VIDOSDxB_BOTRIGHT_X( > + s3c_fb_align_word(info->var.bits_per_pixel, > + (winpos->win_pos_x + info->var.xres - 1))); > + data |= VIDOSDxB_BOTRIGHT_Y(winpos->win_pos_y + > + info->var.yres - 1); > + writel(data, sfb->regs + VIDOSD_B(win->index, sfb->variant)); > + > + shadow_protect_win(win, 0); > + break; Can you move this to separate function? In my opinion, it would be better for readability and consistency. Also, FBIO_WAITFORVSYNC has been implemented in this way. ret = s3c_fb_wait_for_vsync(sfb, crtc); How about adding a function 's3c_fb_set_window_position' for this as belows? +int s3c_fb_set_window_position(struct fb_info *info, + struct s3cfb_window_pos winpos) ........ + case S3CFB_WIN_POSITION: + if (copy_from_user(winpos, (u32 __user *)arg, + sizeof(struct s3cfb_window_pos))) { + ret = -EFAULT; + break; + } + ret = s3c_fb_set_window_position(info, winpos); + break; > default: > ret = -ENOTTY; > } > -- > 1.7.0.4 ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ¥Šwÿº{.nÇ+‰·¥Š{±ıöİzÿâ�Ø^n‡r¡ö¦zË�ëh™¨è­Ú&£ûàz¿äz¹Ş—ú+€Ê+zf£¢·hšˆ§~†­†Ûiÿÿï�êÿ‘êçz_è®æj:+v‰¨ş)ߣøm ``` ## \[PATCH 2/2\] video: s3c-fb: Modify s3c-fb driver to support window positioning - From: Ajay kumar \ - Date: 2011-08-26 05:21:50 - Message: https://ratatoskr.run/linux-fbdev/2011/08/8584163.md ```text Hi Jingoo, 2011/8/26 JinGoo Han [ref]: > Hi, Ajay. >> -----Original Message----- >> From: Ajay Kumar [mailto:] >> Sent: Friday, August 26, 2011 4:52 AM >> To: ; ; linux- >> arm-kernel@lists.infradead.org >> Cc: FlorianSchandinat@gmx.de; ; ; >> m.szyprowski@samsung.com; ben-linux@fluff.org; banajit.g@samsung.com >> Subject: [PATCH 2/2] video: s3c-fb: Modify s3c-fb driver to support window >> positioning >> >> This patch modifies the existing s3c-fb driver to provide >> flexibility to the user to reposition the framebuffer windows. >> >> Signed-off-by: Ajay Kumar >> Signed-off-by: Banajit Goswami >> --- >>  drivers/video/s3c-fb.c |   37 ++++++++++++++++++++++++++++++++----- >>  1 files changed, 32 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c >> index 0fda252..41179d7 100644 >> --- a/drivers/video/s3c-fb.c >> +++ b/drivers/video/s3c-fb.c >> @@ -442,6 +442,7 @@ static int s3c_fb_set_par(struct fb_info *info) >>       struct fb_var_screeninfo *var = &info->var; >>       struct s3c_fb_win *win = info->par; >>       struct s3c_fb *sfb = win->parent; >> +     struct s3cfb_window_pos *winpos = &win->windata->winpos; >>       void __iomem *regs = sfb->regs; >>       void __iomem *buf = regs; >>       int win_no = win->index; >> @@ -539,12 +540,13 @@ static int s3c_fb_set_par(struct fb_info *info) >> >>       /* write 'OSD' registers to control position of framebuffer */ >> >> -     data = VIDOSDxA_TOPLEFT_X(0) | VIDOSDxA_TOPLEFT_Y(0); >> +     data = VIDOSDxA_TOPLEFT_X(winpos->win_pos_x) | >> +            VIDOSDxA_TOPLEFT_Y(winpos->win_pos_y); >>       writel(data, regs + VIDOSD_A(win_no, sfb->variant)); >> >> -     data = VIDOSDxB_BOTRIGHT_X(s3c_fb_align_word(var->bits_per_pixel, >> -                                                  var->xres - 1)) | >> -            VIDOSDxB_BOTRIGHT_Y(var->yres - 1); >> +     data = VIDOSDxB_BOTRIGHT_X((s3c_fb_align_word(var->bits_per_pixel, >> +            (winpos->win_pos_x + var->xres - 1)))) | >> +            VIDOSDxB_BOTRIGHT_Y((winpos->win_pos_y + var->yres - 1)); >> >>       writel(data, regs + VIDOSD_B(win_no, sfb->variant)); >> >> @@ -999,8 +1001,10 @@ static int s3c_fb_ioctl(struct fb_info *info, >> unsigned int cmd, >>  { >>       struct s3c_fb_win *win = info->par; >>       struct s3c_fb *sfb = win->parent; >> -     int ret; >> +     struct s3cfb_window_pos *winpos = &win->windata->winpos; >> +     int ret = 0; >>       u32 crtc; >> +     u32 data; >> >>       switch (cmd) { >>       case FBIO_WAITFORVSYNC: >> @@ -1011,6 +1015,29 @@ static int s3c_fb_ioctl(struct fb_info *info, >> unsigned int cmd, >> >>               ret = s3c_fb_wait_for_vsync(sfb, crtc); >>               break; >> +     case S3CFB_WIN_POSITION: >> +             if (copy_from_user(winpos, (u32 __user *)arg, >> +                                     sizeof(struct s3cfb_window_pos))) { >> +                     ret = -EFAULT; >> +                     break; >> +             } >> + >> +             shadow_protect_win(win, 1); >> + >> +             /* write 'OSD' registers to set position of the window */ >> +             data = VIDOSDxA_TOPLEFT_X(winpos->win_pos_x) | >> +                    VIDOSDxA_TOPLEFT_Y(winpos->win_pos_y); >> +             writel(data, sfb->regs + VIDOSD_A(win->index, sfb->variant)); >> + >> +             data = VIDOSDxB_BOTRIGHT_X( >> +                             s3c_fb_align_word(info->var.bits_per_pixel, >> +                             (winpos->win_pos_x + info->var.xres - 1))); >> +             data |= VIDOSDxB_BOTRIGHT_Y(winpos->win_pos_y + >> +                             info->var.yres - 1); >> +             writel(data, sfb->regs + VIDOSD_B(win->index, sfb->variant)); >> + >> +             shadow_protect_win(win, 0); >> +             break; > Can you move this to separate function? > In my opinion, it would be better for readability and consistency. > Also, FBIO_WAITFORVSYNC has been implemented in this way. >                ret = s3c_fb_wait_for_vsync(sfb, crtc); > How about adding a function 's3c_fb_set_window_position' for this as belows? > +int s3c_fb_set_window_position(struct fb_info *info, > +                               struct s3cfb_window_pos winpos) Thanks for your suggestion. I have considered it, and now I have written a separate function for this. I will be sending it in V2 version. >                ........ > +       case S3CFB_WIN_POSITION: > +               if (copy_from_user(winpos, (u32 __user *)arg, > +                                       sizeof(struct s3cfb_window_pos))) { > +                       ret = -EFAULT; > +                       break; > +               } > +               ret = s3c_fb_set_window_position(info, winpos); > +               break; >>       default: >>               ret = -ENOTTY; >>       } >> -- >> 1.7.0.4 > > > ```