> On 21/08/12(Tue) 20:58, Mark Kettenis wrote:
> > > Date: Tue, 21 Aug 2012 19:17:08 +0200
> > > From: Martin Pieuchot <[email protected]>
> > >
> > > Diff below is mostly a rewrite of vgafb_pci_probe() to determine the
> > > memory and mmio regions based on previously initialized BAR structures.
> > > But it also includes the glue to attach drm(4) to vgafb(4).
> > >
> > > It reuses the following functions from vga(4), the last three are
> > > required by any drm(4) driver:
> > > - vga_pci_bar_init()
> > > - vga_pci_bar_info()
> > > - vga_pci_bar_map()
> > > - vga_pci_bar_unmap()
> >
> > Looks like by "reuses" you mean "copies". That tends to be a bad idea
> > since results in diverging code. WOuldn't it be better to split out
> > these functions into their own file (vga_pci_common.c) and share them
> > between vga(4) and vgafb(4)?
>
> Sure, here's a diff that does only this, tested on i386 and macppc.
> I used Owain's copyright because he is the one who commited this code.
>
> Ok?
Go for it.
(If it wasn't obvious yet, I intend to use this code on sparc64 as well)
> Index: pci/files.pci
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/files.pci,v
> retrieving revision 1.285
> diff -u -p -r1.285 files.pci
> --- pci/files.pci 14 Aug 2012 00:28:23 -0000 1.285
> +++ pci/files.pci 22 Aug 2012 15:03:50 -0000
> @@ -15,6 +15,7 @@ file dev/pci/pci_subr.c pci
> # Generic VGA
> attach vga at pci with vga_pci
> file dev/pci/vga_pci.c vga_pci
> +file dev/pci/vga_pci_common.c vga_pci
>
> device tga: wsemuldisplaydev, rasops8, rasops32
> attach tga at pci
> Index: pci/vga_pci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/vga_pci.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 vga_pci.c
> --- pci/vga_pci.c 14 Apr 2011 21:04:29 -0000 1.67
> +++ pci/vga_pci.c 22 Aug 2012 15:03:36 -0000
> @@ -63,6 +63,7 @@
> */
>
> #include "vga.h"
> +#include "drm.h"
> #if defined(__i386__) || defined(__amd64__)
> #include "acpi.h"
> #endif
> @@ -90,7 +91,6 @@
> #include <dev/ic/vgavar.h>
> #include <dev/pci/vga_pcivar.h>
>
> -
> #include <dev/wscons/wsconsio.h>
> #include <dev/wscons/wsdisplayvar.h>
>
> @@ -103,22 +103,16 @@
> #endif
>
> #include "intagp.h"
> -#include "drm.h"
>
> int vga_pci_match(struct device *, void *, void *);
> void vga_pci_attach(struct device *, struct device *, void *);
> int vga_pci_activate(struct device *, int);
> paddr_t vga_pci_mmap(void* v, off_t off, int prot);
> -void vga_pci_bar_init(struct vga_pci_softc *, struct pci_attach_args *);
>
> #if NINTAGP > 0
> int intagpsubmatch(struct device *, void *, void *);
> int intagp_print(void *, const char *);
> #endif
> -#if NDRM > 0
> -int drmsubmatch(struct device *, void *, void *);
> -int vga_drm_print(void *, const char *);
> -#endif
>
> #ifdef VESAFB
> int vesafb_putcmap(struct vga_pci_softc *, struct wsdisplay_cmap *);
> @@ -138,7 +132,7 @@ void vga_restore_state(struct vga_pci_so
> */
> int (*ws_get_param)(struct wsdisplay_param *);
> int (*ws_set_param)(struct wsdisplay_param *);
> -
> +
>
> struct cfattach vga_pci_ca = {
> sizeof(struct vga_pci_softc), vga_pci_match, vga_pci_attach,
> @@ -369,27 +363,6 @@ intagp_print(void *vaa, const char *pnp)
> }
> #endif
>
> -#if NDRM > 0
> -int
> -drmsubmatch(struct device *parent, void *match, void *aux)
> -{
> - struct cfdata *cf = match;
> - struct cfdriver *cd;
> - size_t len = 0;
> - char *sm;
> -
> - cd = cf->cf_driver;
> -
> - /* is this a *drm device? */
> - len = strlen(cd->cd_name);
> - sm = cd->cd_name + len - 3;
> - if (strncmp(sm, "drm", 3) == 0)
> - return ((*cf->cf_attach->ca_match)(parent, match, aux));
> -
> - return (0);
> -}
> -#endif
> -
> paddr_t
> vga_pci_mmap(void *v, off_t off, int prot)
> {
> @@ -506,116 +479,6 @@ vga_pci_ioctl(void *v, u_long cmd, caddr
> }
>
> return (error);
> -}
> -
> -#ifdef notyet
> -void
> -vga_pci_close(void *v)
> -{
> -}
> -#endif
> -
> -/*
> - * Prepare dev->bars to be used for information. we do this at startup
> - * so we can do the whole array at once, dealing with 64-bit BARs correctly.
> - */
> -void
> -vga_pci_bar_init(struct vga_pci_softc *dev, struct pci_attach_args *pa)
> -{
> - pcireg_t type;
> - int addr = PCI_MAPREG_START, i = 0;
> - memcpy(&dev->pa, pa, sizeof(dev->pa));
> -
> - while (i < VGA_PCI_MAX_BARS) {
> - dev->bars[i] = malloc(sizeof((*dev->bars[i])), M_DEVBUF,
> - M_NOWAIT | M_ZERO);
> - if (dev->bars[i] == NULL) {
> - return;
> - }
> -
> - dev->bars[i]->addr = addr;
> -
> - type = dev->bars[i]->maptype = pci_mapreg_type(pa->pa_pc,
> - pa->pa_tag, addr);
> - if (pci_mapreg_info(pa->pa_pc, pa->pa_tag, addr,
> - dev->bars[i]->maptype, &dev->bars[i]->base,
> - &dev->bars[i]->maxsize, &dev->bars[i]->flags) != 0) {
> - free(dev->bars[i], M_DEVBUF);
> - dev->bars[i] = NULL;
> - }
> -
> - if (type == PCI_MAPREG_MEM_TYPE_64BIT) {
> - addr += 8;
> - i += 2;
> - } else {
> - addr += 4;
> - i++;
> - }
> - }
> -}
> -
> -/*
> - * Get the vga_pci_bar struct for the address in question. returns NULL if
> - * invalid BAR is passed.
> - */
> -struct vga_pci_bar*
> -vga_pci_bar_info(struct vga_pci_softc *dev, int no)
> -{
> - if (dev == NULL || no >= VGA_PCI_MAX_BARS)
> - return (NULL);
> - return (dev->bars[no]);
> -}
> -
> -/*
> - * map the BAR in question, returning the vga_pci_bar struct in case any more
> - * processing needs to be done. Returns NULL on failure. Can be called
> multiple
> - * times.
> - */
> -struct vga_pci_bar*
> -vga_pci_bar_map(struct vga_pci_softc *dev, int addr, bus_size_t size,
> - int busflags)
> -{
> - struct vga_pci_bar *bar = NULL;
> - int i;
> -
> - if (dev == NULL)
> - return (NULL);
> -
> - for (i = 0; i < VGA_PCI_MAX_BARS; i++) {
> - if (dev->bars[i] && dev->bars[i]->addr == addr) {
> - bar = dev->bars[i];
> - break;
> - }
> - }
> - if (bar == NULL) {
> - printf("vga_pci_bar_map: given invalid address 0x%x\n", addr);
> - return (NULL);
> - }
> -
> - if (bar->mapped == 0) {
> - if (pci_mapreg_map(&dev->pa, bar->addr, bar->maptype,
> - bar->flags | busflags, &bar->bst, &bar->bsh, NULL,
> - &bar->size, size)) {
> - printf("vga_pci_bar_map: can't map bar 0x%x\n", addr);
> - return (NULL);
> - }
> - }
> -
> - bar->mapped++;
> - return (bar);
> -}
> -
> -/*
> - * "unmap" the BAR referred to by argument. If more than one place has
> mapped it
> - * we just decrement the reference counter so nothing untoward happens.
> - */
> -void
> -vga_pci_bar_unmap(struct vga_pci_bar *bar)
> -{
> - if (bar != NULL && bar->mapped != 0) {
> - if (--bar->mapped == 0)
> - bus_space_unmap(bar->bst, bar->bsh, bar->size);
> - }
> }
>
> #if !defined(SMALL_KERNEL) && NACPI > 0
> Index: pci/vga_pci_common.c
> ===================================================================
> RCS file: pci/vga_pci_common.c
> diff -N pci/vga_pci_common.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ pci/vga_pci_common.c 22 Aug 2012 15:08:27 -0000
> @@ -0,0 +1,152 @@
> +/*
> + * Copyright (c) 2008 Owain G. Ainsworth <[email protected]>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include "drm.h"
> +
> +#include <sys/param.h>
> +#include <sys/systm.h>
> +#include <sys/malloc.h>
> +
> +#include <machine/bus.h>
> +
> +#include <dev/pci/pcireg.h>
> +#include <dev/pci/pcivar.h>
> +#include <dev/pci/vga_pcivar.h>
> +
> +
> +#if NDRM > 0
> +int
> +drmsubmatch(struct device *parent, void *match, void *aux)
> +{
> + struct cfdata *cf = match;
> + struct cfdriver *cd;
> + size_t len = 0;
> + char *sm;
> +
> + cd = cf->cf_driver;
> +
> + /* is this a *drm device? */
> + len = strlen(cd->cd_name);
> + sm = cd->cd_name + len - 3;
> + if (strncmp(sm, "drm", 3) == 0)
> + return ((*cf->cf_attach->ca_match)(parent, match, aux));
> +
> + return (0);
> +}
> +#endif
> +
> +/*
> + * Prepare dev->bars to be used for information. we do this at startup
> + * so we can do the whole array at once, dealing with 64-bit BARs correctly.
> + */
> +void
> +vga_pci_bar_init(struct vga_pci_softc *dev, struct pci_attach_args *pa)
> +{
> + pcireg_t type;
> + int addr = PCI_MAPREG_START, i = 0;
> + memcpy(&dev->pa, pa, sizeof(dev->pa));
> +
> + while (i < VGA_PCI_MAX_BARS) {
> + dev->bars[i] = malloc(sizeof((*dev->bars[i])), M_DEVBUF,
> + M_NOWAIT | M_ZERO);
> + if (dev->bars[i] == NULL) {
> + return;
> + }
> +
> + dev->bars[i]->addr = addr;
> +
> + type = dev->bars[i]->maptype = pci_mapreg_type(pa->pa_pc,
> + pa->pa_tag, addr);
> + if (pci_mapreg_info(pa->pa_pc, pa->pa_tag, addr,
> + dev->bars[i]->maptype, &dev->bars[i]->base,
> + &dev->bars[i]->maxsize, &dev->bars[i]->flags) != 0) {
> + free(dev->bars[i], M_DEVBUF);
> + dev->bars[i] = NULL;
> + }
> +
> + if (type == PCI_MAPREG_MEM_TYPE_64BIT) {
> + addr += 8;
> + i += 2;
> + } else {
> + addr += 4;
> + i++;
> + }
> + }
> +}
> +
> +/*
> + * Get the vga_pci_bar struct for the address in question. returns NULL if
> + * invalid BAR is passed.
> + */
> +struct vga_pci_bar*
> +vga_pci_bar_info(struct vga_pci_softc *dev, int no)
> +{
> + if (dev == NULL || no >= VGA_PCI_MAX_BARS)
> + return (NULL);
> + return (dev->bars[no]);
> +}
> +
> +/*
> + * map the BAR in question, returning the vga_pci_bar struct in case any more
> + * processing needs to be done. Returns NULL on failure. Can be called
> multiple
> + * times.
> + */
> +struct vga_pci_bar*
> +vga_pci_bar_map(struct vga_pci_softc *dev, int addr, bus_size_t size,
> + int busflags)
> +{
> + struct vga_pci_bar *bar = NULL;
> + int i;
> +
> + if (dev == NULL)
> + return (NULL);
> +
> + for (i = 0; i < VGA_PCI_MAX_BARS; i++) {
> + if (dev->bars[i] && dev->bars[i]->addr == addr) {
> + bar = dev->bars[i];
> + break;
> + }
> + }
> + if (bar == NULL) {
> + printf("vga_pci_bar_map: given invalid address 0x%x\n", addr);
> + return (NULL);
> + }
> +
> + if (bar->mapped == 0) {
> + if (pci_mapreg_map(&dev->pa, bar->addr, bar->maptype,
> + bar->flags | busflags, &bar->bst, &bar->bsh, NULL,
> + &bar->size, size)) {
> + printf("vga_pci_bar_map: can't map bar 0x%x\n", addr);
> + return (NULL);
> + }
> + }
> +
> + bar->mapped++;
> + return (bar);
> +}
> +
> +/*
> + * "unmap" the BAR referred to by argument. If more than one place has
> mapped it
> + * we just decrement the reference counter so nothing untoward happens.
> + */
> +void
> +vga_pci_bar_unmap(struct vga_pci_bar *bar)
> +{
> + if (bar != NULL && bar->mapped != 0) {
> + if (--bar->mapped == 0)
> + bus_space_unmap(bar->bst, bar->bsh, bar->size);
> + }
> +}
> Index: pci/vga_pcivar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/vga_pcivar.h,v
> retrieving revision 1.14
> diff -u -p -r1.14 vga_pcivar.h
> --- pci/vga_pcivar.h 8 Aug 2010 17:21:07 -0000 1.14
> +++ pci/vga_pcivar.h 22 Aug 2012 15:03:36 -0000
> @@ -85,10 +85,15 @@ struct vga_pci_softc {
>
> int vga_pci_cnattach(bus_space_tag_t, bus_space_tag_t,
> pci_chipset_tag_t, int, int, int);
> +void vga_pci_bar_init(struct vga_pci_softc *, struct pci_attach_args *);
> struct vga_pci_bar *vga_pci_bar_info(struct vga_pci_softc *, int);
> struct vga_pci_bar *vga_pci_bar_map(struct vga_pci_softc *, int,
> bus_size_t, int);
> void vga_pci_bar_unmap(struct vga_pci_bar*);
> +
> +#if NDRM > 0
> +int drmsubmatch(struct device *, void *, void *);
> +#endif
>
> #ifdef VESAFB
> int vesafb_find_mode(struct vga_pci_softc *, int, int, int);
