Re: [PATCH 1/3] drm/i915: rearrange vlv dp enable and pre_enable callbacks

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

On Tue, 30 Jul 2013, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Jul 30, 2013 at 11:08:28AM +0300, Jani Nikula wrote:
>> Currently ->pre_enable and ->enable are called back to back. Rearrange
>> the DP callbacks to make it possible to move ->enable call later.
>
> A one sentence explanation why we want ->enable later would be perfect.
> If you can concisely explain why the code was originally written as is,
> and why we want to do it differently in the near future, that would be
> ideal.

Good point. How about,

for patches 1&2:

"""
VLV wants encoder enabling before the pipe is up. This is currently
achieved through calling the ->enable callback early, right after the
->pre_enable callback, in valleyview_crtc_enable(). This loses both the
distinction between ->pre_enable and ->enable on VLV and the possibility
to use a hook at the end of the modeset sequence.

Rearrange the DP callbacks to make it possible to move ->enable call
later. Basically do everything in ->pre_enable on VLV, and make ->enable
a NOP.

There should be no functional changes.
"""

for patch 3:

"""
VLV wants encoder enabling before the pipe is up. With the previously
rearranged VLV DP and HDMI ->pre_enable and ->enable callbacks in place,
this no longer depends on the early ->enable hook call. Move the
->enable call at the end of the sequence, similar to the crtc enable on
other platforms. This will be needed e.g. for moving the eDP backlight
enabling to the right place in the sequence, currently done too early on
VLV.

There should be no functional changes.
"""

I'll send the revised patches once we agree on the wording.

Thanks,
Jani.