- 14 May, 2019 12 commits
-
-
Imre Deak authored
We don't need the AUX power for the whole duration of the detect, only when we're doing AUX transfers. The AUX transfer function takes its own reference on the AUX power domain already. The two places during detect which access display core registers (not specific to a pipe/port/transcoder) only need the power domain that is required for that access. That power domain is equivalent to the device global power domain on most platforms (enabled whenever we hold a runtime PM reference) except on CHV/VLV where it's equivalent to the display power well. Add a new power domain that reflects the above, and use this at the two spots accessing registers. With that we can avoid taking the AUX reference for the whole duration of the detect function. Put the domains asynchronously to avoid the unneeded on-off-on toggling. Also adapt the idea from with_intel_runtime_pm et al. for making it easy to write short sequences where a display power ref is needed. v2: (Ville) - Add with_intel_display_power() helper to simplify things. - s/bool res/bool is_connected/ Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Signed-off-by:
Imre Deak <imre.deak@intel.com> Reviewed-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190509173446.31095-8-imre.deak@intel.com
-
Imre Deak authored
We are not calling this function for eDP, so add an early assert about this for clarity. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:
-
Imre Deak authored
In a follow-up patch we will restrict holding the reference on the AUX power domain to the AUX transfer function. To avoid the unnecessary on-off-on power togglings drop the reference asynchronously. There is no reason we couldn't do this in general and also put the reference asynchronously in pps_unlock(); but that's a separate change that can be done as a follow-up. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Signed-off-by:
-
Imre Deak authored
By disabling a power domain asynchronously we can restrict holding a reference on that power domain to the actual code sequence that requires the power to be on for the HW access it's doing, by also avoiding unneeded on-off-on togglings of the power domain (since the disabling happens with a delay). One benefit is potential power saving due to the following two reasons: 1. The fact that we will now be holding the reference only for the necessary duration by the end of the patchset. While simply not delaying the disabling has the same benefit, it has the problem that frequent on-off-on power switching has its own power cost (see the 2. point below) and the debug trace for power well on/off events will cause a lot of dmesg spam (see details about this further below). 2. Avoiding the power cost of freuqent on-off-on power switching. This requires us to find the optimal disabling delay based on the measured power cost of on->off and off->on switching of each power well vs. the power of keeping the given power well on. In this patchset I'm not providing this optimal delay for two reasons: a) I don't have the means yet to perform the measurement (with high enough signal-to-noise ratio, or with the help of an energy counter that takes switching into account). I'm currently looking for a way to measure this. b) Before reducing the disabling delay we need an alternative way for debug tracing powerwell on/off events. Simply avoiding/throttling the debug messages is not a solution, see further below. Note that even in the case where we can't measure any considerable power cost of frequent on-off switching of powerwells, it still would make sense to do the disabling asynchronously (with 0 delay) to avoid blocking on the disabling. On VLV I measured this disabling time overhead to be 1ms on average with a worst case of 4ms. In the case of the AUX power domains on ICL we would also need to keep the sequence where we hold the power reference short, the way it would be by the end of this patchset where we hold it only for the actual AUX transfer. Anything else would make the locking we need for ICL TypeC ports (whenever we hold a reference on any AUX power domain) rather problematic, adding for instance unnecessary lockdep dependencies to the required TypeC port lock. I chose the disabling delay to be 100msec for now to avoid the unneeded toggling (and so not to introduce dmesg spamming) in the DP MST sideband signaling code. We could optimize this delay later, once we have the means to measure the switching power cost (see above). Note that simply removing/throttling the debug tracing for power well on/off events is not a solution. We need to know the exact spots of these events and cannot rely only on incorrect register accesses caught (due to not holding a wakeref at the time of access). Incorrect powerwell enabling/disabling could lead to other problems, for instance we need to keep certain powerwells enabled for the duration of modesets and AUX transfers. v2: - Clarify the commit log parts about power cost measurement and the problem of simply removing/throttling debug tracing. (Chris) - Optimize out local wakeref vars at intel_runtime_pm_put_raw() and intel_display_power_put_async() call sites if CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n. (Chris) - Rebased on v2 of the wakeref w/o power-on guarantee patch. - Add missing docbook headers. v3: - Checkpatch spelling/missing-empty-line fix. v4: - Fix unintended local wakeref var optimization when using call-arguments with side-effects, by using inline funcs instead of macros. In this patch in particular this will fix the intel_display_power_grab_async_put_ref()->intel_runtime_pm_put_raw() call). No size change in practice (would be the same disregarding the corresponding change in intel_display_power_grab_async_put_ref()): $ size i915-macro.ko text data bss dec hex filename 2455190 105890 10272 2571352 273c58 i915-macro.ko $ size i915-inline.ko text data bss dec hex filename 2455195 105890 10272 2571357 273c5d i915-inline.ko Kudos to Stan for reporting the raw-wakeref WARNs this issue caused. His config has CONFIG_DRM_I915_DEBUG_RUNTIME_PM=n, which I didn't retest after v1, and we are also not testing this config in CI. Now tested both with CONFIG_DRM_I915_DEBUG_RUNTIME_PM=y/n on ICL, connecting both Chamelium and regular DP, HDMI sinks. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Signed-off-by:Chris Wilson <chris@chris-wilson.co.uk> Link: https://patchwork.freedesktop.org/patch/msgid/20190513192533.12586-1-imre.deak@intel.com
-
Imre Deak authored
There is no reason why we couldn't verify the power domains state during suspend in all cases, so do that. I overlooked this when originally adding the check. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Imre Deak authored
Make sure we print and drop the wakeref tracking info during pm_cleanup even if there are wakeref holders (either raw-wakeref or wakelock holders). Dropping the wakeref tracking means that a late put on the ref will WARN since the wakeref will be unknown, but that is rightly so, since the put is late and we want to catch that case. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Imre Deak authored
It's useful to track runtime PM refs that don't guarantee a device power-on state to the rest of the driver. One such case is holding a reference that will be put asynchronously, during which normal users without their own reference shouldn't access the HW. A follow-up patch will add support for disabling display power domains asynchronously which needs this. For this we can split wakeref_count into a low half-word tracking all references (raw-wakerefs) and a high half-word tracking references guaranteeing a power-on state (wakelocks). Follow-up patches will make use of the API added here. While at it add the missing docbook header for the unchecked display-power and runtime_pm put functions. No functional changes, except for printing leaked raw-wakerefs and wakelocks separately in intel_runtime_pm_cleanup(). v2: - Track raw wakerefs/wakelocks in the low/high half-word of wakeref_count, instead of adding a new counter. (Chris) v3: - Add a struct_member(T, m) helper instead of open-coding it. (Chris) - Checkpatch indentation formatting fix. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Imre Deak authored
Add another ICL-Y PCIID that proved to have only 5 ports to the corresponding PCIID list. Meanwhile I'm trying to get a complete list of all PCIIDs with less than 6 ports and/or get a VBT fix to mark these ports non-existent, but until then the only way is to go one-by-one. This fixes the following error on machines with less than 6 port: [drm:intel_power_well_enable [i915]] enabling AUX F ------------[ cut here ]------------ WARN_ON(intel_wait_for_register(&dev_priv->uncore, regs->driver, (0x1 << ((pw_idx) * 2)), (0x1 << ((pw_idx) * 2)), 1)) (Internal reference: BSpec/Index/20584/Issues, HSD/1306084116) Cc: Mika Kahola <mika.kahola@intel.com> References: https://bugs.freedesktop.org/show_bug.cgi?id=108915Signed-off-by:
Mika Kahola <mika.kahola@intel.com> Tested-by:
-
Vandita Kulkarni authored
In case of dual link mode, the mode clock that we get from the VBT is halved. v2: Simplify the calculation (Jani). Signed-off-by:
Vandita Kulkarni <vandita.kulkarni@intel.com> Signed-off-by:
Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/1556809862-31203-4-git-send-email-vandita.kulkarni@intel.com
-
Vandita Kulkarni authored
Read back the pixel fomrat register and get the bpp. v2: Read the PIPE_MISC register (Jani). Signed-off-by:
-
Vandita Kulkarni authored
Move bdw_get_pipemisc_bpp alongside bdw_set_pipemisc Signed-off-by:
-
Vandita Kulkarni authored
Adjust the get transcoder timings for mipi dsi as per the set timing calculations. v2: Use the existing intel_get_pipe_timings and do the dsi specific adjustments in the encoder get_config hook.(Ville, Jani) v3: Exclude VBLANK and HBLANK registers for dsi transcoder. v4: Fix the incomplete conditional logic. Signed-off-by:
-
- 13 May, 2019 4 commits
-
-
Ramalingam C authored
Below Sparsh warnings are fixed. Commit: drm: revocation check at drm subsystem +drivers/gpu/drm/drm_hdcp.c:235:6: warning: symbol 'drm_hdcp_request_srm' was not declared. Should it be static? +drivers/gpu/drm/drm_hdcp.c:27:3: warning: symbol 'srm_data' was not declared. Should it be static? +drivers/gpu/drm/drm_hdcp.c:317:5: warning: symbol 'drm_setup_hdcp_srm' was not declared. Should it be static? +drivers/gpu/drm/drm_hdcp.c:327:6: warning: symbol 'drm_teardown_hdcp_srm' was not declared. Should it be static? cc: Daniel Vetter <daniel.vetter@ffwll.ch> Reported-by:
kbuild test robot <lkp@intel.com> Signed-off-by:
Ramalingam C <ramalingam.c@intel.com> Acked-by:
Dave Airlie <airlied@gmail.com> Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Link: https://patchwork.freedesktop.org/patch/msgid/20190513133504.18612-1-ramalingam.c@intel.com
-
Chris Wilson authored
In all likelihood, the priority and node are already in the CPU cache and by checking them first, we can avoid having to chase the *request->hwsp for the current breadcrumb. Signed-off-by:
Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190513120102.29660-3-chris@chris-wilson.co.uk
-
Chris Wilson authored
To simplify the next patch, update bump_priority and schedule to accept the internal i915_sched_ndoe directly and not expect a request pointer. add/remove: 0/0 grow/shrink: 2/1 up/down: 8/-15 (-7) Function old new delta i915_schedule_bump_priority 109 113 +4 i915_schedule 50 54 +4 __i915_schedule 922 907 -15 v2: Adopt node for the old rq local, since it no longer is a request but the origin node. Signed-off-by:
-
Chris Wilson authored
To avoid pulling in a forward declaration in the next patch, move the i915_sched_node handling to after the main dfs of the scheduler. Signed-off-by:
-
- 09 May, 2019 9 commits
-
-
Ville Syrjälä authored
Convert the HSW pch_pfit.force_thru to a proper state variable with readout and accompanying pipe conf check. Makes the logic a bit more straightforward, and hopefully prevents some breakage in the future. 'force_thru' is probably not the best name for this, but I didn't manage to come up with anything better either, so I left it alone. Signed-off-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
Ville Syrjälä authored
On HSW the pipe A panel fitter lives inside the display power well, and the input MUX for the EDP transcoder needs to be configured appropriately to route the data through the power well as needed. Changing the MUX setting is not allowed while the pipe is active, so we need to force a full modeset whenever we need to change it. Currently we may end up doing a fastset which won't change the MUX settings, but it will drop the power well reference, and that kills the pipe. Cc: stable@vger.kernel.org Cc: Hans de Goede <hdegoede@redhat.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Fixes: d19f958d ("drm/i915: Enable fastset for non-boot modesets.") Signed-off-by:
-
Daniel Drake authored
On many (all?) the Gemini Lake systems we work with, there is frequent momentary graphical corruption at the top of the screen, and it seems that disabling framebuffer compression can avoid this. The ticket was reported 6 months ago and has already affected a multitude of users, without any real progress being made. So, lets disable framebuffer compression on GeminiLake until a solution is found. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108085 Fixes: fd7d6c5c ("drm/i915: enable FBC on gen9+ too") Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: <stable@vger.kernel.org> # v4.11+ Reviewed-by:
Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by:
Daniel Drake <drake@endlessm.com> Signed-off-by:
Jian-Hong Pan <jian-hong@endlessm.com> Signed-off-by:
-
Ramalingam C authored
Considering the significant size of hdcp related code in drm, all hdcp related codes are moved into separate file called drm_hdcp.c. v2: Rebased. v2: Rebased. Signed-off-by:
Daniel Vetter <daniel@ffwll.ch> Reviewed-by:
-
Ramalingam C authored
DRM HDCP SRM revocation check services are used from I915 for HDCP1.4 and 2.2 revocation check during the respective authentication flow. v2: Rebased. v3: %s/*_ksvs_revocated/*_check_ksvs_revoked [Daniel] unwanted noise is removed. Signed-off-by:
-
Ramalingam C authored
On every hdcp revocation check request SRM is read from fw file /lib/firmware/display_hdcp_srm.bin SRM table is parsed and stored at drm_hdcp.c, with functions exported for the services for revocation check from drivers (which implements the HDCP authentication) This patch handles the HDCP1.4 and 2.2 versions of SRM table. v2: moved the uAPI to request_firmware_direct() [Daniel] v3: kdoc added. [Daniel] srm_header unified and bit field definitions are removed. [Daniel] locking improved. [Daniel] vrl length violation is fixed. [Daniel] v4: s/__swab16/be16_to_cpu [Daniel] be24_to_cpu is done through a global func [Daniel] Unused variables are removed. [Daniel] unchecked return values are dropped from static funcs [Daniel] Signed-off-by:
Satyeshwar Singh <satyeshwar.singh@intel.com> Reviewed-by:
-
Ramalingam C authored
Existing functions for converting a 3bytes(be24) of big endian value into u32 of little endian and vice versa are renamed as s/drm_hdcp2_seq_num_to_u32/drm_hdcp_be24_to_cpu s/drm_hdcp2_u32_to_seq_num/drm_hdcp_cpu_to_be24 Signed-off-by:
-
Ramalingam C authored
Adding the HDCP2.2 capability of HDCP src and sink info into debugfs entry "i915_hdcp_sink_capability" This helps the userspace tests to skip the HDCP2.2 test on non HDCP2.2 sinks. v2: Rebased. Signed-off-by:
-
Ramalingam C authored
Content protection property is created once and stored in drm_mode_config. And attached to all HDCP capable connectors. Signed-off-by:
-
- 08 May, 2019 3 commits
-
-
Chris Wilson authored
Currently there is an underlying assumption that i915_request_unsubmit() is synchronous wrt the GPU -- that is the request is no longer in flight as we remove it. In the near future that may change, and this may upset our signaling as we can process an interrupt for that request while it is no longer in flight. CPU0 CPU1 intel_engine_breadcrumbs_irq (queue request completion) i915_request_cancel_signaling ... ... i915_request_enable_signaling dma_fence_signal Hence in the time it took us to drop the lock to signal the request, a preemption event may have occurred and re-queued the request. In the process, that request would have seen I915_FENCE_FLAG_SIGNAL clear and so reused the rq->signal_link that was in use on CPU0, leading to bad pointer chasing in intel_engine_breadcrumbs_irq. A related issue was that if someone started listening for a signal on a completed but no longer in-flight request, we missed the opportunity to immediately signal that request. Furthermore, as intel_contexts may be immediately released during request retirement, in order to be entirely sure that intel_engine_breadcrumbs_irq may no longer dereference the intel_context (ce->signals and ce->signal_link), we must wait for irq spinlock. In order to prevent the race, we use a bit in the fence.flags to signal the transfer onto the signal list inside intel_engine_breadcrumbs_irq. For simplicity, we use the DMA_FENCE_FLAG_SIGNALED_BIT as it then quickly signals to any outside observer that the fence is indeed signaled. v2: Sketch out potential dma-fence API for manual signaling v3: And the test_and_set_bit() Fixes: 52c0fdb2 ("drm/i915: Replace global breadcrumbs with per-context interrupt tracking") Signed-off-by:
-
Chris Wilson authored
After realising we need to sample RING_START to detect context switches from preemption events that do not allow for the seqno to advance, we can also realise that the seqno itself is just a distance along the ring and so can be replaced by sampling RING_HEAD. v2: Bonus comment for the mystery separate CS_STALL before MI_USER_INTERRUPT Signed-off-by:
Mika Kuoppala <mika.kuoppala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190508080704.24223-1-chris@chris-wilson.co.uk
-
Chris Wilson authored
If the HW fails to ack a change in forcewake status, the machine is as good as dead -- it may recover, but in reality it missed the mmio updates and is now in a very inconsistent state. If it happens, we can't trust the CI results (or at least the fails may be genuine but due to the HW being dead and not the actual test!) so reboot the machine (CI checks for a kernel taint in between each test and reboots if the machine is tainted). Signed-off-by:
-
- 07 May, 2019 12 commits
-
-
Aditya Swarup authored
There is a bug in hdmi_deep_color_possible() - we compare pipe_bpp <= 8*3 which returns true every time for hdmi_deep_color_possible 12 bit deep color mode test in intel_hdmi_compute_config().(Even when the requested color mode is 10 bit through max bpc property) Comparing pipe_bpp with bpc * 3 takes care of this condition where requested max bpc is 10 bit, so hdmi_deep_color_possible with 12 bit returns false when requested max bpc is 10.(Ville) v2:Add suggested by Ville Syrjälä Suggested-by:
Aditya Swarup <aditya.swarup@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Manasi Navare <manasi.d.navare@intel.com> Cc: Clinton Taylor <Clinton.A.Taylor@intel.com> Signed-off-by:
-
Ville Syrjälä authored
For us KBP is 100% identical to SPT. Kill the redundant enum value. Also bspec doesn't talk about KBP either, so this might avoid some confusion when cross checking the code against the spec. Signed-off-by:
-
Chris Wilson authored
Do not treat reset as a normal preemption event and avoid giving the guilty request a priority boost for simply being active at the time of reset. Signed-off-by:
-
Chris Wilson authored
If we couple the scheduler more tightly with the execlists policy, we can apply the preemption policy to the question of whether we need to kick the tasklet at all for this priority bump. v2: Rephrase it as a core i915 policy and not an execlists foible. v3: Pull the kick together. Signed-off-by:
-
Chris Wilson authored
If the user is racing a call to debugfs/i915_drop_caches with ongoing submission from another thread/process, we may never end up idling the GPU and be uninterruptibly spinning in debugfs/i915_drop_caches trying to catch an idle moment. Just flush the work once, that should be enough to park the system under correct conditions. Outside of those we either have a driver bug or the user is racing themselves. Sadly, because the user may be provoking the unwanted situation we can't put a warn here to attract attention to a probable bug. Signed-off-by:
-
Chris Wilson authored
Replace the racy continuation check within retire_work with a definite kill-switch on idling. The race was being exposed by gem_concurrent_blit where the retire_worker would be terminated too early leaving us spinning in debugfs/i915_drop_caches with nothing flushing the retirement queue. Although that the igt is trying to idle from one child while submitting from another may be a contributing factor as to why it runs so slowly... v2: Use the non-sync version of cancel_delayed_work(), we only need to stop it from being scheduled as we independently check whether now is the right time to be parking. Testcase: igt/gem_concurrent_blit Fixes: 79ffac85 ("drm/i915: Invert the GEM wakeref hierarchy") Signed-off-by:
-
Chris Wilson authored
The original intent for the delay before running the idle_work was to provide a hysteresis to avoid ping-ponging the device runtime-pm. Since then we have also pulled in some memory management and general device management for parking. But with the inversion of the wakeref handling, GEM is no longer responsible for the wakeref and by the time we call the idle_work, the device is asleep. It seems appropriate now to drop the delay and just run the worker immediately to flush the cached GEM state before sleeping. Signed-off-by:
-
Chris Wilson authored
To complete the idle worker, we must complete 2 passes of wait-for-idle. At the end of the first pass, we queue a switch-to-kernel-context and may only idle after waiting for its completion. Speed up the flush_work by doing the wait explicitly, which then allows us to remove the unbounded loop trying to complete the flush_work in the next patch. References: 79ffac85 ("drm/i915: Invert the GEM wakeref hierarchy") Testcase: igt/gem_ppgtt/flind-and-close-vma-leak Signed-off-by:
-
Clinton Taylor authored
v2: Fix commit msg to reflect why issue occurs(Jani) Set GCP_COLOR_INDICATION only when we set 10/12 bit deep color. Changing settings from 10/12 bit deep color to 8 bit(& vice versa) doesn't work correctly using xrandr max bpc property. When we connect a monitor which supports deep color, the highest deep color setting is selected; which sets GCP_COLOR_INDICATION. When we change the setting to 8 bit color, we still set GCP_COLOR_INDICATION which doesn't allow the switch back to 8 bit color. v3,4: Add comments & drop changes in intel_hdmi_compute_config(Ville) Since HSW+, GCP_COLOR_INDICATION is not required for 8bpc. Drop the changes in intel_hdmi_compute_config as desired_bpp is needed to change values for pipe_bpp based on bw_constrained flag. v5: Fix missing logical && in condition for setting GCP_COLOR_INDICATION. v6: Fix comment formatting (Ville) v7: Add reviewed by Ville v8: Set GCP_COLOR_INDICATION based on spec: For Gen 7.5 or later platforms, indicate color depth only for deep color modes. Bspec: 8135,7751,50524 Pre DDI platforms, indicate color depth if deep color is supported by sink. Bspec: 7854 Exception: CHERRYVIEW behaves like Pre DDI platforms. Bspec: 15975 Check pipe_bpp is less than bpp * 3 in hdmi_deep_color_possible, to not set 12 bit deep color for every modeset. This fixes the issue where 12 bit color was selected even when user selected 10 bit.(Ville) v9: Maintain a consistent behavior for all platforms and support GCP_COLOR_INDICATION only when we are in deep color mode. Remove hdmi_sink_is_deep_color() - no longer needed as checking pipe_bpp > 24 takes care of the deep color mode scenario. Separate patch for fixing switch from 12 bit to 10 bit deep color mode. Co-developed-by:
Clinton Taylor <Clinton.A.Taylor@intel.com> Signed-off-by:
-
Chris Wilson authored
Due to the asynchronous tasklet and recursive GT wakeref, it may happen that we submit to the engine (underneath it's own wakeref) prior to the central wakeref being marked as taken. Switch to checking the local wakeref for greater consistency. Fixes: 79ffac85 ("drm/i915: Invert the GEM wakeref hierarchy") Signed-off-by:
-
Chris Wilson authored
The counter goes to zero at the start of the parking cycle, but the wakeref itself is held until the end. Likewise, the counter becomes one at the end of the unparking, but the wakeref is taken first. If we check the wakeref instead of the counter, we include the unpark/unparking time as intel_wakeref_is_active(), and do not spuriously declare inactive if we fail to park (i.e. the parking and wakeref drop is postponed). Signed-off-by:
-
Chris Wilson authored
Inside the signal handler, we expect the requests to be ordered by their breadcrumb such that no later request may be complete if we find an earlier incomplete. Add an assert to check that the next breadcrumb should not be logically before the current. v2: Move the overhanging line into its own function and reuse it after doing the insertion. Signed-off-by:
-
