On Tue, Jun 18, 2019 at 03:46:29PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-06-18 15:21:06)
> > @@ -59,14 +57,13 @@ TRACE_EVENT(intel_pipe_disable,
> > ),
> >
> > TP_fast_assign(
> > - enum pipe _pipe;
> > - for_each_pipe(dev_priv, _pipe) {
> > - __entry->frame[_pipe] =
> > - dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, _pipe);
> > - __entry->scanline[_pipe] =
> > - intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, _pipe));
> > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > + struct intel_crtc *_crtc;
> > + for_each_intel_crtc(&dev_priv->drm, _crtc) {
> > + __entry->frame[_crtc->pipe] = intel_crtc_get_vblank_counter(_crtc);
> > + __entry->scanline[_crtc->pipe] = intel_get_crtc_scanline(_crtc);
> > }
> > - __entry->pipe = pipe;
> > + __entry->pipe = crtc->pipe;
>
> Ok. Stared hard to make sure it was _crtc and not crtc. Would crtc__ be
> more obvious, or maybe it__ so that it doesn't look anything like the
> crtc argument.
I suppose a more distinct name might be a good idea.
>
> > TP_fast_assign(
> > - enum pipe pipe;
> > - for_each_pipe(dev_priv, pipe) {
> > - __entry->frame[pipe] =
> > - dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm, pipe);
> > - __entry->scanline[pipe] =
> > - intel_get_crtc_scanline(intel_get_crtc_for_pipe(dev_priv, pipe));
> > + struct intel_crtc *crtc;
> > + for_each_intel_crtc(&dev_priv->drm, crtc) {
> > + __entry->frame[crtc->pipe] = intel_crtc_get_vblank_counter(crtc);
> > + __entry->scanline[crtc->pipe] = intel_get_crtc_scanline(crtc);
> > }
>
> Or even a populate_vblanks(i915, __entry->frame, __entry->scanline)
>
> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> -Chris
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
