From: Daniel Vetter <daniel@ffwll.ch>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: intel-gfx@lists.freedesktop.org, "Mcallister,
Jeffrey" <jeffrey.mcallister@intel.com>
Subject: Re: [PATCH 6/9] drm/i915: PCH_NOP suspend/resume
Date: Sun, 17 Mar 2013 22:28:55 +0100 [thread overview]
Message-ID: <20130317212854.GB9021@phenom.ffwll.local> (raw)
In-Reply-To: <1363198868-21787-7-git-send-email-ben@bwidawsk.net>
On Wed, Mar 13, 2013 at 11:21:05AM -0700, Ben Widawsky wrote:
> More registers we can't write.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_suspend.c | 57 ++++++++++++++++++++++++++-----------
> 1 file changed, 41 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index c1e02b0..dd5766a 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -333,11 +333,19 @@ int i915_save_state(struct drm_device *dev)
>
> mutex_lock(&dev->struct_mutex);
>
> - i915_save_display(dev);
> + if (!HAS_PCH_NOP(dev))
> + i915_save_display(dev);
This here looks a bit funny - imo it's better to move this check to the
only two places where we still touch registers in the kms case: lvds & pp
restore.
>
> if (!drm_core_check_feature(dev, DRIVER_MODESET)) {
> /* Interrupt state */
> - if (HAS_PCH_SPLIT(dev)) {
> + if (HAS_PCH_NOP(dev)) {
> + dev_priv->regfile.saveDEIER = I915_READ(DEIER);
> + dev_priv->regfile.saveDEIMR = I915_READ(DEIMR);
> + dev_priv->regfile.saveGTIER = I915_READ(GTIER);
> + dev_priv->regfile.saveGTIMR = I915_READ(GTIMR);
> + dev_priv->regfile.saveMCHBAR_RENDER_STANDBY =
> + I915_READ(RSTDBYCTL);
> + } else if (HAS_PCH_SPLIT(dev)) {
> dev_priv->regfile.saveDEIER = I915_READ(DEIER);
> dev_priv->regfile.saveDEIMR = I915_READ(DEIMR);
> dev_priv->regfile.saveGTIER = I915_READ(GTIER);
> @@ -361,13 +369,18 @@ int i915_save_state(struct drm_device *dev)
> /* Memory Arbitration state */
> dev_priv->regfile.saveMI_ARB_STATE = I915_READ(MI_ARB_STATE);
>
> - /* Scratch space */
> - for (i = 0; i < 16; i++) {
> - dev_priv->regfile.saveSWF0[i] = I915_READ(SWF00 + (i << 2));
> - dev_priv->regfile.saveSWF1[i] = I915_READ(SWF10 + (i << 2));
> + if (!HAS_PCH_NOP(dev)) {
> + /* Scratch space */
> + for (i = 0; i < 16; i++) {
> + dev_priv->regfile.saveSWF0[i] =
> + I915_READ(SWF00 + (i << 2));
> + dev_priv->regfile.saveSWF1[i] =
> + I915_READ(SWF10 + (i << 2));
> + }
> + for (i = 0; i < 3; i++)
> + dev_priv->regfile.saveSWF2[i] =
> + I915_READ(SWF30 + (i << 2));
Blergh, I hate those registers, and I have no idea where we actually need
to restore them for kms. Can you please also add a big "XXX: Do we really
need this for kms?" comment in the scratch space block?
> }
> - for (i = 0; i < 3; i++)
> - dev_priv->regfile.saveSWF2[i] = I915_READ(SWF30 + (i << 2));
>
> mutex_unlock(&dev->struct_mutex);
>
> @@ -383,11 +396,17 @@ int i915_restore_state(struct drm_device *dev)
>
> mutex_lock(&dev->struct_mutex);
>
> - i915_restore_display(dev);
> + if (!HAS_PCH_NOP(dev))
> + i915_restore_display(dev);
>
> if (!drm_core_check_feature(dev, DRIVER_MODESET)) {
> /* Interrupt state */
> - if (HAS_PCH_SPLIT(dev)) {
> + if (HAS_PCH_NOP(dev)) {
> + I915_WRITE(DEIER, dev_priv->regfile.saveDEIER);
> + I915_WRITE(DEIMR, dev_priv->regfile.saveDEIMR);
> + I915_WRITE(GTIER, dev_priv->regfile.saveGTIER);
> + I915_WRITE(GTIMR, dev_priv->regfile.saveGTIMR);
> + } else if (HAS_PCH_SPLIT(dev)) {
> I915_WRITE(DEIER, dev_priv->regfile.saveDEIER);
> I915_WRITE(DEIMR, dev_priv->regfile.saveDEIMR);
> I915_WRITE(GTIER, dev_priv->regfile.saveGTIER);
> @@ -407,16 +426,22 @@ int i915_restore_state(struct drm_device *dev)
> /* Memory arbitration state */
> I915_WRITE(MI_ARB_STATE, dev_priv->regfile.saveMI_ARB_STATE | 0xffff0000);
>
> - for (i = 0; i < 16; i++) {
> - I915_WRITE(SWF00 + (i << 2), dev_priv->regfile.saveSWF0[i]);
> - I915_WRITE(SWF10 + (i << 2), dev_priv->regfile.saveSWF1[i]);
> + if (!HAS_PCH_NOP(dev)) {
> + for (i = 0; i < 16; i++) {
> + I915_WRITE(SWF00 + (i << 2),
> + dev_priv->regfile.saveSWF0[i]);
> + I915_WRITE(SWF10 + (i << 2),
> + dev_priv->regfile.saveSWF1[i]);
> + }
> + for (i = 0; i < 3; i++)
> + I915_WRITE(SWF30 + (i << 2),
> + dev_priv->regfile.saveSWF2[i]);
> }
> - for (i = 0; i < 3; i++)
> - I915_WRITE(SWF30 + (i << 2), dev_priv->regfile.saveSWF2[i]);
>
> mutex_unlock(&dev->struct_mutex);
>
> - intel_i2c_reset(dev);
> + if (!HAS_PCH_NOP(dev))
> + intel_i2c_reset(dev);
>
> return 0;
> }
> --
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
next prev parent reply other threads:[~2013-03-17 21:26 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-13 18:20 [PATCH 0/9] displayless PCH Ben Widawsky
2013-03-13 18:21 ` [PATCH 1/9] drm/i915: Move num_pipes to intel info Ben Widawsky
2013-03-13 18:31 ` Chris Wilson
2013-03-13 18:36 ` Ben Widawsky
2013-03-13 21:05 ` [PATCH 01/10] [v2] " Ben Widawsky
2013-03-19 19:25 ` [PATCH 1/9] " Jesse Barnes
2013-03-19 23:06 ` Daniel Vetter
2013-03-13 18:21 ` [PATCH 2/9] drm/i915: Support PCH no display Ben Widawsky
2013-03-17 21:13 ` Daniel Vetter
2013-03-13 18:21 ` [PATCH 3/9] drm/i915: PCH_NOP Ben Widawsky
2013-03-17 21:05 ` Daniel Vetter
2013-03-17 21:14 ` Daniel Vetter
2013-03-13 18:21 ` [PATCH 4/9] drm/i915: Don't touch South display when PCH_NOP Ben Widawsky
2013-03-14 11:44 ` Jani Nikula
2013-03-14 15:55 ` [PATCH 04/10] [v2] drm/i915: Don't touch South Display " Ben Widawsky
2013-03-17 21:21 ` Daniel Vetter
2013-03-18 23:05 ` Ben Widawsky
2013-03-13 18:21 ` [PATCH 5/9] drm/i915: Don't initialize watermark stuff with PCH_NOP Ben Widawsky
2013-03-13 18:47 ` Chris Wilson
2013-03-13 21:06 ` [PATCH 05/10] [v2] " Ben Widawsky
2013-03-17 21:02 ` Daniel Vetter
2013-03-19 0:45 ` Ben Widawsky
2013-03-13 18:21 ` [PATCH 6/9] drm/i915: PCH_NOP suspend/resume Ben Widawsky
2013-03-17 21:28 ` Daniel Vetter [this message]
2013-03-19 0:51 ` Ben Widawsky
2013-03-19 7:51 ` Daniel Vetter
2013-03-19 17:53 ` Ben Widawsky
2013-03-13 18:21 ` [PATCH 7/9] drm/i915: Don't wait for PCH on reset Ben Widawsky
2013-03-19 19:36 ` Jesse Barnes
2013-03-20 16:43 ` [PATCH] " Ben Widawsky
2013-03-20 16:58 ` Jesse Barnes
2013-03-13 18:21 ` [PATCH 8/9] drm/i915: Set PCH_NOP Ben Widawsky
2013-03-17 21:36 ` Daniel Vetter
2013-03-13 18:21 ` [PATCH 9/9] drm/i915: Add a pipeless ivybridge configuration Ben Widawsky
2013-03-13 18:52 ` Chris Wilson
2013-03-13 21:08 ` [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition Ben Widawsky
2013-03-13 21:08 ` [PATCH 10/10] [v2] drm/i915: Add a pipeless ivybridge configuration Ben Widawsky
2013-03-15 4:05 ` [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition Ben Widawsky
2013-03-13 18:58 ` [PATCH 0/9] displayless PCH Chris Wilson
2013-03-13 19:05 ` Ben Widawsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130317212854.GB9021@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=ben@bwidawsk.net \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jeffrey.mcallister@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.