Date: Thu, 17 Jul 2014 16:45:26 +0300 From: Aleksandr Rybalko <ray@freebsd.org> To: Nathan Whitehorn <nwhitehorn@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r268771 - in head/sys: dev/fb dev/vt/hw/fb sys Message-ID: <20140717164526.b8e749e6cdd8593c0e497d5a@freebsd.org> In-Reply-To: <53C7C62F.1090604@freebsd.org> References: <201407161849.s6GInkmG040492@svn.freebsd.org> <20140717143535.0e337de530fd7d783edd4ad5@ddteam.net> <53C7C62F.1090604@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 17 Jul 2014 05:48:47 -0700 Nathan Whitehorn <nwhitehorn@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 <nwhitehorn@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 <ray@freebsd.org>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140717164526.b8e749e6cdd8593c0e497d5a>