.../platform/synopsys/hdmirx/snps_hdmirx.c | 90 ++++++++++++++++++- .../platform/synopsys/hdmirx/snps_hdmirx.h | 2 + 2 files changed, 90 insertions(+), 2 deletions(-)
Downstream version of RK3588 U-Boot uses customized TF-A that remaps
HDMIRX hardware interrupt, routing it via firmware that isn't supported
by upstream driver.
Detect broken interrupt and print a clarifying error message about a need
to use open-source TF-A with this driver.
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
Changelog:
v2: - Added PHY r/w lock and moved the clarifying error message as
was suggested by Sebastian Reichel.
.../platform/synopsys/hdmirx/snps_hdmirx.c | 90 ++++++++++++++++++-
.../platform/synopsys/hdmirx/snps_hdmirx.h | 2 +
2 files changed, 90 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index b7d278b3889f..e6456352dfa5 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -132,12 +132,14 @@ struct snps_hdmirx_dev {
struct delayed_work delayed_work_hotplug;
struct delayed_work delayed_work_res_change;
struct hdmirx_cec *cec;
+ struct mutex phy_rw_lock; /* to protect phy r/w configuration */
struct mutex stream_lock; /* to lock video stream capture */
struct mutex work_lock; /* to lock the critical section of hotplug event */
struct reset_control_bulk_data resets[HDMIRX_NUM_RST];
struct clk_bulk_data *clks;
struct regmap *grf;
struct regmap *vo1_grf;
+ struct completion cr_read_done;
struct completion cr_write_done;
struct completion timer_base_lock;
struct completion avi_pkt_rcv;
@@ -796,11 +798,50 @@ static int wait_reg_bit_status(struct snps_hdmirx_dev *hdmirx_dev, u32 reg,
return 0;
}
+static int hdmirx_phy_register_read(struct snps_hdmirx_dev *hdmirx_dev,
+ u32 phy_reg, u32 *val)
+{
+ struct device *dev = hdmirx_dev->dev;
+ u32 status;
+
+ guard(mutex)(&hdmirx_dev->phy_rw_lock);
+
+ reinit_completion(&hdmirx_dev->cr_read_done);
+ /* clear irq status */
+ hdmirx_clear_interrupt(hdmirx_dev, MAINUNIT_2_INT_CLEAR, 0xffffffff);
+ /* en irq */
+ hdmirx_update_bits(hdmirx_dev, MAINUNIT_2_INT_MASK_N,
+ PHYCREG_CR_READ_DONE, PHYCREG_CR_READ_DONE);
+ /* write phy reg addr */
+ hdmirx_writel(hdmirx_dev, PHYCREG_CONFIG1, phy_reg);
+ /* config read enable */
+ hdmirx_writel(hdmirx_dev, PHYCREG_CONTROL, PHYCREG_CR_PARA_READ_P);
+
+ if (!wait_for_completion_timeout(&hdmirx_dev->cr_read_done,
+ msecs_to_jiffies(20))) {
+ dev_err(dev, "%s wait cr read done failed\n", __func__);
+ return -ETIMEDOUT;
+ }
+
+ /* read phy reg value */
+ status = hdmirx_readl(hdmirx_dev, PHYCREG_STATUS);
+ if (!(status & PHYCREG_CR_PARA_DATAVALID)) {
+ dev_err(dev, "%s cr read failed\n", __func__);
+ return -EINVAL;
+ }
+
+ *val = status & PHYCREG_CR_PARA_RD_DATA_MASK;
+
+ return 0;
+}
+
static int hdmirx_phy_register_write(struct snps_hdmirx_dev *hdmirx_dev,
u32 phy_reg, u32 val)
{
struct device *dev = hdmirx_dev->dev;
+ guard(mutex)(&hdmirx_dev->phy_rw_lock);
+
reinit_completion(&hdmirx_dev->cr_write_done);
/* clear irq status */
hdmirx_clear_interrupt(hdmirx_dev, MAINUNIT_2_INT_CLEAR, 0xffffffff);
@@ -1814,6 +1855,13 @@ static void mainunit_2_int_handler(struct snps_hdmirx_dev *hdmirx_dev,
*handled = true;
}
+ if (status & PHYCREG_CR_READ_DONE) {
+ hdmirx_update_bits(hdmirx_dev, MAINUNIT_2_INT_MASK_N,
+ PHYCREG_CR_READ_DONE, 0);
+ complete(&hdmirx_dev->cr_read_done);
+ *handled = true;
+ }
+
if (status & TMDSVALID_STABLE_CHG) {
process_signal_change(hdmirx_dev);
v4l2_dbg(2, debug, v4l2_dev, "%s: TMDSVALID_STABLE_CHG\n", __func__);
@@ -2313,18 +2361,51 @@ static void hdmirx_disable_all_interrupts(struct snps_hdmirx_dev *hdmirx_dev)
hdmirx_clear_interrupt(hdmirx_dev, CEC_INT_CLEAR, 0xffffffff);
}
-static void hdmirx_init(struct snps_hdmirx_dev *hdmirx_dev)
+static int hdmirx_detect_broken_interrupt(struct snps_hdmirx_dev *hdmirx_dev)
{
+ int ret;
+ u32 val;
+
+ enable_irq(hdmirx_dev->hdmi_irq);
+
+ hdmirx_writel(hdmirx_dev, PHYCREG_CONFIG0, 0x3);
+
+ ret = hdmirx_phy_register_read(hdmirx_dev,
+ HDMIPCS_DIG_CTRL_PATH_MAIN_FSM_FSM_CONFIG,
+ &val);
+
+ disable_irq(hdmirx_dev->hdmi_irq);
+
+ return ret;
+}
+
+static int hdmirx_init(struct snps_hdmirx_dev *hdmirx_dev)
+{
+ int ret;
+
hdmirx_update_bits(hdmirx_dev, PHY_CONFIG, PHY_RESET | PHY_PDDQ, 0);
regmap_write(hdmirx_dev->vo1_grf, VO1_GRF_VO1_CON2,
(HDMIRX_SDAIN_MSK | HDMIRX_SCLIN_MSK) |
((HDMIRX_SDAIN_MSK | HDMIRX_SCLIN_MSK) << 16));
+
+ /*
+ * RK3588 downstream version of TF-A remaps HDMIRX interrupt and
+ * requires use of a vendor-specific FW API that we don't support
+ * in this driver.
+ */
+ ret = hdmirx_detect_broken_interrupt(hdmirx_dev);
+ if (ret)
+ dev_err_probe(hdmirx_dev->dev, ret,
+ "interrupt not functioning, open-source TF-A is required by this driver\n");
+
/*
* Some interrupts are enabled by default, so we disable
* all interrupts and clear interrupts status first.
*/
hdmirx_disable_all_interrupts(hdmirx_dev);
+
+ return ret;
}
/* hdmi-4k-300mhz EDID produced by v4l2-ctl tool */
@@ -2606,10 +2687,12 @@ static int hdmirx_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(hdmirx_dev->regs),
"failed to remap regs resource\n");
+ mutex_init(&hdmirx_dev->phy_rw_lock);
mutex_init(&hdmirx_dev->stream_lock);
mutex_init(&hdmirx_dev->work_lock);
spin_lock_init(&hdmirx_dev->rst_lock);
+ init_completion(&hdmirx_dev->cr_read_done);
init_completion(&hdmirx_dev->cr_write_done);
init_completion(&hdmirx_dev->timer_base_lock);
init_completion(&hdmirx_dev->avi_pkt_rcv);
@@ -2623,7 +2706,10 @@ static int hdmirx_probe(struct platform_device *pdev)
hdmirx_dev->timings = cea640x480;
hdmirx_enable(dev);
- hdmirx_init(hdmirx_dev);
+
+ ret = hdmirx_init(hdmirx_dev);
+ if (ret)
+ goto err_pm;
v4l2_dev = &hdmirx_dev->v4l2_dev;
strscpy(v4l2_dev->name, dev_name(dev), sizeof(v4l2_dev->name));
diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
index 220ab99ca611..a719439d3ca0 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
@@ -114,6 +114,8 @@
#define PHYCREG_CR_PARA_WRITE_P BIT(1)
#define PHYCREG_CR_PARA_READ_P BIT(0)
#define PHYCREG_STATUS 0x00f4
+#define PHYCREG_CR_PARA_DATAVALID BIT(24)
+#define PHYCREG_CR_PARA_RD_DATA_MASK GENMASK(15, 0)
#define MAINUNIT_STATUS 0x0150
#define TMDSVALID_STABLE_ST BIT(1)
--
2.51.0Hello Dmitry,
On Thu, Oct 02, 2025 at 05:07:50PM +0300, Dmitry Osipenko wrote:
> Downstream version of RK3588 U-Boot uses customized TF-A that remaps
> HDMIRX hardware interrupt, routing it via firmware that isn't supported
> by upstream driver.
>
> Detect broken interrupt and print a clarifying error message about a need
> to use open-source TF-A with this driver.
>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
> Changelog:
>
> v2: - Added PHY r/w lock and moved the clarifying error message as
> was suggested by Sebastian Reichel.
>
> .../platform/synopsys/hdmirx/snps_hdmirx.c | 90 ++++++++++++++++++-
> .../platform/synopsys/hdmirx/snps_hdmirx.h | 2 +
> 2 files changed, 90 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> index b7d278b3889f..e6456352dfa5 100644
> --- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> +++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
#include <linux/cleanup.h>
Otherwise LGTM and hopefully helps people to figure out the root
cause of their problems:
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Greetings,
-- Sebastian
> @@ -132,12 +132,14 @@ struct snps_hdmirx_dev {
> struct delayed_work delayed_work_hotplug;
> struct delayed_work delayed_work_res_change;
> struct hdmirx_cec *cec;
> + struct mutex phy_rw_lock; /* to protect phy r/w configuration */
> struct mutex stream_lock; /* to lock video stream capture */
> struct mutex work_lock; /* to lock the critical section of hotplug event */
> struct reset_control_bulk_data resets[HDMIRX_NUM_RST];
> struct clk_bulk_data *clks;
> struct regmap *grf;
> struct regmap *vo1_grf;
> + struct completion cr_read_done;
> struct completion cr_write_done;
> struct completion timer_base_lock;
> struct completion avi_pkt_rcv;
> @@ -796,11 +798,50 @@ static int wait_reg_bit_status(struct snps_hdmirx_dev *hdmirx_dev, u32 reg,
> return 0;
> }
>
> +static int hdmirx_phy_register_read(struct snps_hdmirx_dev *hdmirx_dev,
> + u32 phy_reg, u32 *val)
> +{
> + struct device *dev = hdmirx_dev->dev;
> + u32 status;
> +
> + guard(mutex)(&hdmirx_dev->phy_rw_lock);
> +
> + reinit_completion(&hdmirx_dev->cr_read_done);
> + /* clear irq status */
> + hdmirx_clear_interrupt(hdmirx_dev, MAINUNIT_2_INT_CLEAR, 0xffffffff);
> + /* en irq */
> + hdmirx_update_bits(hdmirx_dev, MAINUNIT_2_INT_MASK_N,
> + PHYCREG_CR_READ_DONE, PHYCREG_CR_READ_DONE);
> + /* write phy reg addr */
> + hdmirx_writel(hdmirx_dev, PHYCREG_CONFIG1, phy_reg);
> + /* config read enable */
> + hdmirx_writel(hdmirx_dev, PHYCREG_CONTROL, PHYCREG_CR_PARA_READ_P);
> +
> + if (!wait_for_completion_timeout(&hdmirx_dev->cr_read_done,
> + msecs_to_jiffies(20))) {
> + dev_err(dev, "%s wait cr read done failed\n", __func__);
> + return -ETIMEDOUT;
> + }
> +
> + /* read phy reg value */
> + status = hdmirx_readl(hdmirx_dev, PHYCREG_STATUS);
> + if (!(status & PHYCREG_CR_PARA_DATAVALID)) {
> + dev_err(dev, "%s cr read failed\n", __func__);
> + return -EINVAL;
> + }
> +
> + *val = status & PHYCREG_CR_PARA_RD_DATA_MASK;
> +
> + return 0;
> +}
> +
> static int hdmirx_phy_register_write(struct snps_hdmirx_dev *hdmirx_dev,
> u32 phy_reg, u32 val)
> {
> struct device *dev = hdmirx_dev->dev;
>
> + guard(mutex)(&hdmirx_dev->phy_rw_lock);
> +
> reinit_completion(&hdmirx_dev->cr_write_done);
> /* clear irq status */
> hdmirx_clear_interrupt(hdmirx_dev, MAINUNIT_2_INT_CLEAR, 0xffffffff);
> @@ -1814,6 +1855,13 @@ static void mainunit_2_int_handler(struct snps_hdmirx_dev *hdmirx_dev,
> *handled = true;
> }
>
> + if (status & PHYCREG_CR_READ_DONE) {
> + hdmirx_update_bits(hdmirx_dev, MAINUNIT_2_INT_MASK_N,
> + PHYCREG_CR_READ_DONE, 0);
> + complete(&hdmirx_dev->cr_read_done);
> + *handled = true;
> + }
> +
> if (status & TMDSVALID_STABLE_CHG) {
> process_signal_change(hdmirx_dev);
> v4l2_dbg(2, debug, v4l2_dev, "%s: TMDSVALID_STABLE_CHG\n", __func__);
> @@ -2313,18 +2361,51 @@ static void hdmirx_disable_all_interrupts(struct snps_hdmirx_dev *hdmirx_dev)
> hdmirx_clear_interrupt(hdmirx_dev, CEC_INT_CLEAR, 0xffffffff);
> }
>
> -static void hdmirx_init(struct snps_hdmirx_dev *hdmirx_dev)
> +static int hdmirx_detect_broken_interrupt(struct snps_hdmirx_dev *hdmirx_dev)
> {
> + int ret;
> + u32 val;
> +
> + enable_irq(hdmirx_dev->hdmi_irq);
> +
> + hdmirx_writel(hdmirx_dev, PHYCREG_CONFIG0, 0x3);
> +
> + ret = hdmirx_phy_register_read(hdmirx_dev,
> + HDMIPCS_DIG_CTRL_PATH_MAIN_FSM_FSM_CONFIG,
> + &val);
> +
> + disable_irq(hdmirx_dev->hdmi_irq);
> +
> + return ret;
> +}
> +
> +static int hdmirx_init(struct snps_hdmirx_dev *hdmirx_dev)
> +{
> + int ret;
> +
> hdmirx_update_bits(hdmirx_dev, PHY_CONFIG, PHY_RESET | PHY_PDDQ, 0);
>
> regmap_write(hdmirx_dev->vo1_grf, VO1_GRF_VO1_CON2,
> (HDMIRX_SDAIN_MSK | HDMIRX_SCLIN_MSK) |
> ((HDMIRX_SDAIN_MSK | HDMIRX_SCLIN_MSK) << 16));
> +
> + /*
> + * RK3588 downstream version of TF-A remaps HDMIRX interrupt and
> + * requires use of a vendor-specific FW API that we don't support
> + * in this driver.
> + */
> + ret = hdmirx_detect_broken_interrupt(hdmirx_dev);
> + if (ret)
> + dev_err_probe(hdmirx_dev->dev, ret,
> + "interrupt not functioning, open-source TF-A is required by this driver\n");
> +
> /*
> * Some interrupts are enabled by default, so we disable
> * all interrupts and clear interrupts status first.
> */
> hdmirx_disable_all_interrupts(hdmirx_dev);
> +
> + return ret;
> }
>
> /* hdmi-4k-300mhz EDID produced by v4l2-ctl tool */
> @@ -2606,10 +2687,12 @@ static int hdmirx_probe(struct platform_device *pdev)
> return dev_err_probe(dev, PTR_ERR(hdmirx_dev->regs),
> "failed to remap regs resource\n");
>
> + mutex_init(&hdmirx_dev->phy_rw_lock);
> mutex_init(&hdmirx_dev->stream_lock);
> mutex_init(&hdmirx_dev->work_lock);
> spin_lock_init(&hdmirx_dev->rst_lock);
>
> + init_completion(&hdmirx_dev->cr_read_done);
> init_completion(&hdmirx_dev->cr_write_done);
> init_completion(&hdmirx_dev->timer_base_lock);
> init_completion(&hdmirx_dev->avi_pkt_rcv);
> @@ -2623,7 +2706,10 @@ static int hdmirx_probe(struct platform_device *pdev)
> hdmirx_dev->timings = cea640x480;
>
> hdmirx_enable(dev);
> - hdmirx_init(hdmirx_dev);
> +
> + ret = hdmirx_init(hdmirx_dev);
> + if (ret)
> + goto err_pm;
>
> v4l2_dev = &hdmirx_dev->v4l2_dev;
> strscpy(v4l2_dev->name, dev_name(dev), sizeof(v4l2_dev->name));
> diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
> index 220ab99ca611..a719439d3ca0 100644
> --- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
> +++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
> @@ -114,6 +114,8 @@
> #define PHYCREG_CR_PARA_WRITE_P BIT(1)
> #define PHYCREG_CR_PARA_READ_P BIT(0)
> #define PHYCREG_STATUS 0x00f4
> +#define PHYCREG_CR_PARA_DATAVALID BIT(24)
> +#define PHYCREG_CR_PARA_RD_DATA_MASK GENMASK(15, 0)
>
> #define MAINUNIT_STATUS 0x0150
> #define TMDSVALID_STABLE_ST BIT(1)
> --
> 2.51.0
>
On 10/2/25 18:33, Sebastian Reichel wrote: > Hello Dmitry, > > On Thu, Oct 02, 2025 at 05:07:50PM +0300, Dmitry Osipenko wrote: >> Downstream version of RK3588 U-Boot uses customized TF-A that remaps >> HDMIRX hardware interrupt, routing it via firmware that isn't supported >> by upstream driver. >> >> Detect broken interrupt and print a clarifying error message about a need >> to use open-source TF-A with this driver. >> >> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> >> --- >> Changelog: >> >> v2: - Added PHY r/w lock and moved the clarifying error message as >> was suggested by Sebastian Reichel. >> >> .../platform/synopsys/hdmirx/snps_hdmirx.c | 90 ++++++++++++++++++- >> .../platform/synopsys/hdmirx/snps_hdmirx.h | 2 + >> 2 files changed, 90 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c >> index b7d278b3889f..e6456352dfa5 100644 >> --- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c >> +++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c > > #include <linux/cleanup.h> > > Otherwise LGTM and hopefully helps people to figure out the root > cause of their problems: > > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> Thanks for the review. The cleanup.h always included by mutex/spinlock headers, there is no need to include it explicitly. -- Best regards, Dmitry
© 2016 - 2026 Red Hat, Inc.