| Message ID | m360powc4m.fsf@t19.piap.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 22, 2016 at 10:51:37AM +0200, Krzysztof Hałasa wrote:
> I wonder if the following fixes the problem (completely untested).
I have given this a run, and it still hangs.
--
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
Andrey Utkin <andrey_utkin@fastmail.com> writes: > On Thu, Sep 22, 2016 at 10:51:37AM +0200, Krzysztof Hałasa wrote: >> I wonder if the following fixes the problem (completely untested). > > I have given this a run, and it still hangs. Does (only) adding the pci_read_config_word(solo_dev->pdev, PCI_STATUS, &val); in solo_reg_write() help?
On Mon, Sep 26, 2016 at 07:38:05AM +0200, Krzysztof Hałasa wrote: > Andrey Utkin <andrey_utkin@fastmail.com> writes: > > > On Thu, Sep 22, 2016 at 10:51:37AM +0200, Krzysztof Hałasa wrote: > >> I wonder if the following fixes the problem (completely untested). > > > > I have given this a run, and it still hangs. > > Does (only) adding the > > pci_read_config_word(solo_dev->pdev, PCI_STATUS, &val); > > in solo_reg_write() help? Yes. I have posted a patch with this change few days ago, I thought you have noticed it. -- 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
Andrey Utkin <andrey_utkin@fastmail.com> writes: >> Does (only) adding the >> >> pci_read_config_word(solo_dev->pdev, PCI_STATUS, &val); >> >> in solo_reg_write() help? > > Yes. > I have posted a patch with this change few days ago, I thought you have > noticed it. Well, I think you haven't sent me a copy. Anyway, it would be great to determine where exactly writes need a flush. Adding it everywhere is a bit suboptimal, one would think. Can you share some details about the machine you are experiencing the problems on? CPU, chipset? I'd try to see if I can recreate the problem. Alternatively, you could investigate yourself - at first you could put pci_read_config_word() at the end of subroutines (including return statements) using solo_reg_write(). And in that solo_p2m_dma_desc(), before wait_for_completion_timeout(). Then eliminate them using some sort of binary search to see which ones are required.
On Tue, Sep 27, 2016 at 07:27:53AM +0200, Krzysztof Hałasa wrote: > Andrey Utkin <andrey_utkin@fastmail.com> writes: > > >> Does (only) adding the > >> > >> pci_read_config_word(solo_dev->pdev, PCI_STATUS, &val); > >> > >> in solo_reg_write() help? > > > > Yes. > > I have posted a patch with this change few days ago, I thought you have > > noticed it. > > Well, I think you haven't sent me a copy. Anyway, it would be great to > determine where exactly writes need a flush. Adding it everywhere is a > bit suboptimal, one would think. Oh, I'm terribly sorry, I really meant to send you a copy. Actual posting is: lkml.kernel.org/r/20160922000331.4193-1-andrey.utkin@corp.bluecherry.net > Can you share some details about the machine you are experiencing the > problems on? CPU, chipset? I'd try to see if I can recreate the problem. See solo.txt.gz attached. > Alternatively, you could investigate yourself - at first you could put > pci_read_config_word() at the end of subroutines (including return > statements) using solo_reg_write(). And in that solo_p2m_dma_desc(), > before wait_for_completion_timeout(). Then eliminate them using some > sort of binary search to see which ones are required. Sorry, but I've got no time for this long-lasting debug session right now, and except for this issue, users enjoy their mainline kernel driver. So I'd just fix that in mainline kernels as quickly as possible. Now I'm even considering submitting that to longterm 4.4 branch.
Andrey Utkin <andrey_utkin@fastmail.com> writes: >> Can you share some details about the machine you are experiencing the >> problems on? CPU, chipset? I'd try to see if I can recreate the problem. > > See solo.txt.gz attached. Thanks. I can see you have quite a set of video devices there. I will see what I can do with this. BTW does the lookup occur on SOLO6010, 6110, or both?
On Tue, Sep 27, 2016 at 01:33:49PM +0200, Krzysztof Hałasa wrote: > Thanks. I can see you have quite a set of video devices there. > I will see what I can do with this. Yeah, I have got also 4-chip tw5864 board here :) Bluecherry decided to switch to it because they are available for retail purchase, unlike solo* which must be ordered in large batch. It was huge reverse-engineering effort to make it work, though, and there are still issues with H.264 encoding functionality, and audio functionality is not done yet. > BTW does the lookup occur on SOLO6010, 6110, or both? Lockup happens only on 6010. In provided log you can see that 6110 passes just fine right before 6010. Also if 6010 PCI ID is removed from solo6x10 driver's devices list, the freeze doesn't happen. -- 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
Andrey Utkin <andrey_utkin@fastmail.com> writes: > Lockup happens only on 6010. In provided log you can see that 6110 > passes just fine right before 6010. Also if 6010 PCI ID is removed from > solo6x10 driver's devices list, the freeze doesn't happen. Probably explains why I don't see lockups :-) I will have a look.
Em Wed, 28 Sep 2016 07:21:44 +0200 khalasa@piap.pl (Krzysztof Hałasa) escreveu: > Andrey Utkin <andrey_utkin@fastmail.com> writes: > > > Lockup happens only on 6010. In provided log you can see that 6110 > > passes just fine right before 6010. Also if 6010 PCI ID is removed from > > solo6x10 driver's devices list, the freeze doesn't happen. > > Probably explains why I don't see lockups :-) > > I will have a look. Any news on this? Should the patch be applied or not? If not, are there any other patch to fix this regression? Thanks, Mauro -- 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 Mon, Oct 24, 2016 at 05:32:33PM -0200, Mauro Carvalho Chehab wrote: > Em Wed, 28 Sep 2016 07:21:44 +0200 > khalasa@piap.pl (Krzysztof Hałasa) escreveu: > > > Andrey Utkin <andrey_utkin@fastmail.com> writes: > > > > > Lockup happens only on 6010. In provided log you can see that 6110 > > > passes just fine right before 6010. Also if 6010 PCI ID is removed from > > > solo6x10 driver's devices list, the freeze doesn't happen. > > > > Probably explains why I don't see lockups :-) > > > > I will have a look. > > Any news on this? Should the patch be applied or not? If not, are there > any other patch to fix this regression? Actual patch is Subject: [PATCH v2] media: solo6x10: fix lockup by avoiding delayed register write Message-Id: <20161022153436.12076-1-andrey.utkin@corp.bluecherry.net> Date: Sat, 22 Oct 2016 16:34:36 +0100 -- 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
diff --git a/drivers/media/pci/solo6x10/solo6x10-core.c b/drivers/media/pci/solo6x10/solo6x10-core.c index f50d072..2d4900e 100644 --- a/drivers/media/pci/solo6x10/solo6x10-core.c +++ b/drivers/media/pci/solo6x10/solo6x10-core.c @@ -99,6 +99,7 @@ static irqreturn_t solo_isr(int irq, void *data) { struct solo_dev *solo_dev = data; u32 status; + u16 tmp; int i; status = solo_reg_read(solo_dev, SOLO_IRQ_STAT); @@ -129,6 +130,7 @@ static irqreturn_t solo_isr(int irq, void *data) if (status & SOLO_IRQ_G723) solo_g723_isr(solo_dev); + pci_read_config_word(solo_dev->pdev, PCI_STATUS, &tmp) // flush write to SOLO_IRQ_STAT return IRQ_HANDLED; } diff --git a/drivers/media/pci/solo6x10/solo6x10-p2m.c b/drivers/media/pci/solo6x10/solo6x10-p2m.c index 07c4e07..8a51d45 100644 --- a/drivers/media/pci/solo6x10/solo6x10-p2m.c +++ b/drivers/media/pci/solo6x10/solo6x10-p2m.c @@ -70,6 +70,7 @@ int solo_p2m_dma_desc(struct solo_dev *solo_dev, unsigned int config = 0; int ret = 0; int p2m_id = 0; + u16 tmp; /* Get next ID. According to Softlogic, 6110 has problems on !=0 P2M */ if (solo_dev->type != SOLO_DEV_6110 && multi_p2m) { @@ -111,6 +112,7 @@ int solo_p2m_dma_desc(struct solo_dev *solo_dev, desc[1].ctrl); } + pci_read_config_word(solo_dev->pdev, PCI_STATUS, &tmp); // flush writes timeout = wait_for_completion_timeout(&p2m_dev->completion, solo_dev->p2m_jiffies);