| Message ID | 20240929-b4-rk3588-bridge-upstream-v8-0-83538c2cc325@collabora.com |
|---|---|
| Headers | show |
| Series | Add initial support for the Rockchip RK3588 HDMI TX Controller | expand |
Hi Cristian, On 2024-09-29 00:36, Cristian Ciocaltea wrote: > The RK3588 SoC family integrates the newer Synopsys DesignWare HDMI 2.1 > Quad-Pixel (QP) TX controller IP and a HDMI/eDP TX Combo PHY based on a > Samsung IP block. > > Add just the basic support for now, i.e. RGB output up to 4K@60Hz, > without audio, CEC or any of the HDMI 2.1 specific features. > > Co-developed-by: Algea Cao <algea.cao@rock-chips.com> > Signed-off-by: Algea Cao <algea.cao@rock-chips.com> > Tested-by: Heiko Stuebner <heiko@sntech.de> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > drivers/gpu/drm/rockchip/Kconfig | 9 + > drivers/gpu/drm/rockchip/Makefile | 1 + > drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 425 +++++++++++++++++++++++++ > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 + > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + > 5 files changed, 438 insertions(+) > [snip] > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c > new file mode 100644 > index 000000000000..6103d30d40fb [snip] > +static irqreturn_t dw_hdmi_qp_rk3588_irq(int irq, void *dev_id) > +{ > + struct rockchip_hdmi_qp *hdmi = dev_id; > + u32 intr_stat, val; > + int debounce_ms; > + > + regmap_read(hdmi->regmap, RK3588_GRF_SOC_STATUS1, &intr_stat); > + if (!intr_stat) > + return IRQ_NONE; > + > + val = HIWORD_UPDATE(RK3588_HDMI0_HPD_INT_CLR, > + RK3588_HDMI0_HPD_INT_CLR); > + regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON2, val); > + > + debounce_ms = intr_stat & RK3588_HDMI0_LEVEL_INT ? 150 : 20; > + mod_delayed_work(system_wq, &hdmi->hpd_work, > + msecs_to_jiffies(debounce_ms)); If I am understanding this correctly this will wait for 150 ms after HPD goes high and 20 ms after HPD goes low to trigger the hpd_work. Would it not make more sense to be the other way around? In order to reduce unnecessary HOTPLUG=1 uevents from being triggered during short EDID refresh pulses? At least that is what I am playing around with for dw-hdmi. Regards, Jonas > + > + val |= HIWORD_UPDATE(0, RK3588_HDMI0_HPD_INT_MSK); > + regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON2, val); > + > + return IRQ_HANDLED; > +} [snip]
Hi, On Sun, Sep 29, 2024 at 01:36:49AM GMT, Cristian Ciocaltea wrote: > +static void dw_hdmi_qp_rockchip_encoder_enable(struct drm_encoder *encoder) > +{ > + struct rockchip_hdmi_qp *hdmi = to_rockchip_hdmi_qp(encoder); > + struct drm_crtc *crtc = encoder->crtc; > + int rate; > + > + /* Unconditionally switch to TMDS as FRL is not yet supported */ > + gpiod_set_value(hdmi->enable_gpio, 1); > + > + if (crtc && crtc->state) { > + clk_set_rate(hdmi->ref_clk, > + crtc->state->adjusted_mode.crtc_clock * 1000); Sorry, I should have seen it in your previous version, but the rate here should be the TMDS character rate, not the pixel clock, right? Once fixed, Reviewed-by: Maxime Ripard <mripard@kernel.org> Maxime
On Sun, Sep 29, 2024 at 01:36:47AM GMT, Cristian Ciocaltea wrote: > +static enum drm_mode_status > +dw_hdmi_qp_bridge_mode_valid(struct drm_bridge *bridge, > + const struct drm_display_info *info, > + const struct drm_display_mode *mode) > +{ > + struct dw_hdmi_qp *hdmi = bridge->driver_private; > + > + if (mode->clock > HDMI14_MAX_TMDSCLK / 1000) { > + dev_dbg(hdmi->dev, "Unsupported mode clock: %d\n", mode->clock); > + return MODE_CLOCK_HIGH; > + } Similarly, you should use drm_hdmi_compute_mode_clock here, with RGB and 8bpc Once fixed, Reviewed-by: Maxime Ripard <mripard@kernel.org> Maxime
Hi Jonas, On 9/29/24 3:34 AM, Jonas Karlman wrote: > Hi Cristian, > > On 2024-09-29 00:36, Cristian Ciocaltea wrote: >> The RK3588 SoC family integrates the newer Synopsys DesignWare HDMI 2.1 >> Quad-Pixel (QP) TX controller IP and a HDMI/eDP TX Combo PHY based on a >> Samsung IP block. >> >> Add just the basic support for now, i.e. RGB output up to 4K@60Hz, >> without audio, CEC or any of the HDMI 2.1 specific features. >> >> Co-developed-by: Algea Cao <algea.cao@rock-chips.com> >> Signed-off-by: Algea Cao <algea.cao@rock-chips.com> >> Tested-by: Heiko Stuebner <heiko@sntech.de> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> drivers/gpu/drm/rockchip/Kconfig | 9 + >> drivers/gpu/drm/rockchip/Makefile | 1 + >> drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 425 +++++++++++++++++++++++++ >> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 + >> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + >> 5 files changed, 438 insertions(+) >> > > [snip] > >> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c >> new file mode 100644 >> index 000000000000..6103d30d40fb > > [snip] > >> +static irqreturn_t dw_hdmi_qp_rk3588_irq(int irq, void *dev_id) >> +{ >> + struct rockchip_hdmi_qp *hdmi = dev_id; >> + u32 intr_stat, val; >> + int debounce_ms; >> + >> + regmap_read(hdmi->regmap, RK3588_GRF_SOC_STATUS1, &intr_stat); >> + if (!intr_stat) >> + return IRQ_NONE; >> + >> + val = HIWORD_UPDATE(RK3588_HDMI0_HPD_INT_CLR, >> + RK3588_HDMI0_HPD_INT_CLR); >> + regmap_write(hdmi->regmap, RK3588_GRF_SOC_CON2, val); >> + >> + debounce_ms = intr_stat & RK3588_HDMI0_LEVEL_INT ? 150 : 20; >> + mod_delayed_work(system_wq, &hdmi->hpd_work, >> + msecs_to_jiffies(debounce_ms)); > > If I am understanding this correctly this will wait for 150 ms after HPD > goes high and 20 ms after HPD goes low to trigger the hpd_work. > > Would it not make more sense to be the other way around? In order to > reduce unnecessary HOTPLUG=1 uevents from being triggered during short > EDID refresh pulses? At least that is what I am playing around with for > dw-hdmi. Sorry for my late reply, I'm on vacation but eventually managed to get access to a display, so I took the opportunity to prepare v9 [1]. The debouncing setup was borrowed from downstream driver and I haven't payed much attention to it, but now that you pointed this out I think we could simplify it and just use 150 ms in both cases. Thanks, Cristian [1]: https://lore.kernel.org/all/20241010-b4-rk3588-bridge-upstream-v9-0-68c47dde0410@collabora.com/
The Rockchip RK3588 SoC family integrates the Synopsys DesignWare HDMI 2.1 Quad-Pixel (QP) TX controller, which is a new IP block, quite different from those used in the previous generations of Rockchip SoCs. The controller supports the following features, among others: * Fixed Rate Link (FRL) * Display Stream Compression (DSC) * 4K@120Hz and 8K@60Hz video modes * Variable Refresh Rate (VRR) including Quick Media Switching (QMS) * Fast Vactive (FVA) * SCDC I2C DDC access * Multi-stream audio * Enhanced Audio Return Channel (EARC) This is the last component that needs to be supported in order to enable the HDMI output functionality on the RK3588 based SBCs, such as the RADXA Rock 5B. The other components are the Video Output Processor (VOP2) and the Samsung IP based HDMI/eDP TX Combo PHY, for which basic support has been already made available via [1] and [2], respectively. Please note this is a reworked version of the original series, which relied on a commonized dw-hdmi approach. Since the general consensus was to handle it as an entirely new IP, I dropped all patches related to the old dw-hdmi and Rockchip glue code - a few of them might still make sense as general improvements and will be submitted separately. It's worth mentioning the HDMI output support is currently limited to RGB output up to 4K@60Hz, without audio, CEC or any of the HDMI 2.1 specific features. Moreover, the VOP2 driver is not able to properly handle all display modes supported by the connected screens, e.g. it doesn't cope with non-integer refresh rates. A possible workaround consists of enabling the display controller to make use of the clock provided by the HDMI PHY PLL. This is still work in progress and will be submitted later, as well as the required DTS updates. To facilitate testing and experimentation, all HDMI output related patches, including those part of this series, are available at [3]. So far I could only verify this on the RADXA Rock 5B board. Thanks, Cristian [1]: 5a028e8f062f ("drm/rockchip: vop2: Add support for rk3588") [2]: 553be2830c5f ("phy: rockchip: Add Samsung HDMI/eDP Combo PHY driver") [3]: https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commits/rk3588-hdmi-bridge-next-20240913 [4]: https://lore.kernel.org/lkml/20240801-dw-hdmi-qp-tx-v1-0-148f542de5fd@collabora.com/ Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> --- Changes in v8: - Added R-b tag from Maxime on the HDMI controller library patch - Dropped dw_hdmi_qp_rockchip_encoder_mode_set() from the platform driver according to Maxime's review, the ref_clk rate adjustment is already handled in dw_hdmi_qp_rockchip_encoder_enable() - Link to v7: https://lore.kernel.org/r/20240914-b4-rk3588-bridge-upstream-v7-0-2b1348137123@collabora.com Changes in v7: - Added R-b from Krzysztof on DT binding patch (also dropped the superfluous minItems property from resets) - Fixed a sparse warning reported by kernel test robot when returning the error pointer from devm_platform_ioremap_resource() (made use of ERR_CAST() helper) - Simplified locking in dw_hdmi_qp_i2c_xfer() via guard() (Markus) - Dropped high TMDS clock ratio and scrambling support for now (will be submitted separately when ready) - Introduced dw_hdmi_qp_bridge_mode_valid() function to filter out unsupported mode clocks - Dropped the superfluous 'display' parameter of ->init() in struct dw_hdmi_qp_phy_ops - Improved error handling in dw_hdmi_qp_bridge_atomic_enable() - Handled dw_hdmi_qp_i2c_adapter() errors as fatal for bridge setup - Rebased series onto next-20240913 - Updated ROCKCHIP_DW_HDMI_QP kconfig to select the recently introduced DRM_BRIDGE_CONNECTOR dependency (Heiko) - Link to v6: https://lore.kernel.org/r/20240906-b4-rk3588-bridge-upstream-v6-0-a3128fb103eb@collabora.com Changes in v6: - Improved scrambling setup by using a delayed work queue in conjunction with the bridge ->detect() callback to support use cases like modetest where ->atomic_enable() is not called on reconnection (Maxime) - Explicitly include workqueue.h in platform driver - Dropped the common binding patch after merging its content into RK specific one; also moved the clocks & irq setup from the bridge library to the platform driver - Got rid of the phy-names property and fixed indentation in the binding example (Krzysztof) - Link to v5: https://lore.kernel.org/r/20240831-b4-rk3588-bridge-upstream-v5-0-9503bece0136@collabora.com Changes in v5: - Renamed Rockchip binding file to match the SoC compatible (Conor) - Made all clocks mandatory (Conor) - Renamed rockchip,vo1-grf property to rockchip,vo-grf as future SoCs (e.g. RK3576) may refer to it as vo0 instead of vo1 - Reworked the setup of high TMDS clock ratio and scrambling * Dropped curr_conn & pix_clock from struct dw_hdmi_qp * Also removed exported function dw_hdmi_qp_set_high_tmds_clock_ratio() * A few additional (mostly cosmetic) changes - Link to v4: https://lore.kernel.org/r/20240819-b4-rk3588-bridge-upstream-v4-0-6417c72a2749@collabora.com Changes in v4: - Added Tested-by tag from Heiko - Updated "[PATCH v3 3/5] dt-bindings: display: rockchip: Add schema for RK3588 HDMI TX Controller" according to Rob's review * Referenced full path for synopsys,dw-hdmi-qp.yaml * Moved ports to common schema and updated descriptions * Renamed rockchip,vo1_grf to rockchip,vo1-grf and updated "[PATCH v3 5/5] drm/rockchip: Add basic RK3588 HDMI output support" accordingly - Dropped "[PATCH v3 4/5] drm/rockchip: Explicitly include bits header" already applied by Heiko - Link to v3: https://lore.kernel.org/r/20240807-b4-rk3588-bridge-upstream-v3-0-60d6bab0dc7c@collabora.com Changes in v3: - Reintegrated bridge patchset [4] to allow automated testing and simplify reviewing (Krzysztof); the after-split changes were: * Made use of the new bridge HDMI helpers indicated by Dmitry * Dropped connector creation to ensure driver does only support DRM_BRIDGE_ATTACH_NO_CONNECTOR * Updated I2C segment handling to properly handle connected DVI displays (reported and fixed by Heiko) - Updated schema for DW HDMI QP TX IP providing some hardware details - Updated patch for DW HDMI QP TX Controller module referring to a support library instead of a platform driver (Krzysztof) - Drop empty dw_hdmi_qp_unbind() export from the library and related usage from RK platform driver - Drop Fixes tag from "drm/rockchip: Explicitly include bits header" patch (Krzysztof) - Link to v2: https://lore.kernel.org/r/20240801-b4-rk3588-bridge-upstream-v2-0-9fa657a4e15b@collabora.com Changes in v2: - Reworked the glue code for RK3588 into a new Rockchip platform driver - Moved bridge driver patches to a separate series [4] - Dropped all the patches touching to the old dw-hdmi and RK platform drivers - Added connector creation to ensure the HDMI QP bridge driver does only support DRM_BRIDGE_ATTACH_NO_CONNECTOR - Link to v1: https://lore.kernel.org/r/20240601-b4-rk3588-bridge-upstream-v1-0-f6203753232b@collabora.com --- Cristian Ciocaltea (3): drm/bridge: synopsys: Add DW HDMI QP TX Controller support library dt-bindings: display: rockchip: Add schema for RK3588 HDMI TX Controller drm/rockchip: Add basic RK3588 HDMI output support .../rockchip/rockchip,rk3588-dw-hdmi-qp.yaml | 188 +++++ drivers/gpu/drm/bridge/synopsys/Kconfig | 8 + drivers/gpu/drm/bridge/synopsys/Makefile | 2 + drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 645 ++++++++++++++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h | 834 +++++++++++++++++++++ drivers/gpu/drm/rockchip/Kconfig | 9 + drivers/gpu/drm/rockchip/Makefile | 1 + drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 425 +++++++++++ drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 + drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + include/drm/bridge/dw_hdmi_qp.h | 32 + 11 files changed, 2147 insertions(+) --- base-commit: 5acd9952f95fb4b7da6d09a3be39195a80845eb6 change-id: 20240601-b4-rk3588-bridge-upstream-a27baff1b8fc