Re: [PATCH 0/7] drm/i915: Move some load time init steps earlier

[Imre Deak <imre.deak@intel.com>]

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx