On Monday 05 August 2013 01:43 PM, Tomi Valkeinen wrote:
Hi,
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(vpdma, VPDMA_LIST_ATTR,
+ (list_num << VPDMA_LIST_NUM_SHFT) |
+ (list->type << VPDMA_LIST_TYPE_SHFT) |
+ list_size);
+
+ return 0;
+}
+static void vpdma_firmware_cb(const struct firmware *f, void *context)
+{
+ struct vpdma_data *vpdma = context;
+ struct vpdma_buf fw_dma_buf;
+ int i, r;
+
+ dev_dbg(&vpdma->pdev->dev, "firmware callback\n");
+
+ if (!f || !f->data) {
+ dev_err(&vpdma->pdev->dev, "couldn't get firmware\n");
+ return;
+ }
+
+ /* already initialized */
+ if (get_field_reg(vpdma, VPDMA_LIST_ATTR, VPDMA_LIST_RDY_MASK,
+ VPDMA_LIST_RDY_SHFT)) {
+ vpdma->ready = true;
+ return;
+ }
+
+ r = vpdma_buf_alloc(&fw_dma_buf, f->size);
+ if (r) {
+ dev_err(&vpdma->pdev->dev,
+ "failed to allocate dma buffer for firmware\n");
+ goto rel_fw;
+ }
+
+ memcpy(fw_dma_buf.addr, f->data, f->size);
+
+ vpdma_buf_map(vpdma, &fw_dma_buf);
+
+ write_reg(vpdma, VPDMA_LIST_ADDR, (u32) fw_dma_buf.dma_addr);
+
+ for (i = 0; i < 100; i++) { /* max 1 second */
+ msleep_interruptible(10);
You call interruptible version here, but you don't handle the
interrupted case. I believe the loop will just continue looping, even if
the user interrupted.
Okay. I think I don't understand the interruptible version correctly. We
don't need to msleep_interruptible here, we aren't waiting on any wake
up event, we just want to wait till a bit gets set.
I am thinking of implementing something similar to wait_for_bit_change()
in 'drivers/video/omap2/dss/dsi.c'
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
