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

Jani Nikula
2019-05-02T07:13:32Z

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 Nikula, Intel Open Source Graphics Center