On 2023-07-28 18:03:35, Dmitry Baryshkov wrote:
<snip>
> > > - if (irq_idx < 0 || irq_idx >= intr->total_irqs) {
> > > + if (!irq_idx || irq_idx > intr->total_irqs) {
> > > pr_err("invalid IRQ index: [%d]\n", irq_idx);
> >
> > Logs like this might be harder to interpret (and compare) when the
> > numbering is different. In addition, all the IRQs in
> > /d/dri/0/debug/core_irq are shifted up by 1 making them harder to
> > compare to downstream.
> > (Which I hope to not have to do again for a while, now that my INTF TE
> > series is finalized and merged)
>
> I hesitated here. Maybe we should log the register and index instead
> of logging the raw index.
Sure that might help.
> As for the core_irq vs downstream, that's a good question. I don't
> like the idea of adding -1 there. Maybe I'll change that again to
> register + index.
I don't like that either. It also isn't a given that the numbers will
always match up, as they're a software/kernel construct for bookkeeping
purposes and not a hardware index. In other words, it only matches when
enum dpu_hw_intr_reg is in sync with downstream. And I don't think it
still is (in the higher numbers) now that 7xxx variants have been
culled.
Preferably both downstream and upstream should have a named "interrupt
register" together with the bit index, but again it's not something
that's super critical nor used often.
- Marijn
