diff mbox

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

Message ID 1530060959-36219-1-git-send-email-azhar.shaikh@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Azhar Shaikh June 27, 2018, 12:55 a.m. UTC
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 0000: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:00007fff2eafb6f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[    3.835505] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007b5e04953967
[    3.835505] RDX: 00007fff2eafb730 RSI: 00000000c06864a2 RDI: 000000000000000f
[    3.835506] RBP: 00007fff2eafb720 R08: 0000000000000000 R09: 0000000000000000
[    3.835507] R10: 0000000000000070 R11: 0000000000000246 R12: 000000000000000f
[    3.879988] R13: 000056bc9dd7d210 R14: 00007fff2eafb730 R15: 00000000c06864a2
[    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 <azhar.shaikh@intel.com>
---
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(-)

Comments

Sripada, Radhakrishna June 27, 2018, 5:39 p.m. UTC | #1
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 0000: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:00007fff2eafb6f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [    3.835505] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007b5e04953967
> [    3.835505] RDX: 00007fff2eafb730 RSI: 00000000c06864a2 RDI: 000000000000000f
> [    3.835506] RBP: 00007fff2eafb720 R08: 0000000000000000 R09: 0000000000000000
> [    3.835507] R10: 0000000000000070 R11: 0000000000000246 R12: 000000000000000f
> [    3.879988] R13: 000056bc9dd7d210 R14: 00007fff2eafb730 R15: 00000000c06864a2
> [    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 <azhar.shaikh@intel.com>
> ---
> 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_state_alloc(dev);
> +	if (!state)
> +		goto unlock;
Locks are not acquired. It should not call unlock. Use another goto to call 
drm_modeset_acquire_fini

> +
> +	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)) {
> +		goto unlock;
Same as above. Also call drm_atomic_state_put to free the allocated state.

> +	}
> +
> +	for_each_intel_crtc(dev, intel_crtc) {
> +		struct drm_crtc *crtc = &intel_crtc->base;
> +
> +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +		if (IS_ERR(crtc_state)) {
> +			ret = PTR_ERR(crtc_state);
> +			goto out;
> +		}
> +
> +		if (crtc_state->active) {
> +			ret = drm_atomic_add_affected_planes(state, crtc);
> +			if (ret)
> +				goto out;
> +		}
> +	}
> +
> +	ret = drm_atomic_commit(state);
> +	if (ret == -EDEADLK) {
> +		drm_atomic_state_clear(state);
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +
> +out:
> +	if (state) {
> +		drm_atomic_state_put(state);
> +		state = NULL;
> +	}
> +
> +unlock:
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);

Re-order freeing the objects in this case drm_modeset_drop_locks followed by 
drm_atomic_state_put and drm_modeset_acquire_fini

Regards,
Radhakrishna(RK) Sripada
> +
> +	return ret;
> +}
> +
>  int intel_modeset_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
>  	enum pipe pipe;
>  	struct intel_crtc *crtc;
> +	u16 num_active_crtcs;
> +	int ret;
>  
>  	dev_priv->modeset_wq = alloc_ordered_workqueue("i915_modeset", 0);
>  
> @@ -15172,8 +15236,6 @@ int intel_modeset_init(struct drm_device *dev)
>  		      INTEL_INFO(dev_priv)->num_pipes > 1 ? "s" : "");
>  
>  	for_each_pipe(dev_priv, pipe) {
> -		int ret;
> -
>  		ret = intel_crtc_init(dev_priv, pipe);
>  		if (ret) {
>  			drm_mode_config_cleanup(dev);
> @@ -15203,6 +15265,8 @@ int intel_modeset_init(struct drm_device *dev)
>  
>  		if (!crtc->active)
>  			continue;
> +		else
> +			num_active_crtcs++;
>  
>  		/*
>  		 * Note that reserving the BIOS fb up front prevents us
> @@ -15222,6 +15286,19 @@ int intel_modeset_init(struct drm_device *dev)
>  	}
>  
>  	/*
> +	 * Only in case of more than one active crtc in probe, force
> +	 * all active planes to recompute their states. So that on
> +	 * mode_setcrtc after probe, all the intel_plane_state variables
> +	 * are already calculated and there is no assert_plane warnings
> +	 * during bootup.
> +	 */
> +	if (num_active_crtcs > 1) {
> +		ret = intel_initial_commit(dev);
> +		if (ret)
> +			DRM_DEBUG_KMS("Initial commit in probe failed.\n");
> +	}
> +
> +	/*
>  	 * Make sure hardware watermarks really match the state we read out.
>  	 * Note that we need to do this after reconstructing the BIOS fb's
>  	 * since the watermark calculation done here will use pstate->fb.
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä June 27, 2018, 6:08 p.m. UTC | #2
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 0000: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:00007fff2eafb6f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > [    3.835505] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007b5e04953967
> > [    3.835505] RDX: 00007fff2eafb730 RSI: 00000000c06864a2 RDI: 000000000000000f
> > [    3.835506] RBP: 00007fff2eafb720 R08: 0000000000000000 R09: 0000000000000000
> > [    3.835507] R10: 0000000000000070 R11: 0000000000000246 R12: 000000000000000f
> > [    3.879988] R13: 000056bc9dd7d210 R14: 00007fff2eafb730 R15: 00000000c06864a2
> > [    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 <azhar.shaikh@intel.com>
> > ---
> > 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_state_alloc(dev);
> > +	if (!state)
> > +		goto unlock;
> Locks are not acquired. It should not call unlock. Use another goto to call 
> drm_modeset_acquire_fini

Easier to just do the state_alloc() before drm_modeset_acquire_init().
Then you can just return directly if the alloc fails.

> 
> > +
> > +	state->acquire_ctx = &ctx;
> > +
> > +	ret = drm_modeset_lock_all_ctx(dev, &ctx);

lock_all() not required as everything called later will grab the
required locks on its own. I guess with lock_all() you could simplify
the backoff thing to a simple loop, but that would only work if there
are no futher driver private modeset locks taken later. We don't have
any currently so this is not a problem. And we do have some lock_all()
uses elsewhere which wouldn't be quite correct either.

But probably clearner to not use lock_all() at all and avoid these
headaches.

> > +	if (ret == -EDEADLK) {
> > +		drm_atomic_state_clear(state);
> > +		drm_modeset_backoff(&ctx);
> > +		goto retry;
> > +	} else if (WARN_ON(ret)) {
> > +		goto unlock;
> Same as above. Also call drm_atomic_state_put to free the allocated state.
> 
> > +	}
> > +
> > +	for_each_intel_crtc(dev, intel_crtc) {
> > +		struct drm_crtc *crtc = &intel_crtc->base;
> > +
> > +		crtc_state = drm_atomic_get_crtc_state(state, crtc);
> > +		if (IS_ERR(crtc_state)) {
> > +			ret = PTR_ERR(crtc_state);
> > +			goto out;
> > +		}
> > +
> > +		if (crtc_state->active) {
> > +			ret = drm_atomic_add_affected_planes(state, crtc);
> > +			if (ret)
> > +				goto out;
> > +		}
> > +	}
> > +
> > +	ret = drm_atomic_commit(state);
> > +	if (ret == -EDEADLK) {
> > +		drm_atomic_state_clear(state);
> > +		drm_modeset_backoff(&ctx);
> > +		goto retry;
> > +	}
> > +
> > +out:
> > +	if (state) {
> > +		drm_atomic_state_put(state);
> > +		state = NULL;
> > +	}
> > +
> > +unlock:
> > +	drm_modeset_drop_locks(&ctx);
> > +	drm_modeset_acquire_fini(&ctx);
> 
> Re-order freeing the objects in this case drm_modeset_drop_locks followed by 
> drm_atomic_state_put and drm_modeset_acquire_fini
> 
> Regards,
> Radhakrishna(RK) Sripada
> > +
> > +	return ret;
> > +}
> > +
> >  int intel_modeset_init(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
> >  	enum pipe pipe;
> >  	struct intel_crtc *crtc;
> > +	u16 num_active_crtcs;
> > +	int ret;
> >  
> >  	dev_priv->modeset_wq = alloc_ordered_workqueue("i915_modeset", 0);
> >  
> > @@ -15172,8 +15236,6 @@ int intel_modeset_init(struct drm_device *dev)
> >  		      INTEL_INFO(dev_priv)->num_pipes > 1 ? "s" : "");
> >  
> >  	for_each_pipe(dev_priv, pipe) {
> > -		int ret;
> > -
> >  		ret = intel_crtc_init(dev_priv, pipe);
> >  		if (ret) {
> >  			drm_mode_config_cleanup(dev);
> > @@ -15203,6 +15265,8 @@ int intel_modeset_init(struct drm_device *dev)
> >  
> >  		if (!crtc->active)
> >  			continue;
> > +		else
> > +			num_active_crtcs++;
> >  
> >  		/*
> >  		 * Note that reserving the BIOS fb up front prevents us
> > @@ -15222,6 +15286,19 @@ int intel_modeset_init(struct drm_device *dev)
> >  	}
> >  
> >  	/*
> > +	 * Only in case of more than one active crtc in probe, force
> > +	 * all active planes to recompute their states. So that on
> > +	 * mode_setcrtc after probe, all the intel_plane_state variables
> > +	 * are already calculated and there is no assert_plane warnings
> > +	 * during bootup.
> > +	 */
> > +	if (num_active_crtcs > 1) {
> > +		ret = intel_initial_commit(dev);
> > +		if (ret)
> > +			DRM_DEBUG_KMS("Initial commit in probe failed.\n");
> > +	}
> > +
> > +	/*
> >  	 * Make sure hardware watermarks really match the state we read out.
> >  	 * Note that we need to do this after reconstructing the BIOS fb's
> >  	 * since the watermark calculation done here will use pstate->fb.
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > 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
diff mbox

Patch

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)) {
+		goto unlock;
+	}
+
+	for_each_intel_crtc(dev, intel_crtc) {
+		struct drm_crtc *crtc = &intel_crtc->base;
+
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(crtc_state)) {
+			ret = PTR_ERR(crtc_state);
+			goto out;
+		}
+
+		if (crtc_state->active) {
+			ret = drm_atomic_add_affected_planes(state, crtc);
+			if (ret)
+				goto out;
+		}
+	}
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK) {
+		drm_atomic_state_clear(state);
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+
+out:
+	if (state) {
+		drm_atomic_state_put(state);
+		state = NULL;
+	}
+
+unlock:
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	return ret;
+}
+
 int intel_modeset_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	enum pipe pipe;
 	struct intel_crtc *crtc;
+	u16 num_active_crtcs;
+	int ret;
 
 	dev_priv->modeset_wq = alloc_ordered_workqueue("i915_modeset", 0);
 
@@ -15172,8 +15236,6 @@  int intel_modeset_init(struct drm_device *dev)
 		      INTEL_INFO(dev_priv)->num_pipes > 1 ? "s" : "");
 
 	for_each_pipe(dev_priv, pipe) {
-		int ret;
-
 		ret = intel_crtc_init(dev_priv, pipe);
 		if (ret) {
 			drm_mode_config_cleanup(dev);
@@ -15203,6 +15265,8 @@  int intel_modeset_init(struct drm_device *dev)
 
 		if (!crtc->active)
 			continue;
+		else
+			num_active_crtcs++;
 
 		/*
 		 * Note that reserving the BIOS fb up front prevents us
@@ -15222,6 +15286,19 @@  int intel_modeset_init(struct drm_device *dev)
 	}
 
 	/*
+	 * Only in case of more than one active crtc in probe, force
+	 * all active planes to recompute their states. So that on
+	 * mode_setcrtc after probe, all the intel_plane_state variables
+	 * are already calculated and there is no assert_plane warnings
+	 * during bootup.
+	 */
+	if (num_active_crtcs > 1) {
+		ret = intel_initial_commit(dev);
+		if (ret)
+			DRM_DEBUG_KMS("Initial commit in probe failed.\n");
+	}
+
+	/*
 	 * Make sure hardware watermarks really match the state we read out.
 	 * Note that we need to do this after reconstructing the BIOS fb's
 	 * since the watermark calculation done here will use pstate->fb.