Re: [PATCH 3/8] drm/i915: save/restore DSPARB only for chip before gen4

Jesse Barnes
2011-03-21T17:39:57Z

On Mon, 21 Mar 2011 17:27:14 +0800
Zhenyu Wang <zhenyuw@linux.intel.com> wrote:

> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_suspend.c | 12 ++++--------
> 1 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index 422981b..7f00f8b 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -603,7 +603,8 @@ void i915_save_display(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> /* Display arbitration control */
> - dev_priv->saveDSPARB = I915_READ(DSPARB);
> + if (dev_priv->info->gen < 4)
> + dev_priv->saveDSPARB = I915_READ(DSPARB);
>
> /* This is only meaningful in non-KMS mode */
> /* Don't save them in KMS mode */
> @@ -695,7 +696,8 @@ void i915_restore_display(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> /* Display arbitration */
> - I915_WRITE(DSPARB, dev_priv->saveDSPARB);
> + if (dev_priv->info->gen < 4)
> + I915_WRITE(DSPARB, dev_priv->saveDSPARB);

Was it Cantiga that started doing automatic FIFO sizing for us?
Checking the docs... Yeah, only Bearlake-C (G33?) and Cantiga do this
automatically. So a blanket gen4 check probably isn't right.

>
> /* Display port ratios (must be done before clock is set) */
> if (SUPPORTS_INTEGRATED_DP(dev)) {
> @@ -795,9 +797,6 @@ int i915_save_state(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> int i;
>
> - /* Hardware status page */
> - dev_priv->saveHWS = I915_READ(HWS_PGA);
> -
> i915_save_display(dev);
>
> /* Interrupt state */
> @@ -842,9 +841,6 @@ int i915_restore_state(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> int i;
>
> - /* Hardware status page */
> - I915_WRITE(HWS_PGA, dev_priv->saveHWS);
> -
> i915_restore_display(dev);
>
> /* Interrupt state */

These hunks are unrelated to DSPARB; assuming we actually want to do
this it should be a separate patch. Based on earlier reports, it
sounds like we're not properly idling across suspend/resume, in that
we're trying to execute commands at resume before setting up the HWS
again.

--
Jesse Barnes, Intel Open Source Technology Center