Re: [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Had some discussion with Daniel on IRC about how we can fix the deadlock.
We couldn't decide upon any solution but I can give it a try.
He suggested to have a work function for ->hot_plug which will get the power domain lock and not allow ->hot_plug if the power_domain is shut.
Then when it calls hpt_init again from power_domain get, ->hot_plug gets called.
But this looks racy, we can try this out.
If there are any other suggestions, please let me know.
I will work on this after 26th Oct.

Regards,
Sonika

-----Original Message-----
From: Sharma, Shashank 
Sent: Monday, October 12, 2015 5:54 PM
To: Ville Syrjälä; Vetter, Daniel
Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Mukherjee, Indranil; Jindal, Sonika
Subject: RE:  [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases

We were debugging this issue, and we could find the root cause:
In function:
Intel_hpd_init()
	|
encoder->hotplug()
	|
display_power_get()
	|
Intel_powe_well_enable()
	|
power_well->ops->enable()
	|
chv_pipe_power_well_enable()
	|
vlv_display_power_well_init()
	|
intel_hdp_init()

This function ends up calling intel_hpd_init, Which is causing the mutex deadlock due to recursion, as we are calling encoder->hotplug() from hpd_init().

Intel_hpd_init()
	|
encoder->hotplug()
	|
display_power_get()

There are two solutions here:
- remove hpd_init() from get_calls
- remove encoder->hotplug() function call from hpd_init() and put it in some other place. We have added this function from encoder_init() for android trees, and it works well there. 

Regards
Shashank
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Ville Syrjälä
Sent: Thursday, October 08, 2015 7:06 PM
To: Jindal, Sonika
Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re:  [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases

On Mon, Oct 05, 2015 at 04:43:14PM +0530, Sonika Jindal wrote:
> For all the encoders, call the hot_plug if it is registered.
> This is required for connected boot and resume cases to generate fake 
> hpd resulting in reading of edid.
> Removing the initial sdvo hot_plug call too so that it will be called 
> just once from this loop.
> 
> Signed-off-by: Sonika Jindal <sonika.jindal@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_hotplug.c |   11 +++++++++++
>  drivers/gpu/drm/i915/intel_sdvo.c    |    1 -
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index 53c0173..eac4757 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -458,6 +458,7 @@ void intel_hpd_init(struct drm_i915_private
> *dev_priv)  {
>  	struct drm_device *dev = dev_priv->dev;
>  	struct drm_mode_config *mode_config = &dev->mode_config;
> +	struct intel_encoder *encoder;
>  	struct drm_connector *connector;
>  	int i;
>  
> @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>  	if (dev_priv->display.hpd_irq_setup)
>  		dev_priv->display.hpd_irq_setup(dev);
>  	spin_unlock_irq(&dev_priv->irq_lock);
> +
> +	/*
> +	 * Connected boot / resume scenarios can't generate new hot plug.
> +	 * So, probe it manually.
> +	 */
> +	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> +			    base.head) {
> +		if (encoder->hot_plug)
> +			encoder->hot_plug(encoder);
> +	}


This breaks the world on CHV

[ 3187.575198] [drm:intel_hdmi_hot_plug] Live status not up!
[ 3187.585154] =============================================
[ 3187.595010] [ INFO: possible recursive locking detected ]
[ 3187.604685] 4.3.0-rc4-bsw+ #2488 Tainted: G     U  W      
[ 3187.614366] ---------------------------------------------
[ 3187.623892] Xorg/32212 is trying to acquire lock:
[ 3187.632635]  (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3187.647492] [ 3187.647492] but task is already holding lock:
[ 3187.661054]  (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3187.675960] [ 3187.675960] other info that might help us debug this:
[ 3187.690459]  Possible unsafe locking scenario:
[ 3187.690459] 
[ 3187.704224]        CPU0
[ 3187.710485]        ----
[ 3187.716711]   lock(&power_domains->lock);
[ 3187.724718]   lock(&power_domains->lock);
[ 3187.732663]
[ 3187.732663]  *** DEADLOCK ***
[ 3187.732663]
[ 3187.749460]  May be due to missing lock nesting notation [ 3187.749460] [ 3187.763833] 5 locks held by Xorg/32212:
[ 3187.771523]  #0:  (drm_global_mutex){+.+.+.}, at: [<ffffffffa02ad5c8>] drm_release+0x3b/0x49b [drm] [ 3187.785216]  #1:  (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffffa02c4d85>] drm_modeset_lock_all+0x54/0xcd [drm] [ 3187.800437]  #2:  (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffffa02c4d8f>] drm_modeset_lock_all+0x5e/0xcd [drm] [ 3187.815488]  #3:  (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffffa02c46b0>] drm_modeset_lock+0x75/0xfc [drm] [ 3187.830094]  #4:  (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3187.845534] [ 3187.845534] stack backtrace:
[ 3187.857685] CPU: 2 PID: 32212 Comm: Xorg Tainted: G     U  W       4.3.0-rc4-bsw+ #2488
[ 3187.870331] Hardware name: Intel Corporation CHERRYVIEW C0 PLATFORM/Braswell CRB, BIOS BRAS.X64.B085.R00.1509110553 09/11/2015 [ 3187.886827]  0000000000000000 ffff880175eff8e0 ffffffff8128d59e ffffffff823f5ee0 [ 3187.898904]  ffff880175eff958 ffffffff810a7a08 0000000000000000 ffff880179d1c5d0 [ 3187.910954]  0000000000000004 0000000000000006 45422a91588a4c3e 0000000000000005 [ 3187.923011] Call Trace:
[ 3187.929451]  [<ffffffff8128d59e>] dump_stack+0x4e/0x79 [ 3187.938912]  [<ffffffff810a7a08>] __lock_acquire+0x7ab/0x12af [ 3187.949027]  [<ffffffff810a8d04>] lock_acquire+0x10e/0x1a9 [ 3187.958859]  [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3187.970476]  [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3187.982011]  [<ffffffff814df4a3>] mutex_lock_nested+0x71/0x346 [ 3187.992167]  [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3188.003655]  [<ffffffff814e20ee>] ? _raw_spin_unlock_irqrestore+0x4b/0x60
[ 3188.014829]  [<ffffffff81364565>] ? __pm_runtime_resume+0x71/0x7e [ 3188.025269]  [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3188.036544]  [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3188.047968]  [<ffffffffa03e8cc4>] intel_hdmi_set_edid+0x3f/0xd6 [i915] [ 3188.058766]  [<ffffffffa03e8e1a>] intel_hdmi_hot_plug+0xbf/0xfb [i915] [ 3188.069484]  [<ffffffffa03c9e78>] intel_hpd_init+0xfa/0x10b [i915] [ 3188.079753]  [<ffffffffa036968d>] vlv_display_power_well_init+0xdb/0xe8 [i915] [ 3188.091224]  [<ffffffffa0369763>] chv_pipe_power_well_enable+0x62/0x67 [i915] [ 3188.102594]  [<ffffffffa036ad55>] intel_display_power_get+0xa0/0xcb [i915] [ 3188.113657]  [<ffffffffa03bdd7a>] modeset_get_crtc_power_domains+0x11d/0x13c [i915] [ 3188.125589]  [<ffffffffa03bdfc1>] intel_atomic_commit+0x228/0xf1b [i915] [ 3188.136522]  [<ffffffffa02c586c>] ? drm_atomic_check_only+0x37b/0x4da [drm] [ 3188.147676]  [<ffffffffa02c5a18>] drm_atomic_commit+0x4d/0x52 [drm] [ 3188.158023]  [<ffffffffa032fa25>] restore_fbdev_mode+0x11e/0x286 [drm_kms_helper] [ 3188.169734]  [<ffffffffa0331443>] drm_fb_helper_restore_fbdev_mode_unlocked+0x36/0x6d [drm_kms_helper] [ 3188.183557]  [<ffffffffa03d270f>] intel_fbdev_restore_mode+0x22/0x7a [i915] [ 3188.194701]  [<ffffffffa03f70b8>] i915_driver_lastclose+0xe/0x15 [i915] [ 3188.205361]  [<ffffffffa02ad4c2>] drm_lastclose+0x3f/0x10a [drm] [ 3188.215323]  [<ffffffffa02ad9ee>] drm_release+0x461/0x49b [drm] [ 3188.225195]  [<ffffffff8118577c>] __fput+0x100/0x1b3 [ 3188.233956]  [<ffffffff81185865>] ____fput+0xe/0x10 [ 3188.242594]  [<ffffffff81084540>] task_work_run+0x6a/0x93 [ 3188.251816]  [<ffffffff810015d6>] prepare_exit_to_usermode+0x9e/0xaf
[ 3188.262111]  [<ffffffff810017d6>] syscall_return_slowpath+0x1ef/0x264
[ 3188.272486]  [<ffffffff81084462>] ? task_work_add+0x44/0x53 [ 3188.281885]  [<ffffffff811858e3>] ? fput+0x7c/0x83 [ 3188.290389]  [<ffffffff810a6e52>] ? trace_hardirqs_on_caller+0x16/0x196
[ 3188.300941]  [<ffffffff81000c87>] ? trace_hardirqs_on_thunk+0x17/0x19 [ 3188.311245]  [<ffffffff814e2ab1>] int_ret_from_sys_call+0x25/0x9f


>  }
>  
>  void intel_hpd_init_work(struct drm_i915_private *dev_priv) diff 
> --git a/drivers/gpu/drm/i915/intel_sdvo.c
> b/drivers/gpu/drm/i915/intel_sdvo.c
> index 05521b5..55859e9 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2433,7 +2433,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>  		 * Ensure that they get re-enabled when an interrupt happens.
>  		 */
>  		intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
> -		intel_sdvo_enable_hotplug(intel_encoder);
>  	} else {
>  		intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
>  	}
> --
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux