Quoting Mika Kuoppala (2017-10-20 12:12:02) > Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> writes: > >> @@ -890,7 +902,7 @@ static void intel_lrc_irq_handler(unsigned long data) > >> } > >> > >> /* After the final element, the hw should be idle */ > >> - GEM_BUG_ON(port_count(port) == 0 && > >> + GEM_BUG_ON(port_count(execlists_port_head(execlists)) == 0 && > >> !(status & GEN8_CTX_STATUS_ACTIVE_IDLE)); > > > > Why did you stop trusting port variable here? > > > > We did assing it pre loop before. Now we do it inside the loop. Also > I thought I made a favour for reader (and for the bug triager > as GEM_BUG_ON might soon show condition in dmesg) > to note that it is always the first port count we assert > for idleness. I would favour assert(port == el_port_head(el)); where appropriate. The intent is that we comment upon the rationale of the GEM_BUG_ON() so that we have an aide-memoire as to where to begin the hunt. For triaging, we just need to be able to recognise the same BUG_ON as you only need to dupe to the first one to catch up with the bug analysis. We should err on making GEM_BUG_ON() fit the code better than a potential bug report. We should be reading the code far more often... -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
