| Message ID | 1666147936-27368-3-git-send-email-xinlei.lee@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add dpi output format control for MT8186 | expand |
Il 19/10/22 04:52, xinlei.lee@mediatek.com ha scritto: > From: Xinlei Lee <xinlei.lee@mediatek.com> > > Dpi output needs to adjust the output format to dual edge for MT8186. > > Co-developed-by: Jitao Shi <jitao.shi@mediatek.com> > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com> > Signed-off-by: Xinlei Lee <xinlei.lee@mediatek.com> > Reviewed-by: CK Hu <ck.hu@mediatek.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
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 > } [..]
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
On Fri, Oct 21, 2022 at 08:18:25PM +0800, xinlei.lee wrote: > 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: [..] > > > @@ -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? We could check that the format in the else path is a single edge RGB888 format like MEDIA_BUS_FMT_RGB888_1X24 before setting the mmsys config, but there are also other formats possible, and I actually don't think it's worth it to complicate the logic further to protect from an edge-case that can't be hit yet... So just leave it as it is. We can worry about it when/if a non-RGB888 single edge format needs to be setup on mmsys. So, Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> Thanks, Nícolas
On Fri, 2022-10-21 at 11:39 -0400, Nícolas F. R. A. Prado wrote: > On Fri, Oct 21, 2022 at 08:18:25PM +0800, xinlei.lee wrote: > > 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: > > [..] > > > > @@ -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? > > We could check that the format in the else path is a single edge > RGB888 format > like MEDIA_BUS_FMT_RGB888_1X24 before setting the mmsys config, but > there are > also other formats possible, and I actually don't think it's worth it > to > complicate the logic further to protect from an edge-case that can't > be hit > yet... > > So just leave it as it is. We can worry about it when/if a non-RGB888 > single > edge format needs to be setup on mmsys. > > So, > > Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > Thanks, > Nícolas Hi Nícolas: Thanks for your review! Best Regards! xinlei
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index 508a6d994e83..b9d740efdb6a 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -14,6 +14,7 @@ #include <linux/of_graph.h> #include <linux/pinctrl/consumer.h> #include <linux/platform_device.h> +#include <linux/soc/mediatek/mtk-mmsys.h> #include <linux/types.h> #include <video/videomode.h> @@ -29,6 +30,7 @@ #include "mtk_disp_drv.h" #include "mtk_dpi_regs.h" #include "mtk_drm_ddp_comp.h" +#include "mtk_drm_drv.h" enum mtk_dpi_out_bit_num { MTK_DPI_OUT_BIT_NUM_8BITS, @@ -66,6 +68,7 @@ struct mtk_dpi { struct drm_connector *connector; void __iomem *regs; struct device *dev; + struct device *mmsys_dev; struct clk *engine_clk; struct clk *pixel_clk; struct clk *tvd_clk; @@ -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. */ struct mtk_dpi_conf { unsigned int (*cal_factor)(int clock); @@ -152,6 +156,7 @@ struct mtk_dpi_conf { u32 yuv422_en_bit; u32 csc_enable_bit; u32 pixels_per_iter; + bool edge_cfg_in_mmsys; }; static void mtk_dpi_mask(struct mtk_dpi *dpi, u32 offset, u32 val, u32 mask) @@ -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); } } @@ -777,8 +786,10 @@ static int mtk_dpi_bind(struct device *dev, struct device *master, void *data) { struct mtk_dpi *dpi = dev_get_drvdata(dev); struct drm_device *drm_dev = data; + struct mtk_drm_private *priv = drm_dev->dev_private; int ret; + dpi->mmsys_dev = priv->mmsys_dev; ret = drm_simple_encoder_init(drm_dev, &dpi->encoder, DRM_MODE_ENCODER_TMDS); if (ret) {