Re: [Intel-gfx] [PATCH] drm/i915/bios: filter out invalid DDC pins from VBT child devices

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

 



Quoting Jani Nikula (2018-04-11 10:01:46)
> The VBT contains the DDC pin to use for specific ports. Alas, sometimes
> the field appears to contain bogus data, and while we check for it later
> on in intel_gmbus_get_adapter() we fail to check the returned NULL on
> errors. Oops results.
> 
> The simplest approach seems to be to catch and ignore the bogus DDC pins
> already at the VBT parsing phase, reverting to fixed per port default
> pins. This doesn't guarantee display working, but at least it prevents
> the oops. And we continue to be fuzzed by VBT.
> 
> One affected machine is Dell Latitude 5590 where a BIOS upgrade added
> invalid DDC pins.
> 
> Typical backtrace:
> 
> [   35.461411] WARN_ON(!intel_gmbus_is_valid_pin(dev_priv, pin))
> [   35.461432] WARNING: CPU: 6 PID: 411 at drivers/gpu/drm/i915/intel_i2c.c:844 intel_gmbus_get_adapter+0x32/0x37 [i915]
> [   35.461437] Modules linked in: i915 ahci libahci dm_snapshot dm_bufio dm_raid raid456 async_raid6_recov async_pq raid6_pq async_xor xor async_memcpy async_tx
> [   35.461445] CPU: 6 PID: 411 Comm: kworker/u16:2 Not tainted 4.16.0-rc7.x64-g1cda370ffded #1
> [   35.461447] Hardware name: Dell Inc. Latitude 5590/0MM81M, BIOS 1.1.9 03/13/2018
> [   35.461450] Workqueue: events_unbound async_run_entry_fn
> [   35.461465] RIP: 0010:intel_gmbus_get_adapter+0x32/0x37 [i915]
> [   35.461467] RSP: 0018:ffff9b4e43d47c40 EFLAGS: 00010286
> [   35.461469] RAX: 0000000000000000 RBX: ffff98f90639f800 RCX: ffffffffae051960
> [   35.461471] RDX: 0000000000000001 RSI: 0000000000000092 RDI: 0000000000000246
> [   35.461472] RBP: ffff98f905410000 R08: 0000004d062a83f6 R09: 00000000000003bd
> [   35.461474] R10: 0000000000000031 R11: ffffffffad4eda58 R12: ffff98f905410000
> [   35.461475] R13: ffff98f9064c1000 R14: ffff9b4e43d47cf0 R15: ffff98f905410000
> [   35.461477] FS:  0000000000000000(0000) GS:ffff98f92e580000(0000) knlGS:0000000000000000
> [   35.461479] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   35.461481] CR2: 00007f5682359008 CR3: 00000001b700c005 CR4: 00000000003606e0
> [   35.461483] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [   35.461484] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [   35.461486] Call Trace:
> [   35.461501]  intel_hdmi_set_edid+0x37/0x27f [i915]
> [   35.461515]  intel_hdmi_detect+0x7c/0x97 [i915]
> [   35.461518]  drm_helper_probe_single_connector_modes+0xe1/0x6c0
> [   35.461521]  drm_setup_crtcs+0x129/0xa6a
> [   35.461523]  ? __switch_to_asm+0x34/0x70
> [   35.461525]  ? __switch_to_asm+0x34/0x70
> [   35.461527]  ? __switch_to_asm+0x40/0x70
> [   35.461528]  ? __switch_to_asm+0x34/0x70
> [   35.461529]  ? __switch_to_asm+0x40/0x70
> [   35.461531]  ? __switch_to_asm+0x34/0x70
> [   35.461532]  ? __switch_to_asm+0x40/0x70
> [   35.461534]  ? __switch_to_asm+0x34/0x70
> [   35.461536]  __drm_fb_helper_initial_config_and_unlock+0x34/0x46f
> [   35.461538]  ? __switch_to_asm+0x40/0x70
> [   35.461541]  ? _cond_resched+0x10/0x33
> [   35.461557]  intel_fbdev_initial_config+0xf/0x1c [i915]
> [   35.461560]  async_run_entry_fn+0x2e/0xf5
> [   35.461563]  process_one_work+0x15b/0x364
> [   35.461565]  worker_thread+0x2c/0x3a0
> [   35.461567]  ? process_one_work+0x364/0x364
> [   35.461568]  kthread+0x10c/0x122
> [   35.461570]  ? _kthread_create_on_node+0x5d/0x5d
> [   35.461572]  ret_from_fork+0x35/0x40
> [   35.461574] Code: 74 16 89 f6 48 8d 04 b6 48 c1 e0 05 48 29 f0 48 8d 84 c7 e8 11 00 00 c3 48 c7 c6 b0 19 1e c0 48 c7 c7 64 8a 1c c0 e8 47 88 ed ec <0f> 0b 31 c0 c3 8b 87 a4 04 00 00 80 e4 fc 09 c6 89 b7 a4 04 00
> [   35.461604] WARNING: CPU: 6 PID: 411 at drivers/gpu/drm/i915/intel_i2c.c:844 intel_gmbus_get_adapter+0x32/0x37 [i915]
> [   35.461606] ---[ end trace 4fe1e63e2dd93373 ]---
> [   35.461609] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
> [   35.461613] IP: i2c_transfer+0x4/0x86
> [   35.461614] PGD 0 P4D 0
> [   35.461616] Oops: 0000 [#1] SMP PTI
> [   35.461618] Modules linked in: i915 ahci libahci dm_snapshot dm_bufio dm_raid raid456 async_raid6_recov async_pq raid6_pq async_xor xor async_memcpy async_tx
> [   35.461624] CPU: 6 PID: 411 Comm: kworker/u16:2 Tainted: G        W        4.16.0-rc7.x64-g1cda370ffded #1
> [   35.461625] Hardware name: Dell Inc. Latitude 5590/0MM81M, BIOS 1.1.9 03/13/2018
> [   35.461628] Workqueue: events_unbound async_run_entry_fn
> [   35.461630] RIP: 0010:i2c_transfer+0x4/0x86
> [   35.461631] RSP: 0018:ffff9b4e43d47b30 EFLAGS: 00010246
> [   35.461633] RAX: ffff9b4e43d47b6e RBX: 0000000000000005 RCX: 0000000000000001
> [   35.461635] RDX: 0000000000000002 RSI: ffff9b4e43d47b80 RDI: 0000000000000000
> [   35.461636] RBP: ffff9b4e43d47bd8 R08: 0000004d062a83f6 R09: 00000000000003bd
> [   35.461638] R10: 0000000000000031 R11: ffffffffad4eda58 R12: 0000000000000002
> [   35.461639] R13: 0000000000000001 R14: ffff9b4e43d47b6f R15: ffff9b4e43d47c07
> [   35.461641] FS:  0000000000000000(0000) GS:ffff98f92e580000(0000) knlGS:0000000000000000
> [   35.461643] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   35.461645] CR2: 0000000000000010 CR3: 00000001b700c005 CR4: 00000000003606e0
> [   35.461646] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [   35.461647] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [   35.461649] Call Trace:
> [   35.461652]  drm_do_probe_ddc_edid+0xb3/0x128
> [   35.461654]  drm_get_edid+0xe5/0x38d
> [   35.461669]  intel_hdmi_set_edid+0x45/0x27f [i915]
> [   35.461684]  intel_hdmi_detect+0x7c/0x97 [i915]
> [   35.461687]  drm_helper_probe_single_connector_modes+0xe1/0x6c0
> [   35.461689]  drm_setup_crtcs+0x129/0xa6a
> [   35.461691]  ? __switch_to_asm+0x34/0x70
> [   35.461693]  ? __switch_to_asm+0x34/0x70
> [   35.461694]  ? __switch_to_asm+0x40/0x70
> [   35.461696]  ? __switch_to_asm+0x34/0x70
> [   35.461697]  ? __switch_to_asm+0x40/0x70
> [   35.461698]  ? __switch_to_asm+0x34/0x70
> [   35.461700]  ? __switch_to_asm+0x40/0x70
> [   35.461701]  ? __switch_to_asm+0x34/0x70
> [   35.461703]  __drm_fb_helper_initial_config_and_unlock+0x34/0x46f
> [   35.461705]  ? __switch_to_asm+0x40/0x70
> [   35.461707]  ? _cond_resched+0x10/0x33
> [   35.461724]  intel_fbdev_initial_config+0xf/0x1c [i915]
> [   35.461727]  async_run_entry_fn+0x2e/0xf5
> [   35.461729]  process_one_work+0x15b/0x364
> [   35.461731]  worker_thread+0x2c/0x3a0
> [   35.461733]  ? process_one_work+0x364/0x364
> [   35.461734]  kthread+0x10c/0x122
> [   35.461736]  ? _kthread_create_on_node+0x5d/0x5d
> [   35.461738]  ret_from_fork+0x35/0x40
> [   35.461739] Code: 5c fa e1 ad 48 89 df e8 ea fb ff ff e9 2a ff ff ff 0f 1f 44 00 00 31 c0 e9 43 fd ff ff 31 c0 45 31 e4 e9 c5 fd ff ff 41 54 55 53 <48> 8b 47 10 48 83 78 10 00 74 70 41 89 d4 48 89 f5 48 89 fb 65
> [   35.461756] RIP: i2c_transfer+0x4/0x86 RSP: ffff9b4e43d47b30
> [   35.461757] CR2: 0000000000000010
> [   35.461759] ---[ end trace 4fe1e63e2dd93374 ]---
> 
> Based on a patch by Fei Li.
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Fei Li <fei.li@xxxxxxxxx>
> Co-developed-by: Fei Li <fei.li@xxxxxxxxx>
> Reported-by: Pavel Nakonechnyi <zorg1331@xxxxxxxxx>
> Reported-by: Seweryn Kokot <sewkokot@xxxxxxxxx>
> Reported-by: Laszlo Valko <valko@xxxxxxxxxxxxxxxxx>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105549
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105961
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index c5c7530ba157..6db845991a69 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1256,7 +1256,6 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>                 return;
>  
>         aux_channel = child->aux_channel;
> -       ddc_pin = child->ddc_pin;

Could we scope ddc_pin better?

>         is_dvi = child->device_type & DEVICE_TYPE_TMDS_DVI_SIGNALING;
>         is_dp = child->device_type & DEVICE_TYPE_DISPLAYPORT_OUTPUT;
> @@ -1303,9 +1302,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>                 DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
>  
>         if (is_dvi) {
> -               info->alternate_ddc_pin = map_ddc_pin(dev_priv, ddc_pin);
> -
> -               sanitize_ddc_pin(dev_priv, port);
> +               ddc_pin = map_ddc_pin(dev_priv, child->ddc_pin);
> +               if (intel_gmbus_is_valid_pin(dev_priv, ddc_pin)) {
> +                       info->alternate_ddc_pin = ddc_pin;
> +                       sanitize_ddc_pin(dev_priv, port);
> +               } else {
> +                       DRM_DEBUG_KMS("Port %c has invalid DDC pin %d, "
> +                                     "reverting to defaults\n",
> +                                     port_name(port), ddc_pin);

Do we know what the default pin is here?

s/reverting/sticking/?

"invalid DDC pin %d in VBT" ? Not sure if that would be clear from the
message otherwise.

Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
-Chris



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]