* [PATCH v9 0/5] drm/msm/hdmi & phy: use generic PHY framework
@ 2026-05-13 18:14 Dmitry Baryshkov
2026-05-13 18:14 ` [PATCH v9 2/5] phy: qualcomm: hdmi-28lpm: provide dynamic configuration Dmitry Baryshkov
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Dmitry Baryshkov @ 2026-05-13 18:14 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Vinod Koul, Neil Armstrong
Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno, linux-phy,
Dmitry Baryshkov, Konrad Dybcio, Konrad Dybcio
The MSM HDMI PHYs have been using the ad-hoc approach / API instead of
using the generic API framework. Move MSM HDMI PHY drivers to
drivers/phy/qualcomm and rework them to use generic PHY framework. This
way all the QMP-related code is kept at the same place.
Also MSM8974 HDMI PHY, 28nm DSI PHY and apq8964 SATA PHY now can use
common helpers for the UNI PLL.
This also causes some design changes. Currently on MSM8996 the HDMI PLL
implements clock's set_rate(), while other HDMI PHY drivers used the
ad-hoc PHY API for setting the PLL rate (this includes in-tree MSM8960
driver and posted, but not merged, MSM8974 driver). This might result in
the PLL being set to one rate, while the rest of the PHY being tuned to
work at another rate. Adopt the latter idea and always use
phy_configure() to tune the PHY and set the PLL rate.
Merge strategy: Merge the first patch (either through drm/msm or through
the PHY tree), merge the rest of the patches in the next cycle.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Changes in v9:
- Split 28lpm changes to a separate patch (Sashiko)
- Reworked PHY power up /down sequences in hdmi_bridge.c (Sashiko)
- Switched to pm_runtime_put_sync() (Sashiko)
- Link to v8: https://patch.msgid.link/20260401-fd-hdmi-phy-v8-0-51b0e98edf6c@oss.qualcomm.com
Changes in v8:
- Rebased on linux-next, fixing conflicts
- Added missing ids for APQ8084 and MSM8998 (Sashiko)
- Switched to pm_runtime_put() (Sashiko)
- Fixed several missed *1000 after pixclk -> tmds_char_rate conversion
(Sashiko)
- Fixed several math overflows (Sashiko)
- Moved UNIPHY_PLL_LOCK to the common header (Sashiko)
- Link to v7: https://patch.msgid.link/20260324-fd-hdmi-phy-v7-0-b41dde8d83b8@oss.qualcomm.com
Changes in v7:
- Fixed the build issue between msm8974 patches.
- Dropped even more writel / readl wrappers (now from QMP PHYs)
- Link to v6: https://lore.kernel.org/r/20260319-fd-hdmi-phy-v6-0-cefc08a55470@oss.qualcomm.com
Changes in v6:
- Changed MSM8974 HDMI PHY driver to use FIELD_PREP / FIELD_GET (Konrad)
- Fixed rate recalculation for MSM8974 HDMI PHY (Konrad)
- Dropped register read/write wrappers
- Link to v5: https://lore.kernel.org/r/20260314-fd-hdmi-phy-v5-0-58122ae96d3b@oss.qualcomm.com
Changes in v5:
- Kept only a single place which handles extp clk (after PHY power on,
before PHY power off) (Neil)
- Inlined pm_runtime calls in the HDMI TX driver, replaced
pm_runtime_resume_and_get() with pm_runtime_get_sync(), since
atomic_pre_enable() can not fail.
- Renamed registers defines to drop the REG_ prefix.
- Link to v4: https://lore.kernel.org/r/20250520-fd-hdmi-phy-v4-0-fcbaa652ad75@oss.qualcomm.com
Changes in v3-v4:
- Rebased on top of linux-next, solving conflicts
- Squashed add-and-remove patches into a single git mv patch
- Dropped HDMI PHY header patch (merged upstream)
Changes in v2:
- Changed msm8960 / apq8064 to calculate register data instead of using
fixed tables. This extends the list of supported modes.
(Implementation is based on mdss-hdmi-pll-28lpm.c from msm-4.14).
- Fixed the reprogramming of PLL rate on apq8064.
- Merged all non-QMP HDMI PHY drivers into a common PHY_QCOM_HDMI
driver (suggested by Rob Clark)
---
Dmitry Baryshkov (5):
drm/msm/hdmi: switch to generic PHY subsystem
phy: qualcomm: hdmi-28lpm: provide dynamic configuration
phy: qcom: apq8064-sata: extract UNI PLL register defines
phy: qcom-uniphy: add more registers from display PHYs
phy: qualcomm: add MSM8974 HDMI PHY support
drivers/gpu/drm/msm/Makefile | 7 -
drivers/gpu/drm/msm/hdmi/hdmi.c | 59 +-
drivers/gpu/drm/msm/hdmi/hdmi.h | 83 +--
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 88 ++-
drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 226 -------
drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c | 51 --
drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c | 761 ----------------------
drivers/gpu/drm/msm/hdmi/hdmi_phy_8998.c | 765 -----------------------
drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c | 141 -----
drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c | 44 --
drivers/gpu/drm/msm/hdmi/hdmi_pll_8960.c | 460 --------------
drivers/gpu/drm/msm/registers/display/hdmi.xml | 537 ----------------
drivers/phy/qualcomm/Kconfig | 24 +
drivers/phy/qualcomm/Makefile | 14 +
drivers/phy/qualcomm/phy-qcom-apq8064-sata.c | 24 +-
drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c | 353 +++++++++++
drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c | 477 ++++++++++++++
drivers/phy/qualcomm/phy-qcom-hdmi-45nm.c | 186 ++++++
drivers/phy/qualcomm/phy-qcom-hdmi-preqmp.c | 211 +++++++
drivers/phy/qualcomm/phy-qcom-hdmi-preqmp.h | 59 ++
drivers/phy/qualcomm/phy-qcom-qmp-hdmi-base.c | 185 ++++++
drivers/phy/qualcomm/phy-qcom-qmp-hdmi-msm8996.c | 440 +++++++++++++
drivers/phy/qualcomm/phy-qcom-qmp-hdmi-msm8998.c | 489 +++++++++++++++
drivers/phy/qualcomm/phy-qcom-qmp-hdmi.h | 49 ++
drivers/phy/qualcomm/phy-qcom-uniphy.h | 76 +++
25 files changed, 2617 insertions(+), 3192 deletions(-)
---
base-commit: e6e0eed7a4199442ad9be37daafafa4503c3f436
change-id: 20240109-fd-hdmi-phy-44b8319fbcc7
Best regards,
--
With best wishes
Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v9 2/5] phy: qualcomm: hdmi-28lpm: provide dynamic configuration 2026-05-13 18:14 [PATCH v9 0/5] drm/msm/hdmi & phy: use generic PHY framework Dmitry Baryshkov @ 2026-05-13 18:14 ` Dmitry Baryshkov 2026-05-14 10:19 ` sashiko-bot 2026-05-13 18:14 ` [PATCH v9 3/5] phy: qcom: apq8064-sata: extract UNI PLL register defines Dmitry Baryshkov ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Dmitry Baryshkov @ 2026-05-13 18:14 UTC (permalink / raw) To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Vinod Koul, Neil Armstrong Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno, linux-phy Replace fixed value configuration tables with the values calculated at the runtime. In some cases the values might differ from the original values. Those were validated on the IFC6410 board. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c | 325 +++++++++-------------------- 1 file changed, 104 insertions(+), 221 deletions(-) diff --git a/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c b/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c index f1e7113e10bd..90d3331313c0 100644 --- a/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c +++ b/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c @@ -148,222 +148,17 @@ #define HDMI_8960_COMMON_DIV 5 -struct pll_rate { - unsigned long rate; - int num_reg; - struct { - u32 val; - u32 reg; - } conf[32]; -}; - -/* NOTE: keep sorted highest freq to lowest: */ -static const struct pll_rate freqtbl[] = { - { 154000000, 14, { - { 0x08, REG_HDMI_8960_PHY_PLL_REFCLK_CFG }, - { 0x20, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG0 }, - { 0xf9, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG1 }, - { 0x02, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0 }, - { 0x03, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG1 }, - { 0x3b, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG3 }, - { 0x86, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG4 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG5 }, - { 0x0d, REG_HDMI_8960_PHY_PLL_SDM_CFG0 }, - { 0x4d, REG_HDMI_8960_PHY_PLL_SDM_CFG1 }, - { 0x5e, REG_HDMI_8960_PHY_PLL_SDM_CFG2 }, - { 0x42, REG_HDMI_8960_PHY_PLL_SDM_CFG3 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG4 }, - } - }, - /* 1080p60/1080p50 case */ - { 148500000, 27, { - { 0x02, REG_HDMI_8960_PHY_PLL_REFCLK_CFG }, - { 0x02, REG_HDMI_8960_PHY_PLL_CHRG_PUMP_CFG }, - { 0x01, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG0 }, - { 0x33, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG1 }, - { 0x2c, REG_HDMI_8960_PHY_PLL_IDAC_ADJ_CFG }, - { 0x06, REG_HDMI_8960_PHY_PLL_I_VI_KVCO_CFG }, - { 0x0a, REG_HDMI_8960_PHY_PLL_PWRDN_B }, - { 0x76, REG_HDMI_8960_PHY_PLL_SDM_CFG0 }, - { 0x01, REG_HDMI_8960_PHY_PLL_SDM_CFG1 }, - { 0x4c, REG_HDMI_8960_PHY_PLL_SDM_CFG2 }, - { 0xc0, REG_HDMI_8960_PHY_PLL_SDM_CFG3 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG4 }, - { 0x9a, REG_HDMI_8960_PHY_PLL_SSC_CFG0 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SSC_CFG1 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SSC_CFG2 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SSC_CFG3 }, - { 0x10, REG_HDMI_8960_PHY_PLL_LOCKDET_CFG0 }, - { 0x1a, REG_HDMI_8960_PHY_PLL_LOCKDET_CFG1 }, - { 0x0d, REG_HDMI_8960_PHY_PLL_LOCKDET_CFG2 }, - { 0xe6, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0 }, - { 0x02, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG1 }, - { 0x3b, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG3 }, - { 0x86, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG4 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG5 }, - { 0x33, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG6 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG7 }, - } - }, - { 108000000, 13, { - { 0x08, REG_HDMI_8960_PHY_PLL_REFCLK_CFG }, - { 0x21, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG0 }, - { 0xf9, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG1 }, - { 0x1c, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0 }, - { 0x02, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG1 }, - { 0x3b, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2 }, - { 0x86, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG4 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG5 }, - { 0x49, REG_HDMI_8960_PHY_PLL_SDM_CFG0 }, - { 0x49, REG_HDMI_8960_PHY_PLL_SDM_CFG1 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG2 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG3 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG4 }, - } - }, - /* 720p60/720p50/1080i60/1080i50/1080p24/1080p30/1080p25 */ - { 74250000, 8, { - { 0x0a, REG_HDMI_8960_PHY_PLL_PWRDN_B }, - { 0x12, REG_HDMI_8960_PHY_PLL_REFCLK_CFG }, - { 0x01, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG0 }, - { 0x33, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG1 }, - { 0x76, REG_HDMI_8960_PHY_PLL_SDM_CFG0 }, - { 0xe6, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0 }, - { 0x02, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG1 }, - { 0x3b, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2 }, - } - }, - { 74176000, 14, { - { 0x18, REG_HDMI_8960_PHY_PLL_REFCLK_CFG }, - { 0x20, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG0 }, - { 0xf9, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG1 }, - { 0xe5, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0 }, - { 0x02, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG1 }, - { 0x3b, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG3 }, - { 0x86, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG4 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG5 }, - { 0x0c, REG_HDMI_8960_PHY_PLL_SDM_CFG0 }, - { 0x4c, REG_HDMI_8960_PHY_PLL_SDM_CFG1 }, - { 0x7d, REG_HDMI_8960_PHY_PLL_SDM_CFG2 }, - { 0xbc, REG_HDMI_8960_PHY_PLL_SDM_CFG3 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG4 }, - } - }, - { 65000000, 14, { - { 0x18, REG_HDMI_8960_PHY_PLL_REFCLK_CFG }, - { 0x20, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG0 }, - { 0xf9, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG1 }, - { 0x8a, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0 }, - { 0x02, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG1 }, - { 0x3b, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG3 }, - { 0x86, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG4 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG5 }, - { 0x0b, REG_HDMI_8960_PHY_PLL_SDM_CFG0 }, - { 0x4b, REG_HDMI_8960_PHY_PLL_SDM_CFG1 }, - { 0x7b, REG_HDMI_8960_PHY_PLL_SDM_CFG2 }, - { 0x09, REG_HDMI_8960_PHY_PLL_SDM_CFG3 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG4 }, - } - }, - /* 480p60/480i60 */ - { 27030000, 18, { - { 0x0a, REG_HDMI_8960_PHY_PLL_PWRDN_B }, - { 0x38, REG_HDMI_8960_PHY_PLL_REFCLK_CFG }, - { 0x02, REG_HDMI_8960_PHY_PLL_CHRG_PUMP_CFG }, - { 0x20, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG0 }, - { 0xff, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG1 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG0 }, - { 0x4e, REG_HDMI_8960_PHY_PLL_SDM_CFG1 }, - { 0xd7, REG_HDMI_8960_PHY_PLL_SDM_CFG2 }, - { 0x03, REG_HDMI_8960_PHY_PLL_SDM_CFG3 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG4 }, - { 0x2a, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0 }, - { 0x03, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG1 }, - { 0x3b, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG3 }, - { 0x86, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG4 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG5 }, - { 0x33, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG6 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG7 }, - } - }, - /* 576p50/576i50 */ - { 27000000, 27, { - { 0x32, REG_HDMI_8960_PHY_PLL_REFCLK_CFG }, - { 0x02, REG_HDMI_8960_PHY_PLL_CHRG_PUMP_CFG }, - { 0x01, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG0 }, - { 0x33, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG1 }, - { 0x2c, REG_HDMI_8960_PHY_PLL_IDAC_ADJ_CFG }, - { 0x06, REG_HDMI_8960_PHY_PLL_I_VI_KVCO_CFG }, - { 0x0a, REG_HDMI_8960_PHY_PLL_PWRDN_B }, - { 0x7b, REG_HDMI_8960_PHY_PLL_SDM_CFG0 }, - { 0x01, REG_HDMI_8960_PHY_PLL_SDM_CFG1 }, - { 0x4c, REG_HDMI_8960_PHY_PLL_SDM_CFG2 }, - { 0xc0, REG_HDMI_8960_PHY_PLL_SDM_CFG3 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG4 }, - { 0x9a, REG_HDMI_8960_PHY_PLL_SSC_CFG0 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SSC_CFG1 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SSC_CFG2 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SSC_CFG3 }, - { 0x10, REG_HDMI_8960_PHY_PLL_LOCKDET_CFG0 }, - { 0x1a, REG_HDMI_8960_PHY_PLL_LOCKDET_CFG1 }, - { 0x0d, REG_HDMI_8960_PHY_PLL_LOCKDET_CFG2 }, - { 0x2a, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0 }, - { 0x03, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG1 }, - { 0x3b, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG3 }, - { 0x86, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG4 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG5 }, - { 0x33, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG6 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG7 }, - } - }, - /* 640x480p60 */ - { 25200000, 27, { - { 0x32, REG_HDMI_8960_PHY_PLL_REFCLK_CFG }, - { 0x02, REG_HDMI_8960_PHY_PLL_CHRG_PUMP_CFG }, - { 0x01, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG0 }, - { 0x33, REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG1 }, - { 0x2c, REG_HDMI_8960_PHY_PLL_IDAC_ADJ_CFG }, - { 0x06, REG_HDMI_8960_PHY_PLL_I_VI_KVCO_CFG }, - { 0x0a, REG_HDMI_8960_PHY_PLL_PWRDN_B }, - { 0x77, REG_HDMI_8960_PHY_PLL_SDM_CFG0 }, - { 0x4c, REG_HDMI_8960_PHY_PLL_SDM_CFG1 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG2 }, - { 0xc0, REG_HDMI_8960_PHY_PLL_SDM_CFG3 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SDM_CFG4 }, - { 0x9a, REG_HDMI_8960_PHY_PLL_SSC_CFG0 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SSC_CFG1 }, - { 0x00, REG_HDMI_8960_PHY_PLL_SSC_CFG2 }, - { 0x20, REG_HDMI_8960_PHY_PLL_SSC_CFG3 }, - { 0x10, REG_HDMI_8960_PHY_PLL_LOCKDET_CFG0 }, - { 0x1a, REG_HDMI_8960_PHY_PLL_LOCKDET_CFG1 }, - { 0x0d, REG_HDMI_8960_PHY_PLL_LOCKDET_CFG2 }, - { 0xf4, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0 }, - { 0x02, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG1 }, - { 0x3b, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG3 }, - { 0x86, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG4 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG5 }, - { 0x33, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG6 }, - { 0x00, REG_HDMI_8960_PHY_PLL_VCOCAL_CFG7 }, - } - }, -}; - -static const struct pll_rate *qcom_hdmi_8960_pll_find_rate(unsigned long rate) +static inline void write16(u16 val, void __iomem *reg) { - int i; - - for (i = 1; i < ARRAY_SIZE(freqtbl); i++) - if (rate > freqtbl[i].rate) - return &freqtbl[i - 1]; + writel(val & 0xff, reg); + writel(val >> 8, reg + 4); +} - return &freqtbl[i - 1]; +static inline void write24(u32 val, void __iomem *reg) +{ + writel(val & 0xff, reg); + writel((val >> 8) & 0xff, reg + 4); + writel(val >> 16, reg + 8); } static inline u32 read24(void __iomem *reg) @@ -407,6 +202,70 @@ static unsigned long qcom_28lpm_recalc(struct qcom_hdmi_preqmp_phy *hdmi_phy, return rate; } +/* This function is close to UNIPHY, but it has slighly different fields */ +static int qcom_28lpm_set_rate(struct qcom_hdmi_preqmp_phy *hdmi_phy, unsigned long parent_rate, + unsigned long vco_freq, u32 div_idx) +{ + unsigned int pixclk = hdmi_phy->hdmi_opts.tmds_char_rate; + unsigned int int_ref_freq; + unsigned int dc_offset; + unsigned int sdm_freq_seed; + unsigned int val; + bool sdm_mode = false; + u32 refclk_cfg; + u32 lf_cfg0; + u32 lf_cfg1; + u64 tmp; + + dev_dbg(hdmi_phy->dev, "rate=%u, div = %d, vco = %lu", pixclk, div_idx, vco_freq); + + if (vco_freq % (parent_rate / 2) == 0) { + refclk_cfg = FIELD_PREP(HDMI_8960_PHY_CLK0_DIV, 1); + int_ref_freq = parent_rate / 2; + } else { + refclk_cfg = HDMI_8960_PHY_DBLR_EN; + int_ref_freq = parent_rate * 2; + sdm_mode = true; + } + + dc_offset = vco_freq / int_ref_freq - 1; + tmp = vco_freq % int_ref_freq; + tmp *= 0x10000; + sdm_freq_seed = div_u64(tmp, int_ref_freq); + + val = FIELD_PREP(HDMI_8960_PHY_PLL_VCO_DIV, div_idx) | refclk_cfg; + writel(val, hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_REFCLK_CFG); + + lf_cfg0 = dc_offset >= 30 ? 0 : (dc_offset >= 16 ? 0x10 : 0x20); + lf_cfg0 += sdm_mode ? 0 : 1; + + /* XXX: 0xc3 instead of 0x33 for qcs404 */ + lf_cfg1 = dc_offset >= 30 ? 0x33 : (dc_offset >= 16 ? 0xbb : 0xf9); + + writel(lf_cfg0, + hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG0); + writel(lf_cfg1, + hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_LOOP_FLT_CFG1); + + if (sdm_mode) + writel(dc_offset, + hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG0); + else + writel(HDMI_8960_PHY_SDM_BYP | dc_offset, + hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG0); + + writel(HDMI_8960_PHY_DITHER | dc_offset, + hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG1); + + write24(sdm_freq_seed, hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG2); + + write16(vco_freq / 1000, hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0); + + writel(0x3b, hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2); + + return 0; +} + static const unsigned int qcom_hdmi_8960_divs[] = {1, 2, 4, 6}; static unsigned long qcom_hdmi_8960_pll_recalc_rate(struct clk_hw *hw, @@ -423,9 +282,10 @@ static unsigned long qcom_hdmi_8960_pll_recalc_rate(struct clk_hw *hw, static int qcom_hdmi_8960_pll_determine_rate(struct clk_hw *hw, struct clk_rate_request *req) { - const struct pll_rate *pll_rate = qcom_hdmi_8960_pll_find_rate(req->rate); + unsigned long long min_freq = HDMI_8960_VCO_MIN_FREQ / HDMI_8960_COMMON_DIV; + unsigned long long max_freq = HDMI_8960_VCO_MAX_FREQ / HDMI_8960_COMMON_DIV; - req->rate = pll_rate->rate; + req->rate = clamp(req->rate, min_freq / 6, max_freq); return 0; } @@ -510,16 +370,39 @@ static int qcom_hdmi_msm8960_phy_pll_enable(struct qcom_hdmi_preqmp_phy *hdmi_ph return ret; } +static int qcom_hdmi_msm8960_phy_find_div(unsigned long long pixclk) +{ + unsigned long long min_freq = HDMI_8960_VCO_MIN_FREQ / HDMI_8960_COMMON_DIV; + int i; + + if (pixclk > HDMI_8960_VCO_MAX_FREQ / HDMI_8960_COMMON_DIV) + return -E2BIG; + + for (i = 0; i < ARRAY_SIZE(qcom_hdmi_8960_divs); i++) { + if (pixclk >= min_freq / qcom_hdmi_8960_divs[i]) + return i; + } + + return -EINVAL; +} + static int qcom_hdmi_msm8960_phy_set_rate(struct qcom_hdmi_preqmp_phy *hdmi_phy) { unsigned long long pixclk = hdmi_phy->hdmi_opts.tmds_char_rate; - const struct pll_rate *pll_rate = qcom_hdmi_8960_pll_find_rate(pixclk); - int i; + /* XXX: 19.2 for qcs404 */ + unsigned long parent_rate = 27000000; + unsigned long vco_freq; + int div_idx; + u32 div; - for (i = 0; i < pll_rate->num_reg; i++) - writel(pll_rate->conf[i].val, hdmi_phy->pll_reg + pll_rate->conf[i].reg); + div_idx = qcom_hdmi_msm8960_phy_find_div(pixclk); + if (WARN_ON(div_idx < 0)) + return div_idx; - return 0; + div = qcom_hdmi_8960_divs[div_idx]; + vco_freq = pixclk * HDMI_8960_COMMON_DIV * div; + + return qcom_28lpm_set_rate(hdmi_phy, parent_rate, vco_freq, div_idx); } static void qcom_hdmi_msm8960_phy_pll_disable(struct qcom_hdmi_preqmp_phy *hdmi_phy) -- 2.47.3 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v9 2/5] phy: qualcomm: hdmi-28lpm: provide dynamic configuration 2026-05-13 18:14 ` [PATCH v9 2/5] phy: qualcomm: hdmi-28lpm: provide dynamic configuration Dmitry Baryshkov @ 2026-05-14 10:19 ` sashiko-bot 0 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-05-14 10:19 UTC (permalink / raw) To: Dmitry Baryshkov; +Cc: linux-phy, vkoul, neil.armstrong, olteanv Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] VCO calibration frequency is calculated with the wrong unit, causing severe truncation and corrupting calibration registers. - [High] Integer overflow in the `dc_offset` calculation for SDM bypass mode silently overlaps with control bits and severely misconfigures the PLL multiplier. -- commit 26d0a45f7fc0a0f6484ab8dc0e93e6b81f47871e Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> phy: qualcomm: hdmi-28lpm: provide dynamic configuration This commit replaces the fixed value configuration tables for the HDMI PHY with values calculated at runtime. > diff --git a/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c b/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c > index f1e7113e10bda..90d3331313c05 100644 > --- a/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c > +++ b/drivers/phy/qualcomm/phy-qcom-hdmi-28lpm.c [ ... ] > @@ -407,6 +202,70 @@ static unsigned long qcom_28lpm_recalc(struct qcom_hdmi_preqmp_phy *hdmi_phy, > return rate; > } > > +/* This function is close to UNIPHY, but it has slighly different fields */ > +static int qcom_28lpm_set_rate(struct qcom_hdmi_preqmp_phy *hdmi_phy, unsigned long parent_rate, > + unsigned long vco_freq, u32 div_idx) > +{ > + unsigned int pixclk = hdmi_phy->hdmi_opts.tmds_char_rate; > + unsigned int int_ref_freq; > + unsigned int dc_offset; > + unsigned int sdm_freq_seed; > + unsigned int val; > + bool sdm_mode = false; > + u32 refclk_cfg; > + u32 lf_cfg0; > + u32 lf_cfg1; > + u64 tmp; > + > + dev_dbg(hdmi_phy->dev, "rate=%u, div = %d, vco = %lu", pixclk, div_idx, vco_freq); > + > + if (vco_freq % (parent_rate / 2) == 0) { > + refclk_cfg = FIELD_PREP(HDMI_8960_PHY_CLK0_DIV, 1); > + int_ref_freq = parent_rate / 2; > + } else { > + refclk_cfg = HDMI_8960_PHY_DBLR_EN; > + int_ref_freq = parent_rate * 2; > + sdm_mode = true; > + } > + > + dc_offset = vco_freq / int_ref_freq - 1; Could dc_offset overflow the 6-bit hardware field in SDM bypass mode? If a high VCO frequency is requested (e.g., a 216 MHz pixel clock yielding a 1080 MHz VCO), int_ref_freq is 13.5 MHz. The calculation for dc_offset becomes 1080 / 13.5 - 1 = 79 (0x4F). [ ... ] > + if (sdm_mode) > + writel(dc_offset, > + hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG0); > + else > + writel(HDMI_8960_PHY_SDM_BYP | dc_offset, > + hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG0); This value exceeds the 6-bit boundary. Bit 6 (0x40) overlaps directly with the HDMI_8960_PHY_SDM_BYP control bit, so the hardware divider would only receive the truncated lower 5 bits (0x0F = 15). The hardware would therefore synthesize a VCO of 216 MHz instead of the expected 1080 MHz. Does the bypass selection logic earlier need an additional constraint to fall back to fractional SDM mode when the divider limit is exceeded? > + > + writel(HDMI_8960_PHY_DITHER | dc_offset, > + hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG1); > + > + write24(sdm_freq_seed, hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_SDM_CFG2); > + > + write16(vco_freq / 1000, hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_VCOCAL_CFG0); Is dividing by 1000 yielding kHz instead of the expected MHz here? Since vco_freq is in Hz, for a typical 1080p pixel clock (148.5 MHz), vco_freq is 742,500,000 Hz. Dividing by 1000 results in 742,500 (0x0B5454). Because write16 takes a u16 argument, this value would silently truncate to 0x5454, writing 0x54 to both VCOCAL_CFG0 and VCOCAL_CFG1. The previous fixed configuration tables explicitly programmed 0x02E6 (742) for this resolution, indicating the hardware expects the value in MHz. Should this calculation be vco_freq / 1000000 instead? > + > + writel(0x3b, hdmi_phy->pll_reg + REG_HDMI_8960_PHY_PLL_VCOCAL_CFG2); > + > + return 0; > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/20260513-fd-hdmi-phy-v9-0-ca98c72f1f9f@oss.qualcomm.com?part=2 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v9 3/5] phy: qcom: apq8064-sata: extract UNI PLL register defines 2026-05-13 18:14 [PATCH v9 0/5] drm/msm/hdmi & phy: use generic PHY framework Dmitry Baryshkov 2026-05-13 18:14 ` [PATCH v9 2/5] phy: qualcomm: hdmi-28lpm: provide dynamic configuration Dmitry Baryshkov @ 2026-05-13 18:14 ` Dmitry Baryshkov 2026-05-14 10:23 ` sashiko-bot 2026-05-13 18:14 ` [PATCH v9 4/5] phy: qcom-uniphy: add more registers from display PHYs Dmitry Baryshkov 2026-05-13 18:14 ` [PATCH v9 5/5] phy: qualcomm: add MSM8974 HDMI PHY support Dmitry Baryshkov 3 siblings, 1 reply; 8+ messages in thread From: Dmitry Baryshkov @ 2026-05-13 18:14 UTC (permalink / raw) To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Vinod Koul, Neil Armstrong Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno, linux-phy, Dmitry Baryshkov From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> The "uni" PLL is shared between several PHYS: APQ8064's SATA, MSM8974/APQ8084 HDMI, MSM8916 DSI, MSM8974/APQ8084 DSI. Extract the common register names to a separate header with the hope of later having the common code to handle PLL in those PHYs. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- drivers/phy/qualcomm/phy-qcom-apq8064-sata.c | 24 +------------------- drivers/phy/qualcomm/phy-qcom-uniphy.h | 33 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/drivers/phy/qualcomm/phy-qcom-apq8064-sata.c b/drivers/phy/qualcomm/phy-qcom-apq8064-sata.c index cae290a6e19f..52f180e0cfb4 100644 --- a/drivers/phy/qualcomm/phy-qcom-apq8064-sata.c +++ b/drivers/phy/qualcomm/phy-qcom-apq8064-sata.c @@ -15,28 +15,7 @@ #include <linux/platform_device.h> #include <linux/phy/phy.h> -/* PHY registers */ -#define UNIPHY_PLL_REFCLK_CFG 0x000 -#define UNIPHY_PLL_PWRGEN_CFG 0x014 -#define UNIPHY_PLL_GLB_CFG 0x020 -#define UNIPHY_PLL_SDM_CFG0 0x038 -#define UNIPHY_PLL_SDM_CFG1 0x03C -#define UNIPHY_PLL_SDM_CFG2 0x040 -#define UNIPHY_PLL_SDM_CFG3 0x044 -#define UNIPHY_PLL_SDM_CFG4 0x048 -#define UNIPHY_PLL_SSC_CFG0 0x04C -#define UNIPHY_PLL_SSC_CFG1 0x050 -#define UNIPHY_PLL_SSC_CFG2 0x054 -#define UNIPHY_PLL_SSC_CFG3 0x058 -#define UNIPHY_PLL_LKDET_CFG0 0x05C -#define UNIPHY_PLL_LKDET_CFG1 0x060 -#define UNIPHY_PLL_LKDET_CFG2 0x064 -#define UNIPHY_PLL_CAL_CFG0 0x06C -#define UNIPHY_PLL_CAL_CFG8 0x08C -#define UNIPHY_PLL_CAL_CFG9 0x090 -#define UNIPHY_PLL_CAL_CFG10 0x094 -#define UNIPHY_PLL_CAL_CFG11 0x098 -#define UNIPHY_PLL_STATUS 0x0C0 +#include "phy-qcom-uniphy.h" #define SATA_PHY_SER_CTRL 0x100 #define SATA_PHY_TX_DRIV_CTRL0 0x104 @@ -58,7 +37,6 @@ #define SATA_PHY_TX_IMCAL_STAT 0x1E4 #define SATA_PHY_RX_IMCAL_STAT 0x1E8 -#define UNIPHY_PLL_LOCK BIT(0) #define SATA_PHY_TX_CAL BIT(0) #define SATA_PHY_RX_CAL BIT(0) diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy.h b/drivers/phy/qualcomm/phy-qcom-uniphy.h new file mode 100644 index 000000000000..5f18f9593cda --- /dev/null +++ b/drivers/phy/qualcomm/phy-qcom-uniphy.h @@ -0,0 +1,33 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2014, The Linux Foundation. All rights reserved. + */ + +#ifndef PHY_QCOM_UNIPHY_H +#define PHY_QCOM_UNIPHY_H + +/* PHY registers */ +#define UNIPHY_PLL_REFCLK_CFG 0x000 +#define UNIPHY_PLL_PWRGEN_CFG 0x014 +#define UNIPHY_PLL_GLB_CFG 0x020 +#define UNIPHY_PLL_SDM_CFG0 0x038 +#define UNIPHY_PLL_SDM_CFG1 0x03c +#define UNIPHY_PLL_SDM_CFG2 0x040 +#define UNIPHY_PLL_SDM_CFG3 0x044 +#define UNIPHY_PLL_SDM_CFG4 0x048 +#define UNIPHY_PLL_SSC_CFG0 0x04c +#define UNIPHY_PLL_SSC_CFG1 0x050 +#define UNIPHY_PLL_SSC_CFG2 0x054 +#define UNIPHY_PLL_SSC_CFG3 0x058 +#define UNIPHY_PLL_LKDET_CFG0 0x05c +#define UNIPHY_PLL_LKDET_CFG1 0x060 +#define UNIPHY_PLL_LKDET_CFG2 0x064 +#define UNIPHY_PLL_CAL_CFG0 0x06c +#define UNIPHY_PLL_CAL_CFG8 0x08c +#define UNIPHY_PLL_CAL_CFG9 0x090 +#define UNIPHY_PLL_CAL_CFG10 0x094 +#define UNIPHY_PLL_CAL_CFG11 0x098 +#define UNIPHY_PLL_STATUS 0x0c0 +#define UNIPHY_PLL_LOCK BIT(0) + +#endif -- 2.47.3 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v9 3/5] phy: qcom: apq8064-sata: extract UNI PLL register defines 2026-05-13 18:14 ` [PATCH v9 3/5] phy: qcom: apq8064-sata: extract UNI PLL register defines Dmitry Baryshkov @ 2026-05-14 10:23 ` sashiko-bot 0 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-05-14 10:23 UTC (permalink / raw) To: Dmitry Baryshkov; +Cc: linux-phy, olteanv, neil.armstrong, vkoul Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The newly added header file `phy-qcom-uniphy.h` uses the `BIT()` macro but does not include `<linux/bits.h>`. -- commit a5da58070a4428e27ba521e2ed52d6c1d0de97b5 Author: Dmitry Baryshkov <lumag@kernel.org> phy: qcom: apq8064-sata: extract UNI PLL register defines The "uni" PLL is shared between several PHYS. Extract the common register names to a separate header with the hope of later having the common code to handle PLL in those PHYs. > diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy.h b/drivers/phy/qualcomm/phy-qcom-uniphy.h > new file mode 100644 > index 0000000000000..5f18f9593cda7 > --- /dev/null > +++ b/drivers/phy/qualcomm/phy-qcom-uniphy.h > @@ -0,0 +1,33 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2014, The Linux Foundation. All rights reserved. > + */ > + > +#ifndef PHY_QCOM_UNIPHY_H > +#define PHY_QCOM_UNIPHY_H [ ... ] > +#define UNIPHY_PLL_STATUS 0x0c0 > +#define UNIPHY_PLL_LOCK BIT(0) Since this header file uses the BIT() macro, should it include <linux/bits.h> to ensure it is self-contained? Relying on implicit inclusions from the C files that include this header might cause compilation errors in the future if those files do not already have the macro defined in their include chain. > + > +#endif -- Sashiko AI review · https://sashiko.dev/#/patchset/20260513-fd-hdmi-phy-v9-0-ca98c72f1f9f@oss.qualcomm.com?part=3 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v9 4/5] phy: qcom-uniphy: add more registers from display PHYs 2026-05-13 18:14 [PATCH v9 0/5] drm/msm/hdmi & phy: use generic PHY framework Dmitry Baryshkov 2026-05-13 18:14 ` [PATCH v9 2/5] phy: qualcomm: hdmi-28lpm: provide dynamic configuration Dmitry Baryshkov 2026-05-13 18:14 ` [PATCH v9 3/5] phy: qcom: apq8064-sata: extract UNI PLL register defines Dmitry Baryshkov @ 2026-05-13 18:14 ` Dmitry Baryshkov 2026-05-13 18:14 ` [PATCH v9 5/5] phy: qualcomm: add MSM8974 HDMI PHY support Dmitry Baryshkov 3 siblings, 0 replies; 8+ messages in thread From: Dmitry Baryshkov @ 2026-05-13 18:14 UTC (permalink / raw) To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Vinod Koul, Neil Armstrong Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno, linux-phy, Dmitry Baryshkov From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Import register definitions from 28nm DSI and HDMI PHYs, adding more UNI PHY registers. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- drivers/phy/qualcomm/phy-qcom-uniphy.h | 43 ++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy.h b/drivers/phy/qualcomm/phy-qcom-uniphy.h index 5f18f9593cda..e02bcc5e8150 100644 --- a/drivers/phy/qualcomm/phy-qcom-uniphy.h +++ b/drivers/phy/qualcomm/phy-qcom-uniphy.h @@ -8,10 +8,30 @@ /* PHY registers */ #define UNIPHY_PLL_REFCLK_CFG 0x000 +#define UNIPHY_PLL_REFCLK_DBLR BIT(0) +#define UNIPHY_PLL_REFCLK_DIV GENMASK(3, 2) + +#define UNIPHY_PLL_POSTDIV1_CFG 0x004 +#define UNIPHY_PLL_CHGPUMP_CFG 0x008 +#define UNIPHY_PLL_VCOLPF_CFG 0x00c +#define UNIPHY_PLL_VREG_CFG 0x010 #define UNIPHY_PLL_PWRGEN_CFG 0x014 +#define UNIPHY_PLL_DMUX_CFG 0x018 +#define UNIPHY_PLL_AMUX_CFG 0x01c #define UNIPHY_PLL_GLB_CFG 0x020 +#define UNIPHY_PLL_POSTDIV2_CFG 0x024 +#define UNIPHY_PLL_POSTDIV3_CFG 0x028 +#define UNIPHY_PLL_LPFR_CFG 0x02c +#define UNIPHY_PLL_LPFC1_CFG 0x030 +#define UNIPHY_PLL_LPFC2_CFG 0x034 #define UNIPHY_PLL_SDM_CFG0 0x038 +#define UNIPHY_PLL_SDM_BYP BIT(6) +#define UNIPHY_PLL_SDM_BYP_DIV GENMASK(5, 0) + #define UNIPHY_PLL_SDM_CFG1 0x03c +#define UNIPHY_PLL_SDM_DITHER_EN BIT(6) +#define UNIPHY_PLL_SDM_DC_OFFSET GENMASK(5, 0) + #define UNIPHY_PLL_SDM_CFG2 0x040 #define UNIPHY_PLL_SDM_CFG3 0x044 #define UNIPHY_PLL_SDM_CFG4 0x048 @@ -22,12 +42,35 @@ #define UNIPHY_PLL_LKDET_CFG0 0x05c #define UNIPHY_PLL_LKDET_CFG1 0x060 #define UNIPHY_PLL_LKDET_CFG2 0x064 +#define UNIPHY_PLL_TEST_CFG 0x068 #define UNIPHY_PLL_CAL_CFG0 0x06c +#define UNIPHY_PLL_CAL_CFG1 0x070 +#define UNIPHY_PLL_CAL_CFG2 0x074 +#define UNIPHY_PLL_CAL_CFG3 0x078 +#define UNIPHY_PLL_CAL_CFG4 0x07c +#define UNIPHY_PLL_CAL_CFG5 0x080 +#define UNIPHY_PLL_CAL_CFG6 0x084 +#define UNIPHY_PLL_CAL_CFG7 0x088 #define UNIPHY_PLL_CAL_CFG8 0x08c #define UNIPHY_PLL_CAL_CFG9 0x090 #define UNIPHY_PLL_CAL_CFG10 0x094 #define UNIPHY_PLL_CAL_CFG11 0x098 +#define UNIPHY_PLL_EFUSE_CFG 0x09c +#define UNIPHY_PLL_DEBUG_BUS_SEL 0x0a0 +#define UNIPHY_PLL_CTRL_42 0x0a4 +#define UNIPHY_PLL_CTRL_43 0x0a8 +#define UNIPHY_PLL_CTRL_44 0x0ac +#define UNIPHY_PLL_CTRL_45 0x0b0 +#define UNIPHY_PLL_CTRL_46 0x0b4 +#define UNIPHY_PLL_CTRL_47 0x0b8 +#define UNIPHY_PLL_CTRL_48 0x0bc #define UNIPHY_PLL_STATUS 0x0c0 #define UNIPHY_PLL_LOCK BIT(0) +#define UNIPHY_PLL_DEBUG_BUS0 0x0c4 +#define UNIPHY_PLL_DEBUG_BUS1 0x0c8 +#define UNIPHY_PLL_DEBUG_BUS2 0x0cc +#define UNIPHY_PLL_DEBUG_BUS3 0x0d0 +#define UNIPHY_PLL_CTRL_54 0x0d4 + #endif -- 2.47.3 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v9 5/5] phy: qualcomm: add MSM8974 HDMI PHY support 2026-05-13 18:14 [PATCH v9 0/5] drm/msm/hdmi & phy: use generic PHY framework Dmitry Baryshkov ` (2 preceding siblings ...) 2026-05-13 18:14 ` [PATCH v9 4/5] phy: qcom-uniphy: add more registers from display PHYs Dmitry Baryshkov @ 2026-05-13 18:14 ` Dmitry Baryshkov 2026-05-14 11:14 ` sashiko-bot 3 siblings, 1 reply; 8+ messages in thread From: Dmitry Baryshkov @ 2026-05-13 18:14 UTC (permalink / raw) To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie, Simona Vetter, Vinod Koul, Neil Armstrong Cc: linux-kernel, linux-arm-msm, dri-devel, freedreno, linux-phy, Dmitry Baryshkov, Konrad Dybcio Add support for HDMI PHY on Qualcomm MSM8974 / APQ8074 platforms. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Acked-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> --- drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c | 282 +++++++++++++++++++++++++++++ 1 file changed, 282 insertions(+) diff --git a/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c b/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c index 720757f8f393..7d398060b3a3 100644 --- a/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c +++ b/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c @@ -6,10 +6,13 @@ * Author: Rob Clark <robdclark@gmail.com> */ +#include <linux/bitfield.h> #include <linux/delay.h> #include <linux/iopoll.h> +#include <linux/math64.h> #include "phy-qcom-hdmi-preqmp.h" +#include "phy-qcom-uniphy.h" #define REG_HDMI_8x74_ANA_CFG0 0x00000000 #define REG_HDMI_8x74_ANA_CFG1 0x00000004 @@ -31,8 +34,282 @@ #define REG_HDMI_8x74_BIST_PATN3 0x00000048 #define REG_HDMI_8x74_STATUS 0x0000005c +#define HDMI_8974_VCO_MAX_FREQ 1800000000UL +#define HDMI_8974_VCO_MIN_FREQ 600000000UL + +#define HDMI_8974_COMMON_DIV 5 + +static inline void write16(u16 val, void __iomem *reg) +{ + writel(val & 0xff, reg); + writel(val >> 8, reg + 4); +} + +static inline void write24(u32 val, void __iomem *reg) +{ + writel(val & 0xff, reg); + writel((val >> 8) & 0xff, reg + 4); + writel(val >> 16, reg + 8); +} + +static inline u32 read24(void __iomem *reg) +{ + u32 val = readl(reg); + + val |= readl(reg + 4) << 8; + val |= readl(reg + 8) << 16; + + return val; +} + +static void qcom_uniphy_setup(void __iomem *base, unsigned int ref_freq, + bool sdm_mode, + bool ref_freq_mult_2, + bool dither, + unsigned int refclk_div, + unsigned int vco_freq) +{ + unsigned int int_ref_freq = ref_freq * (ref_freq_mult_2 ? 2 : 1); + unsigned int div_in_freq = vco_freq / refclk_div; + unsigned int dc_offset = div_in_freq / int_ref_freq - 1; + unsigned int sdm_freq_seed; + unsigned int val; + u64 tmp = div_in_freq % int_ref_freq; + + tmp *= 0x10000; + sdm_freq_seed = div_u64(tmp, int_ref_freq); + + val = FIELD_PREP(UNIPHY_PLL_REFCLK_DBLR, ref_freq_mult_2) | + FIELD_PREP(UNIPHY_PLL_REFCLK_DIV, refclk_div - 1); + writel(val, base + UNIPHY_PLL_REFCLK_CFG); + + if (sdm_mode) { + writel(0, base + UNIPHY_PLL_SDM_CFG0); + writel(FIELD_PREP(UNIPHY_PLL_SDM_DITHER_EN, dither) | dc_offset, + base + UNIPHY_PLL_SDM_CFG1); + write24(sdm_freq_seed, base + UNIPHY_PLL_SDM_CFG2); + } else { + writel(UNIPHY_PLL_SDM_BYP | dc_offset, base + UNIPHY_PLL_SDM_CFG0); + writel(0, base + UNIPHY_PLL_SDM_CFG1); + write24(0, base + UNIPHY_PLL_SDM_CFG2); + } + + write16(mult_frac(ref_freq, 5, 1000000), base + UNIPHY_PLL_CAL_CFG8); + write16(vco_freq / 16, base + UNIPHY_PLL_CAL_CFG10); +} + +static unsigned long qcom_uniphy_recalc(void __iomem *base, unsigned long parent_rate) +{ + unsigned long rate; + u32 refclk_cfg; + u32 dc_offset; + u64 fraq_n; + u32 val; + + refclk_cfg = readl(base + UNIPHY_PLL_REFCLK_CFG); + if (refclk_cfg & UNIPHY_PLL_REFCLK_DBLR) + parent_rate *= 2; + + val = readl(base + UNIPHY_PLL_SDM_CFG0); + if (FIELD_GET(UNIPHY_PLL_SDM_BYP, val)) { + dc_offset = FIELD_GET(UNIPHY_PLL_SDM_BYP_DIV, val); + fraq_n = 0; + } else { + dc_offset = FIELD_GET(UNIPHY_PLL_SDM_DC_OFFSET, + readl(base + UNIPHY_PLL_SDM_CFG1)); + fraq_n = read24(base + UNIPHY_PLL_SDM_CFG2); + } + + rate = (dc_offset + 1) * parent_rate; + rate += div_u64(fraq_n * parent_rate, 0x10000); + + rate *= FIELD_GET(UNIPHY_PLL_REFCLK_DIV, refclk_cfg) + 1; + + return rate; +} + +static const unsigned int qcom_hdmi_8974_divs[] = {1, 2, 4, 6}; + +static unsigned long qcom_hdmi_8974_pll_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct qcom_hdmi_preqmp_phy *hdmi_phy = hw_clk_to_phy(hw); + u32 div_idx = readl(hdmi_phy->pll_reg + UNIPHY_PLL_POSTDIV1_CFG); + unsigned long rate = qcom_uniphy_recalc(hdmi_phy->pll_reg, parent_rate); + + return rate / HDMI_8974_COMMON_DIV / qcom_hdmi_8974_divs[div_idx & 0x3]; +} + +static int qcom_hdmi_8974_pll_determine_rate(struct clk_hw *hw, + struct clk_rate_request *req) +{ + unsigned long min_freq = HDMI_8974_VCO_MIN_FREQ / HDMI_8974_COMMON_DIV; + unsigned long max_freq = HDMI_8974_VCO_MAX_FREQ / HDMI_8974_COMMON_DIV; + + req->rate = clamp(req->rate, min_freq / 6, max_freq); + + return 0; +} + +static const struct clk_ops qcom_hdmi_8974_pll_ops = { + .recalc_rate = qcom_hdmi_8974_pll_recalc_rate, + .determine_rate = qcom_hdmi_8974_pll_determine_rate, +}; + +static int qcom_hdmi_msm8974_phy_find_div(unsigned long long pixclk) +{ + unsigned long long min_freq = HDMI_8974_VCO_MIN_FREQ / HDMI_8974_COMMON_DIV; + int i; + + if (pixclk > HDMI_8974_VCO_MAX_FREQ / HDMI_8974_COMMON_DIV) + return -EINVAL; + + for (i = 0; i < ARRAY_SIZE(qcom_hdmi_8974_divs); i++) { + if (pixclk >= min_freq / qcom_hdmi_8974_divs[i]) + return i; + } + + return -EINVAL; +} + +static int qcom_hdmi_msm8974_phy_pll_set_rate(struct qcom_hdmi_preqmp_phy *hdmi_phy) +{ + unsigned long long pixclk = hdmi_phy->hdmi_opts.tmds_char_rate; + unsigned long vco_rate; + unsigned int div; + int div_idx = 0; + + div_idx = qcom_hdmi_msm8974_phy_find_div(pixclk); + if (WARN_ON(div_idx < 0)) + return div_idx; + + div = qcom_hdmi_8974_divs[div_idx]; + vco_rate = pixclk * HDMI_8974_COMMON_DIV * div; + + writel(0x81, hdmi_phy->phy_reg + REG_HDMI_8x74_GLB_CFG); + + writel(0x01, hdmi_phy->pll_reg + UNIPHY_PLL_GLB_CFG); + writel(0x19, hdmi_phy->pll_reg + UNIPHY_PLL_VCOLPF_CFG); + writel(0x0e, hdmi_phy->pll_reg + UNIPHY_PLL_LPFR_CFG); + writel(0x20, hdmi_phy->pll_reg + UNIPHY_PLL_LPFC1_CFG); + writel(0x0d, hdmi_phy->pll_reg + UNIPHY_PLL_LPFC2_CFG); + + qcom_uniphy_setup(hdmi_phy->pll_reg, 19200000, true, true, true, 1, vco_rate); + + writel(0x10, hdmi_phy->pll_reg + UNIPHY_PLL_LKDET_CFG0); + writel(0x1a, hdmi_phy->pll_reg + UNIPHY_PLL_LKDET_CFG1); + writel(0x05, hdmi_phy->pll_reg + UNIPHY_PLL_LKDET_CFG2); + + writel(div_idx, hdmi_phy->pll_reg + UNIPHY_PLL_POSTDIV1_CFG); + + writel(0x00, hdmi_phy->pll_reg + UNIPHY_PLL_POSTDIV2_CFG); + writel(0x00, hdmi_phy->pll_reg + UNIPHY_PLL_POSTDIV3_CFG); + writel(0x01, hdmi_phy->pll_reg + UNIPHY_PLL_CAL_CFG2); + + writel(0x1f, hdmi_phy->phy_reg + REG_HDMI_8x74_PD_CTRL0); + udelay(50); + + writel(0x0f, hdmi_phy->pll_reg + UNIPHY_PLL_GLB_CFG); + + writel(0x00, hdmi_phy->phy_reg + REG_HDMI_8x74_PD_CTRL1); + writel(0x10, hdmi_phy->phy_reg + REG_HDMI_8x74_ANA_CFG2); + writel(0xdb, hdmi_phy->phy_reg + REG_HDMI_8x74_ANA_CFG0); + writel(0x43, hdmi_phy->phy_reg + REG_HDMI_8x74_ANA_CFG1); + if (pixclk == 297000000) { + writel(0x06, hdmi_phy->phy_reg + REG_HDMI_8x74_ANA_CFG2); + writel(0x03, hdmi_phy->phy_reg + REG_HDMI_8x74_ANA_CFG3); + } else if (pixclk == 268500000) { + writel(0x05, hdmi_phy->phy_reg + REG_HDMI_8x74_ANA_CFG2); + writel(0x00, hdmi_phy->phy_reg + REG_HDMI_8x74_ANA_CFG3); + } else { + writel(0x02, hdmi_phy->phy_reg + REG_HDMI_8x74_ANA_CFG2); + writel(0x00, hdmi_phy->phy_reg + REG_HDMI_8x74_ANA_CFG3); + } + + writel(0x04, hdmi_phy->pll_reg + UNIPHY_PLL_VREG_CFG); + + writel(0xd0, hdmi_phy->phy_reg + REG_HDMI_8x74_DCC_CFG0); + writel(0x1a, hdmi_phy->phy_reg + REG_HDMI_8x74_DCC_CFG1); + writel(0x00, hdmi_phy->phy_reg + REG_HDMI_8x74_TXCAL_CFG0); + writel(0x00, hdmi_phy->phy_reg + REG_HDMI_8x74_TXCAL_CFG1); + + if (pixclk == 268500000) + writel(0x11, hdmi_phy->phy_reg + REG_HDMI_8x74_TXCAL_CFG2); + else + writel(0x02, hdmi_phy->phy_reg + REG_HDMI_8x74_TXCAL_CFG2); + + writel(0x05, hdmi_phy->phy_reg + REG_HDMI_8x74_TXCAL_CFG3); + udelay(200); + + return 0; +} + +static int qcom_hdmi_msm8974_phy_pll_enable(struct qcom_hdmi_preqmp_phy *hdmi_phy) +{ + int ret; + unsigned long status; + + /* Global enable */ + writel(0x81, hdmi_phy->phy_reg + REG_HDMI_8x74_GLB_CFG); + + /* Power up power gen */ + writel(0x00, hdmi_phy->phy_reg + REG_HDMI_8x74_PD_CTRL0); + udelay(350); + + /* PLL power up */ + writel(0x01, hdmi_phy->pll_reg + UNIPHY_PLL_GLB_CFG); + udelay(5); + + /* Power up PLL LDO */ + writel(0x03, hdmi_phy->pll_reg + UNIPHY_PLL_GLB_CFG); + udelay(350); + + /* PLL power up */ + writel(0x0f, hdmi_phy->pll_reg + UNIPHY_PLL_GLB_CFG); + udelay(350); + + /* Poll for PLL ready status */ + ret = readl_poll_timeout(hdmi_phy->pll_reg + UNIPHY_PLL_STATUS, + status, status & UNIPHY_PLL_LOCK, + 100, 2000); + if (ret) { + dev_warn(hdmi_phy->dev, "HDMI PLL not ready\n"); + goto err; + } + + udelay(350); + + /* Poll for PHY ready status */ + ret = readl_poll_timeout(hdmi_phy->phy_reg + REG_HDMI_8x74_STATUS, + status, status & BIT(0), + 100, 2000); + if (ret) { + dev_warn(hdmi_phy->dev, "HDMI PHY not ready\n"); + goto err; + } + + return 0; + +err: + writel(0, hdmi_phy->pll_reg + UNIPHY_PLL_GLB_CFG); + udelay(5); + writel(0, hdmi_phy->phy_reg + REG_HDMI_8x74_GLB_CFG); + + return ret; +} + static int qcom_hdmi_msm8974_phy_power_on(struct qcom_hdmi_preqmp_phy *hdmi_phy) { + int ret; + + ret = qcom_hdmi_msm8974_phy_pll_set_rate(hdmi_phy); + if (ret) + return ret; + + ret = qcom_hdmi_msm8974_phy_pll_enable(hdmi_phy); + if (ret) + return ret; + writel(0x1b, hdmi_phy->phy_reg + REG_HDMI_8x74_ANA_CFG0); writel(0xf2, hdmi_phy->phy_reg + REG_HDMI_8x74_ANA_CFG1); writel(0x00, hdmi_phy->phy_reg + REG_HDMI_8x74_BIST_CFG0); @@ -49,6 +326,10 @@ static int qcom_hdmi_msm8974_phy_power_off(struct qcom_hdmi_preqmp_phy *hdmi_phy { writel(0x7f, hdmi_phy->phy_reg + REG_HDMI_8x74_PD_CTRL0); + writel(0, hdmi_phy->pll_reg + UNIPHY_PLL_GLB_CFG); + udelay(5); + writel(0, hdmi_phy->phy_reg + REG_HDMI_8x74_GLB_CFG); + return 0; } @@ -67,5 +348,6 @@ const struct qcom_hdmi_preqmp_cfg msm8974_hdmi_phy_cfg = { .power_on = qcom_hdmi_msm8974_phy_power_on, .power_off = qcom_hdmi_msm8974_phy_power_off, + .pll_ops = &qcom_hdmi_8974_pll_ops, .pll_parent = &msm8974_hdmi_pll_parent, }; -- 2.47.3 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v9 5/5] phy: qualcomm: add MSM8974 HDMI PHY support 2026-05-13 18:14 ` [PATCH v9 5/5] phy: qualcomm: add MSM8974 HDMI PHY support Dmitry Baryshkov @ 2026-05-14 11:14 ` sashiko-bot 0 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-05-14 11:14 UTC (permalink / raw) To: Dmitry Baryshkov; +Cc: linux-phy, neil.armstrong, vkoul, olteanv Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [High] Integer truncation when passing vco_freq to write16() writes garbage calibration data - [High] Concurrent access to PLL registers between clock and PHY frameworks without a shared lock - [High] User-controllable input triggers a WARN_ON(), enabling a kernel panic DoS - [Medium] Error path in qcom_hdmi_msm8974_phy_pll_enable() leaks power by failing to restore PD_CTRL0 - [Medium] The read24() helper lacks bitmasks, leading to potential data corruption from upper register bits -- commit d2fb8e86ab045b987f00fdf674f81b4bc2d4b950 Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> phy: qualcomm: add MSM8974 HDMI PHY support Add support for HDMI PHY on Qualcomm MSM8974 / APQ8074 platforms. > diff --git a/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c b/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c > index 720757f8f3931..7d398060b3a3c 100644 > --- a/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c > +++ b/drivers/phy/qualcomm/phy-qcom-hdmi-28hpm.c [ ... ] > @@ -31,8 +34,282 @@ > #define REG_HDMI_8x74_BIST_PATN3 0x00000048 > #define REG_HDMI_8x74_STATUS 0x0000005c [ ... ] > +static inline u32 read24(void __iomem *reg) > +{ > + u32 val = readl(reg); > + > + val |= readl(reg + 4) << 8; > + val |= readl(reg + 8) << 16; Can this result in data corruption if the hardware bus returns non-zero upper bits for these registers? Unlike write24() which correctly masks its values with & 0xff, the readl() results are not masked before bit-shifting. If the hardware returns reserved or uninitialized upper bits, they will bleed into and corrupt the assembled 24-bit value used in qcom_uniphy_recalc(). > + > + return val; > +} > + > +static void qcom_uniphy_setup(void __iomem *base, unsigned int ref_freq, > + bool sdm_mode, > + bool ref_freq_mult_2, > + bool dither, > + unsigned int refclk_div, > + unsigned int vco_freq) > +{ [ ... ] > + write16(mult_frac(ref_freq, 5, 1000000), base + UNIPHY_PLL_CAL_CFG8); > + write16(vco_freq / 16, base + UNIPHY_PLL_CAL_CFG10); Will this cause integer truncation and misconfigure the PLL calibration? The vco_freq parameter can be up to 1.8 GHz, meaning vco_freq / 16 can yield a value up to ~112.5 million, which requires 27 bits. Since write16() takes a u16 parameter, the upper 11 bits are silently discarded, and only the lower 16 bits are written to the CFG10 and CFG11 registers. > +} > + > +static unsigned long qcom_uniphy_recalc(void __iomem *base, unsigned long parent_rate) > +{ > + unsigned long rate; > + u32 refclk_cfg; > + u32 dc_offset; > + u64 fraq_n; > + u32 val; > + > + refclk_cfg = readl(base + UNIPHY_PLL_REFCLK_CFG); [ ... ] > + val = readl(base + UNIPHY_PLL_SDM_CFG0); > + if (FIELD_GET(UNIPHY_PLL_SDM_BYP, val)) { > + dc_offset = FIELD_GET(UNIPHY_PLL_SDM_BYP_DIV, val); > + fraq_n = 0; > + } else { > + dc_offset = FIELD_GET(UNIPHY_PLL_SDM_DC_OFFSET, > + readl(base + UNIPHY_PLL_SDM_CFG1)); > + fraq_n = read24(base + UNIPHY_PLL_SDM_CFG2); > + } Does this code race with qcom_hdmi_msm8974_phy_power_on()? The PHY driver exposes a clock provider, and this recalc_rate callback reads shared PLL hardware registers protected by the clock framework's prepare_lock. However, qcom_hdmi_msm8974_phy_power_on() writes to these exact same PLL registers while holding an unrelated PHY mutex. Without a shared lock synchronizing these paths, reading from the clk_summary debugfs file concurrently with a PHY power-on could cause torn reads, especially since the 24-bit fractional divider is written across three separate writel() calls. [ ... ] > +static int qcom_hdmi_msm8974_phy_pll_set_rate(struct qcom_hdmi_preqmp_phy *hdmi_phy) > +{ > + unsigned long long pixclk = hdmi_phy->hdmi_opts.tmds_char_rate; > + unsigned long vco_rate; > + unsigned int div; > + int div_idx = 0; > + > + div_idx = qcom_hdmi_msm8974_phy_find_div(pixclk); > + if (WARN_ON(div_idx < 0)) > + return div_idx; Is it safe to use a WARN_ON() for user-controllable input? The requested pixel clock (hdmi_opts.tmds_char_rate) is influenced by DRM modesetting or an external monitor's EDID. An unprivileged user could trigger this path by requesting an unsupported display mode, intentionally hitting this WARN_ON(). On systems configured with panic_on_warn=1, this would crash the kernel, potentially leading to a local denial of service. [ ... ] > +static int qcom_hdmi_msm8974_phy_pll_enable(struct qcom_hdmi_preqmp_phy *hdmi_phy) > +{ > + int ret; > + unsigned long status; > + > + /* Global enable */ > + writel(0x81, hdmi_phy->phy_reg + REG_HDMI_8x74_GLB_CFG); > + > + /* Power up power gen */ > + writel(0x00, hdmi_phy->phy_reg + REG_HDMI_8x74_PD_CTRL0); > + udelay(350); [ ... ] > + /* Poll for PHY ready status */ > + ret = readl_poll_timeout(hdmi_phy->phy_reg + REG_HDMI_8x74_STATUS, > + status, status & BIT(0), > + 100, 2000); > + if (ret) { > + dev_warn(hdmi_phy->dev, "HDMI PHY not ready\n"); > + goto err; > + } > + > + return 0; > + > +err: > + writel(0, hdmi_phy->pll_reg + UNIPHY_PLL_GLB_CFG); > + udelay(5); > + writel(0, hdmi_phy->phy_reg + REG_HDMI_8x74_GLB_CFG); > + > + return ret; > +} Does this error path leave the PHY partially powered up? During the power-on sequence, the PHY is powered up by writing to REG_HDMI_8x74_PD_CTRL0. If the subsequent polling fails, the code jumps to the err label. The error path clears the global configuration registers but fails to write 0x7f back to REG_HDMI_8x74_PD_CTRL0 to power down the generator. Because the function returns an error, the PHY framework will not invoke the power_off callback, leaking power indefinitely. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260513-fd-hdmi-phy-v9-0-ca98c72f1f9f@oss.qualcomm.com?part=5 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-05-14 11:14 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-13 18:14 [PATCH v9 0/5] drm/msm/hdmi & phy: use generic PHY framework Dmitry Baryshkov 2026-05-13 18:14 ` [PATCH v9 2/5] phy: qualcomm: hdmi-28lpm: provide dynamic configuration Dmitry Baryshkov 2026-05-14 10:19 ` sashiko-bot 2026-05-13 18:14 ` [PATCH v9 3/5] phy: qcom: apq8064-sata: extract UNI PLL register defines Dmitry Baryshkov 2026-05-14 10:23 ` sashiko-bot 2026-05-13 18:14 ` [PATCH v9 4/5] phy: qcom-uniphy: add more registers from display PHYs Dmitry Baryshkov 2026-05-13 18:14 ` [PATCH v9 5/5] phy: qualcomm: add MSM8974 HDMI PHY support Dmitry Baryshkov 2026-05-14 11:14 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox