[PATCH] media: hdmirx: snps, rk: Fix EDID injection with proper HPD toggle on RK3588
Ross Cawston
ross at r-sc.ca
Mon Mar 16 07:42:34 PDT 2026
On 2026-03-16 7:24 a.m., Hans Verkuil wrote:
> On 09/02/2026 07:16, Ross Cawston wrote:
>> The existing VIDIOC_S_EDID implementation writes EDID data but does not
>> properly trigger source renegotiation on RK3588 boards. This results in
>> no visible change to the source device despite the ioctl succeeding.
>>
>> Even re-plugging the HDMI cable does not trigger renegotiation on previous
>> versions of the driver. This version reliably triggers renegotiation on
>> EDID injection on tested hardware.
>>
>> Fix by mirroring the vendor BSP behavior:
>> - Disable HDMI and DMA IRQs to avoid races.
>> - Simulate plugout if 5V power is present.
>> - Toggle HPD low before write.
>> - Schedule a delayed hotplug workqueue (1000ms) to re-enable HPD and
>> force renegotiation.
>>
>> Bump WAIT_SIGNAL_LOCK_TIME from 300ms to 600ms to ensure lock.
>>
>> This ensures custom EDIDs take effect reliably, allowing userspace to
>> force specific resolutions/timings.
>>
>> Tested on Orange Pi 5 Ultra and Radxa Rock 5B — sources now correctly
>> re-detect and lock to the new EDID modes.
>>
>> Signed-off-by: Ross Cawston <ross at r-sc.ca>
>>
>> ---
>> drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c | 43 +++++++++++++++++------
>> 1 file changed, 37 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
>> index abc123..def456 100644
>> --- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
>> +++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
>> @@ -88,6 +88,10 @@
>> HDMIRX_NUM_RST,
>> };
>>
>> +#define WAIT_SIGNAL_LOCK_TIME 600
>> +#define NO_LOCK_CFG_RETRY_TIME 300
>> +#define WAIT_LOCK_STABLE_TIME 20
>> +
>> static const char *const pix_fmt_str[] = {
>> "RGB888",
>> "YUV422",
>> @@ -620,6 +624,7 @@
>>
>> hdmirx_update_bits(hdmirx_dev, SCDC_CONFIG, POWERPROVIDED, 0);
>> hdmirx_interrupts_setup(hdmirx_dev, false);
>> + hdmirx_hpd_ctrl(hdmirx_dev, false);
>> hdmirx_update_bits(hdmirx_dev, DMA_CONFIG6, HDMIRX_DMA_EN, 0);
>> hdmirx_update_bits(hdmirx_dev, DMA_CONFIG4,
>> LINE_FLAG_INT_EN |
>> @@ -680,16 +685,32 @@
>> */
>> mutex_lock(&hdmirx_dev->work_lock);
>>
>> + /*
>> + * Some sources won't re-read EDID unless we avoid IRQ races and
>> + * force a full plugout/HPD low sequence. On several tested devices,
>> + * leaving IRQs enabled or skipping plugout kept the old EDID cached.
>> + */
> This is weird. There seem to be two (or more) different issues here: IRQ races
> and HPD low->high toggle.
>
> When updating the EDID the HPD must go low before updating the EDID, and
> go high at least 100 ms after it went low. Experience shows that 'HZ / 7'
> is a good value for HDMI receivers. 1000 ms as you are using below is overkill.
>
> So the HPD toggle is required, otherwise video sources won't re-read the EDID.
>
> The IRQ races must be something else, I'm not sure what.
>
> I would recommend splitting this up: first add the HPD toggle since without it
> video sources will not detect the EDID update at all.
That's fair - this was the patch that worked for me reliably after many
revisions. I'll break it up and do some further investigation.
> Then follow up with other patches fixing whatever else is needed to make it
> work reliable. Video sources will know nothing about enabling/disabling the
> IRQ, so if that matters, then it is more likely that somehow keeping the IRQs
> interferes with updating the EDID.
>
> I think you need to better understand what is going on here :-)
Roger that. Thanks for the review.
> Regards,
>
> Hans
>
>> + disable_irq(hdmirx_dev->hdmi_irq);
>> + disable_irq(hdmirx_dev->dma_irq);
>> +
>> + if (tx_5v_power_present(hdmirx_dev))
>> + hdmirx_plugout(hdmirx_dev);
>> +
>> hdmirx_hpd_ctrl(hdmirx_dev, false);
>>
>> if (edid->blocks) {
>> hdmirx_write_edid(hdmirx_dev, edid);
>> - hdmirx_hpd_ctrl(hdmirx_dev, true);
>> } else {
>> cec_phys_addr_invalidate(hdmirx_dev->cec->adap);
>> hdmirx_dev->edid_blocks_written = 0;
>> }
>>
>> + enable_irq(hdmirx_dev->dma_irq);
>> + enable_irq(hdmirx_dev->hdmi_irq);
>> +
>> + queue_delayed_work(system_unbound_wq, &hdmirx_dev->delayed_work_hotplug,
>> + msecs_to_jiffies(1000));
>> +
>> mutex_unlock(&hdmirx_dev->work_lock);
>>
>> return 0;
>> @@ -2082,9 +2103,9 @@
>> {
>> struct v4l2_device *v4l2_dev = &hdmirx_dev->v4l2_dev;
>> u32 mu_status, scdc_status, dma_st10, cmu_st;
>> - u32 i;
>> + u32 i, j = 0;
>>
>> - for (i = 0; i < 300; i++) {
>> + for (i = 1; i < WAIT_SIGNAL_LOCK_TIME; i++) {
>> mu_status = hdmirx_readl(hdmirx_dev, MAINUNIT_STATUS);
>> scdc_status = hdmirx_readl(hdmirx_dev, SCDC_REGBANK_STATUS3);
>> dma_st10 = hdmirx_readl(hdmirx_dev, DMA_STATUS10);
>> @@ -2093,8 +2114,16 @@
>> if ((mu_status & TMDSVALID_STABLE_ST) &&
>> (dma_st10 & HDMIRX_LOCK) &&
>> (cmu_st & TMDSQPCLK_LOCKED_ST))
>> + j++;
>> + else
>> + j = 0;
>> +
>> + if (j > WAIT_LOCK_STABLE_TIME)
>> break;
>>
>> + if (i % NO_LOCK_CFG_RETRY_TIME == 0)
>> + hdmirx_phy_config(hdmirx_dev);
>> +
>> if (!tx_5v_power_present(hdmirx_dev)) {
>> v4l2_dbg(1, debug, v4l2_dev,
>> "%s: HDMI pull out, return\n", __func__);
>> @@ -2104,7 +2133,7 @@
>> hdmirx_tmds_clk_ratio_config(hdmirx_dev);
>> }
>>
>> - if (i == 300) {
>> + if (i == WAIT_SIGNAL_LOCK_TIME) {
>> v4l2_err(v4l2_dev, "%s: signal not lock, tmds_clk_ratio:%d\n",
>> __func__, hdmirx_dev->tmds_clk_ratio);
>> v4l2_err(v4l2_dev, "%s: mu_st:%#x, scdc_st:%#x, dma_st10:%#x\n",
>> @@ -2127,7 +2156,8 @@
>> PKTDEC_AVIIF_RCV_IRQ, 0);
>> }
>>
>> - msleep(50);
>> + hdmirx_reset_dma(hdmirx_dev);
>> + msleep(500);
>> hdmirx_format_change(hdmirx_dev);
>>
>> return 0;
>> @@ -2141,6 +2171,7 @@
>> hdmirx_submodule_init(hdmirx_dev);
>> hdmirx_update_bits(hdmirx_dev, SCDC_CONFIG, POWERPROVIDED,
>> POWERPROVIDED);
>> + hdmirx_hpd_ctrl(hdmirx_dev, true);
>> hdmirx_phy_config(hdmirx_dev);
>> hdmirx_interrupts_setup(hdmirx_dev, true);
>>
>>
More information about the Linux-rockchip
mailing list