All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandre Courbot <acourbot@chromium.org>
To: Tiffany Lin <tiffany.lin@mediatek.com>,
	Andrew-CT Chen <andrew-ct.chen@mediatek.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Tomasz Figa <tfiga@chromium.org>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mediatek@lists.infradead.org,
	Alexandre Courbot <acourbot@chromium.org>
Subject: [PATCH] media: mtk-vcodec: fix builds when remoteproc is disabled
Date: Sat,  3 Oct 2020 22:09:47 +0900	[thread overview]
Message-ID: <20201003130947.555637-1-acourbot@chromium.org> (raw)

The addition of MT8183 support added a dependency on the SCP remoteproc
module. However the initial patch used the "select" Kconfig directive,
which may result in the SCP module to not be compiled if remoteproc was
disabled. In such a case, mtk-vcodec would try to link against
non-existent SCP symbols. "select" was clearly misused here as explained
in kconfig-language.txt.

Replace this by a "depends" directive on at least one of the VPU and
SCP modules, to allow the driver to be compiled as long as one of these
is enabled, and adapt the code to support this new scenario.

Also adapt the Kconfig text to explain the extra requirements for MT8173
and MT8183.

Reported-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
---
 drivers/media/platform/Kconfig                | 11 +--
 .../media/platform/mtk-vcodec/mtk_vcodec_fw.c | 72 ++++++++++++-------
 2 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index a3cb104956d5..e559d9c529b6 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -253,14 +253,17 @@ config VIDEO_MEDIATEK_VCODEC
 	depends on MTK_IOMMU || COMPILE_TEST
 	depends on VIDEO_DEV && VIDEO_V4L2
 	depends on ARCH_MEDIATEK || COMPILE_TEST
+	depends on VIDEO_MEDIATEK_VPU || MTK_SCP
 	select VIDEOBUF2_DMA_CONTIG
 	select V4L2_MEM2MEM_DEV
-	select VIDEO_MEDIATEK_VPU
-	select MTK_SCP
 	help
 	    Mediatek video codec driver provides HW capability to
-	    encode and decode in a range of video formats
-	    This driver rely on VPU driver to communicate with VPU.
+	    encode and decode in a range of video formats on MT8173
+	    and MT8183.
+
+	    Note that support for support for MT8173 requires
+	    VIDEO_MEDIATEK_VPU to also be selected. Support for
+	    MT8183 depends on MTK_SCP.
 
 	    To compile this driver as modules, choose M here: the
 	    modules will be called mtk-vcodec-dec and mtk-vcodec-enc.
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c
index 6c2a2568d844..23a80027a8fb 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c
@@ -13,6 +13,7 @@ struct mtk_vcodec_fw_ops {
 			    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);
+	void (*release)(struct mtk_vcodec_fw *fw);
 };
 
 struct mtk_vcodec_fw {
@@ -22,6 +23,8 @@ struct mtk_vcodec_fw {
 	struct mtk_scp *scp;
 };
 
+#if IS_ENABLED(CONFIG_VIDEO_MEDIATEK_VPU)
+
 static int mtk_vcodec_vpu_load_firmware(struct mtk_vcodec_fw *fw)
 {
 	return vpu_load_firmware(fw->pdev);
@@ -64,6 +67,27 @@ static int mtk_vcodec_vpu_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
 	return vpu_ipi_send(fw->pdev, id, buf, len);
 }
 
+static void mtk_vcodec_vpu_release(struct mtk_vcodec_fw *fw)
+{
+	put_device(&fw->pdev->dev);
+}
+
+static void mtk_vcodec_vpu_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);
+}
+
 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,
@@ -71,8 +95,13 @@ static const struct mtk_vcodec_fw_ops mtk_vcodec_vpu_msg = {
 	.map_dm_addr = mtk_vcodec_vpu_map_dm_addr,
 	.ipi_register = mtk_vcodec_vpu_set_ipi_register,
 	.ipi_send = mtk_vcodec_vpu_ipi_send,
+	.release = mtk_vcodec_vpu_release,
 };
 
+#endif  /* IS_ENABLED(CONFIG_VIDEO_MEDIATEK_VPU) */
+
+#if IS_ENABLED(CONFIG_MTK_SCP)
+
 static int mtk_vcodec_scp_load_firmware(struct mtk_vcodec_fw *fw)
 {
 	return rproc_boot(scp_get_rproc(fw->scp));
@@ -107,6 +136,11 @@ static int mtk_vcodec_scp_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
 	return scp_ipi_send(fw->scp, id, buf, len, wait);
 }
 
+static void mtk_vcodec_scp_release(struct mtk_vcodec_fw *fw)
+{
+	scp_put(fw->scp);
+}
+
 static const struct mtk_vcodec_fw_ops mtk_vcodec_rproc_msg = {
 	.load_firmware = mtk_vcodec_scp_load_firmware,
 	.get_vdec_capa = mtk_vcodec_scp_get_vdec_capa,
@@ -114,23 +148,10 @@ static const struct mtk_vcodec_fw_ops mtk_vcodec_rproc_msg = {
 	.map_dm_addr = mtk_vcodec_vpu_scp_dm_addr,
 	.ipi_register = mtk_vcodec_scp_set_ipi_register,
 	.ipi_send = mtk_vcodec_scp_ipi_send,
+	.release = mtk_vcodec_scp_release,
 };
 
-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);
-}
+#endif  /* IS_ENABLED(CONFIG_MTK_SCP) */
 
 struct mtk_vcodec_fw *mtk_vcodec_fw_select(struct mtk_vcodec_dev *dev,
 					   enum mtk_vcodec_fw_type type,
@@ -143,16 +164,22 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_select(struct mtk_vcodec_dev *dev,
 
 	switch (type) {
 	case VPU:
+#if IS_ENABLED(CONFIG_VIDEO_MEDIATEK_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,
+		vpu_wdt_reg_handler(fw_pdev, mtk_vcodec_vpu_reset_handler,
 				    dev, rst_id);
 		break;
+#else
+		mtk_v4l2_err("no VPU support in this kernel");
+		return ERR_PTR(-ENODEV);
+#endif
 	case SCP:
+#if IS_ENABLED(CONFIG_MTK_SCP)
 		ops = &mtk_vcodec_rproc_msg;
 		scp = scp_get(dev->plat_dev);
 		if (!scp) {
@@ -160,6 +187,10 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_select(struct mtk_vcodec_dev *dev,
 			return ERR_PTR(-EPROBE_DEFER);
 		}
 		break;
+#else
+		mtk_v4l2_err("no SCP support in this kernel");
+		return ERR_PTR(-ENODEV);
+#endif
 	default:
 		mtk_v4l2_err("invalid vcodec fw type");
 		return ERR_PTR(-EINVAL);
@@ -180,14 +211,7 @@ 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;
-	case SCP:
-		scp_put(fw->scp);
-		break;
-	}
+	fw->ops->release(fw);
 }
 EXPORT_SYMBOL_GPL(mtk_vcodec_fw_release);
 
-- 
2.28.0.806.g8561365e88-goog


WARNING: multiple messages have this Message-ID (diff)
From: Alexandre Courbot <acourbot@chromium.org>
To: Tiffany Lin <tiffany.lin@mediatek.com>,
	Andrew-CT Chen <andrew-ct.chen@mediatek.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Tomasz Figa <tfiga@chromium.org>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	Alexandre Courbot <acourbot@chromium.org>,
	linux-media@vger.kernel.org
Subject: [PATCH] media: mtk-vcodec: fix builds when remoteproc is disabled
Date: Sat,  3 Oct 2020 22:09:47 +0900	[thread overview]
Message-ID: <20201003130947.555637-1-acourbot@chromium.org> (raw)

The addition of MT8183 support added a dependency on the SCP remoteproc
module. However the initial patch used the "select" Kconfig directive,
which may result in the SCP module to not be compiled if remoteproc was
disabled. In such a case, mtk-vcodec would try to link against
non-existent SCP symbols. "select" was clearly misused here as explained
in kconfig-language.txt.

Replace this by a "depends" directive on at least one of the VPU and
SCP modules, to allow the driver to be compiled as long as one of these
is enabled, and adapt the code to support this new scenario.

Also adapt the Kconfig text to explain the extra requirements for MT8173
and MT8183.

Reported-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
---
 drivers/media/platform/Kconfig                | 11 +--
 .../media/platform/mtk-vcodec/mtk_vcodec_fw.c | 72 ++++++++++++-------
 2 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index a3cb104956d5..e559d9c529b6 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -253,14 +253,17 @@ config VIDEO_MEDIATEK_VCODEC
 	depends on MTK_IOMMU || COMPILE_TEST
 	depends on VIDEO_DEV && VIDEO_V4L2
 	depends on ARCH_MEDIATEK || COMPILE_TEST
+	depends on VIDEO_MEDIATEK_VPU || MTK_SCP
 	select VIDEOBUF2_DMA_CONTIG
 	select V4L2_MEM2MEM_DEV
-	select VIDEO_MEDIATEK_VPU
-	select MTK_SCP
 	help
 	    Mediatek video codec driver provides HW capability to
-	    encode and decode in a range of video formats
-	    This driver rely on VPU driver to communicate with VPU.
+	    encode and decode in a range of video formats on MT8173
+	    and MT8183.
+
+	    Note that support for support for MT8173 requires
+	    VIDEO_MEDIATEK_VPU to also be selected. Support for
+	    MT8183 depends on MTK_SCP.
 
 	    To compile this driver as modules, choose M here: the
 	    modules will be called mtk-vcodec-dec and mtk-vcodec-enc.
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c
index 6c2a2568d844..23a80027a8fb 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_fw.c
@@ -13,6 +13,7 @@ struct mtk_vcodec_fw_ops {
 			    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);
+	void (*release)(struct mtk_vcodec_fw *fw);
 };
 
 struct mtk_vcodec_fw {
@@ -22,6 +23,8 @@ struct mtk_vcodec_fw {
 	struct mtk_scp *scp;
 };
 
+#if IS_ENABLED(CONFIG_VIDEO_MEDIATEK_VPU)
+
 static int mtk_vcodec_vpu_load_firmware(struct mtk_vcodec_fw *fw)
 {
 	return vpu_load_firmware(fw->pdev);
@@ -64,6 +67,27 @@ static int mtk_vcodec_vpu_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
 	return vpu_ipi_send(fw->pdev, id, buf, len);
 }
 
+static void mtk_vcodec_vpu_release(struct mtk_vcodec_fw *fw)
+{
+	put_device(&fw->pdev->dev);
+}
+
+static void mtk_vcodec_vpu_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);
+}
+
 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,
@@ -71,8 +95,13 @@ static const struct mtk_vcodec_fw_ops mtk_vcodec_vpu_msg = {
 	.map_dm_addr = mtk_vcodec_vpu_map_dm_addr,
 	.ipi_register = mtk_vcodec_vpu_set_ipi_register,
 	.ipi_send = mtk_vcodec_vpu_ipi_send,
+	.release = mtk_vcodec_vpu_release,
 };
 
+#endif  /* IS_ENABLED(CONFIG_VIDEO_MEDIATEK_VPU) */
+
+#if IS_ENABLED(CONFIG_MTK_SCP)
+
 static int mtk_vcodec_scp_load_firmware(struct mtk_vcodec_fw *fw)
 {
 	return rproc_boot(scp_get_rproc(fw->scp));
@@ -107,6 +136,11 @@ static int mtk_vcodec_scp_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
 	return scp_ipi_send(fw->scp, id, buf, len, wait);
 }
 
+static void mtk_vcodec_scp_release(struct mtk_vcodec_fw *fw)
+{
+	scp_put(fw->scp);
+}
+
 static const struct mtk_vcodec_fw_ops mtk_vcodec_rproc_msg = {
 	.load_firmware = mtk_vcodec_scp_load_firmware,
 	.get_vdec_capa = mtk_vcodec_scp_get_vdec_capa,
@@ -114,23 +148,10 @@ static const struct mtk_vcodec_fw_ops mtk_vcodec_rproc_msg = {
 	.map_dm_addr = mtk_vcodec_vpu_scp_dm_addr,
 	.ipi_register = mtk_vcodec_scp_set_ipi_register,
 	.ipi_send = mtk_vcodec_scp_ipi_send,
+	.release = mtk_vcodec_scp_release,
 };
 
-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);
-}
+#endif  /* IS_ENABLED(CONFIG_MTK_SCP) */
 
 struct mtk_vcodec_fw *mtk_vcodec_fw_select(struct mtk_vcodec_dev *dev,
 					   enum mtk_vcodec_fw_type type,
@@ -143,16 +164,22 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_select(struct mtk_vcodec_dev *dev,
 
 	switch (type) {
 	case VPU:
+#if IS_ENABLED(CONFIG_VIDEO_MEDIATEK_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,
+		vpu_wdt_reg_handler(fw_pdev, mtk_vcodec_vpu_reset_handler,
 				    dev, rst_id);
 		break;
+#else
+		mtk_v4l2_err("no VPU support in this kernel");
+		return ERR_PTR(-ENODEV);
+#endif
 	case SCP:
+#if IS_ENABLED(CONFIG_MTK_SCP)
 		ops = &mtk_vcodec_rproc_msg;
 		scp = scp_get(dev->plat_dev);
 		if (!scp) {
@@ -160,6 +187,10 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_select(struct mtk_vcodec_dev *dev,
 			return ERR_PTR(-EPROBE_DEFER);
 		}
 		break;
+#else
+		mtk_v4l2_err("no SCP support in this kernel");
+		return ERR_PTR(-ENODEV);
+#endif
 	default:
 		mtk_v4l2_err("invalid vcodec fw type");
 		return ERR_PTR(-EINVAL);
@@ -180,14 +211,7 @@ 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;
-	case SCP:
-		scp_put(fw->scp);
-		break;
-	}
+	fw->ops->release(fw);
 }
 EXPORT_SYMBOL_GPL(mtk_vcodec_fw_release);
 
-- 
2.28.0.806.g8561365e88-goog


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

             reply	other threads:[~2020-10-03 13:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-03 13:09 Alexandre Courbot [this message]
2020-10-03 13:09 ` [PATCH] media: mtk-vcodec: fix builds when remoteproc is disabled Alexandre Courbot
2020-10-03 16:50 ` Randy Dunlap
2020-10-03 16:50   ` Randy Dunlap
2020-10-04 12:18   ` Alexandre Courbot
2020-10-04 12:18     ` Alexandre Courbot

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=20201003130947.555637-1-acourbot@chromium.org \
    --to=acourbot@chromium.org \
    --cc=andrew-ct.chen@mediatek.com \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=tfiga@chromium.org \
    --cc=tiffany.lin@mediatek.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.