On ke, 2016-03-09 at 16:03 +0000, Chris Wilson wrote: > On Wed, Mar 09, 2016 at 05:31:39PM +0200, Imre Deak wrote: > > While working on the CDCLK init code I realized that the driver > > load time > > dependencies between the different init steps are rather difficult > > to follow > > and so it's not obvious where some new piece of initialization > > needs to be > > added. > > > > Also because some things are initialized too late, other steps > > depending on > > these must be initialized in a non-logical place or even split into > > multiple > > parts. One example is the CDCLK initialization which needs the > > display > > callbacks to be set already, but those callbacks are setup only > > late, so the > > CDCLK initialization must be done in two parts. > > > > As a generic solution, I suggest that we define the following load > > time init phases: > > - state init not requiring device access > > (i.e SW only, like initializing locks, allocating system memory, > > setting > > up callbacks, device attributes) > > - minimal HW setup to enable MMIO access to the device > > - state init requiring device access w/o side effects > > (i.e. read-only HW access, no interface registrations) > > - state init causing device-wide side effects > > (i.e any HW access, no interface registration) > > - registering all interfaces > > On paper sounds goods. The only complaint I have is that we have only > nomenclature for 2 phases: init and init_hw. To be compelling I want > consistent names for each init function so that we know at a glance > what > phase we are in, and the expectations/limitations upon the function. > > init_early() / setup() > init_mmio() > init_late() > init_hw() Ok, I moved around the code according to the above: https://github.com/ideak/linux/commits/driver_init_refactor I left out the init_late() step but also added a registration step. There are still some init steps - mostly on the modesetting path - that will need to be moved into one of the above functions, but that can be done later. I can post this patchset after some more testing. --Imre > > This patchset adds the corresponding comment markers for the first > > two phases above and one common phase for the rest of the current > > init steps. Later we could also add the last three init phases > > above > > and restructure the code accordingly. > > > > For now I only moved earlier a few obvious init steps that fit > > these > > new phases. > > > > I smoke tested this on GEN4, SNB, BXT. > > > > Imre Deak (7): > > drm/i915: Add comments marking the start of load time init phases > > drm/i915: Move laod time PCH detect, DPIO, power domain SW init > > earlier > > drm/i915: Move load time IRQ SW init earlier > > drm/i915: Move load time display/audio callback init earlier > > drm/i915: Move load time clock gating callback init earlier > > drm/i915: Move load time runtime device info init earlier > > drm/i915: Move load time runtime PM get later > > Lgtm. > -Chris > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
