Op5+/HDMI-RX: fix spurious triggering of irq 5v while plugout code is running

[benhoff]

Description

HDMI-rx does not properly report if it is plugged out and says that it is plugged in all the time. The cause is the following.

There is an irq that is watching a gpio for 5v high.

During the plugout method for the driver, the driver resets the device state. If the irq that is watching for gpio 5 volts is not disabled during this time, it triggers on a transient voltage and sets the driver to run the "High"/plugged in logic. By the time the driver reaches the high logic, the gpio is low again, which resets the cycle by re-calling the plugout method.

This is probably why @efectn disabled the log in 0a0eb46 , because it's so noisy and spams the logs.

I don't know if this is the right solution, but I know it prevents this cycle by disabling the irq during the plugout code sequence.

How Has This Been Tested?

Tested on orange pi 5 plus

Checklist:

Please delete options that are not relevant.

• [ x ] My changes generate no new warnings

[EvilOlaf]

I assume this also is the cause of having a loadavg >=1 with hdmirx enabled which was first reported here: Joshua-Riek/ubuntu-rockchip#606

[benhoff]

@EvilOlaf , potentially, if the HDMI is unplugged.

There's still a lot of processing that is happening in the driver if an active HDMI input plugged in but not transmitting. So in the case of an hdmi output from a computer that is sleeping... There's a code path that runs continuously that starts with hdmirx_set_edid, and then continues with the following call tree (loosely):

delayed_work_hotplug calls hdmirx_plugin calls hdmirx_wait_lock_and_get_timing, the result of this function in hdmirx_plugin causes that function to queue up another delayed_work_hotplug, which starts the cycle all over again.

I'm not sure if this is intended behavior or not.

[benhoff]

This second commit stops the circular calls when my monitor is turned off. It removes hdmirx_wait_lock_and_get_timing from the hdmirx_plugin function.

I believe that pkt_2_int_handler called from hdmirx_hdmi_irq_handler functionally does the same thing as hdmirx_wait_lock_and_get_timing does.

I believe when the driver is reset via hdmirx_plugin/hdmirx_plugout, the interrupt, pkt_2 from PKT_2_INT_STATUS, is going to be set by the hardware itself and will call the logic to reset timing info.

So we can remove the code that will circularly call itself endlessly, as I believe that is functionally useless.

Looking at some of my logs when the driver is running, this appears to confirm my suspicion.

By squinting at the driver code it looks like the key to hdmirx_wait_lock_and_get_timing is the line reinit_completion(&hdmirx_dev->avi_pkt_rcv);, which also will get called in pkt_2_int_handler.

I haven't 1000% confirmed my suspicions and I'm honestly not sure how to completely test it.

I do know, however, that my end functionality is still working and I'm not getting endless log spam. So this quashes two bugs that are obviously in the driver.

Maybe someone else smarter than me can come along and confirm why this works.

[igorpecovnik]

my end functionality is still working and I'm not getting endless log spam

Moving in?

[benhoff]

Moving in?

Hey @igorpecovnik ! I'm not sure what you are asking.

The original code path is unstable and runs continuously which spams the logs. With the patchset applied, this removes the unstable code path and does not impact the functionality of the driver.

[igorpecovnik]

! I'm not sure what you are asking.

If we merge this in a present state? This is your call.

[benhoff]

! I'm not sure what you are asking.

If we merge this in a present state? This is your call.

Yes, I would merge. This code is ready & tested quite a bit now.

[rpardini]

Hmm, seems to me this landed to the 6.10 directory after 6.11 was bumped.
Please review and send PR also for 6.11 or this will be lost like tears in the rain...