Hi Chun-Kuang,
Thanks for the review.
On Fri, 2021-08-20 at 07:14 +0800, Chun-Kuang Hu wrote:
> Hi, Jason:
>
> jason-jh.lin <jason-jh.lin@xxxxxxxxxxxx> 於 2021年8月19日 週四 上午10:24寫道：
> >
> > Adjust to the alphabetic order for the define, function, struct
> > and array in mediatek-drm driver
> >
> > Signed-off-by: jason-jh.lin <jason-jh.lin@xxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 133 ++++++++++----
> > ------
> > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 22 ++--
> > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 76 +++++------
> > 3 files changed, 115 insertions(+), 116 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > index 93beb980414f..28bc42fd0b8a 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > @@ -20,17 +20,9 @@
> > #include "mtk_drm_ddp_comp.h"
> > #include "mtk_drm_crtc.h"
> >
> > -#define DISP_REG_OD_EN 0x0000
> > -#define DISP_REG_OD_CFG 0x0020
> > -#define DISP_REG_OD_SIZE 0x0030
> > -#define DISP_REG_DITHER_5 0x0114
> > -#define DISP_REG_DITHER_7 0x011c
> > -#define DISP_REG_DITHER_15 0x013c
> > -#define DISP_REG_DITHER_16 0x0140
> > -
> > -#define DISP_REG_UFO_START 0x0000
> >
> > #define DISP_REG_AAL_EN 0x0000
> > +#define AAL_EN BIT(0)
> > #define DISP_REG_AAL_SIZE 0x0030
> >
> > #define DISP_REG_DITHER_EN 0x0000
> > @@ -38,23 +30,29 @@
> > #define DISP_REG_DITHER_CFG 0x0020
> > #define DITHER_RELAY_MODE BIT(0)
> > #define DITHER_ENGINE_EN BIT(1)
> > -#define DISP_REG_DITHER_SIZE 0x0030
> > -
> > -#define OD_RELAYMODE BIT(0)
> > -
> > -#define UFO_BYPASS BIT(2)
> > -
> > -#define AAL_EN BIT(0)
> >
> > #define DISP_DITHERING BIT(2)
> > +#define DISP_REG_DITHER_SIZE 0x0030
> > +#define DISP_REG_DITHER_5 0x0114
> > +#define DISP_REG_DITHER_7 0x011c
> > +#define DISP_REG_DITHER_15 0x013c
> > #define DITHER_LSB_ERR_SHIFT_R(x) (((x) & 0x7) << 28)
> > #define DITHER_ADD_LSHIFT_R(x) (((x) & 0x7) << 20)
> > #define DITHER_NEW_BIT_MODE BIT(0)
> > +#define DISP_REG_DITHER_16 0x0140
> > #define DITHER_LSB_ERR_SHIFT_B(x) (((x) & 0x7) << 28)
> > #define DITHER_ADD_LSHIFT_B(x) (((x) & 0x7) << 20)
> > #define DITHER_LSB_ERR_SHIFT_G(x) (((x) & 0x7) << 12)
> > #define DITHER_ADD_LSHIFT_G(x) (((x) & 0x7) << 4)
> >
> > +#define DISP_REG_OD_EN 0x0000
> > +#define DISP_REG_OD_CFG 0x0020
> > +#define OD_RELAYMODE BIT(0)
> > +#define DISP_REG_OD_SIZE 0x0030
> > +
> > +#define DISP_REG_UFO_START 0x0000
> > +#define UFO_BYPASS BIT(2)
> > +
> > struct mtk_ddp_comp_dev {
> > struct clk *clk;
> > void __iomem *regs;
> > @@ -106,20 +104,6 @@ void mtk_ddp_write_mask(struct cmdq_pkt
> > *cmdq_pkt, unsigned int value,
> > #endif
> > }
> >
> > -static int mtk_ddp_clk_enable(struct device *dev)
> > -{
> > - struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> > -
> > - return clk_prepare_enable(priv->clk);
> > -}
> > -
> > -static void mtk_ddp_clk_disable(struct device *dev)
> > -{
> > - struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> > -
> > - clk_disable_unprepare(priv->clk);
> > -}
> > -
>
> I would like to place the same group together. mtk_ddp_clk_enable()
> and mtk_ddp_clk_disable() are common function not belong to any sub
> driver. So I would like keep these function here.
>
> Regards,
> Chun-Kuang.
OK, I'll kepp them here.
Regards,
Jason-JH.Lin
>
> > void mtk_dither_set_common(void __iomem *regs, struct
> > cmdq_client_reg *cmdq_reg,
> > unsigned int bpc, unsigned int cfg,
> > unsigned int dither_en, struct cmdq_pkt
> > *cmdq_pkt)
> > @@ -146,38 +130,19 @@ void mtk_dither_set_common(void __iomem
> > *regs, struct cmdq_client_reg *cmdq_reg,
> > }
> > }
> >
> > -static void mtk_dither_set(struct device *dev, unsigned int bpc,
> > - unsigned int cfg, struct cmdq_pkt *cmdq_pkt)
> > -{
> > - struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> > -
> > - mtk_dither_set_common(priv->regs, &priv->cmdq_reg, bpc,
> > cfg,
> > - DISP_DITHERING, cmdq_pkt);
> > -}
> > -
> > -static void mtk_od_config(struct device *dev, unsigned int w,
> > - unsigned int h, unsigned int vrefresh,
> > - unsigned int bpc, struct cmdq_pkt
> > *cmdq_pkt)
> > -{
> > - struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> > -
> > - mtk_ddp_write(cmdq_pkt, w << 16 | h, &priv->cmdq_reg, priv-
> > >regs, DISP_REG_OD_SIZE);
> > - mtk_ddp_write(cmdq_pkt, OD_RELAYMODE, &priv->cmdq_reg,
> > priv->regs, DISP_REG_OD_CFG);
> > - mtk_dither_set(dev, bpc, DISP_REG_OD_CFG, cmdq_pkt);
> > -}
> >
> > -static void mtk_od_start(struct device *dev)
> > +static int mtk_ddp_clk_enable(struct device *dev)
> > {
> > struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> >
> > - writel(1, priv->regs + DISP_REG_OD_EN);
> > + return clk_prepare_enable(priv->clk);
> > }
> >
> > -static void mtk_ufoe_start(struct device *dev)
> > +static void mtk_ddp_clk_disable(struct device *dev)
> > {
> > struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> >
> > - writel(UFO_BYPASS, priv->regs + DISP_REG_UFO_START);
> > + clk_disable_unprepare(priv->clk);
> > }
> >
> > static void mtk_aal_config(struct device *dev, unsigned int w,
> > @@ -237,6 +202,40 @@ static void mtk_dither_stop(struct device
> > *dev)
> > writel_relaxed(0x0, priv->regs + DISP_REG_DITHER_EN);
> > }
> >
> > +static void mtk_dither_set(struct device *dev, unsigned int bpc,
> > + unsigned int cfg, struct cmdq_pkt
> > *cmdq_pkt)
> > +{
> > + struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> > +
> > + mtk_dither_set_common(priv->regs, &priv->cmdq_reg, bpc,
> > cfg,
> > + DISP_DITHERING, cmdq_pkt);
> > +}
> > +
> > +static void mtk_od_config(struct device *dev, unsigned int w,
> > + unsigned int h, unsigned int vrefresh,
> > + unsigned int bpc, struct cmdq_pkt
> > *cmdq_pkt)
> > +{
> > + struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> > +
> > + mtk_ddp_write(cmdq_pkt, w << 16 | h, &priv->cmdq_reg, priv-
> > >regs, DISP_REG_OD_SIZE);
> > + mtk_ddp_write(cmdq_pkt, OD_RELAYMODE, &priv->cmdq_reg,
> > priv->regs, DISP_REG_OD_CFG);
> > + mtk_dither_set(dev, bpc, DISP_REG_OD_CFG, cmdq_pkt);
> > +}
> > +
> > +static void mtk_od_start(struct device *dev)
> > +{
> > + struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> > +
> > + writel(1, priv->regs + DISP_REG_OD_EN);
> > +}
> > +
> > +static void mtk_ufoe_start(struct device *dev)
> > +{
> > + struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> > +
> > + writel(UFO_BYPASS, priv->regs + DISP_REG_UFO_START);
> > +}
> > +
> > static const struct mtk_ddp_comp_funcs ddp_aal = {
> > .clk_enable = mtk_ddp_clk_enable,
> > .clk_disable = mtk_ddp_clk_disable,
> > @@ -331,22 +330,22 @@ static const struct mtk_ddp_comp_funcs
> > ddp_ufoe = {
> > };
> >
> > static const char * const mtk_ddp_comp_stem[MTK_DDP_COMP_TYPE_MAX]
> > = {
> > + [MTK_DISP_AAL] = "aal",
> > + [MTK_DISP_BLS] = "bls",
> > + [MTK_DISP_CCORR] = "ccorr",
> > + [MTK_DISP_COLOR] = "color",
> > + [MTK_DISP_DITHER] = "dither",
> > + [MTK_DISP_GAMMA] = "gamma",
> > + [MTK_DISP_MUTEX] = "mutex",
> > + [MTK_DISP_OD] = "od",
> > [MTK_DISP_OVL] = "ovl",
> > [MTK_DISP_OVL_2L] = "ovl-2l",
> > + [MTK_DISP_PWM] = "pwm",
> > [MTK_DISP_RDMA] = "rdma",
> > - [MTK_DISP_WDMA] = "wdma",
> > - [MTK_DISP_COLOR] = "color",
> > - [MTK_DISP_CCORR] = "ccorr",
> > - [MTK_DISP_AAL] = "aal",
> > - [MTK_DISP_GAMMA] = "gamma",
> > - [MTK_DISP_DITHER] = "dither",
> > [MTK_DISP_UFOE] = "ufoe",
> > - [MTK_DSI] = "dsi",
> > + [MTK_DISP_WDMA] = "wdma",
> > [MTK_DPI] = "dpi",
> > - [MTK_DISP_PWM] = "pwm",
> > - [MTK_DISP_MUTEX] = "mutex",
> > - [MTK_DISP_OD] = "od",
> > - [MTK_DISP_BLS] = "bls",
> > + [MTK_DSI] = "dsi",
> > };
> >
> > struct mtk_ddp_comp_match {
> > @@ -500,12 +499,12 @@ int mtk_ddp_comp_init(struct device_node
> > *node, struct mtk_ddp_comp *comp,
> > type == MTK_DISP_CCORR ||
> > type == MTK_DISP_COLOR ||
> > type == MTK_DISP_GAMMA ||
> > - type == MTK_DPI ||
> > - type == MTK_DSI ||
> > type == MTK_DISP_OVL ||
> > type == MTK_DISP_OVL_2L ||
> > type == MTK_DISP_PWM ||
> > - type == MTK_DISP_RDMA)
> > + type == MTK_DISP_RDMA ||
> > + type == MTK_DPI ||
> > + type == MTK_DSI)
> > return 0;
> >
> > priv = devm_kzalloc(comp->dev, sizeof(*priv), GFP_KERNEL);
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > index bb914d976cf5..d317b944df66 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > @@ -18,22 +18,22 @@ struct mtk_plane_state;
> > struct drm_crtc_state;
> >
> > enum mtk_ddp_comp_type {
> > - MTK_DISP_OVL,
> > - MTK_DISP_OVL_2L,
> > - MTK_DISP_RDMA,
> > - MTK_DISP_WDMA,
> > - MTK_DISP_COLOR,
> > + MTK_DISP_AAL,
> > + MTK_DISP_BLS,
> > MTK_DISP_CCORR,
> > + MTK_DISP_COLOR,
> > MTK_DISP_DITHER,
> > - MTK_DISP_AAL,
> > MTK_DISP_GAMMA,
> > - MTK_DISP_UFOE,
> > - MTK_DSI,
> > - MTK_DPI,
> > - MTK_DISP_PWM,
> > MTK_DISP_MUTEX,
> > MTK_DISP_OD,
> > - MTK_DISP_BLS,
> > + MTK_DISP_OVL,
> > + MTK_DISP_OVL_2L,
> > + MTK_DISP_PWM,
> > + MTK_DISP_RDMA,
> > + MTK_DISP_UFOE,
> > + MTK_DISP_WDMA,
> > + MTK_DPI,
> > + MTK_DSI,
> > MTK_DDP_COMP_TYPE_MAX,
> > };
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > index b46bdb8985da..a95dc1006b82 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> > @@ -396,50 +396,20 @@ static const struct component_master_ops
> > mtk_drm_ops = {
> > };
> >
> > static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
> > - { .compatible = "mediatek,mt2701-disp-ovl",
> > - .data = (void *)MTK_DISP_OVL },
> > - { .compatible = "mediatek,mt8173-disp-ovl",
> > - .data = (void *)MTK_DISP_OVL },
> > - { .compatible = "mediatek,mt8183-disp-ovl",
> > - .data = (void *)MTK_DISP_OVL },
> > - { .compatible = "mediatek,mt8183-disp-ovl-2l",
> > - .data = (void *)MTK_DISP_OVL_2L },
> > - { .compatible = "mediatek,mt2701-disp-rdma",
> > - .data = (void *)MTK_DISP_RDMA },
> > - { .compatible = "mediatek,mt8173-disp-rdma",
> > - .data = (void *)MTK_DISP_RDMA },
> > - { .compatible = "mediatek,mt8183-disp-rdma",
> > - .data = (void *)MTK_DISP_RDMA },
> > - { .compatible = "mediatek,mt8173-disp-wdma",
> > - .data = (void *)MTK_DISP_WDMA },
> > + { .compatible = "mediatek,mt8173-disp-aal",
> > + .data = (void *)MTK_DISP_AAL},
> > { .compatible = "mediatek,mt8183-disp-ccorr",
> > .data = (void *)MTK_DISP_CCORR },
> > { .compatible = "mediatek,mt2701-disp-color",
> > .data = (void *)MTK_DISP_COLOR },
> > { .compatible = "mediatek,mt8173-disp-color",
> > .data = (void *)MTK_DISP_COLOR },
> > - { .compatible = "mediatek,mt8173-disp-aal",
> > - .data = (void *)MTK_DISP_AAL},
> > + { .compatible = "mediatek,mt8183-disp-dither",
> > + .data = (void *)MTK_DISP_DITHER },
> > { .compatible = "mediatek,mt8173-disp-gamma",
> > .data = (void *)MTK_DISP_GAMMA, },
> > { .compatible = "mediatek,mt8183-disp-gamma",
> > .data = (void *)MTK_DISP_GAMMA, },
> > - { .compatible = "mediatek,mt8183-disp-dither",
> > - .data = (void *)MTK_DISP_DITHER },
> > - { .compatible = "mediatek,mt8173-disp-ufoe",
> > - .data = (void *)MTK_DISP_UFOE },
> > - { .compatible = "mediatek,mt2701-dsi",
> > - .data = (void *)MTK_DSI },
> > - { .compatible = "mediatek,mt8173-dsi",
> > - .data = (void *)MTK_DSI },
> > - { .compatible = "mediatek,mt8183-dsi",
> > - .data = (void *)MTK_DSI },
> > - { .compatible = "mediatek,mt2701-dpi",
> > - .data = (void *)MTK_DPI },
> > - { .compatible = "mediatek,mt8173-dpi",
> > - .data = (void *)MTK_DPI },
> > - { .compatible = "mediatek,mt8183-dpi",
> > - .data = (void *)MTK_DPI },
> > { .compatible = "mediatek,mt2701-disp-mutex",
> > .data = (void *)MTK_DISP_MUTEX },
> > { .compatible = "mediatek,mt2712-disp-mutex",
> > @@ -448,12 +418,42 @@ static const struct of_device_id
> > mtk_ddp_comp_dt_ids[] = {
> > .data = (void *)MTK_DISP_MUTEX },
> > { .compatible = "mediatek,mt8183-disp-mutex",
> > .data = (void *)MTK_DISP_MUTEX },
> > + { .compatible = "mediatek,mt8173-disp-od",
> > + .data = (void *)MTK_DISP_OD },
> > + { .compatible = "mediatek,mt2701-disp-ovl",
> > + .data = (void *)MTK_DISP_OVL },
> > + { .compatible = "mediatek,mt8173-disp-ovl",
> > + .data = (void *)MTK_DISP_OVL },
> > + { .compatible = "mediatek,mt8183-disp-ovl",
> > + .data = (void *)MTK_DISP_OVL },
> > + { .compatible = "mediatek,mt8183-disp-ovl-2l",
> > + .data = (void *)MTK_DISP_OVL_2L },
> > { .compatible = "mediatek,mt2701-disp-pwm",
> > .data = (void *)MTK_DISP_BLS },
> > { .compatible = "mediatek,mt8173-disp-pwm",
> > .data = (void *)MTK_DISP_PWM },
> > - { .compatible = "mediatek,mt8173-disp-od",
> > - .data = (void *)MTK_DISP_OD },
> > + { .compatible = "mediatek,mt2701-disp-rdma",
> > + .data = (void *)MTK_DISP_RDMA },
> > + { .compatible = "mediatek,mt8173-disp-rdma",
> > + .data = (void *)MTK_DISP_RDMA },
> > + { .compatible = "mediatek,mt8183-disp-rdma",
> > + .data = (void *)MTK_DISP_RDMA },
> > + { .compatible = "mediatek,mt8173-disp-ufoe",
> > + .data = (void *)MTK_DISP_UFOE },
> > + { .compatible = "mediatek,mt8173-disp-wdma",
> > + .data = (void *)MTK_DISP_WDMA },
> > + { .compatible = "mediatek,mt2701-dpi",
> > + .data = (void *)MTK_DPI },
> > + { .compatible = "mediatek,mt8173-dpi",
> > + .data = (void *)MTK_DPI },
> > + { .compatible = "mediatek,mt8183-dpi",
> > + .data = (void *)MTK_DPI },
> > + { .compatible = "mediatek,mt2701-dsi",
> > + .data = (void *)MTK_DSI },
> > + { .compatible = "mediatek,mt8173-dsi",
> > + .data = (void *)MTK_DSI },
> > + { .compatible = "mediatek,mt8183-dsi",
> > + .data = (void *)MTK_DSI },
> > { }
> > };
> >
> > @@ -542,8 +542,8 @@ static int mtk_drm_probe(struct platform_device
> > *pdev)
> > comp_type == MTK_DISP_OVL ||
> > comp_type == MTK_DISP_OVL_2L ||
> > comp_type == MTK_DISP_RDMA ||
> > - comp_type == MTK_DSI ||
> > - comp_type == MTK_DPI) {
> > + comp_type == MTK_DPI ||
> > + comp_type == MTK_DSI) {
> > dev_info(dev, "Adding component match for
> > %pOF\n",
> > node);
> > drm_of_component_match_add(dev, &match,
> > compare_of,
> > --
> > 2.18.0
> >
--
Jason-JH Lin <jason-jh.lin@xxxxxxxxxxxx>
