Quoting Tvrtko Ursulin (2018-06-04 11:58:00) > > On 04/06/2018 11:12, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-06-04 10:25:46) > >> > >> On 01/06/2018 15:07, Chris Wilson wrote: > >>> Quoting Tvrtko Ursulin (2018-06-01 15:02:38) > >>>> > >>>> On 31/05/2018 19:51, Chris Wilson wrote: > >>>>> In the next patch, we will begin processing the CSB from inside the > >>>>> interrupt handler. This means that updating the execlists->port[] will > >>>> > >>>> The message that we will be processing CSB from irq handler, here and in > >>>> following patch 5/11, doesn't seem to be true. So why is this needed? > >>>> Why not just drop some patches from the series? > >>> > >>> It will still be called from irq context; as submit_request is called > >>> from irq context. Hence we still require irqsafe variants. > >> > >> submit_request is already called from irqoff contexts so I don't get it. > > > > Right, but the patch series pulls the CSB processing into > > submit_request as well. > > > >>> So yes, while the overall direction of the patchset changed slightly to > >>> be less enthusiastic about calling from irq context, such calls were not > >>> eliminated. > >> > >> I have a problem with commit messages where one says we'll be processing > >> CSB directly from hard irq, while another says we'll always use the tasklet. > > > > It's done inside the tasklet callback; the tasklet callback may be > > invoked directly, and that may also happen inside an irq. > > Via notify_ring yes. I was looking for something on the context switch > path all this time. Also don't forget third-party callers may come in from under their irq. > So CSB processing via notify_ring is quite optimistic and my question is > whether it brings anything? Or whole change is purely due dequeue? That's the essence of the major improvement for ring switching. Pretty sure I cooked up gem_exec_latency/rthog to show that one as well, if not that's certainly one I'd like to show. > Another mystery later in the patch series is what happens with > writel(execlists->csb_read), which is mmio, but I see you have removed > forcewake handling and one commit says there is no more mmio. We removed forcewake for that last year. Where I say mmio, think forcewaked mmio reads. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
