- 04 Oct, 2018 4 commits
-
-
Ville Syrjälä authored
Plane sanitation needs vblank interrupts (on account of CxSR disable). So let's restore vblank interrupts earlier. v2: Make it actually build v3: Add comment to explain why we need this (Daniel) Cc: stable@vger.kernel.org Cc: Dennis <dennis.nezic@utoronto.ca> Tested-by:
Dennis <dennis.nezic@utoronto.ca> Tested-by:
Peter Nowee <peter.nowee@gmail.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105637 Fixes: b1e01595 ("drm/i915: Redo plane sanitation during readout") Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20181003144951.4397-1-ville.syrjala@linux.intel.com
-
Ville Syrjälä authored
Always print out the information whether the port and sink can each do MST. And let's include the modparam in the debug output as well. Makes life a little less confusing when you don't have to wonder why MST isn't kicking in. This does cause a slight change in our behaviour towards the sink. Previously we only read the MSTM_CAP register after passing all the other checks. Now we will read that register regardless. Hopefully some crazy sink doesn't get confused by a simple register read. Signed-off-by:
Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
-
Daniele Ceraolo Spurio authored
If the HW has not processed the db invalidation request yet, clearing the cookie can generate a db ring. We clear the cookie when we (re-)allocate the doorbell so no need to do it on destroy as well as no one is going to look at it while the doorbell is inactive v2: fix typo in patch title (Michal) Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by:
Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Reviewed-by:
Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20181002215430.15049-2-daniele.ceraolospurio@intel.com
-
Daniele Ceraolo Spurio authored
GuC stores some data in there, which might be stale after a reset. We already reset the WQ head and tail, but more things are being moved to the descriptor with the interface updates. Instead of trying to track them one by one, always memset and init the descriptors from scratch after GuC is loaded. The code is also reorganized so that the above operations and the doorbell creation are grouped as "client enabling" v2: add proc_desc_fini for symmetry (Daniele), remove unneeded var init, add guc_is_alive() (Michal) Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Signed-off-by:
-
- 03 Oct, 2018 4 commits
-
-
Chris Wilson authored
Inside the execlists submission tasklet, we often make the mistake of assuming that everything beneath the request is available for use. However, the submission and the request live on two separate timelines, and the request contents may be freed from an early retirement before we have had a chance to run the submission tasklet (think ksoftirqd). To safeguard ourselves against any mistakes, flush the tasklet before we unpin the context if execlists still has a reference to this context. v2: Pull hw_context->active tracking into schedule_in and schedule_out. References: 60367132 ("drm/i915: Avoid use-after-free of ctx in request tracepoints") Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20181003110941.27886-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
We do not need to continually clear our dedicated PTE for error capture as it will be updated and invalidated to the next object. Only at the end do we wish to be sure that the PTE doesn't point back to any buffer. Signed-off-by:
-
Chris Wilson authored
The final call to zlib_deflate(Z_FINISH) may require more output space to be allocated and so needs to re-invoked. Failure to do so in the current code leads to incomplete zlib streams (albeit intact due to the use of Z_SYNC_FLUSH) resulting in the occasional short object capture. v2: Check against overrunning our pre-allocated page array v3: Drop Z_SYNC_FLUSH entirely Testcase: igt/i915-error-capture.js Fixes: 0a97015d ("drm/i915: Compress GPU objects in error state") Signed-off-by:
-
Chris Wilson authored
As the kthread may terminate itself, the parent must hold a task_struct reference for it to call kthread_stop(). <4> [498.827675] stack segment: 0000 [#1] PREEMPT SMP PTI <4> [498.827683] CPU: 0 PID: 3872 Comm: drv_selftest Tainted: G U 4.19.0-rc6-CI-CI_DRM_4915+ #1 <4> [498.827686] Hardware name: Intel Corporation NUC7CJYH/NUC7JYB, BIOS JYGLKCPX.86A.0027.2018.0125.1347 01/25/2018 <4> [498.827695] RIP: 0010:kthread_stop+0x36/0x210 <4> [498.827698] Code: 05 df 3d f6 7e 89 c0 48 0f a3 05 95 f8 29 01 0f 82 56 01 00 00 f0 ff 43 20 f6 43 26 20 0f 84 7f 01 00 00 48 8b ab b0 05 00 00 <f0> 80 4d 00 02 48 89 df e8 5d ff ff ff 48 89 df e8 15 c7 00 00 48 <4> [498.827701] RSP: 0018:ffffc900003937d0 EFLAGS: 00010202 <4> [498.827704] RAX: 0000000000000001 RBX: ffff8802165ece40 RCX: 0000000000000001 <4> [498.827707] RDX: 0000000000000000 RSI: 00000000ffffffff RDI: ffffffff82247460 <4> [498.827709] RBP: 6b6b6b6b6b6b6b6b R08: 00000000581395cb R09: 0000000000000001 <4> [498.827711] R10: 0000000000000000 R11: 0000000000000000 R12: ffffc90000393868 <4> [498.827713] R13: ffffc900003937f0 R14: ffff88026c068040 R15: 0000000000001057 <4> [498.827716] FS: 00007fc0c464b980(0000) GS:ffff880277e00000(0000) knlGS:0000000000000000 <4> [498.827718] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 <4> [498.827720] CR2: 000056178c2feca0 CR3: 000000026983c000 CR4: 0000000000340ef0 <4> [498.827723] Call Trace: <4> [498.827824] smoke_crescendo+0x14c/0x1d0 [i915] <4> [498.827837] ? _raw_spin_unlock_irqrestore+0x4c/0x60 <4> [498.827898] ? __i915_gem_context_pin_hw_id+0x69/0x5f0 [i915] <4> [498.827902] ? ida_alloc_range+0x1f2/0x3d0 <4> [498.827907] ? __mutex_unlock_slowpath+0x46/0x2b0 <4> [498.827914] ? rcu_lockdep_current_cpu_online+0x8f/0xd0 <4> [498.827979] live_preempt_smoke+0x2c2/0x470 [i915] <4> [498.828047] __i915_subtests+0x5e/0xf0 [i915] <4> [498.828113] __run_selftests+0x10b/0x190 [i915] <4> [498.828175] i915_live_selftests+0x2c/0x60 [i915] <4> [498.828232] i915_pci_probe+0x50/0xa0 [i915] <4> [498.828238] pci_device_probe+0xa1/0x130 <4> [498.828244] really_probe+0x25d/0x3c0 <4> [498.828249] driver_probe_device+0x10a/0x120 <4> [498.828253] __driver_attach+0xdb/0x100 <4> [498.828256] ? driver_probe_device+0x120/0x120 <4> [498.828259] bus_for_each_dev+0x74/0xc0 <4> [498.828264] bus_add_driver+0x15f/0x250 <4> [498.828268] ? 0xffffffffa00c3000 <4> [498.828271] driver_register+0x56/0xe0 <4> [498.828274] ? 0xffffffffa00c3000 <4> [498.828278] do_one_initcall+0x58/0x2e0 <4> [498.828281] ? rcu_lockdep_current_cpu_online+0x8f/0xd0 <4> [498.828285] ? do_init_module+0x1d/0x1ea <4> [498.828289] ? rcu_read_lock_sched_held+0x6f/0x80 <4> [498.828293] ? kmem_cache_alloc_trace+0x264/0x290 <4> [498.828297] do_init_module+0x56/0x1ea <4> [498.828302] load_module+0x26f5/0x29d0 <4> [498.828309] ? vfs_read+0x122/0x140 <4> [498.828318] ? __se_sys_finit_module+0xd3/0xf0 <4> [498.828321] __se_sys_finit_module+0xd3/0xf0 <4> [498.828329] do_syscall_64+0x55/0x190 <4> [498.828332] entry_SYSCALL_64_after_hwframe+0x49/0xbe <4> [498.828335] RIP: 0033:0x7fc0c3f16839 Fixes: 992d2098 ("drm/i915/selftests: Split preemption smoke test into threads") Signed-off-by:
-
- 02 Oct, 2018 6 commits
-
-
Dhinakaran Pandiyan authored
We have new tests and fixes in place since the feature was last disabled. Try again for gen-9+ hardware and enable only PSR1 by default as a first step. v2: Remove typo fix and comment improvements (Rodrigo) Cc: Jani Nikula <jani.nikula@intel.com> Cc: Jose Roberto de Souza <jose.souza@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> References: commit 2ee7dc49 ("drm/i915: disable PSR by default on HSW/BDW") References: commit dcb2e993 ("Revert "drm/i915: Enable PSR by default on Valleyview and Cherryview."") Signed-off-by:
Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by:
José Roberto de Souza <jose.souza@intel.com> Tested-by:
-
Andi Shyti authored
During driver load it's considered that the i915_driver_create() function fails only in case of insufficient memory. Indeed, in case of failure of i915_driver_create(), the load function returns indiscriminately -ENOMEM ignoring the real cause of failure. In i915_driver_create() get the consistent error value from drm_dev_init() and embed it in the pointer return value. Signed-off-by:
Andi Shyti <andi.shyti@intel.com> Reviewed-by:
-
Chris Wilson authored
Previously we hesitated in adding the hw probe for the actual GPU frequency for rps_boost as it is quite cumbersome, but given some surprising HW behaviour it would be useful to know both the RPS boost state and the actual HW state in one location. v2: vlv/chv needs more tlc Reported-by:
Tomi Sarvela <tomi.p.sarvela@intel.com> Signed-off-by:
-
Chris Wilson authored
A few callsites were deciding on using WC or WB maps based on HAS_LLC(), so replace them with the equivalent helper function i915_coherent_map_type(). Signed-off-by:
-
Maarten Lankhorst authored
Add plane alpha blending support with the different blend modes. This has been tested on a icl to show the correct results, on earlier platforms small rounding errors cause issues. But this already happens case with fully transparant or fully opaque RGB8888 fb's. The recommended HW workaround is to disable alpha blending when the plane alpha is 0 (transparant, hide plane) or 0xff (opaque, disable blending). This is easy to implement on any platform, so just do that. The tests for userspace are also available, and pass on gen11. Changes since v1: - Change mistaken < 0xff0 to 0xff00. - Only set PLANE_KEYMSK_ALPHA_ENABLE when plane alpha < 0xff00, ignore blend mode. - Rework disabling FBC when per pixel alpha is used. Signed-off-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> [mlankhorst: Change MISSING_CASE default to explicit alpha disable (mattrope)] Link: https://patchwork.freedesktop.org/patch/msgid/20180815103405.22679-1-maarten.lankhorst@linux.intel.comReviewed-by:
Matt Roper <matthew.d.roper@intel.com>
-
Jyoti Yadav authored
As DMC Package contain DMC FW for multiple steppings including default stepping. This patch will help to load FW for that particular stepping, if FW for that stepping is available, instead of loading default FW. v2 : Fix formatting issue. Signed-off-by:
Jyoti Yadav <jyoti.r.yadav@intel.com> Reviewed-by:
Imre Deak <imre.deak@intel.com> Signed-off-by:
-
- 01 Oct, 2018 9 commits
-
-
Chris Wilson authored
Latency is in the eye of the beholder. In the case where a client stops and waits for the gpu, give that request chain a small priority boost (not so that it overtakes higher priority clients, to preserve the external ordering) so that ideally the wait completes earlier. v2: Tvrtko recommends to keep the boost-from-user-stall as small as possible and to allow new client flows to be preferred for interactivity over stalls. Testcase: igt/gem_sync/switch-default Signed-off-by:
-
Chris Wilson authored
Currently, the backend scheduling code abuses struct_mutex into order to have a global lock to manipulate a temporary list (without widespread allocation) and to protect against list modifications. This is an extraneous coupling to struct_mutex and further can not extend beyond the local device. Pull all the code that needs to be under the one true lock into i915_scheduler.c, and make it so. Signed-off-by:
-
Chris Wilson authored
Taken from an idea used for FQ_CODEL, we give the first request of a new request flows a small priority boost. These flows are likely to correspond with short, interactive tasks and so be more latency sensitive than the longer free running queues. As soon as the client has more than one request in the queue, further requests are not boosted and it settles down into ordinary steady state behaviour. Such small kicks dramatically help combat the starvation issue, by allowing each client the opportunity to run even when the system is under heavy throughput load (within the constraints of the user selected priority). v2: Mark the preempted request as the start of a new flow, to prevent a single client being continually gazumped by its peers. Testcase: igt/benchmarks/rrul Signed-off-by:
-
Ville Syrjälä authored
Make life simpler by passing around intel_encoder instead of drm_encoder. @r1@ identifier F =~ "infoframe"; identifier I, M; @@ F( - struct drm_encoder *I + struct intel_encoder *I , ...) { <... ( - I->M + I->base.M | - I + &I->base ) ...> } @r2@ identifier F =~ "infoframe"; identifier I; type T, ST; @@ ST { ... T (*F)( - struct drm_encoder *I + struct intel_encoder *encoder , ...); ... }; @@ identifier r1.F; expression E; @@ F( - E + to_intel_encoder(E) ,...) @@ identifier r2.F; expression E, X; @@ ( X.F( - E + to_intel_encoder(E) ,...) | X->F( - E + to_intel_encoder(E) ,...) ) @@ expression E; @@ ( - to_intel_encoder(&E->base) + E | - to_intel_encoder(&E->base.base) + &E->base ) @@ identifier D, M; expression E; @@ D = enc_to_dig_port(&E->base) <... ( - D->base.M + E->M | - &D->base + E ) ...> @@ identifier D; expression E; type T; @@ - T D = enc_to_dig_port(E); ... when != D Signed-off-by: -
Ville Syrjälä authored
Replace the hand rolled memmove() with the real thing. Signed-off-by:
-
Chris Wilson authored
As we are about to allow ourselves to slightly bump the user priority into a few different sublevels, packthose internal priority lists into the same i915_priolist to keep the rbtree compact and avoid having to allocate the default user priority even after the internal bumping. The downside to having an requests[] rather than a node per active list, is that we then have to walk over the empty higher priority lists. To compensate, we track the active buckets and use a small bitmap to skip over any inactive ones. v2: Use MASK of internal levels to simplify our usage. v3: Prevent overflow when SHIFT is zero. Signed-off-by:
-
Chris Wilson authored
In the next few patches, we will want to give a small priority boost to some requests/queues but not so much that we perturb the user controlled order. As such we will shift the user priority bits higher leaving ourselves a few low priority bits for our internal bumping. Signed-off-by:
-
Chris Wilson authored
Include a batch full of a page of arbitration points in order to provide a window for inject_preempt_context() in the preemption smoketests. Signed-off-by:
-
Chris Wilson authored
When submitting chains to each engine, we can do so (mostly) in parallel, so delegate submission to threads on a per-engine basis. Signed-off-by:
-
- 28 Sep, 2018 7 commits
-
-
Jani Nikula authored
We've opted to use the maximum link rate and lane count for eDP panels, because typically the maximum supported configuration reported by the panel has matched the native resolution requirements of the panel, and optimizing the link has lead to problems. With eDP 1.4 rate select method and DSC features, this is decreasingly the case. There's a need to optimize the link parameters. Moreover, already eDP 1.3 states fast link with fewer lanes is preferred over the wide and slow. (Wide and slow should still be more reliable for longer cable lengths.) Additionally, there have been reports of panels failing on arbitrary link configurations, although arguably all configurations they claim to support should work. Optimize eDP 1.4+ link config fast and narrow. Side note: The implementation has a near duplicate of the link config function, with just the two inner for loops turned inside out. Perhaps there'd be a way to make this, say, more table driven to reduce the duplication, but seems like that would lead to duplication in the table generation. We'll also have to see how the link config optimization for DSC turns out. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Manasi Navare <manasi.d.navare@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Matt Atwood <matthew.s.atwood@intel.com> Cc: "Lee, Shawn C" <shawn.c.lee@intel.com> Acked-by:
Manasi Navare <manasi.d.navare@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105267Signed-off-by:
Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180905095321.13843-1-jani.nikula@intel.com
-
Dhinakaran Pandiyan authored
There are two copies of the same code called from long and short pulse handlers. v2: Rebase due to s/int status/enum drm_connector_status in intel_dp_detect() Signed-off-by:
-
Dhinakaran Pandiyan authored
The intel_dp->detect_done flag is no more useful. Pull intel_dp_long_pulse() into the lone caller, Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:
-
Dhinakaran Pandiyan authored
A crtc modeset lock was added for link retraining but intel_dp_retrain_link() knows to take the necessary locks since commit c85d200e ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook") v2: Drop AUX power domain reference in the early return path Fixes: c85d200e ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook") Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: José Roberto de Souza <jose.souza@intel.com> Signed-off-by:
-
Dhinakaran Pandiyan authored
We have two cases of intel_dp to intel_encoder conversions, use a local variable to store the conversion. Signed-off-by:
-
Dhinakaran Pandiyan authored
Commit '3cf71bc9 ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"")' applies a work around for sinks that don't signal link loss. The work around does not need to have to be that broad as the issue was seen with only one particular monitor; limit this only for external displays as eDP features like PSR turn off the link and the driver ends up retraining the link seeeing that link is not synchronized. Cc: Lyude Paul <lyude@redhat.com> Cc: Jan-Marek Glogowski <glogow@fbihome.de> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> References: 3cf71bc9 ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"") Signed-off-by:
-
Dhinakaran Pandiyan authored
Comment claims link needs to be retrained because the connected sink raised a long pulse to indicate link loss. If the sink did so, intel_dp_hotplug() would have handled link retraining. Looking at the logs in Bugzilla referenced in commit '3cf71bc9 ("drm/i915: Re-apply Perform link quality check, unconditionally during long pulse"")', the issue is that the sink does not trigger an interrupt. What we want is ->detect() from user space to check link status and retrain. Ville's review for the original patch also indicates the same root cause. So, rewrite the comment. v2: Patch split and rewrote comment. Cc: Lyude Paul <lyude@redhat.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Jan-Marek Glogowski <glogow@fbihome.de> References: 3cf71bc9 ("drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"") Signed-off-by:
-
- 27 Sep, 2018 10 commits
-
-
Chris Wilson authored
If the request is currently on the HW (in port 0), then we do not need to kick the submission tasklet to evaluate whether we should be preempting itself in order to execute it again. In the case that was annoying me: execlists_schedule: rq(18:211173).prio=0 -> 2 need_preempt: last(18:211174).prio=0, queue.prio=2 We are bumping the priority of the first of a pair of requests running in the current context. Then when evaluating preempt, we would see that that our priority request is higher than the last executing request in ELSP0 and so trigger preemption, not realising that our intended request was already executing. v2: As we assume state of the execlists->port[] that is only valid while we hold the timeline lock we have to repeat some earlier tests that on the validity of the node. v3: Wrap guc submission under the timeline.lock as is now the way of all things. Signed-off-by:
-
Jani Nikula authored
Use uniform prefixes for firmware path, version and size. Unify alignments. Order macro groups as in the if ladder using them. Add platform specific max firmware size macros for all platforms for clarity in the if ladder. Place the max firmware size macros in the platform specific macro groups. No functional changes. Reviewed-by:
-
Hans de Goede authored
So far we have only been calling drm_connector_init_panel_orientation_property(), which checks for panel orientation quirks in the drm_panel_orientation_quirks.c file, for DSI panels as so far only devices with DSI panels have had panels which are not mounted up right. The new GPD win2 device uses a portrait screen in a landscape case, so now we've a device with an eDP panel which needs the panel-orientation property to let the fbcon code and userspace know that the image needs to be fixed-up. This commit makes intel_edp_init_connector() call drm_connector_init_panel_orientation_property() so that the property gets added. Reported-and-tested-by: russianneuromancer@ya.ru Reviewed-by:
Hans de Goede <hdegoede@redhat.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180909133457.10636-2-hdegoede@redhat.com
-
Chris Wilson authored
Now that we are confident in providing full-ppgtt where supported, remove the ability to override the context isolation. v2: Remove faked aliasing-ppgtt for testing as it no longer is accepted. v3: s/USES/HAS/ to match usage and reject attempts to load the module on old GVT-g setups that do not provide support for full-ppgtt. v4: Insulate ABI ppGTT values from our internal enum (later plans involve moving ppGTT depth out of the enum, thus potentially breaking ABI unless we document the current values). Signed-off-by:
Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Acked-by:
Zhi Wang <zhi.a.wang@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20180926201222.5643-1-chris@chris-wilson.co.uk
-
Tvrtko Ursulin authored
We mix hexa- and decimal which is confusing when reading the logs. So make the single odd one out instance decimal for consistency. v2: * Do the intel_ringbuffer.c as well. (Chris Wilson) Signed-off-by:
-
Tvrtko Ursulin authored
Partial views are small but there can be many of them, and since the sg list space for them is allocated pessimistically, we can save some slab by trimming the unused tail entries. Signed-off-by:
-
Chris Wilson authored
Very light stress test to bombard the submission backends with a large stream with requests of randomly assigned priorities. Preemption will be occasionally requested, but unlikely to ever succeed! (Although we may build a long queue of requests and so may trigger an attempt to inject a preempt context, as we emit no batch, the arbitration window is limited to between requests inside the ringbuffer. The likelihood of actually causing a preemption event is therefore very small. A later variant should try to improve the likelihood of preemption events!) v2: Include a second pattern with more frequent preemption Signed-off-by:
-
Jani Nikula authored
With i915.dmc_firmware_path="" it's obvious the intention is to disable CSR firmware loading. Bypass the firmware request altogether in this case, with more obvious debug logging. v2: Use DRM_INFO for logging (Chris) Reviewed-by:
-
Jani Nikula authored
Move max firmware size to the same if ladder with firmware name and required version. This allows us to detect the missing max size for a platform without actually loading the firmware, and makes the whole thing easier to maintain. We need to move the power get earlier to allow for early return in the missing platform case. While at it, extend the comment on why we return with the reference held on errors. We also need to move the module parameter override later to reuse the max firmware size, which is independent of the override. v2: Add comment on why we leak the wakeref on errors (Chris) v3: Rebase Reviewed-by:
-
Jani Nikula authored
Having two separate if ladders gets increasingly hard to maintain. Put them together. v2: Rebase Reviewed-by:
-
