Re: [PATCH 1/3] drm/i915: Fix various tracepoints for gen2

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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@chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx