Hi, Markus:
Markus Schneider-Pargmann msp@baylibre.com 於 2021年9月17日 週五 下午9:33寫道：
Hi Chun-Kuang,
On Tue, Sep 14, 2021 at 07:25:48AM +0800, Chun-Kuang Hu wrote:
Hi, Markus:
Markus Schneider-Pargmann msp@baylibre.com 於 2021年9月10日 週五 下午1:36寫道：
Hi Chun-Kuang,
On Fri, Sep 10, 2021 at 07:37:50AM +0800, Chun-Kuang Hu wrote:
Hi, Markus:
Markus Schneider-Pargmann msp@baylibre.com 於 2021年9月7日 週二 上午3:37寫道：
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 jason-jh.lin@mediatek.com.
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
...
+#define TOP_OFFSET 0x2000 +#define ENC0_OFFSET 0x3000 +#define ENC1_OFFSET 0x3200 +#define TRANS_OFFSET 0x3400 +#define AUX_OFFSET 0x3600 +#define SEC_OFFSET 0x4000
...
+#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... [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
Thanks 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.
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... [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv... [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
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, Markus
Regards, Chun-Kuang.
So I personally would prefer keeping it as part of the whole driver because of the above mentioned reasons. What do you think?
Thanks, Markus
Regards, Chun-Kuang.
+#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