Re: [Intel-gfx] [PATCH 03/53] drm/i915: Fork DG1 interrupt handler

Lucas De Marchi
2021-07-07T15:53:30Z

On Wed, Jul 07, 2021 at 09:39:03AM +0200, Daniel Vetter wrote:
>On Wed, Jul 7, 2021 at 12:48 AM Lucas De Marchi
><lucas.demarchi@intel.com> wrote:
>> On Fri, Jul 02, 2021 at 11:21:10AM +0200, Daniel Vetter wrote:
>> >On Thu, Jul 1, 2021 at 10:26 PM Matt Roper <matthew.d.roper@intel.com> wrote:
>> >>
>> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> >>
>> >> The current interrupt handler is getting increasingly complicated and
>> >> Xe_HP changes will bring even more complexity. Let's split off a new
>> >> interrupt handler starting with DG1 (i.e., when the master tile
>> >> interrupt register was added to the design) and use that as the basis
>> >> for the new Xe_HP changes.
>> >>
>> >> Now that we track the hardware IP's release number as well as the
>> >> version number, we can also properly define DG1 has version "12.10" and
>> >> replace the has_master_unit_irq feature flag with an IP version test.
>> >>
>> >> Bspec: 50875
>> >> Cc: Daniele Spurio Ceraolo <daniele.ceraolospurio@intel.com>
>> >> Cc: Stuart Summers <stuart.summers@intel.com>
>> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> >> Signed-off-by: Tomasz Lis <tomasz.lis@intel.com>
>> >> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>> >
>> >So I know DG1 upstream is decidedly non-smooth, but basic
>> >infrastructure we've had since forever ...
>> >
>> >Why was this prep work not upstreamed earlier with some benign commit
>> >message that doesn't mention DG2? There's zero DG2 stuff in here, this
>> >could have landed months/years ago even. Bringing this up since right
>> >this moment we have an internal chat about trees diverging a bit much.
>>
>> history isn't linear and this commit, the way it is now, didn't exist 1
>> month ago, so your timescale is misleading. has_master_unit_irq was what
>> we thought we would need to share as much code as possible.
>>
>> The biggest reason to fork the irq handler is actually not DG1 nor DG2,
>> but XEHPSDV and without those changes it would basically be a 95% copy
>> of the gen11 handler... for someone not looking to what is in the
>> pipeline, it can be a perfect argument to "consolidate these into a
>> single handler".
>
>At least in the past we've done tons of upstream refactor prep for
>exactly just these "prep for future platform" reasons. Everyone
>understand that's necessary and generally trusts us we're not just
>moving code for fun. But then 1-2 years ago we just kinda stopped
>pushing prep work to upstream because everyone got way too busy with
>other things, and now we're paying the price.

we both know this is not the only reason and looking to the people in Cc
here my impression is you're preaching to the choir. Because the people
in Cc here either moved to other teams before there was something
working related to irq to share or continued to do prep work in upstream
as much as they can.

Lucas De Marchi

>I mean even if the reason to fork it is a platform we can't even talk
>about, the fork patch should go upstream way ahead so that there's
>less patches in internal. Ideally platform enabling is zero code
>shuffling, 100% just plugging code into existing neat places.
>-Daniel
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch