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

[Lucas De Marchi]

On Tue, Feb 08, 2022 at 10:31:10AM +0000, Tvrtko Ursulin wrote:
>
>On 02/02/2022 16:26, Lucas De Marchi wrote:
>>On Wed, Feb 02, 2022 at 10:26:46AM +0000, Tvrtko Ursulin 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 at intel.com> wrote:
>>>>>>On Tue, Feb 01, 2022 at 11:15:31AM +0000, Tvrtko Ursulin wrote:
>>>>>>>From: Tvrtko Ursulin <tvrtko.ursulin at 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.
>>
>>to be clear, the goal is to be more architecture independent
>>and this is being tested with arm64.
>>
>>>
>>>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.
>>
>>yes I remember that. But when it was done the IS_GRAPHICS_VER() was
>>already not much used. I doubt the handful places it was used in would
>>change much of the overall picture we had at that point.
>
>Right, I was referring to the time when N is inside the macro. Like 
>before IS_GEN(p, N) was replaced with GRAPHICS_VER, when N went 
>outside the macro.
>
>>>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.
>>>
>>>3)
>>>Complex step is putting LTO into use to allow dead code 
>>>elimination between compilation units. Think:
>>>
>>>file1.c:
>>>RUNTIME_INFO->needs_blah = IS_PLATFORM && blah
>>>
>>>file2.c
>>>if (RUNTIME_INFO->needs_blah)
>>>  ..dead code eliminated by LTO only..
>>>
>>>Few years ago Andi Kleen had a proof of concept of KBuild LTO but 
>>>I don't know what is the status of that. If LTO can be enabled 
>>>then work from steps 2a&b automatically works much much better.
>>>
>>>4)
>>>If LTO is not there yet maybe see how to slightly refactor the 
>>>code so that we can completely drop compilation units from the 
>>>Makefile. Drop files like gt/intel_ring_submission.c, or similar 
>>>if we have or can create candidates.
>>
>>I think this is something we have to do and to go after. I think it's
>>more future-proof to organize the code to have a better separation of
>>platforms, instead of relying on the linker to do the dead code
>>elimination based on user selection.
>
>Yes, definitely high level refactor. I just think it can't be fully 
>done and some dead code elimination is cheap enough to have to handle 
>the remaining parts.
>
>>I'm fine with compiling out stuff on i915 right now because "this can
>>only and will ever (as far as we know) work on x86". I'm more reluctant
>>about adding a build option though. For the same reasons Jani mentioned
>>and that it suddenly may open the doors for a matrix of configuration
>>(e.g.  build with/without display, or rc6, or dsi, or
>>name-the-feature-you-dont-want).
>
>Yeah okay, I was not saying it absolutely needs to be user visible. 
>Just that it could be argued it could be. Think people building custom 
>kernels and selecting AMD CPU and having Intel discrete graphics.

maybe start with the non-user-visible:  make the config selection depend
on arch. Depending how it goes, we then allow it to be configurable
in future.

Lucas De Marchi

>
>Regards,
>
>Tvrtko