Re: [Intel-gfx] [PATCH v4] drm/i915: Fix assert_plane() warning on bootup with external display

2018-06-27 Thread Ville Syrjälä
On Wed, Jun 27, 2018 at 10:39:56AM -0700, Radhakrishna Sripada wrote:
> On Tue, Jun 26, 2018 at 05:55:59PM -0700, Azhar Shaikh wrote:
> > On KBL, WHL RVPs, booting up with an external display connected, triggers
> > below warning, when the BiOS brings up the external display too.
> > This warning is not seen during hotplug.
> > 
> > [3.615226] [ cut here ]
> > [3.619829] plane 1A assertion failure (expected on, current off)
> > [3.632039] WARNING: CPU: 2 PID: 354 at 
> > drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb
> > [3.633920] iwlwifi :00:14.3: loaded firmware version 38.c0e03d94.0 
> > op_mode iwlmvm
> > [3.647157] Modules linked in: iwlwifi cfg80211 btusb btrtl btbcm 
> > btintel bluetooth ecdh_generic
> > [3.647163] CPU: 2 PID: 354 Comm: frecon Not tainted 
> > 4.17.0-rc7-50176-g655af12d39c2 #3
> > [3.647165] Hardware name: Intel Corporation CoffeeLake Client 
> > Platform/WhiskeyLake U DDR4 ERB, BIOS CNLSFWR1.R00.X140.B00.1804040304 
> > 04/04/2018
> > [3.684509] RIP: 0010:assert_plane+0x71/0xbb
> > [3.764451] Call Trace:
> > [3.766888]  intel_atomic_commit_tail+0xa97/0xb77
> > [3.771569]  intel_atomic_commit+0x26a/0x279
> > [3.771572]  drm_atomic_helper_set_config+0x5c/0x76
> > [3.780670]  __drm_mode_set_config_internal+0x66/0x109
> > [3.780672]  drm_mode_setcrtc+0x4c9/0x5cc
> > [3.780674]  ? drm_mode_getcrtc+0x162/0x162
> > [3.789774]  ? drm_mode_getcrtc+0x162/0x162
> > [3.798108]  drm_ioctl_kernel+0x8d/0xe4
> > [3.801926]  drm_ioctl+0x27d/0x368
> > [3.805311]  ? drm_mode_getcrtc+0x162/0x162
> > [3.805314]  ? selinux_file_ioctl+0x14e/0x199
> > [3.805317]  vfs_ioctl+0x21/0x2f
> > [3.813812]  do_vfs_ioctl+0x491/0x4b4
> > [3.813813]  ? security_file_ioctl+0x37/0x4b
> > [3.813816]  ksys_ioctl+0x55/0x75
> > [3.820672]  __x64_sys_ioctl+0x1a/0x1e
> > [3.820674]  do_syscall_64+0x51/0x5f
> > [3.820678]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > [3.828221] RIP: 0033:0x7b5e04953967
> > [3.835504] RSP: 002b:7fff2eafb6f8 EFLAGS: 0246 ORIG_RAX: 
> > 0010
> > [3.835505] RAX: ffda RBX: 0002 RCX: 
> > 7b5e04953967
> > [3.835505] RDX: 7fff2eafb730 RSI: c06864a2 RDI: 
> > 000f
> > [3.835506] RBP: 7fff2eafb720 R08:  R09: 
> > 
> > [3.835507] R10: 0070 R11: 0246 R12: 
> > 000f
> > [3.879988] R13: 56bc9dd7d210 R14: 7fff2eafb730 R15: 
> > c06864a2
> > [3.887081] Code: 48 c7 c7 06 71 a5 be 84 c0 48 c7 c2 06 fd a3 be 48 89 
> > f9 48 0f 44 ca 84 db 48 0f 45 d7 48 c7 c7 df d3 a4 be 31 c0 e8 af a0 c0 ff 
> > <0f> 0b eb 2b 48 c7 c7 06 fd a3 be 84 c0 48 c7 c2 06 71 a5 be 48
> > [3.905845] WARNING: CPU: 2 PID: 354 at 
> > drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb
> > [3.920964] ---[ end trace dac692f4ac46391a ]---
> > 
> > The warning is seen when mode_setcrtc() is called for pipeB
> > during bootup and before we get a mode_setcrtc() for pipeA,
> > while doing update_crtcs() in intel_atomic_commit_tail().
> > Now since, plane1A is still active after commit, update_crtcs()
> > is done for pipeA and eventually update_plane() for plane1A.
> > 
> > intel_plane_state->ctl for plane1A is not updated since set_modecrtc() is
> > called for pipeB. So intel_plane_state->ctl for plane 1A will be 0x0.
> > So doing an update_plane() for plane1A, will result in clearing
> > PLANE_CTL_ENABLE bit, and hence the warning.
> > 
> > To fix this warning, force all active planes to recompute their states
> > in probe.
> > 
> > Signed-off-by: Azhar Shaikh 
> > ---
> > Changes in v4:
> > - Handle locking in intel_initial_commit()
> > - Move the for loop inside intel_initial_commit() so that
> >   drm_atomic_commit() is called only once
> > - Call intel_initial_commit() only for more than one active crtc on boot.
> > - Save the return value of intel_initial_commit() and print a message in
> >   case of an error
> > 
> > Changes in v3:
> > - Add comments
> > 
> > Changes in v2:
> > - Force all planes to recompute their states.(Ville Syrjälä)
> > - Update the commit message
> 
> Include the changelog in the commit message above Signed-off-by
> > 
> >  drivers/gpu/drm/i915/intel_display.c | 81 
> > +++-
> >  1 file changed, 79 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 3709fa1b6318..40bdb28aa2a5 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15092,12 +15092,76 @@ static void intel_update_fdi_pll_freq(struct 
> > drm_i915_private *dev_priv)
> > DRM_DEBUG_DRIVER("FDI PLL freq=%d\n", dev_priv->fdi_pll_freq);
> >  }
> >  
> > +static int intel_initial_commit(struct drm_device *dev)
> > +{

Re: [Intel-gfx] [PATCH v4] drm/i915: Fix assert_plane() warning on bootup with external display

2018-06-27 Thread Radhakrishna Sripada
On Tue, Jun 26, 2018 at 05:55:59PM -0700, Azhar Shaikh wrote:
> On KBL, WHL RVPs, booting up with an external display connected, triggers
> below warning, when the BiOS brings up the external display too.
> This warning is not seen during hotplug.
> 
> [3.615226] [ cut here ]
> [3.619829] plane 1A assertion failure (expected on, current off)
> [3.632039] WARNING: CPU: 2 PID: 354 at 
> drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb
> [3.633920] iwlwifi :00:14.3: loaded firmware version 38.c0e03d94.0 
> op_mode iwlmvm
> [3.647157] Modules linked in: iwlwifi cfg80211 btusb btrtl btbcm btintel 
> bluetooth ecdh_generic
> [3.647163] CPU: 2 PID: 354 Comm: frecon Not tainted 
> 4.17.0-rc7-50176-g655af12d39c2 #3
> [3.647165] Hardware name: Intel Corporation CoffeeLake Client 
> Platform/WhiskeyLake U DDR4 ERB, BIOS CNLSFWR1.R00.X140.B00.1804040304 
> 04/04/2018
> [3.684509] RIP: 0010:assert_plane+0x71/0xbb
> [3.764451] Call Trace:
> [3.766888]  intel_atomic_commit_tail+0xa97/0xb77
> [3.771569]  intel_atomic_commit+0x26a/0x279
> [3.771572]  drm_atomic_helper_set_config+0x5c/0x76
> [3.780670]  __drm_mode_set_config_internal+0x66/0x109
> [3.780672]  drm_mode_setcrtc+0x4c9/0x5cc
> [3.780674]  ? drm_mode_getcrtc+0x162/0x162
> [3.789774]  ? drm_mode_getcrtc+0x162/0x162
> [3.798108]  drm_ioctl_kernel+0x8d/0xe4
> [3.801926]  drm_ioctl+0x27d/0x368
> [3.805311]  ? drm_mode_getcrtc+0x162/0x162
> [3.805314]  ? selinux_file_ioctl+0x14e/0x199
> [3.805317]  vfs_ioctl+0x21/0x2f
> [3.813812]  do_vfs_ioctl+0x491/0x4b4
> [3.813813]  ? security_file_ioctl+0x37/0x4b
> [3.813816]  ksys_ioctl+0x55/0x75
> [3.820672]  __x64_sys_ioctl+0x1a/0x1e
> [3.820674]  do_syscall_64+0x51/0x5f
> [3.820678]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [3.828221] RIP: 0033:0x7b5e04953967
> [3.835504] RSP: 002b:7fff2eafb6f8 EFLAGS: 0246 ORIG_RAX: 
> 0010
> [3.835505] RAX: ffda RBX: 0002 RCX: 
> 7b5e04953967
> [3.835505] RDX: 7fff2eafb730 RSI: c06864a2 RDI: 
> 000f
> [3.835506] RBP: 7fff2eafb720 R08:  R09: 
> 
> [3.835507] R10: 0070 R11: 0246 R12: 
> 000f
> [3.879988] R13: 56bc9dd7d210 R14: 7fff2eafb730 R15: 
> c06864a2
> [3.887081] Code: 48 c7 c7 06 71 a5 be 84 c0 48 c7 c2 06 fd a3 be 48 89 f9 
> 48 0f 44 ca 84 db 48 0f 45 d7 48 c7 c7 df d3 a4 be 31 c0 e8 af a0 c0 ff <0f> 
> 0b eb 2b 48 c7 c7 06 fd a3 be 84 c0 48 c7 c2 06 71 a5 be 48
> [3.905845] WARNING: CPU: 2 PID: 354 at 
> drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb
> [3.920964] ---[ end trace dac692f4ac46391a ]---
> 
> The warning is seen when mode_setcrtc() is called for pipeB
> during bootup and before we get a mode_setcrtc() for pipeA,
> while doing update_crtcs() in intel_atomic_commit_tail().
> Now since, plane1A is still active after commit, update_crtcs()
> is done for pipeA and eventually update_plane() for plane1A.
> 
> intel_plane_state->ctl for plane1A is not updated since set_modecrtc() is
> called for pipeB. So intel_plane_state->ctl for plane 1A will be 0x0.
> So doing an update_plane() for plane1A, will result in clearing
> PLANE_CTL_ENABLE bit, and hence the warning.
> 
> To fix this warning, force all active planes to recompute their states
> in probe.
> 
> Signed-off-by: Azhar Shaikh 
> ---
> Changes in v4:
> - Handle locking in intel_initial_commit()
> - Move the for loop inside intel_initial_commit() so that
>   drm_atomic_commit() is called only once
> - Call intel_initial_commit() only for more than one active crtc on boot.
> - Save the return value of intel_initial_commit() and print a message in
>   case of an error
> 
> Changes in v3:
> - Add comments
> 
> Changes in v2:
> - Force all planes to recompute their states.(Ville Syrjälä)
> - Update the commit message

Include the changelog in the commit message above Signed-off-by
> 
>  drivers/gpu/drm/i915/intel_display.c | 81 
> +++-
>  1 file changed, 79 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 3709fa1b6318..40bdb28aa2a5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15092,12 +15092,76 @@ static void intel_update_fdi_pll_freq(struct 
> drm_i915_private *dev_priv)
>   DRM_DEBUG_DRIVER("FDI PLL freq=%d\n", dev_priv->fdi_pll_freq);
>  }
>  
> +static int intel_initial_commit(struct drm_device *dev)
> +{
> + struct drm_atomic_state *state = NULL;
> + struct drm_modeset_acquire_ctx ctx;
> + struct drm_crtc_state *crtc_state;
> + struct intel_crtc *intel_crtc;
> + int ret = 0;
> +
> + drm_modeset_acquire_init(&ctx, 0);
> +
> +retry:
> + state = drm_atomic_

[Intel-gfx] [PATCH v4] drm/i915: Fix assert_plane() warning on bootup with external display

2018-06-26 Thread Azhar Shaikh
On KBL, WHL RVPs, booting up with an external display connected, triggers
below warning, when the BiOS brings up the external display too.
This warning is not seen during hotplug.

[3.615226] [ cut here ]
[3.619829] plane 1A assertion failure (expected on, current off)
[3.632039] WARNING: CPU: 2 PID: 354 at 
drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb
[3.633920] iwlwifi :00:14.3: loaded firmware version 38.c0e03d94.0 
op_mode iwlmvm
[3.647157] Modules linked in: iwlwifi cfg80211 btusb btrtl btbcm btintel 
bluetooth ecdh_generic
[3.647163] CPU: 2 PID: 354 Comm: frecon Not tainted 
4.17.0-rc7-50176-g655af12d39c2 #3
[3.647165] Hardware name: Intel Corporation CoffeeLake Client 
Platform/WhiskeyLake U DDR4 ERB, BIOS CNLSFWR1.R00.X140.B00.1804040304 
04/04/2018
[3.684509] RIP: 0010:assert_plane+0x71/0xbb
[3.764451] Call Trace:
[3.766888]  intel_atomic_commit_tail+0xa97/0xb77
[3.771569]  intel_atomic_commit+0x26a/0x279
[3.771572]  drm_atomic_helper_set_config+0x5c/0x76
[3.780670]  __drm_mode_set_config_internal+0x66/0x109
[3.780672]  drm_mode_setcrtc+0x4c9/0x5cc
[3.780674]  ? drm_mode_getcrtc+0x162/0x162
[3.789774]  ? drm_mode_getcrtc+0x162/0x162
[3.798108]  drm_ioctl_kernel+0x8d/0xe4
[3.801926]  drm_ioctl+0x27d/0x368
[3.805311]  ? drm_mode_getcrtc+0x162/0x162
[3.805314]  ? selinux_file_ioctl+0x14e/0x199
[3.805317]  vfs_ioctl+0x21/0x2f
[3.813812]  do_vfs_ioctl+0x491/0x4b4
[3.813813]  ? security_file_ioctl+0x37/0x4b
[3.813816]  ksys_ioctl+0x55/0x75
[3.820672]  __x64_sys_ioctl+0x1a/0x1e
[3.820674]  do_syscall_64+0x51/0x5f
[3.820678]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[3.828221] RIP: 0033:0x7b5e04953967
[3.835504] RSP: 002b:7fff2eafb6f8 EFLAGS: 0246 ORIG_RAX: 
0010
[3.835505] RAX: ffda RBX: 0002 RCX: 7b5e04953967
[3.835505] RDX: 7fff2eafb730 RSI: c06864a2 RDI: 000f
[3.835506] RBP: 7fff2eafb720 R08:  R09: 
[3.835507] R10: 0070 R11: 0246 R12: 000f
[3.879988] R13: 56bc9dd7d210 R14: 7fff2eafb730 R15: c06864a2
[3.887081] Code: 48 c7 c7 06 71 a5 be 84 c0 48 c7 c2 06 fd a3 be 48 89 f9 
48 0f 44 ca 84 db 48 0f 45 d7 48 c7 c7 df d3 a4 be 31 c0 e8 af a0 c0 ff <0f> 0b 
eb 2b 48 c7 c7 06 fd a3 be 84 c0 48 c7 c2 06 71 a5 be 48
[3.905845] WARNING: CPU: 2 PID: 354 at 
drivers/gpu/drm/i915/intel_display.c:1294 assert_plane+0x71/0xbb
[3.920964] ---[ end trace dac692f4ac46391a ]---

The warning is seen when mode_setcrtc() is called for pipeB
during bootup and before we get a mode_setcrtc() for pipeA,
while doing update_crtcs() in intel_atomic_commit_tail().
Now since, plane1A is still active after commit, update_crtcs()
is done for pipeA and eventually update_plane() for plane1A.

intel_plane_state->ctl for plane1A is not updated since set_modecrtc() is
called for pipeB. So intel_plane_state->ctl for plane 1A will be 0x0.
So doing an update_plane() for plane1A, will result in clearing
PLANE_CTL_ENABLE bit, and hence the warning.

To fix this warning, force all active planes to recompute their states
in probe.

Signed-off-by: Azhar Shaikh 
---
Changes in v4:
- Handle locking in intel_initial_commit()
- Move the for loop inside intel_initial_commit() so that
  drm_atomic_commit() is called only once
- Call intel_initial_commit() only for more than one active crtc on boot.
- Save the return value of intel_initial_commit() and print a message in
  case of an error

Changes in v3:
- Add comments

Changes in v2:
- Force all planes to recompute their states.(Ville Syrjälä)
- Update the commit message

 drivers/gpu/drm/i915/intel_display.c | 81 +++-
 1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3709fa1b6318..40bdb28aa2a5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15092,12 +15092,76 @@ static void intel_update_fdi_pll_freq(struct 
drm_i915_private *dev_priv)
DRM_DEBUG_DRIVER("FDI PLL freq=%d\n", dev_priv->fdi_pll_freq);
 }
 
+static int intel_initial_commit(struct drm_device *dev)
+{
+   struct drm_atomic_state *state = NULL;
+   struct drm_modeset_acquire_ctx ctx;
+   struct drm_crtc_state *crtc_state;
+   struct intel_crtc *intel_crtc;
+   int ret = 0;
+
+   drm_modeset_acquire_init(&ctx, 0);
+
+retry:
+   state = drm_atomic_state_alloc(dev);
+   if (!state)
+   goto unlock;
+
+   state->acquire_ctx = &ctx;
+
+   ret = drm_modeset_lock_all_ctx(dev, &ctx);
+   if (ret == -EDEADLK) {
+   drm_atomic_state_clear(state);
+   drm_modeset_backoff(&ctx);
+   goto retry;
+   } else if (WARN_ON(ret)) {
+