Bug 24822
Summary: | Embedded DisplayPort is detected wrongly on HP ProBook 5320m | ||
---|---|---|---|
Product: | Drivers | Reporter: | Takashi Iwai (tiwai) |
Component: | Video(DRI - Intel) | Assignee: | drivers_video-dri-intel (drivers_video-dri-intel) |
Status: | RESOLVED PATCH_ALREADY_AVAILABLE | ||
Severity: | normal | CC: | chris, eric, florian, keithp, maciej.rutecki, mat, rjw, sndirsch |
Priority: | P1 | ||
Hardware: | All | ||
OS: | Linux | ||
Kernel Version: | 2.6.37-rc5 | Subsystem: | |
Regression: | Yes | Bisected commit-id: | |
Bug Depends on: | |||
Bug Blocks: | 21782 | ||
Attachments: |
dmesg output (with backport of 2.6.37-rc5+ DRM)
Ad hoc fix patch for wrong eDP detections Verify eDP_A using "the display chicken fuses" Verify eDP_A using "the display chicken fuses" Probe the eDP device on init Probe the eDP device on init Probe the eDP device on init Your final fix... v2.6.38-stable |
Description
Takashi Iwai
2010-12-13 11:09:38 UTC
Created attachment 39972 [details]
dmesg output (with backport of 2.6.37-rc5+ DRM)
Running on 2.6.32.26 SLE11-SP1, but DRM part was backported.
The result is same with vanilla 2.6.37-rc5+ kernel.
Created attachment 39982 [details]
Ad hoc fix patch for wrong eDP detections
A quick-fix patch for the wrong eDP detections
Looks equally prone to a false detection of LVDS. We will probably want to validate against the VBT instead. DP_A & DP_A_DETECTED: "This bit is qualified with the Embedded DisplayPort A capability fuse." And no, I have no idea what they mean here either. Created attachment 40172 [details]
Verify eDP_A using "the display chicken fuses"
I have no idea if this patch is indeed the correct approach, just something I stumbled across in the docs.
Created attachment 40182 [details]
Verify eDP_A using "the display chicken fuses"
Handled-By : Chris Wilson <chris@chris-wilson.co.uk> Tested the patch in comment 6, but it doesn't work. Both eDP and LVDS are still detected. Created attachment 47502 [details]
Probe the eDP device on init
Got an Oops with the patch in intel_dp_aux_ch() during module init. Isn't it applicable to 2.6.37 kernel? No, the patch is bad. The probe is called before enough of the intel_dp is initialized. Is this still a problem in 2.6.38-rc5? Yes, the bug still remains. Created attachment 50182 [details]
Probe the eDP device on init
Defer probe until after we have initialised the i2c adapter....
Created attachment 50192 [details]
Probe the eDP device on init
Slightly tidier patch.
With the patch, I got an Oops in intel_dp_encoder_destroy(). Looks like the warning message in kobject_put() caused a wrong access, which is called from device_unregister() from i2c_del_adapter(). Apart from the fact that we completely ignore any potential error when constructing the i2c device (grr), there doesn't seem to be an issue with the order of deletion here. Can you attach the oops? The order of the calls does matter indeed. Swapping the call order of intel_dp_destroy() and intel_dp_encoder_destroy() fixes the problem. This is because i2c_del_adapter unregisters the device which parent is intel_connector, and connectors are removed in intel_dp_destroy(). Thus intel_dp_encoder_destroy() must be called before intel_dp_destroy(). With that fix, now the driver doesn't initialize eDP but activates only LVDS. The only nitpicking is that DRM_ERROR() is used in that failure path. Since this is no real error, DRM_INFO() should be used instead, IMO. Otherwise user would be confused and wonder what he did wrong. Created attachment 51162 [details]
Your final fix...
I've queued this patch, thanks!
Thanks! Could you prepare the fix patch for 2.6.37/38 kernels and submit to stable kernel once after the fixes are merged to the upstream? The patch in comment 15 can't be applied cleanly to them. A patch referencing this bug report has been merged in v2.6.38-8876-g036a982: commit 48898b038b69ef4801f0e059026c8f6920684677 Author: Takashi Iwai <tiwai@suse.de> Date: Fri Mar 18 09:06:49 2011 +0000 drm/i915/dp: Correct the order of deletion for ghost eDP devices A patch referencing this bug report has been merged in v2.6.38-8876-g036a982: commit 3d3dc149eda48566619d165f6b34e5eeca00edf1 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Sat Feb 12 10:33:12 2011 +0000 drm/i915/dp: Sanity check eDP existence Created attachment 52412 [details]
v2.6.38-stable
For reference, this is the patch that I intend to send to stable.
Thanks! I see slight differences there, though. Don't we need to change the initial dpms value to -1 for 2.6.38? Also, in the original patch, panel is set off before returning the error. I guess it doesn't matter much (as actually no edp is present), but just wondering the difference of code flow. |