Re: [PATCH v12,2/3] drm: mediatek: Set dpi format in mmsys

From: xinlei.lee <xinlei.lee@mediatek.com>
To: "Nícolas F. R. A. Prado" <nfraprado@collabora.com>
Cc: <matthias.bgg@gmail.com>, <rex-bc.chen@mediatek.com>,
	<angelogioacchino.delregno@collabora.com>,
	<jason-jh.lin@mediatek.com>, <chunkuang.hu@kernel.org>,
	<p.zabel@pengutronix.de>, <airlied@linux.ie>, <daniel@ffwll.ch>,
	<dri-devel@lists.freedesktop.org>,
	<linux-mediatek@lists.infradead.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	<Project_Global_Chrome_Upstream_Group@mediatek.com>,
	Jitao Shi <jitao.shi@mediatek.com>
Subject: Re: [PATCH v12,2/3] drm: mediatek: Set dpi format in mmsys
Date: Fri, 21 Oct 2022 20:18:25 +0800	[thread overview]
Message-ID: <2e1899236144774d4a1c0224532b11ef7e1f9862.camel@mediatek.com> (raw)
In-Reply-To: <20221020164022.vtnumo3fob2sh54w@notapiano>

On Thu, 2022-10-20 at 12:40 -0400, Nícolas F. R. A. Prado wrote:
> On Wed, Oct 19, 2022 at 10:52:15AM +0800, xinlei.lee@mediatek.com
> wrote:
> [..]
> > @@ -134,6 +137,7 @@ struct mtk_dpi_yc_limit {
> >   * @yuv422_en_bit: Enable bit of yuv422.
> >   * @csc_enable_bit: Enable bit of CSC.
> >   * @pixels_per_iter: Quantity of transferred pixels per iteration.
> > + * @edge_cfg_in_mmsys: If the edge configuration for DPI's output
> > needs to be set in MMSYS.
> 
> As Angelo suggested previously, this could be written slightly
> shorter as 
> 
>   * @edge_cfg_in_mmsys: Edge configuration for DPI output has to be
> set in MMSYS.
> 
> >   */
> 
> [..]
> > @@ -448,8 +453,12 @@ static void mtk_dpi_dual_edge(struct mtk_dpi
> > *dpi)
> >  		mtk_dpi_mask(dpi, DPI_OUTPUT_SETTING,
> >  			     dpi->output_fmt ==
> > MEDIA_BUS_FMT_RGB888_2X12_LE ?
> >  			     EDGE_SEL : 0, EDGE_SEL);
> > +		if (dpi->conf->edge_cfg_in_mmsys)
> > +			mtk_mmsys_ddp_dpi_fmt_config(dpi->mmsys_dev,
> > MTK_DPI_RGB888_DDR_CON);
> >  	} else {
> >  		mtk_dpi_mask(dpi, DPI_DDR_SETTING, DDR_EN | DDR_4PHASE,
> > 0);
> > +		if (dpi->conf->edge_cfg_in_mmsys)
> > +			mtk_mmsys_ddp_dpi_fmt_config(dpi->mmsys_dev,
> > MTK_DPI_RGB888_SDR_CON);
> 
> I know this isn't one of the formats supported by MT8186, but since
> we're using
> platform-agnostic formats now... This else branch in theory could
> also run for a
> format like MEDIA_BUS_FMT_YUYV8_1X16. Would it make sense to set
> MTK_DPI_RGB888_SDR_CON in that case?
> 
> Thanks,
> Nícolas
> 
> >  	}
> 
> [..]

Hi Nícolas:

Thanks for your review!

You are right, I understand you think this MTK_DPI_RGB888_SDR_CON 
format seems useless as it will not be set, I confirmed with the 
designer how the setting in mmsys affects the output format of the 
MT8186, this mmsys setting will not be used by other ICs.

As mentioned earlier, the mmsys setting will make the MT8186dpi have 
four output formats, even though the MT8186 dpi may not use them all.

So what needs to change here?

Best Regards!
xinlei

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel