From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9241B3EC808; Mon, 14 Sep 2026 15:47:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=210.118.77.12 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789400838; cv=none; b=hgaw0RgVrdboSsCE5IG1uWTd8vZNW3gxwC07RKWf8d4gjvf7I1zs42x0zq2j/N2k43XVbmw5spGt51q73XlpWIAEfTsJ9hWmm5QGZ8JEyUkaVqUM2lD1GHGQympkejW8uh6J1tvmPQtzjBFrLQl7T8NvWFuoZwOeFwVlWOU/nB8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789400838; c=relaxed/simple; bh=4iSi10eoQTZH7LplbJsxbc1cU7ckseg31EKl5rggNsU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:From:In-Reply-To: Content-Type:References; b=d/24fH+tqw25diy4x5anTog0eaSPz+HFjmqP4HicBgrxaCx7vba+wZZJSjYCTUC/tjr5u1F79zs1Lkh4swy9lvGIErbV37fFwwPBtBp2ruVgS1UFouCFZ4rtM0xEDTqr6NytmxuutMRDXasU9B90+MILMslaQczZCF0xoVydJwU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=samsung.com; spf=pass smtp.mailfrom=samsung.com; dkim=pass (1024-bit key) header.d=samsung.com header.i=@samsung.com header.b=GDO61VSO; arc=none smtp.client-ip=210.118.77.12 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=samsung.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=samsung.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=samsung.com header.i=@samsung.com header.b="GDO61VSO" Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20260914154712euoutp02c5051cd29f404323cd1ce4ebcf1a2814~VOpOJJKAf2060820608euoutp02_; Mon, 14 Sep 2026 15:47:12 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20260914154712euoutp02c5051cd29f404323cd1ce4ebcf1a2814~VOpOJJKAf2060820608euoutp02_ DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1789400832; bh=tG6YbD+cncbaU7j7yoO6MEtQ744hWAYh4LfOY6utBYg=; h=Date:Subject:To:Cc:From:In-Reply-To:References:From; b=GDO61VSOCVO3k1kf5K78UUt2+RmSvMQDujkUqqiRwIRQp54H3XA3u6HAxA1Bzo9g/ tCvyeIbekxEClZFD369Hz0B8bRlNt3ZyQYNKC/wf2QdhH9jD28ZIA21IDZJCW8YtoX k3/tT/pItRCsYTnCqKU6miJvFJQAcAS4lM6izfts= Received: from eusmtip1.samsung.com (unknown [203.254.199.221]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20260914154711eucas1p1a9c438d50d10b5aa67be8d91d3c6bd8b~VOpNwZ4qm0255102551eucas1p1I; Mon, 14 Sep 2026 15:47:11 +0000 (GMT) Received: from [192.168.1.44] (unknown [106.210.136.40]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20260914154709eusmtip1d1d42067dc0186ff40e0a824dfe3578c~VOpL4YvZE0990509905eusmtip17; Mon, 14 Sep 2026 15:47:09 +0000 (GMT) Message-ID: <03a707e8-9382-4d48-9bdc-08fbd0131587@samsung.com> Date: Mon, 14 Sep 2026 17:47:04 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 14/19] drm/bridge: starfive: Add JH7110 HDMI controller driver To: Icenowy Zheng , Vinod Koul , Neil Armstrong , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Andrzej Hajda , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Luca Ceresoli , David Airlie , Simona Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Lee Jones , Andy Yan , Philipp Zabel , Emil Renner Berthing , Hal Feng , Michael Turquette , Stephen Boyd , Brian Masney , Heiko Stuebner , Conor Dooley , Paul Walmsley , Palmer Dabbelt , Albert Ou , Alexandre Ghiti , Dominique Belhachemi , Brian Masney , Jerome Brunet Cc: linux-phy@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, mfd@lists.linux.dev, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-riscv@lists.infradead.org, Andy Yan , Marek Szyprowski , Maud Spierings , Graham Markall Content-Language: en-US From: Michal Wilczynski In-Reply-To: Content-Transfer-Encoding: 8bit X-CMS-MailID: 20260914154711eucas1p1a9c438d50d10b5aa67be8d91d3c6bd8b X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20260904132735eucas1p2c898afe4a4c6e957a7b9eedff321a6dc X-EPHeader: CA X-CMS-RootMailID: 20260904132735eucas1p2c898afe4a4c6e957a7b9eedff321a6dc References: <20260904-jh7110-clean-send-v3-0-484f9ae72715@samsung.com> <20260904-jh7110-clean-send-v3-14-484f9ae72715@samsung.com> On 9/4/26 15:39, Icenowy Zheng wrote: > 在 2026-09-04五的 15:27 +0200，Michal Wilczynski写道： >> Add the HDMI controller (bridge) driver for the StarFive JH7110. >> >> This driver binds to the starfive,jh7110-inno-hdmi-controller node. >> It gets its shared regmap from its parent and its register access, >> module and bus clocks from voutcrg. It consumes the pixel clock and >> the >> PHY from its hdmi_phy sibling. >> >> The driver calls the generic inno_hdmi_probe function and passes the >> shared regmap to it, registering as a DRM bridge. The .enable hook is >> responsible for setting the PHY's pixel clock rate via clk_set_rate() >> and powering on the PHY via phy_power_on(). >> >> The DC8200 has two panels, each exposing a DP and a DPI interface, >> and a >> mux in the video output system controller picks which of them drives >> the >> HDMI transmitter. Program that mux from the port graph rather than >> relying on whatever the bootloader left behind, taking the panel from >> the >> remote port number and the interface from the remote endpoint number. >> >> The generic driver holds the clock it looks up as the register access >> clock enabled for its lifetime, and derives the DDC divider from that >> clock's rate, so point it at the system clock. Naming the pixel clock >> there instead would keep the PHY pre-PLL powered from probe onwards >> and >> size the divider from the wrong rate. >> >> The PHY can only generate the discrete set of pixel clocks described >> by >> its pre-PLL table, so .mode_valid rejects any mode clk_round_rate() >> cannot satisfy. Without it such a mode would be advertised to >> userspace >> and the modeset would appear to succeed while the display stayed >> blank. >> >> .enable returns early when the rate is unsupported or the PHY fails >> to >> power on, so track whether the pixel clock was actually enabled and >> let >> .disable tear down only what was brought up, otherwise the clock >> refcount underflows. >> >> The clocks and the reset are torn down through devm rather than from >> .remove, so that they outlive the bridge that inno_hdmi_probe() adds >> with >> devm_drm_bridge_add(). Releasing them in .remove runs before devres >> unwinds and would leave the bridge registered with its clocks already >> gated. >> >> Signed-off-by: Michal Wilczynski >> --- >> drivers/gpu/drm/bridge/Kconfig | 11 ++ >> drivers/gpu/drm/bridge/Makefile | 1 + >> drivers/gpu/drm/bridge/jh7110-inno-hdmi.c | 318 >> ++++++++++++++++++++++++++++++ >> 3 files changed, 330 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/Kconfig >> b/drivers/gpu/drm/bridge/Kconfig >> index >> 4a57d49b4c6d3ab4b965228835b372d191647197..75b1cf6727d5a32310dcf9fe573 >> 4d95e14eea8fe 100644 >> --- a/drivers/gpu/drm/bridge/Kconfig >> +++ b/drivers/gpu/drm/bridge/Kconfig [snip] >> + >> + /* Data mapping: 8-bit RGB on whichever interface is in use. >> */ >> + mask = VOUT_HDMI_DPI_DP_SEL | VOUT_HDMI_DP_BIT_DEPTH | >> + VOUT_HDMI_DP_YUV_MODE | VOUT_HDMI_DPI_BIT_DEPTH; >> + val = FIELD_PREP(VOUT_HDMI_DPI_DP_SEL, endpoint.id) | >> + FIELD_PREP(VOUT_HDMI_DP_YUV_MODE, >> VOUT_HDMI_DP_YUV_MODE_RGB) | >> + FIELD_PREP(VOUT_HDMI_DPI_BIT_DEPTH, >> VOUT_HDMI_DPI_BIT_DEPTH_8BIT); > > Well it looks like the vendor driver never sets DP interface, is it > tested? I doubt whether the SoC designer messed it up. The DP interface is documented in the TRM and I've also tested it so it does work indeed. Here is my test branch if you would like to see for youself. [1] - https://github.com/mwilczy/linux/commit/7efcba3b0be30734087c79f5773cae19245924db [snip] > >> +} >> + >> +/* >> + * This table is now only used for the generic .mode_valid check. >> + * The real validation happens in the PHY driver's .round_rate. >> + */ >> +static struct inno_hdmi_phy_config stf_hdmi_phy_configs[] = { >> + { 297000000, 0x00, 0x00 }, >> + { ~0UL, 0x00, 0x00 }, /* Sentinel */ >> +}; > > If it's just such a upper bound, why don't just override the function > as a bound check? Yeah actually patch 10 implements mode_valid callback, so it's better to remove this stub altogether, previously this was done satisfy the probe check reasons - v4 will add patch that makes this table optional in inno-hdmi driver. > > Or... should the real table be used here? I start to wonder whether > this is related to Maud's failure on the Framework panel. Maud panel needed another entry in PHY pre-PLL table. Patch 17 in v3 adds this entry. There is a new issue reported by Maud, I will answer that in a separate thread. > > Thanks, > Icenowy > >> + >> +static const struct inno_hdmi_plat_ops stf_inno_hdmi_plat_ops = { >> + .enable = inno_hdmi_starfive_enable, >> + .disable = inno_hdmi_starfive_disable, >> + .mode_valid = inno_hdmi_starfive_mode_valid, >> +}; >> + >> +static const struct inno_hdmi_plat_data stf_inno_hdmi_plat_data = { >> + .ops = &stf_inno_hdmi_plat_ops, >> + .phy_configs = stf_hdmi_phy_configs, >> + .default_phy_config = &stf_hdmi_phy_configs[0], >> +}; >> + >> +static const struct of_device_id starfive_hdmi_controller_dt_ids[] = >> { >> + { .compatible = "starfive,jh7110-inno-hdmi-controller", >> + .data = &stf_inno_hdmi_plat_data }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(of, starfive_hdmi_controller_dt_ids); >> + >> +struct platform_driver starfive_inno_hdmi_controller_driver = { >> + .probe = starfive_inno_hdmi_controller_probe, >> + .driver = { >> + .name = "starfive-inno-hdmi-controller", >> + .of_match_table = starfive_hdmi_controller_dt_ids, >> + }, >> +}; >> +module_platform_driver(starfive_inno_hdmi_controller_driver); >> + >> +MODULE_AUTHOR("Michal Wilczynski "); >> +MODULE_DESCRIPTION("StarFive INNO HDMI Controller Driver"); >> +MODULE_LICENSE("GPL"); > > Best regards, -- Michal Wilczynski