All of lore.kernel.org
 help / color / mirror / Atom feed
From: Imre Deak <imre.deak@intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org, Sagar Kamble <sagar.a.kamble@intel.com>
Subject: Re: [PATCH] drm/i915: fix suspend/resume for GENs w/o runtime PM support
Date: Tue, 26 Aug 2014 12:01:33 +0300	[thread overview]
Message-ID: <1409043693.2960.7.camel@intelbox> (raw)
In-Reply-To: <20140826074538.GB15520@phenom.ffwll.local>


[-- Attachment #1.1: Type: text/plain, Size: 1636 bytes --]

On Tue, 2014-08-26 at 09:45 +0200, Daniel Vetter wrote:
> On Tue, Aug 26, 2014 at 09:44:42AM +0200, Daniel Vetter wrote:
> > On Mon, Aug 18, 2014 at 01:20:06PM +0300, Imre Deak wrote:
> > > Before sharing common parts between the system and runtime s/r
> > > handlers we WARNed if the runtime s/r handlers were called on GENs that
> > > didn't support RPM. But this WARN is not correct if the same handler is
> > > called from the system s/r path, since that can happen on any platform.
> > > This also broke system s/r on old platforms.
> > > 
> > > The issue was introduced in
> > > 
> > > commit 016970beb05da6285c2f3ed2bee1c676cb75972e
> > > Author: Sagar Kamble <sagar.a.kamble@intel.com>
> > > Date:   Wed Aug 13 23:07:06 2014 +0530
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82751
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > 
> > Adding boolean arguments to control warnings always feels a bit too much
> > like just shutting up the warnings. Can't we instead wrap the relevant
> > calls into HAS_RUNTIME_PM checks?

I could instead remove the WARN from intel_suspend_complete/resume, and
do an early return from intel_runtime_suspend/resume for
!HAS_RUNTIME_PM(). Atm we only WARN there.

> > Imo that would also lead to clearer code
> > by making the intention clear - with this you essentially have to git
> > blame to figure out why we sometimes disable the warning.
> 
> Also the patch subject is a bit misleading - we only shut up a wrong
> warning, it's not a code fix.

We return -ENODEV for old GENs which breaks system suspend for them.

--Imre

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2014-08-26  9:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-18 10:20 [PATCH] drm/i915: fix suspend/resume for GENs w/o runtime PM support Imre Deak
2014-08-19  2:52 ` Sagar Arun Kamble
2014-08-26  7:44 ` Daniel Vetter
2014-08-26  7:45   ` Daniel Vetter
2014-08-26  9:01     ` Imre Deak [this message]
2014-08-26 10:26 ` [PATCH v2] " Imre Deak
2014-08-26 10:53   ` Daniel Vetter

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=1409043693.2960.7.camel@intelbox \
    --to=imre.deak@intel.com \
    --cc=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=sagar.a.kamble@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.