From: Tomasz Figa <tfiga@chromium.org>
To: Ezequiel Garcia <ezequiel@collabora.com>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
Hans Verkuil <hans.verkuil@cisco.com>,
kernel@collabora.com,
Nicolas Dufresne <nicolas.dufresne@collabora.com>,
"open list:ARM/Rockchip SoC..."
<linux-rockchip@lists.infradead.org>,
Heiko Stuebner <heiko@sntech.de>, Jonas Karlman <jonas@kwiboo.se>
Subject: Re: [PATCH v2 06/11] rockchip/vpu: Cleanup JPEG bounce buffer management
Date: Thu, 28 Mar 2019 15:15:48 +0900 [thread overview]
Message-ID: <CAAFQd5DjVDhFMc=WpnvF61yBcPm=BUQhVtBWSyphkkM8QubFzA@mail.gmail.com> (raw)
In-Reply-To: <20190304192529.14200-7-ezequiel@collabora.com>
Hi Ezequiel,
On Tue, Mar 5, 2019 at 4:26 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> In order to make the code more generic, introduce a pair of start/stop
> codec operations, and use them to allocate and release the JPEG bounce
> buffer.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> .../media/rockchip/vpu/rk3288_vpu_hw.c | 2 ++
> .../rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c | 4 +--
> .../media/rockchip/vpu/rk3399_vpu_hw.c | 2 ++
> .../rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c | 4 +--
> .../staging/media/rockchip/vpu/rockchip_vpu.h | 12 ++++----
> .../media/rockchip/vpu/rockchip_vpu_drv.c | 10 +++++--
> .../media/rockchip/vpu/rockchip_vpu_enc.c | 23 +++++----------
> .../media/rockchip/vpu/rockchip_vpu_hw.h | 28 +++++++++++++++++++
> .../media/rockchip/vpu/rockchip_vpu_jpeg.c | 25 +++++++++++++++++
> 9 files changed, 81 insertions(+), 29 deletions(-)
>
Thanks for the series! Really sorry for late reply...
[snip]
> diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu.h b/drivers/staging/media/rockchip/vpu/rockchip_vpu.h
> index 1ec2be483e27..76ee24abc141 100644
> --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu.h
> +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu.h
> @@ -124,10 +124,7 @@ struct rockchip_vpu_dev {
> * @jpeg_quality: User-specified JPEG compression quality.
> *
> * @codec_ops: Set of operations related to codec mode.
> - *
> - * @bounce_dma_addr: Bounce buffer bus address.
> - * @bounce_buf: Bounce buffer pointer.
> - * @bounce_size: Bounce buffer size.
> + * @jpeg_enc_ctx: JPEG-encoding context.
> */
> struct rockchip_vpu_ctx {
> struct rockchip_vpu_dev *dev;
> @@ -146,9 +143,10 @@ struct rockchip_vpu_ctx {
>
> const struct rockchip_vpu_codec_ops *codec_ops;
>
> - dma_addr_t bounce_dma_addr;
> - void *bounce_buf;
> - size_t bounce_size;
> + /* Specific for particular codec modes. */
> + union {
> + struct rockchip_vpu_jpeg_enc_hw_ctx jpeg_enc_ctx;
> + };
nit: Would it perhaps make it a bit cleaner to call the union "ctx"?
Then we wouldn't need to repeat "_ctx" in every field.
> };
>
> /**
> diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
> index 5647b0bdac20..1a6dd36c71ab 100644
> --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
> +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
> @@ -64,10 +64,16 @@ static void rockchip_vpu_job_finish(struct rockchip_vpu_dev *vpu,
> avail_size = vb2_plane_size(&dst->vb2_buf, 0) -
> ctx->vpu_dst_fmt->header_size;
> if (bytesused <= avail_size) {
> - if (ctx->bounce_buf) {
> + /*
> + * This works while JPEG is the only encoder this driver
> + * supports. We will have to abstract this step, or get
> + * rid of the bounce buffer before we can support
> + * encoding other codecs.
Hmm, why wouldn't it work for other encoders, which shouldn't require
a bounce buffer?
> + */
> + if (ctx->jpeg_enc_ctx.bounce_buffer.cpu) {
> memcpy(vb2_plane_vaddr(&dst->vb2_buf, 0) +
> ctx->vpu_dst_fmt->header_size,
> - ctx->bounce_buf, bytesused);
> + ctx->jpeg_enc_ctx.bounce_buffer.cpu, bytesused);
> }
> dst->vb2_buf.planes[0].bytesused =
> ctx->vpu_dst_fmt->header_size + bytesused;
[snip]
> /**
> * struct rockchip_vpu_codec_ops - codec mode specific operations
> *
> + * @start: If needed, can be used for initialization.
> + * Optional and called from process context.
> + * @stop: If needed, can be used to undo the .start phase.
> + * Optional and called from process context.
nit: Perhaps .init and .exit could reflect better what these ops do?
Best regards,
Tomasz
next prev parent reply other threads:[~2019-03-28 6:16 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-04 19:25 [PATCH v2 00/11] Add MPEG-2 decoding to Rockchip VPU Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 01/11] rockchip/vpu: Rename pixel format helpers Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 02/11] media: Introduce helpers to fill pixel format structs Ezequiel Garcia
2019-03-12 8:29 ` Hans Verkuil
2019-03-22 17:29 ` Ezequiel Garcia
2019-03-25 14:32 ` Emil Velikov
2019-03-04 19:25 ` [PATCH v2 03/11] rockchip/vpu: Use pixel format helpers Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 04/11] rockchip/vpu: Use v4l2_m2m_buf_copy_metadata Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 05/11] rockchip/vpu: Cleanup macroblock alignment Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 06/11] rockchip/vpu: Cleanup JPEG bounce buffer management Ezequiel Garcia
2019-03-28 6:15 ` Tomasz Figa [this message]
2019-03-28 18:30 ` Ezequiel Garcia
2019-03-29 3:21 ` Tomasz Figa
2019-03-04 19:25 ` [PATCH v2 07/11] rockchip/vpu: Open-code media controller register Ezequiel Garcia
2019-03-28 7:11 ` Tomasz Figa
2019-03-28 20:05 ` Ezequiel Garcia
2019-03-29 7:43 ` Tomasz Figa
2019-03-04 19:25 ` [PATCH v2 08/11] rockchip/vpu: Support the Request API Ezequiel Garcia
2019-03-28 7:20 ` Tomasz Figa
2019-03-28 13:59 ` Hans Verkuil
2019-03-29 3:23 ` Tomasz Figa
2019-03-28 19:07 ` Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 09/11] rockchip/vpu: Add decoder boilerplate Ezequiel Garcia
2019-03-28 9:57 ` Tomasz Figa
2019-03-28 19:23 ` Ezequiel Garcia
2019-03-29 7:40 ` Tomasz Figa
2019-03-04 19:25 ` [PATCH v2 10/11] rockchip/vpu: Add support for non-standard controls Ezequiel Garcia
2019-04-01 3:14 ` Tomasz Figa
2019-04-12 19:25 ` Ezequiel Garcia
2019-04-15 4:07 ` Tomasz Figa
2019-03-04 19:25 ` [PATCH v2 11/11] rockchip/vpu: Add support for MPEG-2 decoding Ezequiel Garcia
2019-04-01 3:52 ` Tomasz Figa
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='CAAFQd5DjVDhFMc=WpnvF61yBcPm=BUQhVtBWSyphkkM8QubFzA@mail.gmail.com' \
--to=tfiga@chromium.org \
--cc=ezequiel@collabora.com \
--cc=hans.verkuil@cisco.com \
--cc=heiko@sntech.de \
--cc=jonas@kwiboo.se \
--cc=kernel@collabora.com \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=nicolas.dufresne@collabora.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).