From: Tomi Valkeinen <tomi.valkeinen@ti.com> To: Archit Taneja <archit@ti.com> Cc: <linux-media@vger.kernel.org>, <linux-omap@vger.kernel.org>, <dagriego@biglakesoftware.com>, <dale@farnsworth.org>, <pawel@osciak.com>, <m.szyprowski@samsung.com>, <hverkuil@xs4all.nl>, <laurent.pinchart@ideasonboard.com> Subject: Re: [PATCH 1/6] v4l: ti-vpe: Create a vpdma helper library Date: Mon, 5 Aug 2013 11:13:40 +0300 [thread overview] Message-ID: <51FF5EB4.8090007@ti.com> (raw) In-Reply-To: <1375452223-30524-2-git-send-email-archit@ti.com> [-- Attachment #1: Type: text/plain, Size: 3081 bytes --] 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? > +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"? > + * 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. > +/* > + * 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? > + 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. > + if (get_field_reg(vpdma, VPDMA_LIST_ATTR, VPDMA_LIST_RDY_MASK, > + VPDMA_LIST_RDY_SHFT)) > + break; > + } > + > + if (i == 100) { > + dev_err(&vpdma->pdev->dev, "firmware upload failed\n"); > + goto free_buf; > + } > + > + vpdma->ready = true; > + > +free_buf: > + vpdma_buf_unmap(vpdma, &fw_dma_buf); > + > + vpdma_buf_free(&fw_dma_buf); > +rel_fw: > + release_firmware(f); > +} Tomi [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 901 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Tomi Valkeinen <tomi.valkeinen@ti.com> To: Archit Taneja <archit@ti.com> Cc: linux-media@vger.kernel.org, linux-omap@vger.kernel.org, dagriego@biglakesoftware.com, dale@farnsworth.org, pawel@osciak.com, m.szyprowski@samsung.com, hverkuil@xs4all.nl, laurent.pinchart@ideasonboard.com Subject: Re: [PATCH 1/6] v4l: ti-vpe: Create a vpdma helper library Date: Mon, 5 Aug 2013 11:13:40 +0300 [thread overview] Message-ID: <51FF5EB4.8090007@ti.com> (raw) In-Reply-To: <1375452223-30524-2-git-send-email-archit@ti.com> [-- Attachment #1: Type: text/plain, Size: 3081 bytes --] 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? > +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"? > + * 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. > +/* > + * 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? > + 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. > + if (get_field_reg(vpdma, VPDMA_LIST_ATTR, VPDMA_LIST_RDY_MASK, > + VPDMA_LIST_RDY_SHFT)) > + break; > + } > + > + if (i == 100) { > + dev_err(&vpdma->pdev->dev, "firmware upload failed\n"); > + goto free_buf; > + } > + > + vpdma->ready = true; > + > +free_buf: > + vpdma_buf_unmap(vpdma, &fw_dma_buf); > + > + vpdma_buf_free(&fw_dma_buf); > +rel_fw: > + release_firmware(f); > +} Tomi [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 901 bytes --]
next prev parent reply other threads:[~2013-08-05 8:13 UTC|newest] Thread overview: 138+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-08-02 14:03 [PATCH 0/6] v4l: VPE mem to mem driver Archit Taneja 2013-08-02 14:03 ` Archit Taneja 2013-08-02 14:03 ` [PATCH 1/6] v4l: ti-vpe: Create a vpdma helper library Archit Taneja 2013-08-02 14:03 ` Archit Taneja 2013-08-05 8:13 ` Tomi Valkeinen [this message] 2013-08-05 8:13 ` Tomi Valkeinen 2013-08-05 11:26 ` Archit Taneja 2013-08-05 11:26 ` Archit Taneja 2013-08-05 12:26 ` Tomi Valkeinen 2013-08-05 12:26 ` Tomi Valkeinen 2013-08-08 21:35 ` Laurent Pinchart 2013-08-14 10:19 ` Archit Taneja 2013-08-14 10:19 ` Archit Taneja 2013-08-08 22:04 ` Laurent Pinchart 2013-08-14 10:57 ` Archit Taneja 2013-08-14 10:57 ` Archit Taneja 2013-08-20 11:39 ` Laurent Pinchart 2013-08-20 12:51 ` Archit Taneja 2013-08-20 12:51 ` Archit Taneja 2013-08-20 13:16 ` Archit Taneja 2013-08-20 13:16 ` Archit Taneja 2013-08-20 13:56 ` Laurent Pinchart 2013-08-21 6:47 ` Archit Taneja 2013-08-21 6:47 ` Archit Taneja 2013-08-02 14:03 ` [PATCH 2/6] v4l: ti-vpe: Add helpers for creating VPDMA descriptors Archit Taneja 2013-08-02 14:03 ` Archit Taneja 2013-08-05 9:11 ` Tomi Valkeinen 2013-08-05 9:11 ` Tomi Valkeinen 2013-08-05 12:05 ` Archit Taneja 2013-08-05 12:05 ` Archit Taneja 2013-08-05 13:03 ` Tomi Valkeinen 2013-08-05 13:03 ` Tomi Valkeinen 2013-08-02 14:03 ` [PATCH 3/6] v4l: ti-vpe: Add VPE mem to mem driver Archit Taneja 2013-08-02 14:03 ` Archit Taneja 2013-08-02 14:36 ` Hans Verkuil 2013-08-02 14:55 ` Archit Taneja 2013-08-02 14:55 ` Archit Taneja 2013-08-05 9:18 ` Tomi Valkeinen 2013-08-05 9:18 ` Tomi Valkeinen 2013-08-02 14:03 ` [PATCH 4/6] v4l: ti-vpe: Add de-interlacer support in VPE Archit Taneja 2013-08-02 14:03 ` Archit Taneja 2013-08-02 14:40 ` Hans Verkuil 2013-08-02 14:03 ` [PATCH 5/6] arm: dra7xx: hwmod data: add VPE hwmod data and ocp_if info Archit Taneja 2013-08-02 14:03 ` Archit Taneja 2013-08-02 14:03 ` [PATCH 6/6] experimental: arm: dts: dra7xx: Add a DT node for VPE Archit Taneja 2013-08-02 14:03 ` Archit Taneja 2013-08-08 22:11 ` Laurent Pinchart 2013-10-25 10:35 ` Archit Taneja 2013-10-25 10:35 ` Archit Taneja 2013-12-03 10:08 ` Archit Taneja 2013-12-03 10:08 ` Archit Taneja 2013-08-20 11:00 ` [PATCH v2 0/6] v4l: VPE mem to mem driver Archit Taneja 2013-08-20 11:00 ` Archit Taneja 2013-08-20 11:00 ` [PATCH v2 1/6] v4l: ti-vpe: Create a vpdma helper library Archit Taneja 2013-08-20 11:00 ` Archit Taneja 2013-08-20 11:00 ` [PATCH v2 2/6] v4l: ti-vpe: Add helpers for creating VPDMA descriptors Archit Taneja 2013-08-20 11:00 ` Archit Taneja 2013-08-20 11:00 ` [PATCH v2 3/6] v4l: ti-vpe: Add VPE mem to mem driver Archit Taneja 2013-08-20 11:00 ` Archit Taneja 2013-08-20 11:00 ` [PATCH v2 4/6] v4l: ti-vpe: Add de-interlacer support in VPE Archit Taneja 2013-08-20 11:00 ` Archit Taneja 2013-08-20 11:00 ` [PATCH v2 5/6] arm: dra7xx: hwmod data: add VPE hwmod data and ocp_if info Archit Taneja 2013-08-20 11:00 ` Archit Taneja 2013-08-20 11:00 ` [PATCH v2 6/6] experimental: arm: dts: dra7xx: Add a DT node for VPE Archit Taneja 2013-08-20 11:00 ` Archit Taneja 2013-08-29 12:32 ` [PATCH v3 0/6] v4l: VPE mem to mem driver Archit Taneja 2013-08-29 12:32 ` Archit Taneja 2013-08-29 12:32 ` [PATCH v3 1/6] v4l: ti-vpe: Create a vpdma helper library Archit Taneja 2013-08-29 12:32 ` Archit Taneja 2013-08-29 12:32 ` [PATCH v3 2/6] v4l: ti-vpe: Add helpers for creating VPDMA descriptors Archit Taneja 2013-08-29 12:32 ` Archit Taneja 2013-08-29 12:32 ` [PATCH v3 3/6] v4l: ti-vpe: Add VPE mem to mem driver Archit Taneja 2013-08-29 12:32 ` Archit Taneja 2013-08-29 13:28 ` Hans Verkuil 2013-08-30 6:47 ` Archit Taneja 2013-08-30 6:47 ` Archit Taneja 2013-08-30 7:07 ` Hans Verkuil 2013-08-30 10:05 ` Archit Taneja 2013-08-30 10:05 ` Archit Taneja 2013-08-30 10:44 ` Hans Verkuil 2013-09-05 5:56 ` Archit Taneja 2013-09-05 5:56 ` Archit Taneja 2013-08-29 12:32 ` [PATCH v3 4/6] v4l: ti-vpe: Add de-interlacer support in VPE Archit Taneja 2013-08-29 12:32 ` Archit Taneja 2013-08-29 12:32 ` [PATCH v3 5/6] arm: dra7xx: hwmod data: add VPE hwmod data and ocp_if info Archit Taneja 2013-08-29 12:32 ` Archit Taneja 2013-08-29 12:42 ` Rajendra Nayak 2013-08-29 12:42 ` Rajendra Nayak 2013-08-29 13:42 ` Archit Taneja 2013-08-29 13:42 ` Archit Taneja 2013-08-29 12:32 ` [PATCH v3 6/6] experimental: arm: dts: dra7xx: Add a DT node for VPE Archit Taneja 2013-08-29 12:32 ` Archit Taneja 2013-09-06 10:12 ` [PATCH v4 0/4] v4l: VPE mem to mem driver Archit Taneja 2013-09-06 10:12 ` Archit Taneja 2013-09-06 10:12 ` [PATCH v4 1/4] v4l: ti-vpe: Create a vpdma helper library Archit Taneja 2013-09-06 10:12 ` Archit Taneja 2013-10-07 7:46 ` Hans Verkuil 2013-09-06 10:12 ` [PATCH v4 2/4] v4l: ti-vpe: Add helpers for creating VPDMA descriptors Archit Taneja 2013-09-06 10:12 ` Archit Taneja 2013-10-07 7:46 ` Hans Verkuil 2013-09-06 10:12 ` [PATCH v4 3/4] v4l: ti-vpe: Add VPE mem to mem driver Archit Taneja 2013-09-06 10:12 ` Archit Taneja 2013-10-07 7:55 ` Hans Verkuil 2013-10-07 9:16 ` Archit Taneja 2013-10-07 9:16 ` Archit Taneja 2013-10-07 9:34 ` Hans Verkuil 2013-10-07 10:22 ` Archit Taneja 2013-10-07 10:22 ` Archit Taneja 2013-10-07 14:02 ` Hans Verkuil 2013-10-07 14:34 ` Archit Taneja 2013-10-07 14:34 ` Archit Taneja 2013-09-06 10:12 ` [PATCH v4 4/4] v4l: ti-vpe: Add de-interlacer support in VPE Archit Taneja 2013-09-06 10:12 ` Archit Taneja 2013-10-07 7:57 ` Hans Verkuil 2013-09-16 6:59 ` [PATCH v4 0/4] v4l: VPE mem to mem driver Archit Taneja 2013-09-16 6:59 ` Archit Taneja 2013-10-07 6:39 ` Archit Taneja 2013-10-07 6:39 ` Archit Taneja 2013-10-09 14:29 ` [PATCH v5 3/4] v4l: ti-vpe: Add " Archit Taneja 2013-10-09 14:29 ` Archit Taneja 2013-10-11 7:46 ` Hans Verkuil 2013-10-15 13:47 ` Archit Taneja 2013-10-15 13:47 ` Archit Taneja 2013-10-15 13:51 ` Hans Verkuil 2013-10-15 14:13 ` Kamil Debski 2013-10-15 15:54 ` Kamil Debski 2013-10-16 5:08 ` Archit Taneja 2013-10-16 5:08 ` Archit Taneja 2013-10-16 5:36 ` [PATCH v5 0/4] v4l: " Archit Taneja 2013-10-16 5:36 ` Archit Taneja 2013-10-16 5:36 ` [PATCH v5 1/4] v4l: ti-vpe: Create a vpdma helper library Archit Taneja 2013-10-16 5:36 ` Archit Taneja 2013-10-16 5:36 ` [PATCH v5 2/4] v4l: ti-vpe: Add helpers for creating VPDMA descriptors Archit Taneja 2013-10-16 5:36 ` Archit Taneja 2013-10-16 5:36 ` [PATCH v5 3/4] v4l: ti-vpe: Add VPE mem to mem driver Archit Taneja 2013-10-16 5:36 ` Archit Taneja 2013-10-16 5:36 ` [PATCH v5 4/4] v4l: ti-vpe: Add de-interlacer support in VPE Archit Taneja 2013-10-16 5:36 ` Archit Taneja
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=51FF5EB4.8090007@ti.com \ --to=tomi.valkeinen@ti.com \ --cc=archit@ti.com \ --cc=dagriego@biglakesoftware.com \ --cc=dale@farnsworth.org \ --cc=hverkuil@xs4all.nl \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-media@vger.kernel.org \ --cc=linux-omap@vger.kernel.org \ --cc=m.szyprowski@samsung.com \ --cc=pawel@osciak.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.