Bug 71051

Summary: Internal display not working on Baytrail notebook.
Product: DRI Reporter: roberth <sarvatt>
Component: DRM/IntelAssignee: Ville Syrjala <ville.syrjala>
Status: CLOSED FIXED QA Contact: Intel GFX Bugs mailing list <intel-gfx-bugs>
Severity: normal    
Priority: medium CC: intel-gfx-bugs
Version: XOrg git   
Hardware: Other   
OS: All   
See Also: https://bugs.freedesktop.org/show_bug.cgi?id=70282
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
dmesg with drm.debug=0xe on 20131023 drm-intel-nightly
none
dmesg with patch from comment #2
none
dmesg with patch from comment #4
none
dmesg with patch from comment #6
none
Proper fix
none
dmesg with patch from comment #8
none
Better fix
none
dmesg with patch from comment #10
none
/sys/kernel/debug/dri/0/i915_opregion
none
Additional patch
none
dmesg with patches from comments 10 and 14 none

Description roberth 2013-10-30 14:52:36 UTC
Created attachment 88370 [details]
dmesg with drm.debug=0xe on 20131023 drm-intel-nightly

The display seems to work fine with efifb but once i915 takes over it doesn't work. Ignore that the kernel here says 3.5, it has DRM from drm-intel-nightly on 20131023.

-- chipset: Intel Corporation ValleyView Gen7 [8086:0f31]
-- system architecture: x86_64
-- Linux distribution: Ubuntu 12.04.2
-- libdrm 2.4.43
-- Kernel versions affected: drm-intel-nightly from 20131023
Comment 1 Chris Wilson 2013-10-30 15:00:01 UTC
It uses DP1 as its connection, which gets reported as disconnected due to SDEIER.
Comment 2 Chris Wilson 2013-10-30 15:02:32 UTC
Try for a little more detail:


diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2e9d75d..93be86d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -895,6 +895,10 @@ bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
                }
        }
 
+       DRM_DEBUG_KMS("checking connected status of port %c: %08x & %08x? %d\n",
+                       port_name(port->port), 
+                       I915_READ(SDEISR), bit,
+                       (I915_READ(SDEISR) & bit) != 0);
        return I915_READ(SDEISR) & bit;
 }
Comment 3 roberth 2013-10-30 19:22:48 UTC
Created attachment 88377 [details]
dmesg with patch from comment #2

No luck getting more info there
Comment 4 Chris Wilson 2013-10-30 22:17:15 UTC
Ugh. Let's annotate the other path then:

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c392ad2..9707c47 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2949,6 +2949,10 @@ g4x_dp_detect(struct intel_dp *intel_dp)
                return connector_status_unknown;
        }
 
+       DRM_DEBUG_KMS("checking connected status of port %c: %08x & %08x? %d\n",
+                       port_name(intel_dig_port->port),
+                       I915_READ(PORT_HOTPLUG_STAT), bit,
+                       (I915_READ(PORT_HOTPLUG_STAT) & bit) != 0);
        if ((I915_READ(PORT_HOTPLUG_STAT) & bit) == 0)
                return connector_status_disconnected;
Comment 5 roberth 2013-10-31 13:59:41 UTC
Created attachment 88407 [details]
dmesg with patch from comment #4

[    9.046060] [drm:g4x_dp_detect], checking connected status of port B: 20000000 & 08000000? 0
[   12.567470] [drm:g4x_dp_detect], checking connected status of port B: 20000000 & 08000000? 0
[   12.567490] [drm:g4x_dp_detect], checking connected status of port B: 20000000 & 08000000? 0
[   14.611456] [drm:g4x_dp_detect], checking connected status of port B: 20000000 & 08000000? 0
[   14.611477] [drm:g4x_dp_detect], checking connected status of port B: 20000000 & 08000000? 0
Comment 6 Ville Syrjala 2013-10-31 14:15:37 UTC
So an internal display on port B. Is it an eDP panel?

Unfortunately the code is hardcoded for eDP on port C and DP on port B.

Maybe this could help:

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 749c605..6ee2cb3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3564,6 +3564,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
        case PORT_A:
                type = DRM_MODE_CONNECTOR_eDP;
                break;
+       case PORT_B:
        case PORT_C:
                if (IS_VALLEYVIEW(dev))
                        type = DRM_MODE_CONNECTOR_eDP;
Comment 7 roberth 2013-10-31 14:42:03 UTC
Created attachment 88409 [details]
dmesg with patch from comment #6

Success!
Comment 8 Ville Syrjala 2013-10-31 15:31:50 UTC
Created attachment 88410 [details] [review]
Proper fix

Here's an attempt at a proper fix that shouldn't regress regular DP users.

Please give it a go and let me know if it works.
Comment 9 roberth 2013-10-31 15:58:56 UTC
Created attachment 88411 [details]
dmesg with patch from comment #8

Thanks so much, that works great.

Tested-by: Robert Hooker <robert.hooker@canonical.com>
Comment 10 Ville Syrjala 2013-10-31 18:57:47 UTC
Created attachment 88448 [details] [review]
Better fix

Looks like I jumped the gun with the patch a bit. I thought we had a way to detect eDP panels from the DPCD data, but it turns out we can't. So the previous patch would mistakenly identify any DP display as eDP, which is clearly not what we want.

I couldn't find any other way to detect eDP than to consul the VBT. Let's hope it has the correct information.

Please re-test with this new patch.
Comment 11 roberth 2013-11-01 03:43:08 UTC
Created attachment 88458 [details]
dmesg with patch from comment #10

No luck with this unfortunately, see the attached dmesg.
Comment 12 Ville Syrjala 2013-11-01 08:57:39 UTC
(In reply to comment #11)
> Created attachment 88458 [details]
> dmesg with patch from comment #10
> 
> No luck with this unfortunately, see the attached dmesg.

Dang. Can you attach a copy of /sys/kernel/debug/dri/0/i915_opregion ?
Comment 13 roberth 2013-11-01 13:49:32 UTC
Created attachment 88475 [details]
/sys/kernel/debug/dri/0/i915_opregion
Comment 14 Ville Syrjala 2013-11-01 15:23:54 UTC
Created attachment 88481 [details] [review]
Additional patch

If you apply this new patch and the patch from comment #10, I think things should work.

Let me know how it goes.
Comment 15 roberth 2013-11-01 16:07:39 UTC
Created attachment 88489 [details]
dmesg with patches from comments 10 and 14

The combo of patches from comments 10 and 14 does work
Comment 16 Jesse Barnes 2013-11-17 21:52:42 UTC
I think this is fixed now?

commit f02586dfedf1dfae4f5ff7eb1990a2c4c56e1425
Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
Date:   Fri Nov 1 20:32:08 2013 +0200

    drm/i915: Make intel_dp_is_edp() less specific
Comment 17 Adam Williamson 2013-11-29 18:06:52 UTC
In Fedora's 3.13.0-0.rc1.git3.1 kernel I see the "Make intel_dp_is_edp() less specific" change, but not the "Check VBT for eDP ports on VLV" change.
Comment 18 Ville Syrjala 2013-11-29 19:05:11 UTC
(In reply to comment #17)
> In Fedora's 3.13.0-0.rc1.git3.1 kernel I see the "Make intel_dp_is_edp()
> less specific" change, but not the "Check VBT for eDP ports on VLV" change.

Yeah some of the patches fell through the cracks a bit. They're now in -fixes.
Comment 19 Adam Williamson 2013-11-29 19:13:09 UTC
Also, for anyone who pitches up here looking for a patch, the patch from c#10 appears to be incomplete, compared against the upstream commit:

http://cgit.freedesktop.org/~danvet/drm-intel/commit/?h=drm-intel-nightly&id=5d8a77529bd6864361005117c3a611b6d810aa77

it's missing the change to intel_ddi.c , and indeed, if you try and build a kernel without that change, it fails (for me at least).

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.