Re: [PATCH v27 01/11] dt-bindings: arm: mediatek: mmsys: add vdosys1 compatible for MT8195

Re: [PATCH v27 01/11] dt-bindings: arm: mediatek: mmsys: add vdosys1 compatible for MT8195

[Nícolas F. R. A. Prado]

On Thu, Nov 03, 2022 at 11:25:02AM +0800, Nancy.Lin wrote:
> Add vdosys1 mmsys compatible for MT8195 platform.
> 
> For MT8195, VDOSYS0 and VDOSYS1 are 2 display HW pipelines binding to
> 2 different power domains, different clock drivers and different
> mediatek-drm drivers.
> 
> Signed-off-by: Nancy.Lin <nancy.lin@mediatek.com>
> Change-Id: I8065205ce44baa417f58176adaae9e1eefa6eadb

No Change-Id tag, please. There are some other commits with it as well.

Otherwise,

Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Thanks,
Nícolas

Re: [PATCH v27 06/11] soc: mediatek: add mtk-mmsys config API for mt8195 vdosys1

[Nícolas F. R. A. Prado]

On Thu, Nov 03, 2022 at 11:25:07AM +0800, Nancy.Lin wrote:
> Add four mmsys config APIs. The config APIs are used for config
> mmsys reg. Some mmsys regs need to be set according to the
> HW engine binding to the mmsys simultaneously.
> 
> 1. mtk_mmsys_merge_async_config: config merge async width/height.
>    async is used for cross-clock domain synchronization.
> 2. mtk_mmsys_hdr_confing: config hdr backend async width/height.
[..]
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
[..]
> +void mtk_mmsys_hdr_confing(struct device *dev, int be_width, int be_height)

confing -> config

Need to update the following commit as well.

Thanks,
Nícolas

> +{
> +	mtk_mmsys_update_bits(dev_get_drvdata(dev), MT8195_VDO1_HDRBE_ASYNC_CFG_WD, ~0,
> +			      be_height << 16 | be_width);
> +}
> +EXPORT_SYMBOL_GPL(mtk_mmsys_hdr_confing);
[..]
> +void mtk_mmsys_hdr_confing(struct device *dev, int be_width, int be_height);
[..]

Re: [PATCH v27 05/11] soc: mediatek: refine code to use mtk_mmsys_update_bits API

[Nícolas F. R. A. Prado]

On Thu, Nov 03, 2022 at 11:25:06AM +0800, Nancy.Lin wrote:
[..]
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
[..]
> +static void mtk_mmsys_update_bits(struct mtk_mmsys *mmsys, u32 offset, u32 mask, u32 val)
> +{
> +	u32 tmp;
> +
> +	tmp = readl_relaxed(mmsys->regs + offset);
> +	tmp = (tmp & ~mask) | val;

You should mask the value before writing to prevent bad values from overwriting
bits outside the mask.

	tmp = (tmp & ~mask) | (val & mask);

With that,

Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Thanks,
Nícolas

Re: [PATCH v27 08/11] soc: mediatek: mmsys: add mmsys for support 64 reset bits

[Nícolas F. R. A. Prado]

On Thu, Nov 03, 2022 at 11:25:09AM +0800, Nancy.Lin wrote:
> Add mmsys for support 64 reset bits. It is a preparation for MT8195
> vdosys1 HW reset. MT8195 vdosys1 has more than 32 reset bits.
> 
> 1. Add the number of reset bits in mmsys private data
> 2. move the whole "reset register code section" behind the
> "get mmsys->data" code section for getting the num_resets in mmsys->data.
> 
> Signed-off-by: Nancy.Lin <nancy.lin@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> Tested-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
> Change-Id: Ic7c30ad6c4f74a6b2ebf75fc0e3f9f207b0416d1

Again, no Change-Id please.

Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Thanks,
Nícolas

Re: [PATCH v27 01/11] dt-bindings: arm: mediatek: mmsys: add vdosys1 compatible for MT8195

[Nancy Lin (林欣螢)]

Dear Nicolas,

Thanks for the review.

On Thu, 2022-11-03 at 15:39 -0400, Nícolas F. R. A. Prado wrote:
> On Thu, Nov 03, 2022 at 11:25:02AM +0800, Nancy.Lin wrote:
> > Add vdosys1 mmsys compatible for MT8195 platform.
> > 
> > For MT8195, VDOSYS0 and VDOSYS1 are 2 display HW pipelines binding
> > to
> > 2 different power domains, different clock drivers and different
> > mediatek-drm drivers.
> > 
> > Signed-off-by: Nancy.Lin <nancy.lin@mediatek.com>
> > Change-Id: I8065205ce44baa417f58176adaae9e1eefa6eadb
> 
> No Change-Id tag, please. There are some other commits with it as
> well.
> 
> Otherwise,
> 
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> Thanks,
> Nícolas

Sorry I forgot to remove Change-Id. I will check all the commits in the
series.

Thanks,
Nancy

Re: [PATCH v27 05/11] soc: mediatek: refine code to use mtk_mmsys_update_bits API

[Nancy Lin (林欣螢)]

Dear Nicolas,

Thanks for the review.

On Thu, 2022-11-03 at 16:32 -0400, Nícolas F. R. A. Prado wrote:
> On Thu, Nov 03, 2022 at 11:25:06AM +0800, Nancy.Lin wrote:
> [..]
> > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> 
> [..]
> > +static void mtk_mmsys_update_bits(struct mtk_mmsys *mmsys, u32
> > offset, u32 mask, u32 val)
> > +{
> > +	u32 tmp;
> > +
> > +	tmp = readl_relaxed(mmsys->regs + offset);
> > +	tmp = (tmp & ~mask) | val;
> 
> You should mask the value before writing to prevent bad values from
> overwriting
> bits outside the mask.
> 
> 	tmp = (tmp & ~mask) | (val & mask);
> 
> With that,
> 
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> Thanks,
> Nícolas

OK, I will fix it in the next revision.

Thanks,
Nancy

Re: [PATCH v27 06/11] soc: mediatek: add mtk-mmsys config API for mt8195 vdosys1

[Nancy Lin (林欣螢)]

Dear Nicolas,

Thanks for the review.

On Thu, 2022-11-03 at 15:56 -0400, Nícolas F. R. A. Prado wrote:
> On Thu, Nov 03, 2022 at 11:25:07AM +0800, Nancy.Lin wrote:
> > Add four mmsys config APIs. The config APIs are used for config
> > mmsys reg. Some mmsys regs need to be set according to the
> > HW engine binding to the mmsys simultaneously.
> > 
> > 1. mtk_mmsys_merge_async_config: config merge async width/height.
> >    async is used for cross-clock domain synchronization.
> > 2. mtk_mmsys_hdr_confing: config hdr backend async width/height.
> 
> [..]
> > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> 
> [..]
> > +void mtk_mmsys_hdr_confing(struct device *dev, int be_width, int
> > be_height)
> 
> confing -> config
> 
> Need to update the following commit as well.
> 
> Thanks,
> Nícolas
> 
Sorry for the typo. I will fix it in the next revision.

Thanks,
Nancy

> > +{
> > +	mtk_mmsys_update_bits(dev_get_drvdata(dev),
> > MT8195_VDO1_HDRBE_ASYNC_CFG_WD, ~0,
> > +			      be_height << 16 | be_width);
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mmsys_hdr_confing);
> 
> [..]
> > +void mtk_mmsys_hdr_confing(struct device *dev, int be_width, int
> > be_height);
> 
> [..]