| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGXv+5HHARvkCYfjPjRKgyWuzv-Dt215z1=yA+_tw4hyasdGQA@mail.gmail.com>
Date: Thu, 8 Jun 2023 16:12:16 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Nícolas F. R. A. Prado
<nfraprado@...labora.com>, Stephen Boyd <sboyd@...nel.org>
Cc: Matthias Brugger <matthias.bgg@...il.com>,
Hans Verkuil <hverkuil-cisco@...all.nl>, kernel@...labora.com,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
Andrew-CT Chen <andrew-ct.chen@...iatek.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Tiffany Lin <tiffany.lin@...iatek.com>,
Yunfei Dong <yunfei.dong@...iatek.com>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v2 3/5] media: mediatek: vcodec: Read HW active status
from clock
On Thu, Jun 8, 2023 at 4:57 AM Nícolas F. R. A. Prado
<nfraprado@...labora.com> wrote:
>
> Remove the requirement of a VDEC_SYS reg iospace. To achieve that, rely
> on the "active" clock being passed through the DT, and read its status
> during IRQ handling to check whether the HW is active.
>
> The old behavior is still present when reg-names aren't supplied, as to
> keep backward compatibility.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
> ---
>
> (no changes since v1)
>
> .../mediatek/vcodec/mtk_vcodec_dec_drv.c | 59 +++++++++++++++----
> .../mediatek/vcodec/mtk_vcodec_dec_hw.c | 20 +++++--
> .../mediatek/vcodec/mtk_vcodec_dec_pm.c | 12 +++-
> .../platform/mediatek/vcodec/mtk_vcodec_drv.h | 1 +
> 4 files changed, 74 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
> index 9c652beb3f19..8038472fb67b 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
> @@ -16,6 +16,7 @@
> #include <media/v4l2-mem2mem.h>
> #include <media/videobuf2-dma-contig.h>
> #include <media/v4l2-device.h>
> +#include <linux/clk-provider.h>
^^^^^^^^^^^^^^
This seems like a violation of the API separation.
> #include "mtk_vcodec_drv.h"
> #include "mtk_vcodec_dec.h"
> @@ -38,22 +39,29 @@ static int mtk_vcodec_get_hw_count(struct mtk_vcodec_dev *dev)
> }
> }
>
> +static bool mtk_vcodec_is_hw_active(struct mtk_vcodec_dev *dev)
> +{
> + u32 cg_status = 0;
> +
> + if (!dev->reg_base[VDEC_SYS])
> + return __clk_is_enabled(dev->pm.vdec_active_clk);
AFAIK this is still around for clk drivers that haven't moved to clk_hw.
It shouldn't be used by clock consumers. Would it be better to just pass
a syscon?
ChenYu
> +
> + cg_status = readl(dev->reg_base[VDEC_SYS]);
> + return (cg_status & VDEC_HW_ACTIVE) == 0;
> +}
> +
> static irqreturn_t mtk_vcodec_dec_irq_handler(int irq, void *priv)
> {
> struct mtk_vcodec_dev *dev = priv;
> struct mtk_vcodec_ctx *ctx;
> - u32 cg_status = 0;
> unsigned int dec_done_status = 0;
> void __iomem *vdec_misc_addr = dev->reg_base[VDEC_MISC] +
> VDEC_IRQ_CFG_REG;
>
> ctx = mtk_vcodec_get_curr_ctx(dev, MTK_VDEC_CORE);
>
> - /* check if HW active or not */
> - cg_status = readl(dev->reg_base[0]);
> - if ((cg_status & VDEC_HW_ACTIVE) != 0) {
> - mtk_v4l2_err("DEC ISR, VDEC active is not 0x0 (0x%08x)",
> - cg_status);
> + if (!mtk_vcodec_is_hw_active(dev)) {
> + mtk_v4l2_err("DEC ISR, VDEC active is not 0x0");
> return IRQ_HANDLED;
> }
>
> @@ -82,6 +90,25 @@ static int mtk_vcodec_get_reg_bases(struct mtk_vcodec_dev *dev)
> {
> struct platform_device *pdev = dev->plat_dev;
> int reg_num, i;
> + struct resource *res;
> + bool no_vdecsys_reg = false;
> + static const char * const mtk_dec_reg_names[] = {
> + "misc",
> + "ld",
> + "top",
> + "cm",
> + "ad",
> + "av",
> + "pp",
> + "hwd",
> + "hwq",
> + "hwb",
> + "hwg"
> + };
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "misc");
> + if (res)
> + no_vdecsys_reg = true;
>
> /* Sizeof(u32) * 4 bytes for each register base. */
> reg_num = of_property_count_elems_of_size(pdev->dev.of_node, "reg",
> @@ -91,12 +118,22 @@ static int mtk_vcodec_get_reg_bases(struct mtk_vcodec_dev *dev)
> return -EINVAL;
> }
>
> - for (i = 0; i < reg_num; i++) {
> - dev->reg_base[i] = devm_platform_ioremap_resource(pdev, i);
> - if (IS_ERR(dev->reg_base[i]))
> - return PTR_ERR(dev->reg_base[i]);
> + if (!no_vdecsys_reg) {
> + for (i = 0; i < reg_num; i++) {
> + dev->reg_base[i] = devm_platform_ioremap_resource(pdev, i);
> + if (IS_ERR(dev->reg_base[i]))
> + return PTR_ERR(dev->reg_base[i]);
>
> - mtk_v4l2_debug(2, "reg[%d] base=%p", i, dev->reg_base[i]);
> + mtk_v4l2_debug(2, "reg[%d] base=%p", i, dev->reg_base[i]);
> + }
> + } else {
> + for (i = 0; i < reg_num; i++) {
> + dev->reg_base[i+1] = devm_platform_ioremap_resource_byname(pdev, mtk_dec_reg_names[i]);
> + if (IS_ERR(dev->reg_base[i+1]))
> + return PTR_ERR(dev->reg_base[i+1]);
> +
> + mtk_v4l2_debug(2, "reg[%d] base=%p", i+1, dev->reg_base[i+1]);
> + }
> }
>
> return 0;
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
> index b753bf54ebd9..4e786821015d 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
> @@ -11,6 +11,7 @@
> #include <linux/of_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/slab.h>
> +#include <linux/clk-provider.h>
>
> #include "mtk_vcodec_drv.h"
> #include "mtk_vcodec_dec.h"
> @@ -63,22 +64,29 @@ static int mtk_vdec_hw_prob_done(struct mtk_vcodec_dev *vdec_dev)
> return 0;
> }
>
> +static bool mtk_vcodec_is_hw_active(struct mtk_vdec_hw_dev *dev)
> +{
> + u32 cg_status;
> +
> + if (!dev->reg_base[VDEC_HW_SYS])
> + return __clk_is_enabled(dev->pm.vdec_active_clk);
> +
> + cg_status = readl(dev->reg_base[VDEC_HW_SYS]);
> + return (cg_status & VDEC_HW_ACTIVE) == 0;
> +}
> +
> static irqreturn_t mtk_vdec_hw_irq_handler(int irq, void *priv)
> {
> struct mtk_vdec_hw_dev *dev = priv;
> struct mtk_vcodec_ctx *ctx;
> - u32 cg_status;
> unsigned int dec_done_status;
> void __iomem *vdec_misc_addr = dev->reg_base[VDEC_HW_MISC] +
> VDEC_IRQ_CFG_REG;
>
> ctx = mtk_vcodec_get_curr_ctx(dev->main_dev, dev->hw_idx);
>
> - /* check if HW active or not */
> - cg_status = readl(dev->reg_base[VDEC_HW_SYS]);
> - if (cg_status & VDEC_HW_ACTIVE) {
> - mtk_v4l2_err("vdec active is not 0x0 (0x%08x)",
> - cg_status);
> + if (!mtk_vcodec_is_hw_active(dev)) {
> + mtk_v4l2_err("vdec active is not 0x0");
> return IRQ_HANDLED;
> }
>
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_pm.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_pm.c
> index 777d445999e9..53e621965950 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_pm.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_pm.c
> @@ -51,6 +51,9 @@ int mtk_vcodec_init_dec_clk(struct platform_device *pdev, struct mtk_vcodec_pm *
> clk_info->clk_name);
> return PTR_ERR(clk_info->vcodec_clk);
> }
> +
> + if (strcmp(clk_info->clk_name, "active") == 0)
> + pm->vdec_active_clk = clk_info->vcodec_clk;
> }
>
> return 0;
> @@ -84,6 +87,9 @@ static void mtk_vcodec_dec_clock_on(struct mtk_vcodec_pm *pm)
>
> dec_clk = &pm->vdec_clk;
> for (i = 0; i < dec_clk->clk_num; i++) {
> + if (strcmp(dec_clk->clk_info[i].clk_name, "active") == 0)
> + continue;
> +
> ret = clk_prepare_enable(dec_clk->clk_info[i].vcodec_clk);
> if (ret) {
> mtk_v4l2_err("clk_prepare_enable %d %s fail %d", i,
> @@ -104,8 +110,12 @@ static void mtk_vcodec_dec_clock_off(struct mtk_vcodec_pm *pm)
> int i;
>
> dec_clk = &pm->vdec_clk;
> - for (i = dec_clk->clk_num - 1; i >= 0; i--)
> + for (i = dec_clk->clk_num - 1; i >= 0; i--) {
> + if (strcmp(dec_clk->clk_info[i].clk_name, "active") == 0)
> + continue;
> +
> clk_disable_unprepare(dec_clk->clk_info[i].vcodec_clk);
> + }
> }
>
> static void mtk_vcodec_dec_enable_irq(struct mtk_vcodec_dev *vdec_dev, int hw_idx)
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> index 9acab54fd650..180e74c69042 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> @@ -208,6 +208,7 @@ struct mtk_vcodec_pm {
> struct mtk_vcodec_clk vdec_clk;
> struct mtk_vcodec_clk venc_clk;
> struct device *dev;
> + struct clk *vdec_active_clk;
> };
>
> /**
> --
> 2.41.0
>
>
Powered by blists - more mailing lists