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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable 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 A736BC433E9 for ; Fri, 5 Mar 2021 09:28:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5578165015 for ; Fri, 5 Mar 2021 09:28:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229653AbhCEJ2T (ORCPT ); Fri, 5 Mar 2021 04:28:19 -0500 Received: from bhuna.collabora.co.uk ([46.235.227.227]:35630 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229591AbhCEJ1y (ORCPT ); Fri, 5 Mar 2021 04:27:54 -0500 Received: from [IPv6:2a01:e0a:4cb:a870:b9e2:e9f:d661:5a2f] (unknown [IPv6:2a01:e0a:4cb:a870:b9e2:e9f:d661:5a2f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: benjamin.gaignard) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 9279D1F468C9; Fri, 5 Mar 2021 09:27:47 +0000 (GMT) Subject: Re: [PATCH v4 05/11] media: hantro: Add a field to distinguish the hardware versions To: Ezequiel Garcia , p.zabel@pengutronix.de, mchehab@kernel.org, robh+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com, gregkh@linuxfoundation.org, mripard@kernel.org, paul.kocialkowski@bootlin.com, wens@csie.org, jernej.skrabec@siol.net, peng.fan@nxp.com, hverkuil-cisco@xs4all.nl, dan.carpenter@oracle.com Cc: linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kernel@collabora.com References: <20210303113952.178519-1-benjamin.gaignard@collabora.com> <20210303113952.178519-6-benjamin.gaignard@collabora.com> <32899bc605ae7173c29b25a396e21d7fad32d4bf.camel@collabora.com> From: Benjamin Gaignard Message-ID: <23f62276-237d-1161-259a-84748db7365b@collabora.com> Date: Fri, 5 Mar 2021 10:27:44 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <32899bc605ae7173c29b25a396e21d7fad32d4bf.camel@collabora.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 03/03/2021 à 23:05, Ezequiel Garcia a écrit : > On Wed, 2021-03-03 at 12:39 +0100, Benjamin Gaignard wrote: >> Decoders hardware blocks could exist in multiple versions: add >> a field to distinguish them at runtime. >> G2 hardware block doesn't have postprocessor hantro_needs_postproc >> function should always returns false in for this hardware. >> hantro_needs_postproc function becoming to much complex to >> stay inline in .h file move it to .c file. >> > Note that I already questioned this patch before: > > https://lkml.org/lkml/2021/2/17/722 > > I think it's better to rely on of_device_id.data for this > type of thing. > > In particular, I was expecting that just using > hantro_variant.postproc_regs would be enough. > > Can you try if that works and avoid reading swreg(0) > and probing the hardware core? I have found a way to remove this: if the variant doesn't define post processor formats, needs_postproc function will always returns false and that what the only useful usage of this version field. Benjamin > > Thanks! > Ezequiel > >> Keep the default behavoir to be G1 hardware. >> >> Signed-off-by: Benjamin Gaignard >> --- >>  drivers/staging/media/hantro/hantro.h          | 13 +++++++------ >>  drivers/staging/media/hantro/hantro_drv.c      |  2 ++ >>  drivers/staging/media/hantro/hantro_postproc.c | 17 +++++++++++++++++ >>  3 files changed, 26 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h >> index a76a0d79db9f..05876e426419 100644 >> --- a/drivers/staging/media/hantro/hantro.h >> +++ b/drivers/staging/media/hantro/hantro.h >> @@ -37,6 +37,9 @@ struct hantro_codec_ops; >>  #define HANTRO_HEVC_DECODER    BIT(19) >>  #define HANTRO_DECODERS                0xffff0000 >> >> +#define HANTRO_G1_REV          0x6731 >> +#define HANTRO_G2_REV          0x6732 >> + >>  /** >>   * struct hantro_irq - irq handler and name >>   * >> @@ -171,6 +174,7 @@ hantro_vdev_to_func(struct video_device *vdev) >>   * @enc_base:          Mapped address of VPU encoder register for convenience. >>   * @dec_base:          Mapped address of VPU decoder register for convenience. >>   * @ctrl_base:         Mapped address of VPU control block. >> + * @core_hw_dec_rev    Runtime detected HW decoder core revision >>   * @vpu_mutex:         Mutex to synchronize V4L2 calls. >>   * @irqlock:           Spinlock to synchronize access to data structures >>   *                     shared with interrupt handlers. >> @@ -190,6 +194,7 @@ struct hantro_dev { >>         void __iomem *enc_base; >>         void __iomem *dec_base; >>         void __iomem *ctrl_base; >> +       u32 core_hw_dec_rev; >> >>         struct mutex vpu_mutex; /* video_device lock */ >>         spinlock_t irqlock; >> @@ -412,12 +417,8 @@ hantro_get_dst_buf(struct hantro_ctx *ctx) >>         return v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx); >>  } >> >> -static inline bool >> -hantro_needs_postproc(const struct hantro_ctx *ctx, >> -                     const struct hantro_fmt *fmt) >> -{ >> -       return !ctx->is_encoder && fmt->fourcc != V4L2_PIX_FMT_NV12; >> -} >> +bool hantro_needs_postproc(const struct hantro_ctx *ctx, >> +                          const struct hantro_fmt *fmt); >> >>  static inline dma_addr_t >>  hantro_get_dec_buf_addr(struct hantro_ctx *ctx, struct vb2_buffer *vb) >> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c >> index f0b68e16fcc0..e3e6df28f470 100644 >> --- a/drivers/staging/media/hantro/hantro_drv.c >> +++ b/drivers/staging/media/hantro/hantro_drv.c >> @@ -836,6 +836,8 @@ static int hantro_probe(struct platform_device *pdev) >>         } >>         vpu->enc_base = vpu->reg_bases[0] + vpu->variant->enc_offset; >>         vpu->dec_base = vpu->reg_bases[0] + vpu->variant->dec_offset; >> +       /* by default decoder is G1 */ >> +       vpu->core_hw_dec_rev = HANTRO_G1_REV; >> >>         ret = dma_set_coherent_mask(vpu->dev, DMA_BIT_MASK(32)); >>         if (ret) { >> diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c >> index 6d2a8f2a8f0b..050880f720d6 100644 >> --- a/drivers/staging/media/hantro/hantro_postproc.c >> +++ b/drivers/staging/media/hantro/hantro_postproc.c >> @@ -50,6 +50,23 @@ const struct hantro_postproc_regs hantro_g1_postproc_regs = { >>         .display_width = {G1_REG_PP_DISPLAY_WIDTH, 0, 0xfff}, >>  }; >> >> +bool hantro_needs_postproc(const struct hantro_ctx *ctx, >> +                          const struct hantro_fmt *fmt) >> +{ >> +       struct hantro_dev *vpu = ctx->dev; >> + >> +       if (ctx->is_encoder) >> +               return false; >> + >> +       if (vpu->core_hw_dec_rev == HANTRO_G1_REV):q >> +               return fmt->fourcc != V4L2_PIX_FMT_NV12; >> + >> +       if (vpu->core_hw_dec_rev == HANTRO_G2_REV) >> +               return false; >> + >> +       return false; >> +} >> + >>  void hantro_postproc_enable(struct hantro_ctx *ctx) >>  { >>         struct hantro_dev *vpu = ctx->dev; > >