Skip to content

2711 HDMI interrupts possibly declared incorrectly #4837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pelwell opened this issue Jan 21, 2022 · 10 comments
Closed

2711 HDMI interrupts possibly declared incorrectly #4837

pelwell opened this issue Jan 21, 2022 · 10 comments

Comments

@pelwell
Copy link
Contributor

pelwell commented Jan 21, 2022

Describe the bug

There are two elements of the HDMI interrupt declaration in bcm2711.dtsi that look incorrect:

  1. The CEC interrupts for HDMI1 appear to be backwards w.r.t. HDMI0 (and reality):
    HDMI0:

    		interrupts = <0>, <1>, <2>,
    			     <3>, <4>, <5>;
    		interrupt-names = "cec-tx", "cec-rx", "cec-low",
    				  "wakeup", "hpd-connected", "hpd-removed";
    

    HDMI1:

    		interrupts = <8>, <7>, <6>,
    			     <9>, <10>, <11>;
    		interrupt-names = "cec-tx", "cec-rx", "cec-low",
    				  "wakeup", "hpd-connected", "hpd-removed";
    

    Notice the 0, 1, 2 and 8. 7, 6 for the CEC interrupts - the RDB headers suggest that HDMI1 should have 6, 7, 8 instead.

  2. An email on the linux-arm-kernel mailing list mentioned that the declaration of the AON_INTR2 interrupt controller looked wrong - it is declared to take an IRQ_TYPE_LEVEL_HIGH interrupt from the GIC, but that the driver requests the edge interrupt flow (handle_edge_irq). I can see hotplug interrupts getting through with either the current IRQ_TYPE_LEVEL_HIGH or IRQ_TYPE_EDGE_RISING, but the fact that the interrupt controller has no form of acknowledge/clear register does suggest it uses edges rather than levels to indicate interrupts to the GIC.

Steps to reproduce the behaviour

I've not seen a problem yet, but in theory CEC on HDMI1 looks like it could be broken.

Device (s)

Other

System

All current kernels.

Logs

No response

Additional context

No response

@pelwell
Copy link
Contributor Author

pelwell commented Jan 21, 2022

Any thoughts, @mripard?

@popcornmix
Copy link
Collaborator

This rings a bell (I remember debugging this). Ah yes:
popcornmix@e51c2f3

@pelwell
Copy link
Contributor Author

pelwell commented Jan 21, 2022

Ho hum - there's no audit trail or comment to explain.

That just leaves the question about the edge-triggered interrupt.

@popcornmix
Copy link
Collaborator

I have recently tested cec on hdmi0 and hdmi1, so can confirm it that both are working.
I don't know whether we want edge or level (except I thought almost everything used edge).
I could test a patch if you want to propose something more correct, but don't have a cec device to test with.

@pelwell
Copy link
Contributor Author

pelwell commented Jan 21, 2022

Which kernel would you prefer to test?

@pelwell
Copy link
Contributor Author

pelwell commented Jan 21, 2022

See #4838 - it cherry-picks cleanly to 5.15 and 5.16.

@popcornmix
Copy link
Collaborator

Any kernel is fine. I'll test when I'm home as cec display is more convenient.

@mripard
Copy link
Contributor

mripard commented Jan 24, 2022

I guess using an edge vs level would be fine in most cases, hence why it could work. I couldn't find any info regarding this, so I guess at the time I just assumed it would be level since the rest of the interrupts were level triggered.

@popcornmix
Copy link
Collaborator

I commented on #4838 that the change seems fine with cec and hotplug detect.

@pelwell
Copy link
Contributor Author

pelwell commented Jan 24, 2022

Since #4838 is merged and cherry-picked, this issue can be closed.

@pelwell pelwell closed this as completed Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants