From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: linux-media@vger.kernel.org, Hans Verkuil <hans.verkuil@cisco.com>
Subject: Re: [PATCH 02/16] cx88: drop the bogus 'queue' list in dmaqueue.
Date: Tue, 28 Oct 2014 16:58:23 -0200 [thread overview]
Message-ID: <20141028165823.5b34cd2a@recife.lan> (raw)
In-Reply-To: <1411216911-7950-3-git-send-email-hverkuil@xs4all.nl>
Em Sat, 20 Sep 2014 14:41:37 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> This list is used some buffers have a different format, but that can
> never happen. Remove it and all associated code.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
> drivers/media/pci/cx88/cx88-mpeg.c | 31 -----------------------------
> drivers/media/pci/cx88/cx88-video.c | 39 +++----------------------------------
> drivers/media/pci/cx88/cx88.h | 1 -
> 3 files changed, 3 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/media/pci/cx88/cx88-mpeg.c b/drivers/media/pci/cx88/cx88-mpeg.c
> index 2803b6f..5f59901 100644
> --- a/drivers/media/pci/cx88/cx88-mpeg.c
> +++ b/drivers/media/pci/cx88/cx88-mpeg.c
> @@ -210,37 +210,7 @@ static int cx8802_restart_queue(struct cx8802_dev *dev,
>
> dprintk( 1, "cx8802_restart_queue\n" );
> if (list_empty(&q->active))
> - {
> - struct cx88_buffer *prev;
> - prev = NULL;
> -
> - dprintk(1, "cx8802_restart_queue: queue is empty\n" );
This is not bogus code. What happens here is that sometimes the DMA
engine stops on cx88 and it needs to be restarted under some temporary
errors.
I don't remember the exact condition, as I don't touch on cx88 on
several years, but I think it happens when the signal drops (for
example, if the antenna cable gets removed, but not 100% sure).
So, removing this code will cause regressions.
Regards,
Mauro
> -
> - for (;;) {
> - if (list_empty(&q->queued))
> - return 0;
> - buf = list_entry(q->queued.next, struct cx88_buffer, vb.queue);
> - if (NULL == prev) {
> - list_move_tail(&buf->vb.queue, &q->active);
> - cx8802_start_dma(dev, q, buf);
> - buf->vb.state = VIDEOBUF_ACTIVE;
> - buf->count = q->count++;
> - mod_timer(&q->timeout, jiffies+BUFFER_TIMEOUT);
> - dprintk(1,"[%p/%d] restart_queue - first active\n",
> - buf,buf->vb.i);
> -
> - } else {
> - list_move_tail(&buf->vb.queue, &q->active);
> - buf->vb.state = VIDEOBUF_ACTIVE;
> - buf->count = q->count++;
> - prev->risc.jmp[1] = cpu_to_le32(buf->risc.dma);
> - dprintk(1,"[%p/%d] restart_queue - move to active\n",
> - buf,buf->vb.i);
> - }
> - prev = buf;
> - }
> return 0;
> - }
>
> buf = list_entry(q->active.next, struct cx88_buffer, vb.queue);
> dprintk(2,"restart_queue [%p/%d]: restart dma\n",
> @@ -486,7 +456,6 @@ static int cx8802_init_common(struct cx8802_dev *dev)
>
> /* init dma queue */
> INIT_LIST_HEAD(&dev->mpegq.active);
> - INIT_LIST_HEAD(&dev->mpegq.queued);
> dev->mpegq.timeout.function = cx8802_timeout;
> dev->mpegq.timeout.data = (unsigned long)dev;
> init_timer(&dev->mpegq.timeout);
> diff --git a/drivers/media/pci/cx88/cx88-video.c b/drivers/media/pci/cx88/cx88-video.c
> index 3075179..10fea4d 100644
> --- a/drivers/media/pci/cx88/cx88-video.c
> +++ b/drivers/media/pci/cx88/cx88-video.c
> @@ -470,7 +470,7 @@ static int restart_video_queue(struct cx8800_dev *dev,
> struct cx88_dmaqueue *q)
> {
> struct cx88_core *core = dev->core;
> - struct cx88_buffer *buf, *prev;
> + struct cx88_buffer *buf;
>
> if (!list_empty(&q->active)) {
> buf = list_entry(q->active.next, struct cx88_buffer, vb.queue);
> @@ -480,33 +480,8 @@ static int restart_video_queue(struct cx8800_dev *dev,
> list_for_each_entry(buf, &q->active, vb.queue)
> buf->count = q->count++;
> mod_timer(&q->timeout, jiffies+BUFFER_TIMEOUT);
> - return 0;
> - }
> -
> - prev = NULL;
> - for (;;) {
> - if (list_empty(&q->queued))
> - return 0;
> - buf = list_entry(q->queued.next, struct cx88_buffer, vb.queue);
> - if (NULL == prev) {
> - list_move_tail(&buf->vb.queue, &q->active);
> - start_video_dma(dev, q, buf);
> - buf->vb.state = VIDEOBUF_ACTIVE;
> - buf->count = q->count++;
> - mod_timer(&q->timeout, jiffies+BUFFER_TIMEOUT);
> - dprintk(2,"[%p/%d] restart_queue - first active\n",
> - buf,buf->vb.i);
> -
> - } else {
> - list_move_tail(&buf->vb.queue, &q->active);
> - buf->vb.state = VIDEOBUF_ACTIVE;
> - buf->count = q->count++;
> - prev->risc.jmp[1] = cpu_to_le32(buf->risc.dma);
> - dprintk(2,"[%p/%d] restart_queue - move to active\n",
> - buf,buf->vb.i);
> - }
> - prev = buf;
> }
> + return 0;
> }
>
> /* ------------------------------------------------------------------ */
> @@ -613,13 +588,7 @@ buffer_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb)
> buf->risc.jmp[0] = cpu_to_le32(RISC_JUMP | RISC_IRQ1 | RISC_CNT_INC);
> buf->risc.jmp[1] = cpu_to_le32(q->stopper.dma);
>
> - if (!list_empty(&q->queued)) {
> - list_add_tail(&buf->vb.queue,&q->queued);
> - buf->vb.state = VIDEOBUF_QUEUED;
> - dprintk(2,"[%p/%d] buffer_queue - append to queued\n",
> - buf, buf->vb.i);
> -
> - } else if (list_empty(&q->active)) {
> + if (list_empty(&q->active)) {
> list_add_tail(&buf->vb.queue,&q->active);
> start_video_dma(dev, q, buf);
> buf->vb.state = VIDEOBUF_ACTIVE;
> @@ -1694,7 +1663,6 @@ static int cx8800_initdev(struct pci_dev *pci_dev,
>
> /* init video dma queues */
> INIT_LIST_HEAD(&dev->vidq.active);
> - INIT_LIST_HEAD(&dev->vidq.queued);
> dev->vidq.timeout.function = cx8800_vid_timeout;
> dev->vidq.timeout.data = (unsigned long)dev;
> init_timer(&dev->vidq.timeout);
> @@ -1703,7 +1671,6 @@ static int cx8800_initdev(struct pci_dev *pci_dev,
>
> /* init vbi dma queues */
> INIT_LIST_HEAD(&dev->vbiq.active);
> - INIT_LIST_HEAD(&dev->vbiq.queued);
> dev->vbiq.timeout.function = cx8800_vbi_timeout;
> dev->vbiq.timeout.data = (unsigned long)dev;
> init_timer(&dev->vbiq.timeout);
> diff --git a/drivers/media/pci/cx88/cx88.h b/drivers/media/pci/cx88/cx88.h
> index ddc7991..77ec542 100644
> --- a/drivers/media/pci/cx88/cx88.h
> +++ b/drivers/media/pci/cx88/cx88.h
> @@ -324,7 +324,6 @@ struct cx88_buffer {
>
> struct cx88_dmaqueue {
> struct list_head active;
> - struct list_head queued;
> struct timer_list timeout;
> struct btcx_riscmem stopper;
> u32 count;
next prev parent reply other threads:[~2014-10-28 18:58 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-20 12:41 [PATCH 00/16] cx88: convert to vb2 Hans Verkuil
2014-09-20 12:41 ` [PATCH 01/16] cx88: remove fmt from the buffer struct Hans Verkuil
2014-09-20 12:41 ` [PATCH 02/16] cx88: drop the bogus 'queue' list in dmaqueue Hans Verkuil
2014-10-28 18:58 ` Mauro Carvalho Chehab [this message]
2014-10-29 7:54 ` Hans Verkuil
2014-10-29 8:42 ` Mauro Carvalho Chehab
2014-09-20 12:41 ` [PATCH 03/16] cx88: drop videobuf abuse in cx88-alsa Hans Verkuil
2014-09-20 12:41 ` [PATCH 04/16] cx88: convert to vb2 Hans Verkuil
2014-09-20 12:41 ` [PATCH 05/16] cx88: fix sparse warning Hans Verkuil
2014-09-20 12:41 ` [PATCH 06/16] cx88: return proper errors during fw load Hans Verkuil
2014-09-20 12:41 ` [PATCH 07/16] cx88: drop cx88_free_buffer Hans Verkuil
2014-09-20 12:41 ` [PATCH 08/16] cx88: remove dependency on btcx-risc Hans Verkuil
2014-09-20 12:41 ` [PATCH 09/16] cx88: increase API command timeout Hans Verkuil
2014-09-20 12:41 ` [PATCH 10/16] cx88: don't pollute the kernel log Hans Verkuil
2014-09-20 12:41 ` [PATCH 11/16] cx88: move width, height and field to core struct Hans Verkuil
2014-09-20 12:41 ` [PATCH 12/16] cx88: drop mpeg_active field Hans Verkuil
2014-09-20 12:41 ` [PATCH 13/16] cx88: don't allow changes while vb2_is_busy Hans Verkuil
2014-09-20 12:41 ` [PATCH 14/16] cx88: consistently use UNSET for absent tuner Hans Verkuil
2014-09-20 12:41 ` [PATCH 15/16] cx88: pci_disable_device comes after free_irq Hans Verkuil
2014-09-20 12:41 ` [PATCH 16/16] cx88: fix VBI support Hans Verkuil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141028165823.5b34cd2a@recife.lan \
--to=mchehab@osg.samsung.com \
--cc=hans.verkuil@cisco.com \
--cc=hverkuil@xs4all.nl \
--cc=linux-media@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).