Re: [RFC PATCH v6 6/9] media: tegra: Add Tegra210 Video input driver

From: Dmitry Osipenko
Date: Mon Apr 06 2020 - 16:02:49 EST


04.04.2020 04:25, Sowjanya Komatineni ÐÐÑÐÑ:
...
> +static int chan_capture_kthread_start(void *data)
> +{
> + struct tegra_vi_channel *chan = data;
> + struct tegra_channel_buffer *buf;
> + int err = 0;
> + int caps_inflight;
> +
> + set_freezable();
> +
> + while (1) {
> + try_to_freeze();
> +
> + wait_event_interruptible(chan->start_wait,
> + !list_empty(&chan->capture) ||
> + kthread_should_stop());

Is it really okay that list_empty() isn't protected with a lock?

Why wait_event is "interruptible"?

> + /*
> + * Frame start and MW_ACK_DONE syncpoint condition FIFOs are
> + * of max depth 2. So make sure max 2 capture requests are
> + * in process by the hardware at a time.
> + */
> + while (!(kthread_should_stop() || list_empty(&chan->capture))) {
> + caps_inflight = chan->capture_reqs - chan->sequence;
> + /*
> + * Source is not streaming if error is non-zero.
> + * So, do not dequeue buffers on capture error or when
> + * syncpoint requests in FIFO are full.
> + */
> + if (err || caps_inflight >= SYNCPT_FIFO_DEPTH)
> + break;
> +
> + /* dequeue the buffer and start capture */
> + spin_lock(&chan->start_lock);
> + if (list_empty(&chan->capture))
> + break;
> + buf = list_entry(chan->capture.next,
> + struct tegra_channel_buffer, queue);

list_first_entry()

> + list_del_init(&buf->queue);
> + spin_unlock(&chan->start_lock);
> +
> + err = tegra_channel_capture_frame(chan, buf);
> + if (err)
> + vb2_queue_error(&chan->queue);
> + }
> +
> + if (kthread_should_stop())
> + break;
> + }
> +
> + return 0;
> +}