2015-02-11 18:19:03

by Andrey Utkin

[permalink] [raw]
Subject: Question on MSI support in PCI and PCI-E devices

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?

--
Andrey Utkin


2015-02-12 14:49:03

by Stephen Hemminger

[permalink] [raw]
Subject: Re: Question on MSI support in PCI and PCI-E devices

On Wed, 11 Feb 2015 18:19:00 +0000
Andrey Utkin <[email protected]> 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.

2015-02-12 15:12:00

by Andrey Utkin

[permalink] [raw]
Subject: Re: Question on MSI support in PCI and PCI-E devices

2015-02-12 16:48 GMT+02:00 Stephen Hemminger <[email protected]>:
> On Wed, 11 Feb 2015 18:19:00 +0000
> Andrey Utkin <[email protected]> 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