linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* Re: [linux-fbdev] [PATCH 2.3.x] fbdev reversion
@ 2000-03-14 10:03 Benjamin Herrenschmidt
  2000-03-14 10:05 ` Geert Uytterhoeven
  2000-03-14 21:22 ` Michael Schmitz
  0 siblings, 2 replies; 10+ messages in thread
From: Benjamin Herrenschmidt @ 2000-03-14 10:03 UTC (permalink / raw)
  To: Geert Uytterhoeven, linux-fbdev, linuxppc-dev


On Tue, Mar 14, 2000, Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
wrote:

>... which they can easily find out by combining /proc/bus/pci with
>fix.{smem,mmio}_{start,len}.
>
>Since XFree86 4.0 already has resource handling, it should be a piece of cake
>to remove all devices corresponding to any fix.{smem,mmio}_{start,len} from
>their list.

Well, while we are at it, we could also work on fixing the PCI problems:
Instead of relying on XFree knowing the host bridge and deducing the
iobase, I beleive we should export from the kernel either one iobase per
PCI device (there can be different busses with different iobases, that's
the case on the new macs and they have the same bus number), or we can
have /proc/bus/pci export "fixed'up" IO addresses (already including the
iobase).

I can code those fixes, but I'm not sure what solution has most chances
of beeing accepted (especially changing /poc/bus/pci semantics). The
problem is that exporting an IO address alone (without the iobase) has
really no meaning on arch that don't have a x86-like separate IO space at
the CPU level.

Back to the subject of fbdev vs. XFree, it would be easy to add an
optional ioctl returning the PCI bus:dev:fn from the fbdev (would be
PCI-specific however). Eventually, it could be a generic ioctl that returns

  - A constant indicating the bus type
  - A string containing a bus-specific identification

I beleive it's cleaner that relying on resource removal, and may be
useful for other things too (like board-specific userland setup tools
that would want to issue some config space accesses to the card, etc...).

Note that I'd love to see this mecanism of a bus-type/id-string extended
to all devices in the kernel (using /proc/ instead of ioctls) since it
would help solving our problem of configuring OF boot path too. But
that's another story...

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [linux-fbdev] [PATCH 2.3.x] fbdev reversion
  2000-03-14 10:03 [linux-fbdev] [PATCH 2.3.x] fbdev reversion Benjamin Herrenschmidt
@ 2000-03-14 10:05 ` Geert Uytterhoeven
  2000-03-14 22:11   ` Michel Lanners
  2000-03-14 22:59   ` Gabriel Paubert
  2000-03-14 21:22 ` Michael Schmitz
  1 sibling, 2 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2000-03-14 10:05 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-fbdev, linuxppc-dev


On Tue, 14 Mar 2000, Benjamin Herrenschmidt wrote:
> On Tue, Mar 14, 2000, Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> wrote:
> Well, while we are at it, we could also work on fixing the PCI problems:
> Instead of relying on XFree knowing the host bridge and deducing the
> iobase, I beleive we should export from the kernel either one iobase per
> PCI device (there can be different busses with different iobases, that's
> the case on the new macs and they have the same bus number), or we can
> have /proc/bus/pci export "fixed'up" IO addresses (already including the
> iobase).
>
> I can code those fixes, but I'm not sure what solution has most chances
> of beeing accepted (especially changing /poc/bus/pci semantics). The
> problem is that exporting an IO address alone (without the iobase) has
> really no meaning on arch that don't have a x86-like separate IO space at
> the CPU level.

Have you talked to Martin Mares about this?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven ------------- Sony Software Development Center Europe (SDCE)
Geert.Uytterhoeven@sonycom.com ------------------- Sint-Stevens-Woluwestraat 55
Voice +32-2-7248638 Fax +32-2-7262686 ---------------- B-1130 Brussels, Belgium


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [linux-fbdev] [PATCH 2.3.x] fbdev reversion
  2000-03-14 10:03 [linux-fbdev] [PATCH 2.3.x] fbdev reversion Benjamin Herrenschmidt
  2000-03-14 10:05 ` Geert Uytterhoeven
@ 2000-03-14 21:22 ` Michael Schmitz
  1 sibling, 0 replies; 10+ messages in thread
From: Michael Schmitz @ 2000-03-14 21:22 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Geert Uytterhoeven, linux-fbdev, linuxppc-dev


> >... which they can easily find out by combining /proc/bus/pci with
> >fix.{smem,mmio}_{start,len}.
> >
> >Since XFree86 4.0 already has resource handling, it should be a piece of cake
> >to remove all devices corresponding to any fix.{smem,mmio}_{start,len} from
> >their list.
>
> Well, while we are at it, we could also work on fixing the PCI problems:
> Instead of relying on XFree knowing the host bridge and deducing the
> iobase, I beleive we should export from the kernel either one iobase per
> PCI device (there can be different busses with different iobases, that's
> the case on the new macs and they have the same bus number), or we can
> have /proc/bus/pci export "fixed'up" IO addresses (already including the
> iobase).

I'd like to see something like this for instance atyfb knows about the
structure of the devices' mem region (wich memory aperture to use for vram
or mmio) but the X server messes up because it only knows the full
aperture, and detects a conflict between MMIO and vram.

If the X server uses /proc/bus/pci to work it's magic, this would be the
place to fix it.

	Michael

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [linux-fbdev] [PATCH 2.3.x] fbdev reversion
  2000-03-14 10:05 ` Geert Uytterhoeven
@ 2000-03-14 22:11   ` Michel Lanners
  2000-03-15  9:00     ` Geert Uytterhoeven
  2000-03-14 22:59   ` Gabriel Paubert
  1 sibling, 1 reply; 10+ messages in thread
From: Michel Lanners @ 2000-03-14 22:11 UTC (permalink / raw)
  To: Geert.Uytterhoeven, mj; +Cc: bh40, linux-fbdev, linuxppc-dev

[-- Attachment #1: Type: TEXT/plain, Size: 2778 bytes --]

Hi all,

On  14 Mar, this message from Geert Uytterhoeven echoed through cyberspace:
>> Well, while we are at it, we could also work on fixing the PCI problems:
>> Instead of relying on XFree knowing the host bridge and deducing the
>> iobase, I beleive we should export from the kernel either one iobase per
>> PCI device (there can be different busses with different iobases, that's
>> the case on the new macs and they have the same bus number), or we can
>> have /proc/bus/pci export "fixed'up" IO addresses (already including the
>> iobase).

I'd clearly vote for this approach. Something along those lines is
neede as well if you want to use more than one IDE controller at the
same time (or anything other than Apple's integrated controller, for
that matter).

>> I can code those fixes, but I'm not sure what solution has most chances
>> of beeing accepted (especially changing /poc/bus/pci semantics). The
>> problem is that exporting an IO address alone (without the iobase) has
>> really no meaning on arch that don't have a x86-like separate IO space at
>> the CPU level.

I have just yesterday completed my patch doing exactly that (ported
over from 2.2.x); it is, however, PowerMac-only. It's attached below,
and also fixes the missing scan of PCI buses other than bus 0 on
PowerMacs.

> Have you talked to Martin Mares about this?

Martin, what is your feeling about this? I think the IO-port-fixup
scheme could be generalized on PPC (or any PCI platform lacking true IO
space); however, it would be best if there were a generic way to find
out the processor base address of the IO space of a specific bus. On
PowerMacs, we get that from OF.

I'm thinking about something along the lines of a resource of the
parent bridge's PCI dev; much like it is available for P2P bridges. So
even though the information is not given directly by host bridges, the
platform fixup code would need to provide those through other means
(typically something firmware-derived).

This would be a step in the direction of a complete resource tree
starting at the host bridges, which, at least on PMacs, is not there
right now. That is needed, however, if dynamic resource allocation is
to produce working results ;-)

Please try this patch if you expierence PCI trouble, and also if you
have IDE in your machine; I'd like to verify I didn't break built-in
PowerMac IDE. Be warned, however....

Flames, comments, cookies, ideas? Beer?

Michel

-------------------------------------------------------------------------
Michel Lanners                 |  " Read Philosophy.  Study Art.
23, Rue Paul Henkes            |    Ask Questions.  Make Mistakes.
L-1710 Luxembourg              |
email   mlan@cpu.lu            |
http://www.cpu.lu/~mlan        |                     Learn Always. "

[-- Attachment #2: pci.diff --]
[-- Type: TEXT/plain, Size: 9373 bytes --]

diff -uNr linux-2.3.paul/include/asm-ppc/io.h linux-2.3.paul-work/include/asm-ppc/io.h
--- linux-2.3.paul/include/asm-ppc/io.h	Sat Mar 11 06:09:07 2000
+++ linux-2.3.paul-work/include/asm-ppc/io.h	Sun Mar 12 17:08:44 2000
@@ -32,7 +32,9 @@
 extern unsigned long isa_io_base;
 extern unsigned long isa_mem_base;
 extern unsigned long pci_dram_offset;
-#define _IO_BASE	isa_io_base
+/* We're correcting io base addresses in pci fixup code */
+/* #define _IO_BASE	isa_io_base */
+#define _IO_BASE	0
 #define _ISA_MEM_BASE	isa_mem_base
 #define PCI_DRAM_OFFSET	pci_dram_offset
 #endif /* CONFIG_APUS */
diff -uNr linux-2.3.paul/arch/ppc/kernel/pmac_pci.c linux-2.3.paul-work/arch/ppc/kernel/pmac_pci.c
--- linux-2.3.paul/arch/ppc/kernel/pmac_pci.c	Fri Feb 11 00:19:51 2000
+++ linux-2.3.paul-work/arch/ppc/kernel/pmac_pci.c	Mon Mar 13 21:07:21 2000
@@ -27,6 +27,15 @@

 #include "pci.h"

+#undef DEBUG
+#define DEBUG
+
+#ifdef DEBUG
+#define DBG(x...) printk(x)
+#else
+#define DBG(x...)
+#endif
+
 struct bridge_data **bridges, *bridge_list;
 static int max_bus;

@@ -41,6 +50,10 @@
 static int uninorth_default = -1;

 static void add_bridges(struct device_node *dev);
+void fix_chaos (struct bridge_data *);
+void fix_planb (struct pci_dev *);
+
+extern struct pci_ops generic_pci_ops;

 /*
  * Magic constants for enabling cache coherency in the bandit/PSX bridge.
@@ -54,6 +67,10 @@
 #define BANDIT_MAGIC	0x50
 #define BANDIT_COHERENT	0x40

+#define CONTROL_DEVID	3
+#define PLANB_DEVID	4
+#define PLANB_BASE      0xf1000000
+
 __pmac
 void *pci_io_base(unsigned int bus)
 {
@@ -629,7 +646,7 @@
 				ioremap(addr->address + 0x800000, 0x1000);
 			bp->cfg_data = (volatile unsigned char *)
 				ioremap(addr->address + 0xc00000, 0x1000);
-			bp->io_base = (void *) ioremap(addr->address, 0x10000);
+			bp->io_base = (void *) ioremap(addr->address, 0x800000);
 		}
 		if (isa_io_base == 0)
 			isa_io_base = (unsigned long) bp->io_base;
@@ -664,8 +681,11 @@
 		if (reg == 0 || ((reg[0] >> 8) & 0xff) != dev->devfn)
 			continue;
 		/* this is the node, see if it has interrupts */
-		if (node->n_intrs > 0)
+		if (node->n_intrs > 0)  {
 			dev->irq = node->intrs[0].line;
+			DBG("PCI: Setting IRQ %d on device %s.\n",
+				dev->irq, dev->slot_name);
+		}
 		break;
 	}
 }
@@ -674,35 +694,214 @@
 pmac_pcibios_fixup(void)
 {
 	struct pci_dev *dev;
+	struct bridge_data *bp;
+	int i;

 	/*
-	 * FIXME: This is broken: We should not assign IRQ's to IRQless
-	 *	  devices (look at PCI_INTERRUPT_PIN) and we also should
-	 *	  honor the existence of multi-function devices where
-	 *	  different functions have different interrupt pins. [mj]
+	 * The generic PCI code scans only bus 0 for devices and P2P
+	 * bridges. We fix this here based on the array of host
+	 * bridges.
 	 */
+	for (bp = bridge_list; bp != NULL; bp = bp->next) {
+
+		if (bp->bus_number == 0) continue;
+
+		if (strcmp(bp->node->name, "chaos") == 0)
+			fix_chaos(bp);
+		else
+			pci_scan_bus(bp->bus_number, &generic_pci_ops, NULL);
+	}
+
 	pci_for_each_dev(dev)
 	{
-		/*
-		 * Open Firmware often doesn't initialize the,
-		 * PCI_INTERRUPT_LINE config register properly, so we
-		 * should find the device node and se if it has an
-		 * AAPL,interrupts property.
-		 */
 		struct bridge_data *bp = bridges[dev->bus->number];
 		unsigned char pin;
+
+		DBG("PCI: Fixing device %02x:%02x (%04x:%04x)\n",
+			dev->bus->number, dev->devfn,
+			dev->vendor, dev->device);
+
+		/* SPECIAL DEVICES
+		 * ---------------
+		 * control was fixed in fix_chaos().
+		 */
+		if (dev->vendor == APPLE_VENDID &&
+		    dev->device == CONTROL_DEVID) {
+			continue;
+		}
+		/* planb needs special care.
+		 */
+		if (dev->vendor == APPLE_VENDID &&
+		    dev->device == PLANB_DEVID) {
+			fix_planb (dev);
+			continue;
+		}
+		/* INTERRUPT FIXING
+		 * ----------------
+		 * Open Firmware doesn't initialize the PCI_INTERRUPT_LINE
+		 * config register, so we need to find the device node and
+		 * see if it has an AAPL,interrupts property.
+		 *
+		 * Note that INTA# - INTD# are OR'ed together per slot,
+		 * so no need to worry about multifunction cards.
+		 */

-		if (pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin) ||
-		    !pin)
-			continue; /* No interrupt generated -> no fixup */
-		/* We iterate all instances of uninorth for now */
-		if (uninorth_count && dev->bus->number == 0) {
-			int i;
-			for (i=0;i<uninorth_count;i++)
-				fix_intr(uninorth_bridges[i].node->child, dev);
-		} else
-                	fix_intr(bp->node->child, dev);
+		/* Is there an interrupt? */
+		if ( !(pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin) ||
+		    !pin) ) {
+			/* We iterate all instances of uninorth for now */
+			if (uninorth_count && dev->bus->number == 0) {
+				int i;
+				for (i=0;i<uninorth_count;i++)
+					fix_intr(uninorth_bridges[i].node->child, dev);
+			} else
+       		         	fix_intr(bp->node->child, dev);
+		}
+		/*
+		 * Since the PMac host bridges do translate IO accesses,
+		 * we correct the kernel PCI device database here with an
+		 * offset we add to IO region's base address. The exact
+		 * offset is provided by the bridge controling the
+		 * device's bus.
+		 */
+		if (dev->hdr_type != PCI_HEADER_TYPE_NORMAL) goto nextfix;
+
+		for(i=0; i<6; i++) {
+			struct resource *res = &dev->resource[i];
+			unsigned long base;
+			unsigned long a = res->start;
+
+			if (res->flags & PCI_BASE_ADDRESS_SPACE_IO) {
+				base = (unsigned long)
+					pci_io_base(dev->bus->number);
+				/* unassigned region? */
+				if (a == 0) continue;
+				/* strange value? */
+				if (a > 0x800000) continue;
+				if (a < base) a += base;
+				if (a > base) {
+					res->start = a;
+					res->end += base;
+					DBG("PCI: Correcting IO address %d on "
+					    "device %s, now %08lx.\n",
+					    i, dev->slot_name, a);
+				}
+			}
+		}
+nextfix:
+		/*
+		 * Open Firmware does not enable I/O and memory space
+		 * response on PCI devices. We try to fix this, but we need
+		 * to be sure that OF didn't forget to assign an address
+		 * to the device. [mj]
+		 *
+		 * FIXME: How can we know? We should use OF properties....
+		 *        Or maybe the new 2.3 resource code?
+		 */
+		pcibios_enable_device(dev);
 	}
+}
+
+/*
+ * The chaos hostbridge, controlling the separate video bus
+ * on the 7x00/8x00 PowerMacs, doesn't like being probed for
+ * attached devices. Therefore, we rely on OF to discover those.
+ */
+
+void __init
+fix_chaos (struct bridge_data *bp)
+{
+	struct device_node *nd;
+	struct pci_dev temp;
+	struct pci_dev *dev = NULL;
+	struct pci_bus *b;
+
+	b = pci_alloc_primary_bus(bp->bus_number);
+	b->sysdata = NULL;
+	b->ops = &generic_pci_ops;
+	b->subordinate = bp->max_bus;
+	/*
+	 * Walk OF's list of devices on this bus.
+	 */
+	for (nd = bp->node->child; nd; nd = nd->sibling) {
+		/*
+		 * We need at least one address entry to get the PCI
+		 * device / function values.
+		 */
+		if (nd->n_addrs == 0) {
+			printk(KERN_ERR "PCI: %s: not a PCI device!\n",
+				nd->name);
+			continue;
+		}
+		temp.devfn = (nd->addrs[0].space >> 8) & 0xff;
+		temp.bus = b;
+		pci_scan_slot(&temp);
+	}
+	return;
+}
+
+void __init
+fix_planb (struct pci_dev *pcidev)
+{
+	/* There is a bug with the way OF assigns addresses
+	 * to the devices behind the chaos bridge.
+	 * control needs only 0x1000 of space, yet decodes only
+	 * the upper 16 bits. It therefore occupies a full 64K.
+	 * OF assigns the planb controller memory within this space;
+	 * so we need to change that here in order to access planb.
+	 * Note that the new address (0xf1000000) is within chaos'
+	 * address space, so it should never get assigned to other
+	 * devices by OF.
+	 * planb also gets its interrupt set.
+	 */
+	struct device_node *planb_device;
+	struct resource *res;
+	unsigned char bus, devfn, confreg;
+	unsigned int reg;
+	u32 sz;
+
+	DBG("PCI: fixing PlanB...\n");
+	planb_device = find_devices("planb");
+	if (planb_device == 0) {
+		printk(KERN_WARNING "PCI: Error fixing planb: no OF device.\n");
+		return;
+	}
+	if (planb_device->next != NULL)
+		printk(KERN_WARNING "PCI: Only fixing first planb device.\n");
+
+	if (planb_device->n_addrs != 1) {
+		printk(KERN_WARNING "PCI: Error fixing planb: expected 1 "
+			"address, got %d.\n", planb_device->n_addrs);
+		return;
+	}
+	if (planb_device->n_intrs == 0) {
+		printk(KERN_WARNING "PCI: Error fixing planb: no IRQ.\n");
+		return;
+	}
+	bus = (planb_device->addrs[0].space >> 16) & 0xff;
+	devfn = (planb_device->addrs[0].space >> 8) & 0xff;
+	if ((bus != pcidev->bus->number) || (devfn != pcidev->devfn)) {
+		printk(KERN_WARNING "PCI: Error fixing planb: OF and PCI "
+			"device don't match!\n");
+		return;
+	}
+	pcidev->irq = planb_device->intrs[0].line;
+	confreg = planb_device->addrs[0].space & 0xff;
+	reg = (confreg - PCI_BASE_ADDRESS_0) >> 2;
+	/* Set the new base address */
+	pcibios_write_config_dword(bus, devfn, confreg, ~0);
+	pcibios_read_config_dword(bus, devfn, confreg, &sz);
+	pcibios_write_config_dword(bus, devfn, confreg, PLANB_BASE);
+	res = &pcidev->resource[reg];
+	res->start = PLANB_BASE & PCI_BASE_ADDRESS_MEM_MASK;
+	sz = ~(sz & PCI_BASE_ADDRESS_MEM_MASK);
+	res->end = res->start + (unsigned long) sz;
+	/*
+	 * Everything else should be set up right from the generic scan
+	 * code. Now enable PlanB...
+	 */
+	pcibios_enable_device(pcidev);
+	return;
 }

 void __init

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [linux-fbdev] [PATCH 2.3.x] fbdev reversion
  2000-03-14 10:05 ` Geert Uytterhoeven
  2000-03-14 22:11   ` Michel Lanners
@ 2000-03-14 22:59   ` Gabriel Paubert
  1 sibling, 0 replies; 10+ messages in thread
From: Gabriel Paubert @ 2000-03-14 22:59 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Benjamin Herrenschmidt, linux-fbdev, linuxppc-dev




On Tue, 14 Mar 2000, Geert Uytterhoeven wrote:

> > I can code those fixes, but I'm not sure what solution has most chances
> > of beeing accepted (especially changing /poc/bus/pci semantics). The
> > problem is that exporting an IO address alone (without the iobase) has
> > really no meaning on arch that don't have a x86-like separate IO space at
> > the CPU level.

Indeed, there is one and only one CPU address space on _all_ architectures
except x86 (even HP's IA64 has does not have the silly I/O space). This
makes /proc/ioports an heresy while /proc/iomem is reasonable. How do you
handle I/O accesses in a machine with several bridges which have
independant PCI I/O spaces which a) appear at discontiguous physical
addresses b) map to the same or overlapping range of I/O addresses c)
support drivers which access hardwired I/O addresses ?

That's simple, there is no solution ;-)(especially if, god forbid, several
buses have an ISA bridge) So you have to select your goals:

- support non-transparent bridges, which is what PPC bridges are,
as "transparently" as possible, this will give support for non transparent
bridges (DEC21554) used in clusters almost for free

- get rid of iobase since the number of instructions which load iobase
into a register in an average kernel is far from negligible and this will
save a few pages: in some sequences it's about 30% of the instructions

- maintain maximum compatibility with existing code (don't need to modify
most drivers),

I think this is possible provided you accept to kill condition c) above,
forcing to modify many ISA drivers. This probably makes it a 2.5 task...

I might even give it a try because it would make inserting my VME driver
and devices inside the resource tree feasible, while it's currently a
mess.

	Gabriel.


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [linux-fbdev] [PATCH 2.3.x] fbdev reversion
  2000-03-14 22:11   ` Michel Lanners
@ 2000-03-15  9:00     ` Geert Uytterhoeven
  2000-03-15  9:57       ` Gabriel Paubert
  0 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2000-03-15  9:00 UTC (permalink / raw)
  To: Michel Lanners; +Cc: mj, bh40, linux-fbdev, linuxppc-dev


On Tue, 14 Mar 2000, Michel Lanners wrote:
> Flames, comments, cookies, ideas? Beer?

#define _IO_BASE       0

breaks CHRP and PReP boxes that use plain ISA drivers (inb() and friends).

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven ------------- Sony Software Development Center Europe (SDCE)
Geert.Uytterhoeven@sonycom.com ------------------- Sint-Stevens-Woluwestraat 55
Voice +32-2-7248638 Fax +32-2-7262686 ---------------- B-1130 Brussels, Belgium


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [linux-fbdev] [PATCH 2.3.x] fbdev reversion
  2000-03-15  9:00     ` Geert Uytterhoeven
@ 2000-03-15  9:57       ` Gabriel Paubert
  2000-03-15 11:31         ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 10+ messages in thread
From: Gabriel Paubert @ 2000-03-15  9:57 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Michel Lanners, mj, bh40, linux-fbdev, linuxppc-dev




On Wed, 15 Mar 2000, Geert Uytterhoeven wrote:

>
> On Tue, 14 Mar 2000, Michel Lanners wrote:
> > Flames, comments, cookies, ideas? Beer?
>
> #define _IO_BASE       0
>
> breaks CHRP and PReP boxes that use plain ISA drivers (inb() and friends).

Indeed, but that's unavoidable. Except that the breakage has to wait for
2.5 to be acceptable. Given the choice I prefer to break this and properly
use resources even for drivers which believe that they have to use fixed
addresses assigned by God, or rather the devil given the mess that the PC
`architecture' is.

Then all ISA drivers would have to ask with some function (a substitute
for request_region) for a resource and take te I/O address from the
resource.start field. This avoids useless code bloat in so many other
drivers which right now add 3 elements for most I/O accesses: _IO_BASE,
their own port base address, and a register offset.

out[wl] instructions are defined as inline assembly modifying volatile
memory and the compiler has to assume that this could modify any variable,
among them _IO_BASE, which means that it is reloaded between two succesive
outs for example and the address for the next out[wl] is recomputed from
scratch. This is one load + 2 adds at least since we force the address to
be in a register for st[hw]brx instructions. That's bloat and that's IMHO
the most important reason for which _IO_BASE should be killed.

This said if anybody knows a trick to avoid this kind of stupid code
generation from the compiler (I suspect a __builtin_byteswap will have to
be implemented sooner or later as well with compiler constraints to use
indexed addressing mode only which is needed anyway for Altivec).

	Gabriel.


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [linux-fbdev] [PATCH 2.3.x] fbdev reversion
  2000-03-15  9:57       ` Gabriel Paubert
@ 2000-03-15 11:31         ` Benjamin Herrenschmidt
  2000-03-15 20:38           ` Michel Lanners
  2000-03-15 21:06           ` Gabriel Paubert
  0 siblings, 2 replies; 10+ messages in thread
From: Benjamin Herrenschmidt @ 2000-03-15 11:31 UTC (permalink / raw)
  To: Gabriel Paubert, linux-fbdev, linuxppc-dev


On Wed, Mar 15, 2000, Gabriel Paubert <paubert@iram.es> wrote:

>> #define _IO_BASE       0
>>
>> breaks CHRP and PReP boxes that use plain ISA drivers (inb() and friends).
>
>Indeed, but that's unavoidable. Except that the breakage has to wait for
>2.5 to be acceptable. Given the choice I prefer to break this and properly
>use resources even for drivers which believe that they have to use fixed
>addresses assigned by God, or rather the devil given the mess that the PC
>`architecture' is.

Well, we need a fix _now_ (that I will backport to 2.2.x as soon as
possible too), at least for PowerMacs. The problem of ISA drivers with
legacy hard-coded addresses is indeed a real one, but it can be
temporarily worked around by having the kernel maintaing a separate
mapping of the "old" IO base to the legacy ISA bus if one exist.

We have, I think, no legacy code using /proc/bus/pci, do we ? If I'm
right, then we can chance it to return physical addresses without
breaking anyone so at least we have a temporary fix for XFree. We must
get the kernel<->userland interface right asap so that we don't need to
change XFree again and again.

I'll play a bit with Michel patches next week-end. Unfortunately, I can't
test much on 2.3.x until the OHCI driver is fixed.

Ben.


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [linux-fbdev] [PATCH 2.3.x] fbdev reversion
  2000-03-15 11:31         ` Benjamin Herrenschmidt
@ 2000-03-15 20:38           ` Michel Lanners
  2000-03-15 21:06           ` Gabriel Paubert
  1 sibling, 0 replies; 10+ messages in thread
From: Michel Lanners @ 2000-03-15 20:38 UTC (permalink / raw)
  To: bh40; +Cc: paubert, linux-fbdev, linuxppc-dev


On  15 Mar, this message from Benjamin Herrenschmidt echoed through cyberspace:

>>> #define _IO_BASE       0
>>>
>>> breaks CHRP and PReP boxes that use plain ISA drivers (inb() and friends).
>>
>>Indeed, but that's unavoidable. Except that the breakage has to wait for
>>2.5 to be acceptable. Given the choice I prefer to break this and properly
>>use resources even for drivers which believe that they have to use fixed
>>addresses assigned by God, or rather the devil given the mess that the PC
>>`architecture' is.
>
> Well, we need a fix _now_ (that I will backport to 2.2.x as soon as
> possible too), at least for PowerMacs.

Hmmm, I do vote for a solution like mine _right now_ for PMac, given
that PRep and CHRP do use real ISA. That means the above #define can't
be used, but we need to get back to a working ppc_md.something aproach.

This leaves time to implement a unified solution for 2.5, but that
solution needs to be coordinated with other platforms.

> The problem of ISA drivers with
> legacy hard-coded addresses is indeed a real one, but it can be
> temporarily worked around by having the kernel maintaing a separate
> mapping of the "old" IO base to the legacy ISA bus if one exist.

That would be difficult to implement... as all inb() and friends
include the _IO_BASE offset. Either you remove that, and you need to
map the ISA bus at kernel address 0x0, or you leave it in, but then
inb() are unusable from drivers using 'corrected' IO ports.

> We have, I think, no legacy code using /proc/bus/pci, do we ? If I'm
> right, then we can change it to return physical addresses without
> breaking anyone so at least we have a temporary fix for XFree. We must
> get the kernel<->userland interface right asap so that we don't need to
> change XFree again and again.

Agreed. But, now that I think about it: what address space should the
'translated' port be in? Processor physical or kernel virtual? They are
the same for the addresses I've come across on my PMac, but they need
not be... PMac bridge init code ioremap()'s the IO region; is that the
same on other platforms/architectures?

> I'll play a bit with Michel patches next week-end. Unfortunately, I can't
> test much on 2.3.x until the OHCI driver is fixed.

Good luck!

-------------------------------------------------------------------------
Michel Lanners                 |  " Read Philosophy.  Study Art.
23, Rue Paul Henkes            |    Ask Questions.  Make Mistakes.
L-1710 Luxembourg              |
email   mlan@cpu.lu            |
http://www.cpu.lu/~mlan        |                     Learn Always. "


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [linux-fbdev] [PATCH 2.3.x] fbdev reversion
  2000-03-15 11:31         ` Benjamin Herrenschmidt
  2000-03-15 20:38           ` Michel Lanners
@ 2000-03-15 21:06           ` Gabriel Paubert
  1 sibling, 0 replies; 10+ messages in thread
From: Gabriel Paubert @ 2000-03-15 21:06 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-fbdev, linuxppc-dev




On Wed, 15 Mar 2000, Benjamin Herrenschmidt wrote:

> Well, we need a fix _now_ (that I will backport to 2.2.x as soon as
> possible too), at least for PowerMacs. The problem of ISA drivers with
> legacy hard-coded addresses is indeed a real one, but it can be
> temporarily worked around by having the kernel maintaing a separate
> mapping of the "old" IO base to the legacy ISA bus if one exist.

- Mapping address 0 does not help in catching null pointer accesses and
has the slight defect of overlapping  user space :-),

-  conditionally adding _IO_BASE if address is too low will bloat code

- systematically oring with _IO_BASE instead of adding it might prove
reasonable (oring with 0 has little side effects for PMacs, and on all
other boards I know it should work properly given that the alignement
of I/O space is larger than its size), although it does not solve the code
bloat issue.

But you have to consider that, AFAIK, on all architectures, the I/O space
is accessed using the start field in the resource struct. So these have to
be kernel virtual addresses, which are hard to distinguish from physical
addresses with 1:1 mappings as we have now. But that's exactly this
confusion of physical addresses with kernel virtual addresses which
prohibits increasing user space to 3Gb since on PreP I/O is mapped at 2
Gb...

MMIO is not a problem since what is in the resource field is first
translated by ioremap() so that you can put physical addresses. But for
I/O space some form of cookie is used which can be used as a parameter
for in and out instructions and is wildly varying depending on
architectures:

- on PPC, add _IO_BASE and use the result as kernel _virtual_ address.

- on SPARC64, I don't know what the address space identifiers exactly are,
but it seems to use only physical addresses. SPARC32 on the other hand
uses kernel virtual addresses but they don't have any ISA legacy it seems.

- on ALPHA, seems a mess (just spotted in core_tsunami.h):
        /* ??? I wish I could get rid of this.  But there's no ioremap
           equivalent for I/O space.  PCI I/O can be forced into the
           correct hose's I/O region, but that doesn't take care of
           legacy ISA crap.  */
        addr += TSUNAMI_IO_BIAS;
        return __kernel_ldbu(*(vucp)addr);

and it's much worse with generic kernels or older machines which did not
support byte and 16 bit transfers. Ok, they acknowledge that something has
to be done for legacy I/O, perhaps functions prefixed with isa_ IIRC what
Richard Henderson said at some point.

- on ix86: fairly straightforward for obvious reasons, after all it's the
machine that we inherit the marvellously broken concept of separate spaces
from, and it's inherited from the 8008...

- on IA64, extremely baroque, using 4kB of virtual address space for every
4 bytes of I/O space and still limited to 64kB. Some things never
change...

I can't think of any simple solution right now, using physical addresses
does not seem to be an option because it forces 1:1 mappings and it's
definitely not what we want. How does cat /proc/ioports (which now
actually lists the contents of pci devices resource fields) look like on
architectures other than x86 and PPC with a recent 2.3 ?

> We have, I think, no legacy code using /proc/bus/pci, do we ? If I'm
> right, then we can chance it to return physical addresses without
> breaking anyone so at least we have a temporary fix for XFree. We must
> get the kernel<->userland interface right asap so that we don't need to
> change XFree again and again.

I suspect you mean /proc/bus/pci/devices not /proc/bus/pci/xx/xx.x which
directly access configuration space and can't be fixed up.

I agree to put physical addresses for MMIO (actually I do it by fixing up
addresses in resource srtuctures when the bridge is in PreP mode), but it
requires more thought for I/O space, we want a definitive solution not one
which will break in another 6 months.

BTW: is it possible to add specific ioctls to /proc code, this might be a
solution, ugly but given the current state it might be the only solution.

	Gabriel.


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2000-03-15 21:06 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2000-03-14 10:03 [linux-fbdev] [PATCH 2.3.x] fbdev reversion Benjamin Herrenschmidt
2000-03-14 10:05 ` Geert Uytterhoeven
2000-03-14 22:11   ` Michel Lanners
2000-03-15  9:00     ` Geert Uytterhoeven
2000-03-15  9:57       ` Gabriel Paubert
2000-03-15 11:31         ` Benjamin Herrenschmidt
2000-03-15 20:38           ` Michel Lanners
2000-03-15 21:06           ` Gabriel Paubert
2000-03-14 22:59   ` Gabriel Paubert
2000-03-14 21:22 ` Michael Schmitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).