On Thu, May 16, 2024 at 09:31:01PM +0200, Mark Kettenis wrote:
> 
> You're on to something.
> 
> The main difference between legacy INTx interrupts and MSIs is that
> the former are level-triggered, where as the latter are
> edge-triggered.  Level-triggered interrupts are a bit more forgiving
> in the sense that if you don't handle all interrupts, the interrupt
> will just fire again.  But edge-triggered interrupts will only trigger
> if the interrupt condition is properly cleared and a new interrupt
> happens.
> 
> At first sight the driver seems to do the right thing for
> edge-triggered interrupts.  The handler reads the INTSTS register and
> then writes to it to clear the interrupt condition.  It is reasonable
> to expect that if further interrupts are triggered, we will get a new
> MSI message.  However, if you look at the spec:
> 
> https://www.intel.com/content/www/us/en/standards/high-definition-audio-specification.html
> 
> you'll see the INTSTS is a RO (read-only) registers; not a W1C
> (write-one to clear) register.  So on spec-compliant hardware, this
> write doesn't do anything!  Which means that any new interrupts coming
> in while we're processing the initial ones will cause the interrupt to
> remain asserted without a new MSI to happen.
> 
> So instead the interrupt handler has to continue to process interrupts
> as long as bits for the enabled interrupts are set.  Only when none of
> the bits are set can we return and be sure a new MSI to happen.
> 
> Below is a new diff that does this and gets rid of all the bogus
> INTSTS writes.
> 
> This seems to work fine for me on Intel hardware.
> 
>

Thank you for looking at this.

The "clear interrupt" logic you describe is already present in the
rirb and stream handlers and seems correct. Despite writing on a RO
register being wrong, there's no need to clear the interrupt source at
the controller level.

Does it fix the AMD 17h/1xh problem?