Re: Question on MSI support in PCI and PCI-E devices

[Andrey Utkin <andrey.krieger.utkin@gmail.com>]

2015-02-12 16:48 GMT+02:00 Stephen Hemminger <stephen@networkplumber.org>:
> On Wed, 11 Feb 2015 18:19:00 +0000
> Andrey Utkin <andrey.krieger.utkin@gmail.com> wrote:
>
>> Is it true that _every_ PCI or PCI Express device supporting MSI is
>> indicated by some mention of MSI in "lspci -v", and if there's no such
>> mention, it surely doesn't support MSI?
>>
>
> Look at kernel source (drivers/pci/msi.c) function pci_msi_supported
> there are many things which can block MSI.
>
> There can be cases where PCI quirks in kernel block MSI
> because for example the device supports MSI, but the motherboard
> BIOS is broken. This only happens on really old systems.

Thank you for your reply.
However, I was more interested in the case when lspci for device
doesn't mention MSI at all, so I wonder if it makes sense to try to
enable it in the driver at all.

04:05.0 Multimedia video controller: Bluecherry BC-H16480A 16 port
H.264 video and audio encoder / decoder
        Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop-
ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap- 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium
>TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 64 (250ns max), Cache Line Size: 32 bytes
        Interrupt: pin A routed to IRQ 16
        Region 0: Memory at faff0000 (32-bit, prefetchable) [size=64K]
        Kernel driver in use: solo6x10

We have such cards, and we have issues with them - at some moment they
stop producing interrupts. No matter whether they share interrupt
number or not.
There was a recent commit from Krzysztof Hałasa
(3c787b108fe0d1c341a76e718a25897ae14673cf) which improved things, but
the issue still happens regularly on some setups.
Now I've tried the following change, i've introduced such a loop which
I see in bt8xx and ddbridge drivers. This also didn't help. So I'm out
of ideas now (any comments are highly appreciated!); I have read about
MSI, that this interrupts transmission mechanism is more reliable and
fast, but from lspci output it is not clear whether our cards support
MSI at all.

--- a/drivers/media/pci/solo6x10/solo6x10-core.c
+++ b/drivers/media/pci/solo6x10/solo6x10-core.c
@@ -100,10 +100,13 @@ static irqreturn_t solo_isr(int irq, void *data)
        struct solo_dev *solo_dev = data;
        u32 status;
        int i;
+       int handled = 0;

+       while (1) {
        status = solo_reg_read(solo_dev, SOLO_IRQ_STAT);
        if (!status)
-               return IRQ_NONE;
+               break;
+       handled++;

        /* Acknowledge all interrupts immediately */
        solo_reg_write(solo_dev, SOLO_IRQ_STAT, status);
@@ -129,7 +132,11 @@ static irqreturn_t solo_isr(int irq, void *data)
        if (status & SOLO_IRQ_G723)
                solo_g723_isr(solo_dev);

-       return IRQ_HANDLED;
+       }
+
+       if (handled > 1)
+               solo_dev->isr_more_laps++;
+       return IRQ_RETVAL(handled);
 }

 static void free_solo_dev(struct solo_dev *solo_dev)
@@ -232,6 +239,16 @@ static ssize_t p2m_timeouts_show(struct device *dev,
        return sprintf(buf, "%d\n", solo_dev->p2m_timeouts);
 }

+static ssize_t isr_more_laps_show(struct device *dev,
+                                 struct device_attribute *attr,
+                                 char *buf)
+{
+       struct solo_dev *solo_dev =
+               container_of(dev, struct solo_dev, dev);
+
+       return sprintf(buf, "%d\n", solo_dev->isr_more_laps);
+}
+
 static ssize_t sdram_size_show(struct device *dev,
                               struct device_attribute *attr,
                               char *buf)
@@ -415,6 +432,7 @@ static const struct device_attribute solo_dev_attrs[] = {
        __ATTR_RO(input_map),
        __ATTR_RO(intervals),
        __ATTR_RO(sdram_offsets),
+       __ATTR_RO(isr_more_laps),
 };

 static void solo_device_release(struct device *dev)
index d19c0ae..dffd7d7
--- a/drivers/media/pci/solo6x10/solo6x10-enc.c
+++ b/drivers/media/pci/solo6x10/solo6x10-enc.c
@@ -28,7 +28,7 @@
 #define VI_PROG_HSIZE                  (1280 - 16)
 #define VI_PROG_VSIZE                  (1024 - 16)

-#define IRQ_LEVEL                      2
+#define IRQ_LEVEL                      3

 static void solo_capture_config(struct solo_dev *solo_dev)
 {
index 6c9bc70..4799ea2
--- a/drivers/media/pci/solo6x10/solo6x10.h
+++ b/drivers/media/pci/solo6x10/solo6x10.h
@@ -277,6 +277,8 @@ struct solo_dev {
        spinlock_t              slock;
        int                     old_write;
        struct list_head        vidq_active;
+
+       int                     isr_more_laps;
 };

 static inline u32 solo_reg_read(struct solo_dev *solo_dev, int reg)

-- 
Andrey Utkin