[Intel-gfx] [RFC] drm/i915: Smarten up and use to_i915() everywhere

Chris Wilson chris at chris-wilson.co.uk
Thu Mar 17 19:17:54 UTC 2016

On Thu, Mar 17, 2016 at 08:46:05PM +0200, Jani Nikula wrote:
> On Thu, 17 Mar 2016, Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com> wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >
> > There is a lot of ways to get to our dev_priv depending on which
> > object is at hand and often what was chosen by the developer.
> >
> > We can make to_i915() accept different pointers by using compile
> > time magic. Like:
> >
> >   dev_priv = to_i915(request);
> >   dev_priv = to_i915(engine);
> >   dev_priv = to_i915(ctx);
> >   dev_priv = to_i915(dev);
> >   dev_priv = to_i915(guc);
> >   dev_priv = to_i915(device);
> >
> > If an unknown pointer is passed to the function it will cause
> > a compile time failure.
> >
> > Main advantage is that with this in place we could add and
> > remove shourtcuts to dev_priv from supported structures easily
> > and without touching the code which uses it. If we wanted to
> > fiddle with the balance of structure sizes and number of pointer
> > dereferencing for example. And it makes the code a bit tidier
> > and uniform.
> >
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > --
> > However the churn is huge so I don't really think this is a
> > must have.
> The "magic" __I915__() macro was added to support a transition from
> using dev pointer to using dev_priv pointer. I like the transition, and
> we slowly keep doing it.
> IMO there have been two problems with that. First, the transition is
> slow, because there's nothing forcing us to switch. This was expected,
> as we explicitly didn't want a huge patch (like this). Second, it
> appears to *still* confuse people after over a year that you can pass
> either type of pointer to the macros in C.
> I object to this patch both because it's huge (and I'll get my fair
> share of the conflicts) and, more importantly, because it promotes an
> appearance of a sort of dynamic typing in a statically typed
> language. The latter contains an element of surprise to the programmer,
> and surprising is not a quality I want to associate with code.

Hmm, when I looked it, I thought I can replace all of my T_to_i915()
with just to_i915() which I expect will pay dividends in making the code
readable. Plus in many instances it allows us to drop random locals etc.

As it stands this patch doesn't show any advantages.

Chris Wilson, Intel Open Source Technology Centre

More information about the Intel-gfx mailing list