All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: Imre Deak <imre.deak@intel.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 03/10] drm/i915: Add support for asynchronous display power disabling
Date: Fri, 03 May 2019 13:16:04 +0100	[thread overview]
Message-ID: <155688576496.3139.256837794873689597@skylake-alporthouse-com> (raw)
In-Reply-To: <20190502232648.4450-4-imre.deak@intel.com>

Quoting Imre Deak (2019-05-03 00:26:41)
> By disabling a power domain asynchronously we can restrict holding a
> reference on that power domain to the actual code sequence that
> requires the power to be on for the HW access it's doing, by also
> avoiding unneeded on-off-on togglings of the power domain (since the
> disabling happens with a delay).

That applies to no-delay. Are we not starting from a state where the
code acquires the powerwell for its immediate use, or it takes and holds
it for protracted durations even when the powerwell is not being used?

> One benefit is potential power saving if the delay is chosen properly.

Which is suggesting that some delay is better power saving than
no-delay? It's believable that powering on cost mores more energy than
normal. But do please fill in a few details on how the delay should be
chosen.

> In the case of the AUX power domain holding the reference on the domain
> for the minimal amount of time at defined spots is also a requirement:

Do you mean that there is a minimum duration for which the power well
must be asserted once acquired before being released?

> on ICL we need a stricter control of when either kind of AUX power
> domain (TBT-alt or DP-alt) can be enabled and the locking we need for
> that becomes problematic (due to dependencies on other locks) if we
> allow the reference to be held for arbitrarily long periods/places in
> the code.
> 
> I chose the disabling delay to be 100msec for now to avoid the unneeded
> toggling (and so not to introduce dmesg spamming) in the DP MST sideband
> signaling code. We could optimize this delay later.

Or removing the spam. Are we at a point where the error detection is
good enough to pin-point the lack of a particular powerwell wakeref?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-05-03 12:16 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-02 23:26 [PATCH 00/10] drm/i915: Add support for asynchronous display power disabling Imre Deak
2019-05-02 23:26 ` [PATCH 01/10] drm/i915: Add support for tracking wakerefs w/o power-on guarantee Imre Deak
2019-05-07 14:03   ` Chris Wilson
2019-05-02 23:26 ` [PATCH 02/10] drm/i915: Verify power domains state during suspend in all cases Imre Deak
2019-05-02 23:26 ` [PATCH 03/10] drm/i915: Add support for asynchronous display power disabling Imre Deak
2019-05-03 12:16   ` Chris Wilson [this message]
2019-05-03 13:42     ` Imre Deak
2019-05-06 11:12   ` [PATCH v2 " Imre Deak
2019-05-02 23:26 ` [PATCH 04/10] drm/i915: Disable power asynchronously during DP AUX transfers Imre Deak
2019-05-02 23:26 ` [PATCH 05/10] drm/i915: WARN for eDP encoders in intel_dp_detect_dpcd() Imre Deak
2019-05-02 23:26 ` [PATCH 06/10] drm/i915: Remove the unneeded AUX power ref from intel_dp_detect() Imre Deak
2019-05-02 23:26 ` [PATCH 07/10] drm/i915: Remove the unneeded AUX power ref from intel_dp_hpd_pulse() Imre Deak
2019-05-02 23:26 ` [PATCH 08/10] drm/i915: Replace use of PLLS power domain with DISPLAY_CORE domain Imre Deak
2019-05-02 23:26 ` [PATCH 09/10] drm/i915: Avoid taking the PPS lock for non-eDP/VLV/CHV Imre Deak
2019-05-06 12:35   ` Ville Syrjälä
2019-05-06 13:12     ` Ville Syrjälä
2019-05-06 13:15       ` Imre Deak
2019-05-02 23:26 ` [PATCH 10/10] drm/i915: Assert that TypeC ports are not used for eDP Imre Deak
2019-05-03  0:34 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add support for asynchronous display power disabling Patchwork
2019-05-03  0:38 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-05-03  1:12 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-03  7:50 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-05-03 10:07   ` Imre Deak
2019-05-03 12:37     ` Chris Wilson
2019-05-03 13:52       ` Imre Deak
2019-05-03 14:21         ` Imre Deak
2019-05-06  9:44         ` Imre Deak
2019-05-06 11:29 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add support for asynchronous display power disabling (rev2) Patchwork
2019-05-06 11:49 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-06 12:57 ` ✓ Fi.CI.IGT: " Patchwork

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=155688576496.3139.256837794873689597@skylake-alporthouse-com \
    --to=chris@chris-wilson.co.uk \
    --cc=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /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.