On platforms capable of allowing 8K (7680 x 4320) modes, pinning 2 or
more framebuffers/scanout buffers results in only one that is mappable/
fenceable. Therefore, pageflipping between these 2 FBs where only one
is mappable/fenceable creates latencies large enough to miss alternate
vblanks thereby producing less optimal framerate.
This mainly happens because when i915_gem_object_pin_to_display_plane()
is called to pin one of the FB objs, the associated vma is identified
as misplaced -- because there is no space for it in the aperture --
and therefore i915_vma_unbind() is called which unbinds and evicts it.
This misplaced vma gets subseqently pinned only when
i915_gem_object_ggtt_pin_ww() is called without PIN_MAPPABLE. This whole
thing results in a latency of ~10ms and happens every other repaint cycle.
Therefore, to fix this issue, we just ensure that the misplaced VMA
does not get evicted when we try to pin it with PIN_MAPPABLE -- by
returning early if the mappable/fenceable flag is not set.
Testcase:
Running Weston and weston-simple-egl on an Alderlake_S (ADLS) platform
with a 8K@60 mode results in only ~40 FPS (compared to ~59 FPS with
this patch). Since upstream Weston submits a frame ~7ms before the
next vblank, the latencies seen between atomic commit and flip event
are 7, 24 (7 + 16.66), 7, 24..... suggesting that it misses the
vblank every other frame.
Here is the ftrace snippet that shows the source of the ~10ms latency:
i915_gem_object_pin_to_display_plane() {
0.102 us | i915_gem_object_set_cache_level();
i915_gem_object_ggtt_pin_ww() {
0.390 us | i915_vma_instance();
0.178 us | i915_vma_misplaced();
i915_vma_unbind() {
__i915_active_wait() {
0.082 us | i915_active_acquire_if_busy();
0.475 us | }
intel_runtime_pm_get() {
0.087 us | intel_runtime_pm_acquire();
0.259 us | }
__i915_active_wait() {
0.085 us | i915_active_acquire_if_busy();
0.240 us | }
__i915_vma_evict() {
ggtt_unbind_vma() {
gen8_ggtt_clear_range() {
10507.255 us | }
10507.689 us | }
10508.516 us | }
v2:
- Expand the code comments to describe the ping-pong issue.
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220321005431.1113890-1-vivek.kasireddy@intel.com
For modern platforms the spec explicitly states that a
SAGV block time of zero means that SAGV is not supported.
Let's extend that to all platforms. Supposedly there should
be no systems where this isn't true, and it'll allow us to:
- use the same code regardless of older vs. newer platform
- wm latencies already treat 0 as disabled, so this fits well
with other related code
- make it a bit more clear when SAGV is used vs. not
- avoid overflows from adding U32_MAX with a u16 wm0 latency value
which could cause us to miscalculate the SAGV watermarks on tgl+
Cc: stable@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220309164948.10671-2-ville.syrjala@linux.intel.com
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
(cherry picked from commit d8f5855b31)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Throttling here refers to the GT frequency being clipped. Each of
the throttle reason attributes will have a 0 or 1 value depending
upon whether there is throttling and also the specific reason for
it.
The following is a brief description of the sysfs throttle
frequency attributes added:
- throttle_reason_status: when set indicates that there is GT
frequency clipping.
- throttle_reason_pl1: when set indicates that PBM PL1 (platform
or package PL1) has caused GT frequency clipping.
- throttle_reason_pl2: when set indicates that PBM PL2 or PL3
(platform or package PL2 or PL3) has caused GT frequency
clipping.
- throttle_reason_pl4: when set indicates that PL4 or IccMax has
caused GT frequency clipping.
- throttle_reason_thermal: when set indicates that Thermal event
has caused GT frequency clipping.
- throttle_reason_prochot: when set indicates that PROCHOT# has
caused GT frequency clipping.
- throttle_reason_ratl: when set indicates that Running Average
Thermal Limit has caused GT frequency clipping.
- throttle_reason_vr_thermalert: when set indicates that Hot VR
(any processor VR) has caused GT frequency clipping.
- throttle_reason_vr_tdc: when set indicates that VR TDC
(Thermal Design Current) has caused GT frequency clipping.
Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Dale B Stimson <dale.b.stimson@intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220318233938.149744-8-andi.shyti@linux.intel.com
Now tiles have their own sysfs interfaces under the gt/
directory. Because RPS is a property that can be configured on a
tile basis, then each tile should have its own interface
The new sysfs structure will have a similar layout for the 4 tile
case:
/sys/.../card0
├── gt
│ ├── gt0
│ │ ├── id
│ │ ├── rc6_enable
│ │ ├── rc6_residency_ms
│ │ ├── rps_act_freq_mhz
│ │ ├── rps_boost_freq_mhz
│ │ ├── rps_cur_freq_mhz
│ │ ├── rps_max_freq_mhz
│ │ ├── rps_min_freq_mhz
│ │ ├── rps_RP0_freq_mhz
│ │ ├── rps_RP1_freq_mhz
│ │ └── rps_RPn_freq_mhz
. .
. .
. .
│ └── gtN
│ ├── id
│ ├── rc6_enable
│ ├── rc6_residency_ms
│ ├── rps_act_freq_mhz
│ ├── rps_boost_freq_mhz
│ ├── rps_cur_freq_mhz
│ ├── rps_max_freq_mhz
│ ├── rps_min_freq_mhz
│ ├── rps_RP0_freq_mhz
│ ├── rps_RP1_freq_mhz
│ └── rps_RPn_freq_mhz
├── gt_act_freq_mhz -+
├── gt_boost_freq_mhz |
├── gt_cur_freq_mhz | Original interface
├── gt_max_freq_mhz +─-> kept as existing ABI;
├── gt_min_freq_mhz | it points to gt0/
├── gt_RP0_freq_mhz |
├── gt_RP1_freq_mhz |
└── gt_RPn_freq_mhz -+
The existing interfaces have been kept in their original location
to preserve the existing ABI. They act on all the GTs: when
writing they loop through all the GTs and write the information
on each interface. When reading they provide the average value
from all the GTs.
This patch is not really adding exposing new interfaces (new
ABI) other than adapting the existing one to more tiles. In any
case this new set of interfaces will be a basic tool for system
managers and administrators when using i915.
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220318233938.149744-7-andi.shyti@linux.intel.com
Now tiles have their own sysfs interfaces under the gt/
directory. Because RC6 is a property that can be configured on a
tile basis, then each tile should have its own interface
The new sysfs structure will have a similar layout for the 4 tile
case:
/sys/.../card0
├── gt
│ ├── gt0
│ │ ├── id
│ │ ├── rc6_enable
│ │ ├── rc6_residency_ms
. . .
. . .
. .
│ └── gtN
│ ├── id
│ ├── rc6_enable
│ ├── rc6_residency_ms
│ .
│ .
│
└── power/ -+
├── rc6_enable | Original interface
├── rc6_residency_ms +-> kept as existing ABI;
. | it multiplexes over
. | the GTs
-+
The existing interfaces have been kept in their original location
to preserve the existing ABI. They act on all the GTs: when
reading they provide the average value from all the GTs.
This patch is not really adding exposing new interfaces (new
ABI) other than adapting the existing one to more tiles. In any
case this new set of interfaces will be a basic tool for system
managers and administrators when using i915.
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220318233938.149744-6-andi.shyti@linux.intel.com
Add logical mapping for VDBOXs. This mapping is required for
split-frame workloads, which otherwise fail with
00000000-F8C53528: [GUC] 0441-INVALID_ENGINE_SUBMIT_MASK
... if the application is using the logical id to reorder the engines and
then using it for the batch buffer submission. It's not a big problem on
media version 11 and 12 as they have only 2 instances of VCS and the
logical to physical mapping is monotonically increasing - if the
application is not using the logical id.
Changing it for the previous platforms allows the media driver
implementation for the next ones (12.50 and above) to be the same,
checking the logical id. It should also not introduce any bug for the
old versions of userspace not checking the id.
The mapping added here is the complete map needed by XEHPSDV. Previous
platforms with only 2 instances will just use a partial map and should
still work.
v2: Remove static from map variable (José)
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
[ Extend the mapping to media versions 11 and 12 and give proper
justification in the commit message why ]
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Acked-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220316234538.434357-2-lucas.demarchi@intel.com
For modern platforms the spec explicitly states that a
SAGV block time of zero means that SAGV is not supported.
Let's extend that to all platforms. Supposedly there should
be no systems where this isn't true, and it'll allow us to:
- use the same code regardless of older vs. newer platform
- wm latencies already treat 0 as disabled, so this fits well
with other related code
- make it a bit more clear when SAGV is used vs. not
- avoid overflows from adding U32_MAX with a u16 wm0 latency value
which could cause us to miscalculate the SAGV watermarks on tgl+
Cc: stable@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220309164948.10671-2-ville.syrjala@linux.intel.com
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Implement support for fetching the hardware description table from the
GuC. The call is made twice - once without a destination buffer to
query the size and then a second time to fill in the buffer.
The table is stored in the GT structure so that it can be fetched once
at driver load time. Keeping inside a GuC structure would mean it
would be release and reloaded on a GuC reset (part of a full GT
reset). However, the table does not change just because the GT has been
reset and the GuC reloaded. Also, dynamic memory allocations inside
the reset path are a problem.
Note that the table is only available on ADL-P and later platforms.
v2 (John's v2 patch):
* Move to GT level to avoid memory allocation during reset path (and
unnecessary re-read of the table on a reset).
v5 (of Jordan's posting):
* Various changes made by Jordan and recommended by Michal
- Makefile ordering
- Adjust "struct intel_guc_hwconfig hwconfig" comment
- Set Copyright year to 2022 in intel_guc_hwconfig.c/.h
- Drop inline from hwconfig_to_guc()
- Replace hwconfig param with guc in __guc_action_get_hwconfig()
- Move zero size check into guc_hwconfig_discover_size()
- Change comment to say zero size offset/size is needed to get size
- Add has_guc_hwconfig to devinfo and drop has_table()
- Change drm_err to notice in __uc_init_hw() and use %pe
v6 (of Jordan's posting):
* Added a couple more small changes recommended by Michal
* Merge in John's v2 patch, but note:
- Using drm_notice as recommended by Michal
- Reverted Michal's suggestion of using devinfo
v7 (of Jordan's posting):
* Change back to drm_err as preferred by John
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Acked-by: Jon Bloomfield <jon.bloomfield@intel.com>
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220306232157.1174335-2-jordan.l.justen@intel.com
On integrated it looks like the GGTT base should always 1:1 maps to
somewhere within DSM. On discrete the base seems to be pre-programmed with
a normal lmem address, and is not 1:1 mapped with the base address. On
such devices probe the lmem address directly from the PTE.
v2(Ville):
- The base is actually the pre-programmed GGTT address, which is then
meant to 1:1 map to somewhere inside dsm. In the case of dgpu the
base looks to just be some offset within lmem, but this also happens
to be the exact dsm start, on dg1. Therefore we should only need to
fudge the physical address, before allocating from stolen.
- Bail if it's not located in dsm.
v3:
- Scratch that. There doesn't seem to be any relationship with the
base and PTE address, on at least DG1. Let's instead just grab the
lmem address from the PTE itself.
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Nirmoy Das <nirmoy.das@linux.intel.com>
Reviewed-by: Nirmoy Das <nirmoy.das@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220315181425.576828-7-matthew.auld@intel.com
Let's just do a full DRRS disable/enable across all pipe updates.
This guarantees that the DRRS work doesn't interfere with anything
while the atomic commit is busy reprogramming the pipe.
Needed so that we can start reprogramming M/N seamlessly during
fastsets whenever possible. Also avoids the pre-bdw DRRS PIPECONF
rmw racing with the potential PIPECONF write from the atomic
commit (eg. due to GAMMA_MODE changes).
v2: Include has_drrs in state dump (José)
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220315213944.17132-1-ville.syrjala@linux.intel.com
With static DRRS the user might ask for the lowest possible refresh
rate of the panel, in which case we're not going to find a suitable
downclock mode for it and we should not try to enable seamless DRRS.
This will in fact oops.
We used to check for the presence of the downclock mode here, but
that got removed in commit f0a57798fb ("drm/i915: Introduce
intel_panel_drrs_type()") as redundant (which it was at the time).
But we do need the check again now that static DRRS is a thing.
I must have not re-tested static DRRS fully after introducing
intel_panel_drrs_type() :/
Fixes: c5ee23437c ("drm/i915: Implement static DRRS")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220315132752.11849-2-ville.syrjala@linux.intel.com
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
struct drm_display_mode embeds a list head, so overwriting
the full struct with another one will corrupt the list
(if the destination mode is on a list). Use drm_mode_copy()
instead which explicitly preserves the list head of
the destination mode.
Even if we know the destination mode is not on any list
using drm_mode_copy() seems decent as it sets a good
example. Bad examples of not using it might eventually
get copied into code where preserving the list head
actually matters.
Obviously one case not covered here is when the mode
itself is embedded in a larger structure and the whole
structure is copied. But if we are careful when copying
into modes embedded in structures I think we can be a
little more reassured that bogus list heads haven't been
propagated in.
@is_mode_copy@
@@
drm_mode_copy(...)
{
...
}
@depends on !is_mode_copy@
struct drm_display_mode *mode;
expression E, S;
@@
(
- *mode = E
+ drm_mode_copy(mode, &E)
|
- memcpy(mode, E, S)
+ drm_mode_copy(mode, E)
)
@depends on !is_mode_copy@
struct drm_display_mode mode;
expression E;
@@
(
- mode = E
+ drm_mode_copy(&mode, &E)
|
- memcpy(&mode, E, S)
+ drm_mode_copy(&mode, E)
)
@@
struct drm_display_mode *mode;
@@
- &*mode
+ mode
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220218100403.7028-20-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Commit 13ea6db2cf ("drm/i915/edp: Ignore short pulse when panel
powered off") completely broke short pulse handling for eDP as it is
usually generated by sink when it is displaying image and there is
some error or status that source needs to handle.
When power panel is enabled, this state is enough to power aux
transactions and VDD override is disabled, so intel_pps_have_power()
is always returning false causing short pulses to be ignored.
So here better naming this function that intends to check if aux
lines are powered to avoid the endless cycle mentioned in the commit
being fixed and fixing the check for what it is intended.
v2:
- renamed to intel_pps_have_panel_power_or_vdd()
- fixed indentation
Fixes: 13ea6db2cf ("drm/i915/edp: Ignore short pulse when panel powered off")
Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220311185149.110527-1-jose.souza@intel.com
(cherry picked from commit 8f0c1c0949)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
GuC has its own steering mechanism and can't use the default set by i915,
so we need to provide the steering information that the FW will need to
save/restore registers while processing an engine reset. The GUC
interface allows us to do so as part of the register save/restore list
and it requires us to specify the steering for all multicast register, even
those that would be covered by the default setting for cpu access. Given
that we do not distinguish between registers that do not need steering and
registers that are guaranteed to work the default steering, we set the
steering for all entries in the guc list that do not require a special
steering (e.g. mslice) to the default settings; this will cost us a few
extra writes during engine reset but allows us to keep the steering
logic simple.
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220314234203.799268-3-matthew.d.roper@intel.com
