Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver
From: Chun-Kuang Hu <chunkuang.hu@kernel.org>
Date: 2021-09-17 14:39:40
Also in:
dri-devel, linux-mediatek
Possibly related (same subject, not in this thread)
- 2021-09-09 · Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver · Markus Schneider-Pargmann <msp@baylibre.com>
- 2021-09-09 · Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver · Markus Schneider-Pargmann <msp@baylibre.com>
- 2021-09-07 · Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver · Philipp Zabel <p.zabel@pengutronix.de>
- 2021-09-06 · Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver · Sam Ravnborg <hidden>
Hi, Markus: Markus Schneider-Pargmann [off-list ref] 於 2021年9月17日 週五 下午9:33寫道：
Hi Chun-Kuang, On Tue, Sep 14, 2021 at 07:25:48AM +0800, Chun-Kuang Hu wrote:quoted
Hi, Markus: Markus Schneider-Pargmann [off-list ref] 於 2021年9月10日 週五 下午1:36寫道：quoted
Hi Chun-Kuang, On Fri, Sep 10, 2021 at 07:37:50AM +0800, Chun-Kuang Hu wrote:quoted
Hi, Markus: Markus Schneider-Pargmann [off-list ref] 於 2021年9月7日 週二 上午3:37寫道：quoted
This patch adds a DisplayPort driver for the Mediatek mt8195 SoC. It supports both functional units on the mt8195, the embedded DisplayPort as well as the external DisplayPort units. It offers hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up to 4 lanes. This driver is based on an initial version by Jason-JH.Lin [off-list ref]. Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> --- Notes: Changes RFC -> v1: - Removed unused register definitions. - Replaced workqueue with threaded irq. - Removed connector code. - Move to atomic_* drm functions. - General cleanups of the code. - Remove unused select GENERIC_PHY. drivers/gpu/drm/mediatek/Kconfig | 6 + drivers/gpu/drm/mediatek/Makefile | 2 + drivers/gpu/drm/mediatek/mtk_dp.c | 2881 +++++++++++++++++++++++++ drivers/gpu/drm/mediatek/mtk_dp_reg.h | 580 +++++ 4 files changed, 3469 insertions(+) create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h...quoted
quoted
+#define TOP_OFFSET 0x2000 +#define ENC0_OFFSET 0x3000 +#define ENC1_OFFSET 0x3200 +#define TRANS_OFFSET 0x3400 +#define AUX_OFFSET 0x3600 +#define SEC_OFFSET 0x4000...quoted
quoted
+ +#define DP_PHY_DIG_PLL_CTL_1 0x1014 +# define TPLL_SSC_EN BIT(3)It seems that register 0x1000 ~ 0x1fff is to control phy and 0x2000 ~ 0x4fff is to control non-phy part. For mipi and hdmi, the phy part is an independent device [1] and the phy driver is independent [2] , so I would like this phy to be an independent device. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.14 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/mediatek?h=v5.14Thanks for your feedback. I looked into both mipi and hdmi phy drivers that you referenced. It looks like both are really separate units in their SoCs having their own registerspaces located at a completely different range than the units using the phy. For this displayport driver, the phy registers are listed as part of the (e)DP_TX unit in the datasheet. Next to the phy registers all the other parts are listed as well in the same overall register ranges (see above), e.g. TOP_OFFSET, ENC_OFFSET or SEC_OFFSET. Also I would like to avoid splitting it up into a separate unit in the devicetree as the datasheet handles it as a single unit (including the phy registers).OK, according to the datasheet, let it to be a single device.quoted
From a practical perspective there is also not much to these PHY registers. The only things that would be done in the driver are: - initializing the lane driving parameters with static values - setup the bitrate - enable/disable SSC - do a reset Exporting these four used functions over a driver boundary wouldn't help clarity I think and the code probably can't be reused by any other component anyways.Use mmsys device [1] as an example. mmsys has both clock control function and other function including routing function. The main driver [2] is placed in soc folder, and the clock control part [3] is separated to clk folder but the clock control part just simply control clock gating. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.15-rc1#n992 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/mediatek/mtk-mmsys.c?h=v5.15-rc1 [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/mediatek/clk-mt8173-mm.c?h=v5.15-rc1 I think many phy driver could be used only by single device driver, so we don't need to consider the reusability.Thank you, I have split of a display port phy similar to the patches you cited. I will send the next version soon. Another short question. This series has a functional dependency on the rest of the video pipeline, so mostly the patch series vdosys0 and vdosys1. So I suppose the theoretical order for merging (once this series is ready for merge) is first vdosys0/1 and dependencies and then this displayport series, is that correct?
Sometimes I would apply partial reviewed patches in a series. Even though these partial patches would not functionally work without other patches, they are so independent and I would like to apply them to prevent conflicts in the future. But these patches should build pass and not break other workable function. Regards, Chun-Kuang.
Thanks, Markusquoted
Regards, Chun-Kuang.quoted
So I personally would prefer keeping it as part of the whole driver because of the above mentioned reasons. What do you think? Thanks, Markusquoted
Regards, Chun-Kuang.quoted
+ +#define DP_PHY_DIG_BIT_RATE 0x103C +# define BIT_RATE_RBR 0 +# define BIT_RATE_HBR 1 +# define BIT_RATE_HBR2 2 +# define BIT_RATE_HBR3 3 + +#define DP_PHY_DIG_SW_RST 0x1038 +# define DP_GLB_SW_RST_PHYD BIT(0) + +#define MTK_DP_LANE0_DRIVING_PARAM_3 0x1138 +#define MTK_DP_LANE1_DRIVING_PARAM_3 0x1238 +#define MTK_DP_LANE2_DRIVING_PARAM_3 0x1338 +#define MTK_DP_LANE3_DRIVING_PARAM_3 0x1438 +# define XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT 0x10 +# define XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT (0x14 << 8) +# define XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT (0x18 << 16) +# define XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT (0x20 << 24) +# define DRIVING_PARAM_3_DEFAULT (XTP_LN_TX_LCTXC0_SW0_PRE0_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW0_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW0_PRE2_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW0_PRE3_DEFAULT) + +#define MTK_DP_LANE0_DRIVING_PARAM_4 0x113C +#define MTK_DP_LANE1_DRIVING_PARAM_4 0x123C +#define MTK_DP_LANE2_DRIVING_PARAM_4 0x133C +#define MTK_DP_LANE3_DRIVING_PARAM_4 0x143C +# define XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT 0x18 +# define XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT (0x1e << 8) +# define XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT (0x24 << 16) +# define XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT (0x20 << 24) +# define DRIVING_PARAM_4_DEFAULT (XTP_LN_TX_LCTXC0_SW1_PRE0_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW1_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW1_PRE2_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW2_PRE0_DEFAULT) + +#define MTK_DP_LANE0_DRIVING_PARAM_5 0x1140 +#define MTK_DP_LANE1_DRIVING_PARAM_5 0x1240 +#define MTK_DP_LANE2_DRIVING_PARAM_5 0x1340 +#define MTK_DP_LANE3_DRIVING_PARAM_5 0x1440 +# define XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT 0x28 +# define XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT (0x30 << 8) +# define DRIVING_PARAM_5_DEFAULT (XTP_LN_TX_LCTXC0_SW2_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXC0_SW3_PRE0_DEFAULT) + +#define MTK_DP_LANE0_DRIVING_PARAM_6 0x1144 +#define MTK_DP_LANE1_DRIVING_PARAM_6 0x1244 +#define MTK_DP_LANE2_DRIVING_PARAM_6 0x1344 +#define MTK_DP_LANE3_DRIVING_PARAM_6 0x1444 +# define XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT 0x00 +# define XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT (0x04 << 8) +# define XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT (0x08 << 16) +# define XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT (0x10 << 24) +# define DRIVING_PARAM_6_DEFAULT (XTP_LN_TX_LCTXCP1_SW0_PRE0_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW0_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW0_PRE2_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW0_PRE3_DEFAULT) + +#define MTK_DP_LANE0_DRIVING_PARAM_7 0x1148 +#define MTK_DP_LANE1_DRIVING_PARAM_7 0x1248 +#define MTK_DP_LANE2_DRIVING_PARAM_7 0x1348 +#define MTK_DP_LANE3_DRIVING_PARAM_7 0x1448 +# define XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT 0x00 +# define XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT (0x06 << 8) +# define XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT (0x0c << 16) +# define XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT (0x00 << 24) +# define DRIVING_PARAM_7_DEFAULT (XTP_LN_TX_LCTXCP1_SW1_PRE0_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW1_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW1_PRE2_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW2_PRE0_DEFAULT) + +#define MTK_DP_LANE0_DRIVING_PARAM_8 0x114C +#define MTK_DP_LANE1_DRIVING_PARAM_8 0x124C +#define MTK_DP_LANE2_DRIVING_PARAM_8 0x134C +#define MTK_DP_LANE3_DRIVING_PARAM_8 0x144C +# define XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT 0x08 +# define XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT (0x00 << 8) +# define DRIVING_PARAM_8_DEFAULT (XTP_LN_TX_LCTXCP1_SW2_PRE1_DEFAULT | \ + XTP_LN_TX_LCTXCP1_SW3_PRE0_DEFAULT) + +#endif /*_MTK_DP_REG_H_*/ -- 2.33.0
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel