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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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.

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