Re: [Intel-gfx] [PATCH v1] drm/i915: Skip modeset for cdclk changes if possible
+Susanta
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
Kumar, Abhay
Sent: Tuesday, August 28, 2018 5:55 PM
To: Ville Syrjälä
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v1] drm/i915: Skip modeset for cdclk changes if
possible
On 8/28/2018 5:39 AM, Ville Syrjälä wrote:
On Mon, Aug 27, 2018 at 11:50:32AM -0700, Abhay Kumar wrote:
From: Ville Syrjälä
<mailto:ville.syrj...@linux.intel.com>
If we have only a single active pipe and the cdclk change only requires
the cd2x divider to be updated bxt+ can do the update with forcing a full
modeset on the pipe. Try to hook that up.
Signed-off-by: Ville Syrjälä
<mailto:ville.syrj...@linux.intel.com>
Signed-off-by: Abhay Kumar <mailto:abhay.ku...@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/i915_reg.h | 3 +-
drivers/gpu/drm/i915/intel_cdclk.c | 105 +--
drivers/gpu/drm/i915/intel_display.c | 20 ++-
drivers/gpu/drm/i915/intel_drv.h | 9 ++-
5 files changed, 105 insertions(+), 35 deletions(-)
@@ -12252,12 +12253,24 @@ static int intel_modeset_checks(struct
drm_atomic_state *state)
return ret;
}
+
/* All pipes must be switched off while we change the cdclk. */
- if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
- &intel_state->cdclk.actual)) {
+ if (is_power_of_2(intel_state->active_crtcs) &&
+ intel_cdclk_needs_cd2x_update(dev_priv,
+ &dev_priv->cdclk.actual,
+ &intel_state->cdclk.actual)) {
+ ret = intel_lock_all_pipes(state);
+ if (ret < 0)
+ return ret;
+
+ intel_state->cdclk.pipe = ilog2(intel_state->active_crtcs);
BTW on further reflection this probably isn't quite sufficient. Let's
say we have a commit with allow_modeset=true, but we aren't actually
required to do a modeset based on any of the state changes. If we still
have to change cdclk we should actually be doing the cd2x update
atomically with the plane updates, or we should do it before or after
the plane updates depending on whether the cdclk freq is going up or
down.
Doing the update atomically with the plane updates might be nicer in the
end, but for that we would likely need to split the .set_cdclk() hooks
into three parts (pre+commit+post). Whereas just doing the update before
or after the plane updates as needed would probably be a little simpler.
Yeah. That might also get rid of cdclk mismatch warning during multiple suspend
resume cycle.
[ 280.600259] cdclk state doesn't match!
[ 280.600270] calling 1-8+ @ 3110, parent: usb1, cb: usb_dev_resume
[ 280.600276] WARNING: CPU: 3 PID: 5224 at /mnt/host/source/src/third_party/ker
nel/v4.14/drivers/gpu/drm/i915/intel_cdclk.c:1867 intel_set_cdclk+0xaa/0xdb
[ 280.600277] Modules linked in: cmac rfcomm uinput snd_soc_sst_bxt_da7219_max9
8357a snd_soc_hdac_hdmi snd_soc_dmic lzo lzo_compress snd_soc_skl snd_soc_skl_ip
c snd_soc_sst_ipc snd_soc_sst_dsp snd_soc_acpi snd_hda_ext_core snd_hda_core
[ 280.600307] calling phy0+ @ 3102, parent: :00:0c.0, cb: wiphy_resume [cf
g80211]
[ 280.600308] zram snd_soc_max98357a acpi_als snd_soc_da7219 bridge stp llc ip
t_MASQUERADE nf_nat_masquerade_ipv4 xt_mark fuse snd_seq_dummy snd_seq snd_seq_d
evice btusb btrtl btbcm iio_trig_sysfs btintel uvcvideo bluetooth videobuf2_vmal
loc videobuf2_memops videobuf2_v4l2 ecdh_generic videobuf2_core cros_ec_sensors
cros_ec_sensors_ring cros_ec_sensors_core industrialio_triggered_buffer kfifo_bu
[ 280.600346] RDX: b8258dd0 RSI: 0002 RDI: b8258db0
[ 280.600347] RBP: bb9546b73aa0 R08: R09:
[ 280.600348] R10: R11: b86d8518 R12: a2207579
[ 280.600349] R13: a22075793d24 R14: R15: a2202eec9800
000
[ 280.600352] CS: 0010 DS: ES: CR0: 80050033
[ 280.600353] CR2: 7f43c04d0e50 CR3: 000224112000 CR4: 003406e0
[ 280.600354] Call Trace:
[ 280.600361] intel_atomic_commit_tail+0x20a/0xacb
[ 280.600363] ? intel_atomic_commit_ready+0x44/0x4c
[ 280.600365] intel_atomic_commit+0x227/0x238
[ 280.600368] glk_force_audio_cdclk+0x9f/0x119
[ 280.600370] i915_audio_component_get_power+0x3e/0x4d
[ 280.600376] snd_hdac_display_power+0x53/0x97 [snd_hda_core]
[ 280.600379] calling 1-9+ @ 3086, parent: usb1, cb: usb_dev_resume
[ 280.600384] skl_resume+0x3a/0x17a [snd_soc_skl]
[ 280.600387] ? pci_pm_suspend_noirq+0x1e9/0x1e9
[ 280.600391] dpm_run_callback+0x59/0xbf
[ 280.600394] device_resume+0x192/0
Re: [Intel-gfx] [PATCH v1] drm/i915: Skip modeset for cdclk changes if possible
On 8/28/2018 5:39 AM, Ville Syrjälä wrote:
On Mon, Aug 27, 2018 at 11:50:32AM -0700, Abhay Kumar wrote:
From: Ville Syrjälä
If we have only a single active pipe and the cdclk change only requires
the cd2x divider to be updated bxt+ can do the update with forcing a full
modeset on the pipe. Try to hook that up.
Signed-off-by: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/i915_reg.h | 3 +-
drivers/gpu/drm/i915/intel_cdclk.c | 105 +--
drivers/gpu/drm/i915/intel_display.c | 20 ++-
drivers/gpu/drm/i915/intel_drv.h | 9 ++-
5 files changed, 105 insertions(+), 35 deletions(-)
@@ -12252,12 +12253,24 @@ static int intel_modeset_checks(struct
drm_atomic_state *state)
return ret;
}
+
/* All pipes must be switched off while we change the cdclk. */
- if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
- &intel_state->cdclk.actual)) {
+ if (is_power_of_2(intel_state->active_crtcs) &&
+ intel_cdclk_needs_cd2x_update(dev_priv,
+ &dev_priv->cdclk.actual,
+ &intel_state->cdclk.actual)) {
+ ret = intel_lock_all_pipes(state);
+ if (ret < 0)
+ return ret;
+
+ intel_state->cdclk.pipe =
ilog2(intel_state->active_crtcs);
BTW on further reflection this probably isn't quite sufficient. Let's
say we have a commit with allow_modeset=true, but we aren't actually
required to do a modeset based on any of the state changes. If we still
have to change cdclk we should actually be doing the cd2x update
atomically with the plane updates, or we should do it before or after
the plane updates depending on whether the cdclk freq is going up or
down.
Doing the update atomically with the plane updates might be nicer in the
end, but for that we would likely need to split the .set_cdclk() hooks
into three parts (pre+commit+post). Whereas just doing the update before
or after the plane updates as needed would probably be a little simpler.
Yeah. That might also get rid of cdclk mismatch warning during multiple
suspend resume cycle.
[280.600259] cdclk state doesn't match!
[280.600270] calling1-8+ @ 3110, parent: usb1, cb: usb_dev_resume
[280.600276] WARNING: CPU: 3 PID: 5224 at
/mnt/host/source/src/third_party/ker
nel/v4.14/drivers/gpu/drm/i915/intel_cdclk.c:1867 intel_set_cdclk+0xaa/0xdb
[280.600277] Modules linked in: cmac rfcomm uinput
snd_soc_sst_bxt_da7219_max9
8357a snd_soc_hdac_hdmi snd_soc_dmic lzo lzo_compress snd_soc_skl
snd_soc_skl_ip
c snd_soc_sst_ipc snd_soc_sst_dsp snd_soc_acpi snd_hda_ext_core snd_hda_core
[280.600307] callingphy0+ @ 3102, parent: :00:0c.0, cb: wiphy_resume [cf
g80211]
[280.600308]zram snd_soc_max98357a acpi_als snd_soc_da7219 bridge stp llc ip
t_MASQUERADE nf_nat_masquerade_ipv4 xt_mark fuse snd_seq_dummy snd_seq
snd_seq_d
evice btusb btrtl btbcm iio_trig_sysfs btintel uvcvideo bluetooth
videobuf2_vmal
loc videobuf2_memops videobuf2_v4l2 ecdh_generic videobuf2_core
cros_ec_sensors
cros_ec_sensors_ring cros_ec_sensors_core industrialio_triggered_buffer
kfifo_bu
[280.600346] RDX: b8258dd0 RSI: 0002 RDI:
b8258db0
[280.600347] RBP: bb9546b73aa0 R08: R09:
[280.600348] R10: R11: b86d8518 R12:
a2207579
[280.600349] R13: a22075793d24 R14: R15:
a2202eec9800
000
[280.600352] CS:0010 DS: ES: CR0: 80050033
[280.600353] CR2: 7f43c04d0e50 CR3: 000224112000 CR4:
003406e0
[280.600354] Call Trace:
[280.600361]intel_atomic_commit_tail+0x20a/0xacb
[280.600363]? intel_atomic_commit_ready+0x44/0x4c
[280.600365]intel_atomic_commit+0x227/0x238
[280.600368]glk_force_audio_cdclk+0x9f/0x119
[280.600370]i915_audio_component_get_power+0x3e/0x4d
[280.600376]snd_hdac_display_power+0x53/0x97 [snd_hda_core]
[280.600379] calling1-9+ @ 3086, parent: usb1, cb: usb_dev_resume
[280.600384]skl_resume+0x3a/0x17a [snd_soc_skl]
[280.600387]? pci_pm_suspend_noirq+0x1e9/0x1e9
[280.600391]dpm_run_callback+0x59/0xbf
[280.600394]device_resume+0x192/0x1d4
[280.600396]dpm_resume+0x145/0x1da
[280.600398]dpm_resume_end+0x11/0x1a
[280.600403]suspend_devices_and_enter+0x354/0x5c2
[280.600407]? remove_wait_queue+0x51/0x51
[280.600409]pm_suspend+0x29c/0x2e2
[280.600411]state_store+0xa2/0xcb
[280.600415]kernfs_fop_write+0x103/0x14a
[280.600420]__vfs_write+0x37/0xd0
[280.600424]? inode_security+0x19/0x20
[280.600426]? selinux_file_permission+0x78/0xad
[280.600428]vfs_write+0xb9/0xfd
[280.600430]SyS_write+0x5f/0xa3
[280.600434]do_syscall_64+0x64/0x
Re: [Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
Hi Ville,
Can we please get this merged to DINQ?
Regards,
Abhay
-Original Message-
From: Du, Wenkai
Sent: Thursday, June 21, 2018 1:16 PM
To: Kumar, Abhay ; intel-gfx@lists.freedesktop.org;
Syrjala, Ville
Cc: Nikula, Jani
Subject: Re: [Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on glk/cnl
when audio power is enabled
On 6/21/2018 11:43 AM, Kumar, Abhay wrote:
> + Wenkai
>
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On
> Behalf Of Abhay Kumar
> Sent: Tuesday, June 19, 2018 3:01 PM
> To: intel-gfx@lists.freedesktop.org; Syrjala, Ville
>
> Cc: Nikula, Jani
> Subject: [Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on
> glk/cnl when audio power is enabled
>
> From: Ville Syrjälä
>
> CDCLK has to be at least twice the BLCK regardless of audio. Audio driver has
> to probe using this hook and increase the clock even in absence of any
> display.
>
> v2: Use atomic refcount for get_power, put_power so that we can
> call each once(Abhay).
> v3: Reset power well 2 to avoid any transaction on iDisp link
> during cdclk change(Abhay).
> v4: Remove Power well 2 reset workaround(Ville).
> v5: Remove unwanted Power well 2 register defined in v4(Abhay).
>
> Signed-off-by: Ville Syrjälä
> Signed-off-by: Abhay Kumar
Tested-by: Wenkai Du
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 ++
> drivers/gpu/drm/i915/intel_audio.c | 67
> +---
> drivers/gpu/drm/i915/intel_cdclk.c | 29 +---
> drivers/gpu/drm/i915/intel_display.c | 7 +++-
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> 5 files changed, 83 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 6104d7115054..a4a386a5db69
> 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1702,6 +1702,7 @@ struct drm_i915_private {
> unsigned int hpll_freq;
> unsigned int fdi_pll_freq;
> unsigned int czclk_freq;
> + u32 get_put_refcount;
>
> struct {
> /*
> @@ -1719,6 +1720,8 @@ struct drm_i915_private {
> struct intel_cdclk_state actual;
> /* The current hardware cdclk state */
> struct intel_cdclk_state hw;
> +
> + int force_min_cdclk;
> } cdclk;
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 3ea566f99450..ca8f04c7cbb3 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -618,7 +618,6 @@ void intel_audio_codec_enable(struct intel_encoder
> *encoder,
>
> if (!connector->eld[0])
> return;
> -
> DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
>connector->base.id,
>connector->name,
> @@ -713,14 +712,74 @@ void intel_init_audio_hooks(struct drm_i915_private
> *dev_priv)
> }
> }
>
> +static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
> + bool enable)
> +{
> + struct drm_modeset_acquire_ctx ctx;
> + struct drm_atomic_state *state;
> + int ret;
> +
> + drm_modeset_acquire_init(&ctx, 0);
> + state = drm_atomic_state_alloc(&dev_priv->drm);
> + if (WARN_ON(!state))
> + return;
> +
> + state->acquire_ctx = &ctx;
> +
> +retry:
> + to_intel_atomic_state(state)->modeset = true;
> + to_intel_atomic_state(state)->cdclk.force_min_cdclk =
> + enable ? 2 * 96000 : 0;
> +
> + /*
> + * Protects dev_priv->cdclk.force_min_cdclk
> + * Need to lock this here in case we have no active pipes
> + * and thus wouldn't lock it during the commit otherwise.
> + */
> + ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
> &ctx);
> + if (!ret)
> + ret = drm_atomic_commit(state);
> +
> + if (ret == -EDEADLK) {
> + drm_atomic_state_clear(state);
> + drm_modeset_backoff(&ctx);
> + goto retry;
> + }
> +
> + WARN_ON(ret);
> +
> + drm_atomic_state_put(state);
> +
> + drm_modeset_drop_locks(&ctx);
> + drm_modeset_acquire_fini(&ctx);
> +}
> +
> static void i915_audio_component_get_power(struct device *kdev) {
> - intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
> + struct drm_i915_private *dev_priv = kdev_to_i915(kde
Re: [Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
+ Wenkai
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
Abhay Kumar
Sent: Tuesday, June 19, 2018 3:01 PM
To: intel-gfx@lists.freedesktop.org; Syrjala, Ville
Cc: Nikula, Jani
Subject: [Intel-gfx] [PATCH v5] drm/i915: Force 2*96 MHz cdclk on glk/cnl when
audio power is enabled
From: Ville Syrjälä
CDCLK has to be at least twice the BLCK regardless of audio. Audio driver has
to probe using this hook and increase the clock even in absence of any display.
v2: Use atomic refcount for get_power, put_power so that we can
call each once(Abhay).
v3: Reset power well 2 to avoid any transaction on iDisp link
during cdclk change(Abhay).
v4: Remove Power well 2 reset workaround(Ville).
v5: Remove unwanted Power well 2 register defined in v4(Abhay).
Signed-off-by: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/intel_audio.c | 67 +---
drivers/gpu/drm/i915/intel_cdclk.c | 29 +---
drivers/gpu/drm/i915/intel_display.c | 7 +++-
drivers/gpu/drm/i915/intel_drv.h | 2 ++
5 files changed, 83 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6104d7115054..a4a386a5db69 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1702,6 +1702,7 @@ struct drm_i915_private {
unsigned int hpll_freq;
unsigned int fdi_pll_freq;
unsigned int czclk_freq;
+ u32 get_put_refcount;
struct {
/*
@@ -1719,6 +1720,8 @@ struct drm_i915_private {
struct intel_cdclk_state actual;
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+
+ int force_min_cdclk;
} cdclk;
/**
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index 3ea566f99450..ca8f04c7cbb3 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -618,7 +618,6 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
if (!connector->eld[0])
return;
-
DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
connector->base.id,
connector->name,
@@ -713,14 +712,74 @@ void intel_init_audio_hooks(struct drm_i915_private
*dev_priv)
}
}
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+ bool enable)
+{
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ int ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+ state = drm_atomic_state_alloc(&dev_priv->drm);
+ if (WARN_ON(!state))
+ return;
+
+ state->acquire_ctx = &ctx;
+
+retry:
+ to_intel_atomic_state(state)->modeset = true;
+ to_intel_atomic_state(state)->cdclk.force_min_cdclk =
+ enable ? 2 * 96000 : 0;
+
+ /*
+* Protects dev_priv->cdclk.force_min_cdclk
+* Need to lock this here in case we have no active pipes
+* and thus wouldn't lock it during the commit otherwise.
+*/
+ ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
&ctx);
+ if (!ret)
+ ret = drm_atomic_commit(state);
+
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ drm_modeset_backoff(&ctx);
+ goto retry;
+ }
+
+ WARN_ON(ret);
+
+ drm_atomic_state_put(state);
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
static void i915_audio_component_get_power(struct device *kdev) {
- intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
+ struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+ dev_priv->get_put_refcount++;
+
+ /* Force cdclk to 2*BCLK during first time get power call */
+ if (dev_priv->get_put_refcount == 1)
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ glk_force_audio_cdclk(dev_priv, true);
+
+ intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
}
static void i915_audio_component_put_power(struct device *kdev) {
- intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
+ struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+ dev_priv->get_put_refcount--;
+
+ /* Force required cdclk during last time put power call */
+ if (dev_priv->get_put_refcount == 0)
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ glk_force_audio_cdclk(dev_priv, false);
+
+ intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
}
static void i915_audio_component_codec_wake_override(struct device *kdev, @@
-959,7 +1018,7 @@ void i915_audio_compone
Re: [Intel-gfx] [PATCH v4 0/4] Enable Dynamic cdclk and HDA together on GLK
Hi Ville, Looks like we have right fix from audio and we will not need powerwell 2 reset workaround when changing cdclk. we need only one patch in this series to get merged. drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled. Regards, Abhay On 6/13/2018 11:41 AM, Abhay Kumar wrote: Patches needed to change cdclk to 2*BCLK before accessing HDA Codec. Ville Syrjälä (4): drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled drm/i915: Introduce for_each_intel_dp() drm/i915: Lock gmbus/aux mutexes while changing cdclk drm/i915: Shut off PW2 when changing cdclk on glk drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/i915_reg.h | 4 ++ drivers/gpu/drm/i915/intel_audio.c | 67 ++-- drivers/gpu/drm/i915/intel_cdclk.c | 68 +++-- drivers/gpu/drm/i915/intel_display.c| 7 +++- drivers/gpu/drm/i915/intel_display.h| 4 ++ drivers/gpu/drm/i915/intel_dp.c | 38 -- drivers/gpu/drm/i915/intel_drv.h| 21 ++ drivers/gpu/drm/i915/intel_i2c.c| 1 - drivers/gpu/drm/i915/intel_runtime_pm.c | 34 + 11 files changed, 191 insertions(+), 57 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled (rev3)
On 6/12/2018 11:09 AM, Saarinen, Jani wrote: HI, -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Patchwork Sent: tiistai 12. kesäkuuta 2018 11.38 To: Kumar, Abhay Cc: intel-gfx@lists.freedesktop.org Subject: [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled (rev3) == Series Details == Series: drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled (rev3) URL : https://patchwork.freedesktop.org/series/42459/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4304_full -> Patchwork_9268_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_9268_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_9268_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_9268_full: === IGT changes === Warnings igt@gem_exec_schedule@deep-bsd2: shard-kbl: PASS -> SKIP igt@gem_exec_schedule@deep-vebox: shard-kbl: SKIP -> PASS == Known issues == Here are the changes found in Patchwork_9268_full that come from known issues: === IGT changes === Issues hit igt@kms_flip@2x-flip-vs-expired-vblank-interruptible: shard-hsw: PASS -> FAIL (fdo#102887) igt@kms_setmode@basic: shard-kbl: PASS -> FAIL (fdo#99912) Possible fixes igt@drv_selftest@live_gtt: shard-kbl: FAIL (fdo#105347) -> PASS igt@drv_suspend@shrink: shard-hsw: INCOMPLETE (fdo#103540) -> PASS igt@kms_rotation_crc@sprite-rotation-180: shard-hsw: FAIL (fdo#103925, fdo#104724) -> PASS fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540 fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925 fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724 fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (5 -> 4) == Missing(1): shard-glk There glk's are but some issues: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9268/shard-glk1/ https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9268/shard-glk2/run2.log etc... Worth to investigate... May be this is due to turning off and on Power well 2. Sent another patchset where we restart power well2 for any cdclk change only for glk. + Tomi too. == Build changes == * Linux: CI_DRM_4304 -> Patchwork_9268 CI_DRM_4304: 2313a1dc588ef63d9650ccbaaf576bc4b47dc255 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4515: a0f2d23b7d3d4226a0a7637a9240bfa86f08c1d3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_9268: 7e66d7400ee9f80e00633e6cfdecc354dda8e049 @ git://anongit.freedesktop.org/gfx-ci/linux piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm- tip/Patchwork_9268/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
On 6/12/2018 5:13 AM, Ville Syrjälä wrote:
On Tue, Jun 12, 2018 at 12:17:41AM -0700, Abhay Kumar wrote:
From: Ville Syrjälä
CDCLK has to be at least twice the BLCK regardless of audio. Audio
driver has to probe using this hook and increase the clock even in
absence of any display.
v2: Use atomic refcount for get_power, put_power so that we can
call each once(Abhay).
v3: Reset power well 2 to avoid any transaction on iDisp link
during cdclk change(Abhay).
Signed-off-by: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/i915_reg.h | 4 ++
drivers/gpu/drm/i915/intel_audio.c | 87 ++--
drivers/gpu/drm/i915/intel_cdclk.c | 29
drivers/gpu/drm/i915/intel_display.c | 7 ++-
drivers/gpu/drm/i915/intel_drv.h | 2 +
6 files changed, 107 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6104d7115054..a4a386a5db69 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1702,6 +1702,7 @@ struct drm_i915_private {
unsigned int hpll_freq;
unsigned int fdi_pll_freq;
unsigned int czclk_freq;
+ u32 get_put_refcount;
struct {
/*
@@ -1719,6 +1720,8 @@ struct drm_i915_private {
struct intel_cdclk_state actual;
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+
+ int force_min_cdclk;
} cdclk;
/**
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 987def26ce82..cef770184245 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8869,6 +8869,10 @@ enum skl_power_gate {
* SKL Clocks
*/
+/* Power well 2 */
+#define POWER_WELL_2 _MMIO(0x45404)
+#define POWER_WELL_2_REQUEST (1<<31)
+
/* CDCLK_CTL */
#define CDCLK_CTL _MMIO(0x46000)
#define CDCLK_FREQ_SEL_MASK (3 << 26)
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index 3ea566f99450..1f5a9af13ef0 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -618,7 +618,6 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
if (!connector->eld[0])
return;
-
DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
connector->base.id,
connector->name,
@@ -713,14 +712,94 @@ void intel_init_audio_hooks(struct drm_i915_private
*dev_priv)
}
}
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+ bool enable)
+{
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ int ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+ state = drm_atomic_state_alloc(&dev_priv->drm);
+ if (WARN_ON(!state))
+ return;
+
+ state->acquire_ctx = &ctx;
+
+retry:
+ to_intel_atomic_state(state)->modeset = true;
+ to_intel_atomic_state(state)->cdclk.force_min_cdclk =
+ enable ? 2 * 96000 : 0;
+
+ /*
+* Protects dev_priv->cdclk.force_min_cdclk
+* Need to lock this here in case we have no active pipes
+* and thus wouldn't lock it during the commit otherwise.
+*/
+ ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
&ctx);
+ if (!ret)
+ ret = drm_atomic_commit(state);
+
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ drm_modeset_backoff(&ctx);
+ goto retry;
+ }
+
+ WARN_ON(ret);
+
+ drm_atomic_state_put(state);
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
static void i915_audio_component_get_power(struct device *kdev)
{
- intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
+ struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+ u32 tmp;
+
+ dev_priv->get_put_refcount++;
+
+ /* Force cdclk to 2*BCLK during first time get power call */
+ if (dev_priv->get_put_refcount == 1) {
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
+
+ /*FIXME: Make sure there is no transaction
+* on iDisp link while changing cdclk
+*/
+
+ /* Turn off power well 2*/
+ tmp = I915_READ(POWER_WELL_2);
+ tmp = tmp & ~POWER_WELL_2_REQUEST;
+ I915_WRITE(POWER_WELL_2, tmp);
+ tmp = I915_READ(POWER_WELL_2);
+
+ /* Turn on power well 2*/
+ tmp = I915_READ(POWER_WELL_2);
+ tmp = tmp | POWE
Re: [Intel-gfx] [PATCH v2] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
On 5/11/2018 5:33 AM, Ville Syrjälä wrote:
On Wed, May 09, 2018 at 06:25:32PM -0700, Abhay Kumar wrote:
From: Ville Syrjälä
CDCLK has to be at least twice the BLCK regardless of audio. Audio
driver has to probe using this hook and increase the clock even in
absence of any display.
v2: Use atomic refcount for get_power, put_power so that we can
call each once(Abhay).
Signed-off-by: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/intel_audio.c | 66 +---
drivers/gpu/drm/i915/intel_cdclk.c | 29 +---
drivers/gpu/drm/i915/intel_display.c | 7 +++-
drivers/gpu/drm/i915/intel_drv.h | 2 ++
5 files changed, 82 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 24c5e4765afd..9c4ea767688a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1692,6 +1692,7 @@ struct drm_i915_private {
unsigned int hpll_freq;
unsigned int fdi_pll_freq;
unsigned int czclk_freq;
+ atomic_t get_put_refcount;
struct {
/*
@@ -1709,6 +1710,8 @@ struct drm_i915_private {
struct intel_cdclk_state actual;
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+
+ int force_min_cdclk;
} cdclk;
/**
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index 3ea566f99450..a1e2c4daae6e 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -618,7 +618,6 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
if (!connector->eld[0])
return;
-
DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
connector->base.id,
connector->name,
@@ -713,14 +712,73 @@ void intel_init_audio_hooks(struct drm_i915_private
*dev_priv)
}
}
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+ bool enable)
+{
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ int ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+ state = drm_atomic_state_alloc(&dev_priv->drm);
+ if (WARN_ON(!state))
+ return;
+
+ state->acquire_ctx = &ctx;
+
+retry:
+ to_intel_atomic_state(state)->modeset = true;
+ to_intel_atomic_state(state)->cdclk.force_min_cdclk =
+ enable ? 2 * 96000 : 0;
+
+ /*
+* Protects dev_priv->cdclk.force_min_cdclk
+* Need to lock this here in case we have no active pipes
+* and thus wouldn't lock it during the commit otherwise.
+*/
+ ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
&ctx);
+ if (!ret)
+ ret = drm_atomic_commit(state);
+
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ drm_modeset_backoff(&ctx);
+ goto retry;
+ }
+
+ WARN_ON(ret);
+
+ drm_atomic_state_put(state);
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
static void i915_audio_component_get_power(struct device *kdev)
{
- intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
+ struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+ intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
+ atomic_inc(&dev_priv->get_put_refcount);
+
+ /* Force cdclk to 2*BCLK during first time get power call */
+ if (atomic_read(&dev_priv->get_put_refcount) == 1)
If it needs to be atomic (ie. we have concurrent callers of get/put_power())
then you would also need to do the inc+check atomically. But that in itself
wouldn't help because only the first caller would do the cdclk change,
and the second call would just immediately proceed without waiting for the
cdclk to actually have been changed.
So the first question we should ask is whether we can even have
concurrent callers, or if there's some form of mutual exclusion
already on the caller side?
As per my understanding I don't think we have any concurrent callers as
these calls will be in sequence.
Do you prefer to use static instead?
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ glk_force_audio_cdclk(dev_priv, true);
}
static void i915_audio_component_put_power(struct device *kdev)
{
- intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
+ struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+ atomic_dec(&dev_priv->get_put_refcount);
+
+ /* Force required cdclk during last time put power call */
+ if (atomic_read(&dev_priv->get_put_refcount) == 0)
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev
Re: [Intel-gfx] [PATCH] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
On 5/2/2018 10:14 AM, Ville Syrjälä wrote:
On Wed, May 02, 2018 at 09:57:01AM -0700, Kumar, Abhay wrote:
On 5/2/2018 8:12 AM, Ville Syrjälä wrote:
On Sun, Apr 29, 2018 at 01:39:13PM -0700, Abhay Kumar wrote:
From: me
mostly
CDCLK has to be at least twice the BLCK regardless of audio. Audio
driver has to probe using this hook and increase the clock even in
absence of any display.
Signed-off-by: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/intel_audio.c | 46
drivers/gpu/drm/i915/intel_cdclk.c | 34 +++---
drivers/gpu/drm/i915/intel_display.c | 7 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
5 files changed, 63 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 193176bcddf5..34c31ef0761e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1708,6 +1708,8 @@ struct drm_i915_private {
struct intel_cdclk_state actual;
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+
+ int force_min_cdclk;
} cdclk;
/**
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index 3ea566f99450..f001fcf05d3a 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -594,6 +594,7 @@ static void ilk_audio_codec_enable(struct intel_encoder
*encoder,
I915_WRITE(aud_config, tmp);
}
+
/**
* intel_audio_codec_enable - Enable the audio codec for HD audio
* @encoder: encoder on which to enable audio
@@ -713,6 +714,48 @@ void intel_init_audio_hooks(struct drm_i915_private
*dev_priv)
}
}
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+ bool enable)
+{
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ int ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+ state = drm_atomic_state_alloc(&dev_priv->drm);
+ if (WARN_ON(!state))
+ return;
+
+ state->acquire_ctx = &ctx;
+
+retry:
+ to_intel_atomic_state(state)->modeset = true;
+ to_intel_atomic_state(state)->cdclk.force_min_cdclk =
+ enable ? 2 * 96000 : 0;
+
+ /*
+* Protects dev_priv->cdclk.force_min_cdclk
+* Need to lock this here in case we have no active pipes
+* and thus wouldn't lock it during the commit otherwise.
+*/
+ ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
&ctx);
+ if (!ret)
+ ret = drm_atomic_commit(state);
+
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ drm_modeset_backoff(&ctx);
+ goto retry;
+ }
+
+ WARN_ON(ret);
+
+ drm_atomic_state_put(state);
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
static void i915_audio_component_get_power(struct device *kdev)
{
intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
@@ -732,6 +775,9 @@ static void i915_audio_component_codec_wake_override(struct
device *kdev,
if (!IS_GEN9(dev_priv))
return;
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ glk_force_audio_cdclk(dev_priv, true);
+
Where did the put_power counterpart go?
with put_power counterpart the cdclk again goes back to low and then HDA
doesn't get detected. that's why i just kept the bump up.
Then fix hda to grab the power when it needs it?
I am afraid that this leads lot more changes as everywhere where there
is a probe we need to bump the clock.
Otherwise we permanently lock the cdclk to >=2*96 MHz. Ie. it would
be no different to what we have now, except a lot more complex.
It is different specially in resume/s0ix path. with the one we have
right now during resume time the cdclk is still 79.2 and
thus either the hda doesn't comeup or we have 4sec delay in coming up as
it loops and wait for hda verb command response.
This patch makes sure that all the time at probe we have right cdclk.
one more thing. we did probe power numbers and there was
not much significant delta.
i915_audio_component_get_power(kdev);
/*
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
b/drivers/gpu/drm/i915/intel_cdclk.c
index ebca83a44d9b..4086730018f9 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2141,24 +2141,6 @@ int intel_crtc_compute_min_cdclk(const struct
intel_crtc_state *crtc_state)
}
/*
-* According to BSpec, "The CD clock frequency must be at least twice
-* the frequency of the Azalia BCLK." and BC
Re: [Intel-gfx] [PATCH] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
On 5/2/2018 8:12 AM, Ville Syrjälä wrote:
On Sun, Apr 29, 2018 at 01:39:13PM -0700, Abhay Kumar wrote:
From: me
mostly
CDCLK has to be at least twice the BLCK regardless of audio. Audio
driver has to probe using this hook and increase the clock even in
absence of any display.
Signed-off-by: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/intel_audio.c | 46
drivers/gpu/drm/i915/intel_cdclk.c | 34 +++---
drivers/gpu/drm/i915/intel_display.c | 7 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
5 files changed, 63 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 193176bcddf5..34c31ef0761e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1708,6 +1708,8 @@ struct drm_i915_private {
struct intel_cdclk_state actual;
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+
+ int force_min_cdclk;
} cdclk;
/**
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index 3ea566f99450..f001fcf05d3a 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -594,6 +594,7 @@ static void ilk_audio_codec_enable(struct intel_encoder
*encoder,
I915_WRITE(aud_config, tmp);
}
+
/**
* intel_audio_codec_enable - Enable the audio codec for HD audio
* @encoder: encoder on which to enable audio
@@ -713,6 +714,48 @@ void intel_init_audio_hooks(struct drm_i915_private
*dev_priv)
}
}
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+ bool enable)
+{
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ int ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+ state = drm_atomic_state_alloc(&dev_priv->drm);
+ if (WARN_ON(!state))
+ return;
+
+ state->acquire_ctx = &ctx;
+
+retry:
+ to_intel_atomic_state(state)->modeset = true;
+ to_intel_atomic_state(state)->cdclk.force_min_cdclk =
+ enable ? 2 * 96000 : 0;
+
+ /*
+* Protects dev_priv->cdclk.force_min_cdclk
+* Need to lock this here in case we have no active pipes
+* and thus wouldn't lock it during the commit otherwise.
+*/
+ ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
&ctx);
+ if (!ret)
+ ret = drm_atomic_commit(state);
+
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ drm_modeset_backoff(&ctx);
+ goto retry;
+ }
+
+ WARN_ON(ret);
+
+ drm_atomic_state_put(state);
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+}
+
static void i915_audio_component_get_power(struct device *kdev)
{
intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
@@ -732,6 +775,9 @@ static void i915_audio_component_codec_wake_override(struct
device *kdev,
if (!IS_GEN9(dev_priv))
return;
+ if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+ glk_force_audio_cdclk(dev_priv, true);
+
Where did the put_power counterpart go?
with put_power counterpart the cdclk again goes back to low and then HDA
doesn't get detected. that's why i just kept the bump up.
i915_audio_component_get_power(kdev);
/*
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
b/drivers/gpu/drm/i915/intel_cdclk.c
index ebca83a44d9b..4086730018f9 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2141,24 +2141,6 @@ int intel_crtc_compute_min_cdclk(const struct
intel_crtc_state *crtc_state)
}
/*
-* According to BSpec, "The CD clock frequency must be at least twice
-* the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
-*
-* FIXME: Check the actual, not default, BCLK being used.
-*
-* FIXME: This does not depend on ->has_audio because the higher CDCLK
-* is required for audio probe, also when there are no audio capable
-* displays connected at probe time. This leads to unnecessarily high
-* CDCLK when audio is not required.
-*
-* FIXME: This limit is only applied when there are displays connected
-* at probe time. If we probe without displays, we'll still end up using
-* the platform minimum CDCLK, failing audio probe.
-*/
- if (INTEL_GEN(dev_priv) >= 9)
- min_cdclk = max(2 * 96000, min_cdclk);
I suspect we just want to revert the commit that made this uncoditional.
Otherwise the user may get a display blink every time audio playback is
started/stopped.
yeah. we should keep it. I
Re: [Intel-gfx] [PATCH] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
+ Ville
On 5/1/2018 4:42 PM, Kumar, Abhay wrote:
On 5/1/2018 2:15 AM, Saarinen, Jani wrote:
HI,
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
Du,Wenkai
Sent: maanantai 30. huhtikuuta 2018 21.23
To: Kumar, Abhay;intel-gfx@lists.freedesktop.org
Cc: Nikula, Jani
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Force 2*96 MHz cdclk on glk/cnl when
audio power is enabled
On 4/29/2018 1:39 PM, Abhay Kumar wrote:
CDCLK has to be at least twice the BLCK regardless of audio. Audio
driver has to probe using this hook and increase the clock even in
absence of any display.
Signed-off-by: Ville Syrjälä
Signed-off-by: Abhay Kumar
Tested-by: Wenkai Du
Please note that failed on
CI/GLK:https://patchwork.freedesktop.org/series/42459/
Possible regressions
igt@drv_module_reload@basic-reload:
shard-glk: PASS -> INCOMPLETE
Br,
Jani
Hi Jani,
Saw panic
@https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8838/shard-glk8/pstore11-1525091313_Panic_3.log
and was trying to reproduce this on my end with IGT but it passed for
me and never failed with DINQ, drm-tip both.
with or without external monitor connected it never fails.
ocalhost /usr/libexec/intel-gpu-tools # ./drv_module_reload
IGT-Version: 1.20-NOT-GIT (x86_64) (Linux:
4.17.0-rc3-g844dd95837ab-dirty x86_64)
Subtest basic-reload: SUCCESS (0.212s)
Reloading i915 with disable_display=1
Subtest basic-no-display: SUCCESS (0.054s)
Reloading i915 with inject_load_failure=0
Reloading i915 with inject_load_failure=1
Reloading i915 with inject_load_failure=2
Reloading i915 with inject_load_failure=3
Subtest basic-reload-inject: SUCCESS (0.329s)
localhost /usr/libexec/intel-gpu-tools # uname -r
4.17.0-rc3-g844dd95837ab-dirty
localhost /usr/libexec/intel-gpu-tools # aplay -l
List of PLAYBACK Hardware Devices
card 0: PCH [HDA Intel PCH], device 3: HDMI 0 [HDMI 0]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: PCH [HDA Intel PCH], device 7: HDMI 1 [HDMI 1]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: PCH [HDA Intel PCH], device 8: HDMI 2 [HDMI 2]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: PCH [HDA Intel PCH], device 9: HDMI 3 [HDMI 3]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: PCH [HDA Intel PCH], device 10: HDMI 4 [HDMI 4]
Subdevices: 1/1
Subdevice #0: subdevice #0
localhost /usr/libexec/intel-gpu-tools #
Regards,
Abhay
Thanks,
Wenkai
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/intel_audio.c | 46
drivers/gpu/drm/i915/intel_cdclk.c | 34 +++---
drivers/gpu/drm/i915/intel_display.c | 7 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
5 files changed, 63 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h index 193176bcddf5..34c31ef0761e
100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1708,6 +1708,8 @@ struct drm_i915_private {
struct intel_cdclk_state actual;
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+
+ int force_min_cdclk;
} cdclk;
/**
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index 3ea566f99450..f001fcf05d3a 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -594,6 +594,7 @@ static void ilk_audio_codec_enable(struct
intel_encoder *encoder,
I915_WRITE(aud_config, tmp);
}
+
/**
* intel_audio_codec_enable - Enable the audio codec for HD audio
* @encoder: encoder on which to enable audio @@ -713,6 +714,48 @@
void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
}
}
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+ bool enable) {
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ int ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+ state = drm_atomic_state_alloc(&dev_priv->drm);
+ if (WARN_ON(!state))
+ return;
+
+ state->acquire_ctx = &ctx;
+
+retry:
+ to_intel_atomic_state(state)->modeset = true;
+ to_intel_atomic_state(state)->cdclk.force_min_cdclk =
+ enable ? 2 * 96000 : 0;
+
+ /*
+* Protects dev_priv->cdclk.force_min_cdclk
+* Need to lock this here in case we have no active pipes
+* and thus wouldn't lock it during the commit otherwise.
+*/
+ ret = drm_modeset_lock(&dev_priv-
drm.mode_config.connection_mutex, &ctx);
+ if (!ret)
+ ret = drm_atomic_commit(state);
+
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ drm_modese
Re: [Intel-gfx] [PATCH] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
On 5/1/2018 2:15 AM, Saarinen, Jani wrote:
HI,
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
Du,Wenkai
Sent: maanantai 30. huhtikuuta 2018 21.23
To: Kumar, Abhay ; intel-gfx@lists.freedesktop.org
Cc: Nikula, Jani
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Force 2*96 MHz cdclk on glk/cnl when
audio power is enabled
On 4/29/2018 1:39 PM, Abhay Kumar wrote:
CDCLK has to be at least twice the BLCK regardless of audio. Audio
driver has to probe using this hook and increase the clock even in
absence of any display.
Signed-off-by: Ville Syrjälä
Signed-off-by: Abhay Kumar
Tested-by: Wenkai Du
Please note that failed on CI/GLK:
https://patchwork.freedesktop.org/series/42459/
Possible regressions
igt@drv_module_reload@basic-reload:
shard-glk: PASS -> INCOMPLETE
Br,
Jani
Hi Jani,
Saw panic
@https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8838/shard-glk8/pstore11-1525091313_Panic_3.log
and was trying to reproduce this on my end with IGT but it passed for
me and never failed with DINQ, drm-tip both.
with or without external monitor connected it never fails.
ocalhost /usr/libexec/intel-gpu-tools # ./drv_module_reload
IGT-Version: 1.20-NOT-GIT (x86_64) (Linux:
4.17.0-rc3-g844dd95837ab-dirty x86_64)
Subtest basic-reload: SUCCESS (0.212s)
Reloading i915 with disable_display=1
Subtest basic-no-display: SUCCESS (0.054s)
Reloading i915 with inject_load_failure=0
Reloading i915 with inject_load_failure=1
Reloading i915 with inject_load_failure=2
Reloading i915 with inject_load_failure=3
Subtest basic-reload-inject: SUCCESS (0.329s)
localhost /usr/libexec/intel-gpu-tools # uname -r
4.17.0-rc3-g844dd95837ab-dirty
localhost /usr/libexec/intel-gpu-tools # aplay -l
List of PLAYBACK Hardware Devices
card 0: PCH [HDA Intel PCH], device 3: HDMI 0 [HDMI 0]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: PCH [HDA Intel PCH], device 7: HDMI 1 [HDMI 1]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: PCH [HDA Intel PCH], device 8: HDMI 2 [HDMI 2]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: PCH [HDA Intel PCH], device 9: HDMI 3 [HDMI 3]
Subdevices: 1/1
Subdevice #0: subdevice #0
card 0: PCH [HDA Intel PCH], device 10: HDMI 4 [HDMI 4]
Subdevices: 1/1
Subdevice #0: subdevice #0
localhost /usr/libexec/intel-gpu-tools #
Regards,
Abhay
Thanks,
Wenkai
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/intel_audio.c | 46
drivers/gpu/drm/i915/intel_cdclk.c | 34 +++---
drivers/gpu/drm/i915/intel_display.c | 7 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
5 files changed, 63 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h index 193176bcddf5..34c31ef0761e
100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1708,6 +1708,8 @@ struct drm_i915_private {
struct intel_cdclk_state actual;
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+
+ int force_min_cdclk;
} cdclk;
/**
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index 3ea566f99450..f001fcf05d3a 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -594,6 +594,7 @@ static void ilk_audio_codec_enable(struct
intel_encoder *encoder,
I915_WRITE(aud_config, tmp);
}
+
/**
* intel_audio_codec_enable - Enable the audio codec for HD audio
* @encoder: encoder on which to enable audio @@ -713,6 +714,48 @@
void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
}
}
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+ bool enable) {
+ struct drm_modeset_acquire_ctx ctx;
+ struct drm_atomic_state *state;
+ int ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+ state = drm_atomic_state_alloc(&dev_priv->drm);
+ if (WARN_ON(!state))
+ return;
+
+ state->acquire_ctx = &ctx;
+
+retry:
+ to_intel_atomic_state(state)->modeset = true;
+ to_intel_atomic_state(state)->cdclk.force_min_cdclk =
+ enable ? 2 * 96000 : 0;
+
+ /*
+* Protects dev_priv->cdclk.force_min_cdclk
+* Need to lock this here in case we have no active pipes
+* and thus wouldn't lock it during the commit otherwise.
+*/
+ ret = drm_modeset_lock(&dev_priv-
drm.mode_config.connection_mutex, &ctx);
+ if (!ret)
+ ret = drm_atomic_commit(state);
+
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ drm_modeset_backoff(&ctx);
+ goto retry;
+ }
+
+ WAR
Re: [Intel-gfx] [PATCH v3] drm/i915: set minimum CD clock to twice the BCLK.
On 4/18/2018 8:41 AM, Ville Syrjälä wrote: On Wed, Apr 18, 2018 at 01:49:23PM +0300, Jani Nikula wrote: On Tue, 17 Apr 2018, "Kumar, Abhay" wrote: On 4/17/2018 12:06 PM, Abhay Kumar wrote: In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup. This result in no audio forever as cdclk is < 96Mhz. This change will ensure CD clock to be twice of BCLK. v2: - Address comment (Jani) - New design approach v3: - Typo fix on top of v1 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937 Signed-off-by: Abhay Kumar --- drivers/gpu/drm/i915/intel_cdclk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index dc7db8a2caf8..6e93af4a46ea 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -2143,7 +2143,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) /* According to BSpec, "The CD clock frequency must be at least twice * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default. */ - if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) + if (INTEL_GEN(dev_priv) >= 9) min_cdclk = max(2 * 96000, min_cdclk); /* Hi Ville, Jani, Since right version of this patch is taking time(doing modeset and cdclk bump) as we need to poke sound driver. Can we please get this v3(same as v1 with typo fix in comment) version of patch merged. This works all the time and will unblock Audio and lot of folks. without this patch audio card is not getting detected at all and blocking basic audio feature. I expanded on the code comment, rewrote the commit message, added cc: stable, and resent the patch [1]. It's not a patch I much like, it's not even a complete fix, and I would like this to be addressed properly going forward. However, the proper fix is I think too invasive for stable, so here we are. I'm trying to at least explain in the comment and commit message what's going on, for posterity. Ville, I'm not going to push the patch without your ack, but we can't sit on this forever either. The patch papers over the most common failure case, for now, so perhaps it'll buy us time to find a proper solution. While I don't particularly like this patch I also agree that it's the least risky way to go for stable. So Acked-by: Ville Syrjälä Abhay, if we merge this, I do expect your support in figuring out and testing the proper fix. This is not it. I also want to see a better solution going forward. Yes will work on this. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: set minimum CD clock to twice the BCLK.
On 4/17/2018 12:06 PM, Abhay Kumar wrote: In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup. This result in no audio forever as cdclk is < 96Mhz. This change will ensure CD clock to be twice of BCLK. v2: - Address comment (Jani) - New design approach v3: - Typo fix on top of v1 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937 Signed-off-by: Abhay Kumar --- drivers/gpu/drm/i915/intel_cdclk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index dc7db8a2caf8..6e93af4a46ea 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -2143,7 +2143,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) /* According to BSpec, "The CD clock frequency must be at least twice * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default. */ - if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) + if (INTEL_GEN(dev_priv) >= 9) min_cdclk = max(2 * 96000, min_cdclk); /* Hi Ville, Jani, Since right version of this patch is taking time(doing modeset and cdclk bump) as we need to poke sound driver. Can we please get this v3(same as v1 with typo fix in comment) version of patch merged. This works all the time and will unblock Audio and lot of folks. without this patch audio card is not getting detected at all and blocking basic audio feature. Regards, Abhay ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/audio: Fix audio detection issue on GLK
On 4/17/2018 12:18 AM, Gaurav K Singh wrote: On Geminilake, sometimes audio card is not getting detected after reboot. This is a spurious issue happening on Geminilake. HW codec and HD audio controller link was going out of sync for which there was a fix in i915 driver but was not getting invoked for GLK. Extending this fix to GLK as well. Tested by Du,Wenkai on GLK board. Bspec: 21829 v2: Instead of checking GEN9_BC, BXT and GLK macros, use IS_GEN9 macro Signed-off-by: Gaurav K Singh Reviewed-by: Jani Nikula Reviewed-by: Abhay Kumar --- drivers/gpu/drm/i915/intel_audio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 656f6c931341..3ea566f99450 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -729,7 +729,7 @@ static void i915_audio_component_codec_wake_override(struct device *kdev, struct drm_i915_private *dev_priv = kdev_to_i915(kdev); u32 tmp; - if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv)) + if (!IS_GEN9(dev_priv)) return; i915_audio_component_get_power(kdev); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: set minimum CD clock to twice the BCLK.
On 4/10/2018 1:49 AM, Nikula, Jani wrote: On Tue, 10 Apr 2018, Jani Nikula wrote: On Mon, 09 Apr 2018, "Kumar, Abhay" wrote: Dynamic cdclk is disabled in BIOS/GOP hence gop makes it to highest clock i.e 316.8. Will attach logs with drm debug enabled in bug. I am also inclined towards 192 which will make max cdclk.. Please also attach /sys/kernel/debug/dri/0/i915_vbt to the bug. It doesn't look like we look at the VBT dynamic cdclk field. Does dynamic debug disabled mean we should go for max? currently in kernel we don't honor this field. GOP does and by disabling it cdclk will run at max speed. attached vbt dump in bug. with patchset 1 i found one issue where while resuming HDA takes 4 second and also that time cdclk is 79.2 below logs shows which function takes more time. 78.485868] Suspending console(s) (use no_console_suspend to debug) [ 78.521654] HDMI HDA Codec ehdaudio0D2: Enter: hdmi_codec_prepare [ 78.620851] HDMI HDA Codec ehdaudio0D2: Enter1: hdac_hdmi_runtime_resume [ 78.620856] HDMI HDA Codec ehdaudio0D2: Enter2: hdac_hdmi_runtime_resume [ 78.620863] HDMI HDA Codec ehdaudio0D2: Enter3: hdac_hdmi_runtime_resume [ 78.620878] HDMI HDA Codec ehdaudio0D2: Enter4: hdac_hdmi_runtime_resume [ 78.620886] cdclk_1 79200 [ 78.624431] cdclk_1 79200 [ 78.626222] HDMI HDA Codec ehdaudio0D2: Enter5: hdac_hdmi_runtime_resume [ 78.626226] HDMI HDA Codec ehdaudio0D2: Enter6: hdac_hdmi_runtime_resume [ 79.629307] HDMI HDA Codec ehdaudio0D2: Enter7: hdac_hdmi_runtime_resume [ 80.632308] HDMI HDA Codec ehdaudio0D2: Enter8: hdac_hdmi_runtime_resume [ 81.635303] HDMI HDA Codec ehdaudio0D2: Exit: hdac_hdmi_runtime_resume [ 82.638302] HDMI HDA Codec ehdaudio0D2: Exit: hdmi_codec_prepare [ 82.638348] calling input11+ @ 2310, parent: card0 [ 82.638353] call input11+ returned 0 after 1 usecs but when i hardcode cdcdlk glk_calc_cdclk minimum 158.4 then there is no 4sec delay in these function. Ville, I tried to look at how to disable dynamic cdclk for some platforms based on the VBT, but it gets a bit hairy. The code seems pretty wired for going to lowest possible. I've got the trivial VBT parsing part, but plugging that into the cdclk code in a clean way that could *also* be backported to stable is driving me nuts. Any ideas? I'd like to fix the issue first, and (then have you ;) embark on the refactoring afterwards. It's trivial to plug the check into intel_crtc_compute_min_cdclk() and return dev_priv->max_cdclk_freq, but a) I think that place should be reserved for crtc_state related limitations, and seems the completely wrong place to do system level things, b) it's not optimal to go through all the cdclk code to do nothing in the end, and c) it doesn't work for the no crtc's active case at init time. Just setting the .set_cdclk and .modeset_calc_cdclk hooks to NULL was another idea, but the hooks get initialized before VBT parsing. And I don't think that works for init either. BR, Jani. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/audio: Fix audio detection issue on GLK
On 4/9/2018 4:20 PM, Pandiyan, Dhinakaran wrote: On Mon, 2018-04-09 at 12:18 -0700, Kumar, Abhay wrote: On 4/9/2018 12:10 PM, Rodrigo Vivi wrote: On Mon, Apr 09, 2018 at 05:07:31PM +0300, Jani Nikula wrote: On Sun, 08 Apr 2018, Gaurav K Singh wrote: On Geminilake, sometimes audio card is not getting detected after reboot. This is a spurious issue happening on Geminilake. HW codec and HD audio controller link was going out of sync for which there was a fix in i915 driver but was not getting invoked for GLK. Extending this fix to GLK as well. Tested by Du,Wenkai on GLK board. Signed-off-by: Gaurav K Singh --- drivers/gpu/drm/i915/intel_audio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 656f6c931341..73b1e0b96f88 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -729,7 +729,8 @@ static void i915_audio_component_codec_wake_override(struct device *kdev, struct drm_i915_private *dev_priv = kdev_to_i915(kdev); u32 tmp; - if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv)) + if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv) && + !IS_GEMINILAKE(dev_priv)) That could be written as if (!IS_GEN9_BC(dev_priv) && !IS_GEN9_LP(dev_priv)) which in turn could just be written as if (!IS_GEN9(dev_priv)) ...but since GLK has gen 10 display, so I'm wondering if the same issue will be present in gen 10 too, and whether this should just become if (INTEL_GEN(dev_priv) < 9) +1. I opened here to exactly add same comment. I am checking with DINQ and without this patch for GLK it can enumerate HDA codec. Ofcourse after cdclk fix. How about the other way around? i.e., does codec enumeration work this patch but without the cdclk change? Nop. with DINQ we need to have cdclk change to make Codec detection work. With or without this patch. BR, Jani. return; i915_audio_component_get_power(kdev); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: set minimum CD clock to twice the BCLK.
On 4/9/2018 3:33 AM, Ville Syrjälä wrote:
On Fri, Apr 06, 2018 at 04:47:08PM +0300, Jani Nikula wrote:
On Thu, 05 Apr 2018, Abhay Kumar wrote:
In glk when device boots with 1366x768 panel, HDA codec doesn't comeup.
This result in no audio forever as cdclk is < 96Mhz.
This chagne will ensure CD clock to be twice of BCLK.
In short, we can't poke around CDCLK like this. It needs a full modeset,
and it's non-trivial from the path you're calling this from.
I tried to cobble something together quickly:
git://github.com/vsyrjala/linux.git glk_cnl_cdclk_audio
Not tested at all, and I have no idea if my assumptions about
snd_hda_intel actually hold.
Hi Ville,
Tried your patch but as soon as it enters "glk_force_audio_cdclk"
the device locks up and reboots. waited for 30-40 times and it always
reboot at same place.
It never reaches Chrome OS login screen.
Thanks.
I'm considering pushing the original patch [1], because we haven't come
up with working alternatives. Please confirm that the patch reliably
fixes the issue.
(Though I think if you boot with *all* outputs disabled, we'll choose
the lowest CDCLK possible regardless of the patch, reproducing the same
issue.)
What's the CDCLK frequency set by the BIOS/GOP at boot? There are no
logs with drm.debug=14 attached to the referenced bug.
I see that bspec says, "158.4 MHz CD (Display Audio enumeration and
playback OK)" but that's *not* twice the BCLK. I'm inclined to lean
towards 192 MHz min leading to max CDCLK on GLK.
BR,
Jani.
Hi Jani,
Dynamic cdclk is disabled in BIOS/GOP hence gop makes it to highest
clock i.e 316.8. Will attach logs with drm debug enabled in bug.
I am also inclined towards 192 which will make max cdclk..
Currently testing all scenario to confirm if patchset 1 fixes all issue
or not. There was 4s delay issue during s0ix from audio which i
specifically want to test.
Thanks.
[1] https://patchwork.freedesktop.org/patch/184778/
v2:
- Address comment (Jani)
- New design approach
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_audio.c | 33 ++---
drivers/gpu/drm/i915/intel_cdclk.c | 21 +
drivers/gpu/drm/i915/intel_drv.h | 1 +
3 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index 709d6ca68074..f7dd3d532e93 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -723,15 +723,37 @@ static void i915_audio_component_put_power(struct device
*kdev)
intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
}
+/* Get CDCLK in kHz */
+static int i915_audio_component_get_cdclk_freq(struct device *kdev)
+{
+ struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+ if (WARN_ON_ONCE(!HAS_DDI(dev_priv)))
+ return -ENODEV;
+
+ return dev_priv->cdclk.hw.cdclk;
+}
+
static void i915_audio_component_codec_wake_override(struct device *kdev,
bool enable)
{
struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
u32 tmp;
+ int current_cdclk;
if (!IS_GEN9_BC(dev_priv))
return;
+ current_cdclk = i915_audio_component_get_cdclk_freq(kdev);
+
+ /*
+* Before probing for HDA Codec we need to make sure
+* "The CD clock frequency must be at least twice
+* the frequency of the Azalia BCLK."
+*/
+ if (INTEL_GEN(dev_priv) >= 9 && current_cdclk <= 192000)
+ intel_cdclk_bump(dev_priv);
+
i915_audio_component_get_power(kdev);
/*
@@ -753,17 +775,6 @@ static void
i915_audio_component_codec_wake_override(struct device *kdev,
i915_audio_component_put_power(kdev);
}
-/* Get CDCLK in kHz */
-static int i915_audio_component_get_cdclk_freq(struct device *kdev)
-{
- struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
-
- if (WARN_ON_ONCE(!HAS_DDI(dev_priv)))
- return -ENODEV;
-
- return dev_priv->cdclk.hw.cdclk;
-}
-
/*
* get the intel_encoder according to the parameter port and pipe
* intel_encoder is saved by the index of pipe
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
b/drivers/gpu/drm/i915/intel_cdclk.c
index dc7db8a2caf8..9426e1b7badc 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1516,6 +1516,27 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
}
/**
+ * intel_cdclk_bump - Increase cdclk to 2* BCLK
+ * @dev_priv: i915 device
+ *
+ * Increase CDCLK for GKL and CNL. This is done only
+ * during HDA codec probe.
+ */
+void intel_cdclk_bump(struct drm_i915_private *dev_priv)
+{
+ struct intel_cdclk_state cdclk_state;
+
+ cdclk_state = dev_priv->cdclk.hw;
+
+ if (IS_GEMINILAKE(dev_priv)) {
+ cdcl
Re: [Intel-gfx] [PATCH] drm/i915/audio: Fix audio detection issue on GLK
On 4/9/2018 12:10 PM, Rodrigo Vivi wrote: On Mon, Apr 09, 2018 at 05:07:31PM +0300, Jani Nikula wrote: On Sun, 08 Apr 2018, Gaurav K Singh wrote: On Geminilake, sometimes audio card is not getting detected after reboot. This is a spurious issue happening on Geminilake. HW codec and HD audio controller link was going out of sync for which there was a fix in i915 driver but was not getting invoked for GLK. Extending this fix to GLK as well. Tested by Du,Wenkai on GLK board. Signed-off-by: Gaurav K Singh --- drivers/gpu/drm/i915/intel_audio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 656f6c931341..73b1e0b96f88 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -729,7 +729,8 @@ static void i915_audio_component_codec_wake_override(struct device *kdev, struct drm_i915_private *dev_priv = kdev_to_i915(kdev); u32 tmp; - if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv)) + if (!IS_GEN9_BC(dev_priv) && !IS_BROXTON(dev_priv) && + !IS_GEMINILAKE(dev_priv)) That could be written as if (!IS_GEN9_BC(dev_priv) && !IS_GEN9_LP(dev_priv)) which in turn could just be written as if (!IS_GEN9(dev_priv)) ...but since GLK has gen 10 display, so I'm wondering if the same issue will be present in gen 10 too, and whether this should just become if (INTEL_GEN(dev_priv) < 9) +1. I opened here to exactly add same comment. I am checking with DINQ and without this patch for GLK it can enumerate HDA codec. Ofcourse after cdclk fix. BR, Jani. return; i915_audio_component_get_power(kdev); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] FW: [PATCH] drm/i915: set minimum CD clock to twice the BCLK.
On 3/23/2018 12:21 PM, Kumar, Abhay wrote:
-Original Message-
From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
Sent: Wednesday, February 14, 2018 10:00 AM
To: Kumar, Abhay ; Intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: set minimum CD clock to twice the
BCLK.
On Wed, 25 Oct 2017, abhay.ku...@intel.com wrote:
From: Abhay Kumar
In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup.
This result in no audio forever as cdclk is < 96Mhz.
This chagne will ensure CD clock to be twice of BCLK.
So this issue was never resolved was it?
Summing up, I think the ideas for solution were in order of preference:
1) Tie higher cdclk requirement to audio component get/put power
callbacks, and bump up cdclk when audio requests power
2) Bump up cdclk during i915 probe, after that require higher cdclk only
when has_audio is true
3) Require higher cdclk whenever there are display outputs that are
capable of hda
4) Always require higher cdclk (this patch)
BR,
Jani.
First of all sorry for addressing this patch late as i got tied up in
something else.
Yes this bug is still there and never resolved.
we can also honour VBT field which says enable or disable Dynamic cdclk
and control this from BIOS.
Let me figure out best way and comeup with patch.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_cdclk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
b/drivers/gpu/drm/i915/intel_cdclk.c
index e8884c2ade98..185a70f0921c 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1920,7 +1920,7 @@ int intel_crtc_compute_min_cdclk(const struct
intel_crtc_state *crtc_state)
/* According to BSpec, "The CD clock frequency must be at least twice
* the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
*/
- if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
+ if (INTEL_GEN(dev_priv) >= 9)
min_cdclk = max(2 * 96000, min_cdclk);
if (min_cdclk > dev_priv->max_cdclk_freq) {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: set minimum CD clock to twice the BCLK.
On 10/30/2017 5:21 PM, Pandiyan, Dhinakaran wrote:
On Sun, 2017-10-29 at 03:04 +, Kumar, Abhay wrote:
+ Subhransu
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
Kumar, Abhay
Sent: Thursday, October 26, 2017 12:10 PM
To: Jani Nikula ; Dhinakaran Pandiyan
; subransu.s.pru...@intel.com
Cc: intel-gfx@lists.freedesktop.org; Nujella, Sathyanarayana
Subject: Re: [Intel-gfx] [PATCH] drm/i915: set minimum CD clock to twice the
BCLK.
On 10/26/2017 1:45 AM, Jani Nikula wrote:
On Wed, 25 Oct 2017, Dhinakaran Pandiyan wrote:
On Wednesday, October 25, 2017 3:02:12 PM PDT abhay.ku...@intel.com wrote:
From: Abhay Kumar
In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup.
This result in no audio forever as cdclk is < 96Mhz.
Forever... or until next modeset with audio enabled?
Soundcard probing/detection and creation happens only during bootup. So even
though we do modeset later there is no soundcard driver to handle the event.
This chagne will ensure CD clock to be twice of BCLK.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_cdclk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
b/drivers/gpu/drm/i915/intel_cdclk.c index
e8884c2ade98..185a70f0921c
100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1920,7 +1920,7 @@ int intel_crtc_compute_min_cdclk(const struct
intel_crtc_state *crtc_state) /* According to BSpec, "The CD clock
frequency must be at least twice * the frequency of the Azalia
BCLK." and BCLK is 96 MHz by default. */
- if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
+ if (INTEL_GEN(dev_priv) >= 9)
Why should cdclk be increased when audio is not being enabled?
Indeed. I can easily imagine a counter-bug reporting excessive cdclk
when audio is not enabled.
During bootup time audio driver is trying to acquire HDA audio power well
inside i915 and then it will send HDA verb commands.
since cdclk is lower than 96Mhz HDA will not comeup resulting in timeout.
This was working fine before SKL/APL since there was no 2 PPC .
Is it ok to bump up cdclk while bootup of system/HDA and then reduce to needed
CDCLK?
I think it is worth exploring, do you have code to test whether it
solves this particular issue?
No i don't have test code for this but what i learned from other OS that
glk run at 148000 and cnl 96000*2 due to this limitation all the time.
@Shubhransu : can you please answer this doubt which we all have. This
we should be able to get from HDA specs.
wondering if this approach can cause any issue to subsequent HDA verb commands
..
BR,
Jani.
min_cdclk = max(2 * 96000, min_cdclk);
if (min_cdclk > dev_priv->max_cdclk_freq) {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: set minimum CD clock to twice the BCLK.
+ Subhransu
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
Kumar, Abhay
Sent: Thursday, October 26, 2017 12:10 PM
To: Jani Nikula ; Dhinakaran Pandiyan
; subransu.s.pru...@intel.com
Cc: intel-gfx@lists.freedesktop.org; Nujella, Sathyanarayana
Subject: Re: [Intel-gfx] [PATCH] drm/i915: set minimum CD clock to twice the
BCLK.
On 10/26/2017 1:45 AM, Jani Nikula wrote:
> On Wed, 25 Oct 2017, Dhinakaran Pandiyan
> wrote:
>> On Wednesday, October 25, 2017 3:02:12 PM PDT abhay.ku...@intel.com wrote:
>>> From: Abhay Kumar
>>>
>>> In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup.
>>> This result in no audio forever as cdclk is < 96Mhz.
> Forever... or until next modeset with audio enabled?
Soundcard probing/detection and creation happens only during bootup. So even
though we do modeset later there is no soundcard driver to handle the event.
>
>>> This chagne will ensure CD clock to be twice of BCLK.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937
>>> Signed-off-by: Abhay Kumar
>>> ---
>>> drivers/gpu/drm/i915/intel_cdclk.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
>>> b/drivers/gpu/drm/i915/intel_cdclk.c index
>>> e8884c2ade98..185a70f0921c
>>> 100644
>>> --- a/drivers/gpu/drm/i915/intel_cdclk.c
>>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
>>> @@ -1920,7 +1920,7 @@ int intel_crtc_compute_min_cdclk(const struct
>>> intel_crtc_state *crtc_state) /* According to BSpec, "The CD clock
>>> frequency must be at least twice * the frequency of the Azalia
>>> BCLK." and BCLK is 96 MHz by default. */
>>> - if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
>>> + if (INTEL_GEN(dev_priv) >= 9)
>> Why should cdclk be increased when audio is not being enabled?
> Indeed. I can easily imagine a counter-bug reporting excessive cdclk
> when audio is not enabled.
During bootup time audio driver is trying to acquire HDA audio power well
inside i915 and then it will send HDA verb commands.
since cdclk is lower than 96Mhz HDA will not comeup resulting in timeout.
This was working fine before SKL/APL since there was no 2 PPC .
Is it ok to bump up cdclk while bootup of system/HDA and then reduce to needed
CDCLK?
wondering if this approach can cause any issue to subsequent HDA verb commands
..
>
> BR,
> Jani.
>
>>> min_cdclk = max(2 * 96000, min_cdclk);
>>>
>>> if (min_cdclk > dev_priv->max_cdclk_freq) {
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: set minimum CD clock to twice the BCLK.
On 10/26/2017 1:45 AM, Jani Nikula wrote:
On Wed, 25 Oct 2017, Dhinakaran Pandiyan wrote:
On Wednesday, October 25, 2017 3:02:12 PM PDT abhay.ku...@intel.com wrote:
From: Abhay Kumar
In glk when device boots with only 1366x768 panel, HDA codec doesn't comeup.
This result in no audio forever as cdclk is < 96Mhz.
Forever... or until next modeset with audio enabled?
Soundcard probing/detection and creation happens only during bootup. So
even though we do modeset later there is no soundcard driver to handle
the event.
This chagne will ensure CD clock to be twice of BCLK.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102937
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_cdclk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
b/drivers/gpu/drm/i915/intel_cdclk.c index e8884c2ade98..185a70f0921c
100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1920,7 +1920,7 @@ int intel_crtc_compute_min_cdclk(const struct
intel_crtc_state *crtc_state) /* According to BSpec, "The CD clock
frequency must be at least twice * the frequency of the Azalia BCLK." and
BCLK is 96 MHz by default. */
- if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
+ if (INTEL_GEN(dev_priv) >= 9)
Why should cdclk be increased when audio is not being enabled?
Indeed. I can easily imagine a counter-bug reporting excessive cdclk
when audio is not enabled.
During bootup time audio driver is trying to acquire HDA audio power
well inside i915 and then it will send HDA verb commands.
since cdclk is lower than 96Mhz HDA will not comeup resulting in
timeout. This was working fine before SKL/APL since there was no 2 PPC .
Is it ok to bump up cdclk while bootup of system/HDA and then reduce to
needed CDCLK?
wondering if this approach can cause any issue to subsequent HDA verb
commands ..
BR,
Jani.
min_cdclk = max(2 * 96000, min_cdclk);
if (min_cdclk > dev_priv->max_cdclk_freq) {
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V5] drm/i915: edp resume/On time optimization.
On 1/22/2016 5:39 PM, Kumar, Abhay wrote:
From: Abhay Kumar
Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.
v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
delay calculation(Ville).
v3: Addressed below comments
1. Tracking time from where last powercycle is initiated.
2. Used ktime_get_bootime() wrapper for boottime clock.
3. Used ktime_ms_delta() to get time difference.
v4: Updated v3 change log in detail.
v5: Removed static from panel_power_on_time(Stéphane).
Cc: Ville Syrjälä
Signed-off-by: Abhay Kumar
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 19 ++-
drivers/gpu/drm/i915/intel_drv.h | 2 +-
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e2bea710..29f21c1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1811,12 +1811,21 @@ static void wait_panel_off(struct intel_dp *intel_dp)
static void wait_panel_power_cycle(struct intel_dp *intel_dp)
{
+ ktime_t panel_power_on_time;
+ s64 panel_power_off_duration;
+
DRM_DEBUG_KMS("Wait for panel power cycle\n");
+ /* take the difference of currrent time and panel power off time
+* and then make panel wait for t11_t12 if needed. */
+ panel_power_on_time = ktime_get_boottime();
+ panel_power_off_duration = ktime_ms_delta(panel_power_on_time,
intel_dp->panel_power_off_time);
+
/* When we disable the VDD override bit last we have to do the manual
* wait. */
- wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
- intel_dp->panel_power_cycle_delay);
+ if (panel_power_off_duration < (s64)intel_dp->panel_power_cycle_delay)
+ wait_remaining_ms_from_jiffies(jiffies,
+ intel_dp->panel_power_cycle_delay -
panel_power_off_duration);
wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
}
@@ -1968,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp
*intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
if ((pp & POWER_TARGET_ON) == 0)
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
power_domain = intel_display_port_aux_power_domain(intel_encoder);
intel_display_power_put(dev_priv, power_domain);
@@ -2117,7 +2126,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
@@ -5116,7 +5125,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
drm_connector *connect
static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
{
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bc97012..137e40d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -770,9 +770,9 @@ struct intel_dp {
int backlight_off_delay;
struct delayed_work panel_vdd_work;
bool want_panel_vdd;
- unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ ktime_t panel_power_off_time;
struct notifier_block edp_notifier;
Hi Daniel,
can we please get this patch Queued?
Thanks
Regards,
Abhay Kumar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915: edp resume/On time optimization.
On 1/21/2016 9:48 PM, Kumar, Shobhit wrote:
On 01/21/2016 05:47 PM, Kumar, Abhay wrote:
On 1/20/2016 5:06 AM, Ville Syrjälä wrote:
On Wed, Jan 20, 2016 at 03:29:00PM +0530, Kumar, Shobhit wrote:
On 01/20/2016 02:30 PM, Daniel Vetter wrote:
On Tue, Jan 19, 2016 at 02:37:55PM -0800, Kumar, Abhay wrote:
On 1/12/2016 5:57 PM, Kumar, Abhay wrote:
From: Abhay Kumar
Make resume/on codepath not to wait for
panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.
v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
delay calculation(Ville).
v3: Addressed below comments
1. Tracking time from where last powercycle is initiated.
2. Used ktime_get_bootime() wrapper for boottime clock.
3. Used ktime_ms_delta() to get time difference.
v4: Updated v3 change log in detail.
Cc: Ville Syrjälä
Signed-off-by: Abhay Kumar
I can't see the r-b here nor anywhere else in this thread. That's why I
didn't pick it up. Where is it?
V3 of this patch was r-b by Ville and had comment on the commit msg. I
updated commit msg and pushed V4.
So practically there is no code change and Ville r-b should hold good.
Daniel, please wait before merging as another update is on the way
because of some more review comments which also already has Ville's r-b
Regards
Shobhit
Daniel, V5 of this patch is pushed,reviewed and looks good. Please
Queue it.
Regards,
Abhay kumar
Sorry in case i missed to keep that r-b by Ville in V3 here.
I didn't see it either. Ville can you please have a look at the latest
changes.
I don't think I'll bother since I'm pretty sure I already gave the r-b
during the last round. People really should learn to hang on to the r-bs
with some vigor.
Regards
Shobhit
-Daniel
---
drivers/gpu/drm/i915/intel_dp.c | 19 ++-
drivers/gpu/drm/i915/intel_drv.h | 2 +-
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..0042693 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1812,12 +1812,21 @@ static void wait_panel_off(struct intel_dp
*intel_dp)
static void wait_panel_power_cycle(struct intel_dp *intel_dp)
{
+static ktime_t panel_power_on_time;
+s64 panel_power_off_duration;
+
DRM_DEBUG_KMS("Wait for panel power cycle\n");
+/* take the difference of currrent time and panel power off time
+ * and then make panel wait for t11_t12 if needed. */
+panel_power_on_time = ktime_get_boottime();
+panel_power_off_duration =
ktime_ms_delta(panel_power_on_time, intel_dp->panel_power_off_time);
+
/* When we disable the VDD override bit last we have to do
the manual
* wait. */
-wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
- intel_dp->panel_power_cycle_delay);
+if (panel_power_off_duration <
(s64)intel_dp->panel_power_cycle_delay)
+wait_remaining_ms_from_jiffies(jiffies,
+ intel_dp->panel_power_cycle_delay -
panel_power_off_duration);
wait_panel_status(intel_dp, IDLE_CYCLE_MASK,
IDLE_CYCLE_VALUE);
}
@@ -1969,7 +1978,7 @@ static void edp_panel_vdd_off_sync(struct
intel_dp *intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
if ((pp & POWER_TARGET_ON) == 0)
-intel_dp->last_power_cycle = jiffies;
+intel_dp->panel_power_off_time = ktime_get_boottime();
power_domain =
intel_display_port_aux_power_domain(intel_encoder);
intel_display_power_put(dev_priv, power_domain);
@@ -2118,7 +2127,7 @@ static void edp_panel_off(struct intel_dp
*intel_dp)
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
-intel_dp->last_power_cycle = jiffies;
+intel_dp->panel_power_off_time = ktime_get_boottime();
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
@@ -5122,7 +5131,7 @@ intel_dp_add_properties(struct intel_dp
*intel_dp, struct drm_connector *connect
static void intel_dp_init_panel_power_timestamps(struct
intel_dp *intel_dp)
{
-intel_dp->last_power_cycle = jiffies;
+intel_dp->panel_power_off_time = ktime_get_boottime();
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index bdfe403..06b37b8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -793,9 +793,9 @@ struct intel_dp {
int backlight_off_delay;
struct delayed_work panel_vdd_work;
bool want_panel_vdd;
-unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ktime_t panel_power_off_time;
struct notifier_block edp_notifier;
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: edp resume/On time optimization. (rev10)
On 1/26/2016 3:36 AM, Ville Syrjälä wrote: On Sat, Jan 23, 2016 at 08:43:33AM -, Patchwork wrote: == Summary == Built on 8fe9e785ae04fa7c37f7935cff12d62e38054b60 drm-intel-nightly: 2016y-01m-21d-11h-02m-42s UTC integration manifest Test gem_ctx_basic: pass -> FAIL (bdw-ultra) This is another 'returncode -15'. I think that's just some piglit fail or something. I think it would indicate someone sent a SIGTERM to the test. Not sure why that would happen. Some timeout perhaps? This might be a false alarm as changes between v4 and v5 is just a static keyword removal and that shouldn't cause timeout to happen. V4 results seems to be fine. Shall we ignore this? Is there any way to rerun the tests apart from pushing patch again or debug this in case? Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (ilk-hp8440p) And that's the ilk underrrun thing: https://bugs.freedesktop.org/show_bug.cgi?id=93787 bdw-nuci7total:140 pass:131 dwarn:0 dfail:0 fail:0 skip:9 bdw-ultratotal:143 pass:136 dwarn:0 dfail:0 fail:1 skip:6 bsw-nuc-2total:143 pass:119 dwarn:0 dfail:0 fail:0 skip:24 byt-nuc total:143 pass:128 dwarn:0 dfail:0 fail:0 skip:15 hsw-brixbox total:143 pass:136 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:143 pass:139 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:143 pass:103 dwarn:2 dfail:0 fail:0 skip:38 ivb-t430stotal:143 pass:137 dwarn:0 dfail:0 fail:0 skip:6 skl-i5k-2total:143 pass:134 dwarn:1 dfail:0 fail:0 skip:8 snb-dellxps total:143 pass:129 dwarn:0 dfail:0 fail:0 skip:14 snb-x220ttotal:143 pass:129 dwarn:0 dfail:0 fail:1 skip:13 Results at /archive/results/CI_IGT_test/Patchwork_1256/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915: edp resume/On time optimization.
On 1/20/2016 5:06 AM, Ville Syrjälä wrote:
On Wed, Jan 20, 2016 at 03:29:00PM +0530, Kumar, Shobhit wrote:
On 01/20/2016 02:30 PM, Daniel Vetter wrote:
On Tue, Jan 19, 2016 at 02:37:55PM -0800, Kumar, Abhay wrote:
On 1/12/2016 5:57 PM, Kumar, Abhay wrote:
From: Abhay Kumar
Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.
v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
delay calculation(Ville).
v3: Addressed below comments
1. Tracking time from where last powercycle is initiated.
2. Used ktime_get_bootime() wrapper for boottime clock.
3. Used ktime_ms_delta() to get time difference.
v4: Updated v3 change log in detail.
Cc: Ville Syrjälä
Signed-off-by: Abhay Kumar
I can't see the r-b here nor anywhere else in this thread. That's why I
didn't pick it up. Where is it?
V3 of this patch was r-b by Ville and had comment on the commit msg. I
updated commit msg and pushed V4.
So practically there is no code change and Ville r-b should hold good.
Sorry in case i missed to keep that r-b by Ville in V3 here.
I didn't see it either. Ville can you please have a look at the latest
changes.
I don't think I'll bother since I'm pretty sure I already gave the r-b
during the last round. People really should learn to hang on to the r-bs
with some vigor.
Regards
Shobhit
-Daniel
---
drivers/gpu/drm/i915/intel_dp.c | 19 ++-
drivers/gpu/drm/i915/intel_drv.h | 2 +-
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..0042693 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1812,12 +1812,21 @@ static void wait_panel_off(struct intel_dp *intel_dp)
static void wait_panel_power_cycle(struct intel_dp *intel_dp)
{
+ static ktime_t panel_power_on_time;
+ s64 panel_power_off_duration;
+
DRM_DEBUG_KMS("Wait for panel power cycle\n");
+ /* take the difference of currrent time and panel power off time
+* and then make panel wait for t11_t12 if needed. */
+ panel_power_on_time = ktime_get_boottime();
+ panel_power_off_duration = ktime_ms_delta(panel_power_on_time,
intel_dp->panel_power_off_time);
+
/* When we disable the VDD override bit last we have to do the manual
* wait. */
- wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
- intel_dp->panel_power_cycle_delay);
+ if (panel_power_off_duration < (s64)intel_dp->panel_power_cycle_delay)
+ wait_remaining_ms_from_jiffies(jiffies,
+ intel_dp->panel_power_cycle_delay -
panel_power_off_duration);
wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
}
@@ -1969,7 +1978,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp
*intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
if ((pp & POWER_TARGET_ON) == 0)
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
power_domain = intel_display_port_aux_power_domain(intel_encoder);
intel_display_power_put(dev_priv, power_domain);
@@ -2118,7 +2127,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
@@ -5122,7 +5131,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
drm_connector *connect
static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
{
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bdfe403..06b37b8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -793,9 +793,9 @@ struct intel_dp {
int backlight_off_delay;
struct delayed_work panel_vdd_work;
bool want_panel_vdd;
- unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ ktime_t panel_power_off_time;
struct notifier_block edp_notifier;
Since this is already reviewed. Can we please get this Queued for -next ?
Regards,
Abhay Kumar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915: edp resume/On time optimization.
On 1/12/2016 5:57 PM, Kumar, Abhay wrote:
From: Abhay Kumar
Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.
v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
delay calculation(Ville).
v3: Addressed below comments
1. Tracking time from where last powercycle is initiated.
2. Used ktime_get_bootime() wrapper for boottime clock.
3. Used ktime_ms_delta() to get time difference.
v4: Updated v3 change log in detail.
Cc: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_dp.c | 19 ++-
drivers/gpu/drm/i915/intel_drv.h | 2 +-
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..0042693 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1812,12 +1812,21 @@ static void wait_panel_off(struct intel_dp *intel_dp)
static void wait_panel_power_cycle(struct intel_dp *intel_dp)
{
+ static ktime_t panel_power_on_time;
+ s64 panel_power_off_duration;
+
DRM_DEBUG_KMS("Wait for panel power cycle\n");
+ /* take the difference of currrent time and panel power off time
+* and then make panel wait for t11_t12 if needed. */
+ panel_power_on_time = ktime_get_boottime();
+ panel_power_off_duration = ktime_ms_delta(panel_power_on_time,
intel_dp->panel_power_off_time);
+
/* When we disable the VDD override bit last we have to do the manual
* wait. */
- wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
- intel_dp->panel_power_cycle_delay);
+ if (panel_power_off_duration < (s64)intel_dp->panel_power_cycle_delay)
+ wait_remaining_ms_from_jiffies(jiffies,
+ intel_dp->panel_power_cycle_delay -
panel_power_off_duration);
wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
}
@@ -1969,7 +1978,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp
*intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
if ((pp & POWER_TARGET_ON) == 0)
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
power_domain = intel_display_port_aux_power_domain(intel_encoder);
intel_display_power_put(dev_priv, power_domain);
@@ -2118,7 +2127,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
@@ -5122,7 +5131,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
drm_connector *connect
static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
{
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bdfe403..06b37b8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -793,9 +793,9 @@ struct intel_dp {
int backlight_off_delay;
struct delayed_work panel_vdd_work;
bool want_panel_vdd;
- unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ ktime_t panel_power_off_time;
struct notifier_block edp_notifier;
Since this is already reviewed. Can we please get this Queued for -next ?
Regards,
Abhay Kumar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915: edp resume/On time optimization.
On 1/12/2016 5:57 PM, Kumar, Abhay wrote:
From: Abhay Kumar
Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.
v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
delay calculation(Ville).
v3: Addressed below comments
1. Tracking time from where last powercycle is initiated.
2. Used ktime_get_bootime() wrapper for boottime clock.
3. Used ktime_ms_delta() to get time difference.
v4: Updated v3 change log in detail.
Cc: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_dp.c | 19 ++-
drivers/gpu/drm/i915/intel_drv.h | 2 +-
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..0042693 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1812,12 +1812,21 @@ static void wait_panel_off(struct intel_dp *intel_dp)
static void wait_panel_power_cycle(struct intel_dp *intel_dp)
{
+ static ktime_t panel_power_on_time;
+ s64 panel_power_off_duration;
+
DRM_DEBUG_KMS("Wait for panel power cycle\n");
+ /* take the difference of currrent time and panel power off time
+* and then make panel wait for t11_t12 if needed. */
+ panel_power_on_time = ktime_get_boottime();
+ panel_power_off_duration = ktime_ms_delta(panel_power_on_time,
intel_dp->panel_power_off_time);
+
/* When we disable the VDD override bit last we have to do the manual
* wait. */
- wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
- intel_dp->panel_power_cycle_delay);
+ if (panel_power_off_duration < (s64)intel_dp->panel_power_cycle_delay)
+ wait_remaining_ms_from_jiffies(jiffies,
+ intel_dp->panel_power_cycle_delay -
panel_power_off_duration);
wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
}
@@ -1969,7 +1978,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp
*intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
if ((pp & POWER_TARGET_ON) == 0)
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
power_domain = intel_display_port_aux_power_domain(intel_encoder);
intel_display_power_put(dev_priv, power_domain);
@@ -2118,7 +2127,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
@@ -5122,7 +5131,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
drm_connector *connect
static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
{
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time = ktime_get_boottime();
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bdfe403..06b37b8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -793,9 +793,9 @@ struct intel_dp {
int backlight_off_delay;
struct delayed_work panel_vdd_work;
bool want_panel_vdd;
- unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ ktime_t panel_power_off_time;
struct notifier_block edp_notifier;
Hi Ville,
If we have everything addressed can we please get this pulled into
drm-nightly?
Regards,
Abhay Kumar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
On 1/7/2016 10:15 AM, Ville Syrjälä wrote:
On Mon, Dec 21, 2015 at 05:18:52PM -0800, abhay.ku...@intel.com wrote:
From: Abhay Kumar
Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.
v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
delay calculation(Ville).
The approach seems reasonable enough to me. There are a few issues with
the patch though, see below.
Cc: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_ddi.c | 3 +++
drivers/gpu/drm/i915/intel_dp.c | 22 ++
drivers/gpu/drm/i915/intel_drv.h | 2 +-
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5..480697d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder
*intel_encoder)
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_edp_panel_vdd_on(intel_dp);
intel_edp_panel_off(intel_dp);
+
+ /* storing panel power off time */
The comment seems rather pointless.
+ intel_dp->panel_power_off_time =
ktime_get_with_offset(TK_OFFS_BOOT);
There appears to be a wrapper for this: ktime_get_boottime().
Not sure why you're adding this here anyway. Should be enough to just
replace the places where we currently sample jiffies to sample the boot
clock AFAICS.
}
if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..c813605 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,7 +38,6 @@
#include "intel_drv.h"
#include
#include "i915_drv.h"
-
Spurious change.
#define DP_LINK_CHECK_TIMEOUT (10 * 1000)
/* Compliance test status bits */
@@ -1812,13 +1811,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
static void wait_panel_power_cycle(struct intel_dp *intel_dp)
{
+ ktime_t panel_power_on_time;
+ u32 panel_power_off_duration;
+
DRM_DEBUG_KMS("Wait for panel power cycle\n");
- /* When we disable the VDD override bit last we have to do the manual
-* wait. */
- wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
- intel_dp->panel_power_cycle_delay);
+/* take the diffrence of currrent time and panel power off time
+ and then make panel wait for t11_t12 if needed */
Indent fail. Also the comment isn't in proper format.
+ panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
+ panel_power_off_duration = (panel_power_on_time.tv64 -
intel_dp->panel_power_off_time.tv64);
+ panel_power_off_duration = panel_power_off_duration / 100;
ktime_ms_delta() perhaps?
sure.
+ /* When we disable the VDD override bit last we have to do the manual
+* wait */
This comment formatting is a bit wonky too. Maybe polish it up while at
it.
+ if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
+ wait_remaining_ms_from_jiffies(jiffies,
+ (intel_dp->panel_power_cycle_delay -
panel_power_off_duration));
wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
}
@@ -1969,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
if ((pp & POWER_TARGET_ON) == 0)
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time =
ktime_get_with_offset(TK_OFFS_BOOT);
power_domain = intel_display_port_aux_power_domain(intel_encoder);
intel_display_power_put(dev_priv, power_domain);
@@ -2118,7 +2126,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
- intel_dp->last_power_cycle = jiffies;
Removing this doens't seem correct. Instead we should sample the clock
here as well.
Do we really need "last_power_cycle" ? As the ktime_get_bootime() will
always calculate the boot time in fresh boot from zero and we only need
to track the delta time when we go to suspend and resume.
this is the reason i removed last_power_cycle sampling and also
initialization. Please let me know if this is ok and make sense?
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
@@ -5122,7 +5129,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
drm_connector *connect
static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
{
- intel_dp->last_power_cycle = jiffies;
and I suppose here too.
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies;
}
diff --gi
Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
On 12/21/2015 5:18 PM, Kumar, Abhay wrote:
From: Abhay Kumar
Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.
v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
delay calculation(Ville).
Cc: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_ddi.c | 3 +++
drivers/gpu/drm/i915/intel_dp.c | 22 ++
drivers/gpu/drm/i915/intel_drv.h | 2 +-
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5..480697d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder
*intel_encoder)
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_edp_panel_vdd_on(intel_dp);
intel_edp_panel_off(intel_dp);
+
+ /* storing panel power off time */
+ intel_dp->panel_power_off_time =
ktime_get_with_offset(TK_OFFS_BOOT);
}
if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..c813605 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,7 +38,6 @@
#include "intel_drv.h"
#include
#include "i915_drv.h"
-
#define DP_LINK_CHECK_TIMEOUT (10 * 1000)
/* Compliance test status bits */
@@ -1812,13 +1811,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
static void wait_panel_power_cycle(struct intel_dp *intel_dp)
{
+ ktime_t panel_power_on_time;
+ u32 panel_power_off_duration;
+
DRM_DEBUG_KMS("Wait for panel power cycle\n");
- /* When we disable the VDD override bit last we have to do the manual
-* wait. */
- wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
- intel_dp->panel_power_cycle_delay);
+/* take the diffrence of currrent time and panel power off time
+ and then make panel wait for t11_t12 if needed */
+ panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
+ panel_power_off_duration = (panel_power_on_time.tv64 -
intel_dp->panel_power_off_time.tv64);
+ panel_power_off_duration = panel_power_off_duration / 100;
+ /* When we disable the VDD override bit last we have to do the manual
+* wait */
+ if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
+ wait_remaining_ms_from_jiffies(jiffies,
+ (intel_dp->panel_power_cycle_delay -
panel_power_off_duration));
wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
}
@@ -1969,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
if ((pp & POWER_TARGET_ON) == 0)
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time =
ktime_get_with_offset(TK_OFFS_BOOT);
power_domain = intel_display_port_aux_power_domain(intel_encoder);
intel_display_power_put(dev_priv, power_domain);
@@ -2118,7 +2126,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
- intel_dp->last_power_cycle = jiffies;
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
@@ -5122,7 +5129,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
drm_connector *connect
static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
{
- intel_dp->last_power_cycle = jiffies;
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d523ebb..84ad134 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -765,9 +765,9 @@ struct intel_dp {
int backlight_off_delay;
struct delayed_work panel_vdd_work;
bool want_panel_vdd;
- unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ ktime_t panel_power_off_time;
struct notifier_block edp_notifier;
Is this close to what you wanted? thoughts?
Pardon me for pinging again as last time send with wrong email client.
Regards,
Abhay Kumar
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
On 1/5/2016 3:04 AM, Daniel Vetter wrote: On Tue, Jan 05, 2016 at 01:30:53AM +, Kumar, Abhay wrote: Ville, Is this patch is coming close to what you wanted? Please don't bottom-post but not quote properly - no one will ever find your comment and assume you accidentally sent out the patch twice. If you have to use a broken mailer that can't quote, then top-post. But please just install something that does work (I personally use Thunderbird on Windows, it getst the job done). Anything else just doesn't work. Thanks, Daniel Thanks Daniel. I used wrong email client. Will take care of this from next time for bottom post. Regards, Abhay Kumar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12) if
this time is already spent in suspend/poweron time.
v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
delay calculation(Ville).
Cc: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_ddi.c | 3 +++ drivers/gpu/drm/i915/intel_dp.c |
22 ++ drivers/gpu/drm/i915/intel_drv.h | 2 +-
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5..480697d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder
*intel_encoder)
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_edp_panel_vdd_on(intel_dp);
intel_edp_panel_off(intel_dp);
+
+ /* storing panel power off time */
+ intel_dp->panel_power_off_time =
ktime_get_with_offset(TK_OFFS_BOOT);
}
if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) diff --git
a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index
796e3d3..c813605 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,7 +38,6 @@
#include "intel_drv.h"
#include
#include "i915_drv.h"
-
#define DP_LINK_CHECK_TIMEOUT (10 * 1000)
/* Compliance test status bits */
@@ -1812,13 +1811,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
static void wait_panel_power_cycle(struct intel_dp *intel_dp) {
+ ktime_t panel_power_on_time;
+ u32 panel_power_off_duration;
+
DRM_DEBUG_KMS("Wait for panel power cycle\n");
- /* When we disable the VDD override bit last we have to do the manual
-* wait. */
- wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
- intel_dp->panel_power_cycle_delay);
+/* take the diffrence of currrent time and panel power off time
+ and then make panel wait for t11_t12 if needed */
+ panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
+ panel_power_off_duration = (panel_power_on_time.tv64 -
intel_dp->panel_power_off_time.tv64);
+ panel_power_off_duration = panel_power_off_duration / 100;
+ /* When we disable the VDD override bit last we have to do the manual
+* wait */
+ if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
+ wait_remaining_ms_from_jiffies(jiffies,
+ (intel_dp->panel_power_cycle_delay -
+panel_power_off_duration));
wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE); }
@@ -1969,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp
*intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
if ((pp & POWER_TARGET_ON) == 0)
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time =
ktime_get_with_offset(TK_OFFS_BOOT);
power_domain = intel_display_port_aux_power_domain(intel_encoder);
intel_display_power_put(dev_priv, power_domain); @@ -2118,7 +2126,6 @@
static void edp_panel_off(struct intel_dp *intel_dp)
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
- intel_dp->last_power_cycle = jiffies;
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */ @@ -5122,7 +5129,6 @@
intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) {
- intel_dp->last_power_cycle = jiffies;
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies; } diff --git
a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d523ebb..84ad134 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -765,9 +765,9 @@ struct intel_dp {
int backlight_off_delay;
struct delayed_work panel_vdd_work;
bool want_panel_vdd;
- unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ ktime_t panel_power_off_time;
struct notifier_block edp_notifier;
Ville,
Is this patch is coming close to what you wanted?
Regards,
Abhay Kumar
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
Thanks Daniel. Will push today with proper comment.
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Monday, December 21, 2015 7:58 AM
To: Kumar, Abhay
Cc: Intel-gfx@lists.freedesktop.org; Syrjala, Ville; Paulo Zanoni
Subject: Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
On Mon, Dec 21, 2015 at 05:33:41AM +, Kumar, Abhay wrote:
> Changed the implementation using boottime and removed jiffies. Please review
> and let us know if this is close.
Comments like these should be part of the patch submission itself, including a
note about which reviewer made the suggestion and adding everyone who commented
to the cc list. So something like this at the
bottom:
v2:
- Change (Ville).
- Use boootime instead of jiffies (Ville).
Cc: Ville (full mail address here).
Singed-off-by: ...
Without either the in-patch changelog or the cc it's much harder for reviewers
to follow along, especially when they're commenting on 10+ patch series at the
same time.
Please resubmit with that added.
Thanks, Daniel
>
> -Original Message-
> From: Kumar, Abhay
> Sent: Friday, December 18, 2015 11:55 AM
> To: Intel-gfx@lists.freedesktop.org
> Cc: Kumar, Abhay
> Subject: [PATCH] drm/i915: edp resume/On time optimization.
>
> From: Abhay Kumar
>
> Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12) if
> this time is already spent in suspend/poweron time.
>
> Change-Id: Ied0f10f82776af8e6e8ff561bb4e5c0ce1dad4b3
> Signed-off-by: Abhay Kumar
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 3 +++
> drivers/gpu/drm/i915/intel_dp.c | 22 ++
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> 3 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index cbabcb4..fe99d72 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2347,6 +2347,9 @@ static void intel_ddi_post_disable(struct intel_encoder
> *intel_encoder)
> intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> intel_edp_panel_vdd_on(intel_dp);
> intel_edp_panel_off(intel_dp);
> +
> + /* storing panel power off time */
> + intel_dp->panel_power_off_time =
> +ktime_get_with_offset(TK_OFFS_BOOT);
> }
>
> if (IS_SKYLAKE(dev))
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c index acda70e..845944d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -38,7 +38,6 @@
> #include "intel_drv.h"
> #include
> #include "i915_drv.h"
> -
> #define DP_LINK_CHECK_TIMEOUT(10 * 1000)
>
> /* Compliance test status bits */
> @@ -1654,13 +1653,22 @@ static void wait_panel_off(struct intel_dp
> *intel_dp)
>
> static void wait_panel_power_cycle(struct intel_dp *intel_dp) {
> + ktime_t panel_power_on_time;
> + u32 panel_power_off_duration;
> +
> DRM_DEBUG_KMS("Wait for panel power cycle\n");
>
> - /* When we disable the VDD override bit last we have to do the manual
> - * wait. */
> - wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
> -intel_dp->panel_power_cycle_delay);
> +/* take the diffrence of currrent time and panel power off time
> + and then make panel wait for t11_t12 if needed */
> + panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
> + panel_power_off_duration = (panel_power_on_time.tv64 -
> intel_dp->panel_power_off_time.tv64);
> + panel_power_off_duration = panel_power_off_duration / 100;
>
> + /* When we disable the VDD override bit last we have to do the manual
> + * wait */
> + if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
> + wait_remaining_ms_from_jiffies(jiffies,
> +(intel_dp->panel_power_cycle_delay -
> +panel_power_off_duration));
> wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE); }
>
> @@ -1811,7 +1819,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp
> *intel_dp)
> I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>
> if ((pp & POWER_TARGET_ON) == 0)
> - intel_dp->last_power_cycle = jiffies;
> + intel_dp->panel_power_off_time =
> +ktime_get_with_offset(TK_OFFS_BOOT);
>
> power_domain = intel_display_port_power_domain(intel_encoder);
> intel_display_power_put(dev_priv, power_domain); @@ -1960,7 +1968,6 @@
> static
Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
Changed the implementation using boottime and removed jiffies. Please review
and let us know if this is close.
-Original Message-
From: Kumar, Abhay
Sent: Friday, December 18, 2015 11:55 AM
To: Intel-gfx@lists.freedesktop.org
Cc: Kumar, Abhay
Subject: [PATCH] drm/i915: edp resume/On time optimization.
From: Abhay Kumar
Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12) if
this time is already spent in suspend/poweron time.
Change-Id: Ied0f10f82776af8e6e8ff561bb4e5c0ce1dad4b3
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_ddi.c | 3 +++ drivers/gpu/drm/i915/intel_dp.c |
22 ++ drivers/gpu/drm/i915/intel_drv.h | 2 +-
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cbabcb4..fe99d72 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2347,6 +2347,9 @@ static void intel_ddi_post_disable(struct intel_encoder
*intel_encoder)
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_edp_panel_vdd_on(intel_dp);
intel_edp_panel_off(intel_dp);
+
+ /* storing panel power off time */
+ intel_dp->panel_power_off_time =
ktime_get_with_offset(TK_OFFS_BOOT);
}
if (IS_SKYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index acda70e..845944d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,7 +38,6 @@
#include "intel_drv.h"
#include
#include "i915_drv.h"
-
#define DP_LINK_CHECK_TIMEOUT (10 * 1000)
/* Compliance test status bits */
@@ -1654,13 +1653,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
static void wait_panel_power_cycle(struct intel_dp *intel_dp) {
+ ktime_t panel_power_on_time;
+ u32 panel_power_off_duration;
+
DRM_DEBUG_KMS("Wait for panel power cycle\n");
- /* When we disable the VDD override bit last we have to do the manual
-* wait. */
- wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
- intel_dp->panel_power_cycle_delay);
+/* take the diffrence of currrent time and panel power off time
+ and then make panel wait for t11_t12 if needed */
+ panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
+ panel_power_off_duration = (panel_power_on_time.tv64 -
intel_dp->panel_power_off_time.tv64);
+ panel_power_off_duration = panel_power_off_duration / 100;
+ /* When we disable the VDD override bit last we have to do the manual
+* wait */
+ if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
+ wait_remaining_ms_from_jiffies(jiffies,
+ (intel_dp->panel_power_cycle_delay -
+panel_power_off_duration));
wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE); }
@@ -1811,7 +1819,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp
*intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
if ((pp & POWER_TARGET_ON) == 0)
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time =
ktime_get_with_offset(TK_OFFS_BOOT);
power_domain = intel_display_port_power_domain(intel_encoder);
intel_display_power_put(dev_priv, power_domain); @@ -1960,7 +1968,6 @@
static void edp_panel_off(struct intel_dp *intel_dp)
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
- intel_dp->last_power_cycle = jiffies;
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */ @@ -5196,7 +5203,6 @@
intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp) {
- intel_dp->last_power_cycle = jiffies;
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies; } diff --git
a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d44f2f5..ef82e8f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -722,9 +722,9 @@ struct intel_dp {
int backlight_off_delay;
struct delayed_work panel_vdd_work;
bool want_panel_vdd;
- unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ ktime_t panel_power_off_time;
struct notifier_block edp_notifier;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
Sure. In next patch set will get rid of jiffies altogether and will use
getboottime() instead of do_gettimeofday() for panel_power_cycle_delay.
Does this make sense?
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Wednesday, December 16, 2015 2:56 AM
To: Kumar, Abhay
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
On Tue, Dec 15, 2015 at 02:16:38PM -0800, abhay.ku...@intel.com wrote:
> From: Abhay Kumar
>
> Make resume codepath not to wait for panel_power_cycle_delay(t11_t12)
> if this time is already spent in suspend/poweron time.
>
> Signed-off-by: Abhay Kumar
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 3 +++
> drivers/gpu/drm/i915/intel_dp.c | 18 ++
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> 3 files changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index f00a3c9..d2a5a89 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder
> *intel_encoder)
> intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> intel_edp_panel_vdd_on(intel_dp);
> intel_edp_panel_off(intel_dp);
> +
> + /* storing panel power off time */
> + do_gettimeofday(&intel_dp->panel_power_off_timestamp);
I think what we want to use is CLOCK_BOOTTIME. It's like MONOTONIC, except it
counts the suspend time too.
Initially I figured we'd use REALTIME, and only do the adjustment around
suspend/resume. But actually BOOTTIME should be perfectly safe to use all the
time (changing the system time doesn't affect it). So maybe we just want to
convert the power cycle delay handling entirely over to using the BOOTTIME
clock instead of jiffies?
> }
>
> if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) diff --git
> a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0f1eb96..1ca01b1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2032,6 +2032,9 @@ static void edp_panel_on(struct intel_dp *intel_dp)
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 pp;
> i915_reg_t pp_ctrl_reg;
> + u32 panel_power_off_duration;
> + u32 temp_power_cycle_delay;
> +
>
> lockdep_assert_held(&dev_priv->pps_mutex);
>
> @@ -2045,8 +2048,22 @@ static void edp_panel_on(struct intel_dp *intel_dp)
>"eDP port %c panel power already on\n",
>port_name(dp_to_dig_port(intel_dp)->port)))
> return;
> + /* taking the diffrence of currrent time and panel power off time
> +and then make panel to wait for T12 if needed */
> + do_gettimeofday(&intel_dp->panel_power_on_timestamp);
> +
> + panel_power_off_duration =
> (intel_dp->panel_power_on_timestamp.tv_sec-intel_dp->panel_power_off_timestamp.tv_sec)
> * 100 +
> intel_dp->panel_power_on_timestamp.tv_usec-intel_dp->panel_power_off_timestamp.tv_usec;
> + panel_power_off_duration = panel_power_off_duration / 1000 ;
> + temp_power_cycle_delay = intel_dp->panel_power_cycle_delay;
> +
> + if(panel_power_off_duration >= intel_dp->panel_power_cycle_delay) {
> + intel_dp->panel_power_cycle_delay = 0;
> + } else {
> + intel_dp->panel_power_cycle_delay =
> intel_dp->panel_power_cycle_delay - panel_power_off_duration;
> + }
>
> wait_panel_power_cycle(intel_dp);
> + intel_dp->panel_power_cycle_delay = temp_power_cycle_delay;
>
> pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
> pp = ironlake_get_pp_control(intel_dp);
> @@ -5127,6 +5144,7 @@ static void intel_dp_init_panel_power_timestamps(struct
> intel_dp *intel_dp)
> intel_dp->last_power_cycle = jiffies;
> intel_dp->last_power_on = jiffies;
> intel_dp->last_backlight_off = jiffies;
> + do_gettimeofday(&intel_dp->panel_power_off_timestamp);
> }
>
> static void
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 76dfa28..66ed2cb 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -769,6 +769,8 @@ struct intel_dp {
> unsigned long last_power_cycle;
> unsigned long last_power_on;
> unsigned long last_backlight_off;
> + struct timeval panel_power_off_timestamp;
> + struct timeval panel_power_on_timestamp;
>
> struct notifier_block edp_notifier;
>
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
Is this something close to what we wanted to optimize for edp resume time and
using wall clock.
-Original Message-
From: Kumar, Abhay
Sent: Tuesday, December 15, 2015 2:17 PM
To: Intel-gfx@lists.freedesktop.org
Cc: Kumar, Abhay
Subject: [PATCH] drm/i915: edp resume/On time optimization.
From: Abhay Kumar
Make resume codepath not to wait for panel_power_cycle_delay(t11_t12) if this
time is already spent in suspend/poweron time.
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_ddi.c | 3 +++ drivers/gpu/drm/i915/intel_dp.c |
18 ++ drivers/gpu/drm/i915/intel_drv.h | 2 ++
3 files changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f00a3c9..d2a5a89 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder
*intel_encoder)
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_edp_panel_vdd_on(intel_dp);
intel_edp_panel_off(intel_dp);
+
+ /* storing panel power off time */
+ do_gettimeofday(&intel_dp->panel_power_off_timestamp);
}
if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) diff --git
a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index
0f1eb96..1ca01b1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2032,6 +2032,9 @@ static void edp_panel_on(struct intel_dp *intel_dp)
struct drm_i915_private *dev_priv = dev->dev_private;
u32 pp;
i915_reg_t pp_ctrl_reg;
+ u32 panel_power_off_duration;
+ u32 temp_power_cycle_delay;
+
lockdep_assert_held(&dev_priv->pps_mutex);
@@ -2045,8 +2048,22 @@ static void edp_panel_on(struct intel_dp *intel_dp)
"eDP port %c panel power already on\n",
port_name(dp_to_dig_port(intel_dp)->port)))
return;
+ /* taking the diffrence of currrent time and panel power off time
+ and then make panel to wait for T12 if needed */
+ do_gettimeofday(&intel_dp->panel_power_on_timestamp);
+
+ panel_power_off_duration =
(intel_dp->panel_power_on_timestamp.tv_sec-intel_dp->panel_power_off_timestamp.tv_sec)
* 100 +
intel_dp->panel_power_on_timestamp.tv_usec-intel_dp->panel_power_off_timestamp.tv_usec;
+ panel_power_off_duration = panel_power_off_duration / 1000 ;
+ temp_power_cycle_delay = intel_dp->panel_power_cycle_delay;
+
+ if(panel_power_off_duration >= intel_dp->panel_power_cycle_delay) {
+ intel_dp->panel_power_cycle_delay = 0;
+ } else {
+ intel_dp->panel_power_cycle_delay =
intel_dp->panel_power_cycle_delay - panel_power_off_duration;
+ }
wait_panel_power_cycle(intel_dp);
+ intel_dp->panel_power_cycle_delay = temp_power_cycle_delay;
pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
pp = ironlake_get_pp_control(intel_dp);
@@ -5127,6 +5144,7 @@ static void intel_dp_init_panel_power_timestamps(struct
intel_dp *intel_dp)
intel_dp->last_power_cycle = jiffies;
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies;
+ do_gettimeofday(&intel_dp->panel_power_off_timestamp);
}
static void
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 76dfa28..66ed2cb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -769,6 +769,8 @@ struct intel_dp {
unsigned long last_power_cycle;
unsigned long last_power_on;
unsigned long last_backlight_off;
+ struct timeval panel_power_off_timestamp;
+ struct timeval panel_power_on_timestamp;
struct notifier_block edp_notifier;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Suspend resume timing optimization.
This is for Chrome os perspective which is using upstream kernel. We have S3 suspend and resume time limit of 1sec. Currently suspend of i915 driver takes 280ms and resume takes 600ms. To make resume faster and also follow edp spec this patch will move Half of t12 time from resume path to suspend path. Will take care of comment in next patchset. [cid:image001.png@01D130F9.DFFC0300] -Original Message- From: Paulo Zanoni [mailto:przan...@gmail.com] Sent: Monday, December 7, 2015 12:52 PM To: Kumar, Abhay Cc: Intel Graphics Development Subject: Re: [Intel-gfx] [PATCH] drm/i915: Suspend resume timing optimization. 2015-12-07 18:28 GMT-02:00 mailto:abhay.ku...@intel.com>>: > From: Abhay Kumar mailto:abhay.ku...@intel.com>> > > Moving 250ms from T12 timing to suspend path so that resume path will > be faster. Can you please elaborate more on your motivation for this patch? I'm a little confused. You're trying to make resume faster by making suspend slower? What are your main arguments for this? > > Signed-off-by: Abhay Kumar > mailto:abhay.ku...@intel.com>> > --- > drivers/gpu/drm/i915/intel_ddi.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 7f618cf..2679c9e 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2389,6 +2389,12 @@ static void intel_ddi_post_disable(struct > intel_encoder *intel_encoder) Funcion intel_ddi_post_disable() doesn't only run on suspend situations, yet your commit message suggests you're optimizing suspend. Maybe this commit makes non-suspend modesets slower because now we need to wait the panel power cycle earlier? Have you measured the possible downsides? > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); > intel_edp_panel_vdd_on(intel_dp); > intel_edp_panel_off(intel_dp); > + > + /* Give additional delay of 250 ms so that resume time will > + be faster and also meets T12 delay. > + */ The comment says 250ms, but the code doesn't. Also, there's a missing '*' char in the comment. > + wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle, > + > + (intel_dp->panel_power_cycle_delay/2)); Why wait half the panel power cycle? Why did you choose exactly this value? Thanks, Paulo > } > > if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) > -- > 1.9.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org<mailto:Intel-gfx@lists.freedesktop.org> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/chv: Recomputing CHV watermark.
We received this 23usec number from the power team and same is being used by
other BSW products. With this number we are getting 78% SRR
From current 8%.
Regards,
Abhay Kumar
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Tuesday, May 12, 2015 11:17 AM
To: Kumar, Abhay
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/chv: Recomputing CHV watermark.
On Mon, May 11, 2015 at 04:27:40PM -0700, abhay.ku...@intel.com wrote:
> From: Abhay
>
> Current WM calculation is causing regression on SR residency.
> Recomputing WM using new formula as provided by VPG
Well, really it's the same formula that's been in use since forever for
watermarks. I think we have at least a couple of copies if that formula in the
wm code (should unify a bit at some point).
However to get DDR DVFS enabled we'll need to go quite a bit beyond this. In
the meantime I suppose we might try to just use the old formula with some kind
of pessimistic latency value (not sure where the 23usec came from, last time I
heard it was 33usec for DDR DVFS, and 11usec for PM5). But that would probably
mean we'd already need to hardcode the drain latency to a small value. In fact
the current drain latency values we're using still seem insufficient in some
cases.
>
> Change-Id: I9dbd6a7b70c84454748dee41738130934230b763
> Signed-off-by: Abhay
> ---
> drivers/gpu/drm/i915/intel_pm.c | 26 +++---
> 1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c index a960cdd..01a5d79 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1546,6 +1546,13 @@ static int vlv_compute_wm(struct intel_crtc *crtc,
> int fifo_size)
> {
> int clock, entries, pixel_size;
> + uint32_t line_time = 0,buffer_wm = 0;
> +
> + /* using memory DVFS latency of 23usec */
> + int latency = 23000;
> +
> + int htotal = crtc->config.adjusted_mode.crtc_htotal;
> + int hdisplay = to_intel_crtc(crtc)->config.pipe_src_w;
>
> /*
>* FIXME the plane might have an fb
> @@ -1557,18 +1564,15 @@ static int vlv_compute_wm(struct intel_crtc *crtc,
> pixel_size = drm_format_plane_cpp(plane->base.fb->pixel_format, 0);
> clock = crtc->config.adjusted_mode.crtc_clock;
>
> - entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
> + line_time = max(htotal * 1000 / clock, 1);
> +
> + /* Max FIFO size */
> + fifo_size = 96 * 1024;
> +
> + buffer_wm = (fifo_size - (((latency /line_time /1000) + 1 ) *
> + hdisplay * pixel_size));
> + return buffer_wm;
>
> - /*
> - * Set up the watermark such that we don't start issuing memory
> - * requests until we are within PND's max deadline value (256us).
> - * Idea being to be idle as long as possible while still taking
> - * advatange of PND's deadline scheduling. The limit of 8
> - * cachelines (used when the FIFO will anyway drain in less time
> - * than 256us) should match what we would be done if trickle
> - * feed were enabled.
> - */
> - return fifo_size - clamp(DIV_ROUND_UP(256 * entries, 64), 0, fifo_size
> - 8);
> }
>
> static bool vlv_compute_sr_wm(struct drm_device *dev,
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
