[Intel-gfx] [PATCH 6/6] drm/i915: implement HSW display sequences for package C8+

[Chris Wilson]

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