| Message ID | 20190304192529.14200-7-ezequiel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add MPEG-2 decoding to Rockchip VPU | expand |
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
On Thu, 2019-03-28 at 15:15 +0900, Tomasz Figa wrote: > 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. > Well, the struct is already rockchip_vpu_ctx, so access would look like: ctx->ctx.jpeg_enc, ctx->ctx.mpeg2_dec Perhaps we can just drop the _ctx, and so: ctx->jpeg_enc, ctx->mpeg2_dec > > }; > > > > /** > > 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? > Well, I had a TODO item to remove the bounce buffer for JPEG. IIRC, the ChromeOS VPU driver doesn't have any bounce buffer, so I was under the impression we wouldn't need one. However, we have yet to discuss the encoder implementation. We might decide to produce the headers for some video coded format, in which case we might need a bounce buffer. In any case, this is just a comment to reflect that this piece of code is just temporary. > > + */ > > + 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? > > Yeah, that'd be better. Thanks for the feedback, Eze
On Fri, Mar 29, 2019 at 3:30 AM Ezequiel Garcia <ezequiel@collabora.com> wrote: > > On Thu, 2019-03-28 at 15:15 +0900, Tomasz Figa wrote: > > 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. > > > > Well, the struct is already rockchip_vpu_ctx, so access would look like: > > ctx->ctx.jpeg_enc, ctx->ctx.mpeg2_dec > > Perhaps we can just drop the _ctx, and so: > > ctx->jpeg_enc, ctx->mpeg2_dec > Even better, thanks. > > > }; > > > > > > /** > > > 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? > > > > Well, I had a TODO item to remove the bounce buffer for JPEG. > > IIRC, the ChromeOS VPU driver doesn't have any bounce buffer, > so I was under the impression we wouldn't need one. > > However, we have yet to discuss the encoder implementation. > We might decide to produce the headers for some video coded format, > in which case we might need a bounce buffer. We don't really need a bounce buffer to handle headers, we can do a memmove() instead, as we did for the VP8 encoder [1]. However, it was more like a loopback - the driver would just copy the header that the userspace set through a control, so we decided to drop it and move completely to the userspace [2] for H.264. [1] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/5bc4e95afddd1300368d0552b228824252bd789f/drivers/media/platform/rk3288-vpu/rk3288_vpu_hw_vp8e.c#52 [2] https://chromium.googlesource.com/chromiumos/third_party/libv4lplugins/+/5e6034258146af6be973fb6a5bb6b9d6e7489437/libv4l-rockchip_v2/libvepu/h264e/h264e.c#591 > > In any case, this is just a comment to reflect that this piece > of code is just temporary. > My point is that the comment suggests that other encoders would be affected by this, which I don't believe is true, because the code explicitly checks if the bounce buffer pointer is non-NULL and for other encoders it would be NULL. /* * The bounce buffer is only for the JPEG encoder. * TODO: Rework the JPEG encoder to eliminate the need * for a bounce buffer. */ Best regards, Tomasz
diff --git a/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw.c b/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw.c index a5e9d183fffd..056ee017c798 100644 --- a/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw.c +++ b/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw.c @@ -98,6 +98,8 @@ static const struct rockchip_vpu_codec_ops rk3288_vpu_codec_ops[] = { [RK_VPU_MODE_JPEG_ENC] = { .run = rk3288_vpu_jpeg_enc_run, .reset = rk3288_vpu_enc_reset, + .start = rockchip_vpu_jpeg_enc_start, + .stop = rockchip_vpu_jpeg_enc_stop, }, }; diff --git a/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c b/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c index 06daea66fb49..b05b1fdcff90 100644 --- a/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c +++ b/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c @@ -37,9 +37,9 @@ static void rk3288_vpu_jpeg_enc_set_buffers(struct rockchip_vpu_dev *vpu, WARN_ON(pix_fmt->num_planes > 3); - vepu_write_relaxed(vpu, ctx->bounce_dma_addr, + vepu_write_relaxed(vpu, ctx->jpeg_enc_ctx.bounce_buffer.dma, VEPU_REG_ADDR_OUTPUT_STREAM); - vepu_write_relaxed(vpu, ctx->bounce_size, + vepu_write_relaxed(vpu, ctx->jpeg_enc_ctx.bounce_buffer.size, VEPU_REG_STR_BUF_LIMIT); if (pix_fmt->num_planes == 1) { diff --git a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw.c b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw.c index 6fdef61e2127..8469e474c975 100644 --- a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw.c +++ b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw.c @@ -98,6 +98,8 @@ static const struct rockchip_vpu_codec_ops rk3399_vpu_codec_ops[] = { [RK_VPU_MODE_JPEG_ENC] = { .run = rk3399_vpu_jpeg_enc_run, .reset = rk3399_vpu_enc_reset, + .start = rockchip_vpu_jpeg_enc_start, + .stop = rockchip_vpu_jpeg_enc_stop, }, }; diff --git a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c index 3d438797692e..fe92c40d0a84 100644 --- a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c +++ b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c @@ -69,9 +69,9 @@ static void rk3399_vpu_jpeg_enc_set_buffers(struct rockchip_vpu_dev *vpu, WARN_ON(pix_fmt->num_planes > 3); - vepu_write_relaxed(vpu, ctx->bounce_dma_addr, + vepu_write_relaxed(vpu, ctx->jpeg_enc_ctx.bounce_buffer.dma, VEPU_REG_ADDR_OUTPUT_STREAM); - vepu_write_relaxed(vpu, ctx->bounce_size, + vepu_write_relaxed(vpu, ctx->jpeg_enc_ctx.bounce_buffer.size, VEPU_REG_STR_BUF_LIMIT); if (pix_fmt->num_planes == 1) { 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; + }; }; /** 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. + */ + 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; diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c b/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c index ae1ff3d9b9d2..2b28403314bc 100644 --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c @@ -514,6 +514,7 @@ static int rockchip_vpu_start_streaming(struct vb2_queue *q, unsigned int count) { struct rockchip_vpu_ctx *ctx = vb2_get_drv_priv(q); enum rockchip_vpu_codec_mode codec_mode; + int ret = 0; if (V4L2_TYPE_IS_OUTPUT(q->type)) ctx->sequence_out = 0; @@ -526,17 +527,10 @@ static int rockchip_vpu_start_streaming(struct vb2_queue *q, unsigned int count) vpu_debug(4, "Codec mode = %d\n", codec_mode); ctx->codec_ops = &ctx->dev->variant->codec_ops[codec_mode]; - /* A bounce buffer is needed for the JPEG payload */ - if (!V4L2_TYPE_IS_OUTPUT(q->type)) { - ctx->bounce_size = ctx->dst_fmt.plane_fmt[0].sizeimage - - ctx->vpu_dst_fmt->header_size; - ctx->bounce_buf = dma_alloc_attrs(ctx->dev->dev, - ctx->bounce_size, - &ctx->bounce_dma_addr, - GFP_KERNEL, - DMA_ATTR_ALLOC_SINGLE_PAGES); - } - return 0; + if (!V4L2_TYPE_IS_OUTPUT(q->type)) + if (ctx->codec_ops && ctx->codec_ops->start) + ret = ctx->codec_ops->start(ctx); + return ret; } static void rockchip_vpu_stop_streaming(struct vb2_queue *q) @@ -544,11 +538,8 @@ static void rockchip_vpu_stop_streaming(struct vb2_queue *q) struct rockchip_vpu_ctx *ctx = vb2_get_drv_priv(q); if (!V4L2_TYPE_IS_OUTPUT(q->type)) - dma_free_attrs(ctx->dev->dev, - ctx->bounce_size, - ctx->bounce_buf, - ctx->bounce_dma_addr, - DMA_ATTR_ALLOC_SINGLE_PAGES); + if (ctx->codec_ops && ctx->codec_ops->stop) + ctx->codec_ops->stop(ctx); /* * The mem2mem framework calls v4l2_m2m_cancel_job before diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_hw.h b/drivers/staging/media/rockchip/vpu/rockchip_vpu_hw.h index 2b955da1be1a..e2e84526f263 100644 --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_hw.h +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_hw.h @@ -18,9 +18,33 @@ struct rockchip_vpu_ctx; struct rockchip_vpu_buf; struct rockchip_vpu_variant; +/** + * struct rockchip_vpu_aux_buf - auxiliary DMA buffer for hardware data + * @cpu: CPU pointer to the buffer. + * @dma: DMA address of the buffer. + * @size: Size of the buffer. + */ +struct rockchip_vpu_aux_buf { + void *cpu; + dma_addr_t dma; + size_t size; +}; + +/** + * struct rockchip_vpu_jpeg_enc_hw_ctx + * @bounce_buffer: Bounce buffer + */ +struct rockchip_vpu_jpeg_enc_hw_ctx { + struct rockchip_vpu_aux_buf bounce_buffer; +}; + /** * 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. * @run: Start single {en,de)coding job. Called from atomic context * to indicate that a pair of buffers is ready and the hardware * should be programmed and started. @@ -28,6 +52,8 @@ struct rockchip_vpu_variant; * @reset: Reset the hardware in case of a timeout. */ struct rockchip_vpu_codec_ops { + int (*start)(struct rockchip_vpu_ctx *ctx); + void (*stop)(struct rockchip_vpu_ctx *ctx); void (*run)(struct rockchip_vpu_ctx *ctx); void (*done)(struct rockchip_vpu_ctx *ctx, enum vb2_buffer_state); void (*reset)(struct rockchip_vpu_ctx *ctx); @@ -54,5 +80,7 @@ void rockchip_vpu_irq_done(struct rockchip_vpu_dev *vpu, void rk3288_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx); void rk3399_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx); +int rockchip_vpu_jpeg_enc_start(struct rockchip_vpu_ctx *ctx); +void rockchip_vpu_jpeg_enc_stop(struct rockchip_vpu_ctx *ctx); #endif /* ROCKCHIP_VPU_HW_H_ */ diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_jpeg.c b/drivers/staging/media/rockchip/vpu/rockchip_vpu_jpeg.c index 0ff0badc1f7a..3244cca4e915 100644 --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_jpeg.c +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_jpeg.c @@ -6,9 +6,11 @@ * Copyright (C) Jean-Francois Moine (http://moinejf.free.fr) * Copyright (C) 2014 Philipp Zabel, Pengutronix */ +#include <linux/dma-mapping.h> #include <linux/kernel.h> #include <linux/string.h> #include "rockchip_vpu_jpeg.h" +#include "rockchip_vpu.h" #define LUMA_QUANT_OFF 7 #define CHROMA_QUANT_OFF 72 @@ -288,3 +290,26 @@ void rockchip_vpu_jpeg_header_assemble(struct rockchip_vpu_jpeg_ctx *ctx) jpeg_set_quality(buf, ctx->quality); } + +int rockchip_vpu_jpeg_enc_start(struct rockchip_vpu_ctx *ctx) +{ + ctx->jpeg_enc_ctx.bounce_buffer.size = ctx->dst_fmt.plane_fmt[0].sizeimage - + ctx->vpu_dst_fmt->header_size; + ctx->jpeg_enc_ctx.bounce_buffer.cpu = dma_alloc_attrs(ctx->dev->dev, + ctx->jpeg_enc_ctx.bounce_buffer.size, + &ctx->jpeg_enc_ctx.bounce_buffer.dma, + GFP_KERNEL, + DMA_ATTR_ALLOC_SINGLE_PAGES); + if (!ctx->jpeg_enc_ctx.bounce_buffer.cpu) + return -ENOMEM; + return 0; +} + +void rockchip_vpu_jpeg_enc_stop(struct rockchip_vpu_ctx *ctx) +{ + dma_free_attrs(ctx->dev->dev, + ctx->jpeg_enc_ctx.bounce_buffer.size, + ctx->jpeg_enc_ctx.bounce_buffer.cpu, + ctx->jpeg_enc_ctx.bounce_buffer.dma, + DMA_ATTR_ALLOC_SINGLE_PAGES); +}
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(-)