| Message ID | 20240203165307.7806-1-aford173@gmail.com |
|---|---|
| Headers | show |
| Series | soc: imx8mp: Add support for HDMI | expand |
Le 03/02/2024 à 17:52, Adam Ford a écrit : > From: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> > > This adds the driver for the Samsung HDMI PHY found on the > i.MX8MP SoC. > > Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> > Signed-off-by: Adam Ford <aford173-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Tested-by: Alexander Stein <alexander.stein-W3o+9BuWjQaZox4op4iWzw@public.gmane.org> > --- ... > +static int fsl_samsung_hdmi_phy_probe(struct platform_device *pdev) > +{ > + struct fsl_samsung_hdmi_phy *phy; > + int ret; > + > + phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL); > + if (!phy) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, phy); > + phy->dev = &pdev->dev; > + > + phy->regs = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(phy->regs)) > + return PTR_ERR(phy->regs); > + > + phy->apbclk = devm_clk_get(phy->dev, "apb"); > + if (IS_ERR(phy->apbclk)) > + return dev_err_probe(phy->dev, PTR_ERR(phy->apbclk), > + "failed to get apb clk\n"); > + > + phy->refclk = devm_clk_get(phy->dev, "ref"); > + if (IS_ERR(phy->refclk)) > + return dev_err_probe(phy->dev, PTR_ERR(phy->refclk), > + "failed to get ref clk\n"); > + > + ret = clk_prepare_enable(phy->apbclk); > + if (ret) { > + dev_err(phy->dev, "failed to enable apbclk\n"); > + return ret; > + } > + > + pm_runtime_get_noresume(phy->dev); > + pm_runtime_set_active(phy->dev); > + pm_runtime_enable(phy->dev); > + > + ret = phy_clk_register(phy); > + if (ret) { > + dev_err(&pdev->dev, "register clk failed\n"); > + goto register_clk_failed; > + } > + > + pm_runtime_put(phy->dev); > + > + return 0; > + > +register_clk_failed: > + clk_disable_unprepare(phy->apbclk); > + > + return ret; > +} > + > +static int fsl_samsung_hdmi_phy_remove(struct platform_device *pdev) > +{ > + of_clk_del_provider(pdev->dev.of_node); A clk_disable_unprepare(phy->apbclk) call seems to be missing here. Or maybe devm_clk_get_enabled() should be used for 'apbclk'? CJ > + > + return 0; > +} ...
On Sat, 3 Feb 2024 at 17:53, Adam Ford <aford173@gmail.com> wrote: > > From: Lucas Stach <l.stach@pengutronix.de> > > This adds the driver for the Samsung HDMI PHY found on the > i.MX8MP SoC. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > Signed-off-by: Adam Ford <aford173@gmail.com> > Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > V4: Make lookup table hex values lower case. > > V3: Re-order the Makefile to keep it alphabetical > Remove unused defines > > V2: Fixed some whitespace found from checkpatch > Change error handling when enabling apbclk to use dev_err_probe > Rebase on Linux-Next > > I (Adam) tried to help move this along, so I took Lucas' patch and > attempted to apply fixes based on feedback. I don't have > all the history, so apologies for that. > --- > drivers/phy/freescale/Kconfig | 6 + > drivers/phy/freescale/Makefile | 1 + > drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 1075 ++++++++++++++++++ > 3 files changed, 1082 insertions(+) > create mode 100644 drivers/phy/freescale/phy-fsl-samsung-hdmi.c > > diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig > index 853958fb2c06..5c2b73042dfc 100644 > --- a/drivers/phy/freescale/Kconfig > +++ b/drivers/phy/freescale/Kconfig > @@ -35,6 +35,12 @@ config PHY_FSL_IMX8M_PCIE > Enable this to add support for the PCIE PHY as found on > i.MX8M family of SOCs. > > +config PHY_FSL_SAMSUNG_HDMI_PHY > + tristate "Samsung HDMI PHY support" > + depends on OF && HAS_IOMEM > + help > + Enable this to add support for the Samsung HDMI PHY in i.MX8MP. > + > endif > > config PHY_FSL_LYNX_28G > diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile > index cedb328bc4d2..79e5f16d3ce8 100644 > --- a/drivers/phy/freescale/Makefile > +++ b/drivers/phy/freescale/Makefile > @@ -4,3 +4,4 @@ obj-$(CONFIG_PHY_MIXEL_LVDS_PHY) += phy-fsl-imx8qm-lvds-phy.o > obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY) += phy-fsl-imx8-mipi-dphy.o > obj-$(CONFIG_PHY_FSL_IMX8M_PCIE) += phy-fsl-imx8m-pcie.o > obj-$(CONFIG_PHY_FSL_LYNX_28G) += phy-fsl-lynx-28g.o > +obj-$(CONFIG_PHY_FSL_SAMSUNG_HDMI_PHY) += phy-fsl-samsung-hdmi.o > diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c > new file mode 100644 > index 000000000000..bf0e2299d00f > --- /dev/null > +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c > @@ -0,0 +1,1075 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2020 NXP > + * Copyright 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de> > + */ > + > +#include <linux/clk-provider.h> > +#include <linux/clk.h> > +#include <linux/delay.h> > +#include <linux/io.h> > +#include <linux/iopoll.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > + > +#define PHY_REG_33 0x84 > +#define REG33_MODE_SET_DONE BIT(7) > +#define REG33_FIX_DA BIT(1) > + > +#define PHY_REG_34 0x88 > +#define REG34_PHY_READY BIT(7) > +#define REG34_PLL_LOCK BIT(6) > +#define REG34_PHY_CLK_READY BIT(5) > + > + > +#define PHY_PLL_REGS_NUM 48 > + > +struct phy_config { > + u32 clk_rate; > + u8 regs[PHY_PLL_REGS_NUM]; > +}; > + > +const struct phy_config phy_pll_cfg[] = { > + { 22250000, { > + 0x00, 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40, > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x15, 0x25, 0x80, > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32, > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00, > + }, > + }, { > + 23750000, { > + 0x00, 0xd1, 0x50, 0xf1, 0x86, 0x85, 0x80, 0x40, > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x03, 0x25, 0x80, > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32, > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00, > + }, Generally I see that these entries contain a high level of duplication. Could you please extract the common part and a rate-dependent part. Next, it would be best if instead of writing the register values via the rate LUT, the driver could calculate those values. This allows us to support other HDMI modes if the need arises at some point. > + }, { > + 24000000, { > + 0x00, 0xd1, 0x50, 0xf0, 0x00, 0x00, 0x80, 0x00, > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x01, 0x25, 0x80, > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32, > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00, > + },
On Sat, Feb 03, 2024 at 10:52:46AM -0600, Adam Ford wrote: > From: Lucas Stach <l.stach@pengutronix.de> > > The HDMI irqsteer is a secondary interrupt controller within the HDMI > subsystem that maps all HDMI peripheral IRQs into a single upstream > IRQ line. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> This is missing your signed-off-by, and in other patches of this series your signed-off-by is not the last, as it should be. Please have a look and fix this and the other instances. Thanks for this work! Francesco
On Sun, Feb 4, 2024 at 6:00 AM Francesco Dolcini <francesco@dolcini.it> wrote: > > On Sat, Feb 03, 2024 at 10:52:46AM -0600, Adam Ford wrote: > > From: Lucas Stach <l.stach@pengutronix.de> > > > > The HDMI irqsteer is a secondary interrupt controller within the HDMI > > subsystem that maps all HDMI peripheral IRQs into a single upstream > > IRQ line. > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > This is missing your signed-off-by, and in other patches of this series Opps. I thought I caught all those. > your signed-off-by is not the last, as it should be. > > Please have a look and fix this and the other instances. > OK. I have some work to do on some other portions, so I'll clean up that too. adam > Thanks for this work! Thanks for the feedback. adam > > Francesco >
Hi Adam, thanks for working on this. Am Samstag, 3. Februar 2024, 17:52:45 CET schrieb Adam Ford: > From: Lucas Stach <l.stach@pengutronix.de> > > This adds the PGC and HDMI blk-ctrl nodes providing power control for > HDMI subsystem peripherals. > > Signed-off-by: Adam Ford <aford173@gmail.com> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > V2: Add missing power-domains hdcp and hrv > --- > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 38 +++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index > 76c73daf546b..5c54073de615 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > @@ -836,6 +836,23 @@ pgc_mediamix: power-domain@10 { > <&clk IMX8MP_CLK_MEDIA_APB_ROOT>; > }; > > + pgc_hdmimix: power- domains@14 { As per Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml the node should be called power-domain@. > + #power-domain- cells = <0>; > + reg = <IMX8MP_POWER_DOMAIN_HDMIMIX>; > + clocks = <&clk IMX8MP_CLK_HDMI_ROOT>, > + <&clk IMX8MP_CLK_HDMI_APB>; > + assigned-clocks = <&clk IMX8MP_CLK_HDMI_AXI>, > + <&clk IMX8MP_CLK_HDMI_APB>; > + assigned-clock- parents = <&clk IMX8MP_SYS_PLL2_500M>, > + <&clk IMX8MP_SYS_PLL1_133M>; > + assigned-clock- rates = <500000000>, <133000000>; > + }; > + > + pgc_hdmi_phy: power- domains@15 { As per Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml the node should be called power-domain@. > + #power-domain- cells = <0>; > + reg = <IMX8MP_POWER_DOMAIN_HDMI_PHY>; > + }; > + > pgc_mipi_phy2: power- domain@16 { > #power-domain- cells = <0>; > reg = <IMX8MP_POWER_DOMAIN_MIPI_PHY2>; > @@ -1361,6 +1378,27 @@ eqos: ethernet@30bf0000 { > intf_mode = <&gpr 0x4>; > status = "disabled"; > }; > + > + hdmi_blk_ctrl: blk-ctrl@32fc0000 { > + compatible = "fsl,imx8mp-hdmi-blk- ctrl", "syscon"; > + reg = <0x32fc0000 0x23c>; > + clocks = <&clk IMX8MP_CLK_HDMI_APB>, > + <&clk IMX8MP_CLK_HDMI_ROOT>, > + <&clk IMX8MP_CLK_HDMI_REF_266M>, > + <&clk IMX8MP_CLK_HDMI_24M>, > + <&clk IMX8MP_CLK_HDMI_FDCC_TST>; > + clock-names = "apb", "axi", "ref_266m", "ref_24m", "fdcc"; > + power-domains = <&pgc_hdmimix>, <&pgc_hdmimix>, > + <&pgc_hdmimix>, <&pgc_hdmimix>, > + <&pgc_hdmimix>, <&pgc_hdmimix>, > + <&pgc_hdmimix>, <&pgc_hdmi_phy>, > + <&pgc_hdmimix>, <&pgc_hdmimix>; > + power-domain-names = "bus", "irqsteer", "lcdif", > + "pai", "pvi", "trng", > + "hdmi-tx", "hdmi-tx-phy", > + "hdcp", "hrv"; > + #power-domain-cells = <1>; > + }; > }; > According to RM this block is part of AIPS4, so it should be below hsio_blk_ctrl. Best regards, Alexander > aips5: bus@30c00000 {
Hi Adam, thanks for working on this. Am Samstag, 3. Februar 2024, 17:52:46 CET schrieb Adam Ford: > From: Lucas Stach <l.stach@pengutronix.de> > > The HDMI irqsteer is a secondary interrupt controller within the HDMI > subsystem that maps all HDMI peripheral IRQs into a single upstream > IRQ line. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index > 5c54073de615..5e51a766f3d9 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > @@ -1399,6 +1399,19 @@ hdmi_blk_ctrl: blk-ctrl@32fc0000 { > "hdcp", "hrv"; > #power-domain-cells = <1>; > }; > + > + irqsteer_hdmi: interrupt-controller@32fc2000 { > + compatible = "fsl,imx-irqsteer"; > + reg = <0x32fc2000 0x44>; > + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-controller; > + #interrupt-cells = <1>; > + fsl,channel = <1>; > + fsl,num-irqs = <64>; > + clocks = <&clk IMX8MP_CLK_HDMI_APB>; > + clock-names = "ipg"; > + power-domains = <&hdmi_blk_ctrl IMX8MP_HDMIBLK_PD_IRQSTEER>; > + }; According to RM this block is part of HDMI_TX which is part of AIPS4, so it should be below hsio_blk_ctrl. Best regards, Alexander > }; > > aips5: bus@30c00000 {
Hi Adam, thanks for working on this. Am Samstag, 3. Februar 2024, 17:52:51 CET schrieb Adam Ford: > From: Lucas Stach <l.stach@pengutronix.de> > > This adds the DT nodes for all the peripherals that make up the > HDMI display pipeline. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > Signed-off-by: Adam Ford <aford173@gmail.com> > > --- > V2: I took this from Lucas' original submission with the following: > Removed extra clock from HDMI-TX since it is now part of the > power domain > Added interrupt-parent to PVI > Changed the name of the HDMI tranmitter to fsl,imx8mp-hdmi-tx > Added ports to HDMI-tx > --- > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 94 +++++++++++++++++++++++ > 1 file changed, 94 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index > 5e51a766f3d9..e84b4f40e570 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > @@ -1412,6 +1412,100 @@ irqsteer_hdmi: interrupt-controller@32fc2000 { > clock-names = "ipg"; > power-domains = <&hdmi_blk_ctrl IMX8MP_HDMIBLK_PD_IRQSTEER>; > }; > + > + hdmi_pvi: display-bridge@32fc4000 { > + compatible = "fsl,imx8mp-hdmi-pvi"; > + reg = <0x32fc4000 0x40>; > + interrupt-parent = <&irqsteer_hdmi>; > + interrupts = <12 IRQ_TYPE_LEVEL_HIGH>; irqsteer_hdmi has #interrupt-cells = <1>, so IRQ flags should be removed. dtbs_check also warns about this. > + power-domains = <&hdmi_blk_ctrl IMX8MP_HDMIBLK_PD_PVI>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + pvi_from_lcdif3: endpoint { > + remote- endpoint = <&lcdif3_to_pvi>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + pvi_to_hdmi_tx: endpoint { > + remote- endpoint = <&hdmi_tx_from_pvi>; > + }; > + }; > + }; > + }; > + > + lcdif3: display-controller@32fc6000 { > + compatible = "fsl,imx8mp-lcdif"; > + reg = <0x32fc6000 0x238>; > + interrupts = <8 IRQ_TYPE_LEVEL_HIGH>; irqsteer_hdmi has #interrupt-cells = <1>, so IRQ flags should be removed. dtbs_check also warns about this. > + interrupt-parent = <&irqsteer_hdmi>; > + clocks = <&hdmi_tx_phy>, > + <&clk IMX8MP_CLK_HDMI_APB>, > + <&clk IMX8MP_CLK_HDMI_ROOT>; > + clock-names = "pix", "axi", "disp_axi"; > + power-domains = <&hdmi_blk_ctrl IMX8MP_HDMIBLK_PD_LCDIF>; > + > + port { > + lcdif3_to_pvi: endpoint { > + remote-endpoint = <&pvi_from_lcdif3>; > + }; > + }; > + }; > + > + hdmi_tx: hdmi@32fd8000 { > + compatible = "fsl,imx8mp-hdmi-tx"; > + reg = <0x32fd8000 0x7eff>; > + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; irqsteer_hdmi has #interrupt-cells = <1>, so IRQ flags should be removed. dtbs_check also warns about this. > + interrupt-parent = <&irqsteer_hdmi>; > + clocks = <&clk IMX8MP_CLK_HDMI_APB>, > + <&clk IMX8MP_CLK_HDMI_REF_266M>, > + <&clk IMX8MP_CLK_32K>, > + <&hdmi_tx_phy>; > + clock-names = "iahb", "isfr", "cec", "pix"; > + assigned-clocks = <&clk IMX8MP_CLK_HDMI_REF_266M>; > + assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_266M>; > + power-domains = <&hdmi_blk_ctrl IMX8MP_HDMIBLK_PD_HDMI_TX>; > + reg-io-width = <1>; > + status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + hdmi_tx_from_pvi: endpoint { > + remote- endpoint = <&pvi_to_hdmi_tx>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + /* Point endpoint to the HDMI connector */ > + }; > + }; > + }; > + > + hdmi_tx_phy: phy@32fdff00 { > + compatible = "fsl,imx8mp-hdmi-phy"; > + reg = <0x32fdff00 0x100>; > + clocks = <&clk IMX8MP_CLK_HDMI_APB>, > + <&clk IMX8MP_CLK_HDMI_24M>; > + clock-names = "apb", "ref"; > + assigned-clocks = <&clk IMX8MP_CLK_HDMI_24M>; > + assigned-clock-parents = <&clk IMX8MP_CLK_24M>; > + power-domains = <&hdmi_blk_ctrl IMX8MP_HDMIBLK_PD_HDMI_TX_PHY>; > + #clock-cells = <0>; > + #phy-cells = <0>; > + status = "disabled"; > + }; According to RM these blocks are part of AIPS4, so it should be below hsio_blk_ctrl. Best regards, Alexander > }; > > aips5: bus@30c00000 {
On 24-02-04, Dmitry Baryshkov wrote: > On Sat, 3 Feb 2024 at 17:53, Adam Ford <aford173@gmail.com> wrote: > > > > From: Lucas Stach <l.stach@pengutronix.de> > > > > This adds the driver for the Samsung HDMI PHY found on the > > i.MX8MP SoC. > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > Signed-off-by: Adam Ford <aford173@gmail.com> > > Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> > > --- > > V4: Make lookup table hex values lower case. > > > > V3: Re-order the Makefile to keep it alphabetical > > Remove unused defines > > > > V2: Fixed some whitespace found from checkpatch > > Change error handling when enabling apbclk to use dev_err_probe > > Rebase on Linux-Next > > > > I (Adam) tried to help move this along, so I took Lucas' patch and > > attempted to apply fixes based on feedback. I don't have > > all the history, so apologies for that. > > --- > > drivers/phy/freescale/Kconfig | 6 + > > drivers/phy/freescale/Makefile | 1 + > > drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 1075 ++++++++++++++++++ > > 3 files changed, 1082 insertions(+) > > create mode 100644 drivers/phy/freescale/phy-fsl-samsung-hdmi.c > > > > diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig > > index 853958fb2c06..5c2b73042dfc 100644 > > --- a/drivers/phy/freescale/Kconfig > > +++ b/drivers/phy/freescale/Kconfig > > @@ -35,6 +35,12 @@ config PHY_FSL_IMX8M_PCIE > > Enable this to add support for the PCIE PHY as found on > > i.MX8M family of SOCs. > > > > +config PHY_FSL_SAMSUNG_HDMI_PHY > > + tristate "Samsung HDMI PHY support" > > + depends on OF && HAS_IOMEM > > + help > > + Enable this to add support for the Samsung HDMI PHY in i.MX8MP. > > + > > endif > > > > config PHY_FSL_LYNX_28G > > diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile > > index cedb328bc4d2..79e5f16d3ce8 100644 > > --- a/drivers/phy/freescale/Makefile > > +++ b/drivers/phy/freescale/Makefile > > @@ -4,3 +4,4 @@ obj-$(CONFIG_PHY_MIXEL_LVDS_PHY) += phy-fsl-imx8qm-lvds-phy.o > > obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY) += phy-fsl-imx8-mipi-dphy.o > > obj-$(CONFIG_PHY_FSL_IMX8M_PCIE) += phy-fsl-imx8m-pcie.o > > obj-$(CONFIG_PHY_FSL_LYNX_28G) += phy-fsl-lynx-28g.o > > +obj-$(CONFIG_PHY_FSL_SAMSUNG_HDMI_PHY) += phy-fsl-samsung-hdmi.o > > diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c > > new file mode 100644 > > index 000000000000..bf0e2299d00f > > --- /dev/null > > +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c > > @@ -0,0 +1,1075 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright 2020 NXP > > + * Copyright 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de> > > + */ > > + > > +#include <linux/clk-provider.h> > > +#include <linux/clk.h> > > +#include <linux/delay.h> > > +#include <linux/io.h> > > +#include <linux/iopoll.h> > > +#include <linux/module.h> > > +#include <linux/of_device.h> > > +#include <linux/of.h> > > +#include <linux/platform_device.h> > > +#include <linux/pm_runtime.h> > > + > > +#define PHY_REG_33 0x84 > > +#define REG33_MODE_SET_DONE BIT(7) > > +#define REG33_FIX_DA BIT(1) > > + > > +#define PHY_REG_34 0x88 > > +#define REG34_PHY_READY BIT(7) > > +#define REG34_PLL_LOCK BIT(6) > > +#define REG34_PHY_CLK_READY BIT(5) > > + > > + > > +#define PHY_PLL_REGS_NUM 48 > > + > > +struct phy_config { > > + u32 clk_rate; > > + u8 regs[PHY_PLL_REGS_NUM]; > > +}; > > + > > +const struct phy_config phy_pll_cfg[] = { > > + { 22250000, { > > + 0x00, 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40, > > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x15, 0x25, 0x80, > > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32, > > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, > > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00, > > + }, > > + }, { > > + 23750000, { > > + 0x00, 0xd1, 0x50, 0xf1, 0x86, 0x85, 0x80, 0x40, > > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x03, 0x25, 0x80, > > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32, > > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, > > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00, > > + }, > > Generally I see that these entries contain a high level of duplication. > Could you please extract the common part and a rate-dependent part. > Next, it would be best if instead of writing the register values via > the rate LUT, the driver could calculate those values. > This allows us to support other HDMI modes if the need arises at some point. Hi Adam, can you please have a look at: https://lore.kernel.org/all/4830698.GXAFRqVoOG@steina-w/ there we have fixed this already. Not sure which version you picked for your work. Regards, Marco > > > + }, { > > + 24000000, { > > + 0x00, 0xd1, 0x50, 0xf0, 0x00, 0x00, 0x80, 0x00, > > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x01, 0x25, 0x80, > > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32, > > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, > > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00, > > + }, > > > -- > With best wishes > Dmitry > >
Hi, On Sat, 03 Feb 2024 10:52:40 -0600, Adam Ford wrote: > The i.MX8M Plus has an HDMI controller, but it depends on two > other systems, the Parallel Video Interface (PVI) and the > HDMI PHY from Samsung. The LCDIF controller generates the display > and routes it to the PVI which converts passes the parallel video > to the HDMI bridge. The HDMI system has a corresponding power > domain controller whose driver was partially written, but the > device tree for it was never applied, so some changes to the > power domain should be harmless because they've not really been > used yet. > > [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next) [07/12] dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI https://cgit.freedesktop.org/drm/drm-misc/commit/?id=f490d0cb9360466f6df0def3eccc47fabba9775b [08/12] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface https://cgit.freedesktop.org/drm/drm-misc/commit/?id=059c53e877ca6e723e10490c27c1487a63e66efe
On Mon, Feb 5, 2024 at 1:26 AM Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Hi Adam, > > thanks for working on this. > > Am Samstag, 3. Februar 2024, 17:52:45 CET schrieb Adam Ford: > > From: Lucas Stach <l.stach@pengutronix.de> > > > > This adds the PGC and HDMI blk-ctrl nodes providing power control for > > HDMI subsystem peripherals. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > --- > > V2: Add missing power-domains hdcp and hrv > > --- > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 38 +++++++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index > > 76c73daf546b..5c54073de615 100644 > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > @@ -836,6 +836,23 @@ pgc_mediamix: power-domain@10 { > > <&clk > IMX8MP_CLK_MEDIA_APB_ROOT>; > > }; > > > > + pgc_hdmimix: power- > domains@14 { > Alexander, Thanks for the feedback. > As per Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml the node > should be called power-domain@. > > > + #power-domain- > cells = <0>; > > + reg = > <IMX8MP_POWER_DOMAIN_HDMIMIX>; > > + clocks = <&clk > IMX8MP_CLK_HDMI_ROOT>, > > + <&clk > IMX8MP_CLK_HDMI_APB>; > > + assigned-clocks = > <&clk IMX8MP_CLK_HDMI_AXI>, > > + > <&clk IMX8MP_CLK_HDMI_APB>; > > + assigned-clock- > parents = <&clk IMX8MP_SYS_PLL2_500M>, > > + > <&clk IMX8MP_SYS_PLL1_133M>; > > + assigned-clock- > rates = <500000000>, <133000000>; > > + }; > > + > > + pgc_hdmi_phy: power- > domains@15 { > > As per Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml the node > should be called power-domain@. Whoops. I totally missed these when I applied them. I'll have them fixed on the next spin. > > > + #power-domain- > cells = <0>; > > + reg = > <IMX8MP_POWER_DOMAIN_HDMI_PHY>; > > + }; > > + > > pgc_mipi_phy2: power- > domain@16 { > > #power-domain- > cells = <0>; > > reg = > <IMX8MP_POWER_DOMAIN_MIPI_PHY2>; > > @@ -1361,6 +1378,27 @@ eqos: ethernet@30bf0000 { > > intf_mode = <&gpr 0x4>; > > status = "disabled"; > > }; > > + > > + hdmi_blk_ctrl: blk-ctrl@32fc0000 { > > + compatible = "fsl,imx8mp-hdmi-blk- > ctrl", "syscon"; > > + reg = <0x32fc0000 0x23c>; > > + clocks = <&clk IMX8MP_CLK_HDMI_APB>, > > + <&clk > IMX8MP_CLK_HDMI_ROOT>, > > + <&clk > IMX8MP_CLK_HDMI_REF_266M>, > > + <&clk IMX8MP_CLK_HDMI_24M>, > > + <&clk > IMX8MP_CLK_HDMI_FDCC_TST>; > > + clock-names = "apb", "axi", > "ref_266m", "ref_24m", "fdcc"; > > + power-domains = <&pgc_hdmimix>, > <&pgc_hdmimix>, > > + <&pgc_hdmimix>, > <&pgc_hdmimix>, > > + <&pgc_hdmimix>, > <&pgc_hdmimix>, > > + <&pgc_hdmimix>, > <&pgc_hdmi_phy>, > > + <&pgc_hdmimix>, > <&pgc_hdmimix>; > > + power-domain-names = "bus", > "irqsteer", "lcdif", > > + "pai", "pvi", > "trng", > > + "hdmi-tx", > "hdmi-tx-phy", > > + "hdcp", > "hrv"; > > + #power-domain-cells = <1>; > > + }; > > }; > > > > According to RM this block is part of AIPS4, so it should be below > hsio_blk_ctrl. This is how it was when I got it, but I should have caught it. Thanks for that. It looks like the subsequent HDMI, IRQ_steerting, LCDIF and PHY ones are also out of place. adam > > Best regards, > Alexander > > > aips5: bus@30c00000 { > > > -- > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > Amtsgericht München, HRB 105018 > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > http://www.tq-group.com/ > >
On Mon, Feb 5, 2024 at 2:17 AM Marco Felsch <m.felsch@pengutronix.de> wrote: > > On 24-02-04, Dmitry Baryshkov wrote: > > On Sat, 3 Feb 2024 at 17:53, Adam Ford <aford173@gmail.com> wrote: > > > > > > From: Lucas Stach <l.stach@pengutronix.de> > > > > > > This adds the driver for the Samsung HDMI PHY found on the > > > i.MX8MP SoC. > > > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> > > > --- > > > V4: Make lookup table hex values lower case. > > > > > > V3: Re-order the Makefile to keep it alphabetical > > > Remove unused defines > > > > > > V2: Fixed some whitespace found from checkpatch > > > Change error handling when enabling apbclk to use dev_err_probe > > > Rebase on Linux-Next > > > > > > I (Adam) tried to help move this along, so I took Lucas' patch and > > > attempted to apply fixes based on feedback. I don't have > > > all the history, so apologies for that. > > > --- > > > drivers/phy/freescale/Kconfig | 6 + > > > drivers/phy/freescale/Makefile | 1 + > > > drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 1075 ++++++++++++++++++ > > > 3 files changed, 1082 insertions(+) > > > create mode 100644 drivers/phy/freescale/phy-fsl-samsung-hdmi.c > > > > > > diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig > > > index 853958fb2c06..5c2b73042dfc 100644 > > > --- a/drivers/phy/freescale/Kconfig > > > +++ b/drivers/phy/freescale/Kconfig > > > @@ -35,6 +35,12 @@ config PHY_FSL_IMX8M_PCIE > > > Enable this to add support for the PCIE PHY as found on > > > i.MX8M family of SOCs. > > > > > > +config PHY_FSL_SAMSUNG_HDMI_PHY > > > + tristate "Samsung HDMI PHY support" > > > + depends on OF && HAS_IOMEM > > > + help > > > + Enable this to add support for the Samsung HDMI PHY in i.MX8MP. > > > + > > > endif > > > > > > config PHY_FSL_LYNX_28G > > > diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile > > > index cedb328bc4d2..79e5f16d3ce8 100644 > > > --- a/drivers/phy/freescale/Makefile > > > +++ b/drivers/phy/freescale/Makefile > > > @@ -4,3 +4,4 @@ obj-$(CONFIG_PHY_MIXEL_LVDS_PHY) += phy-fsl-imx8qm-lvds-phy.o > > > obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY) += phy-fsl-imx8-mipi-dphy.o > > > obj-$(CONFIG_PHY_FSL_IMX8M_PCIE) += phy-fsl-imx8m-pcie.o > > > obj-$(CONFIG_PHY_FSL_LYNX_28G) += phy-fsl-lynx-28g.o > > > +obj-$(CONFIG_PHY_FSL_SAMSUNG_HDMI_PHY) += phy-fsl-samsung-hdmi.o > > > diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c > > > new file mode 100644 > > > index 000000000000..bf0e2299d00f > > > --- /dev/null > > > +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c > > > @@ -0,0 +1,1075 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/* > > > + * Copyright 2020 NXP > > > + * Copyright 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de> > > > + */ > > > + > > > +#include <linux/clk-provider.h> > > > +#include <linux/clk.h> > > > +#include <linux/delay.h> > > > +#include <linux/io.h> > > > +#include <linux/iopoll.h> > > > +#include <linux/module.h> > > > +#include <linux/of_device.h> > > > +#include <linux/of.h> > > > +#include <linux/platform_device.h> > > > +#include <linux/pm_runtime.h> > > > + > > > +#define PHY_REG_33 0x84 > > > +#define REG33_MODE_SET_DONE BIT(7) > > > +#define REG33_FIX_DA BIT(1) > > > + > > > +#define PHY_REG_34 0x88 > > > +#define REG34_PHY_READY BIT(7) > > > +#define REG34_PLL_LOCK BIT(6) > > > +#define REG34_PHY_CLK_READY BIT(5) > > > + > > > + > > > +#define PHY_PLL_REGS_NUM 48 > > > + > > > +struct phy_config { > > > + u32 clk_rate; > > > + u8 regs[PHY_PLL_REGS_NUM]; > > > +}; > > > + > > > +const struct phy_config phy_pll_cfg[] = { > > > + { 22250000, { > > > + 0x00, 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40, > > > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x15, 0x25, 0x80, > > > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32, > > > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, > > > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > > > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00, > > > + }, > > > + }, { > > > + 23750000, { > > > + 0x00, 0xd1, 0x50, 0xf1, 0x86, 0x85, 0x80, 0x40, > > > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x03, 0x25, 0x80, > > > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32, > > > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, > > > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > > > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00, > > > + }, > > > > Generally I see that these entries contain a high level of duplication. > > Could you please extract the common part and a rate-dependent part. > > Next, it would be best if instead of writing the register values via > > the rate LUT, the driver could calculate those values. > > This allows us to support other HDMI modes if the need arises at some point. > > Hi Adam, > > can you please have a look at: https://lore.kernel.org/all/4830698.GXAFRqVoOG@steina-w/ > > there we have fixed this already. Not sure which version you picked for > your work. It must have been an earlier version. I got the list from Fabio, but I might have also gotten it mixed up. I'll look at this version and base my series on it and try to address comments others made. It'll likely take me a few days to catch up. thanks for the pointer. adam > > Regards, > Marco > > > > > > + }, { > > > + 24000000, { > > > + 0x00, 0xd1, 0x50, 0xf0, 0x00, 0x00, 0x80, 0x00, > > > + 0x4f, 0x30, 0x33, 0x65, 0x00, 0x01, 0x25, 0x80, > > > + 0x6c, 0xf2, 0x67, 0x00, 0x10, 0x8f, 0x30, 0x32, > > > + 0x60, 0x8f, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, > > > + 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > > > + 0x00, 0xe0, 0x83, 0x0f, 0x3e, 0xf8, 0x00, 0x00, > > > + }, > > > > > > -- > > With best wishes > > Dmitry > > > >
Hi, On Sat, 03 Feb 2024 10:52:40 -0600, Adam Ford wrote: > The i.MX8M Plus has an HDMI controller, but it depends on two > other systems, the Parallel Video Interface (PVI) and the > HDMI PHY from Samsung. The LCDIF controller generates the display > and routes it to the PVI which converts passes the parallel video > to the HDMI bridge. The HDMI system has a corresponding power > domain controller whose driver was partially written, but the > device tree for it was never applied, so some changes to the > power domain should be harmless because they've not really been > used yet. > > [...] Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next) [09/12] dt-bindings: display: imx: add binding for i.MX8MP HDMI TX https://cgit.freedesktop.org/drm/drm-misc/commit/?id=8933d29e7703f6f905bc84186b915b0ab4fe03bb [10/12] drm/bridge: imx: add bridge wrapper driver for i.MX8MP DWC HDMI https://cgit.freedesktop.org/drm/drm-misc/commit/?id=1f36d634670d8001a45fe2f2dcae546819f9c7d8
Hi all, On Sat, Feb 03, 2024 at 10:52:48AM -0600, Adam Ford wrote: > From: Lucas Stach <l.stach@pengutronix.de> > > This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a > full timing generator and can switch between different video sources. On > the i.MX8MP however the only supported source is the LCDIF. The block > just needs to be powered up and told about the polarity of the video > sync signals to act in bypass mode. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v7) > Tested-by: Marek Vasut <marex@denx.de> (v1) > Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v7) > Tested-by: Richard Leitner <richard.leitner@skidata.com> (v2) > Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> (v2) > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> (v3) > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > Tested-by: Fabio Estevam <festevam@gmail.com> > Signed-off-by: Adam Ford <aford173@gmail.com> <snip> > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > new file mode 100644 > index 000000000000..a76b7669fe8a > --- /dev/null > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c ... > +static void imx8mp_hdmi_pvi_bridge_enable(struct drm_bridge *bridge, > + struct drm_bridge_state *bridge_state) > +{ > + struct drm_atomic_state *state = bridge_state->base.state; > + struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge); > + struct drm_connector_state *conn_state; > + const struct drm_display_mode *mode; > + struct drm_crtc_state *crtc_state; > + struct drm_connector *connector; > + u32 bus_flags, val; > + > + connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder); > + conn_state = drm_atomic_get_new_connector_state(state, connector); > + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); > + > + if (WARN_ON(pm_runtime_resume_and_get(pvi->dev))) > + return; > + > + mode = &crtc_state->adjusted_mode; > + > + val = FIELD_PREP(PVI_CTRL_MODE_MASK, PVI_CTRL_MODE_LCDIF) | PVI_CTRL_EN; > + > + if (mode->flags & DRM_MODE_FLAG_PVSYNC) > + val |= PVI_CTRL_OP_VSYNC_POL | PVI_CTRL_INP_VSYNC_POL; > + > + if (mode->flags & DRM_MODE_FLAG_PHSYNC) > + val |= PVI_CTRL_OP_HSYNC_POL | PVI_CTRL_INP_HSYNC_POL; > + > + if (pvi->next_bridge->timings) > + bus_flags = pvi->next_bridge->timings->input_bus_flags; > + else if (bridge_state) > + bus_flags = bridge_state->input_bus_cfg.flags; > + > + if (bus_flags & DRM_BUS_FLAG_DE_HIGH) > + val |= PVI_CTRL_OP_DE_POL | PVI_CTRL_INP_DE_POL; > + > + writel(val, pvi->regs + HTX_PVI_CTRL); > +} Apologies if this has already been reported or fixed, I searched lore and did not find anything. Clang warns (or errors with CONFIG_WERROR=y) for this function: drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:81:11: error: variable 'bus_flags' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] 81 | else if (bridge_state) | ^~~~~~~~~~~~ drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:84:6: note: uninitialized use occurs here 84 | if (bus_flags & DRM_BUS_FLAG_DE_HIGH) | ^~~~~~~~~ drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:81:7: note: remove the 'if' if its condition is always true 81 | else if (bridge_state) | ^~~~~~~~~~~~~~~~~ 82 | bus_flags = bridge_state->input_bus_cfg.flags; drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:60:15: note: initialize the variable 'bus_flags' to silence this warning 60 | u32 bus_flags, val; | ^ | = 0 1 error generated. This seems legitimate. If bridge_state can be NULL, should bus_flags be initialized to zero like it suggests or should that 'else if' be turned into a plain 'else'? I am happy to send a patch with that guidance. Cheers, Nathan
On Sat, 3 Feb 2024 10:52:42 -0600 Adam Ford <aford173@gmail.com> wrote: > From: Lucas Stach <l.stach@pengutronix.de> > > This adds the driver for the Samsung HDMI PHY found on the > i.MX8MP SoC. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > Signed-off-by: Adam Ford <aford173@gmail.com> > Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> [...] > +#define PHY_REG_33 0x84 > +#define REG33_MODE_SET_DONE BIT(7) > +#define REG33_FIX_DA BIT(1) > + > +#define PHY_REG_34 0x88 > +#define REG34_PHY_READY BIT(7) > +#define REG34_PLL_LOCK BIT(6) > +#define REG34_PHY_CLK_READY BIT(5) > + > + Nitpick: only one empty line here. > +#define PHY_PLL_REGS_NUM 48 [...] > +static int phy_clk_register(struct fsl_samsung_hdmi_phy *phy) > +{ > + struct device *dev = phy->dev; > + struct device_node *np = dev->of_node; > + struct clk_init_data init; > + const char *parent_name; > + struct clk *phyclk; > + int ret; > + > + parent_name = __clk_get_name(phy->refclk); > + > + init.parent_names = &parent_name; > + init.num_parents = 1; > + init.flags = 0; > + init.name = "hdmi_pclk"; > + init.ops = &phy_clk_ops; > + > + phy->hw.init = &init; > + > + phyclk = devm_clk_register(dev, &phy->hw); > + if (IS_ERR(phyclk)) > + return dev_err_probe(dev, PTR_ERR(phyclk), > + "failed to register clock\n"); > + > + ret = of_clk_add_provider(np, of_clk_src_simple_get, phyclk); Ouch: > This function is *deprecated*. Use of_clk_add_hw_provider() instead. Appears as an easy replacement though. Otherwise looks good. Luca
On Sat, 3 Feb 2024 10:52:50 -0600 Adam Ford <aford173@gmail.com> wrote: > From: Lucas Stach <l.stach@pengutronix.de> > > Add a simple wrapper driver for the DWC HDMI bridge driver that > implements the few bits that are necessary to abstract the i.MX8MP > SoC integration. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Tested-by: Marek Vasut <marex@denx.de> > Tested-by: Adam Ford <aford173@gmail.com> #imx8mp-beacon > Tested-by: Richard Leitner <richard.leitner@skidata.com> > Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> > Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > Signed-off-by: Adam Ford <aford173@gmail.com> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
On Tue, Feb 6, 2024 at 11:06 AM Nathan Chancellor <nathan@kernel.org> wrote: > > Hi all, > > On Sat, Feb 03, 2024 at 10:52:48AM -0600, Adam Ford wrote: > > From: Lucas Stach <l.stach@pengutronix.de> > > > > This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a > > full timing generator and can switch between different video sources. On > > the i.MX8MP however the only supported source is the LCDIF. The block > > just needs to be powered up and told about the polarity of the video > > sync signals to act in bypass mode. > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v7) > > Tested-by: Marek Vasut <marex@denx.de> (v1) > > Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v7) > > Tested-by: Richard Leitner <richard.leitner@skidata.com> (v2) > > Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> (v2) > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> (v3) > > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > > Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > > Tested-by: Fabio Estevam <festevam@gmail.com> > > Signed-off-by: Adam Ford <aford173@gmail.com> > > <snip> > > > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > > new file mode 100644 > > index 000000000000..a76b7669fe8a > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > ... > > +static void imx8mp_hdmi_pvi_bridge_enable(struct drm_bridge *bridge, > > + struct drm_bridge_state *bridge_state) > > +{ > > + struct drm_atomic_state *state = bridge_state->base.state; > > + struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge); > > + struct drm_connector_state *conn_state; > > + const struct drm_display_mode *mode; > > + struct drm_crtc_state *crtc_state; > > + struct drm_connector *connector; > > + u32 bus_flags, val; > > + > > + connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder); > > + conn_state = drm_atomic_get_new_connector_state(state, connector); > > + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); > > + > > + if (WARN_ON(pm_runtime_resume_and_get(pvi->dev))) > > + return; > > + > > + mode = &crtc_state->adjusted_mode; > > + > > + val = FIELD_PREP(PVI_CTRL_MODE_MASK, PVI_CTRL_MODE_LCDIF) | PVI_CTRL_EN; > > + > > + if (mode->flags & DRM_MODE_FLAG_PVSYNC) > > + val |= PVI_CTRL_OP_VSYNC_POL | PVI_CTRL_INP_VSYNC_POL; > > + > > + if (mode->flags & DRM_MODE_FLAG_PHSYNC) > > + val |= PVI_CTRL_OP_HSYNC_POL | PVI_CTRL_INP_HSYNC_POL; > > + > > + if (pvi->next_bridge->timings) > > + bus_flags = pvi->next_bridge->timings->input_bus_flags; > > + else if (bridge_state) > > + bus_flags = bridge_state->input_bus_cfg.flags; > > + > > + if (bus_flags & DRM_BUS_FLAG_DE_HIGH) > > + val |= PVI_CTRL_OP_DE_POL | PVI_CTRL_INP_DE_POL; > > + > > + writel(val, pvi->regs + HTX_PVI_CTRL); > > +} > > Apologies if this has already been reported or fixed, I searched lore > and did not find anything. Clang warns (or errors with CONFIG_WERROR=y) > for this function: > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:81:11: error: variable 'bus_flags' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] > 81 | else if (bridge_state) > | ^~~~~~~~~~~~ > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:84:6: note: uninitialized use occurs here > 84 | if (bus_flags & DRM_BUS_FLAG_DE_HIGH) > | ^~~~~~~~~ > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:81:7: note: remove the 'if' if its condition is always true > 81 | else if (bridge_state) > | ^~~~~~~~~~~~~~~~~ > 82 | bus_flags = bridge_state->input_bus_cfg.flags; > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:60:15: note: initialize the variable 'bus_flags' to silence this warning > 60 | u32 bus_flags, val; > | ^ > | = 0 > 1 error generated. > > This seems legitimate. If bridge_state can be NULL, should bus_flags be > initialized to zero like it suggests or should that 'else if' be turned > into a plain 'else'? I am happy to send a patch with that guidance. I don't think we can turn the else-if into a blind else, because in order to make bus_flags point to bridge_state->input_bus_cfg.flags, bridge_state must not be NULL, but we could add an additional else to set bus_flags to 0, but I think the simplest thing to do would be to set bus_flags = 0 at the initialization on line 60 as it suggests. If you agree, I can submit a patch later tonight. I need to fix another issue found by the build-bot [1] to make line 113 return NULL instead of 0 anyway. I figured I could just fix them both at the same time. adam [1] - https://lore.kernel.org/oe-kbuild-all/202402062134.a6CqAt3s-lkp@intel.com/ > > Cheers, > Nathan
On Tue, Feb 06, 2024 at 12:50:16PM -0600, Adam Ford wrote: > On Tue, Feb 6, 2024 at 11:06 AM Nathan Chancellor <nathan@kernel.org> wrote: > > > > Hi all, > > > > On Sat, Feb 03, 2024 at 10:52:48AM -0600, Adam Ford wrote: > > > From: Lucas Stach <l.stach@pengutronix.de> > > > > > > This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a > > > full timing generator and can switch between different video sources. On > > > the i.MX8MP however the only supported source is the LCDIF. The block > > > just needs to be powered up and told about the polarity of the video > > > sync signals to act in bypass mode. > > > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v7) > > > Tested-by: Marek Vasut <marex@denx.de> (v1) > > > Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v7) > > > Tested-by: Richard Leitner <richard.leitner@skidata.com> (v2) > > > Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> (v2) > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> (v3) > > > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > > > Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > > > Tested-by: Fabio Estevam <festevam@gmail.com> > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > <snip> > > > > > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > > > new file mode 100644 > > > index 000000000000..a76b7669fe8a > > > --- /dev/null > > > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > > ... > > > +static void imx8mp_hdmi_pvi_bridge_enable(struct drm_bridge *bridge, > > > + struct drm_bridge_state *bridge_state) > > > +{ > > > + struct drm_atomic_state *state = bridge_state->base.state; > > > + struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge); > > > + struct drm_connector_state *conn_state; > > > + const struct drm_display_mode *mode; > > > + struct drm_crtc_state *crtc_state; > > > + struct drm_connector *connector; > > > + u32 bus_flags, val; > > > + > > > + connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder); > > > + conn_state = drm_atomic_get_new_connector_state(state, connector); > > > + crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc); > > > + > > > + if (WARN_ON(pm_runtime_resume_and_get(pvi->dev))) > > > + return; > > > + > > > + mode = &crtc_state->adjusted_mode; > > > + > > > + val = FIELD_PREP(PVI_CTRL_MODE_MASK, PVI_CTRL_MODE_LCDIF) | PVI_CTRL_EN; > > > + > > > + if (mode->flags & DRM_MODE_FLAG_PVSYNC) > > > + val |= PVI_CTRL_OP_VSYNC_POL | PVI_CTRL_INP_VSYNC_POL; > > > + > > > + if (mode->flags & DRM_MODE_FLAG_PHSYNC) > > > + val |= PVI_CTRL_OP_HSYNC_POL | PVI_CTRL_INP_HSYNC_POL; > > > + > > > + if (pvi->next_bridge->timings) > > > + bus_flags = pvi->next_bridge->timings->input_bus_flags; > > > + else if (bridge_state) > > > + bus_flags = bridge_state->input_bus_cfg.flags; > > > + > > > + if (bus_flags & DRM_BUS_FLAG_DE_HIGH) > > > + val |= PVI_CTRL_OP_DE_POL | PVI_CTRL_INP_DE_POL; > > > + > > > + writel(val, pvi->regs + HTX_PVI_CTRL); > > > +} > > > > Apologies if this has already been reported or fixed, I searched lore > > and did not find anything. Clang warns (or errors with CONFIG_WERROR=y) > > for this function: > > > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:81:11: error: variable 'bus_flags' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] > > 81 | else if (bridge_state) > > | ^~~~~~~~~~~~ > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:84:6: note: uninitialized use occurs here > > 84 | if (bus_flags & DRM_BUS_FLAG_DE_HIGH) > > | ^~~~~~~~~ > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:81:7: note: remove the 'if' if its condition is always true > > 81 | else if (bridge_state) > > | ^~~~~~~~~~~~~~~~~ > > 82 | bus_flags = bridge_state->input_bus_cfg.flags; > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c:60:15: note: initialize the variable 'bus_flags' to silence this warning > > 60 | u32 bus_flags, val; > > | ^ > > | = 0 > > 1 error generated. > > > > This seems legitimate. If bridge_state can be NULL, should bus_flags be > > initialized to zero like it suggests or should that 'else if' be turned > > into a plain 'else'? I am happy to send a patch with that guidance. > > I don't think we can turn the else-if into a blind else, because in > order to make bus_flags point to bridge_state->input_bus_cfg.flags, > bridge_state must not be NULL, but we could add an additional else to > set bus_flags to 0, but I think the simplest thing to do would be to > set bus_flags = 0 at the initialization on line 60 as it suggests. > > If you agree, I can submit a patch later tonight. I need to fix > another issue found by the build-bot [1] to make line 113 return NULL > instead of 0 anyway. I figured I could just fix them both at the same > time. > > [1] - https://lore.kernel.org/oe-kbuild-all/202402062134.a6CqAt3s-lkp@intel.com/ Seems reasonable to me, thanks! Nathan
>The i.MX8M Plus has an HDMI controller, but it depends on two >other systems, the Parallel Video Interface (PVI) and the >HDMI PHY from Samsung. The LCDIF controller generates the display >and routes it to the PVI which converts passes the parallel video >to the HDMI bridge. The HDMI system has a corresponding power >domain controller whose driver was partially written, but the >device tree for it was never applied, so some changes to the >power domain should be harmless because they've not really been >used yet. >This series is adapted from multiple series from Lucas Stach with >edits and suggestions from feedback from various series, but it >since it's difficult to use and test them independently, >I merged them into on unified series. The version history is a >bit ambiguous since different components were submitted at different times and had different amount of retries. In an effort to merge them >I used the highest version attempt. Tested-by: Joao Paulo Goncalves <joao.goncalves@toradex.com> Tested on Toradex Verdin-iMX8MP. Thanks! Regards, Joao Paulo Goncalves
Hi Adam, Lucas, Thanks for this series. This series make HDMI work on evk. All is working properly on my side. Tested on: Linux imx8mp-lpddr4-evk 6.9.0-rc1. Hope this help. Tested-by: Tommaso Merciai <tomm.merciai@gmail.com> Thanks & Regards, Tommaso On Sat, Feb 03, 2024 at 10:52:40AM -0600, Adam Ford wrote: > The i.MX8M Plus has an HDMI controller, but it depends on two > other systems, the Parallel Video Interface (PVI) and the > HDMI PHY from Samsung. The LCDIF controller generates the display > and routes it to the PVI which converts passes the parallel video > to the HDMI bridge. The HDMI system has a corresponding power > domain controller whose driver was partially written, but the > device tree for it was never applied, so some changes to the > power domain should be harmless because they've not really been > used yet. > > This series is adapted from multiple series from Lucas Stach with > edits and suggestions from feedback from various series, but it > since it's difficult to use and test them independently, > I merged them into on unified series. The version history is a > bit ambiguous since different components were submitted at different > times and had different amount of retries. In an effort to merge them > I used the highest version attempt. > > Adam Ford (3): > dt-bindings: soc: imx: add missing clock and power-domains to > imx8mp-hdmi-blk-ctrl > pmdomain: imx8mp-blk-ctrl: imx8mp_blk: Add fdcc clock to hdmimix > domain > arm64: defconfig: Enable DRM_IMX8MP_DW_HDMI_BRIDGE as module > > Lucas Stach (9): > dt-bindings: phy: add binding for the i.MX8MP HDMI PHY > phy: freescale: add Samsung HDMI PHY > arm64: dts: imx8mp: add HDMI power-domains > arm64: dts: imx8mp: add HDMI irqsteer > dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI > drm/bridge: imx: add driver for HDMI TX Parallel Video Interface > dt-bindings: display: imx: add binding for i.MX8MP HDMI TX > drm/bridge: imx: add bridge wrapper driver for i.MX8MP DWC HDMI > arm64: dts: imx8mp: add HDMI display pipeline > > .../display/bridge/fsl,imx8mp-hdmi-tx.yaml | 102 ++ > .../display/imx/fsl,imx8mp-hdmi-pvi.yaml | 84 ++ > .../bindings/phy/fsl,imx8mp-hdmi-phy.yaml | 62 + > .../soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml | 22 +- > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 145 +++ > arch/arm64/configs/defconfig | 1 + > drivers/gpu/drm/bridge/imx/Kconfig | 18 + > drivers/gpu/drm/bridge/imx/Makefile | 2 + > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 207 ++++ > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 154 +++ > drivers/phy/freescale/Kconfig | 6 + > drivers/phy/freescale/Makefile | 1 + > drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 1075 +++++++++++++++++ > drivers/pmdomain/imx/imx8mp-blk-ctrl.c | 10 +- > 14 files changed, 1876 insertions(+), 13 deletions(-) > create mode 100644 Documentation/devicetree/bindings/display/bridge/fsl,imx8mp-hdmi-tx.yaml > create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml > create mode 100644 Documentation/devicetree/bindings/phy/fsl,imx8mp-hdmi-phy.yaml > create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > create mode 100644 drivers/phy/freescale/phy-fsl-samsung-hdmi.c > > -- > 2.43.0 > >
Hi Tommaso, On Mon, Mar 25, 2024 at 10:48:56PM +0100, Tommaso Merciai wrote: > Hi Adam, Lucas, > Thanks for this series. > > This series make HDMI work on evk. > All is working properly on my side. > > Tested on: Linux imx8mp-lpddr4-evk 6.9.0-rc1. > Hope this help. > > Tested-by: Tommaso Merciai <tomm.merciai@gmail.com> The DRM side has been merged already. The only missing patches are for the PHY, and the latest version can be found in https://lore.kernel.org/linux-phy/20240227220444.77566-1-aford173@gmail.com/. You can test that series and send a Tested-by tag. I'm crossing my fingers and hoping it will be merged in v6.10. > On Sat, Feb 03, 2024 at 10:52:40AM -0600, Adam Ford wrote: > > The i.MX8M Plus has an HDMI controller, but it depends on two > > other systems, the Parallel Video Interface (PVI) and the > > HDMI PHY from Samsung. The LCDIF controller generates the display > > and routes it to the PVI which converts passes the parallel video > > to the HDMI bridge. The HDMI system has a corresponding power > > domain controller whose driver was partially written, but the > > device tree for it was never applied, so some changes to the > > power domain should be harmless because they've not really been > > used yet. > > > > This series is adapted from multiple series from Lucas Stach with > > edits and suggestions from feedback from various series, but it > > since it's difficult to use and test them independently, > > I merged them into on unified series. The version history is a > > bit ambiguous since different components were submitted at different > > times and had different amount of retries. In an effort to merge them > > I used the highest version attempt. > > > > Adam Ford (3): > > dt-bindings: soc: imx: add missing clock and power-domains to > > imx8mp-hdmi-blk-ctrl > > pmdomain: imx8mp-blk-ctrl: imx8mp_blk: Add fdcc clock to hdmimix > > domain > > arm64: defconfig: Enable DRM_IMX8MP_DW_HDMI_BRIDGE as module > > > > Lucas Stach (9): > > dt-bindings: phy: add binding for the i.MX8MP HDMI PHY > > phy: freescale: add Samsung HDMI PHY > > arm64: dts: imx8mp: add HDMI power-domains > > arm64: dts: imx8mp: add HDMI irqsteer > > dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI > > drm/bridge: imx: add driver for HDMI TX Parallel Video Interface > > dt-bindings: display: imx: add binding for i.MX8MP HDMI TX > > drm/bridge: imx: add bridge wrapper driver for i.MX8MP DWC HDMI > > arm64: dts: imx8mp: add HDMI display pipeline > > > > .../display/bridge/fsl,imx8mp-hdmi-tx.yaml | 102 ++ > > .../display/imx/fsl,imx8mp-hdmi-pvi.yaml | 84 ++ > > .../bindings/phy/fsl,imx8mp-hdmi-phy.yaml | 62 + > > .../soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml | 22 +- > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 145 +++ > > arch/arm64/configs/defconfig | 1 + > > drivers/gpu/drm/bridge/imx/Kconfig | 18 + > > drivers/gpu/drm/bridge/imx/Makefile | 2 + > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 207 ++++ > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 154 +++ > > drivers/phy/freescale/Kconfig | 6 + > > drivers/phy/freescale/Makefile | 1 + > > drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 1075 +++++++++++++++++ > > drivers/pmdomain/imx/imx8mp-blk-ctrl.c | 10 +- > > 14 files changed, 1876 insertions(+), 13 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/display/bridge/fsl,imx8mp-hdmi-tx.yaml > > create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml > > create mode 100644 Documentation/devicetree/bindings/phy/fsl,imx8mp-hdmi-phy.yaml > > create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > > create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > > create mode 100644 drivers/phy/freescale/phy-fsl-samsung-hdmi.c
Hi Laurent, On Tue, Mar 26, 2024 at 12:03:38AM +0200, Laurent Pinchart wrote: > Hi Tommaso, > > On Mon, Mar 25, 2024 at 10:48:56PM +0100, Tommaso Merciai wrote: > > Hi Adam, Lucas, > > Thanks for this series. > > > > This series make HDMI work on evk. > > All is working properly on my side. > > > > Tested on: Linux imx8mp-lpddr4-evk 6.9.0-rc1. > > Hope this help. > > > > Tested-by: Tommaso Merciai <tomm.merciai@gmail.com> > > The DRM side has been merged already. The only missing patches are for > the PHY, and the latest version can be found in > https://lore.kernel.org/linux-phy/20240227220444.77566-1-aford173@gmail.com/. > You can test that series and send a Tested-by tag. I'm crossing my > fingers and hoping it will be merged in v6.10. (same here :) ) Thanks for sharing! :) To be honest I test all this series rebasing my alvium next branch on top of media_stage/master (6.9.0-rc1) All is working properly on my side. I found v8 into the commit msg here: https://patches.linaro.org/project/linux-pm/patch/20240203165307.7806-9-aford173@gmail.com/ then I'm thinking this is the latest. I'm going to switch to your suggestion that looks more recent :) Thanks again, Tommaso > > > On Sat, Feb 03, 2024 at 10:52:40AM -0600, Adam Ford wrote: > > > The i.MX8M Plus has an HDMI controller, but it depends on two > > > other systems, the Parallel Video Interface (PVI) and the > > > HDMI PHY from Samsung. The LCDIF controller generates the display > > > and routes it to the PVI which converts passes the parallel video > > > to the HDMI bridge. The HDMI system has a corresponding power > > > domain controller whose driver was partially written, but the > > > device tree for it was never applied, so some changes to the > > > power domain should be harmless because they've not really been > > > used yet. > > > > > > This series is adapted from multiple series from Lucas Stach with > > > edits and suggestions from feedback from various series, but it > > > since it's difficult to use and test them independently, > > > I merged them into on unified series. The version history is a > > > bit ambiguous since different components were submitted at different > > > times and had different amount of retries. In an effort to merge them > > > I used the highest version attempt. > > > > > > Adam Ford (3): > > > dt-bindings: soc: imx: add missing clock and power-domains to > > > imx8mp-hdmi-blk-ctrl > > > pmdomain: imx8mp-blk-ctrl: imx8mp_blk: Add fdcc clock to hdmimix > > > domain > > > arm64: defconfig: Enable DRM_IMX8MP_DW_HDMI_BRIDGE as module > > > > > > Lucas Stach (9): > > > dt-bindings: phy: add binding for the i.MX8MP HDMI PHY > > > phy: freescale: add Samsung HDMI PHY > > > arm64: dts: imx8mp: add HDMI power-domains > > > arm64: dts: imx8mp: add HDMI irqsteer > > > dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI > > > drm/bridge: imx: add driver for HDMI TX Parallel Video Interface > > > dt-bindings: display: imx: add binding for i.MX8MP HDMI TX > > > drm/bridge: imx: add bridge wrapper driver for i.MX8MP DWC HDMI > > > arm64: dts: imx8mp: add HDMI display pipeline > > > > > > .../display/bridge/fsl,imx8mp-hdmi-tx.yaml | 102 ++ > > > .../display/imx/fsl,imx8mp-hdmi-pvi.yaml | 84 ++ > > > .../bindings/phy/fsl,imx8mp-hdmi-phy.yaml | 62 + > > > .../soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml | 22 +- > > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 145 +++ > > > arch/arm64/configs/defconfig | 1 + > > > drivers/gpu/drm/bridge/imx/Kconfig | 18 + > > > drivers/gpu/drm/bridge/imx/Makefile | 2 + > > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 207 ++++ > > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 154 +++ > > > drivers/phy/freescale/Kconfig | 6 + > > > drivers/phy/freescale/Makefile | 1 + > > > drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 1075 +++++++++++++++++ > > > drivers/pmdomain/imx/imx8mp-blk-ctrl.c | 10 +- > > > 14 files changed, 1876 insertions(+), 13 deletions(-) > > > create mode 100644 Documentation/devicetree/bindings/display/bridge/fsl,imx8mp-hdmi-tx.yaml > > > create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml > > > create mode 100644 Documentation/devicetree/bindings/phy/fsl,imx8mp-hdmi-phy.yaml > > > create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > > > create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > > > create mode 100644 drivers/phy/freescale/phy-fsl-samsung-hdmi.c > > -- > Regards, > > Laurent Pinchart
On Tue, Mar 26, 2024 at 2:46 AM Tommaso Merciai <tomm.merciai@gmail.com> wrote: > > Hi Laurent, > > On Tue, Mar 26, 2024 at 12:03:38AM +0200, Laurent Pinchart wrote: > > Hi Tommaso, > > > > On Mon, Mar 25, 2024 at 10:48:56PM +0100, Tommaso Merciai wrote: > > > Hi Adam, Lucas, > > > Thanks for this series. > > > > > > This series make HDMI work on evk. > > > All is working properly on my side. > > > > > > Tested on: Linux imx8mp-lpddr4-evk 6.9.0-rc1. > > > Hope this help. > > > > > > Tested-by: Tommaso Merciai <tomm.merciai@gmail.com> > > > > The DRM side has been merged already. The only missing patches are for > > the PHY, and the latest version can be found in > > https://lore.kernel.org/linux-phy/20240227220444.77566-1-aford173@gmail.com/. > > You can test that series and send a Tested-by tag. I'm crossing my > > fingers and hoping it will be merged in v6.10. > (same here :) ) > > Thanks for sharing! :) > > To be honest I test all this series rebasing my alvium next branch on top of media_stage/master (6.9.0-rc1) > All is working properly on my side. > I found v8 into the commit msg here: https://patches.linaro.org/project/linux-pm/patch/20240203165307.7806-9-aford173@gmail.com/ > then I'm thinking this is the latest. > > I'm going to switch to your suggestion that looks more recent :) Sorry about the confusion. I was confused by the versioning too when I pulled from different parts of Lucas' stuff. Since varying components were applied at different times, and the remaining part was based on the wrong starting point and not applied, I reverted back to the versioning of the PHY which was the only remaining part other than device tree stuff. adam > > Thanks again, > Tommaso > > > > > > On Sat, Feb 03, 2024 at 10:52:40AM -0600, Adam Ford wrote: > > > > The i.MX8M Plus has an HDMI controller, but it depends on two > > > > other systems, the Parallel Video Interface (PVI) and the > > > > HDMI PHY from Samsung. The LCDIF controller generates the display > > > > and routes it to the PVI which converts passes the parallel video > > > > to the HDMI bridge. The HDMI system has a corresponding power > > > > domain controller whose driver was partially written, but the > > > > device tree for it was never applied, so some changes to the > > > > power domain should be harmless because they've not really been > > > > used yet. > > > > > > > > This series is adapted from multiple series from Lucas Stach with > > > > edits and suggestions from feedback from various series, but it > > > > since it's difficult to use and test them independently, > > > > I merged them into on unified series. The version history is a > > > > bit ambiguous since different components were submitted at different > > > > times and had different amount of retries. In an effort to merge them > > > > I used the highest version attempt. > > > > > > > > Adam Ford (3): > > > > dt-bindings: soc: imx: add missing clock and power-domains to > > > > imx8mp-hdmi-blk-ctrl > > > > pmdomain: imx8mp-blk-ctrl: imx8mp_blk: Add fdcc clock to hdmimix > > > > domain > > > > arm64: defconfig: Enable DRM_IMX8MP_DW_HDMI_BRIDGE as module > > > > > > > > Lucas Stach (9): > > > > dt-bindings: phy: add binding for the i.MX8MP HDMI PHY > > > > phy: freescale: add Samsung HDMI PHY > > > > arm64: dts: imx8mp: add HDMI power-domains > > > > arm64: dts: imx8mp: add HDMI irqsteer > > > > dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI > > > > drm/bridge: imx: add driver for HDMI TX Parallel Video Interface > > > > dt-bindings: display: imx: add binding for i.MX8MP HDMI TX > > > > drm/bridge: imx: add bridge wrapper driver for i.MX8MP DWC HDMI > > > > arm64: dts: imx8mp: add HDMI display pipeline > > > > > > > > .../display/bridge/fsl,imx8mp-hdmi-tx.yaml | 102 ++ > > > > .../display/imx/fsl,imx8mp-hdmi-pvi.yaml | 84 ++ > > > > .../bindings/phy/fsl,imx8mp-hdmi-phy.yaml | 62 + > > > > .../soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml | 22 +- > > > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 145 +++ > > > > arch/arm64/configs/defconfig | 1 + > > > > drivers/gpu/drm/bridge/imx/Kconfig | 18 + > > > > drivers/gpu/drm/bridge/imx/Makefile | 2 + > > > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 207 ++++ > > > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 154 +++ > > > > drivers/phy/freescale/Kconfig | 6 + > > > > drivers/phy/freescale/Makefile | 1 + > > > > drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 1075 +++++++++++++++++ > > > > drivers/pmdomain/imx/imx8mp-blk-ctrl.c | 10 +- > > > > 14 files changed, 1876 insertions(+), 13 deletions(-) > > > > create mode 100644 Documentation/devicetree/bindings/display/bridge/fsl,imx8mp-hdmi-tx.yaml > > > > create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml > > > > create mode 100644 Documentation/devicetree/bindings/phy/fsl,imx8mp-hdmi-phy.yaml > > > > create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > > > > create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > > > > create mode 100644 drivers/phy/freescale/phy-fsl-samsung-hdmi.c > > > > -- > > Regards, > > > > Laurent Pinchart
Hi Adam, On Tue, Mar 26, 2024 at 06:43:26AM -0500, Adam Ford wrote: > On Tue, Mar 26, 2024 at 2:46 AM Tommaso Merciai <tomm.merciai@gmail.com> wrote: > > > > Hi Laurent, > > > > On Tue, Mar 26, 2024 at 12:03:38AM +0200, Laurent Pinchart wrote: > > > Hi Tommaso, > > > > > > On Mon, Mar 25, 2024 at 10:48:56PM +0100, Tommaso Merciai wrote: > > > > Hi Adam, Lucas, > > > > Thanks for this series. > > > > > > > > This series make HDMI work on evk. > > > > All is working properly on my side. > > > > > > > > Tested on: Linux imx8mp-lpddr4-evk 6.9.0-rc1. > > > > Hope this help. > > > > > > > > Tested-by: Tommaso Merciai <tomm.merciai@gmail.com> > > > > > > The DRM side has been merged already. The only missing patches are for > > > the PHY, and the latest version can be found in > > > https://lore.kernel.org/linux-phy/20240227220444.77566-1-aford173@gmail.com/. > > > You can test that series and send a Tested-by tag. I'm crossing my > > > fingers and hoping it will be merged in v6.10. > > (same here :) ) > > > > Thanks for sharing! :) > > > > To be honest I test all this series rebasing my alvium next branch on top of media_stage/master (6.9.0-rc1) > > All is working properly on my side. > > I found v8 into the commit msg here: https://patches.linaro.org/project/linux-pm/patch/20240203165307.7806-9-aford173@gmail.com/ > > then I'm thinking this is the latest. > > > > I'm going to switch to your suggestion that looks more recent :) > > Sorry about the confusion. I was confused by the versioning too when > I pulled from different parts of Lucas' stuff. Since varying > components were applied at different times, and the remaining part was > based on the wrong starting point and not applied, I reverted back to > the versioning of the PHY which was the only remaining part other than > device tree stuff. No problem, thanks for clarify :) Thanks & Regards, Tommaso > > adam > > > > Thanks again, > > Tommaso > > > > > > > > > On Sat, Feb 03, 2024 at 10:52:40AM -0600, Adam Ford wrote: > > > > > The i.MX8M Plus has an HDMI controller, but it depends on two > > > > > other systems, the Parallel Video Interface (PVI) and the > > > > > HDMI PHY from Samsung. The LCDIF controller generates the display > > > > > and routes it to the PVI which converts passes the parallel video > > > > > to the HDMI bridge. The HDMI system has a corresponding power > > > > > domain controller whose driver was partially written, but the > > > > > device tree for it was never applied, so some changes to the > > > > > power domain should be harmless because they've not really been > > > > > used yet. > > > > > > > > > > This series is adapted from multiple series from Lucas Stach with > > > > > edits and suggestions from feedback from various series, but it > > > > > since it's difficult to use and test them independently, > > > > > I merged them into on unified series. The version history is a > > > > > bit ambiguous since different components were submitted at different > > > > > times and had different amount of retries. In an effort to merge them > > > > > I used the highest version attempt. > > > > > > > > > > Adam Ford (3): > > > > > dt-bindings: soc: imx: add missing clock and power-domains to > > > > > imx8mp-hdmi-blk-ctrl > > > > > pmdomain: imx8mp-blk-ctrl: imx8mp_blk: Add fdcc clock to hdmimix > > > > > domain > > > > > arm64: defconfig: Enable DRM_IMX8MP_DW_HDMI_BRIDGE as module > > > > > > > > > > Lucas Stach (9): > > > > > dt-bindings: phy: add binding for the i.MX8MP HDMI PHY > > > > > phy: freescale: add Samsung HDMI PHY > > > > > arm64: dts: imx8mp: add HDMI power-domains > > > > > arm64: dts: imx8mp: add HDMI irqsteer > > > > > dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI > > > > > drm/bridge: imx: add driver for HDMI TX Parallel Video Interface > > > > > dt-bindings: display: imx: add binding for i.MX8MP HDMI TX > > > > > drm/bridge: imx: add bridge wrapper driver for i.MX8MP DWC HDMI > > > > > arm64: dts: imx8mp: add HDMI display pipeline > > > > > > > > > > .../display/bridge/fsl,imx8mp-hdmi-tx.yaml | 102 ++ > > > > > .../display/imx/fsl,imx8mp-hdmi-pvi.yaml | 84 ++ > > > > > .../bindings/phy/fsl,imx8mp-hdmi-phy.yaml | 62 + > > > > > .../soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml | 22 +- > > > > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 145 +++ > > > > > arch/arm64/configs/defconfig | 1 + > > > > > drivers/gpu/drm/bridge/imx/Kconfig | 18 + > > > > > drivers/gpu/drm/bridge/imx/Makefile | 2 + > > > > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 207 ++++ > > > > > drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 154 +++ > > > > > drivers/phy/freescale/Kconfig | 6 + > > > > > drivers/phy/freescale/Makefile | 1 + > > > > > drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 1075 +++++++++++++++++ > > > > > drivers/pmdomain/imx/imx8mp-blk-ctrl.c | 10 +- > > > > > 14 files changed, 1876 insertions(+), 13 deletions(-) > > > > > create mode 100644 Documentation/devicetree/bindings/display/bridge/fsl,imx8mp-hdmi-tx.yaml > > > > > create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml > > > > > create mode 100644 Documentation/devicetree/bindings/phy/fsl,imx8mp-hdmi-phy.yaml > > > > > create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c > > > > > create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c > > > > > create mode 100644 drivers/phy/freescale/phy-fsl-samsung-hdmi.c > > > > > > -- > > > Regards, > > > > > > Laurent Pinchart
The i.MX8M Plus has an HDMI controller, but it depends on two other systems, the Parallel Video Interface (PVI) and the HDMI PHY from Samsung. The LCDIF controller generates the display and routes it to the PVI which converts passes the parallel video to the HDMI bridge. The HDMI system has a corresponding power domain controller whose driver was partially written, but the device tree for it was never applied, so some changes to the power domain should be harmless because they've not really been used yet. This series is adapted from multiple series from Lucas Stach with edits and suggestions from feedback from various series, but it since it's difficult to use and test them independently, I merged them into on unified series. The version history is a bit ambiguous since different components were submitted at different times and had different amount of retries. In an effort to merge them I used the highest version attempt. Adam Ford (3): dt-bindings: soc: imx: add missing clock and power-domains to imx8mp-hdmi-blk-ctrl pmdomain: imx8mp-blk-ctrl: imx8mp_blk: Add fdcc clock to hdmimix domain arm64: defconfig: Enable DRM_IMX8MP_DW_HDMI_BRIDGE as module Lucas Stach (9): dt-bindings: phy: add binding for the i.MX8MP HDMI PHY phy: freescale: add Samsung HDMI PHY arm64: dts: imx8mp: add HDMI power-domains arm64: dts: imx8mp: add HDMI irqsteer dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI drm/bridge: imx: add driver for HDMI TX Parallel Video Interface dt-bindings: display: imx: add binding for i.MX8MP HDMI TX drm/bridge: imx: add bridge wrapper driver for i.MX8MP DWC HDMI arm64: dts: imx8mp: add HDMI display pipeline .../display/bridge/fsl,imx8mp-hdmi-tx.yaml | 102 ++ .../display/imx/fsl,imx8mp-hdmi-pvi.yaml | 84 ++ .../bindings/phy/fsl,imx8mp-hdmi-phy.yaml | 62 + .../soc/imx/fsl,imx8mp-hdmi-blk-ctrl.yaml | 22 +- arch/arm64/boot/dts/freescale/imx8mp.dtsi | 145 +++ arch/arm64/configs/defconfig | 1 + drivers/gpu/drm/bridge/imx/Kconfig | 18 + drivers/gpu/drm/bridge/imx/Makefile | 2 + drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 207 ++++ drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 154 +++ drivers/phy/freescale/Kconfig | 6 + drivers/phy/freescale/Makefile | 1 + drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 1075 +++++++++++++++++ drivers/pmdomain/imx/imx8mp-blk-ctrl.c | 10 +- 14 files changed, 1876 insertions(+), 13 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/fsl,imx8mp-hdmi-tx.yaml create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml create mode 100644 Documentation/devicetree/bindings/phy/fsl,imx8mp-hdmi-phy.yaml create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c create mode 100644 drivers/phy/freescale/phy-fsl-samsung-hdmi.c