[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <871taulbkj.fsf@intel.com>
Date: Thu, 10 Dec 2015 14:42:20 +0200
From: Jani Nikula <jani.nikula@...ux.intel.com>
To: Daniel Vetter <daniel@...ll.ch>,
Stefan Lippers-Hollmann <s.l-h@....de>
Cc: Daniel Vetter <daniel.vetter@...ll.ch>,
linux-kernel@...r.kernel.org,
DRI mailing list <dri-devel@...ts.freedesktop.org>,
Rodrigo Vivi <rodrigo.vivi@...el.com>
Subject: Re: [git pull] drm for 4.4-rc1
On Thu, 10 Dec 2015, Daniel Vetter <daniel@...ll.ch> wrote:
> On Thu, Dec 10, 2015 at 04:04:20AM +0100, Stefan Lippers-Hollmann wrote:
>> Hi
>>
>> On 2015-11-09, Dave Airlie wrote:
>> [...]
>> The following changes since commit 06d1ee32a4d25356a710b49d5e95dbdd68bdf505:
>>
>> Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux (2015-10-13 09:45:21 -0700)
>>
>> are available in the git repository at:
>>
>> git://people.freedesktop.org/~airlied/linux drm-next
>>
>> for you to fetch changes up to 816d2206f0f9953ca854e4ff1a2749a5cbd62715:
>>
>> Merge tag 'drm-intel-next-fixes-2015-11-06' of git://anongit.freedesktop.org/drm-intel into drm-next (2015-11-07 17:16:59 +1000)
>> [...]
>> > drm/i915: Check live status before reading edid
>> [...]
>>
>> This patch seems to introduce a regression for i915 in Linus'
>> v4.4-rc4-60-g9a0f76f, relative to v4.3 (and 4.3.1), on a sandy-bridge
>> (Intel DH67CL) system with a single HDMI connected monitor (Medion MD20094)
>> attached. Immediately after the modeswitch, the monitor switches off and
>> stays off. Nothing catches my eyes in dmesg or Xorg.0.log, but bisecting
>> the issue points me at:
>>
>> 237ed86c693d8a8e4db476976aeb30df4deac74b is the first bad commit
>> commit 237ed86c693d8a8e4db476976aeb30df4deac74b
>> Author: Sonika Jindal <sonika.jindal@...el.com>
>> Date: Tue Sep 15 09:44:20 2015 +0530
>>
>> drm/i915: Check live status before reading edid
>>
>> The Bspec is very clear that Live status must be checked about before
>> trying to read EDID over DDC channel. This patch makes sure that HDMI
>> EDID is read only when live status is up.
>>
>> The live status doesn't seem to perform very consistent across various
>> platforms when tested with different monitors. The reason behind that is
>> some monitors are late to provide right voltage to set live_status up.
>> So, after getting the interrupt, for a small duration, live status reg
>> fluctuates, and then settles down showing the correct staus.
>>
>> This is explained here in, in a rough way:
>> HPD line ________________
>> |\ T1 = Monitor Hotplug causing IRQ
>> | \______________________________________
>> | |
>> | |
>> | | T2 = Live status is stable
>> | | _____________________________________
>> | | /|
>> Live status _____________|_|/ |
>> | | |
>> | | |
>> | | |
>> T0 T1 T2
>>
>> (Between T1 and T2 Live status fluctuates or can be even low, depending on
>> the monitor)
>>
>> After several experiments, we have concluded that a max delay
>> of 30ms is enough to allow the live status to settle down with
>> most of the monitors. This total delay of 30ms has been split into
>> a resolution of 3 retries of 10ms each, for the better cases.
>>
>> This delay is kept at 30ms, keeping in consideration that, HDCP compliance
>> expect the HPD handler to respond a plug out in 100ms, by disabling port.
>>
>> v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
>> to check digital port status. Adding a separate function to get bxt live
>> status (Daniel)
>> v3: Using intel_encoder->hpd_pin to check the live status (Siva)
>> Moving the live status read to intel_hdmi_probe and passing parameter
>> to read/not to read the edid. (me)
>> v4:
>> * Added live status check for all platforms using
>> intel_digital_port_connected.
>> * Rebased on top of Jani's DP cleanup series
>> * Some monitors take time in setting the live status. So retry for few
>> times if this is a connect HPD
>> v5: Removed extra "drm/i915" from commit message. Adding Shashank's sob
>> which was missed.
>> v6: Drop the (!detect_edid && !live_status check) check because for DDI
>> ports which are enumerated as hdmi as well as DP, we don't have a
>> mechanism to differentiate between DP and hdmi inside the encoder's
>> hot_plug. This leads to call to the hdmi's hot_plug hook for DP as well
>> as hdmi which leads to issues during unplug because of the above check.
>> v7: Make intel_digital_port_connected global in this patch, some
>> reformatting of while loop, adding a print when live status is not
>> up. (Rodrigo)
>> v8: Rebase it on nightly which involved skipping the hot_plug hook for now
>> and letting the live_status check happen in detect until the hpd handling
>> part is finalized (Daniel)
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@...el.com>
>> Signed-off-by: Sonika Jindal <sonika.jindal@...el.com>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@...el.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@...ll.ch>
>>
>> :040000 040000 85f19ff45ffe39cee99a7a9b0e9dadf9fe71553f 0094b6c634580a496c5cdbf77f13fe5fa769c3e3 M drivers
>>
>> $ git bisect log
>> # bad: [9a0f76fde9ad2c00c0cf13aaf3dfb9d886dc578c] Merge tag 'for-linus-4.4-1' of git://git.code.sf.net/p/openipmi/linux-ipmi
>> # good: [6a13feb9c82803e2b815eca72fa7a9f5561d7861] Linux 4.3
>> git bisect start 'HEAD' 'v4.3'
>> # good: [8e483ed1342a4ea45b70f0f33ac54eff7a33d918] Merge tag 'char-misc-4.4-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc
>> git bisect good 8e483ed1342a4ea45b70f0f33ac54eff7a33d918
>> # bad: [3e82806b97398d542a5e03bd94861f79ce10ecee] Merge branch 'drm-next' of git://people.freedesktop.org/~airlied/linux
>> git bisect bad 3e82806b97398d542a5e03bd94861f79ce10ecee
>> # good: [0280d1a099da1d211e76ec47cc0944c993a36316] Merge tag 'sound-4.4-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
>> git bisect good 0280d1a099da1d211e76ec47cc0944c993a36316
>> # good: [123a28d8b522b03dd97c1f791245924088616ac0] Merge branch 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs
>> git bisect good 123a28d8b522b03dd97c1f791245924088616ac0
>> # bad: [b10f1b20171945b49988b2b1fe68cb312cc36d32] drm/i915/skl: Prevent unclaimed register writes on skylake.
>> git bisect bad b10f1b20171945b49988b2b1fe68cb312cc36d32
>> # bad: [89e3e1427629027dc33e576fc002880a02a7e50c] drm/i915: Support NV12 in rotated GGTT mapping
>> git bisect bad 89e3e1427629027dc33e576fc002880a02a7e50c
>> # good: [0b2eb33e5e2b4aaeaa786d551a1b5f08904a9bfc] drm/i915: LPT:LP needs port A HPD enabled in both north and south
>> git bisect good 0b2eb33e5e2b4aaeaa786d551a1b5f08904a9bfc
>> # good: [eb120ef6a3e18780e7464e7de70221d06dfe5845] drm/i915: add more debug info for when atomic updates fail v3
>> git bisect good eb120ef6a3e18780e7464e7de70221d06dfe5845
>> # bad: [7aefe2b50a0be5c85e7702bff110e5e187fbe4b5] drm/i915: Fix fastboot scalers for skylake.
>> git bisect bad 7aefe2b50a0be5c85e7702bff110e5e187fbe4b5
>> # good: [83e3337204b2385d20e149c8fe91bb2719978df2] drm/i915: Increase maximum polling time to 50ms for forcewake request/clear ack
>> git bisect good 83e3337204b2385d20e149c8fe91bb2719978df2
>> # bad: [9eca6832f7254d49d25494da7d47c0f8a24f7862] drm/i915: Move scanline_offset and timestamping constant setup to intel_modeset_readout_hw_state()
>> git bisect bad 9eca6832f7254d49d25494da7d47c0f8a24f7862
>> # good: [d2e08c0f34438af791482de8abf2c8e4e573b1d3] drm/i915/bxt: DSI prepare changes for BXT
>> git bisect good d2e08c0f34438af791482de8abf2c8e4e573b1d3
>> # bad: [7f4c62840cc416986f983a8c68f7010c97e06daf] drm/i915: Assign hwmode after encoder state readout
>> git bisect bad 7f4c62840cc416986f983a8c68f7010c97e06daf
>> # bad: [237ed86c693d8a8e4db476976aeb30df4deac74b] drm/i915: Check live status before reading edid
>> git bisect bad 237ed86c693d8a8e4db476976aeb30df4deac74b
>> # first bad commit: [237ed86c693d8a8e4db476976aeb30df4deac74b] drm/i915: Check live status before reading edid
>>
>> Unfortunately reverting only this patch on top of v4.4-rc4-60-g9a0f76f
>> doesn't appear to be quite straight forward. What is the best approach
>> to debug this further?
>>
>> The very same kernel works fine on two ivy-bridge systems (different
>> monitors, one connected via HDMI, one via DVI) and an Atom N270 based
>> system (Intel D945GSEJT, monitor connected via DVI).
>
> A few things to test:
The very first thing to do is to ensure you've tried v4.4-rc4, which
contains
commit 0f5a9be15797f78c9a34e432f26c796165b6e49a
Author: Imre Deak <imre.deak@...el.com>
Date: Fri Nov 27 18:55:29 2015 +0200
drm/i915: take a power domain reference while checking the HDMI live status
BR,
Jani.
> - How does that screen fare on a working machine? Would tell us if the
> issue is with the sink or the source.
>
> - Please boot up with drm.debug=0xe on a working and broken kernel, grab
> dmesg for both.
>
> - Extending the timeout magic might be worth a shot like in the below
> patch. Crank up retry if it doesn't help.
>
> Thanks, Daniel
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index c3978bad5ca0..86ebaac548b3 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1387,7 +1387,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> struct drm_i915_private *dev_priv = to_i915(connector->dev);
> bool live_status = false;
> - unsigned int retry = 3;
> + unsigned int retry = 10;
>
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> connector->base.id, connector->name);
> @@ -1397,7 +1397,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> while (!live_status && --retry) {
> live_status = intel_digital_port_connected(dev_priv,
> hdmi_to_dig_port(intel_hdmi));
> - mdelay(10);
> + msleep(10);
> }
>
> if (!live_status)
--
Jani Nikula, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists