From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.7 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 30C94C43603 for ; Thu, 5 Dec 2019 14:33:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0BB6324651 for ; Thu, 5 Dec 2019 14:33:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729572AbfLEOdY (ORCPT ); Thu, 5 Dec 2019 09:33:24 -0500 Received: from bhuna.collabora.co.uk ([46.235.227.227]:49636 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729099AbfLEOdX (ORCPT ); Thu, 5 Dec 2019 09:33:23 -0500 Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: ezequiel) with ESMTPSA id 62EBD29227C Message-ID: <88a48cb78843458b55896eeb3af2f46488d42744.camel@collabora.com> Subject: Re: [PATCH v3 2/3] media: hantro: Support color conversion via post-processing From: Ezequiel Garcia To: Philipp Zabel , linux-media@vger.kernel.org Cc: kernel@collabora.com, Tomasz Figa , linux-rockchip@lists.infradead.org, Heiko Stuebner , Jonas Karlman , Boris Brezillon , Chris Healy , linux-kernel@vger.kernel.org Date: Thu, 05 Dec 2019 11:33:13 -0300 In-Reply-To: References: <20191113175603.24742-1-ezequiel@collabora.com> <20191113175603.24742-3-ezequiel@collabora.com> <1e1c7a0e3d25187723ccac1a8360b5aae9aed8cd.camel@pengutronix.de> Organization: Collabora Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.34.1-2 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Philipp, On Fri, 2019-11-15 at 12:44 -0300, Ezequiel Garcia wrote: > Hello Philipp, > > Thanks for reviewing. > > On Thu, 2019-11-14 at 10:48 +0100, Philipp Zabel wrote: > > Hi Ezequiel, > > > > On Wed, 2019-11-13 at 14:56 -0300, Ezequiel Garcia wrote: > > > The Hantro G1 decoder is able to enable a post-processor > > > on the decoding pipeline, which can be used to perform > > > scaling and color conversion. > > > > > > The post-processor is integrated to the decoder, and it's > > > possible to use it in a way that is completely transparent > > > to the user. > > > > > > This commit enables color conversion via post-processing, > > > which means the driver now exposes YUV packed, in addition to NV12. > > > > > > Signed-off-by: Ezequiel Garcia > > > --- > > > drivers/staging/media/hantro/Makefile | 1 + > > > drivers/staging/media/hantro/hantro.h | 64 +++++++- > > > drivers/staging/media/hantro/hantro_drv.c | 8 +- > > > .../staging/media/hantro/hantro_g1_h264_dec.c | 2 +- > > > .../media/hantro/hantro_g1_mpeg2_dec.c | 2 +- > > > drivers/staging/media/hantro/hantro_g1_regs.h | 53 +++++++ > > > .../staging/media/hantro/hantro_g1_vp8_dec.c | 2 +- > > > drivers/staging/media/hantro/hantro_h264.c | 6 +- > > > drivers/staging/media/hantro/hantro_hw.h | 13 ++ > > > .../staging/media/hantro/hantro_postproc.c | 141 ++++++++++++++++++ > > > drivers/staging/media/hantro/hantro_v4l2.c | 52 ++++++- > > > drivers/staging/media/hantro/rk3288_vpu_hw.c | 10 ++ > > > 12 files changed, 343 insertions(+), 11 deletions(-) > > > create mode 100644 drivers/staging/media/hantro/hantro_postproc.c > > > > > > [..] > > > > pix_mp->plane_fmt[0].sizeimage += > > > 128 * DIV_ROUND_UP(pix_mp->width, 16) * > > > DIV_ROUND_UP(pix_mp->height, 16); > > > @@ -611,10 +643,23 @@ static int hantro_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]; > > > - if (ctx->codec_ops->init) > > > + if (ctx->codec_ops->init) { > > > ret = ctx->codec_ops->init(ctx); > > > + if (ret) > > > + return ret; > > > + } > > > + > > > + if (hantro_needs_postproc(ctx)) { > > > + ret = hantro_postproc_alloc(ctx); > > > > Why is this done in start_streaming? Wouldn't capture side REQBUFS be a > > better place for this? > > > > Yes, makes sense as well. > This didn't work so well, so I have decided to leave it as-is in the just submitted v4 series. The vb2 framework provides two mechanism for drivers to allocate buffers, REQBUFS and CREATEBUFS, so the bounce buffer allocation has to be hooked on both of them. Also, REQBUFS and CREATEBUFS can be called multiple times to grow/shrink the vb2_queue, so the driver has to check if the bounce buffers were already created or not. Not a big deal, but I felt the implementation ended up being too nasty for my taste. If fragmentation turns out to be an issue and we want to avoid allocating and destroying in start and stop (STREAMOFF, STREAMON), we can revisit this. Thanks, Ezequiel