linux-kernel - Re: [PATCH 1/2] media: Add timberdale video-in driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4BD45EB1.5020804@redhat.com>
Date:	Sun, 25 Apr 2010 12:24:33 -0300
From:	Mauro Carvalho Chehab <mchehab@...hat.com>
To:	Richard Röjfors 
	<richard.rojfors@...agicore.com>
CC:	Linux Media Mailing List <linux-media@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Douglas Schilling Landgraf <dougsland@...il.com>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	"Williams, Dan J" <dan.j.williams@...el.com>
Subject: Re: [PATCH 1/2] media: Add timberdale video-in driver

Richard Röjfors wrote:
> This patch adds the timberdale video-in driver.
> 
> The video IP of timberdale delivers the video data via DMA.
> The driver uses the DMA api to handle DMA transfers, and make use
> of the V4L2 videobuffers to handle buffers against user space.
> Due to some timing constraint it makes sense to do DMA into an
> intermediate buffer and then copy the data to vmalloc:ed buffers.
> 
> If available the driver uses an encoder to get/set the video standard
> 
> Signed-off-by: Richard Röjfors <richard.rojfors@...agicore.com>
> +#define TIMBLOGIW_DMA_BUFFER_SIZE	(TIMBLOGIW_BYTES_PER_LINE * 576)

...

> +static int __timblogiw_alloc_dma(struct timblogiw_fh *fh, struct device *dev)
> +{
> +	dma_addr_t addr;
> +	int err, i, pos;
> +	int bytes_per_desc = TIMBLOGIW_LINES_PER_DESC *
> +		timblogiw_bytes_per_line(fh->cur_norm);
> +
> +	fh->dma.cookie = -1;
> +	fh->dma.dev = dev;
> +
> +	fh->dma.buf = kzalloc(TIMBLOGIW_DMA_BUFFER_SIZE, GFP_KERNEL);
> +	if (!fh->dma.buf)
> +		return -ENOMEM;


Why do you need a fixed DMA buffer size? Just allocate the buffer size dynamically at
buffer_prepare callback.
> +	videobuf_queue_vmalloc_init(&fh->vb_vidq, &timblogiw_video_qops,
> +			NULL, &fh->queue_lock, V4L2_BUF_TYPE_VIDEO_CAPTURE,
> +			V4L2_FIELD_NONE, sizeof(struct videobuf_buffer), fh);

You should be using, instead, videobuf_dma_sg or videobuf_cont, instead of
using videobuf-vmalloc. This way, you'll avoid double buffering.

-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/