[Intel-gfx] [PATCH 6/6] drm/i915/icl: Fix crash when getting DPLL of a MST encoder in TC ports

Tue Oct 30 17:56:24 UTC 2018

On Wed, Oct 10, 2018 at 02:35:08PM -0700, José Roberto de Souza wrote:
> enc_to_dig_port() returns NULL for encoders of type
> INTEL_OUTPUT_DP_MST causing the crash bellow:
> 
> [ 2832.836101] BUG: unable to handle kernel paging request at 00000000000012b8
> [ 2832.843062] PGD 0 P4D 0
> [ 2832.845610] Oops: 0000 [#1] SMP
> [ 2832.848764] CPU: 2 PID: 3577 Comm: kworker/2:0 Tainted: G        W         4.19.0-rc7+ #491
> [ 2832.857106] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.2352.A01.1808281852 08/28/2018
> [ 2832.870734] Workqueue: events output_poll_execute
> [ 2832.875480] RIP: 0010:icl_get_dpll+0xa4/0x5d0 [i915]
> [ 2832.880449] Code: e9 03 f3 48 ab 8b 6e 74 41 8b 8c 24 5c 03 00 00 85 ed 0f 88 3f 02 00 00 83 fd 01 0f 8e ad 01 00 00 83 fd 05 0f 8f 2d 02 00 00 <83> ba b8 12 00 00 02 48 8b 36 0f 84 39 02 00 00 44 8b be ec 89 00
> [ 2832.899176] RSP: 0018:ffffc90001b57a78 EFLAGS: 00010293
> [ 2832.904404] RAX: 0000000000000000 RBX: ffffc90001b57a94 RCX: 0000000000083d60
> [ 2832.911536] RDX: 0000000000000000 RSI: ffff8804a8c0dc00 RDI: ffffc90001b57b18
> [ 2832.918668] RBP: 0000000000000003 R08: ffff8804a8c1f990 R09: ffff8804a8c1f990
> [ 2832.925797] R10: 0000000000000000 R11: ffff8804a8e99600 R12: ffff8804a7760000
> [ 2832.932930] R13: ffff88049e94d000 R14: ffff88049e94d000 R15: 000000000000000e
> [ 2832.940063] FS:  0000000000000000(0000) GS:ffff8804b0300000(0000) knlGS:0000000000000000
> [ 2832.948147] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 2832.953893] CR2: 00000000000012b8 CR3: 0000000004a1d004 CR4: 0000000000760ee0
> [ 2832.961027] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 2832.968155] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 2832.975286] PKRU: 55555554
> [ 2832.978003] Call Trace:
> [ 2832.980496]  haswell_crtc_compute_clock+0x3d/0x68 [i915]
> [ 2832.985841]  intel_crtc_atomic_check+0x61/0x340 [i915]
> [ 2832.990987]  drm_atomic_helper_check_planes+0x130/0x1c0
> [ 2832.996245]  intel_atomic_check+0x4d5/0x10f0 [i915]
> [ 2833.001147]  drm_atomic_check_only+0x484/0x690
> [ 2833.005629]  drm_atomic_commit+0x13/0x50
> [ 2833.009564]  restore_fbdev_mode_atomic+0x1c9/0x1e0
> [ 2833.014363]  drm_fb_helper_restore_fbdev_mode_unlocked+0x47/0x90
> [ 2833.020368]  drm_fb_helper_set_par+0x29/0x50
> [ 2833.024641]  drm_fb_helper_hotplug_event.part.33+0x92/0xb0
> [ 2833.030130]  drm_kms_helper_hotplug_event+0x26/0x30
> [ 2833.035013]  output_poll_execute+0x192/0x1b0
> [ 2833.039293]  process_one_work+0x2a5/0x5f0
> [ 2833.043315]  worker_thread+0x2d/0x3d0
> [ 2833.046988]  ? rescuer_thread+0x340/0x340
> [ 2833.051009]  kthread+0x112/0x130
> [ 2833.054247]  ? kthread_create_worker_on_cpu+0x70/0x70
> [ 2833.059307]  ret_from_fork+0x3a/0x50
> [ 2833.062893] Modules linked in: i915 prime_numbers snd_hda_codec_realtek snd_hda_codec_generic asix snd_usb_audio snd_usbmidi_lib snd_seq_midi snd_seq_midi_event snd_rawmidi cdc_ether usbnet x86_pkg_temp_thermal xhci_pci xhci_hcd ucsi_acpi typec_ucsi typec efivarfs [last unloaded: prime_numbers]
> [ 2833.088917] CR2: 00000000000012b8
> [ 2833.092241] ---[ end trace 25f9fe3d47af2e75 ]---
> [ 2833.096895] RIP: 0010:icl_get_dpll+0xa4/0x5d0 [i915]
> [ 2833.101866] Code: e9 03 f3 48 ab 8b 6e 74 41 8b 8c 24 5c 03 00 00 85 ed 0f 88 3f 02 00 00 83 fd 01 0f 8e ad 01 00 00 83 fd 05 0f 8f 2d 02 00 00 <83> ba b8 12 00 00 02 48 8b 36 0f 84 39 02 00 00 44 8b be ec 89 00
> [ 2833.120589] RSP: 0018:ffffc90001b57a78 EFLAGS: 00010293
> [ 2833.125815] RAX: 0000000000000000 RBX: ffffc90001b57a94 RCX: 0000000000083d60
> [ 2833.132946] RDX: 0000000000000000 RSI: ffff8804a8c0dc00 RDI: ffffc90001b57b18
> [ 2833.140080] RBP: 0000000000000003 R08: ffff8804a8c1f990 R09: ffff8804a8c1f990
> [ 2833.147213] R10: 0000000000000000 R11: ffff8804a8e99600 R12: ffff8804a7760000
> [ 2833.154350] R13: ffff88049e94d000 R14: ffff88049e94d000 R15: 000000000000000e
> [ 2833.161483] FS:  0000000000000000(0000) GS:ffff8804b0300000(0000) knlGS:0000000000000000
> [ 2833.169565] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 2833.175313] CR2: 00000000000012b8 CR3: 0000000004a1d004 CR4: 0000000000760ee0
> [ 2833.182449] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 2833.189578] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 2833.196712] PKRU: 55555554
> 
> MST ports are allocated from struct intel_dp_mst_encoder not from
> struct intel_digital_port as regular ports, so to get the TC type it
> is necessary check the primary digital port of the mst encoder.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 874646357ad1..23a1bc17a3f9 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2867,8 +2867,7 @@ static struct intel_shared_dpll *
>  icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>  	     struct intel_encoder *encoder)
>  {
> -	struct intel_digital_port *intel_dig_port =
> -			enc_to_dig_port(&encoder->base);
> +	struct intel_digital_port *intel_dig_port;
>  	struct intel_shared_dpll *pll;
>  	struct intel_dpll_hw_state pll_state = {};
>  	enum port port = encoder->port;
> @@ -2888,6 +2887,15 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>  	case PORT_D:
>  	case PORT_E:
>  	case PORT_F:
> +		if (encoder->type == INTEL_OUTPUT_DP_MST) {
> +			struct intel_dp_mst_encoder *mst_encoder;
> +
> +			mst_encoder = enc_to_mst(&encoder->base);
> +			intel_dig_port = mst_encoder->primary;
> +		} else {
> +			intel_dig_port = enc_to_dig_port(&encoder->base);
> +		}

Argh. I thought we had finally got rid of all this. I guess this should
get fixed in a nicer way if we start to track the tc_type (or something
like it) in the crtc_state.

In the meantime
Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

> +
>  		if (intel_dig_port->tc_type == TC_PORT_TBT) {
>  			min = DPLL_ID_ICL_TBTPLL;
>  			max = min;
> -- 
> 2.19.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel