Date: Fri, 6 Nov 2009 16:47:00 -0500 From: John Baldwin <jhb@freebsd.org> To: Jung-uk Kim <jkim@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r199002 - in head/sys: dev/fb dev/pci isa Message-ID: <200911061647.00983.jhb@freebsd.org> In-Reply-To: <200911062032.nA6KWRXb027876@svn.freebsd.org> References: <200911062032.nA6KWRXb027876@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Friday 06 November 2009 3:32:26 pm Jung-uk Kim wrote: > Author: jkim > Date: Fri Nov 6 20:32:26 2009 > New Revision: 199002 > URL: http://svn.freebsd.org/changeset/base/199002 > > Log: > Remove duplicate suspend/resume code from vga_pci.c and let vga(4) register > itself to an associated PCI device if it exists. It is little bit hackish > but it should fix build without frame buffer driver since r198964. > Fix some style(9) nits in vga_isa.c while we are here. Hmm, did you consider having vga_isa use an identify routine to attach itself as a child of vgapci0? The hack of knowing the first thing in the softc is a pointer is really gross and I'd rather avoid it. Just creating a child of vgapci0 will automatically cause suspend and resume to work w/o vgapci having to have any special knowledge about vga_isa. > Modified: > head/sys/dev/fb/vgareg.h > head/sys/dev/pci/vga_pci.c > head/sys/isa/vga_isa.c > > Modified: head/sys/dev/fb/vgareg.h > ============================================================================== > --- head/sys/dev/fb/vgareg.h Fri Nov 6 20:23:42 2009 (r199001) > +++ head/sys/dev/fb/vgareg.h Fri Nov 6 20:32:26 2009 (r199002) > @@ -69,6 +69,7 @@ > struct video_adapter; > typedef struct vga_softc { > struct video_adapter *adp; > + device_t pci_dev; > void *state_buf; > void *pal_buf; > #ifdef FB_INSTALL_CDEV > > Modified: head/sys/dev/pci/vga_pci.c > ============================================================================== > --- head/sys/dev/pci/vga_pci.c Fri Nov 6 20:23:42 2009 (r199001) > +++ head/sys/dev/pci/vga_pci.c Fri Nov 6 20:32:26 2009 (r199002) > @@ -40,15 +40,12 @@ __FBSDID("$FreeBSD$"); > > #include <sys/param.h> > #include <sys/bus.h> > -#include <sys/fbio.h> > #include <sys/kernel.h> > -#include <sys/malloc.h> > #include <sys/module.h> > #include <sys/rman.h> > #include <sys/sysctl.h> > #include <sys/systm.h> > > -#include <dev/fb/fbreg.h> > #include <dev/fb/vgareg.h> > > #include <dev/pci/pcireg.h> > @@ -60,6 +57,7 @@ struct vga_resource { > }; > > struct vga_pci_softc { > + device_t vga_isa_dev; /* Sister isavga driver. */ > device_t vga_msi_child; /* Child driver using MSI. */ > struct vga_resource vga_res[PCIR_MAX_BAR_0 + 1]; > }; > @@ -117,86 +115,23 @@ vga_pci_attach(device_t dev) > static int > vga_pci_suspend(device_t dev) > { > - vga_softc_t *sc; > - devclass_t dc; > - int err, nbytes; > - > - err = bus_generic_suspend(dev); > - if (err) > - return (err); > - > - sc = NULL; > - if (device_get_unit(dev) == vga_pci_default_unit) { > - dc = devclass_find(VGA_DRIVER_NAME); > - if (dc != NULL) > - sc = devclass_get_softc(dc, 0); > - } > - if (sc == NULL) > - return (0); > - > - /* Save the video state across the suspend. */ > - if (sc->state_buf != NULL) > - goto save_palette; > - nbytes = vidd_save_state(sc->adp, NULL, 0); > - if (nbytes <= 0) > - goto save_palette; > - sc->state_buf = malloc(nbytes, M_TEMP, M_NOWAIT); > - if (sc->state_buf == NULL) > - goto save_palette; > - if (bootverbose) > - device_printf(dev, "saving %d bytes of video state\n", nbytes); > - if (vidd_save_state(sc->adp, sc->state_buf, nbytes) != 0) { > - device_printf(dev, "failed to save state (nbytes=%d)\n", > - nbytes); > - free(sc->state_buf, M_TEMP); > - sc->state_buf = NULL; > - } > + struct vga_pci_softc *sc; > > -save_palette: > - /* Save the color palette across the suspend. */ > - if (sc->pal_buf != NULL) > - return (0); > - sc->pal_buf = malloc(256 * 3, M_TEMP, M_NOWAIT); > - if (sc->pal_buf != NULL) { > - if (bootverbose) > - device_printf(dev, "saving color palette\n"); > - if (vidd_save_palette(sc->adp, sc->pal_buf) != 0) { > - device_printf(dev, "failed to save palette\n"); > - free(sc->pal_buf, M_TEMP); > - sc->pal_buf = NULL; > - } > - } > + sc = device_get_softc(dev); > + if (sc->vga_isa_dev != NULL) > + (void)DEVICE_SUSPEND(sc->vga_isa_dev); > > - return (0); > + return (bus_generic_suspend(dev)); > } > > static int > vga_pci_resume(device_t dev) > { > - vga_softc_t *sc; > - devclass_t dc; > - > - sc = NULL; > - if (device_get_unit(dev) == vga_pci_default_unit) { > - dc = devclass_find(VGA_DRIVER_NAME); > - if (dc != NULL) > - sc = devclass_get_softc(dc, 0); > - } > - if (sc == NULL) > - return (bus_generic_resume(dev)); > + struct vga_pci_softc *sc; > > - if (sc->state_buf != NULL) { > - if (vidd_load_state(sc->adp, sc->state_buf) != 0) > - device_printf(dev, "failed to reload state\n"); > - free(sc->state_buf, M_TEMP); > - sc->state_buf = NULL; > - } > - if (sc->pal_buf != NULL) { > - if (vidd_load_palette(sc->adp, sc->pal_buf) != 0) > - device_printf(dev, "failed to reload palette\n"); > - free(sc->pal_buf, M_TEMP); > - sc->pal_buf = NULL; > - } > + sc = device_get_softc(dev); > + if (sc->vga_isa_dev != NULL) > + (void)DEVICE_RESUME(sc->vga_isa_dev); > > return (bus_generic_resume(dev)); > } > > Modified: head/sys/isa/vga_isa.c > ============================================================================== > --- head/sys/isa/vga_isa.c Fri Nov 6 20:23:42 2009 (r199001) > +++ head/sys/isa/vga_isa.c Fri Nov 6 20:32:26 2009 (r199002) > @@ -117,13 +117,17 @@ isavga_probe(device_t dev) > isa_set_msize(dev, adp.va_mem_size); > #endif > } > - return error; > + return (error); > } > > static int > isavga_attach(device_t dev) > { > vga_softc_t *sc; > + devclass_t dc; > + device_t *devs; > + void *vgapci_sc; > + int count, i; > int unit; > int rid; > int error; > @@ -140,13 +144,13 @@ isavga_attach(device_t dev) > > error = vga_attach_unit(unit, sc, device_get_flags(dev)); > if (error) > - return error; > + return (error); > > #ifdef FB_INSTALL_CDEV > /* attach a virtual frame buffer device */ > error = fb_attach(VGA_MKMINOR(unit), sc->adp, &isavga_cdevsw); > if (error) > - return error; > + return (error); > #endif /* FB_INSTALL_CDEV */ > > if (0 && bootverbose) > @@ -157,20 +161,43 @@ isavga_attach(device_t dev) > bus_generic_attach(dev); > #endif > > - return 0; > + /* Find the matching PCI video controller. */ > + if (unit == 0) { > + dc = devclass_find("vgapci"); > + if (dc != NULL && > + devclass_get_devices(dc, &devs, &count) == 0) { > + for (i = 0; i < count; i++) > + if (device_get_flags(devs[i]) != 0) { > + sc->pci_dev = devs[i]; > + break; > + } > + free(devs, M_TEMP); > + } > + if (sc->pci_dev != NULL) { > + vgapci_sc = device_get_softc(sc->pci_dev); > + *(device_t *)vgapci_sc = dev; > + device_printf(dev, "associated with %s\n", > + device_get_nameunit(sc->pci_dev)); > + } > + } > + > + return (0); > } > > static int > isavga_suspend(device_t dev) > { > vga_softc_t *sc; > + device_t isa_dev; > int err, nbytes; > > - err = bus_generic_suspend(dev); > - if (err) > - return (err); > - > - sc = device_get_softc(dev); > + err = 0; > + isa_dev = dev; > + sc = device_get_softc(isa_dev); > + if (sc->pci_dev != NULL) > + dev = sc->pci_dev; > + else > + err = bus_generic_suspend(isa_dev); > > /* Save the video state across the suspend. */ > if (sc->state_buf != NULL) > @@ -193,7 +220,7 @@ isavga_suspend(device_t dev) > save_palette: > /* Save the color palette across the suspend. */ > if (sc->pal_buf != NULL) > - return (0); > + return (err); > sc->pal_buf = malloc(256 * 3, M_TEMP, M_NOWAIT); > if (sc->pal_buf != NULL) { > if (bootverbose) > @@ -205,15 +232,19 @@ save_palette: > } > } > > - return (0); > + return (err); > } > > static int > isavga_resume(device_t dev) > { > vga_softc_t *sc; > + device_t isa_dev; > > - sc = device_get_softc(dev); > + isa_dev = dev; > + sc = device_get_softc(isa_dev); > + if (sc->pci_dev != NULL) > + dev = sc->pci_dev; > > if (sc->state_buf != NULL) { > if (vidd_load_state(sc->adp, sc->state_buf) != 0) > @@ -228,7 +259,10 @@ isavga_resume(device_t dev) > sc->pal_buf = NULL; > } > > - return (bus_generic_resume(dev)); > + if (isa_dev != dev) > + return (0); > + > + return (bus_generic_resume(isa_dev)); > } > > #ifdef FB_INSTALL_CDEV > @@ -236,37 +270,37 @@ isavga_resume(device_t dev) > static int > isavga_open(struct cdev *dev, int flag, int mode, struct thread *td) > { > - return vga_open(dev, VGA_SOFTC(VGA_UNIT(dev)), flag, mode, td); > + return (vga_open(dev, VGA_SOFTC(VGA_UNIT(dev)), flag, mode, td)); > } > > static int > isavga_close(struct cdev *dev, int flag, int mode, struct thread *td) > { > - return vga_close(dev, VGA_SOFTC(VGA_UNIT(dev)), flag, mode, td); > + return (vga_close(dev, VGA_SOFTC(VGA_UNIT(dev)), flag, mode, td)); > } > > static int > isavga_read(struct cdev *dev, struct uio *uio, int flag) > { > - return vga_read(dev, VGA_SOFTC(VGA_UNIT(dev)), uio, flag); > + return (vga_read(dev, VGA_SOFTC(VGA_UNIT(dev)), uio, flag)); > } > > static int > isavga_write(struct cdev *dev, struct uio *uio, int flag) > { > - return vga_write(dev, VGA_SOFTC(VGA_UNIT(dev)), uio, flag); > + return (vga_write(dev, VGA_SOFTC(VGA_UNIT(dev)), uio, flag)); > } > > static int > isavga_ioctl(struct cdev *dev, u_long cmd, caddr_t arg, int flag, struct thread *td) > { > - return vga_ioctl(dev, VGA_SOFTC(VGA_UNIT(dev)), cmd, arg, flag, td); > + return (vga_ioctl(dev, VGA_SOFTC(VGA_UNIT(dev)), cmd, arg, flag, td)); > } > > static int > isavga_mmap(struct cdev *dev, vm_offset_t offset, vm_paddr_t *paddr, int prot) > { > - return vga_mmap(dev, VGA_SOFTC(VGA_UNIT(dev)), offset, paddr, prot); > + return (vga_mmap(dev, VGA_SOFTC(VGA_UNIT(dev)), offset, paddr, prot)); > } > > #endif /* FB_INSTALL_CDEV */ > -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200911061647.00983.jhb>