Re: [RFC] drm/i915: Smarten up and use to_i915() everywhere

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 17, 2016 at 08:46:05PM +0200, Jani Nikula wrote:
> On Thu, 17 Mar 2016, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > --
> > 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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux