Re: [PATCH resend for CI 2/2] drm/i915: Use shorter route to dev_private where possible

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Mar 17, 2016 at 01:55:57PM +0000, Chris Wilson wrote:
> On Thu, Mar 17, 2016 at 01:47:31PM +0000, Tvrtko Ursulin wrote:
> > 
> > On 17/03/16 13:17, Chris Wilson wrote:
> > >On Thu, Mar 17, 2016 at 01:04:10PM +0000, Tvrtko Ursulin wrote:
> > >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >>
> > >>Where we have a request we can use req->i915 directly instead
> > >>of going through the engine and device. Coccinelle script:
> > >>
> > >>@@
> > >>function f;
> > >>identifier r;
> > >>@@
> > >>f(..., struct drm_i915_gem_request *r, ...)
> > >>{
> > >>...
> > >>- engine->dev->dev_private
> > >>+ r->i915
> > >>...
> > >>}
> > >>@@
> > >>struct drm_i915_gem_request *req;
> > >>@@
> > >>(
> > >>   req->
> > >>- engine->dev->dev_private
> > >>+ i915
> > >>)
> > >
> > >struct intel_engine_cs *e;
> > >- e->dev->dev_private
> > >+ e->i915
> > >
> > >Pretty please?
> > 
> > There is no e->i915 yet as far as I can see.
> 
> Oh, you have seen it. It might be over there instead :|
> 
> > >INTEL_INFO(engine->dev) (IS_GEN, HAS_ etc)
> > >if r: INTEL_INFO(r->i915)
> > >if e: INTEL_INFO(e->i915)
> > >?
> > 
> > I have a patch to do:
> > 
> > - dev->dev_private
> > + to_i915(dev)
> > 
> > But that is huge and to little gain.
> > 
> > Maybe we should have:
> > 
> > to_i915(req)
> > to_i915(engine)
> > to_i915(dev)
> > 
> > And a smart macro which does the right thing at compile time?
> 
> Tempting. Should look nice and consistent and such magic would improve
> the look of many opening stanzas.

This is imo a bit too much magic ... reg_to_i915, engine_to_i915 and
to_i915 all sound like good ideas (we have lots of examples where we avoid
the superclass, e.g. to_hdmi or similar).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx