Unfortunately, it seems that the HPD IRQ storm problem from the early
days of Intel GPUs was never entirely solved, only mostly. Within the
last couple of days, I got a bug report from one of our customers who
had been having issues with their machine suddenly booting up very
slowly after having updated. The amount of time it took to boot went
from around 30 seconds, to over 6 minutes consistently.
After some investigation, I discovered that i915 was reporting massive
amounts of short HPD IRQ spam on this system from the DisplayPort port,
despite there not being anything actually connected. The symptoms would
start with one "long" HPD IRQ being detected at boot:
[ 1.891398] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00440000, dig 0x00440000, pins 0x000000a0
[ 1.891436] [drm:intel_hpd_irq_handler [i915]] digital hpd port B - long
[ 1.891472] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on PIN 5 - cnt: 0
[ 1.891508] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - long
[ 1.891544] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on PIN 7 - cnt: 0
[ 1.891592] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port B - long
[ 1.891628] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port D - long
…
followed by constant short IRQs afterwards:
[ 1.895091] [drm:intel_encoder_hotplug [i915]] [CONNECTOR:66:DP-1] status updated from unknown to disconnected
[ 1.895129] [drm:i915_hotplug_work_func [i915]] Connector DP-3 (pin 7) received hotplug event.
[ 1.895165] [drm:intel_dp_detect [i915]] [CONNECTOR:72:DP-3]
[ 1.895275] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00200000, dig 0x00200000, pins 0x00000080
[ 1.895312] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
[ 1.895762] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00200000, dig 0x00200000, pins 0x00000080
[ 1.895799] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
[ 1.896239] [drm:intel_dp_aux_xfer [i915]] dp_aux_ch timeout status 0x71450085
[ 1.896293] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00200000, dig 0x00200000, pins 0x00000080
[ 1.896330] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
[ 1.896781] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00200000, dig 0x00200000, pins 0x00000080
[ 1.896817] [drm:intel_hpd_irq_handler [i915]] digital hpd port D - short
[ 1.897275] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 0x00200000, dig 0x00200000, pins 0x00000080
The customer's system in question has a GM45 GPU, which is apparently
well known for hotplugging storms.
So, workaround this impressively broken hardware by changing the default
HPD storm threshold from 5 to 50. Then, make long IRQs count for 10, and
short IRQs count for 1. This makes it so that 5 long IRQs will trigger
an HPD storm, and on systems with short HPD storm detection 50 short
IRQs will trigger an HPD storm. 50 short IRQs amounts to 100ms of
constant pulsing, which seems like a good middleground between being too
sensitive and not being sensitive enough (which would cause visible
stutters in userspace every time a storm occurs).
And just to be extra safe: we don't enable this by default on systems
with MST support. There's too high of a chance of MST support triggering
storm detection, and systems that are new enough to support MST are a
lot less likely to have issues with IRQ storms anyway.
As a note: this patch was tested using a ThinkPad T450s and a Chamelium
to simulate the short IRQ storms.
Changes since v1:
- Don't use two separate thresholds, just make long IRQs count for 10
each and short IRQs count for 1. This simplifies the code a bit
- Ville Syrjälä
Changes since v2:
- Document @long_hpd in intel_hpd_irq_storm_detect, no functional
changes
Changes since v4:
- Remove !! in long_hpd assignment - Ville Syrjälä
- queue_hp = true - Ville Syrjälä
Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181106213017.14563-6-lyude@redhat.com
This is rather confusing to look at as-is:
dev_priv->display.hpd_irq_setup(dev_priv); in intel_hpd_irq_handler()
handles disabling the actual HPD IRQ, while
intel_hpd_irq_storm_disable() handles moving the HPD pin state over from
MARK_DISABLED to DISABLED along with enabling polling for it.
Changes since v3:
- Rename i915_hpd_irq_storm_disable() to
i915_hpd_irq_storm_switch_to_polling() - Rodrigo Vivi
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181106213017.14563-5-lyude@redhat.com
Currently in intel_hpd_irq_storm_detect() when we detect that the last
recorded hotplug wasn't within the period defined by
HPD_STORM_DETECT_DELAY, we make the mistake of resetting the HPD count
to 0 without incrementing it. This results in us only enabling storm
detection when we go +2 above the threshold, e.g. an HPD threshold of 5
would not trigger a storm until we reach a total of 7 hotplugs.
So: rework the code a bit so we reset the HPD count when
HPD_STORM_DETECT_DELAY has passed, then increment the count afterwards.
Also, clean things up a bit to make it easier to undertand.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181106213017.14563-4-lyude@redhat.com
This patch unifies the naming of DRM functions for reference counting
of struct drm_device. The resulting code is more aligned with the rest
of the Linux kernel interfaces.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Exercising the gpu reloc path strenuously revealed an issue where the
updated relocations (from MI_STORE_DWORD_IMM) were not being observed
upon execution. After some experiments with adding pipecontrols (a lot
of pipecontrols (32) as gen4/5 do not have a bit to wait on earlier pipe
controls or even the current on), it was discovered that we merely
needed to delay the EMIT_INVALIDATE by several flushes. It is important
to note that it is the EMIT_INVALIDATE as opposed to the EMIT_FLUSH that
needs the delay as opposed to what one might first expect -- that the
delay is required for the TLB invalidation to take effect (one presumes
to purge any CS buffers) as opposed to a delay after flushing to ensure
the writes have landed before triggering invalidation.
Testcase: igt/gem_tiled_fence_blits
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181105094305.5767-1-chris@chris-wilson.co.uk
Three regressions
- Revert frame counter support
. This patch fixes a issue which doesn't work extension and clone
mode because some CRTC devices don't provide frame counter value
properly.
- Fix lack of fbdev on Rinato and trats boards.
. This patch considers for connector to be registered by DSI after
DRM device is registered, and also it makes fbdev initializaion
to be done even if no connector at the moment.
- Check for dsi->panel object correctly
. This patch fixes checking for dsi->panel. of_drm_find_panel
function returns panel object or error value so error value
should be checked using IS_ERR macro.
Signed-off-by: Dave Airlie <airlied@redhat.com>
From: Inki Dae <inki.dae@samsung.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1541407733-7632-1-git-send-email-inki.dae@samsung.com
Similar to ppfeaturemask. Allows you to selectively enable/disable
DC features.
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
The value is dependent on whether fbc is available.
v2: only check if num_pipes is valid
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
RETIMER_REDRIVER_INFO shows the buffer as a decimal value with a '0x'
prefix, which is somewhat misleading.
Fix it to print hexadecimal, as was intended.
Fixes: 2f14bc89("drm/amd/display: add retimer log for HWQ tuning use.")
Cc: Charlene Liu <charlene.liu@amd.com>
Cc: Dmytro Laktyushkin <Dmytro.Laktyushkin@amd.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
Reviewed-by: Leo Li <sunpeng.li@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
This reverts commit 0cafc82fae.
This breaks some apps that assume 0 is minimum brightness.
Revert for 4.20. This is fixed properly for drm-next/4.21 in:
"drm/amd: Don't fail on backlight = 0"
However, that patch depends on more extensive changes to the
backlight interface which are too invasive for -fixes.
Fixes: Bugzilla: https://bugs.freedesktop.org/108668
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Add a function to check whether there is at least one plane that
supports a specific format and modifier combination. Drivers can
use this to reject unsupported formats/modifiers in .fb_create().
v2: Accept anyformat if the driver doesn't do planes (Eric)
s/planes_have_format/any_plane_has_format/ (Eric)
Check the modifier as well since we already have a function
that does both
v3: Don't do the check in the core since we may not know the
modifier yet, instead export the function and let drivers
call it themselves
Cc: Eric Anholt <eric@anholt.net>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181029183453.28541-1-ville.syrjala@linux.intel.com
Setup and tear down xgmi as part of psp.
v2:
- make psp_xgmi_terminate static
- squash in:
drm/amdgpu: only issue xgmi cmd when it is enabled
drm/amdgpu/psp: terminate xgmi ta in suspend and hw_fini phase
Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
RETIMER_REDRIVER_INFO shows the buffer as a decimal value with a '0x'
prefix, which is somewhat misleading.
Fix it to print hexadecimal, as was intended.
Fixes: 2f14bc89("drm/amd/display: add retimer log for HWQ tuning use.")
Cc: Charlene Liu <charlene.liu@amd.com>
Cc: Dmytro Laktyushkin <Dmytro.Laktyushkin@amd.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
Reviewed-by: Leo Li <sunpeng.li@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Amgpu's backlight update status function was
returning 1 (an error value) when the backlight
property was 0. This breaks users that assume
0 is a valid backlight value (which is a
correct assumption)
If the user passes in a backlight value of 0,
tell them everything is fine, then write a value of
1 to hardware.
Signed-off-by: David Francis <David.Francis@amd.com>
Bugzilla: https://bugs.freedesktop.org/108668
Fixes: 416615ea9578 ("drm/amd/display: set backlight level limit to 1")
Cc: Suresh.Guttula@amd.com
Cc: Harry.Wentland@amd.com
Cc: Samantham@posteo.net
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
It's lockless, and userspace might chance it underneath us. That's not
really a problem, all userspace gets is a slightly dysfunctional
lease with the current code. But this might change, and gcc might
decide to reload a few too many times, and then boom. So better safe
than sorry.
v2: Remove the now unused lessor_priv argument from validate_lease()
(Keith).
v3: Actually add everything ... silly me.
Cc: Keith Packard <keithp@keithp.com>
Cc: Dave Airlie <airlied@gmail.com>
Acked-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181105101211.22737-1-daniel.vetter@ffwll.ch
This is the only place in the driver that should have to deal with
the raw hardware fences. To avoid any further confusion, consolidate
the fence handling in this file and remove any traces of this from
the header files.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
There is no need to track the currently active fence. The GPU scheduler
keeps track of all the in-flight jobs.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
