On Thu, 17 Jul 2014 05:48:47 -0700
Nathan Whitehorn <nwhiteh...@freebsd.org> wrote:

> 
> On 07/17/14 04:35, Aleksandr Rybalko wrote:
> > Hi Nathan!
> >
> > On Wed, 16 Jul 2014 18:49:46 +0000 (UTC)
> > Nathan Whitehorn <nwhiteh...@freebsd.org> wrote:
> >
> >> Author: nwhitehorn
> >> Date: Wed Jul 16 18:49:46 2014
> >> New Revision: 268771
> >> URL: http://svnweb.freebsd.org/changeset/base/268771
> >>
> >> Log:
> >>    Allow console drivers active from early boot to be used with 
> >> xf86-video-scfb,
> >>    rather than only drivers attached later on. This involves a small 
> >> amount of
> >>    code duplication with dev/fb/fbd.c, which will fixed later on.
> >>    
> >>    Also improve performance of vt_blank() by making it not read from the
> >>    framebuffer unnecessarily.
> >>
> >> Modified:
> >>    head/sys/dev/fb/fbd.c
> >>    head/sys/dev/vt/hw/fb/vt_fb.c
> >>    head/sys/dev/vt/hw/fb/vt_fb.h
> >>    head/sys/sys/fbio.h
> >>
> >> Modified: head/sys/dev/fb/fbd.c
> >> ==============================================================================
> >> --- head/sys/dev/fb/fbd.c  Wed Jul 16 16:42:58 2014        (r268770)
> >> +++ head/sys/dev/fb/fbd.c  Wed Jul 16 18:49:46 2014        (r268771)
> >> @@ -257,9 +257,6 @@ fb_probe(struct fb_info *info)
> >>    } else if (info->fb_vbase != 0) {
> >>            if (info->fb_pbase == 0) {
> >>                    info->fb_flags |= FB_FLAG_NOMMAP;
> >> -          } else {
> >> -                  if (info->fb_mmap == NULL)
> >> -                          info->fb_mmap = &fb_mmap;
> >>            }
> >>            info->wr1 = &vt_fb_mem_wr1;
> >>            info->wr2 = &vt_fb_mem_wr2;
> >> @@ -268,10 +265,6 @@ fb_probe(struct fb_info *info)
> >>    } else
> >>            return (ENXIO);
> >>   
> >> -  if (info->fb_ioctl == NULL)
> >> -          info->fb_ioctl = &fb_ioctl;
> >> -
> >> -
> >>    return (0);
> >>   }
> >>   
> >>
> >> Modified: head/sys/dev/vt/hw/fb/vt_fb.c
> >> ==============================================================================
> >> --- head/sys/dev/vt/hw/fb/vt_fb.c  Wed Jul 16 16:42:58 2014        
> >> (r268770)
> >> +++ head/sys/dev/vt/hw/fb/vt_fb.c  Wed Jul 16 18:49:46 2014        
> >> (r268771)
> >> @@ -41,10 +41,6 @@ __FBSDID("$FreeBSD$");
> >>   #include <dev/vt/hw/fb/vt_fb.h>
> >>   #include <dev/vt/colors/vt_termcolors.h>
> >>   
> >> -static int vt_fb_ioctl(struct vt_device *vd, u_long cmd, caddr_t data,
> >> -    struct thread *td);
> >> -static int vt_fb_mmap(struct vt_device *vd, vm_ooffset_t offset,
> >> -    vm_paddr_t *paddr, int prot, vm_memattr_t *memattr);
> >>   void vt_fb_drawrect(struct vt_device *vd, int x1, int y1, int x2, int y2,
> >>       int fill, term_color_t color);
> >>   void vt_fb_setpixel(struct vt_device *vd, int x, int y, term_color_t 
> >> color);
> >> @@ -65,20 +61,47 @@ static struct vt_driver vt_fb_driver = {
> >>   
> >>   VT_DRIVER_DECLARE(vt_fb, vt_fb_driver);
> >>   
> >> -static int
> >> +int
> >>   vt_fb_ioctl(struct vt_device *vd, u_long cmd, caddr_t data, struct 
> >> thread *td)
> >>   {
> >>    struct fb_info *info;
> >> +  int error = 0;
> >>   
> >>    info = vd->vd_softc;
> >>   
> >> -  if (info->fb_ioctl == NULL)
> >> -          return (-1);
> >> +  switch (cmd) {
> >> +  case FBIOGTYPE:
> >> +          bcopy(info, (struct fbtype *)data, sizeof(struct fbtype));
> >> +          break;
> >> +
> >> +  case FBIO_GETWINORG:    /* get frame buffer window origin */
> >> +          *(u_int *)data = 0;
> >> +          break;
> >> +
> >> +  case FBIO_GETDISPSTART: /* get display start address */
> >> +          ((video_display_start_t *)data)->x = 0;
> >> +          ((video_display_start_t *)data)->y = 0;
> >> +          break;
> >> +
> >> +  case FBIO_GETLINEWIDTH: /* get scan line width in bytes */
> >> +          *(u_int *)data = info->fb_stride;
> >> +          break;
> >>   
> >> -  return (info->fb_ioctl(info->fb_cdev, cmd, data, 0, td));
> >> +  case FBIO_BLANK:        /* blank display */
> >> +          if (vd->vd_driver->vd_blank == NULL)
> >> +                  return (ENODEV);
> >> +          vd->vd_driver->vd_blank(vd, TC_BLACK);
> >> +          break;
> >> +
> >> +  default:
> >> +          error = ENOIOCTL;
> >> +          break;
> >> +  }
> >> +
> >> +  return (error);
> >>   }
> >>   
> >> -static int
> >> +int
> >>   vt_fb_mmap(struct vt_device *vd, vm_ooffset_t offset, vm_paddr_t *paddr,
> >>       int prot, vm_memattr_t *memattr)
> >>   {
> >> @@ -86,10 +109,18 @@ vt_fb_mmap(struct vt_device *vd, vm_ooff
> >>   
> >>    info = vd->vd_softc;
> >>   
> >> -  if (info->fb_ioctl == NULL)
> >> -          return (ENXIO);
> >> +  if (info->fb_flags & FB_FLAG_NOMMAP)
> >> +          return (ENODEV);
> >>   
> >> -  return (info->fb_mmap(info->fb_cdev, offset, paddr, prot, memattr));
> >> +  if (offset >= 0 && offset < info->fb_size) {
> >> +          *paddr = info->fb_pbase + offset;
> >> +  #ifdef VM_MEMATTR_WRITE_COMBINING
> >> +          *memattr = VM_MEMATTR_WRITE_COMBINING;
> >> +  #endif
> >> +          return (0);
> >> +  }
> >> +
> >> +  return (EINVAL);
> >>   }
> >>   
> >>   void
> >> @@ -147,41 +178,42 @@ vt_fb_blank(struct vt_device *vd, term_c
> >>   {
> >>    struct fb_info *info;
> >>    uint32_t c;
> >> -  u_int o;
> >> +  u_int o, h;
> >>   
> >>    info = vd->vd_softc;
> >>    c = info->fb_cmap[color];
> >>   
> >>    switch (FBTYPE_GET_BYTESPP(info)) {
> >>    case 1:
> >> -          for (o = 0; o < info->fb_stride; o++)
> >> -                  info->wr1(info, o, c);
> >> +          for (h = 0; h < info->fb_stride; h++)
> >> +                  for (o = 0; o < info->fb_stride; o++)
> >> +                          info->wr1(info, h*info->fb_stride + o, c);
> > Tell me please, what is it? ^^^^^^
> > Looks like you mean "for (h = 0; h < info->fb_height; h++)", but anyway
> > you can do one loop up to info->fb_size.
> 
> Wow. That's embarrassing. I just fixed it. Thanks! I didn't do fb_size 
> in case the framebuffer were discontiguous. Maybe the caution is 
> unnecessary, though.
> -Nathan

You doing info->wr1(), so this method should care about access model. 

Anyway, thanks for doing that.
Looks like I did overdesigned it with way for replacing every method.
Just one thought to discussion (maybe):
Is not it will be better to make default implementation of mmap/munmap
methods and assign it if driver do not supply own?

> 
> >>            break;
> >>    case 2:
> >> -          for (o = 0; o < info->fb_stride; o += 2)
> >> -                  info->wr2(info, o, c);
> >> +          for (h = 0; h < info->fb_stride; h++)
> >> +                  for (o = 0; o < info->fb_stride; o += 2)
> >> +                          info->wr2(info, h*info->fb_stride + o, c);
> >>            break;
> >>    case 3:
> >> -          /* line 0 */
> >> -          for (o = 0; o < info->fb_stride; o += 3) {
> >> -                  info->wr1(info, o, (c >> 16) & 0xff);
> >> -                  info->wr1(info, o + 1, (c >> 8) & 0xff);
> >> -                  info->wr1(info, o + 2, c & 0xff);
> >> -          }
> >> +          for (h = 0; h < info->fb_stride; h++)
> >> +                  for (o = 0; o < info->fb_stride; o += 3) {
> >> +                          info->wr1(info, h*info->fb_stride + o,
> >> +                              (c >> 16) & 0xff);
> >> +                          info->wr1(info, h*info->fb_stride + o + 1,
> >> +                              (c >> 8) & 0xff);
> >> +                          info->wr1(info, h*info->fb_stride + o + 2,
> >> +                              c & 0xff);
> >> +                  }
> >>            break;
> >>    case 4:
> >> -          for (o = 0; o < info->fb_stride; o += 4)
> >> -                  info->wr4(info, o, c);
> >> +          for (h = 0; h < info->fb_stride; h++)
> >> +                  for (o = 0; o < info->fb_stride; o += 4)
> >> +                          info->wr4(info, o, c);
> >>            break;
> >>    default:
> >>            /* panic? */
> >>            return;
> >>    }
> >> -  /* Copy line0 to all other lines. */
> >> -  /* XXX will copy with borders. */
> >> -  for (o = info->fb_stride; o < info->fb_size; o += info->fb_stride) {
> >> -          info->copy(info, o, 0, info->fb_stride);
> >> -  }
> >>   }
> >>   
> >>   void
> >>
> >> Modified: head/sys/dev/vt/hw/fb/vt_fb.h
> >> ==============================================================================
> >> --- head/sys/dev/vt/hw/fb/vt_fb.h  Wed Jul 16 16:42:58 2014        
> >> (r268770)
> >> +++ head/sys/dev/vt/hw/fb/vt_fb.h  Wed Jul 16 18:49:46 2014        
> >> (r268771)
> >> @@ -43,5 +43,7 @@ vd_blank_t       vt_fb_blank;
> >>   vd_bitbltchr_t   vt_fb_bitbltchr;
> >>   vd_maskbitbltchr_t vt_fb_maskbitbltchr;
> >>   vd_postswitch_t  vt_fb_postswitch;
> >> +vd_fb_ioctl_t     vt_fb_ioctl;
> >> +vd_fb_mmap_t      vt_fb_mmap;
> >>   
> >>   #endif /* _DEV_VT_HW_FB_VT_FB_H_ */
> >>
> >> Modified: head/sys/sys/fbio.h
> >> ==============================================================================
> >> --- head/sys/sys/fbio.h    Wed Jul 16 16:42:58 2014        (r268770)
> >> +++ head/sys/sys/fbio.h    Wed Jul 16 18:49:46 2014        (r268771)
> >> @@ -141,8 +141,6 @@ struct fb_info {
> >>    /* Methods. */
> >>    fb_write_t      *fb_write;      /* if NULL, direct mem write. */
> >>    fb_read_t       *fb_read;       /* if NULL, direct mem read. */
> >> -  fb_ioctl_t      *fb_ioctl;      /* Can be NULL. */
> >> -  fb_mmap_t       *fb_mmap;       /* Can be NULL. */
> >>   
> >>    struct cdev     *fb_cdev;
> >>   
> >>
> > Thanks!
> >
> > WBW
> 

WBW
-- 
Aleksandr Rybalko <r...@freebsd.org>
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to