Re: [PATCH 1/6] v4l: ti-vpe: Create a vpdma helper library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday 20 August 2013 05:09 PM, Laurent Pinchart wrote:
Hi Archit,

On Wednesday 14 August 2013 16:27:57 Archit Taneja wrote:
On Friday 09 August 2013 03:34 AM, Laurent Pinchart wrote:
On Friday 02 August 2013 19:33:38 Archit Taneja wrote:
The primary function of VPDMA is to move data between external memory and
internal processing modules(in our case, VPE) that source or sink data.
VPDMA is capable of buffering this data and then delivering the data as
demanded to the modules as programmed. The modules that source or sink
data are referred to as clients or ports. A channel is setup inside the
VPDMA to connect a specific memory buffer to a specific client. The VPDMA
centralizes the DMA control functions and buffering required to allow all
the clients to minimize the effect of long latency times.

Add the following to the VPDMA helper:

- A data struct which describe VPDMA channels. For now, these channels
are the ones used only by VPE, the list of channels will increase when
VIP(Video Input Port) also uses the VPDMA library. This channel
information will be used to populate fields required by data descriptors.

- Data structs which describe the different data types supported by
VPDMA. This data type information will be used to populate fields
required by data descriptors and used by the VPE driver to map a V4L2
format to the corresponding VPDMA data type.

- Provide VPDMA register offset definitions, functions to read, write and
modify VPDMA registers.

- Functions to create and submit a VPDMA list. A list is a group of
descriptors that makes up a set of DMA transfers that need to be
completed. Each descriptor will either perform a DMA transaction to fetch
input buffers and write to output buffers(data descriptors), or configure
the MMRs of sub blocks of VPE(configuration descriptors), or provide
control information to VPDMA (control descriptors).

- Functions to allocate, map and unmap buffers needed for the descriptor
list, payloads containing MMR values and motion vector buffers. These use
the DMA mapping APIs to ensure exclusive access to VPDMA.

- Functions to enable VPDMA interrupts. VPDMA can trigger an interrupt on
the VPE interrupt line when a descriptor list is parsed completely and
the DMA transactions are completed. This requires masking the events in
VPDMA registers and configuring some top level VPE interrupt registers.

- Enable some VPDMA specific parameters: frame start event(when to start
DMA for a client) and line mode(whether each line fetched should be
mirrored or not).

- Function to load firmware required by VPDMA. VPDMA requires a firmware
for it's internal list manager. We add the required request_firmware
apis to fetch this firmware from user space.

- Function to dump VPDMA registers.

- A function to initialize VPDMA, this will be called by the VPE driver
with it's platform device pointer, this function will take care of
loading VPDMA firmware and returning a handle back to the VPE driver.
The VIP driver will also call the same init function to initialize it's
own VPDMA instance.

Signed-off-by: Archit Taneja <archit@xxxxxx>

[snip]

+/*
+ * Allocate a DMA buffer
+ */
+int vpdma_buf_alloc(struct vpdma_buf *buf, size_t size)
+{
+	buf->size = size;
+	buf->mapped = 0;
+	buf->addr = kzalloc(size, GFP_KERNEL);

You should use the dma allocation API (depending on your needs,
dma_alloc_coherent for instance) to allocate DMA-able buffers.

I'm not sure about this, dma_map_single() api works fine on kzalloc'd
buffers. The above function is used to allocate small contiguous buffers
(never more than 1024 bytes) needed for descriptors for the DMA IP. I
thought of using DMA pool, but that creates small buffers of the same size.
So I finally went with kzalloc.

OK, I mistakenly thought it would allocate larger buffers as well. If it's
used to allocate descriptors only, would it be better to rename it to
vpdma_desc_alloc() (or something similar) ?

Actually, I just thought about this again. We use this api to allocate a motion vector buffer for the de-interlacer, that's a buffer which is proportional to the size of the frame, it takes up 4 bits per pixel. So for a 1080i frame(our limit), it would take around 51 kilobytes for it. I should probably use dma_alloc_coherent there.


+	if (!buf->addr)
+		return -ENOMEM;
+
+	WARN_ON((u32) buf->addr & VPDMA_DESC_ALIGN);
+
+	return 0;
+}

[snip]

+static int vpdma_load_firmware(struct vpdma_data *vpdma)
+{
+	int r;
+	struct device *dev = &vpdma->pdev->dev;
+
+	r = request_firmware_nowait(THIS_MODULE, 1,
+		(const char *) VPDMA_FIRMWARE, dev, GFP_KERNEL, vpdma,
+		vpdma_firmware_cb);

Is there a reason not to use the synchronous interface ? That would
simplify both this code and the callers, as they won't have to check
whether the firmware has been correctly loaded.

I'm not clear what you mean by that, the firmware would be stored in the
filesystem. If the driver is built-in, then the synchronous interface
wouldn't work unless the firmware is appended to the kernel image. Am I
missing something here? I'm not very aware of the firmware api.

request_firmware() would just sleep (with a 30 seconds timeout if I'm not
mistaken) until userspace provides the firmware. As devices are probed
asynchronously (in kernel threads) the system will just boot normally, and the
request_firmware() call will return when the firmware is available.

+	if (r) {
+		dev_err(dev, "firmware not available %s\n", VPDMA_FIRMWARE);
+		return r;
+	} else {
+		dev_info(dev, "loading firmware %s\n", VPDMA_FIRMWARE);
+	}
+
+	return 0;
+}


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux