| Message ID | 20240601-b4-rk3588-bridge-upstream-v1-0-f6203753232b@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add initial support for the Rockchip RK3588 HDMI TX Controller | expand |
On Sat, Jun 01, 2024 at 04:12:22PM +0300, Cristian Ciocaltea wrote: > The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the > Synopsys DesignWare HDMI TX controller used in the previous SoCs. > > It is HDMI 2.1 compliant and supports the following features, among > others: > > * Fixed Rate Link (FRL) > * 4K@120Hz and 8K@60Hz video modes > * Variable Refresh Rate (VRR) including Quick Media Switching (QMS) > * Fast Vactive (FVA) > * SCDC I2C DDC access > * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 > * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds > * Multi-stream audio > * Enhanced Audio Return Channel (EARC) It would be really nice if you can take a look at using the HDMI connector framework (landed few days ago) with adaptations for the drm_bridge / drm_bridge_connector ([1]). Your comments for the drm_bridge patches would be defeinitely appreciated. [1] https://lore.kernel.org/dri-devel/20240531-bridge-hdmi-connector-v4-0-5110f7943622@linaro.org/ > > This is the last required 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. > > The patches are grouped as follows: > * PATCH 1..7: DW HDMI TX driver refactor to minimize code duplication in > the new QP driver (no functional changes intended) > > * PATCH 8..11: Rockchip DW HDMI glue driver cleanup/improvements (no > functional changes intended) > > * PATCH 12..13: The new DW HDMI QP TX driver reusing the previously > exported functions and structs from existing DW HDMI TX driver > > * PATCH 14: Rockchip DW HDMI glue driver update to support RK3588 and > make use of DW HDMI QP TX > > They provide just the basic HDMI support for now, i.e. RGB output up to > 4K@60Hz, without audio, CEC or any of the HDMI 2.1 specific features. > Also note the vop2 driver is currently 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 3A and 5B boards. > > 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-v6.10-rc1 > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > Cristian Ciocaltea (14): > drm/bridge: dw-hdmi: Simplify clock handling > drm/bridge: dw-hdmi: Add dw-hdmi-common.h header > drm/bridge: dw-hdmi: Commonize dw_hdmi_i2c_adapter() > drm/bridge: dw-hdmi: Factor out AVI infoframe setup > drm/bridge: dw-hdmi: Factor out vmode setup > drm/bridge: dw-hdmi: Factor out hdmi_data_info setup > drm/bridge: dw-hdmi: Commonize dw_hdmi_connector_create() > drm/rockchip: dw_hdmi: Use modern drm_device based logging > drm/rockchip: dw_hdmi: Simplify clock handling > drm/rockchip: dw_hdmi: Use devm_regulator_get_enable() > drm/rockchip: dw_hdmi: Drop superfluous assignments of mpll_cfg, cur_ctr and phy_config > dt-bindings: display: rockchip,dw-hdmi: Add compatible for RK3588 > drm/bridge: synopsys: Add DW HDMI QP TX controller driver > drm/rockchip: dw_hdmi: Add basic RK3588 support > > .../display/rockchip/rockchip,dw-hdmi.yaml | 127 +++- > drivers/gpu/drm/bridge/synopsys/Makefile | 2 +- > drivers/gpu/drm/bridge/synopsys/dw-hdmi-common.h | 179 +++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 787 +++++++++++++++++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h | 831 +++++++++++++++++++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 353 +++------ > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 351 +++++++-- > include/drm/bridge/dw_hdmi.h | 8 + > 8 files changed, 2290 insertions(+), 348 deletions(-) > --- > base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 > change-id: 20240601-b4-rk3588-bridge-upstream-a27baff1b8fc >
(resent as plain text instead of html) Cristian, I was awaiting over year for this work! I’m devel. 2 distros where single mainline kernel serves 2835/2711/2712/h6/h313/h616/h618/rk3328/rk3399/rk3566/rk3568/rk3588/s905/s912/sm1/g12. Before this work rk3588 was excluded because rk3588 hdmi was regressing hdmi on other socs. With this code all other socs seems work ok now. Perfect. As one of my project is multimedia appliance - good news is that now i can nicely play hdtv on rk3588 using mainline common 6.9.3 kernel and….started to hear from my users a lot of Qs like: „ah so nice! rk3588 now works nicely….but where is hdmi audio and cec?” It will be fantastic to add (e.g. by backport Detlev https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/tree/rk3588-hdmi-audio?ref_type=heads ) audio code to get basic support hdmi audio? thx again for fantastic work! > Wiadomość napisana przez Cristian Ciocaltea <cristian.ciocaltea@collabora.com> w dniu 01.06.2024, o godz. 15:12: > > The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the > Synopsys DesignWare HDMI TX controller used in the previous SoCs. > > It is HDMI 2.1 compliant and supports the following features, among > others: > > * Fixed Rate Link (FRL) > * 4K@120Hz and 8K@60Hz video modes > * Variable Refresh Rate (VRR) including Quick Media Switching (QMS) > * Fast Vactive (FVA) > * SCDC I2C DDC access > * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 > * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds > * Multi-stream audio > * Enhanced Audio Return Channel (EARC) > > This is the last required 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. > > The patches are grouped as follows: > * PATCH 1..7: DW HDMI TX driver refactor to minimize code duplication in > the new QP driver (no functional changes intended) > > * PATCH 8..11: Rockchip DW HDMI glue driver cleanup/improvements (no > functional changes intended) > > * PATCH 12..13: The new DW HDMI QP TX driver reusing the previously > exported functions and structs from existing DW HDMI TX driver > > * PATCH 14: Rockchip DW HDMI glue driver update to support RK3588 and > make use of DW HDMI QP TX > > They provide just the basic HDMI support for now, i.e. RGB output up to > 4K@60Hz, without audio, CEC or any of the HDMI 2.1 specific features. > Also note the vop2 driver is currently 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 3A and 5B boards. > > 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-v6.10-rc1 > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > Cristian Ciocaltea (14): > drm/bridge: dw-hdmi: Simplify clock handling > drm/bridge: dw-hdmi: Add dw-hdmi-common.h header > drm/bridge: dw-hdmi: Commonize dw_hdmi_i2c_adapter() > drm/bridge: dw-hdmi: Factor out AVI infoframe setup > drm/bridge: dw-hdmi: Factor out vmode setup > drm/bridge: dw-hdmi: Factor out hdmi_data_info setup > drm/bridge: dw-hdmi: Commonize dw_hdmi_connector_create() > drm/rockchip: dw_hdmi: Use modern drm_device based logging > drm/rockchip: dw_hdmi: Simplify clock handling > drm/rockchip: dw_hdmi: Use devm_regulator_get_enable() > drm/rockchip: dw_hdmi: Drop superfluous assignments of mpll_cfg, cur_ctr and phy_config > dt-bindings: display: rockchip,dw-hdmi: Add compatible for RK3588 > drm/bridge: synopsys: Add DW HDMI QP TX controller driver > drm/rockchip: dw_hdmi: Add basic RK3588 support > > .../display/rockchip/rockchip,dw-hdmi.yaml | 127 +++- > drivers/gpu/drm/bridge/synopsys/Makefile | 2 +- > drivers/gpu/drm/bridge/synopsys/dw-hdmi-common.h | 179 +++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 787 +++++++++++++++++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h | 831 +++++++++++++++++++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 353 +++------ > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 351 +++++++-- > include/drm/bridge/dw_hdmi.h | 8 + > 8 files changed, 2290 insertions(+), 348 deletions(-) > --- > base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 > change-id: 20240601-b4-rk3588-bridge-upstream-a27baff1b8fc > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
Hi Christian, On 01/06/2024 15:12, Cristian Ciocaltea wrote: > The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the > Synopsys DesignWare HDMI TX controller used in the previous SoCs. > > It is HDMI 2.1 compliant and supports the following features, among > others: > . .. > * SCDC I2C DDC access > * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 > * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds > * Multi-stream audio > * Enhanced Audio Return Channel (EARC) -> Those features were already supported by the HDMI 2.0a compliant HW, just list the _new_ features for HDMI 2.1 I did a quick review of your patchset and I don't understand why you need to add a separate dw-hdmi-qp.c since you only need simple variants of the I2C bus, infoframe and bridge setup. Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller version detectable at runtime ? I would prefer to keep a single dw-hdmi driver if possible. Thanks, Neil > > This is the last required 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. > > The patches are grouped as follows: > * PATCH 1..7: DW HDMI TX driver refactor to minimize code duplication in > the new QP driver (no functional changes intended) > > * PATCH 8..11: Rockchip DW HDMI glue driver cleanup/improvements (no > functional changes intended) > > * PATCH 12..13: The new DW HDMI QP TX driver reusing the previously > exported functions and structs from existing DW HDMI TX driver > > * PATCH 14: Rockchip DW HDMI glue driver update to support RK3588 and > make use of DW HDMI QP TX > > They provide just the basic HDMI support for now, i.e. RGB output up to > 4K@60Hz, without audio, CEC or any of the HDMI 2.1 specific features. > Also note the vop2 driver is currently 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 3A and 5B boards. > > 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-v6.10-rc1 > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > Cristian Ciocaltea (14): > drm/bridge: dw-hdmi: Simplify clock handling > drm/bridge: dw-hdmi: Add dw-hdmi-common.h header > drm/bridge: dw-hdmi: Commonize dw_hdmi_i2c_adapter() > drm/bridge: dw-hdmi: Factor out AVI infoframe setup > drm/bridge: dw-hdmi: Factor out vmode setup > drm/bridge: dw-hdmi: Factor out hdmi_data_info setup > drm/bridge: dw-hdmi: Commonize dw_hdmi_connector_create() > drm/rockchip: dw_hdmi: Use modern drm_device based logging > drm/rockchip: dw_hdmi: Simplify clock handling > drm/rockchip: dw_hdmi: Use devm_regulator_get_enable() > drm/rockchip: dw_hdmi: Drop superfluous assignments of mpll_cfg, cur_ctr and phy_config > dt-bindings: display: rockchip,dw-hdmi: Add compatible for RK3588 > drm/bridge: synopsys: Add DW HDMI QP TX controller driver > drm/rockchip: dw_hdmi: Add basic RK3588 support > > .../display/rockchip/rockchip,dw-hdmi.yaml | 127 +++- > drivers/gpu/drm/bridge/synopsys/Makefile | 2 +- > drivers/gpu/drm/bridge/synopsys/dw-hdmi-common.h | 179 +++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 787 +++++++++++++++++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h | 831 +++++++++++++++++++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 353 +++------ > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 351 +++++++-- > include/drm/bridge/dw_hdmi.h | 8 + > 8 files changed, 2290 insertions(+), 348 deletions(-) > --- > base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 > change-id: 20240601-b4-rk3588-bridge-upstream-a27baff1b8fc >
Hi Neil: On 6/3/24 16:55, Neil Armstrong wrote: > Hi Christian, > > On 01/06/2024 15:12, Cristian Ciocaltea wrote: >> The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the >> Synopsys DesignWare HDMI TX controller used in the previous SoCs. >> >> It is HDMI 2.1 compliant and supports the following features, among >> others: >> > . > > .. > >> * SCDC I2C DDC access >> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 >> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds >> * Multi-stream audio >> * Enhanced Audio Return Channel (EARC) > -> Those features were already supported by the HDMI 2.0a compliant HW, just > list the _new_ features for HDMI 2.1 > > I did a quick review of your patchset and I don't understand why you need > to add a separate dw-hdmi-qp.c since you only need simple variants of the I2C > bus, infoframe and bridge setup. > > Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller version > detectable at runtime ? > > I would prefer to keep a single dw-hdmi driver if possible. The QP HDMI controller is a completely different variant with totally different registers layout, see PATCH 13/14. I think make it a separate driver will be easier for development and maintenance. > > Thanks, > Neil > >> >> This is the last required 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. >> >> The patches are grouped as follows: >> * PATCH 1..7: DW HDMI TX driver refactor to minimize code duplication in >> the new QP driver (no functional changes intended) >> >> * PATCH 8..11: Rockchip DW HDMI glue driver cleanup/improvements (no >> functional changes intended) >> >> * PATCH 12..13: The new DW HDMI QP TX driver reusing the previously >> exported functions and structs from existing DW HDMI TX driver >> >> * PATCH 14: Rockchip DW HDMI glue driver update to support RK3588 and >> make use of DW HDMI QP TX >> >> They provide just the basic HDMI support for now, i.e. RGB output up to >> 4K@60Hz, without audio, CEC or any of the HDMI 2.1 specific features. >> Also note the vop2 driver is currently 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 3A and 5B boards. >> >> 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-v6.10-rc1 >> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> Cristian Ciocaltea (14): >> drm/bridge: dw-hdmi: Simplify clock handling >> drm/bridge: dw-hdmi: Add dw-hdmi-common.h header >> drm/bridge: dw-hdmi: Commonize dw_hdmi_i2c_adapter() >> drm/bridge: dw-hdmi: Factor out AVI infoframe setup >> drm/bridge: dw-hdmi: Factor out vmode setup >> drm/bridge: dw-hdmi: Factor out hdmi_data_info setup >> drm/bridge: dw-hdmi: Commonize dw_hdmi_connector_create() >> drm/rockchip: dw_hdmi: Use modern drm_device based logging >> drm/rockchip: dw_hdmi: Simplify clock handling >> drm/rockchip: dw_hdmi: Use devm_regulator_get_enable() >> drm/rockchip: dw_hdmi: Drop superfluous assignments of mpll_cfg, cur_ctr and phy_config >> dt-bindings: display: rockchip,dw-hdmi: Add compatible for RK3588 >> drm/bridge: synopsys: Add DW HDMI QP TX controller driver >> drm/rockchip: dw_hdmi: Add basic RK3588 support >> >> .../display/rockchip/rockchip,dw-hdmi.yaml | 127 +++- >> drivers/gpu/drm/bridge/synopsys/Makefile | 2 +- >> drivers/gpu/drm/bridge/synopsys/dw-hdmi-common.h | 179 +++++ >> drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 787 +++++++++++++++++++ >> drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h | 831 +++++++++++++++++++++ >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 353 +++------ >> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 351 +++++++-- >> include/drm/bridge/dw_hdmi.h | 8 + >> 8 files changed, 2290 insertions(+), 348 deletions(-) >> --- >> base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 >> change-id: 20240601-b4-rk3588-bridge-upstream-a27baff1b8fc >> >
Am Montag, 3. Juni 2024, 14:14:17 CEST schrieb Andy Yan: > Hi Neil: > > On 6/3/24 16:55, Neil Armstrong wrote: > > Hi Christian, > > > > On 01/06/2024 15:12, Cristian Ciocaltea wrote: > >> The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the > >> Synopsys DesignWare HDMI TX controller used in the previous SoCs. > >> > >> It is HDMI 2.1 compliant and supports the following features, among > >> others: > >> > > . > > > > .. > > > >> * SCDC I2C DDC access > >> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 > >> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds > >> * Multi-stream audio > >> * Enhanced Audio Return Channel (EARC) > > -> Those features were already supported by the HDMI 2.0a compliant HW, just > > list the _new_ features for HDMI 2.1 > > > > I did a quick review of your patchset and I don't understand why you need > > to add a separate dw-hdmi-qp.c since you only need simple variants of the I2C > > bus, infoframe and bridge setup. > > > > Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller version > > detectable at runtime ? > > > > I would prefer to keep a single dw-hdmi driver if possible. > > > > The QP HDMI controller is a completely different variant with totally different > registers layout, see PATCH 13/14. > I think make it a separate driver will be easier for development and maintenance. I'm with Andy here. Trying to navigate a driver for two IP blocks really sounds taxing especially when both are so different. Synopsis also created a new dsi controller for the DSI2 standard, with a vastly different registers layout. I guess at some point there is time to say this really is a new IP ;-) . Though while on that thought, I don't fully understand why both a compiled under the dw_hdmi kconfig symbol. People going for a minimal kernel might want one or the other, but not both for their specific board. Heiko
Hi, On 03/06/2024 15:03, Heiko Stuebner wrote: > Am Montag, 3. Juni 2024, 14:14:17 CEST schrieb Andy Yan: >> Hi Neil: >> >> On 6/3/24 16:55, Neil Armstrong wrote: >>> Hi Christian, >>> >>> On 01/06/2024 15:12, Cristian Ciocaltea wrote: >>>> The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the >>>> Synopsys DesignWare HDMI TX controller used in the previous SoCs. >>>> >>>> It is HDMI 2.1 compliant and supports the following features, among >>>> others: >>>> >>> . >>> >>> .. >>> >>>> * SCDC I2C DDC access >>>> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 >>>> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds >>>> * Multi-stream audio >>>> * Enhanced Audio Return Channel (EARC) >>> -> Those features were already supported by the HDMI 2.0a compliant HW, just >>> list the _new_ features for HDMI 2.1 >>> >>> I did a quick review of your patchset and I don't understand why you need >>> to add a separate dw-hdmi-qp.c since you only need simple variants of the I2C >>> bus, infoframe and bridge setup. >>> >>> Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller version >>> detectable at runtime ? >>> >>> I would prefer to keep a single dw-hdmi driver if possible. >> >> >> >> The QP HDMI controller is a completely different variant with totally different >> registers layout, see PATCH 13/14. >> I think make it a separate driver will be easier for development and maintenance. > > I'm with Andy here. Trying to navigate a driver for two IP blocks really > sounds taxing especially when both are so different. I agree, I just wanted more details than "variant of the Synopsys DesignWare HDMI TX controller", if the register mapping is 100% different, and does not match at all with the old IP, then it's indeed time to make a brand new driver, but instead of doing a mix up, it's time to extract the dw-hdmi code that could be common helpers into a dw-hdmi-common module and use them. As I see, no "driver" code can be shared, only DRM plumbings, so perhaps those plumbing code should go into the DRM core ? In any case, please add more details on the cover letter, including the detailed HW differrence and the design you chose so support this new IP. Neil > > Synopsis also created a new dsi controller for the DSI2 standard, with > a vastly different registers layout. > > I guess at some point there is time to say this really is a new IP ;-) . > > > Though while on that thought, I don't fully understand why both a compiled > under the dw_hdmi kconfig symbol. People going for a minimal kernel might > want one or the other, but not both for their specific board. > > > Heiko > >
On Mon, Jun 03, 2024 at 03:03:12PM GMT, Heiko Stuebner wrote: > Am Montag, 3. Juni 2024, 14:14:17 CEST schrieb Andy Yan: > > Hi Neil: > > > > On 6/3/24 16:55, Neil Armstrong wrote: > > > Hi Christian, > > > > > > On 01/06/2024 15:12, Cristian Ciocaltea wrote: > > >> The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the > > >> Synopsys DesignWare HDMI TX controller used in the previous SoCs. > > >> > > >> It is HDMI 2.1 compliant and supports the following features, among > > >> others: > > >> > > > . > > > > > > .. > > > > > >> * SCDC I2C DDC access > > >> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 > > >> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds > > >> * Multi-stream audio > > >> * Enhanced Audio Return Channel (EARC) > > > -> Those features were already supported by the HDMI 2.0a compliant HW, just > > > list the _new_ features for HDMI 2.1 > > > > > > I did a quick review of your patchset and I don't understand why you need > > > to add a separate dw-hdmi-qp.c since you only need simple variants of the I2C > > > bus, infoframe and bridge setup. > > > > > > Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller version > > > detectable at runtime ? > > > > > > I would prefer to keep a single dw-hdmi driver if possible. > > > > The QP HDMI controller is a completely different variant with totally different > > registers layout, see PATCH 13/14. > > I think make it a separate driver will be easier for development and maintenance. > > I'm with Andy here. Trying to navigate a driver for two IP blocks really > sounds taxing especially when both are so different. If it's a completely new controller, I agree that it needs a new driver, but then why do we need to share code between the two? Maxime
On 6/1/24 7:32 PM, Dmitry Baryshkov wrote: > On Sat, Jun 01, 2024 at 04:12:22PM +0300, Cristian Ciocaltea wrote: >> The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the >> Synopsys DesignWare HDMI TX controller used in the previous SoCs. >> >> It is HDMI 2.1 compliant and supports the following features, among >> others: >> >> * Fixed Rate Link (FRL) >> * 4K@120Hz and 8K@60Hz video modes >> * Variable Refresh Rate (VRR) including Quick Media Switching (QMS) >> * Fast Vactive (FVA) >> * SCDC I2C DDC access >> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 >> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds >> * Multi-stream audio >> * Enhanced Audio Return Channel (EARC) > > It would be really nice if you can take a look at using the HDMI > connector framework (landed few days ago) with adaptations for the > drm_bridge / drm_bridge_connector ([1]). Your comments for the > drm_bridge patches would be defeinitely appreciated. > > [1] https://lore.kernel.org/dri-devel/20240531-bridge-hdmi-connector-v4-0-5110f7943622@linaro.org/ I will definitely check and try to use it, but I'd rather wait a bit until this gets stabilized and focus instead on the mandatory changes required to upstream this driver. That's mostly because my limited availability and expertise on the matter, while trying to unblock other work depending on this. Thanks, Cristian
On 6/2/24 10:59 AM, Piotr Oniszczuk wrote: > (resent as plain text instead of html) > > Cristian, > > I was awaiting over year for this work! > > I’m devel. 2 distros where single mainline kernel serves 2835/2711/2712/h6/h313/h616/h618/rk3328/rk3399/rk3566/rk3568/rk3588/s905/s912/sm1/g12. > > Before this work rk3588 was excluded because rk3588 hdmi was regressing hdmi on other socs. > With this code all other socs seems work ok now. Perfect. Many thanks for giving this a try on a broad range of SoCs, especially considering my limited testing capabilities! > As one of my project is multimedia appliance - good news is that now i can nicely play hdtv on rk3588 using mainline common 6.9.3 kernel and….started to hear from my users a lot of Qs like: „ah so nice! rk3588 now works nicely….but where is hdmi audio and cec?” > > It will be fantastic to add (e.g. by backport Detlev https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/tree/rk3588-hdmi-audio?ref_type=heads ) audio code to get basic support hdmi audio? The main focus is now on upstreaming the basic support. This should further facilitate adding the missing features, so we will slowly get there, eventually.
On 6/3/24 4:08 PM, neil.armstrong@linaro.org wrote: > Hi, > > On 03/06/2024 15:03, Heiko Stuebner wrote: >> Am Montag, 3. Juni 2024, 14:14:17 CEST schrieb Andy Yan: >>> Hi Neil: >>> >>> On 6/3/24 16:55, Neil Armstrong wrote: >>>> Hi Christian, >>>> >>>> On 01/06/2024 15:12, Cristian Ciocaltea wrote: >>>>> The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the >>>>> Synopsys DesignWare HDMI TX controller used in the previous SoCs. >>>>> >>>>> It is HDMI 2.1 compliant and supports the following features, among >>>>> others: >>>>> >>>> . >>>> >>>> .. >>>> >>>>> * SCDC I2C DDC access >>>>> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 >>>>> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds >>>>> * Multi-stream audio >>>>> * Enhanced Audio Return Channel (EARC) >>>> -> Those features were already supported by the HDMI 2.0a compliant >>>> HW, just >>>> list the _new_ features for HDMI 2.1 >>>> >>>> I did a quick review of your patchset and I don't understand why you >>>> need >>>> to add a separate dw-hdmi-qp.c since you only need simple variants >>>> of the I2C >>>> bus, infoframe and bridge setup. >>>> >>>> Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller >>>> version >>>> detectable at runtime ? >>>> >>>> I would prefer to keep a single dw-hdmi driver if possible. >>> >>> >>> >>> The QP HDMI controller is a completely different variant with totally >>> different >>> registers layout, see PATCH 13/14. >>> I think make it a separate driver will be easier for development and >>> maintenance. >> >> I'm with Andy here. Trying to navigate a driver for two IP blocks really >> sounds taxing especially when both are so different. Thank you all for the valuable feedback! > I agree, I just wanted more details than "variant of the > Synopsys DesignWare HDMI TX controller", if the register mapping is 100% > different, and does not match at all with the old IP, then it's indeed time > to make a brand new driver, but instead of doing a mix up, it's time to > extract > the dw-hdmi code that could be common helpers into a dw-hdmi-common module > and use them. Sounds good, will handle this in v2. > As I see, no "driver" code can be shared, only DRM plumbings, so perhaps > those > plumbing code should go into the DRM core ? > > In any case, please add more details on the cover letter, including the > detailed > HW differrence and the design you chose so support this new IP. Andy, could you please help with a summary of the HW changes? The information I could provide is rather limited, since I don't have access to any DW IP datasheets and I'm also not familiar enough with the old variant. > Neil > >> >> Synopsis also created a new dsi controller for the DSI2 standard, with >> a vastly different registers layout. >> >> I guess at some point there is time to say this really is a new IP ;-) . >> >> >> Though while on that thought, I don't fully understand why both a >> compiled >> under the dw_hdmi kconfig symbol. People going for a minimal kernel might >> want one or the other, but not both for their specific board. Indeed, it makes sense to have a dedicated Kconfig option. This is mostly a leftover from downstream implementation, will fix in v2. Thanks again, Cristian
On Tue, Jun 04, 2024 at 10:44:04PM +0300, Cristian Ciocaltea wrote: > On 6/1/24 7:32 PM, Dmitry Baryshkov wrote: > > On Sat, Jun 01, 2024 at 04:12:22PM +0300, Cristian Ciocaltea wrote: > >> The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the > >> Synopsys DesignWare HDMI TX controller used in the previous SoCs. > >> > >> It is HDMI 2.1 compliant and supports the following features, among > >> others: > >> > >> * Fixed Rate Link (FRL) > >> * 4K@120Hz and 8K@60Hz video modes > >> * Variable Refresh Rate (VRR) including Quick Media Switching (QMS) > >> * Fast Vactive (FVA) > >> * SCDC I2C DDC access > >> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 > >> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds > >> * Multi-stream audio > >> * Enhanced Audio Return Channel (EARC) > > > > It would be really nice if you can take a look at using the HDMI > > connector framework (landed few days ago) with adaptations for the > > drm_bridge / drm_bridge_connector ([1]). Your comments for the > > drm_bridge patches would be defeinitely appreciated. > > > > [1] https://lore.kernel.org/dri-devel/20240531-bridge-hdmi-connector-v4-0-5110f7943622@linaro.org/ > > I will definitely check and try to use it, but I'd rather wait a bit > until this gets stabilized and focus instead on the mandatory changes > required to upstream this driver. That's mostly because my limited > availability and expertise on the matter, while trying to unblock other > work depending on this. Ack.
Hi, At 2024-06-05 04:33:57, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote: >On 6/3/24 4:08 PM, neil.armstrong@linaro.org wrote: >> Hi, >> >> On 03/06/2024 15:03, Heiko Stuebner wrote: >>> Am Montag, 3. Juni 2024, 14:14:17 CEST schrieb Andy Yan: >>>> Hi Neil: >>>> >>>> On 6/3/24 16:55, Neil Armstrong wrote: >>>>> Hi Christian, >>>>> >>>>> On 01/06/2024 15:12, Cristian Ciocaltea wrote: >>>>>> The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the >>>>>> Synopsys DesignWare HDMI TX controller used in the previous SoCs. >>>>>> >>>>>> It is HDMI 2.1 compliant and supports the following features, among >>>>>> others: >>>>>> >>>>> . >>>>> >>>>> .. >>>>> >>>>>> * SCDC I2C DDC access >>>>>> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 >>>>>> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds >>>>>> * Multi-stream audio >>>>>> * Enhanced Audio Return Channel (EARC) >>>>> -> Those features were already supported by the HDMI 2.0a compliant >>>>> HW, just >>>>> list the _new_ features for HDMI 2.1 >>>>> >>>>> I did a quick review of your patchset and I don't understand why you >>>>> need >>>>> to add a separate dw-hdmi-qp.c since you only need simple variants >>>>> of the I2C >>>>> bus, infoframe and bridge setup. >>>>> >>>>> Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller >>>>> version >>>>> detectable at runtime ? >>>>> >>>>> I would prefer to keep a single dw-hdmi driver if possible. >>>> >>>> >>>> >>>> The QP HDMI controller is a completely different variant with totally >>>> different >>>> registers layout, see PATCH 13/14. >>>> I think make it a separate driver will be easier for development and >>>> maintenance. >>> >>> I'm with Andy here. Trying to navigate a driver for two IP blocks really >>> sounds taxing especially when both are so different. > >Thank you all for the valuable feedback! > >> I agree, I just wanted more details than "variant of the >> Synopsys DesignWare HDMI TX controller", if the register mapping is 100% >> different, and does not match at all with the old IP, then it's indeed time >> to make a brand new driver, but instead of doing a mix up, it's time to >> extract >> the dw-hdmi code that could be common helpers into a dw-hdmi-common module >> and use them. > >Sounds good, will handle this in v2. > >> As I see, no "driver" code can be shared, only DRM plumbings, so perhaps >> those >> plumbing code should go into the DRM core ? >> >> In any case, please add more details on the cover letter, including the >> detailed >> HW differrence and the design you chose so support this new IP. > >Andy, could you please help with a summary of the HW changes? >The information I could provide is rather limited, since I don't have >access to any DW IP datasheets and I'm also not familiar enough with the >old variant. > Accurately, we should refer to it as an entirely new IP，it has nothing in common with the current mainline dw-hdmi。 The only commonality is that they both come from Synopsys DesignWare： （1）It has a 100% different register mapping （2）It supports FRL and DSC （3）different configuration flow in many places。 So I have the same feeling with Heiko and Maxime： The DW_HDMI_QP should have a separate driver and with it's own CONFIG such as DRM_DW_HDMI_QP in Kconfig. and the rockchip part should also be split from dw_hdmi-rockchip.c. I am sorry we mixed them in dw_hdmi-rockchip.c when we develop the bsp driver，but we really regretted this decision when we repeatedly broke compatibility with dw-hdmi on other socs。 >> Neil >> >>> >>> Synopsis also created a new dsi controller for the DSI2 standard, with >>> a vastly different registers layout. >>> >>> I guess at some point there is time to say this really is a new IP ;-) . >>> >>> >>> Though while on that thought, I don't fully understand why both a >>> compiled >>> under the dw_hdmi kconfig symbol. People going for a minimal kernel might >>> want one or the other, but not both for their specific board. > >Indeed, it makes sense to have a dedicated Kconfig option. This is >mostly a leftover from downstream implementation, will fix in v2. > >Thanks again, >Cristian > >_______________________________________________ >linux-arm-kernel mailing list >linux-arm-kernel@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi, On 05/06/2024 11:25, Andy Yan wrote: > > Hi, > > At 2024-06-05 04:33:57, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote: >> On 6/3/24 4:08 PM, neil.armstrong@linaro.org wrote: >>> Hi, >>> >>> On 03/06/2024 15:03, Heiko Stuebner wrote: >>>> Am Montag, 3. Juni 2024, 14:14:17 CEST schrieb Andy Yan: >>>>> Hi Neil: >>>>> >>>>> On 6/3/24 16:55, Neil Armstrong wrote: >>>>>> Hi Christian, >>>>>> >>>>>> On 01/06/2024 15:12, Cristian Ciocaltea wrote: >>>>>>> The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the >>>>>>> Synopsys DesignWare HDMI TX controller used in the previous SoCs. >>>>>>> >>>>>>> It is HDMI 2.1 compliant and supports the following features, among >>>>>>> others: >>>>>>> >>>>>> . >>>>>> >>>>>> .. >>>>>> >>>>>>> * SCDC I2C DDC access >>>>>>> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 >>>>>>> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds >>>>>>> * Multi-stream audio >>>>>>> * Enhanced Audio Return Channel (EARC) >>>>>> -> Those features were already supported by the HDMI 2.0a compliant >>>>>> HW, just >>>>>> list the _new_ features for HDMI 2.1 >>>>>> >>>>>> I did a quick review of your patchset and I don't understand why you >>>>>> need >>>>>> to add a separate dw-hdmi-qp.c since you only need simple variants >>>>>> of the I2C >>>>>> bus, infoframe and bridge setup. >>>>>> >>>>>> Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller >>>>>> version >>>>>> detectable at runtime ? >>>>>> >>>>>> I would prefer to keep a single dw-hdmi driver if possible. >>>>> >>>>> >>>>> >>>>> The QP HDMI controller is a completely different variant with totally >>>>> different >>>>> registers layout, see PATCH 13/14. >>>>> I think make it a separate driver will be easier for development and >>>>> maintenance. >>>> >>>> I'm with Andy here. Trying to navigate a driver for two IP blocks really >>>> sounds taxing especially when both are so different. >> >> Thank you all for the valuable feedback! >> >>> I agree, I just wanted more details than "variant of the >>> Synopsys DesignWare HDMI TX controller", if the register mapping is 100% >>> different, and does not match at all with the old IP, then it's indeed time >>> to make a brand new driver, but instead of doing a mix up, it's time to >>> extract >>> the dw-hdmi code that could be common helpers into a dw-hdmi-common module >>> and use them. >> >> Sounds good, will handle this in v2. >> >>> As I see, no "driver" code can be shared, only DRM plumbings, so perhaps >>> those >>> plumbing code should go into the DRM core ? >>> >>> In any case, please add more details on the cover letter, including the >>> detailed >>> HW differrence and the design you chose so support this new IP. >> >> Andy, could you please help with a summary of the HW changes? >> The information I could provide is rather limited, since I don't have >> access to any DW IP datasheets and I'm also not familiar enough with the >> old variant. >> > Accurately, we should refer to it as an entirely new IP，it has nothing in common with > the current mainline dw-hdmi。 The only commonality is that they both come from > Synopsys DesignWare： > （1）It has a 100% different register mapping > （2）It supports FRL and DSC > （3）different configuration flow in many places。 > > So I have the same feeling with Heiko and Maxime： > The DW_HDMI_QP should have a separate driver and with it's own CONFIG such as DRM_DW_HDMI_QP in Kconfig. > and the rockchip part should also be split from dw_hdmi-rockchip.c. > I am sorry we mixed them in dw_hdmi-rockchip.c when we develop the bsp driver，but we really regretted this decision > when we repeatedly broke compatibility with dw-hdmi on other socs。 Yes please, and as I say, if there's code common with the old dw-hdmi, please add a common module if this code can't be moved in core bridge helpers. Neil > > > >>> Neil >>> >>>> >>>> Synopsis also created a new dsi controller for the DSI2 standard, with >>>> a vastly different registers layout. >>>> >>>> I guess at some point there is time to say this really is a new IP ;-) . >>>> >>>> >>>> Though while on that thought, I don't fully understand why both a >>>> compiled >>>> under the dw_hdmi kconfig symbol. People going for a minimal kernel might >>>> want one or the other, but not both for their specific board. >> >> Indeed, it makes sense to have a dedicated Kconfig option. This is >> mostly a leftover from downstream implementation, will fix in v2. >> >> Thanks again, >> Cristian >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Jun 05, 2024 at 11:28:41AM GMT, neil.armstrong@linaro.org wrote: > Hi, > > On 05/06/2024 11:25, Andy Yan wrote: > > > > Hi, > > > > At 2024-06-05 04:33:57, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote: > > > On 6/3/24 4:08 PM, neil.armstrong@linaro.org wrote: > > > > Hi, > > > > > > > > On 03/06/2024 15:03, Heiko Stuebner wrote: > > > > > Am Montag, 3. Juni 2024, 14:14:17 CEST schrieb Andy Yan: > > > > > > Hi Neil: > > > > > > > > > > > > On 6/3/24 16:55, Neil Armstrong wrote: > > > > > > > Hi Christian, > > > > > > > > > > > > > > On 01/06/2024 15:12, Cristian Ciocaltea wrote: > > > > > > > > The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the > > > > > > > > Synopsys DesignWare HDMI TX controller used in the previous SoCs. > > > > > > > > > > > > > > > > It is HDMI 2.1 compliant and supports the following features, among > > > > > > > > others: > > > > > > > > > > > > > > > . > > > > > > > > > > > > > > .. > > > > > > > > > > > > > > > * SCDC I2C DDC access > > > > > > > > * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 > > > > > > > > * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds > > > > > > > > * Multi-stream audio > > > > > > > > * Enhanced Audio Return Channel (EARC) > > > > > > > -> Those features were already supported by the HDMI 2.0a compliant > > > > > > > HW, just > > > > > > > list the _new_ features for HDMI 2.1 > > > > > > > > > > > > > > I did a quick review of your patchset and I don't understand why you > > > > > > > need > > > > > > > to add a separate dw-hdmi-qp.c since you only need simple variants > > > > > > > of the I2C > > > > > > > bus, infoframe and bridge setup. > > > > > > > > > > > > > > Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller > > > > > > > version > > > > > > > detectable at runtime ? > > > > > > > > > > > > > > I would prefer to keep a single dw-hdmi driver if possible. > > > > > > > > > > > > > > > > > > > > > > > > The QP HDMI controller is a completely different variant with totally > > > > > > different > > > > > > registers layout, see PATCH 13/14. > > > > > > I think make it a separate driver will be easier for development and > > > > > > maintenance. > > > > > > > > > > I'm with Andy here. Trying to navigate a driver for two IP blocks really > > > > > sounds taxing especially when both are so different. > > > > > > Thank you all for the valuable feedback! > > > > > > > I agree, I just wanted more details than "variant of the > > > > Synopsys DesignWare HDMI TX controller", if the register mapping is 100% > > > > different, and does not match at all with the old IP, then it's indeed time > > > > to make a brand new driver, but instead of doing a mix up, it's time to > > > > extract > > > > the dw-hdmi code that could be common helpers into a dw-hdmi-common module > > > > and use them. > > > > > > Sounds good, will handle this in v2. > > > > > > > As I see, no "driver" code can be shared, only DRM plumbings, so perhaps > > > > those > > > > plumbing code should go into the DRM core ? > > > > > > > > In any case, please add more details on the cover letter, including the > > > > detailed > > > > HW differrence and the design you chose so support this new IP. > > > > > > Andy, could you please help with a summary of the HW changes? > > > The information I could provide is rather limited, since I don't have > > > access to any DW IP datasheets and I'm also not familiar enough with the > > > old variant. > > > > > Accurately, we should refer to it as an entirely new IP，it has nothing in common with > > the current mainline dw-hdmi。 The only commonality is that they both come from > > Synopsys DesignWare： > > （1）It has a 100% different register mapping > > （2）It supports FRL and DSC > > （3）different configuration flow in many places。 > > > > So I have the same feeling with Heiko and Maxime： > > The DW_HDMI_QP should have a separate driver and with it's own CONFIG such as DRM_DW_HDMI_QP in Kconfig. > > and the rockchip part should also be split from dw_hdmi-rockchip.c. > > I am sorry we mixed them in dw_hdmi-rockchip.c when we develop the bsp driver，but we really regretted this decision > > when we repeatedly broke compatibility with dw-hdmi on other socs。 > > Yes please, and as I say, if there's code common with the old dw-hdmi, please add a common > module if this code can't be moved in core bridge helpers. And chances are that the common code is actually there to deal with HDMI spec itself and not really the hardware, which is solved by moving both drivers to the HDMI helpers that just got merged. Maxime
Hi, At 2024-06-05 17:39:48, "Maxime Ripard" <mripard@kernel.org> wrote: >On Wed, Jun 05, 2024 at 11:28:41AM GMT, neil.armstrong@linaro.org wrote: >> Hi, >> >> On 05/06/2024 11:25, Andy Yan wrote: >> > >> > Hi, >> > >> > At 2024-06-05 04:33:57, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote: >> > > On 6/3/24 4:08 PM, neil.armstrong@linaro.org wrote: >> > > > Hi, >> > > > >> > > > On 03/06/2024 15:03, Heiko Stuebner wrote: >> > > > > Am Montag, 3. Juni 2024, 14:14:17 CEST schrieb Andy Yan: >> > > > > > Hi Neil: >> > > > > > >> > > > > > On 6/3/24 16:55, Neil Armstrong wrote: >> > > > > > > Hi Christian, >> > > > > > > >> > > > > > > On 01/06/2024 15:12, Cristian Ciocaltea wrote: >> > > > > > > > The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the >> > > > > > > > Synopsys DesignWare HDMI TX controller used in the previous SoCs. >> > > > > > > > >> > > > > > > > It is HDMI 2.1 compliant and supports the following features, among >> > > > > > > > others: >> > > > > > > > >> > > > > > > . >> > > > > > > >> > > > > > > .. >> > > > > > > >> > > > > > > > * SCDC I2C DDC access >> > > > > > > > * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 >> > > > > > > > * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds >> > > > > > > > * Multi-stream audio >> > > > > > > > * Enhanced Audio Return Channel (EARC) >> > > > > > > -> Those features were already supported by the HDMI 2.0a compliant >> > > > > > > HW, just >> > > > > > > list the _new_ features for HDMI 2.1 >> > > > > > > >> > > > > > > I did a quick review of your patchset and I don't understand why you >> > > > > > > need >> > > > > > > to add a separate dw-hdmi-qp.c since you only need simple variants >> > > > > > > of the I2C >> > > > > > > bus, infoframe and bridge setup. >> > > > > > > >> > > > > > > Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller >> > > > > > > version >> > > > > > > detectable at runtime ? >> > > > > > > >> > > > > > > I would prefer to keep a single dw-hdmi driver if possible. >> > > > > > >> > > > > > >> > > > > > >> > > > > > The QP HDMI controller is a completely different variant with totally >> > > > > > different >> > > > > > registers layout, see PATCH 13/14. >> > > > > > I think make it a separate driver will be easier for development and >> > > > > > maintenance. >> > > > > >> > > > > I'm with Andy here. Trying to navigate a driver for two IP blocks really >> > > > > sounds taxing especially when both are so different. >> > > >> > > Thank you all for the valuable feedback! >> > > >> > > > I agree, I just wanted more details than "variant of the >> > > > Synopsys DesignWare HDMI TX controller", if the register mapping is 100% >> > > > different, and does not match at all with the old IP, then it's indeed time >> > > > to make a brand new driver, but instead of doing a mix up, it's time to >> > > > extract >> > > > the dw-hdmi code that could be common helpers into a dw-hdmi-common module >> > > > and use them. >> > > >> > > Sounds good, will handle this in v2. >> > > >> > > > As I see, no "driver" code can be shared, only DRM plumbings, so perhaps >> > > > those >> > > > plumbing code should go into the DRM core ? >> > > > >> > > > In any case, please add more details on the cover letter, including the >> > > > detailed >> > > > HW differrence and the design you chose so support this new IP. >> > > >> > > Andy, could you please help with a summary of the HW changes? >> > > The information I could provide is rather limited, since I don't have >> > > access to any DW IP datasheets and I'm also not familiar enough with the >> > > old variant. >> > > >> > Accurately, we should refer to it as an entirely new IP，it has nothing in common with >> > the current mainline dw-hdmi。 The only commonality is that they both come from >> > Synopsys DesignWare： >> > （1）It has a 100% different register mapping >> > （2）It supports FRL and DSC >> > （3）different configuration flow in many places。 >> > >> > So I have the same feeling with Heiko and Maxime： >> > The DW_HDMI_QP should have a separate driver and with it's own CONFIG such as DRM_DW_HDMI_QP in Kconfig. >> > and the rockchip part should also be split from dw_hdmi-rockchip.c. >> > I am sorry we mixed them in dw_hdmi-rockchip.c when we develop the bsp driver，but we really regretted this decision >> > when we repeatedly broke compatibility with dw-hdmi on other socs。 >> >> Yes please, and as I say, if there's code common with the old dw-hdmi, please add a common >> module if this code can't be moved in core bridge helpers. > >And chances are that the common code is actually there to deal with HDMI >spec itself and not really the hardware, which is solved by moving both >drivers to the HDMI helpers that just got merged. > Yes, +1. I don't think we need to share some common code with dw-hdmi here. >Maxime
On 6/5/24 12:49 PM, Andy Yan wrote: > > Hi, > At 2024-06-05 17:39:48, "Maxime Ripard" <mripard@kernel.org> wrote: >> On Wed, Jun 05, 2024 at 11:28:41AM GMT, neil.armstrong@linaro.org wrote: >>> Hi, >>> >>> On 05/06/2024 11:25, Andy Yan wrote: >>>> >>>> Hi, >>>> >>>> At 2024-06-05 04:33:57, "Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> wrote: >>>>> On 6/3/24 4:08 PM, neil.armstrong@linaro.org wrote: >>>>>> Hi, >>>>>> >>>>>> On 03/06/2024 15:03, Heiko Stuebner wrote: >>>>>>> Am Montag, 3. Juni 2024, 14:14:17 CEST schrieb Andy Yan: >>>>>>>> Hi Neil: >>>>>>>> >>>>>>>> On 6/3/24 16:55, Neil Armstrong wrote: >>>>>>>>> Hi Christian, >>>>>>>>> >>>>>>>>> On 01/06/2024 15:12, Cristian Ciocaltea wrote: >>>>>>>>>> The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the >>>>>>>>>> Synopsys DesignWare HDMI TX controller used in the previous SoCs. >>>>>>>>>> >>>>>>>>>> It is HDMI 2.1 compliant and supports the following features, among >>>>>>>>>> others: >>>>>>>>>> >>>>>>>>> . >>>>>>>>> >>>>>>>>> .. >>>>>>>>> >>>>>>>>>> * SCDC I2C DDC access >>>>>>>>>> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 >>>>>>>>>> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds >>>>>>>>>> * Multi-stream audio >>>>>>>>>> * Enhanced Audio Return Channel (EARC) >>>>>>>>> -> Those features were already supported by the HDMI 2.0a compliant >>>>>>>>> HW, just >>>>>>>>> list the _new_ features for HDMI 2.1 >>>>>>>>> >>>>>>>>> I did a quick review of your patchset and I don't understand why you >>>>>>>>> need >>>>>>>>> to add a separate dw-hdmi-qp.c since you only need simple variants >>>>>>>>> of the I2C >>>>>>>>> bus, infoframe and bridge setup. >>>>>>>>> >>>>>>>>> Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller >>>>>>>>> version >>>>>>>>> detectable at runtime ? >>>>>>>>> >>>>>>>>> I would prefer to keep a single dw-hdmi driver if possible. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> The QP HDMI controller is a completely different variant with totally >>>>>>>> different >>>>>>>> registers layout, see PATCH 13/14. >>>>>>>> I think make it a separate driver will be easier for development and >>>>>>>> maintenance. >>>>>>> >>>>>>> I'm with Andy here. Trying to navigate a driver for two IP blocks really >>>>>>> sounds taxing especially when both are so different. >>>>> >>>>> Thank you all for the valuable feedback! >>>>> >>>>>> I agree, I just wanted more details than "variant of the >>>>>> Synopsys DesignWare HDMI TX controller", if the register mapping is 100% >>>>>> different, and does not match at all with the old IP, then it's indeed time >>>>>> to make a brand new driver, but instead of doing a mix up, it's time to >>>>>> extract >>>>>> the dw-hdmi code that could be common helpers into a dw-hdmi-common module >>>>>> and use them. >>>>> >>>>> Sounds good, will handle this in v2. >>>>> >>>>>> As I see, no "driver" code can be shared, only DRM plumbings, so perhaps >>>>>> those >>>>>> plumbing code should go into the DRM core ? >>>>>> >>>>>> In any case, please add more details on the cover letter, including the >>>>>> detailed >>>>>> HW differrence and the design you chose so support this new IP. >>>>> >>>>> Andy, could you please help with a summary of the HW changes? >>>>> The information I could provide is rather limited, since I don't have >>>>> access to any DW IP datasheets and I'm also not familiar enough with the >>>>> old variant. >>>>> >>>> Accurately, we should refer to it as an entirely new IP，it has nothing in common with >>>> the current mainline dw-hdmi。 The only commonality is that they both come from >>>> Synopsys DesignWare： >>>> （1）It has a 100% different register mapping >>>> （2）It supports FRL and DSC >>>> （3）different configuration flow in many places。 >>>> >>>> So I have the same feeling with Heiko and Maxime： >>>> The DW_HDMI_QP should have a separate driver and with it's own CONFIG such as DRM_DW_HDMI_QP in Kconfig. >>>> and the rockchip part should also be split from dw_hdmi-rockchip.c. >>>> I am sorry we mixed them in dw_hdmi-rockchip.c when we develop the bsp driver，but we really regretted this decision >>>> when we repeatedly broke compatibility with dw-hdmi on other socs。 >>> >>> Yes please, and as I say, if there's code common with the old dw-hdmi, please add a common >>> module if this code can't be moved in core bridge helpers. >> >> And chances are that the common code is actually there to deal with HDMI >> spec itself and not really the hardware, which is solved by moving both >> drivers to the HDMI helpers that just got merged. I will make use of the new HDMI helpers and see if there is anything else remaining in terms of common code. > Yes, +1. > I don't think we need to share some common code with dw-hdmi here. Ok, I will completely separate the new driver's code, including the Rockchip glue layer. Thanks, Cristian
Cristian, I'm hacking with adding cec support for rk3588 hdmi on 6.10-rc7 mainline. Cec kernel module is my backport from bsp. Module loads. Cec line (observed on osciloscope) has pulses when i'm issuing i.e. cec-ctl -d /dev/cec0 --phys-addr=1.0.0.0 —playback My issue is that timings are 2,9 times longer that should be (start bit is 10,7mS instead of 3.6; zero is 4.4 instead 1.5 while one is 1,7 instead of 0.6). This suggests me issue is cec clock isn't? Looking on bsp code https://github.com/radxa/kernel/blob/linux-6.1-stan-rkr1/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp-cec.c#L186 there is nothing with clock. So probably issue is in 3588 clk code, or…… Maybe you have some hints how to move forward with this issue? > Wiadomość napisana przez Cristian Ciocaltea <cristian.ciocaltea@collabora.com> w dniu 01.06.2024, o godz. 15:12: > > The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the > Synopsys DesignWare HDMI TX controller used in the previous SoCs. > > It is HDMI 2.1 compliant and supports the following features, among > others: > > * Fixed Rate Link (FRL) > * 4K@120Hz and 8K@60Hz video modes > * Variable Refresh Rate (VRR) including Quick Media Switching (QMS) > * Fast Vactive (FVA) > * SCDC I2C DDC access > * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 > * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds > * Multi-stream audio > * Enhanced Audio Return Channel (EARC) > > This is the last required 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. > > The patches are grouped as follows: > * PATCH 1..7: DW HDMI TX driver refactor to minimize code duplication in > the new QP driver (no functional changes intended) > > * PATCH 8..11: Rockchip DW HDMI glue driver cleanup/improvements (no > functional changes intended) > > * PATCH 12..13: The new DW HDMI QP TX driver reusing the previously > exported functions and structs from existing DW HDMI TX driver > > * PATCH 14: Rockchip DW HDMI glue driver update to support RK3588 and > make use of DW HDMI QP TX > > They provide just the basic HDMI support for now, i.e. RGB output up to > 4K@60Hz, without audio, CEC or any of the HDMI 2.1 specific features. > Also note the vop2 driver is currently 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 3A and 5B boards. > > 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-v6.10-rc1 > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > Cristian Ciocaltea (14): > drm/bridge: dw-hdmi: Simplify clock handling > drm/bridge: dw-hdmi: Add dw-hdmi-common.h header > drm/bridge: dw-hdmi: Commonize dw_hdmi_i2c_adapter() > drm/bridge: dw-hdmi: Factor out AVI infoframe setup > drm/bridge: dw-hdmi: Factor out vmode setup > drm/bridge: dw-hdmi: Factor out hdmi_data_info setup > drm/bridge: dw-hdmi: Commonize dw_hdmi_connector_create() > drm/rockchip: dw_hdmi: Use modern drm_device based logging > drm/rockchip: dw_hdmi: Simplify clock handling > drm/rockchip: dw_hdmi: Use devm_regulator_get_enable() > drm/rockchip: dw_hdmi: Drop superfluous assignments of mpll_cfg, cur_ctr and phy_config > dt-bindings: display: rockchip,dw-hdmi: Add compatible for RK3588 > drm/bridge: synopsys: Add DW HDMI QP TX controller driver > drm/rockchip: dw_hdmi: Add basic RK3588 support > > .../display/rockchip/rockchip,dw-hdmi.yaml | 127 +++- > drivers/gpu/drm/bridge/synopsys/Makefile | 2 +- > drivers/gpu/drm/bridge/synopsys/dw-hdmi-common.h | 179 +++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 787 +++++++++++++++++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h | 831 +++++++++++++++++++++ > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 353 +++------ > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 351 +++++++-- > include/drm/bridge/dw_hdmi.h | 8 + > 8 files changed, 2290 insertions(+), 348 deletions(-) > --- > base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 > change-id: 20240601-b4-rk3588-bridge-upstream-a27baff1b8fc > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the Synopsys DesignWare HDMI TX controller used in the previous SoCs. It is HDMI 2.1 compliant and supports the following features, among others: * Fixed Rate Link (FRL) * 4K@120Hz and 8K@60Hz video modes * Variable Refresh Rate (VRR) including Quick Media Switching (QMS) * Fast Vactive (FVA) * SCDC I2C DDC access * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4 * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds * Multi-stream audio * Enhanced Audio Return Channel (EARC) This is the last required 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. The patches are grouped as follows: * PATCH 1..7: DW HDMI TX driver refactor to minimize code duplication in the new QP driver (no functional changes intended) * PATCH 8..11: Rockchip DW HDMI glue driver cleanup/improvements (no functional changes intended) * PATCH 12..13: The new DW HDMI QP TX driver reusing the previously exported functions and structs from existing DW HDMI TX driver * PATCH 14: Rockchip DW HDMI glue driver update to support RK3588 and make use of DW HDMI QP TX They provide just the basic HDMI support for now, i.e. RGB output up to 4K@60Hz, without audio, CEC or any of the HDMI 2.1 specific features. Also note the vop2 driver is currently 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 3A and 5B boards. 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-v6.10-rc1 Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> --- Cristian Ciocaltea (14): drm/bridge: dw-hdmi: Simplify clock handling drm/bridge: dw-hdmi: Add dw-hdmi-common.h header drm/bridge: dw-hdmi: Commonize dw_hdmi_i2c_adapter() drm/bridge: dw-hdmi: Factor out AVI infoframe setup drm/bridge: dw-hdmi: Factor out vmode setup drm/bridge: dw-hdmi: Factor out hdmi_data_info setup drm/bridge: dw-hdmi: Commonize dw_hdmi_connector_create() drm/rockchip: dw_hdmi: Use modern drm_device based logging drm/rockchip: dw_hdmi: Simplify clock handling drm/rockchip: dw_hdmi: Use devm_regulator_get_enable() drm/rockchip: dw_hdmi: Drop superfluous assignments of mpll_cfg, cur_ctr and phy_config dt-bindings: display: rockchip,dw-hdmi: Add compatible for RK3588 drm/bridge: synopsys: Add DW HDMI QP TX controller driver drm/rockchip: dw_hdmi: Add basic RK3588 support .../display/rockchip/rockchip,dw-hdmi.yaml | 127 +++- drivers/gpu/drm/bridge/synopsys/Makefile | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi-common.h | 179 +++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 787 +++++++++++++++++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h | 831 +++++++++++++++++++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 353 +++------ drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 351 +++++++-- include/drm/bridge/dw_hdmi.h | 8 + 8 files changed, 2290 insertions(+), 348 deletions(-) --- base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 change-id: 20240601-b4-rk3588-bridge-upstream-a27baff1b8fc