On pe, 2017-03-24 at 08:59 +0000, Chris Wilson wrote: > On Thu, Mar 23, 2017 at 09:36:10AM -0700, Oscar Mateo wrote: > > > > On 03/23/2017 03:57 PM, Chris Wilson wrote: > > > > > > I'm not happy with moving subfeature detection logic into the core GEM > > > code. if (i915.enable_guc_loading) firstly should never be a module > > > parameter (it's derived state!) and secondly it should reside next to > > > the dependent logic and not be interrupting the central control flow. > > What do you mean it's derived state? from what? > > The set of features, whether to use guc submission, huc, or whether > there is a platform requirement to load the firmware, define whether or > not we need to request and upload a particular firmware. Every module > option should be a quirk to alter driver behaviour (i.e. a debugging > crutch), few and strongly justified (we have too many) and necessarily > global in scope. Device specific options should ideally use a more > specific interface (most clear examples are the panel specific quirks). Misguidance here was probably me enforcing the view that by hoisting and unifying the checks, it'll be easier to comprehend the code when a check covers greater amount of lines. I have to agree that the top- level driver control flow should not be having the checks, but each code path existing only within a .c file should have the "skip this branch" check earlier than later. Sorry for misguidance :P Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
