From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Summers, Stuart" <stuart.summers@intel.com>,
"Ceraolo Spurio, Daniele" <daniele.ceraolospurio@intel.com>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 5/6] drm/i915: Remove inline from sseu helper functions
Date: Thu, 02 May 2019 10:15:43 +0300 [thread overview]
Message-ID: <87muk5pbs0.fsf@intel.com> (raw)
In-Reply-To: <970d6582f0c5a02c73fabd11631ebfec192b387e.camel@intel.com>
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-05-02 7:13 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-01 15:34 [PATCH 0/6] Refactor to expand subslice mask Stuart Summers
2019-05-01 15:34 ` [PATCH 1/6] drm/i915: Use local variable for SSEU info in GETPARAM ioctl Stuart Summers
2019-05-01 17:54 ` Daniele Ceraolo Spurio
2019-05-01 19:38 ` Summers, Stuart
2019-05-01 15:34 ` [PATCH 2/6] drm/i915: Add macro for SSEU stride calculation Stuart Summers
2019-05-01 18:11 ` Daniele Ceraolo Spurio
2019-05-01 19:37 ` Summers, Stuart
2019-05-01 15:34 ` [PATCH 3/6] drm/i915: Move calculation of subslices per slice to new function Stuart Summers
2019-05-01 18:14 ` Daniele Ceraolo Spurio
2019-05-01 19:37 ` Summers, Stuart
2019-05-01 15:34 ` [PATCH 4/6] drm/i915: Move sseu helper functions to intel_sseu.h Stuart Summers
2019-05-01 18:48 ` Daniele Ceraolo Spurio
2019-05-01 19:36 ` Summers, Stuart
2019-05-01 15:34 ` [PATCH 5/6] drm/i915: Remove inline from sseu helper functions Stuart Summers
2019-05-01 20:04 ` Daniele Ceraolo Spurio
2019-05-01 21:04 ` Summers, Stuart
2019-05-01 21:19 ` Daniele Ceraolo Spurio
2019-05-01 21:28 ` Summers, Stuart
2019-05-02 7:15 ` Jani Nikula [this message]
2019-05-02 14:50 ` Summers, Stuart
2019-05-02 14:58 ` Jani Nikula
2019-05-02 14:58 ` Summers, Stuart
2019-05-01 15:34 ` [PATCH 6/6] drm/i915: Expand subslice mask Stuart Summers
2019-05-01 18:22 ` Tvrtko Ursulin
2019-05-01 18:29 ` Tvrtko Ursulin
2019-05-01 19:40 ` Summers, Stuart
2019-05-01 22:04 ` Daniele Ceraolo Spurio
2019-05-02 14:47 ` Summers, Stuart
2019-05-03 9:05 ` Lionel Landwerlin
2019-05-03 14:28 ` Summers, Stuart
2019-05-01 15:58 ` ✗ Fi.CI.CHECKPATCH: warning for Refactor to expand subslice mask (rev7) Patchwork
2019-05-01 16:01 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-05-01 16:14 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-02 9:14 ` ✓ Fi.CI.IGT: " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2019-04-30 23:06 [PATCH 0/6] Refactor to expand subslice mask Stuart Summers
2019-04-30 23:06 ` [PATCH 5/6] drm/i915: Remove inline from sseu helper functions Stuart Summers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87muk5pbs0.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=daniele.ceraolospurio@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=stuart.summers@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox