On Wed, 02 Feb 2022, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote: > On 02/02/2022 11:20, Jani Nikula wrote: >> On Wed, 02 Feb 2022, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> 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@xxxxxxxxx> wrote: >>>>>> On Tue, Feb 01, 2022 at 11:15:31AM +0000, Tvrtko Ursulin wrote: >>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> >>>>>>> >>>>>>> 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
