| 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: <78f6d86ccad40442ff24946e2dabbbbb9862c535.camel@mediatek.com>
Date: Fri, 21 Feb 2025 11:54:05 +0000
From: Paul-pl Chen (陳柏霖) <Paul-pl.Chen@...iatek.com>
To: CK Hu (胡俊光) <ck.hu@...iatek.com>,
"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>, "AngeloGioacchino Del
Regno" <angelogioacchino.delregno@...labora.com>
CC: Sunny Shen (沈姍姍) <Sunny.Shen@...iatek.com>,
Singo Chang (張興國) <Singo.Chang@...iatek.com>,
"treapking@...omium.org" <treapking@...omium.org>,
Project_Global_Chrome_Upstream_Group
<Project_Global_Chrome_Upstream_Group@...iatek.com>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
Jason-JH Lin (林睿祥) <Jason-JH.Lin@...iatek.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Nancy Lin (林欣螢) <Nancy.Lin@...iatek.com>,
Xiandong Wang (王先冬)
<Xiandong.Wang@...iatek.com>, "matthias.bgg@...il.com"
<matthias.bgg@...il.com>, "linux-mediatek@...ts.infradead.org"
<linux-mediatek@...ts.infradead.org>, "p.zabel@...gutronix.de"
<p.zabel@...gutronix.de>, Sirius Wang (王皓昱)
<Sirius.Wang@...iatek.com>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 10/12] drm/mediatek: add OUTPROC support for MT8196
On Wed, 2025-02-12 at 04:08 +0000, CK Hu (胡俊光) wrote:
> Hi, Paul:
>
> On Fri, 2025-01-10 at 20:34 +0800, paul-pl.chen wrote:
> > From: "Nancy.Lin" <nancy.lin@...iatek.com>
> >
> > OUTPROC handles the post-stage of pixel processing in
> > the overlapping procedure.OUTPROC manages pixels for
> > gamma correction and ensures that pixel values are
> > within the correct range.
> >
> > Signed-off-by: Nancy.Lin <nancy.lin@...iatek.com>
> > Signed-off-by: Paul-pl.Chen <paul-pl.chen@...iatek.com>
> > ---
> > drivers/gpu/drm/mediatek/Makefile | 1 +
> > drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 1 +
> > drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 1 +
> > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 14 ++
> > drivers/gpu/drm/mediatek/mtk_disp_outproc.c | 244
> > ++++++++++++++++++++
> > drivers/gpu/drm/mediatek/mtk_disp_outproc.h | 22 ++
> > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
> > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 +
> > 8 files changed, 285 insertions(+)
> > create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_outproc.c
> > create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_outproc.h
> >
> >
//snip
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_outproc.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_outproc.c
> > new file mode 100644
> > index 000000000000..af473eb2fb77
> > --- /dev/null
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_outproc.c
> > @@ -0,0 +1,244 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2021 MediaTek Inc.
> > + */
> > +
> > +#include <drm/drm_fourcc.h>
> > +#include <drm/drm_framebuffer.h>
> > +#include <linux/clk.h>
> > +#include <linux/component.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_address.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/reset.h>
> > +#include <linux/soc/mediatek/mtk-cmdq.h>
> > +#include <linux/soc/mediatek/mtk-mmsys.h>
> > +
> > +#include "mtk_crtc.h"
> > +#include "mtk_ddp_comp.h"
> > +#include <linux/of.h>
> > +#include "mtk_drm_drv.h"
> > +#include "mtk_disp_outproc.h"
> > +
> > +#define
> > DISP_REG_OVL_OUTPROC_INTEN 0x004
> > +#define
> > OVL_OUTPROC_FME_CPL_INTEN BIT(1)
> > +#define
> > DISP_REG_OVL_OUTPROC_INTSTA 0x008
> > +#define
> > DISP_REG_OVL_OUTPROC_DATAPATH_CON 0x010
> > +#define
> > DATAPATH_CON_OUTPUT_CLAMP BIT(26)
> > +
> > +#define
> > DISP_REG_OVL_OUTPROC_EN 0x020
> > +#define
> > OVL_OUTPROC_OVL_EN BIT(0)
> > +#define
> > DISP_REG_OVL_OUTPROC_RST 0x024
> > +#define
> > OVL_OUTPROC_RST BIT(0)
> > +#define
> > DISP_REG_OVL_OUTPROC_SHADOW_CTRL 0x028
> > +#define
> > OVL_OUTPROC_BYPASS_SHADOW BIT(2)
> > +#define
> > DISP_REG_OVL_OUTPROC_ROI_SIZE 0x030
> > +
> > +struct mtk_disp_outproc {
> > + void __iomem *regs;
> > + struct clk *clk;
> > + void (*vblank_cb)(void *data);
> > + void *vblank_cb_data;
> > + int irq;
> > + struct cmdq_client_reg cmdq_reg;
> > +};
> > +
> > +void mtk_disp_outproc_register_vblank_cb(struct device *dev,
> > + void (*vblank_cb)(void
> > *),
> > + void *vblank_cb_data)
> > +{
> > + struct mtk_disp_outproc *priv = dev_get_drvdata(dev);
> > +
> > + priv->vblank_cb = vblank_cb;
> > + priv->vblank_cb_data = vblank_cb_data;
> > +}
> > +
> > +void mtk_disp_outproc_unregister_vblank_cb(struct device *dev)
> > +{
> > + struct mtk_disp_outproc *priv = dev_get_drvdata(dev);
> > +
> > + priv->vblank_cb = NULL;
> > + priv->vblank_cb_data = NULL;
> > +}
> > +
> > +void mtk_disp_outproc_enable_vblank(struct device *dev)
> > +{
> > + struct mtk_disp_outproc *priv = dev_get_drvdata(dev);
> > +
> > + writel(OVL_OUTPROC_FME_CPL_INTEN, priv->regs +
> > DISP_REG_OVL_OUTPROC_INTEN);
> > +}
> > +
> > +void mtk_disp_outproc_disable_vblank(struct device *dev)
> > +{
> > + struct mtk_disp_outproc *priv = dev_get_drvdata(dev);
> > +
> > + writel(0x0, priv->regs + DISP_REG_OVL_OUTPROC_INTEN);
> > +}
> > +
> > +static irqreturn_t mtk_disp_outproc_irq_handler(int irq, void
> > *dev_id)
> > +{
> > + struct mtk_disp_outproc *priv = dev_id;
> > +
> > + writel(0x0, priv->regs + DISP_REG_OVL_OUTPROC_INTSTA);
> > +
> > + if (!priv->vblank_cb)
> > + return IRQ_NONE;
> > +
> > + priv->vblank_cb(priv->vblank_cb_data);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +void mtk_disp_outproc_config(struct device *dev, unsigned int w,
> > + unsigned int h, unsigned int
> > vrefresh,
> > + unsigned int bpc, struct cmdq_pkt
> > *cmdq_pkt)
> > +{
> > + struct mtk_disp_outproc *priv = dev_get_drvdata(dev);
> > + unsigned int tmp;
> > +
> > + dev_dbg(dev, "%s-w:%d, h:%d\n", __func__, w, h);
> > +
> > + tmp = readl(priv->regs +
> > DISP_REG_OVL_OUTPROC_SHADOW_CTRL);
> > + tmp = tmp | OVL_OUTPROC_BYPASS_SHADOW;
> > + writel(tmp, priv->regs +
> > DISP_REG_OVL_OUTPROC_SHADOW_CTRL);
>
> Move bypass shadow to mtk_disp_outproc_start().
>
Sure, we will move the bypass shadow to mtk_disp_outproc_start().
> > +
> > + mtk_ddp_write_mask(cmdq_pkt, h << 16 | w, &priv->cmdq_reg,
> > priv->regs,
> > + DISP_REG_OVL_OUTPROC_ROI_SIZE, ~0);
>
> mtk_ddp_write();
>
> > + mtk_ddp_write_mask(cmdq_pkt, DATAPATH_CON_OUTPUT_CLAMP,
> > &priv->cmdq_reg, priv->regs,
> > + DISP_REG_OVL_OUTPROC_DATAPATH_CON,
> > DATAPATH_CON_OUTPUT_CLAMP);
> > +}
> > +
> > +void mtk_disp_outproc_start(struct device *dev)
> > +{
> > + struct mtk_disp_outproc *priv = dev_get_drvdata(dev);
> > +
> > + mtk_ddp_write_mask(NULL, OVL_OUTPROC_RST, &priv->cmdq_reg,
> > priv->regs,
> > + DISP_REG_OVL_OUTPROC_RST,
> > OVL_OUTPROC_RST);
>
> writel();
>
Sure, I will use the writel() to replace the mtk_ddp_write_mask().
> > + mtk_ddp_write_mask(NULL, 0, &priv->cmdq_reg, priv->regs,
> > + DISP_REG_OVL_OUTPROC_RST,
> > OVL_OUTPROC_RST);
>
> Some hardware reset before start.
> Some hardware reset after stop.
>
> Could align this?
>
Sure, we will align this part.
> > + mtk_ddp_write(NULL, 0, &priv->cmdq_reg, priv->regs,
> > + DISP_REG_OVL_OUTPROC_INTSTA);
> > + mtk_ddp_write_mask(NULL, OVL_OUTPROC_OVL_EN, &priv-
> > >cmdq_reg, priv->regs,
> > + DISP_REG_OVL_OUTPROC_EN,
> > OVL_OUTPROC_OVL_EN);
> > +}
> > +
> > +void mtk_disp_outproc_stop(struct device *dev)
> > +{
> > + struct mtk_disp_outproc *priv = dev_get_drvdata(dev);
> > +
> > + mtk_ddp_write(NULL, 0, &priv->cmdq_reg, priv->regs,
> > + DISP_REG_OVL_OUTPROC_INTEN);
>
> OVL driver does not disable interrupt when stop.
> Align this with OVL driver.
>
Sure I will check the driver and align this part.
>
//snip
> > +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > + ret = cmdq_dev_get_client_reg(dev, &priv->cmdq_reg, 0);
> > + if (ret)
> > + dev_dbg(dev, "No mediatek,gce-client-reg\n");
> > +#endif
> > +
> > + if (of_property_read_u32_index(dev->of_node, "interrupts",
> > 0, &ret)) {
> > + dev_dbg(dev, "interrupts not defined\n");
>
> In binding document, interrupt is required.
> So return with error code.
>
The interrupt is not required, I have rewrite the binding document.
> > + } else {
> > + priv->irq = platform_get_irq(pdev, 0);
> > + if (priv->irq < 0)
> > + priv->irq = 0;
>
> In binding document, interrupt is required.
> So return with error code.
>
> Regards,
> CK
>
The interrupt is not required, I have rewrite the binding document.
The next upstream version of patch would fix this problem
Best,
Paul
>
Powered by blists - more mailing lists