Hi, Yongqiang: On Wed, 2019-03-27 at 14:19 +0800, yongqiang.niu@xxxxxxxxxxxx wrote: > From: Yongqiang Niu <yongqiang.niu@xxxxxxxxxxxx> > > This patch add ddp write register common api > this is preparation patch for ovl/ovl_2l direct link > usecase. > in that case, we need this funtion to set one bit of ovl_2l > register The reason is so strange. You could grep 'readl' in mediatek drm driver, and you would find that many place need this function. > > Signed-off-by: Yongqiang Niu <yongqiang.niu@xxxxxxxxxxxx> > --- > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 11 +++++++++++ > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 4 ++++ > 2 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > index afd6ca2..72288b4 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > @@ -84,6 +84,17 @@ > #define DITHER_ADD_LSHIFT_G(x) (((x) & 0x7) << 4) > #define DITHER_ADD_RSHIFT_G(x) (((x) & 0x7) << 0) > > +void mtk_ddp_write_mask(unsigned int value, > + struct mtk_ddp_comp *comp, > + unsigned int offset, > + unsigned int mask) > +{ > + unsigned int tmp = readl(comp->regs + offset); > + > + tmp = (tmp & ~mask) | (value & mask); Why not just place this calculation into writel()? Regards, CK > + writel(tmp, comp->regs + offset); > +} > + > void mtk_dither_set(struct mtk_ddp_comp *comp, unsigned int bpc, > unsigned int CFG) > { > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > index 20e8061..ed715ff 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > @@ -198,5 +198,9 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *comp_node, > void mtk_ddp_comp_unregister(struct drm_device *drm, struct mtk_ddp_comp *comp); > void mtk_dither_set(struct mtk_ddp_comp *comp, unsigned int bpc, > unsigned int CFG); > +void mtk_ddp_write_mask(unsigned int value, > + struct mtk_ddp_comp *comp, > + unsigned int offset, > + unsigned int mask); > > #endif /* MTK_DRM_DDP_COMP_H */