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?
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);
