Quoting Maxime Ripard (2022-02-21 10:44:20)
Hi
(Now I remember your series ;)
Hi,
I'm not sure this is a good thing though :)
On Fri, Feb 18, 2022 at 03:54:31PM +0100, Guillaume Ranquet wrote:
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 jason-jh.lin@mediatek.com.
Signed-off-by: Markus Schneider-Pargmann msp@baylibre.com Signed-off-by: Guillaume Ranquet granquet@baylibre.com
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]);elseclk_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
You're right about the error management... but even worse, those clocks were removed in an earlier version of the patch [1] I have reintroduced that bit of code in v7 by mistake when I applied fixes coming from mediatek engineers.
I don't know anything about dp/dpi or display in general, but the design posted by markus in v1 [2] seemed closed to what you are suggesting.
Thx, Guillaume.
[1] https://patchwork.kernel.org/project/linux-mediatek/patch/20210816192523.173... [2] https://patchwork.kernel.org/project/linux-mediatek/patch/20210906193529.718...