| Message ID | m3lhneez9h.fsf@t19.piap.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2014-11-14 16:35 GMT+04:00 Krzysztof Ha?asa <khalasa@piap.pl>: > The IRQs have to be acknowledged before they are serviced, otherwise some events > may be skipped. Also, acknowledging IRQs just before returning from the handler > doesn't leave enough time for the device to deassert the INTx line, and for > bridges to propagate this change. This resulted in twice the IRQ rate on ARMv6 > dual core CPU. Thanks! I'm not experienced in interaction with hardware in this regard... could you please point to some reading which explains this moment? Or you just know this from experience? The solo device specs are very terse about this, so I considered that it should work fine without regard to how fast we write back to that register.
Also while you're at it, and if this really makes sense, you could merge these two writes (unrecognized bits, then recognized bits) to one write act.
Andrey Utkin <andrey.krieger.utkin@gmail.com> writes: > could you please point to some reading which explains this moment? Or > you just know this from experience? The solo device specs are very > terse about this, so I considered that it should work fine without > regard to how fast we write back to that register. The SOLO IRQ controller does the common thing, all drivers (for chips using the relatively modern "write 1 to clear") have to follow this sequence: first ACK the interrupts sources (so they are deasserted, though they can be asserted again if new events arrive), and only then service the chip. > Also while you're at it, and if this really makes sense, you could > merge these two writes (unrecognized bits, then recognized bits) to > one write act. I think my patch does exactly this, merges both writes.
2014-11-15 1:33 GMT+04:00 Krzysztof Ha?asa <khalasa@piap.pl>: > The SOLO IRQ controller does the common thing, all drivers (for chips > using the relatively modern "write 1 to clear") have to follow this > sequence: first ACK the interrupts sources (so they are deasserted, > though they can be asserted again if new events arrive), and only then > service the chip. Thanks for explanation. > I think my patch does exactly this, merges both writes. Ah right, sorry.
Krzysztof, it seems to really help. The host is working stable for 2.5 hours at the moment, with original framerate of 2 fps. Thank you very much.
Hi Andrey, Please always prefix the subject line with [PATCH] when you post a patch. That way it will be picked up by patchwork (https://patchwork.linuxtv.org/project/linux-media/list/) and the patch won't be lost. Can you repost with such a prefix? Thanks! Hans On 11/15/2014 11:34 AM, Andrey Utkin wrote: > From: khalasa@piap.pl (Krzysztof =?utf-8?Q?Ha=C5=82asa?=) > > The IRQs have to be acknowledged before they are serviced, otherwise some events > may be skipped. Also, acknowledging IRQs just before returning from the handler > doesn't leave enough time for the device to deassert the INTx line, and for > bridges to propagate this change. This resulted in twice the IRQ rate on ARMv6 > dual core CPU. > > Signed-off-by: Krzysztof Ha?asa <khalasa@piap.pl> > Acked-by: Andrey Utkin <andrey.utkin@corp.bluecherry.net> > Tested-by: Andrey Utkin <andrey.utkin@corp.bluecherry.net> > > --- a/drivers/media/pci/solo6x10/solo6x10-core.c > +++ b/drivers/media/pci/solo6x10/solo6x10-core.c > @@ -105,11 +105,8 @@ static irqreturn_t solo_isr(int irq, void *data) > if (!status) > return IRQ_NONE; > > - if (status & ~solo_dev->irq_mask) { > - solo_reg_write(solo_dev, SOLO_IRQ_STAT, > - status & ~solo_dev->irq_mask); > - status &= solo_dev->irq_mask; > - } > + /* Acknowledge all interrupts immediately */ > + solo_reg_write(solo_dev, SOLO_IRQ_STAT, status); > > if (status & SOLO_IRQ_PCI_ERR) > solo_p2m_error_isr(solo_dev); > @@ -132,9 +129,6 @@ static irqreturn_t solo_isr(int irq, void *data) > if (status & SOLO_IRQ_G723) > solo_g723_isr(solo_dev); > > - /* Clear all interrupts handled */ > - solo_reg_write(solo_dev, SOLO_IRQ_STAT, status); > - > return IRQ_HANDLED; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 14 Nov 2014 13:35:06 +0100 khalasa@piap.pl (Krzysztof Ha?asa) wrote: > The IRQs have to be acknowledged before they are serviced, otherwise > some events may be skipped. Also, acknowledging IRQs just before > returning from the handler doesn't leave enough time for the device > to deassert the INTx line, and for bridges to propagate this change. > This resulted in twice the IRQ rate on ARMv6 dual core CPU. > > Signed-off-by: Krzysztof Ha?asa <khalasa@piap.pl> > > --- a/drivers/media/pci/solo6x10/solo6x10-core.c > +++ b/drivers/media/pci/solo6x10/solo6x10-core.c > @@ -105,11 +105,8 @@ static irqreturn_t solo_isr(int irq, void *data) > if (!status) > return IRQ_NONE; > > - if (status & ~solo_dev->irq_mask) { > - solo_reg_write(solo_dev, SOLO_IRQ_STAT, > - status & ~solo_dev->irq_mask); > - status &= solo_dev->irq_mask; > - } > + /* Acknowledge all interrupts immediately */ > + solo_reg_write(solo_dev, SOLO_IRQ_STAT, status); > > if (status & SOLO_IRQ_PCI_ERR) > solo_p2m_error_isr(solo_dev); > @@ -132,9 +129,6 @@ static irqreturn_t solo_isr(int irq, void *data) > if (status & SOLO_IRQ_G723) > solo_g723_isr(solo_dev); > > - /* Clear all interrupts handled */ > - solo_reg_write(solo_dev, SOLO_IRQ_STAT, status); > - > return IRQ_HANDLED; > } > > Signed-off-by: Ismael Luceno <ismael.luceno@corp.bluecherry.net>
--- a/drivers/media/pci/solo6x10/solo6x10-core.c +++ b/drivers/media/pci/solo6x10/solo6x10-core.c @@ -105,11 +105,8 @@ static irqreturn_t solo_isr(int irq, void *data) if (!status) return IRQ_NONE; - if (status & ~solo_dev->irq_mask) { - solo_reg_write(solo_dev, SOLO_IRQ_STAT, - status & ~solo_dev->irq_mask); - status &= solo_dev->irq_mask; - } + /* Acknowledge all interrupts immediately */ + solo_reg_write(solo_dev, SOLO_IRQ_STAT, status); if (status & SOLO_IRQ_PCI_ERR) solo_p2m_error_isr(solo_dev); @@ -132,9 +129,6 @@ static irqreturn_t solo_isr(int irq, void *data) if (status & SOLO_IRQ_G723) solo_g723_isr(solo_dev); - /* Clear all interrupts handled */ - solo_reg_write(solo_dev, SOLO_IRQ_STAT, status); - return IRQ_HANDLED; }
The IRQs have to be acknowledged before they are serviced, otherwise some events may be skipped. Also, acknowledging IRQs just before returning from the handler doesn't leave enough time for the device to deassert the INTx line, and for bridges to propagate this change. This resulted in twice the IRQ rate on ARMv6 dual core CPU. Signed-off-by: Krzysztof Ha?asa <khalasa@piap.pl>