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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.