| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
In my haste to remove irq_port[] I accidentally changed the
way we deal with hpd pins that are shared by multiple encoders
(DP and HDMI for pre-DDI platforms). Previously we would only
handle such pins via ->hpd_pulse(), but now we queue up the
hotplug work for the HDMI encoder directly. Worse yet, we now
count each hpd twice and this increment the hpd storm count
twice as fast. This can lead to spurious storms being detected.
Go back to the old way of doing things, ie. delegate to
->hpd_pulse() for any pin which has an encoder with that hook
implemented. I don't really like the idea of adding irq_port[]
back so let's loop through the encoders first to check if we
have an encoder with ->hpd_pulse() for the pin, and then go
through all the pins and decided on the correct course of action
based on the earlier findings.
I have occasionally toyed with the idea of unifying the pre-DDI
HDMI and DP encoders into a single encoder as well. Besides the
hotplug processing it would have the other benefit of preventing
userspace from trying to enable both encoders at the same time.
That is simply illegal as they share the same clock/data pins.
We have some testcases that will attempt that and thus fail on
many older machines. But for now let's stick to fixing just the
hotplug code.
Cc: stable@vger.kernel.org # 4.19+
Cc: Lyude Paul <lyude@redhat.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Fixes: b6ca3eee18ba ("drm/i915: Nuke dev_priv->irq_port[]")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181108200424.28371-1-ville.syrjala@linux.intel.com
Reviewed-by: Lyude Paul <lyude@redhat.com>
|
|
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
|
|
Turns out that if you trigger an HPD storm on a system that has an MST
topology connected to it, you'll end up causing the kernel to eventually
hit a NULL deref:
[ 332.339041] BUG: unable to handle kernel NULL pointer dereference at 00000000000000ec
[ 332.340906] PGD 0 P4D 0
[ 332.342750] Oops: 0000 [#1] SMP PTI
[ 332.344579] CPU: 2 PID: 25 Comm: kworker/2:0 Kdump: loaded Tainted: G O 4.18.0-rc3short-hpd-storm+ #2
[ 332.346453] Hardware name: LENOVO 20BWS1KY00/20BWS1KY00, BIOS JBET71WW (1.35 ) 09/14/2018
[ 332.348361] Workqueue: events intel_hpd_irq_storm_reenable_work [i915]
[ 332.350301] RIP: 0010:intel_hpd_irq_storm_reenable_work.cold.3+0x2f/0x86 [i915]
[ 332.352213] Code: 00 00 ba e8 00 00 00 48 c7 c6 c0 aa 5f a0 48 c7 c7 d0 73 62 a0 4c 89 c1 4c 89 04 24 e8 7f f5 af e0 4c 8b 04 24 44 89 f8 29 e8 <41> 39 80 ec 00 00 00 0f 85 43 13 fc ff 41 0f b6 86 b8 04 00 00 41
[ 332.354286] RSP: 0018:ffffc90000147e48 EFLAGS: 00010006
[ 332.356344] RAX: 0000000000000005 RBX: ffff8802c226c9d4 RCX: 0000000000000006
[ 332.358404] RDX: 0000000000000000 RSI: 0000000000000082 RDI: ffff88032dc95570
[ 332.360466] RBP: 0000000000000005 R08: 0000000000000000 R09: ffff88031b3dc840
[ 332.362528] R10: 0000000000000000 R11: 000000031a069602 R12: ffff8802c226ca20
[ 332.364575] R13: ffff8802c2268000 R14: ffff880310661000 R15: 000000000000000a
[ 332.366615] FS: 0000000000000000(0000) GS:ffff88032dc80000(0000) knlGS:0000000000000000
[ 332.368658] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 332.370690] CR2: 00000000000000ec CR3: 000000000200a003 CR4: 00000000003606e0
[ 332.372724] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 332.374773] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 332.376798] Call Trace:
[ 332.378809] process_one_work+0x1a1/0x350
[ 332.380806] worker_thread+0x30/0x380
[ 332.382777] ? wq_update_unbound_numa+0x10/0x10
[ 332.384772] kthread+0x112/0x130
[ 332.386740] ? kthread_create_worker_on_cpu+0x70/0x70
[ 332.388706] ret_from_fork+0x35/0x40
[ 332.390651] Modules linked in: i915(O) vfat fat joydev btusb btrtl btbcm btintel bluetooth ecdh_generic iTCO_wdt wmi_bmof i2c_algo_bit drm_kms_helper intel_rapl syscopyarea sysfillrect x86_pkg_temp_thermal sysimgblt coretemp fb_sys_fops crc32_pclmul drm psmouse pcspkr mei_me mei i2c_i801 lpc_ich mfd_core i2c_core tpm_tis tpm_tis_core thinkpad_acpi wmi tpm rfkill video crc32c_intel serio_raw ehci_pci xhci_pci ehci_hcd xhci_hcd [last unloaded: i915]
[ 332.394963] CR2: 00000000000000ec
This appears to be due to the fact that with an MST topology, not all
intel_connector structs will have ->encoder set. So, fix this by
skipping connectors without encoders in
intel_hpd_irq_storm_reenable_work().
For those wondering, this bug was found on accident while simulating HPD
storms using a Chamelium connected to a ThinkPad T450s (Broadwell).
Changes since v1:
- Check intel_connector->mst_port instead of intel_connector->encoder
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181106213017.14563-3-lyude@redhat.com
|
|
We're doing a pointless translation from hpd_pin to port simply for
passing the thing to long_pulse_detect(). Let's pass the hpd_pin
directly instead.
This removes the assumption that the hpd_pin and port always
match. The only other place where we make that assumption anymore
is intel_hpd_pin_default() and that's fine as it's what determines
the relationship between the two. If we ever get hardware where
the hpd pins are wired in more interesting ways it should be
trivial to handle from now on.
This should also fix the IS_CNL_WITH_PORT_F() case as that mapped
pin E back to port F and passed that to
spt_port_hotplug2_long_detect() which would always return false
for port F. Now that we pass in pin E directly it'll actually
do the right thing.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Fixes: cf53902f48c3 ("drm/i915/cnl: Add HPD support for Port F.")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180705164357.28512-7-ville.syrjala@linux.intel.com
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
|
|
Use the enum hpd_pin type when talking about HPD pins, and rename the
variable from a very nondescript 'i' to 'pin', a name we already
use in other parts of the code.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180705164357.28512-6-ville.syrjala@linux.intel.com
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
|
|
Instead of looping over ports and hpd_pins, let's loop over
the encoders when doing hotplug processing. And instead of
depending on dev_priv->irq_port[] to tell us whether the
encoder has the ->hpd_pulse() hook or not, we can just
check for that directly. So we can just nuke irq_port[] entirely.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180705164357.28512-5-ville.syrjala@linux.intel.com
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
|
|
Extend enum hpd_pin to port F so that we can start using this for ICL.
v2: Rebase.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180323172419.24911-6-paulo.r.zanoni@intel.com
|
|
The LG 4k TV I have doesn't deassert HPD when I turn the TV off, but
when I turn it back on it will pulse the HPD line. By that time it has
forgotten everything we told it about scrambling and the clock ratio.
Hence if we want to get a picture out if it again we have to tell it
whether we're currently sending scrambled data or not. Implement
that via the encoder->hotplug() hook.
v2: Force a full modeset to not follow the HDMI 2.0 spec more
closely (Shashank)
[pushed with whitespace fixes to make sparse happy]
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Lyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180117192149.17760-1-ville.syrjala@linux.intel.com
|
|
On CNP boards that are using DDI F,
bit 25 (SDE_PORTE_HOTPLUG_SPT) is representing
the Digital Port F hotplug line when the Digital
Port F hotplug detect input is enabled.
v2: Reuse all existent structure instead of adding a
new HPD_PORT_F pointing to pin of port E.
v3: Use IS_CNL_WITH_PORT_F so we can start upstreaming
this right now. If that SKU ever get a proper name
we come back and update it.
v4: Rebase on top of digital connected port using encoder
instead of port.
v5: Moved IS_CNL_WITH_PORT_F definition to the PCI IDs patch.
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180129232223.766-8-rodrigo.vivi@intel.com
|
|
The idea is to have an unique place to decide the pin-port
per platform.
So let's create this function now without any functional
change. Just adding together code from hdmi and dp together.
v2: Add missing pin for port A.
v3: Fix typo on subject.
Avoid behaviour change so add WARN_ON and return
if port A on HDMI. (by DK).
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170811182650.14327-2-rodrigo.vivi@intel.com
|
|
We will soon need to make that pin port association per
platform, so let's try to simplify it beforehand.
Also we are moving the backwards port to pin
here as well so let's use a standardized way.
One extra possibility here would be to add a
MISSING_CASE along with PORT_NONE, but I don't want
to change this behaviour for now.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170811182650.14327-1-rodrigo.vivi@intel.com
|
|
mode_valid() called from drm_helper_probe_single_connector_modes()
may need to look at connector->state because what a valid mode is may
depend on connector properties being set. For example some HDMI modes
might be rejected when a connector property forces the connector
into DVI mode.
Some implementations of detect() already lock all state,
so we have to pass an acquire_ctx to them to prevent a deadlock.
This means changing the function signature of detect() slightly,
and passing the acquire_ctx for locking multiple crtc's.
For the callbacks, it will always be non-zero. To allow callers
not to worry about this, drm_helper_probe_detect_ctx is added
which might handle -EDEADLK for you.
Changes since v1:
- Always set ctx parameter.
Changes since v2:
- Always take connection_mutex when probing.
Changes since v3:
- Remove the ctx from intel_dp_long_pulse, and add
WARN_ON(!connection_mutex) (danvet)
- Update docs to clarify the locking situation. (danvet)
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1491504920-4017-1-git-send-email-maarten.lankhorst@linux.intel.com
|
|
Nothing special, just rote conversion.
v2: Rebase onto the iter_get/put->iter_begin/end rename.
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170301095226.30584-2-daniel.vetter@ffwll.ch
|
|
assert_spin_locked() becomes an unconditionally compiled BUG_ON(),
adding debug code right into the heart of critical routines like
interrupt handlers.
text data bss dec hex
1296480 19944 2272 1318696 141f28 before (lockdep disabled)
1295984 19944 2272 1318200 141d38 after
1336261 21139 3208 1360608 14c2e0 before (lockdep enabled)
1339920 21139 3208 1364267 14d12b after
Small saving for release; hopefully more instructive in debug.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170302132801.599-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
|
|
In order to prevent accessing the hpd registers outside of the display
power wells, we should refrain from writing to the registers before the
display interrupts are enabled.
[ 4.740136] WARNING: CPU: 1 PID: 221 at drivers/gpu/drm/i915/intel_uncore.c:795 __unclaimed_reg_debug+0x44/0x50 [i915]
[ 4.740155] Unclaimed read from register 0x1e1110
[ 4.740168] Modules linked in: i915(+) intel_gtt drm_kms_helper prime_numbers
[ 4.740190] CPU: 1 PID: 221 Comm: systemd-udevd Not tainted 4.10.0-rc6+ #384
[ 4.740203] Hardware name: / , BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[ 4.740220] Call Trace:
[ 4.740236] dump_stack+0x4d/0x6f
[ 4.740251] __warn+0xc1/0xe0
[ 4.740265] warn_slowpath_fmt+0x4a/0x50
[ 4.740281] ? insert_work+0x77/0xc0
[ 4.740355] ? fwtable_write32+0x90/0x130 [i915]
[ 4.740431] __unclaimed_reg_debug+0x44/0x50 [i915]
[ 4.740507] fwtable_read32+0xd8/0x130 [i915]
[ 4.740575] i915_hpd_irq_setup+0xa5/0x100 [i915]
[ 4.740649] intel_hpd_init+0x68/0x80 [i915]
[ 4.740716] i915_driver_load+0xe19/0x1380 [i915]
[ 4.740784] i915_pci_probe+0x32/0x90 [i915]
[ 4.740799] pci_device_probe+0x8b/0xf0
[ 4.740815] driver_probe_device+0x2b6/0x450
[ 4.740828] __driver_attach+0xda/0xe0
[ 4.740841] ? driver_probe_device+0x450/0x450
[ 4.740853] bus_for_each_dev+0x5b/0x90
[ 4.740865] driver_attach+0x19/0x20
[ 4.740878] bus_add_driver+0x166/0x260
[ 4.740892] driver_register+0x5b/0xd0
[ 4.740906] ? 0xffffffffa0166000
[ 4.740920] __pci_register_driver+0x47/0x50
[ 4.740985] i915_init+0x5c/0x5e [i915]
[ 4.740999] do_one_initcall+0x3e/0x160
[ 4.741015] ? __vunmap+0x7c/0xc0
[ 4.741029] ? kmem_cache_alloc+0xcf/0x120
[ 4.741045] do_init_module+0x55/0x1c4
[ 4.741060] load_module+0x1f3f/0x25b0
[ 4.741073] ? __symbol_put+0x40/0x40
[ 4.741086] ? kernel_read_file+0x100/0x190
[ 4.741100] SYSC_finit_module+0xbc/0xf0
[ 4.741112] SyS_finit_module+0x9/0x10
[ 4.741125] entry_SYSCALL_64_fastpath+0x17/0x98
[ 4.741135] RIP: 0033:0x7f8559a140f9
[ 4.741145] RSP: 002b:00007fff7509a3e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[ 4.741161] RAX: ffffffffffffffda RBX: 00007f855aba02d1 RCX: 00007f8559a140f9
[ 4.741172] RDX: 0000000000000000 RSI: 000055b6db0914f0 RDI: 0000000000000011
[ 4.741183] RBP: 0000000000020000 R08: 0000000000000000 R09: 000000000000000e
[ 4.741193] R10: 0000000000000011 R11: 0000000000000246 R12: 000055b6db0854d0
[ 4.741204] R13: 000055b6db091150 R14: 0000000000000000 R15: 000055b6db035924
v2: Set dev_priv->display_irqs_enabled to true for all platforms other
than vlv/chv that manually control the display power domain.
Fixes: 19625e85c6ec ("drm/i915: Enable polling when we don't have hpd")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97798
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Lyude <cpaul@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
Cc: stable@vger.kernel.org
Link: http://patchwork.freedesktop.org/patch/msgid/20170215131547.5064-1-chris@chris-wilson.co.uk
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
|
|
This adds a file in i915's debugfs directory that allows userspace to
manually control HPD storm detection. This is mainly for hotplugging
tests, where we might want to test HPD storm functionality or disable
storm detection to speed up hotplugging tests without breaking anything.
Changes since v1:
- Make HPD storm interval configurable
- Misc code cleanup
Signed-off-by: Lyude <lyude@redhat.com>
Acked-by: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Tomeu Vizoso <tomeu@tomeuvizoso.net>
|
|
This reverts commit 54a07c7bb0da0343734c78212bbe9f3735394962,
and reinstates the original.
[airlied: this might be a bad plan for git].
commit 3846fd9b86001bea171943cc3bb9222cb6da6b42
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Jan 11 10:01:17 2017 +0100
drm/probe-helpers: Drop locking from poll_enable
It was only needed to protect the connector_list walking, see
commit 8c4ccc4ab6f64e859d4ff8d7c02c2ed2e956e07f
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu Jul 9 23:44:26 2015 +0200
drm/probe-helper: Grab mode_config.mutex in poll_init/enable
Unfortunately the commit message of that patch fails to mention that
the new locking check was for the connector_list.
But that requirement disappeared in
commit c36a3254f7857f1ad9badbe3578ccc92be541a8e
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Thu Dec 15 16:58:43 2016 +0100
drm: Convert all helpers to drm_connector_list_iter
and so we can drop this again.
This fixes a locking inversion on nouveau, where the rpm code needs to
re-enable. But in other places the rpm_get() calls are nested within
the big modeset locks.
While at it, also improve the kerneldoc for these two functions a
notch.
v2: Update the kerneldoc even more to explain that these functions
can't be called concurrently, or bad things happen (Chris).
|
|
A small selection of macros which can only accept dev_priv from
now on and a resulting trickle of fixups.
v2: Keep original order. (Ville Syrjala)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
|
|
Local function with forgotten static declaration.
Fixes: 19625e85c6ec ("drm/i915: Enable polling when we don't have hpd")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Lyude <cpaul@redhat.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1471432146-5196-2-git-send-email-chris@chris-wilson.co.uk
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
|
|
Not sure why so much slips through when 0day is catching these. Hopefully
the much faster sphinx toolchain helps in unlazying people.
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1468612088-9721-10-git-send-email-daniel.vetter@ffwll.ch
|
|
Unfortunately, there's two situations where we lose hpd right now:
- Runtime suspend
- When we've shut off all of the power wells on Valleyview/Cherryview
While it would be nice if this didn't cause issues, this has the
ability to get us in some awkward states where a user won't be able to
get their display to turn on. For instance; if we boot a Valleyview
system without any monitors connected, it won't need any of it's power
wells and thus shut them off. Since this causes us to lose HPD, this
means that unless the user knows how to ssh into their machine and do a
manual reprobe for monitors, none of the monitors they connect after
booting will actually work.
Eventually we should come up with a better fix then having to enable
polling for this, since this makes rpm a lot less useful, but for now
the infrastructure in i915 just isn't there yet to get hpd in these
situations.
Changes since v1:
- Add comment explaining the addition of the if
(!mode_config->poll_running) in intel_hpd_init()
- Remove unneeded if (!dev->mode_config.poll_enabled) in
i915_hpd_poll_init_work()
- Call to drm_helper_hpd_irq_event() after we disable polling
- Add cancel_work_sync() call to intel_hpd_cancel_work()
Changes since v2:
- Apparently dev->mode_config.poll_running doesn't actually reflect
whether or not a poll is currently in progress, and is actually used
for dynamic module paramter enabling/disabling. So now we instead
keep track of our own poll_running variable in dev_priv->hotplug
- Clean i915_hpd_poll_init_work() a little bit
Changes since v3:
- Remove the now-redundant connector loop in intel_hpd_init(), just
rely on intel_hpd_poll_enable() for setting connector->polled
correctly on each connector
- Get rid of poll_running
- Don't assign enabled in i915_hpd_poll_init_work before we actually
lock dev->mode_config.mutex
- Wrap enabled assignment in i915_hpd_poll_init_work() in READ_ONCE()
for doc purposes
- Do the same for dev_priv->hotplug.poll_enabled with WRITE_ONCE in
intel_hpd_poll_enable()
- Add some comments about racing not mattering in intel_hpd_poll_enable
Changes since v4:
- Rename intel_hpd_poll_enable() to intel_hpd_poll_init()
- Drop the bool argument from intel_hpd_poll_init()
- Remove redundant calls to intel_hpd_poll_init()
- Rename poll_enable_work to poll_init_work
- Add some kerneldoc for intel_hpd_poll_init()
- Cross-reference intel_hpd_poll_init() in intel_hpd_init()
- Just copy the loop from intel_hpd_init() in intel_hpd_poll_init()
Changes since v5:
- Minor kerneldoc nitpicks
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Lyude <cpaul@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
|
One of the things preventing us from using polling is the fact that
calling valleyview_crt_detect_hotplug() when there's a VGA cable
connected results in sending another hotplug. With polling enabled when
HPD is disabled, this results in a scenario like this:
- We enable power wells and reset the ADPA
- output_poll_exec does force probe on VGA, triggering a hpd
- HPD handler waits for poll to unlock dev->mode_config.mutex
- output_poll_exec shuts off the ADPA, unlocks dev->mode_config.mutex
- HPD handler runs, resets ADPA and brings us back to the start
This results in an endless irq storm getting sent from the ADPA
whenever a VGA connector gets detected in the middle of polling.
Somewhat based off of the "drm/i915: Disable CRT HPD around force
trigger" patch Ville Syrjälä sent a while back
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Lyude <cpaul@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
|
Since drm_i915_private is now a subclass of drm_device we do not need to
chase the drm_i915_private->dev backpointer and can instead simply
access drm_i915_private->drm directly.
text data bss dec hex filename
1068757 4565 416 1073738 10624a drivers/gpu/drm/i915/i915.ko
1066949 4565 416 1071930 105b3a drivers/gpu/drm/i915/i915.ko
Created by the coccinelle script:
@@
struct drm_i915_private *d;
identifier i;
@@
(
- d->dev->i
+ d->drm.i
|
- d->dev
+ &d->drm
)
and for good measure the dev_priv->dev backpointer was removed entirely.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467711623-2905-4-git-send-email-chris@chris-wilson.co.uk
|
|
I have noticed some of our interrupt handlers use both dev and
dev_priv while they could get away with only dev_priv in the
huge majority of cases.
Tidying that up had a cascading effect on changing functions
prototypes, so relatively big churn factor, but I think it is
for the better.
For example even where changes cascade out of i915_irq.c, for
functions prefixed with intel_, genX_ or <plat>_, it makes more
sense to take dev_priv directly anyway.
This allows us to eliminate local variables and intermixed usage
of dev and dev_priv where only one is good enough.
End result is shrinkage of both source and the resulting binary.
i915.ko:
- .text 000b0899
+ .text 000b0619
Or if we look at the Gen8 display irq chain:
-00000000000006ad t gen8_irq_handler
+0000000000000663 t gen8_irq_handler
-0000000000000028 T intel_opregion_asle_intr
+0000000000000024 T intel_opregion_asle_intr
-000000000000008c t ilk_hpd_irq_handler
+000000000000007f t ilk_hpd_irq_handler
-0000000000000116 T intel_check_page_flip
+0000000000000112 T intel_check_page_flip
-000000000000011a T intel_prepare_page_flip
+0000000000000119 T intel_prepare_page_flip
-0000000000000014 T intel_finish_page_flip_plane
+0000000000000013 T intel_finish_page_flip_plane
-0000000000000053 t hsw_pipe_crc_irq_handler
+000000000000004c t hsw_pipe_crc_irq_handler
-000000000000022e t cpt_irq_handler
+0000000000000213 t cpt_irq_handler
So small shrinkage but it is all fast paths so doesn't harm.
Situation is similar in other interrupt handlers as well.
v2: Tidy intel_queue_rps_boost_for_request as well. (Chris Wilson)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
|
|
This fixes reprobing of display connectors on resume. After some
talking with danvet on IRC, I learned that calling
drm_helper_hpd_irq_event() does actually trigger a full reprobe of each
connector's status. It turns out this is the actual reason reprobing on
resume hasn't been working (this was observed on a T440s):
- We call hpd_init()
- We check each connector for a couple of things before marking
connector->polled with DRM_CONNECTOR_POLL_HPD, one of which is an
active encoder. Of course, a disconnected port won't have an
active encoder, so we don't add the flag to any of the
connectors.
- We call drm_helper_hpd_irq_event()
- drm_helper_irq_event() checks each connector for the
DRM_CONNECTOR_POLL_HPD flag. The only one that has it is eDP-1,
so we skip reprobing each connector except that one.
In addition, we also now avoid setting connector->polled to
DRM_CONNECTOR_POLL_HPD for MST connectors, since their reprobing is
handled by the mst helpers. This is probably what was originally
intended to happen here.
Changes since V1:
* Use the explanation of the issue as the commit message instead
* Change the title of the commit, since this does more then just stop a
check for an encoder now
* Add "Fixes" line for the patch that introduced this regression
* Don't enable DRM_CONNECTOR_POLL_HPD for mst connectors
Changes since V2:
* Put patch changelog above Signed-off-by
* Follow Daniel Vetter's suggestion for making the code here a bit more
legible
Fixes: 0e32b39ceed6 ("drm/i915: add DP 1.2 MST support (v0.7)")
Cc: stable@vger.kernel.org
Signed-off-by: Lyude <cpaul@redhat.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1452181408-14777-1-git-send-email-cpaul@redhat.com
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
|
The cherryview device shares many characteristics with the valleyview
device. When support was added to the driver for cherryview, the
corresponding device info structure included .is_valleyview = 1.
This is not correct and leads to some confusion.
This patch changes .is_valleyview to .is_cherryview in the cherryview
device info structure and simplifies the IS_CHERRYVIEW macro.
Then where appropriate, instances of IS_VALLEYVIEW are replaced with
IS_VALLEYVIEW || IS_CHERRYVIEW or equivalent.
v2: Use IS_VALLEYVIEW || IS_CHERRYVIEW instead of defining a new macro.
Also add followup patches to fix issues discovered during the first
review. (Ville)
v3: Fix some style issues and one gen check. Remove CRT related changes
as CRT is not supported on CHV. (Imre, Ville)
v4: Make a few more optimizations. (Ville)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Wayne Boyer <wayne.boyer@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1449692975-14803-1-git-send-email-wayne.boyer@intel.com
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
|
|
drm_kms_helper_poll_enable() is called from a context in
intel_hpd_irq_storm_disable() where the the mode_config mutex is
already locked.
When this function was converted to lock this mutex in
commit 8c4ccc4ab6f6 ("drm/probe-helper: Grab mode_config.mutex
in poll_init/enable") a deadlock occurred.
Call the newly implemented non-locking version of this function.
Changes since v1:
- use function name suffix '_locked' for the function that
is to be called from a locked context.
Signed-off-by: Egbert Eich <eich@suse.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
v2: fix one error found by checkpath.pl
v3: Add one ignored break for switch-case. DDI-E hotplug
function doesn't work after updating drm-intel tree,
I checked the code and found this missing which isn't
the root cause for broke DDI-E hp. The broken
DDI-E hp function is fixed by "Adding DDI_E power
well domain".
Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Tested-by: Timo Aaltonen <timo.aaltonen@canonical.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Currently HPD_PORT_A is used as an alias for HPD_NONE to mean that the
given port doesn't support long/short HPD pulse detection. SDVO and CRT
ports are like this and for these ports we only want to know whether an
hot plug event was detected on the corresponding pin. Since at least on
BXT we need long/short pulse detection on PORT A as well (added by the
next patch) remove this aliasing of HPD_PORT_A/HPD_NONE and let the
return value of intel_hpd_pin_to_port() show whether long/short pulse
detection is supported on the passed in pin.
No functional change.
v2:
- rebase on top of -nightly (Daniel)
- make the check for intel_hpd_pin_to_port() return value more readable
(Sivakumar)
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Sonika Jindal <sonika.jindal@intel.com>
Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
|
Update the hotplug documentation to explain that hotplug storm
is not expected for Display port panels and hence is not handled
in current code.
v2: update the statements as recommended by Daniel
Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
|
Add an overview of the drm/i915 hotplug handling.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
|
We have enough generic hotplug functions sprinkled all over i915_irq.c
to warrant moving them to a file of their own. This should further
underline the distinction between generic code in the new file and
platform specific hotplug and irq code that remains in i915_irq.c.
Add new intel_hpd_init_work to keep work functions static, and rename
get_port_from_pin to intel_hpd_pin_to_port while increasing its
visibility, but keep everything else the same.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
|
Ville Syrjälä
Lyude Paul
Dhinakaran Pandiyan
Rodrigo Vivi
Maarten Lankhorst
Daniel Vetter
Chris Wilson
Dave Airlie
Tvrtko Ursulin
Lyude
Wayne Boyer
Egbert Eich
Xiong Zhang
Imre Deak
Thulasimani,Sivakumar
Jani Nikula