Skip site navigation (1)Skip section navigation (2)
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>