On Wed, Apr 06, 2011 at 03:55:03PM +0200, Claudio Jeker wrote:
> 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.
Wait. It seems more is needed. Will come back when we have a better
solution.
> --
> :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
>
--
:wq Claudio
