On Friday 09 August 2013 03:05 AM, Laurent Pinchart wrote:
Hi Archit,

On Monday 05 August 2013 16:56:46 Archit Taneja wrote:
On Monday 05 August 2013 01:43 PM, Tomi Valkeinen wrote:
On 02/08/13 17:03, Archit Taneja wrote:
+struct vpdma_data_format vpdma_yuv_fmts[] = {
+       [VPDMA_DATA_FMT_Y444] = {
+               .data_type      = DATA_TYPE_Y444,
+               .depth          = 8,
+       },

This, and all the other tables, should probably be consts?

That's true, I'll fix those.

+static void insert_field(u32 *valp, u32 field, u32 mask, int shift)
+{
+       u32 val = *valp;
+
+       val &= ~(mask << shift);
+       val |= (field & mask) << shift;
+       *valp = val;
+}

I think "insert" normally means, well, inserting a thing in between
something. What you do here is overwriting.

Why not just call it "write_field"?

sure, will change it.

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

Maybe true/false is clearer here that 0/1.

okay.

+/*
+ * submit a list of DMA descriptors to the VPE VPDMA, do not wait for
completion + */
+int vpdma_submit_descs(struct vpdma_data *vpdma, struct vpdma_desc_list
*list) +{
+       /* we always use the first list */
+       int list_num = 0;
+       int list_size;
+
+       if (vpdma_list_busy(vpdma, list_num))
+               return -EBUSY;
+
+       /* 16-byte granularity */
+       list_size = (list->next - list->buf.addr) >> 4;
+
+       write_reg(vpdma, VPDMA_LIST_ADDR, (u32) list->buf.dma_addr);
+       wmb();

What is the wmb() for?

VPDMA_LIST_ADDR needs to be written before VPDMA_LIST_ATTR, otherwise
VPDMA doesn't work. wmb() ensures the ordering.

write_reg() calls iowrite32(), which already includes an __iowmb().

I wasn't aware of that. I'll remove the wmb() call. Thanks for sharing this info.

Archit

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

Reply via email to