Re: [PATCH v8 13/19] drm/mediatek: dpi: Add dpintf support
From: Maxime Ripard <hidden>
Date: 2022-02-21 10:25:07
Also in:
dri-devel, linux-devicetree, linux-fbdev, linux-mediatek, linux-phy, lkml
Hi (Now I remember your series ;) On Fri, Feb 18, 2022 at 03:54:31PM +0100, Guillaume Ranquet wrote:
quoted hunk ↗ jump to hunk
dpintf is the displayport interface hardware unit. This unit is similar to dpi and can reuse most of the code. This patch adds support for mt8195-dpintf to this dpi driver. Main differences are: - Some features/functional components are not available for dpintf which are now excluded from code execution once is_dpintf is set - dpintf can and needs to choose between different clockdividers based on the clockspeed. This is done by choosing a different clock parent. - There are two additional clocks that need to be managed. These are only set for dpintf and will be set to NULL if not supplied. The clk_* calls handle these as normal clocks then. - Some register contents differ slightly between the two components. To work around this I added register bits/masks with a DPINTF_ prefix and use them where different. Based on a separate driver for dpintf created by Jason-JH.Lin [off-list ref]. Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> Signed-off-by: Guillaume Ranquet <redacted> --- drivers/gpu/drm/mediatek/mtk_dpi.c | 164 +++++++++++++++++--- drivers/gpu/drm/mediatek/mtk_dpi_regs.h | 38 +++++ drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 8 + drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 + drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +- include/linux/soc/mediatek/mtk-mmsys.h | 2 + 6 files changed, 198 insertions(+), 20 deletions(-)diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index be99399faf1bb..c5639ada868f8 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c@@ -63,6 +63,14 @@ enum mtk_dpi_out_color_format { MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL }; +enum TVDPLL_CLK { + TVDPLL_PLL = 0, + TVDPLL_D2 = 2, + TVDPLL_D4 = 4, + TVDPLL_D8 = 8, + TVDPLL_D16 = 16, +}; + struct mtk_dpi { struct drm_encoder encoder; struct drm_bridge bridge;@@ -71,8 +79,10 @@ struct mtk_dpi { void __iomem *regs; struct device *dev; struct clk *engine_clk; + struct clk *dpi_ck_cg; struct clk *pixel_clk; struct clk *tvd_clk; + struct clk *pclk_src[5]; int irq; struct drm_display_mode mode; const struct mtk_dpi_conf *conf;@@ -126,6 +136,7 @@ struct mtk_dpi_conf { const u32 *output_fmts; u32 num_output_fmts; bool is_ck_de_pol; + bool is_dpintf; bool swap_input_support; // Mask used for HWIDTH, HPORCH, VSYNC_WIDTH and VSYNC_PORCH (no shift) u32 dimension_mask;@@ -384,6 +395,25 @@ static void mtk_dpi_config_disable_edge(struct mtk_dpi *dpi) mtk_dpi_mask(dpi, dpi->conf->reg_h_fre_con, 0, EDGE_SEL_EN); } +static void mtk_dpi_matrix_sel(struct mtk_dpi *dpi, enum mtk_dpi_out_color_format format) +{ + u32 matrix_sel = 0; + + switch (format) { + case MTK_DPI_COLOR_FORMAT_YCBCR_422: + case MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL: + case MTK_DPI_COLOR_FORMAT_YCBCR_444: + case MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL: + case MTK_DPI_COLOR_FORMAT_XV_YCC: + if (dpi->mode.hdisplay <= 720) + matrix_sel = 0x2; + break; + default: + break; + } + mtk_dpi_mask(dpi, DPI_MATRIX_SET, matrix_sel, INT_MATRIX_SEL_MASK); +} + static void mtk_dpi_config_color_format(struct mtk_dpi *dpi, enum mtk_dpi_out_color_format format) {@@ -391,6 +421,7 @@ static void mtk_dpi_config_color_format(struct mtk_dpi *dpi, (format == MTK_DPI_COLOR_FORMAT_YCBCR_444_FULL)) { mtk_dpi_config_yuv422_enable(dpi, false); mtk_dpi_config_csc_enable(dpi, true); + mtk_dpi_matrix_sel(dpi, format); if (dpi->conf->swap_input_support) mtk_dpi_config_swap_input(dpi, false); mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_BGR);@@ -398,6 +429,7 @@ static void mtk_dpi_config_color_format(struct mtk_dpi *dpi, (format == MTK_DPI_COLOR_FORMAT_YCBCR_422_FULL)) { mtk_dpi_config_yuv422_enable(dpi, true); mtk_dpi_config_csc_enable(dpi, true); + mtk_dpi_matrix_sel(dpi, format); if (dpi->conf->swap_input_support) mtk_dpi_config_swap_input(dpi, true); mtk_dpi_config_channel_swap(dpi, MTK_DPI_OUT_CHANNEL_SWAP_RGB);@@ -438,6 +470,8 @@ static void mtk_dpi_power_off(struct mtk_dpi *dpi) mtk_dpi_disable(dpi); clk_disable_unprepare(dpi->pixel_clk); clk_disable_unprepare(dpi->engine_clk); + clk_disable_unprepare(dpi->dpi_ck_cg); + clk_disable_unprepare(dpi->tvd_clk); } static int mtk_dpi_power_on(struct mtk_dpi *dpi)@@ -447,12 +481,24 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi) if (++dpi->refcount != 1) return 0; + ret = clk_prepare_enable(dpi->tvd_clk); + if (ret) { + dev_err(dpi->dev, "Failed to enable tvd pll: %d\n", ret); + goto err_pixel; + } + ret = clk_prepare_enable(dpi->engine_clk); if (ret) { dev_err(dpi->dev, "Failed to enable engine clock: %d\n", ret); goto err_refcount; } + ret = clk_prepare_enable(dpi->dpi_ck_cg); + if (ret) { + dev_err(dpi->dev, "Failed to enable dpi_ck_cg clock: %d\n", ret); + goto err_ck_cg; + } + ret = clk_prepare_enable(dpi->pixel_clk); if (ret) { dev_err(dpi->dev, "Failed to enable pixel clock: %d\n", ret);@@ -462,10 +508,11 @@ static int mtk_dpi_power_on(struct mtk_dpi *dpi) if (dpi->pinctrl && dpi->pins_dpi) pinctrl_select_state(dpi->pinctrl, dpi->pins_dpi); - mtk_dpi_enable(dpi); return 0; err_pixel: + clk_disable_unprepare(dpi->dpi_ck_cg); +err_ck_cg: clk_disable_unprepare(dpi->engine_clk); err_refcount: dpi->refcount--;@@ -497,12 +544,21 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi, pll_rate = clk_get_rate(dpi->tvd_clk); vm.pixelclock = pll_rate / factor; - if ((dpi->output_fmt == MEDIA_BUS_FMT_RGB888_2X12_LE) || - (dpi->output_fmt == MEDIA_BUS_FMT_RGB888_2X12_BE)) + if (dpi->conf->is_dpintf) { + if (factor == 1) + clk_set_parent(dpi->pixel_clk, dpi->pclk_src[2]); + else if (factor == 2) + clk_set_parent(dpi->pixel_clk, dpi->pclk_src[3]); + else if (factor == 4) + clk_set_parent(dpi->pixel_clk, dpi->pclk_src[4]); + else + clk_set_parent(dpi->pixel_clk, dpi->pclk_src[2]);
You get a reference to these clocks using:
+ dpi->pclk_src[1] = devm_clk_get(dev, "TVDPLL_D2"); + dpi->pclk_src[2] = devm_clk_get(dev, "TVDPLL_D4"); + dpi->pclk_src[3] = devm_clk_get(dev, "TVDPLL_D8"); + dpi->pclk_src[4] = devm_clk_get(dev, "TVDPLL_D16"); +
So if the clock isn't there, you'll get an error pointer, won't check it, but will dereference it. If the clock is optional, you should use clk_get_optional, otherwise you should check the error returned. That clock setup is weird though. I don't have any knowledge about your platform, but either the clock should be a single one if it has multiple dividers, or the pixel_clk clock should behave as a mux and just pick the best parent for the rate it's given. Maxime