Received: by 2002:a05:6a10:a0d1:0:0:0:0 with SMTP id j17csp142240pxa; Fri, 21 Aug 2020 03:37:13 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyCfVljpVbCux441ApkW59k6GxUC6WBt+/1OPpqQxYkNaMxLeDD9Wg7cxYR6rbAc9En+paG X-Received: by 2002:a17:906:3b4e:: with SMTP id h14mr2342778ejf.546.1598006233253; Fri, 21 Aug 2020 03:37:13 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1598006233; cv=none; d=google.com; s=arc-20160816; b=AZ6beqTwxIzZJ03KujwihvtjvcxpxkKX9iP6MaPrrAnzHk/fKwWLOf/UDFr+BbHQvE g639cUAgwgQi0hfqCHlW1z2UnFLBUYGMiWfBEBIIksovwk8sUaCL71epamqycRyuEBG5 uJBAWCLn9sjiGPQc2b0zT5okDoGUVuCfneWufXEcRoZ6Qx6r2S1lCHUCrfKQzt9cc0I9 CgaSp8Xyxz3qYflkX7tOuCzacKV9y2ElCWwym941cfMPo/3M2J7dTnDs/i2g716M/L+s EzQiV8SBgg11RFUulspeXDG1gHOYIfd2n/bhwUi4zTY8agw6ZJw0UozOmDwfaYxntwrk 2heg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=+tEqBuZDx8aRp8oEgxXgfV7eP8rUllwIwZZ3eDH4gQk=; b=f1PeprnCiGcwRrez3ey2QcSWbiHYtD3fUqg14J1qSt+RlwzVZKCLaAx2MvmpMEmVwo CV8153zA9ZZJPUdmbwI5YWsudco0ctW3OfSeUcyBvrjQ8/I4UYZnqwK7LACvN9uCbjru 6o70ay5dGSdt9P+KQ0D0N1iUtgNBqDdC7r+RI/LS3cEXcBP94+XrHfw6hkQBaHVdabqt PDJPYTlzEdfqUpmOeWSJ6Te6Xsak/+GZkHBDobPm1QBvIu349ecEUQj7SnnMMXEk5Wzn jqiOCDTYaj0vqFsl1FbGyWhV0TPE7fNhGf6HlmMZXE8b1bL55lTakhwp1bMS8vHYg2yP eIyA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=iXqvGG6U; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id z7si967947edq.250.2020.08.21.03.36.49; Fri, 21 Aug 2020 03:37:13 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=iXqvGG6U; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728265AbgHUKfd (ORCPT + 99 others); Fri, 21 Aug 2020 06:35:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36122 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728218AbgHUKf2 (ORCPT ); Fri, 21 Aug 2020 06:35:28 -0400 Received: from mail-oo1-xc44.google.com (mail-oo1-xc44.google.com [IPv6:2607:f8b0:4864:20::c44]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 147AEC061387 for ; Fri, 21 Aug 2020 03:35:28 -0700 (PDT) Received: by mail-oo1-xc44.google.com with SMTP id x1so240669oox.6 for ; Fri, 21 Aug 2020 03:35:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+tEqBuZDx8aRp8oEgxXgfV7eP8rUllwIwZZ3eDH4gQk=; b=iXqvGG6UzrvqsOKwMqVyWhUn8epXHxWgFkRvEO4fXpBOBQjNFuAWEIVWuAO7glrucn +jRu7hsZry45FSFhoULiEO2zyhlqGZn1oepV07fP4p+kulvqAIS2fqxZELVNPyguNeKB P1isi5cANrAEDAGUYrYymrucG9xNsIlghrqv8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+tEqBuZDx8aRp8oEgxXgfV7eP8rUllwIwZZ3eDH4gQk=; b=WepO6mt68JvXT8rS7fPJvhNAxq1Z6zUYq8G9+8UCsri78AULkcjBST9s64vFqenksi udLgYTn3SnxHiRbSiE/znYY2zMJEfTK1YS1CboLEDz6p7skecmVJkEEGNYc6O7KkLNpr CnWKKkOSTyn1DDkpVoRjw1AFdgoDnqTQp6Ge3ZXTjkaTSrTdH5erJzNhIglMSSwbhzrL N61D28XiQuJJc13YA591Q8M1SqM+LFX9QXnbpuPrEu10XjhEf5yIfO0W81ZL3eCK68OL dEOOa6lDvvxD0LJQZt7bzzl/Wq5ShqJZwxoL2pSmJKvgRUbQTfCsdom0o6yFRzDCyk3u a9Zg== X-Gm-Message-State: AOAM533TZTyuyr9fYkHepeWM0h7FPxr066nVmbVecwEmVFZqrdMXvL80 UycTq1st4QjCbHkKJgIjFwRYfUsWdRaCf4aw X-Received: by 2002:a4a:2f4b:: with SMTP id p72mr1537951oop.39.1598006125482; Fri, 21 Aug 2020 03:35:25 -0700 (PDT) Received: from mail-ot1-f52.google.com (mail-ot1-f52.google.com. [209.85.210.52]) by smtp.gmail.com with ESMTPSA id r124sm242648oib.22.2020.08.21.03.35.23 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 21 Aug 2020 03:35:24 -0700 (PDT) Received: by mail-ot1-f52.google.com with SMTP id h16so1198724oti.7 for ; Fri, 21 Aug 2020 03:35:23 -0700 (PDT) X-Received: by 2002:a9d:7d09:: with SMTP id v9mr1359881otn.97.1598006123082; Fri, 21 Aug 2020 03:35:23 -0700 (PDT) MIME-Version: 1.0 References: <20200713060842.471356-1-acourbot@chromium.org> <20200713060842.471356-2-acourbot@chromium.org> In-Reply-To: From: Alexandre Courbot Date: Fri, 21 Aug 2020 19:35:11 +0900 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3 01/16] media: mtk-vcodec: abstract firmware interface To: Ezequiel Garcia Cc: Tiffany Lin , Andrew-CT Chen , Hans Verkuil , Yunfei Dong , Maoguang Meng , linux-media , "moderated list:ARM/Mediatek SoC support" , devicetree , Linux Kernel Mailing List , Pi-Hsun Shih Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jul 27, 2020 at 11:25 PM Ezequiel Garcia wrote: > > On Mon, 27 Jul 2020 at 11:24, Ezequiel Garcia > wrote: > > > > On Mon, 27 Jul 2020 at 06:06, Alexandre Courbot wrote: > > > > > > On Thu, Jul 23, 2020 at 6:23 AM Ezequiel Garcia > > > wrote: > > > > > > > > On Mon, 13 Jul 2020 at 03:09, Alexandre Courbot wrote: > > > > > > > > > > From: Yunfei Dong > > > > > > > > > > MT8183's codec firwmare is run by a different remote processor from > > > > > MT8173. While the firmware interface is basically the same, the way to > > > > > invoke it differs. Abstract all firmware calls under a layer that will > > > > > allow us to handle both firmware types transparently. > > > > > > > > > > Signed-off-by: Yunfei Dong > > > > > [acourbot: refactor, cleanup and split] > > > > > Co-developed-by: Alexandre Courbot > > > > > Signed-off-by: Alexandre Courbot > > > > > [pihsun: fix error path and add mtk_vcodec_fw_release] > > > > > Signed-off-by: Pi-Hsun Shih > > > > > Reviewed-by: Tiffany Lin > > > > > Acked-by: Tiffany Lin > > > > > --- > > > > > drivers/media/platform/mtk-vcodec/Makefile | 4 +- > > > > > .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 50 ++--- > > > > > .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 1 - > > > > > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 5 +- > > > > > .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 47 ++--- > > > > > .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 2 - > > > > > .../media/platform/mtk-vcodec/mtk_vcodec_fw.c | 172 ++++++++++++++++++ > > > > > .../media/platform/mtk-vcodec/mtk_vcodec_fw.h | 36 ++++ > > > > > .../platform/mtk-vcodec/mtk_vcodec_util.c | 1 - > > > > > .../platform/mtk-vcodec/vdec/vdec_h264_if.c | 1 - > > > > > .../platform/mtk-vcodec/vdec/vdec_vp8_if.c | 1 - > > > > > .../platform/mtk-vcodec/vdec/vdec_vp9_if.c | 1 - > > > > > .../media/platform/mtk-vcodec/vdec_drv_base.h | 2 - > > > > > .../media/platform/mtk-vcodec/vdec_drv_if.c | 1 - > > > > > .../media/platform/mtk-vcodec/vdec_vpu_if.c | 12 +- > > > > > .../media/platform/mtk-vcodec/vdec_vpu_if.h | 11 +- > > > > > .../platform/mtk-vcodec/venc/venc_h264_if.c | 15 +- > > > > > .../platform/mtk-vcodec/venc/venc_vp8_if.c | 8 +- > > > > > .../media/platform/mtk-vcodec/venc_drv_if.c | 1 - > > > > > .../media/platform/mtk-vcodec/venc_vpu_if.c | 17 +- > > > > > .../media/platform/mtk-vcodec/venc_vpu_if.h | 5 +- > > > > > 21 files changed, 290 insertions(+), 103 deletions(-) > > > > > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c > > > > > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.h > > > > > > > > > > diff --git a/drivers/media/platform/mtk-vcodec/Makefile b/drivers/media/platform/mtk-vcodec/Makefile > > > > > index 37b94b555fa1..b8636119ed0a 100644 > > > > > --- a/drivers/media/platform/mtk-vcodec/Makefile > > > > > +++ b/drivers/media/platform/mtk-vcodec/Makefile > > > > > @@ -12,7 +12,7 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \ > > > > > vdec_vpu_if.o \ > > > > > mtk_vcodec_dec.o \ > > > > > mtk_vcodec_dec_pm.o \ > > > > > - > > > > > + mtk_vcodec_fw.o > > > > > > > > > > mtk-vcodec-enc-y := venc/venc_vp8_if.o \ > > > > > venc/venc_h264_if.o \ > > > > > @@ -25,5 +25,3 @@ mtk-vcodec-enc-y := venc/venc_vp8_if.o \ > > > > > > > > > > mtk-vcodec-common-y := mtk_vcodec_intr.o \ > > > > > mtk_vcodec_util.o\ > > > > > - > > > > > -ccflags-y += -I$(srctree)/drivers/media/platform/mtk-vpu > > > > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > > > > > index 97a1b6664c20..4f07a5fcce7f 100644 > > > > > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > > > > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > > > > > @@ -20,7 +20,7 @@ > > > > > #include "mtk_vcodec_dec_pm.h" > > > > > #include "mtk_vcodec_intr.h" > > > > > #include "mtk_vcodec_util.h" > > > > > -#include "mtk_vpu.h" > > > > > +#include "mtk_vcodec_fw.h" > > > > > > > > > > #define VDEC_HW_ACTIVE 0x10 > > > > > #define VDEC_IRQ_CFG 0x11 > > > > > @@ -77,22 +77,6 @@ static irqreturn_t mtk_vcodec_dec_irq_handler(int irq, void *priv) > > > > > return IRQ_HANDLED; > > > > > } > > > > > > > > > > -static void mtk_vcodec_dec_reset_handler(void *priv) > > > > > -{ > > > > > - struct mtk_vcodec_dev *dev = priv; > > > > > - struct mtk_vcodec_ctx *ctx; > > > > > - > > > > > - mtk_v4l2_err("Watchdog timeout!!"); > > > > > - > > > > > - mutex_lock(&dev->dev_mutex); > > > > > - list_for_each_entry(ctx, &dev->ctx_list, list) { > > > > > - ctx->state = MTK_STATE_ABORT; > > > > > - mtk_v4l2_debug(0, "[%d] Change to state MTK_STATE_ERROR", > > > > > - ctx->id); > > > > > - } > > > > > - mutex_unlock(&dev->dev_mutex); > > > > > -} > > > > > - > > > > > static int fops_vcodec_open(struct file *file) > > > > > { > > > > > struct mtk_vcodec_dev *dev = video_drvdata(file); > > > > > @@ -144,21 +128,20 @@ static int fops_vcodec_open(struct file *file) > > > > > if (v4l2_fh_is_singular(&ctx->fh)) { > > > > > mtk_vcodec_dec_pw_on(&dev->pm); > > > > > /* > > > > > - * vpu_load_firmware checks if it was loaded already and > > > > > - * does nothing in that case > > > > > + * Does nothing if firmware was already loaded. > > > > > */ > > > > > - ret = vpu_load_firmware(dev->vpu_plat_dev); > > > > > + ret = mtk_vcodec_fw_load_firmware(dev->fw_handler); > > > > > if (ret < 0) { > > > > > /* > > > > > * Return 0 if downloading firmware successfully, > > > > > * otherwise it is failed > > > > > */ > > > > > - mtk_v4l2_err("vpu_load_firmware failed!"); > > > > > + mtk_v4l2_err("failed to load firmware!"); > > > > > goto err_load_fw; > > > > > } > > > > > > > > > > dev->dec_capability = > > > > > - vpu_get_vdec_hw_capa(dev->vpu_plat_dev); > > > > > + mtk_vcodec_fw_get_vdec_capa(dev->fw_handler); > > > > > mtk_v4l2_debug(0, "decoder capability %x", dev->dec_capability); > > > > > } > > > > > > > > > > @@ -228,6 +211,8 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > > > > > struct mtk_vcodec_dev *dev; > > > > > struct video_device *vfd_dec; > > > > > struct resource *res; > > > > > + phandle rproc_phandle; > > > > > + enum mtk_vcodec_fw_type fw_type; > > > > > int i, ret; > > > > > > > > > > dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); > > > > > @@ -237,19 +222,21 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > > > > > INIT_LIST_HEAD(&dev->ctx_list); > > > > > dev->plat_dev = pdev; > > > > > > > > > > - dev->vpu_plat_dev = vpu_get_plat_device(dev->plat_dev); > > > > > - if (dev->vpu_plat_dev == NULL) { > > > > > - mtk_v4l2_err("[VPU] vpu device in not ready"); > > > > > - return -EPROBE_DEFER; > > > > > + if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu", > > > > > + &rproc_phandle)) { > > > > > + fw_type = VPU; > > > > > + } else { > > > > > + mtk_v4l2_err("Could not get vdec IPI device"); > > > > > + return -ENODEV; > > > > > } > > > > > - > > > > > - vpu_wdt_reg_handler(dev->vpu_plat_dev, mtk_vcodec_dec_reset_handler, > > > > > - dev, VPU_RST_DEC); > > > > > + dev->fw_handler = mtk_vcodec_fw_select(dev, fw_type, VPU_RST_DEC); > > > > > + if (IS_ERR(dev->fw_handler)) > > > > > + return PTR_ERR(dev->fw_handler); > > > > > > > > > > ret = mtk_vcodec_init_dec_pm(dev); > > > > > if (ret < 0) { > > > > > dev_err(&pdev->dev, "Failed to get mt vcodec clock source"); > > > > > - return ret; > > > > > + goto err_dec_pm; > > > > > } > > > > > > > > > > for (i = 0; i < NUM_MAX_VDEC_REG_BASE; i++) { > > > > > @@ -352,6 +339,8 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > > > > > v4l2_device_unregister(&dev->v4l2_dev); > > > > > err_res: > > > > > mtk_vcodec_release_dec_pm(dev); > > > > > +err_dec_pm: > > > > > + mtk_vcodec_fw_release(dev->fw_handler); > > > > > return ret; > > > > > } > > > > > > > > > > @@ -376,6 +365,7 @@ static int mtk_vcodec_dec_remove(struct platform_device *pdev) > > > > > > > > > > v4l2_device_unregister(&dev->v4l2_dev); > > > > > mtk_vcodec_release_dec_pm(dev); > > > > > + mtk_vcodec_fw_release(dev->fw_handler); > > > > > return 0; > > > > > } > > > > > > > > > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c > > > > > index 5a6ec8fb52da..36dfe3fc056a 100644 > > > > > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c > > > > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c > > > > > @@ -12,7 +12,6 @@ > > > > > > > > > > #include "mtk_vcodec_dec_pm.h" > > > > > #include "mtk_vcodec_util.h" > > > > > -#include "mtk_vpu.h" > > > > > > > > > > int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev) > > > > > { > > > > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > > > > > index 9fd56dee7fd1..e132c4ec463a 100644 > > > > > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > > > > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > > > > > @@ -309,13 +309,13 @@ struct mtk_vcodec_ctx { > > > > > * @m2m_dev_dec: m2m device for decoder > > > > > * @m2m_dev_enc: m2m device for encoder. > > > > > * @plat_dev: platform device > > > > > - * @vpu_plat_dev: mtk vpu platform device > > > > > * @ctx_list: list of struct mtk_vcodec_ctx > > > > > * @irqlock: protect data access by irq handler and work thread > > > > > * @curr_ctx: The context that is waiting for codec hardware > > > > > * > > > > > * @reg_base: Mapped address of MTK Vcodec registers. > > > > > * > > > > > + * @fw_handler: used to communicate with the firmware. > > > > > * @id_counter: used to identify current opened instance > > > > > * > > > > > * @encode_workqueue: encode work queue > > > > > @@ -344,12 +344,13 @@ struct mtk_vcodec_dev { > > > > > struct v4l2_m2m_dev *m2m_dev_dec; > > > > > struct v4l2_m2m_dev *m2m_dev_enc; > > > > > struct platform_device *plat_dev; > > > > > - struct platform_device *vpu_plat_dev; > > > > > struct list_head ctx_list; > > > > > spinlock_t irqlock; > > > > > struct mtk_vcodec_ctx *curr_ctx; > > > > > void __iomem *reg_base[NUM_MAX_VCODEC_REG_BASE]; > > > > > > > > > > + struct mtk_vcodec_fw *fw_handler; > > > > > + > > > > > unsigned long id_counter; > > > > > > > > > > struct workqueue_struct *decode_workqueue; > > > > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c > > > > > index 4d31f1ed113f..4340ea10afd0 100644 > > > > > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c > > > > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c > > > > > @@ -21,7 +21,7 @@ > > > > > #include "mtk_vcodec_enc_pm.h" > > > > > #include "mtk_vcodec_intr.h" > > > > > #include "mtk_vcodec_util.h" > > > > > -#include "mtk_vpu.h" > > > > > +#include "mtk_vcodec_fw.h" > > > > > > > > > > module_param(mtk_v4l2_dbg_level, int, S_IRUGO | S_IWUSR); > > > > > module_param(mtk_vcodec_dbg, bool, S_IRUGO | S_IWUSR); > > > > > @@ -101,22 +101,6 @@ static irqreturn_t mtk_vcodec_enc_lt_irq_handler(int irq, void *priv) > > > > > return IRQ_HANDLED; > > > > > } > > > > > > > > > > -static void mtk_vcodec_enc_reset_handler(void *priv) > > > > > -{ > > > > > - struct mtk_vcodec_dev *dev = priv; > > > > > - struct mtk_vcodec_ctx *ctx; > > > > > - > > > > > - mtk_v4l2_debug(0, "Watchdog timeout!!"); > > > > > - > > > > > - mutex_lock(&dev->dev_mutex); > > > > > - list_for_each_entry(ctx, &dev->ctx_list, list) { > > > > > - ctx->state = MTK_STATE_ABORT; > > > > > - mtk_v4l2_debug(0, "[%d] Change to state MTK_STATE_ABORT", > > > > > - ctx->id); > > > > > - } > > > > > - mutex_unlock(&dev->dev_mutex); > > > > > -} > > > > > - > > > > > static int fops_vcodec_open(struct file *file) > > > > > { > > > > > struct mtk_vcodec_dev *dev = video_drvdata(file); > > > > > @@ -159,10 +143,10 @@ static int fops_vcodec_open(struct file *file) > > > > > > > > > > if (v4l2_fh_is_singular(&ctx->fh)) { > > > > > /* > > > > > - * vpu_load_firmware checks if it was loaded already and > > > > > + * load fireware to checks if it was loaded already and > > > > > * does nothing in that case > > > > > */ > > > > > - ret = vpu_load_firmware(dev->vpu_plat_dev); > > > > > + ret = mtk_vcodec_fw_load_firmware(dev->fw_handler); > > > > > if (ret < 0) { > > > > > /* > > > > > * Return 0 if downloading firmware successfully, > > > > > @@ -173,7 +157,7 @@ static int fops_vcodec_open(struct file *file) > > > > > } > > > > > > > > > > dev->enc_capability = > > > > > - vpu_get_venc_hw_capa(dev->vpu_plat_dev); > > > > > + mtk_vcodec_fw_get_venc_capa(dev->fw_handler); > > > > > mtk_v4l2_debug(0, "encoder capability %x", dev->enc_capability); > > > > > } > > > > > > > > > > @@ -235,6 +219,8 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > > > > > struct mtk_vcodec_dev *dev; > > > > > struct video_device *vfd_enc; > > > > > struct resource *res; > > > > > + phandle rproc_phandle; > > > > > + enum mtk_vcodec_fw_type fw_type; > > > > > int i, j, ret; > > > > > > > > > > dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); > > > > > @@ -244,19 +230,21 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > > > > > INIT_LIST_HEAD(&dev->ctx_list); > > > > > dev->plat_dev = pdev; > > > > > > > > > > - dev->vpu_plat_dev = vpu_get_plat_device(dev->plat_dev); > > > > > - if (dev->vpu_plat_dev == NULL) { > > > > > - mtk_v4l2_err("[VPU] vpu device in not ready"); > > > > > - return -EPROBE_DEFER; > > > > > + if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu", > > > > > + &rproc_phandle)) { > > > > > + fw_type = VPU; > > > > > + } else { > > > > > + mtk_v4l2_err("Could not get venc IPI device"); > > > > > + return -ENODEV; > > > > > } > > > > > - > > > > > - vpu_wdt_reg_handler(dev->vpu_plat_dev, mtk_vcodec_enc_reset_handler, > > > > > - dev, VPU_RST_ENC); > > > > > + dev->fw_handler = mtk_vcodec_fw_select(dev, fw_type, VPU_RST_ENC); > > > > > + if (IS_ERR(dev->fw_handler)) > > > > > + return PTR_ERR(dev->fw_handler); > > > > > > > > > > ret = mtk_vcodec_init_enc_pm(dev); > > > > > if (ret < 0) { > > > > > dev_err(&pdev->dev, "Failed to get mt vcodec clock source!"); > > > > > - return ret; > > > > > + goto err_enc_pm; > > > > > } > > > > > > > > > > for (i = VENC_SYS, j = 0; i < NUM_MAX_VCODEC_REG_BASE; i++, j++) { > > > > > @@ -377,6 +365,8 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > > > > > v4l2_device_unregister(&dev->v4l2_dev); > > > > > err_res: > > > > > mtk_vcodec_release_enc_pm(dev); > > > > > +err_enc_pm: > > > > > + mtk_vcodec_fw_release(dev->fw_handler); > > > > > return ret; > > > > > } > > > > > > > > > > @@ -401,6 +391,7 @@ static int mtk_vcodec_enc_remove(struct platform_device *pdev) > > > > > > > > > > v4l2_device_unregister(&dev->v4l2_dev); > > > > > mtk_vcodec_release_enc_pm(dev); > > > > > + mtk_vcodec_fw_release(dev->fw_handler); > > > > > return 0; > > > > > } > > > > > > > > > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c > > > > > index 3e2bfded79a6..ee22902aaa71 100644 > > > > > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c > > > > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c > > > > > @@ -12,8 +12,6 @@ > > > > > > > > > > #include "mtk_vcodec_enc_pm.h" > > > > > #include "mtk_vcodec_util.h" > > > > > -#include "mtk_vpu.h" > > > > > - > > > > > > > > > > int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev) > > > > > { > > > > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c > > > > > new file mode 100644 > > > > > index 000000000000..967bb100a990 > > > > > --- /dev/null > > > > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c > > > > > @@ -0,0 +1,172 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > + > > > > > +#include "mtk_vcodec_fw.h" > > > > > +#include "mtk_vcodec_util.h" > > > > > +#include "mtk_vcodec_drv.h" > > > > > + > > > > > +struct mtk_vcodec_fw_ops { > > > > > + int (*load_firmware)(struct mtk_vcodec_fw *fw); > > > > > + unsigned int (*get_vdec_capa)(struct mtk_vcodec_fw *fw); > > > > > + unsigned int (*get_venc_capa)(struct mtk_vcodec_fw *fw); > > > > > + void * (*map_dm_addr)(struct mtk_vcodec_fw *fw, u32 dtcm_dmem_addr); > > > > > + int (*ipi_register)(struct mtk_vcodec_fw *fw, int id, > > > > > + mtk_vcodec_ipi_handler handler, const char *name, void *priv); > > > > > + int (*ipi_send)(struct mtk_vcodec_fw *fw, int id, void *buf, > > > > > + unsigned int len, unsigned int wait); > > > > > +}; > > > > > + > > > > > +struct mtk_vcodec_fw { > > > > > + enum mtk_vcodec_fw_type type; > > > > > + const struct mtk_vcodec_fw_ops *ops; > > > > > + struct platform_device *pdev; > > > > > +}; > > > > > + > > > > > +static int mtk_vcodec_vpu_load_firmware(struct mtk_vcodec_fw *fw) > > > > > +{ > > > > > + return vpu_load_firmware(fw->pdev); > > > > > +} > > > > > + > > > > > +static unsigned int mtk_vcodec_vpu_get_vdec_capa(struct mtk_vcodec_fw *fw) > > > > > +{ > > > > > + return vpu_get_vdec_hw_capa(fw->pdev); > > > > > +} > > > > > + > > > > > +static unsigned int mtk_vcodec_vpu_get_venc_capa(struct mtk_vcodec_fw *fw) > > > > > +{ > > > > > + return vpu_get_venc_hw_capa(fw->pdev); > > > > > +} > > > > > + > > > > > +static void *mtk_vcodec_vpu_map_dm_addr(struct mtk_vcodec_fw *fw, > > > > > + u32 dtcm_dmem_addr) > > > > > +{ > > > > > + return vpu_mapping_dm_addr(fw->pdev, dtcm_dmem_addr); > > > > > +} > > > > > + > > > > > +static int mtk_vcodec_vpu_set_ipi_register(struct mtk_vcodec_fw *fw, int id, > > > > > + mtk_vcodec_ipi_handler handler, const char *name, void *priv) > > > > > +{ > > > > > + /* > > > > > + * The handler we receive takes a void * as its first argument. We > > > > > + * cannot change this because it needs to be passed down to the rproc > > > > > + * subsystem when SCP is used. VPU takes a const argument, which is > > > > > + * more constrained, so the conversion below is safe. > > > > > + */ > > > > > + ipi_handler_t handler_const = (ipi_handler_t)handler; > > > > > + > > > > > + return vpu_ipi_register(fw->pdev, id, handler_const, name, priv); > > > > > +} > > > > > + > > > > > +static int mtk_vcodec_vpu_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf, > > > > > + unsigned int len, unsigned int wait) > > > > > +{ > > > > > + return vpu_ipi_send(fw->pdev, id, buf, len); > > > > > +} > > > > > + > > > > > +static const struct mtk_vcodec_fw_ops mtk_vcodec_vpu_msg = { > > > > > + .load_firmware = mtk_vcodec_vpu_load_firmware, > > > > > + .get_vdec_capa = mtk_vcodec_vpu_get_vdec_capa, > > > > > + .get_venc_capa = mtk_vcodec_vpu_get_venc_capa, > > > > > + .map_dm_addr = mtk_vcodec_vpu_map_dm_addr, > > > > > + .ipi_register = mtk_vcodec_vpu_set_ipi_register, > > > > > + .ipi_send = mtk_vcodec_vpu_ipi_send, > > > > > +}; > > > > > + > > > > > +static void mtk_vcodec_reset_handler(void *priv) > > > > > +{ > > > > > + struct mtk_vcodec_dev *dev = priv; > > > > > + struct mtk_vcodec_ctx *ctx; > > > > > + > > > > > + mtk_v4l2_err("Watchdog timeout!!"); > > > > > + > > > > > + mutex_lock(&dev->dev_mutex); > > > > > + list_for_each_entry(ctx, &dev->ctx_list, list) { > > > > > + ctx->state = MTK_STATE_ABORT; > > > > > + mtk_v4l2_debug(0, "[%d] Change to state MTK_STATE_ABORT", > > > > > + ctx->id); > > > > > + } > > > > > + mutex_unlock(&dev->dev_mutex); > > > > > +} > > > > > + > > > > > +struct mtk_vcodec_fw *mtk_vcodec_fw_select(struct mtk_vcodec_dev *dev, > > > > > + enum mtk_vcodec_fw_type type, > > > > > + enum rst_id rst_id) > > > > > +{ > > > > > + const struct mtk_vcodec_fw_ops *ops; > > > > > + struct mtk_vcodec_fw *fw; > > > > > + struct platform_device *fw_pdev = NULL; > > > > > + > > > > > + switch (type) { > > > > > + case VPU: > > > > > + ops = &mtk_vcodec_vpu_msg; > > > > > + fw_pdev = vpu_get_plat_device(dev->plat_dev); > > > > > + if (!fw_pdev) { > > > > > + mtk_v4l2_err("firmware device is not ready"); > > > > > + return ERR_PTR(-EINVAL); > > > > > + } > > > > > + vpu_wdt_reg_handler(fw_pdev, mtk_vcodec_reset_handler, > > > > > + dev, rst_id); > > > > > + break; > > > > > + default: > > > > > + mtk_v4l2_err("invalid vcodec fw type"); > > > > > + return ERR_PTR(-EINVAL); > > > > > + } > > > > > + > > > > > + fw = devm_kzalloc(&dev->plat_dev->dev, sizeof(*fw), GFP_KERNEL); > > > > > + if (!fw) > > > > > + return ERR_PTR(-EINVAL); > > > > > + > > > > > + fw->type = type; > > > > > + fw->ops = ops; > > > > > + fw->pdev = fw_pdev; > > > > > + > > > > > + return fw; > > > > > +} > > > > > +EXPORT_SYMBOL_GPL(mtk_vcodec_fw_select); > > > > > + > > > > > +void mtk_vcodec_fw_release(struct mtk_vcodec_fw *fw) > > > > > +{ > > > > > + switch (fw->type) { > > > > > + case VPU: > > > > > + put_device(&fw->pdev->dev); > > > > > + break; > > > > > + } > > > > > +} > > > > > +EXPORT_SYMBOL_GPL(mtk_vcodec_fw_release); > > > > > + > > > > > > > > What are these symbols exported for? > > > > > > This driver is made of three modules: mtk_vcodec_common, > > > mtk_vcodec_dec and mtk_vcodec_enc. These functions are used by both > > > the encoder and decoder module, so they need to be exported. > > > > > > There is mistake in the patch though that I noticed while > > > investigating your comment: the file defining these functions > > > (mtk_vcodec_fw.c) is linked against the mtk_vcodec_dec module while it > > > should be part of mtk_vcodec_common, so thanks for asking. :) > > > > Oh, I missed that. Out of curiosity, what's the design decision motivating > > this split? > > > > FWIW, I've had a somewhat painful experience with splitted > > modules, so I try to minimize that if possible. > > > > If you decide to keep the split, how about you consider > > also splitting the user option. Something around two visible > > ones: VIDEO_MEDIATEK_VCODEC_ENC/DEC > > and one hidden for the _COMMON one. > > > > That would make the design more obvious, I think. > > > > Minor nitpick, currently the Kconfig help states: > > > > """ > > To compile this driver as a module, choose M here: the > > module will be called mtk-vcodec > > """ > > > > Ah, one more thing. > > In order to build this with COMPILE_TEST, I believe you need > something along these lines: > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > index c4d1731295eb..ee83f407b9a3 100644 > --- a/drivers/remoteproc/Kconfig > +++ b/drivers/remoteproc/Kconfig > @@ -34,7 +34,7 @@ config INGENIC_VPU_RPROC > > config MTK_SCP > tristate "Mediatek SCP support" > - depends on ARCH_MEDIATEK > + depends on ARCH_MEDIATEK || COMPILE_TEST > select RPMSG_MTK_SCP > help > Say y here to support Mediatek's System Companion Processor (SCP) via Sure, but the file you pointed at is for the SCP which is untouched by this patch. The VCODEC entry has COMPILE_TEST enabled. You are right nonetheless, so I'll submit another patch for that. > > Thanks, > Ezequiel