Re: [Intel-gfx] [RFC 0/2] Compile out integrated

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 02 Feb 2022, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> On 02/02/2022 11:20, Jani Nikula wrote:
>> On Wed, 02 Feb 2022, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>>> On 01/02/2022 17:28, Lucas De Marchi wrote:
>>>> On Tue, Feb 01, 2022 at 07:09:14PM +0200, Jani Nikula wrote:
>>>>> On Tue, 01 Feb 2022, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>>>>>> On Tue, Feb 01, 2022 at 11:15:31AM +0000, Tvrtko Ursulin wrote:
>>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>
>>>>>>> Quicky and dirty hack based on some old ideas. Thought maybe the
>>>>>>> approach might
>>>>>>> interest the Arm port guys. But with IS_GEN_RANGE removed easy gains
>>>>>>> are not so
>>>>>>> big so meh.. Maybe some more easy wins with IS_DISPLAY_VER but I
>>>>>>> haven't looked
>>>>>>> into that side.
>>>>>>>
>>>>>>> 3884664  449681    6720 4341065  423d49 i915.ko.tip
>>>>>>> 3599989  429034    6688 4035711  3d947f i915.ko.noigp
>>>>>>
>>>>>> By these numbers probably it's hard to justify. Another thing to
>>>>>> consider
>>>>>> is that it's very common to have on the same system both
>>>>>> integrated and discrete - doing this would remove at compile time any
>>>>>> chance of driving the integrated one.
>>>>>
>>>>> I guess the point was, the arm systems won't have integrated, and it's
>>>>> anyway going to be a separate build.
>>>>
>>>> so probably the focus and argument here should not be about size
>>>> reduction. From patch 1 I see:
>>>>
>>>> +config DRM_I915_INTEGRATED_GPU_SUPPORT
>>>> +       bool "Support integrated GPUs"
>>>> +       default y
>>>> +       depends on DRM_I915
>>>> +       help
>>>> +         Include support for integrated GPUs.
>>>>
>>>> If it's something that depends on arch rather than providing an
>>>> option in menuconfig, then I think it could be some interesting
>>>> investigation. However, I can't see how it would help with removing
>>>> some code paths in the driver (e.g. the clflush() calls we were talking
>>>> about in another patch series) since the code elimination would all
>>>> happen at link time.
>>>
>>> Clflush class of problems is yet another orthogonal set of problems.
>>>
>>> Yes, idea was that the Kconfig option would be selected by Arm, or
>>> deselected by x86, whatever. But there is also a case for letting it be
>>> user visible.
>>>
>>> In general, I thought at least, we should look into not
>>> building/deploying binary code for irrelevant hardware on Arm builds. If
>>> that is clear and agreeable then I think the approach how to get there
>>> is really multi-pronged.
>>>
>>> 1)
>>> What you are partly doing with "clflush" type series. Make Arm relevant
>>> code paths actually compile on Arm.
>>>
>>> 2a)
>>> What I sent in this series - it's simple/easy dead code elimination from
>>> a single compilation unit.
>>>
>>> 2b)
>>> *If* we resurrected GRAPHICS_VER check where "ver" is part of the macro,
>>> eg. not doing "if (GRAPHICS_VER <=> N)" but "if (GRAPHICS_VERN)", or "if
>>> IS_GRAPHICS_VER(N, FOREVER)", then the same approach would be more
>>> effective.
>>>
>>> Because if N or range is the macro parameter, we can make it dead code
>>> based on Kconfig.
>>>
>>> This is what I demonstrated few years ago by being able to compile out
>>> ~3rd of a driver when selecting only execlists platforms, AFAIR.
>>>
>>> And which is why I was a bit unhappy this was getting removed not so
>>> long ago.
>> 
>> The main problem with that, as well as the Kconfig here, is maintenance.
>> 
>> If it's fancy but unused, it's just added complexity for no benefit,
>> just the drawbacks. Every change needs to take the complexity into
>> account. If it's unused and untested, it's just going to bitrot anyway.
>> 
>> For example, I think a config option for disabling igfx should have both
>> build and runtime testing in place before we should consider taking on
>> the burden of maintaining it. Otherwise it's just haphazard struggle,
>> and the burden falls on a handful of interested people working on it on
>> the side, occasionally fixing things as they break. And they'll break
>> because nobody else cares.
>> 
>> If someone shows up and says i915.ko is too big, they need to be serious
>> enough to invest in maintaining the configurable size reductions, per
>> target platform.
>
> Yeah no disagreement for the most part.
>
> Whether there is a cheap way (as in maintenance/intrusiveness) which 
> brings gains large enough.
>
> In my view it is also not a question of "too big" per se, but a question 
> of professional pride of doing things properly, even if it is hard, 
> instead of padding the binaries with dead code and sending them on 
> rounds over the world, to "use electrons" unreachable on uncountable 
> hypothetical customer machines and travel as updates over undersea 
> cables for no use. :D Just because we, sitting on the source/upstream, 
> decided it's too hard.

Well, not because it's too hard, but mostly because of picking the
battles? Also a matter of taste in what constitutes as professional
pride. If it's worth doing, it's worth doing properly?

But I guess we are, mostly, on the same page here.

BR,
Jani.


>
> Not pressuring anything though, we are not quite there yet to worry 
> about scale deployments of Intel discrete on !x86. There is time. :) 
> I've thrown some seeds out there, if they don't take, they don't. It is 
> fine to tackle the "make it build and work" steps first.
>
> Regards,
>
> Tvrtko

-- 
Jani Nikula, Intel Open Source Graphics Center