linux-kernel - Re: [PATCH v5, 12/32] drm/mediatek: add mmsys private data for ddp path config

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 30 Aug 2019 13:40:27 +0800
From:   CK Hu <ck.hu@...iatek.com>
To:     <yongqiang.niu@...iatek.com>
CC:     Philipp Zabel <p.zabel@...gutronix.de>,
        Rob Herring <robh+dt@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        "David Airlie" <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
        Mark Rutland <mark.rutland@....com>,
        <dri-devel@...ts.freedesktop.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH v5, 12/32] drm/mediatek: add mmsys private data for ddp
 path config

Hi, Yongqiang:

On Fri, 2019-08-30 at 13:27 +0800, CK Hu wrote:
> Hi, Yongqiang:
> 
> On Thu, 2019-08-29 at 22:50 +0800, yongqiang.niu@...iatek.com wrote:
> > From: Yongqiang Niu <yongqiang.niu@...iatek.com>
> > 
> > This patch add mmsys private data for ddp path config
> > all these register offset and value will be different in future SOC
> > add these define into mmsys private data
> > 	u32 ovl0_mout_en;
> > 	u32 rdma1_sout_sel_in;
> > 	u32 rdma1_sout_dsi0;
> > 	u32 dpi0_sel_in;
> > 	u32 dpi0_sel_in_rdma1;
> > 	u32 dsi0_sel_in;
> > 	u32 dsi0_sel_in_rdma1;
> > 
> > Signed-off-by: Yongqiang Niu <yongqiang.niu@...iatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c |  4 ++
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp.c  | 86 +++++++++++++++++++++++----------
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp.h  |  5 ++
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c  |  3 ++
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.h  |  3 ++
> >  5 files changed, 76 insertions(+), 25 deletions(-)
> > 
> 
> [snip]
> 
> >  
> >  void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
> > +			      const struct mtk_mmsys_reg_data *reg_data,
> >  			      enum mtk_ddp_comp_id cur,
> >  			      enum mtk_ddp_comp_id next)
> >  {
> >  	unsigned int addr, value, reg;
> >  
> > -	value = mtk_ddp_mout_en(cur, next, &addr);
> > +	value = mtk_ddp_mout_en(reg_data, cur, next, &addr);
> >  	if (value) {
> >  		reg = readl_relaxed(config_regs + addr) | value;
> >  		writel_relaxed(reg, config_regs + addr);
> >  	}
> >  
> > -	mtk_ddp_sout_sel(config_regs, cur, next);
> > +	value = mtk_ddp_sout_sel(reg_data, cur, next, &addr);
> > +	if (value)
> > +		writel_relaxed(value, config_regs + addr);
> 
> I think the register could be written inside mtk_ddp_sout_sel(), why do
> you move out of that function?

OK, after review other patch, I understand what you do here.
You want to align the method of mtk_ddp_mout_en(), mtk_ddp_sout_sel(),
and mtk_ddp_sel_in(). so modification in these three function would be
the same style. So I think this alignment should be an independent patch
before this patch.

Regards,
CK

> 
> Regards,
> CK
> 
> >  
> > -	value = mtk_ddp_sel_in(cur, next, &addr);
> > +	value = mtk_ddp_sel_in(reg_data, cur, next, &addr);
> >  	if (value) {
> >  		reg = readl_relaxed(config_regs + addr) | value;
> >  		writel_relaxed(reg, config_regs + addr);
> > @@ -420,18 +455,19 @@ void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
> >  }
> >  
> >  
> >  
>