On Wed, Apr 06, 2011 at 01:22:41PM +0200, Peter Hallin wrote:
> On 2011-04-05 14:35, Claudio Jeker wrote:
> > Can you give the following diff a spin and see if that makes the card act
> > faster. This disables the ppb hotplug interrupt which is shared with the
> > em2 and em3 interrupts.
> >
> > --
> > :wq Claudio
>
> Ok, that did the trick.
>
> I made the changes to the 4.8 source and ppb hotplug was disabled.
>
> I then tested the dual port cards and got close to 1 Gbit/s but without
> the high CPU usage (only about 30% intr).
>
> So my question now is: Do we need the ppb hotplug? What is it good for?
It is needed for handling hotplug events especially on the expresscard
slots on modern laptops.
Here is a better version that may get commited if it works for you.
Currently only amd64 is fixed, we're looking into i386 to do the same
dance with the interrupt return values.
So the idea is to establish the interrupt handler for the ppb as last and
jump out of interrupt processing if the handler returns 1 (HW was the
source of interrupt). So we should not end up in the slow ppb interrupt
handler unless it is actually a hotplug interrupt.
--
:wq Claudio
Index: arch/amd64/amd64/vector.S
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/vector.S,v
retrieving revision 1.28
diff -u -p -r1.28 vector.S
--- arch/amd64/amd64/vector.S 1 Apr 2011 22:51:45 -0000 1.28
+++ arch/amd64/amd64/vector.S 6 Apr 2011 13:18:45 -0000
@@ -484,7 +484,9 @@ IDTVEC(intr_##name##num)
;\
call *IH_FUN(%rbx) /* call it */ ;\
orq %rax,%rax /* should it be counted? */ ;\
jz 4f ;\
- incq IH_COUNT(%rbx) ;\
+ incq IH_COUNT(%rbx) /* -1 or 1 */ ;\
+ orq %rax,%rax ;\
+ jns 5f ;\
4: movq IH_NEXT(%rbx),%rbx /* next handler in chain */ ;\
testq %rbx,%rbx ;\
jnz 6b ;\
Index: dev/pci/ppb.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/ppb.c,v
retrieving revision 1.47
diff -u -p -r1.47 ppb.c
--- dev/pci/ppb.c 30 Dec 2010 00:58:22 -0000 1.47
+++ dev/pci/ppb.c 6 Apr 2011 12:50:33 -0000
@@ -142,7 +142,7 @@ ppbattach(struct device *parent, struct
pci_intr_handle_t ih;
pcireg_t busdata, reg, blr;
char *name;
- int pin;
+ int pin, has_hotplug = 0;
sc->sc_pc = pc;
sc->sc_tag = pa->pa_tag;
@@ -169,21 +169,9 @@ ppbattach(struct device *parent, struct
/* Check for PCI Express capabilities and setup hotplug support. */
if (pci_get_capability(pc, pa->pa_tag, PCI_CAP_PCIEXPRESS,
&sc->sc_cap_off, ®) && (reg & PCI_PCIE_XCAP_SI)) {
- if (pci_intr_map(pa, &ih) == 0)
- sc->sc_intrhand = pci_intr_establish(pc, ih, IPL_TTY,
- ppb_intr, sc, self->dv_xname);
-
- if (sc->sc_intrhand) {
+ if (pci_intr_map(pa, &ih) == 0) {
printf(": %s", pci_intr_string(pc, ih));
-
- /* Enable hotplug interrupt. */
- reg = pci_conf_read(pc, pa->pa_tag,
- sc->sc_cap_off + PCI_PCIE_SLCSR);
- reg |= (PCI_PCIE_SLCSR_HPE | PCI_PCIE_SLCSR_PDE);
- pci_conf_write(pc, pa->pa_tag,
- sc->sc_cap_off + PCI_PCIE_SLCSR, reg);
-
- timeout_set(&sc->sc_to, ppb_hotplug_insert_finish, sc);
+ has_hotplug = 1;
}
}
@@ -305,6 +293,22 @@ ppbattach(struct device *parent, struct
pba.pba_intrtag = pa->pa_intrtag;
sc->sc_psc = config_found(self, &pba, ppbprint);
+
+ if (has_hotplug) {
+ sc->sc_intrhand = pci_intr_establish(pc, ih, IPL_TTY,
+ ppb_intr, sc, self->dv_xname);
+ if (sc->sc_intrhand) {
+
+ /* Enable hotplug interrupt. */
+ reg = pci_conf_read(pc, pa->pa_tag,
+ sc->sc_cap_off + PCI_PCIE_SLCSR);
+ reg |= (PCI_PCIE_SLCSR_HPE | PCI_PCIE_SLCSR_PDE);
+ pci_conf_write(pc, pa->pa_tag,
+ sc->sc_cap_off + PCI_PCIE_SLCSR, reg);
+
+ timeout_set(&sc->sc_to, ppb_hotplug_insert_finish, sc);
+ }
+ }
}
int
