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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 09 Mar 2016, Chris Wilson <chris@chris-wilson.co.uk> 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()

Agreed.

>
>> 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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx