On Wed, Jun 05, 2013 at 02:21:56PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>
> This patch implements "Display Sequences for Package C8", from the
> "Display Mode Set Sequence" section from the Haswell documentation.
>
> Notice that even if we allow PC8+, there's no guarantee that we will
> actually enter PC8+, since the other devices also need to allow it.
> Also notice that we need i915.disable_power_well=1 in order to test
> this feature: we don't allow PC8+ if the power well is still enabled.
>
> v2: - Rebase
> - Implement many review comments form Daniel Vetter
> - Call intel_prepare_ddi and i915_gem_init_swizzling when
> returning from C8+ so we can actually see the screen contents
Really don't like the colour you've chosen in places, and there seem to
be a lot of placeholder code.
Highlights:
- intel_aux_display_shutdown_forbid / allow ->
intel_aux_display_get / put
Not convinced by "aux" there either. Perhaps,
intel_display_wakelock_get() instead. Or we could go with
intel_display_forcewake_get(). Hmm, forcewake is better as we already
have that concept in our code (and wakelock may end up being confused
with the system-wide wakelocks.)
- move your c8 state into a struct:
struct i915_package_c8 {
struct mutex mutex;
union {
struct hsw_package_c8_registers hsw_regfile;
};
bool enabled; /* was allowing_package_c8 */
int forcewake; /* was c8_forbid_refcnt */
};
- The interrupt register handling is messy. We already track the values
of the ones that we change, the others should be static. Looks
mostly correct (barring a race with an interrupt!).
- Hooking into modeset_global_resources seems strange and never
explained.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
