Re: [PATCH 5/6] drm/i915: Remove inline from sseu helper functions

Summers, Stuart
2019-05-02T14:50:46Z

[-- Attachment #1.1: Type: text/plain, Size: 2234 bytes --]

On Thu, 2019-05-02 at 10:15 +0300, Jani Nikula wrote:
> On Wed, 01 May 2019, "Summers, Stuart" <stuart.summers@intel.com>
> wrote:
> > On Wed, 2019-05-01 at 14:19 -0700, Daniele Ceraolo Spurio wrote:
> > >
> > > On 5/1/19 2:04 PM, Summers, Stuart wrote:
> > > > On Wed, 2019-05-01 at 13:04 -0700, Daniele Ceraolo Spurio
> > > > wrote:
> > > > > Can you elaborate a bit more on what's the rationale for
> > > > > this? do
> > > > > you just want to avoid having too many inlines since the
> > > > > paths
> > > > > they're used in are not critical, or do you have some more
> > > > > functional reason? This is not a critic to the patch, I just
> > > > > want to understand where you're coming from ;)
> > > >
> > > > This was a request from Jani Nikula in a previous series
> > > > update. I
> > > > don't have a strong preference either way personally. If you
> > > > don't
> > > > have any major concerns, I'd prefer to keep the series as-is to
> > > > prevent too much thrash here, but let me know.
> > > >
> > >
> > > No concerns, just please update the commit message to explain
> > > that
> > > we're moving them because there is no need for them to be inline
> > > since they're not on a critical path where we need preformance.
> >
> > Sounds great.
>
> I've become critical of superfluous inlines. They break the
> abstraction
> by exposing the internals in the header, and make the
> interdependencies
> of headers harder to resolve.
>
> As the driver keeps growing and more people contribute to it, I think
> we
> need to pay more attention on how we structure the source. To this
> end
> we've added new gt/ subdir, are about to add gem/ and likely display/
> too before long, and we've significantly split off the monster
> i915_drv.h and intel_drv.h headers.
>
> Obviously inlines have their place and purpose, but I think we
> sprinkle
> them a bit too eagerly without paying attention.
>
> I like the patch.
>
> Acked-by: Jani Nikula <jani.nikula@intel.com>

Jani, based on Daniele's feedback, I'm planning on squashing this patch
with the patch that moves these helper functions to intel_sseu.h. Any
issue keeping your Ack here?

-Stuart

>
>

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3270 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]