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....@mediatek.com wrote:
> > From: Yongqiang Niu <yongqiang....@mediatek.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....@mediatek.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,
> >  }
> >  
> >  
> >  
> 


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to