Ville Syrjälä [Fri, 19 Sep 2025 18:50:13 +0000 (21:50 +0300)]
drm/i915/pm: Do pci_restore_state() in switcheroo resume hook
Since this switcheroo garbage bypasses all the core pm we
have to manually manage the pci state. To that end add the
missing pci_restore_state() to the switcheroo resume hook.
We already have the pci_save_state() counterpart on the
suspend side.
Arguably none of this code should exist in the driver
in the first place, and instead the entire switcheroo
mechanism should be rewritten and properly integrated into
core pm code...
Ville Syrjälä [Fri, 19 Sep 2025 18:50:12 +0000 (21:50 +0300)]
drm/i915/pm: Move the hibernate+D3 quirk stuff into noirq() pm hooks
If the driver doesn't call pci_save_state() drivers/pci will
normally save+power manage the device from the _noirq() pm hooks.
We can't let that happen as some old BIOSes fail to hibernate
when the device is in D3. However, we can get very close to
the standard behaviour by doing our explicit pci_save_state()
and pci_set_power_state() stuff from driver provided _noirq()
hooks.
This results in a change of behaviour where we no longer go
into D3 at the end of freeze_late, so when it comes time
to thaw() we'll already be in D0, and thus we can drop the
explicit pci_set_power_state(D0) call.
Presumably switcheroo suspend will want to go into D3 so
call the _noirq() stuff from the switcheroo suspend hook,
and since we dropped the pci_set_power_state(D0) from
resume_early() we'll need to add one back into the
switcheroo resume hook.
Ville Syrjälä [Fri, 19 Sep 2025 18:50:11 +0000 (21:50 +0300)]
drm/i915/pm: Hoist pci_save_state()+pci_set_power_state() to the end of pm _late() hook
drivers/pci does the pci_save_state()+pci_set_power_state() from
the _noirq() pm hooks. Move our manual calls (needed for the
hibernate vs. D3 workaround with buggy BIOSes) towards that same
point. We currently have no _noirq() hooks, so end of _late()
hooks is the best we can do right now.
Ville Syrjälä [Fri, 19 Sep 2025 18:50:10 +0000 (21:50 +0300)]
drm/i915/pm: Simplify pm hook documentation
Stop spelling out each variant of the hook ("" vs. "_late" vs.
"_early") and just say eg. "@thaw*" to indicate all of them.
Avoids having to update the docs whenever we start/stop using
one of the variants.
drm/i915/ddi: Guard reg_val against a INVALID_TRANSCODER
Currently we check if the encoder is INVALID or -1 and throw a
WARN_ON but we still end up writing the temp value which will
overflow and corrupt the whole programmed value.
--v2
-Assign a bogus transcoder to master in case we get a INVALID
TRANSCODER [Jani]
Rename intel_vrr_flipline_offset() to intel_vrr_vmin_flipline_offset()
to better reflect the fact that it gives us the minimum offset allowed
between vmin and flipline.
Ville Syrjälä [Thu, 18 Sep 2025 23:22:25 +0000 (02:22 +0300)]
drm/i915/vrr: Hide the ICL/TGL intel_vrr_flipline_offset() mangling better
ICL/TGL VRR hardware won't allow us to program flipline==vmin. If we do
that the actual effect will be the same as if we had programmed
flipline=vmin+1, which would make the minimum vtotal one scanline taller
than expected.
To compensate for this we reduce vmin by one, and then program
flipline=vmin+1. So we end up with a flipline value that matches
the expected minimum vtotal. Currently this adjustment happens
in intel_vrr_compute_config() which means that crtc_state->vrr.vmin
will no longer be directly usable for the remainder of the high
level VRR code. That is annoying at best, fragile at worst.
Hide the adjustment in low level code instead. This will allow most
of the higher level VRR code to remain blissfully ignorant about this
fact. Afterwards crtc_state->vrr.{vmin,flipline} will be equal
and match the minimum vtotal, exactly how things already work
on ADL+.
The only slight downside is that the actual register value will no
longer match crtc_state->vrr.vmin on ICL/TGL, but that may already
be the case on TGL because the register value will also have been
adjusted by the SCL.
Note that we must change the guardband calculation to account
for intel_vrr_extra_vblank_delay() explicitly. Previously that
was accidentally handled by the earlier vmin reduction by
intel_vrr_flipline_offset().
drm/i915/dmc: explicitly sanitize num_entries from package_header
num_entries comes from package_header, which is read from an external
firmware blob and thus untrusted. In parse_dmc_fw_package() we assign
package_header->num_entries to a local variable, but the range check
still uses the struct field directly.
Switch the check to use the local copy instead. This makes the
sanitization explicit and avoids a redundant dereference.
Jani Nikula [Thu, 18 Sep 2025 13:38:35 +0000 (16:38 +0300)]
drm/i915/irq: add ilk_display_irq_reset()
Abstract ilk_display_irq_reset(), moving display related reset
there. This results in a slightly different order between GT and PCH
reset, hopefully with no impact.
Jani Nikula [Thu, 18 Sep 2025 12:25:45 +0000 (15:25 +0300)]
drm/i915/irq: use a dedicated IMR cache for gen 5-7
There are three groups of platforms using i915->irq_mask independently:
gen 2-4, VLV/CHV, and gen 5-7.
The gen 5-7 usage is primarily limited to display. Move its irq_mask
usage to struct intel_display as ilk_de_imr_mask for gen 5-7.
ilk_de_imr_mask could be put inside a union with with vlv_imr_mask and
de_irq_mask[], but keep them separate to avoid accidental aliasing of
the values.
With this, we can also drop the irq_mask member from struct xe_device.
i915 and xe do different things on the failure path; i915 calls
drm_gem_object_put() while xe calls xe_bo_unpin_map_no_vm(). Add a
helper to enable further refactoring.
v2: Call drm_gem_object_put() on intel_fbdev_fb_bo_destroy()
Jani Nikula [Thu, 18 Sep 2025 08:40:52 +0000 (11:40 +0300)]
drm/i915/fbdev: make intel_framebuffer_create() error return handling explicit
It's sketchy to pass error pointers via to_intel_framebuffer(). It
probably works as long as struct intel_framebuffer embeds struct
drm_framebuffer at offset 0, but be explicit about it.
Jani Nikula [Thu, 18 Sep 2025 08:40:51 +0000 (11:40 +0300)]
drm/xe/fbdev: use the same 64-byte stride alignment as i915
For reasons unknown, xe uses XE_PAGE_SIZE alignment for
stride. Presumably it's just a confusion between stride alignment and bo
allocation size alignment. Switch to 64 byte alignment to, uh, align
with i915.
This will also be helpful in deduplicating and unifying the xe and i915
framebuffer allocation.
Ville Syrjälä [Wed, 17 Sep 2025 20:34:46 +0000 (23:34 +0300)]
drm/i915/vrr: Move the TGL SCL mangling of vmin/vmax/flipline deeper
Currently our crtc_state->vrr.{vmin.vmax,flipline} are mangled on
TGL to account for the SCL delay (the hardware requires this mangling
or the actual vtotals will become incorrect). Unfortunately this
means that one can't simply use these values directly in many places,
and instead we always have to go through functions that undo the
damage first. This is all rather fragile.
Simplify our lives a bit by hiding this mangling deeper inside
the low level VRR code, leaving the number stored in the crtc
state actually something that humans can use.
This does introduce a dependdency as intel_vrr_get_config()
will now need to know the SCL value, which is read out in
intel_get_transcoder_timings(). I suppose we could simply
duplicate the SCL readout in both places should this become
a real hinderance. For now just leave a note around the
intel_get_transcoder_timings() call to remind us.
Ville Syrjälä [Wed, 17 Sep 2025 20:34:45 +0000 (23:34 +0300)]
drm/i915/vrr: Annotate some functions with "hw"
intel_vrr_fixed_rr_*() return values that have had the TGL
SCL adjustment applied to them. So we should indicate that these
values are only really useful when fed to the hardware. Add
a "_hw_" indicator to the function names to reflect that fact.
Ville Syrjälä [Wed, 17 Sep 2025 20:34:44 +0000 (23:34 +0300)]
drm/i915/vrr: Store guardband in crtc state even for icl/tgl
While ICL/TGL VRR hardware doesn't have a register for the guardband
value, our lives will be simpler if we pretend that it does. Start
by computing the guardband the same as on ADL+ and storing it in
the state, and only then we convert it into the corresponding
pipeline_full value that the hardware can consume. During readout we
do the opposite.
I was debating whether to completely remove pipeline_full from the
crtc state, but decided to keep it for now. Mainly because we check
it in vrr_params_changed() and simply checking the guardband instead
isn't 100% equivalent; Theoretically, framestart_delay may have
changed in the opposite direction to pipeline_full, keeping the
derived guardband value unchaged. One solution would be to also check
framestart_delay, but that feels a bit leaky abstraction wise.
Also note that we don't currently handle the maximum limit of 255
scanlines for the pipeline_full in a very nice way. The actual
position of the delayed vblank will move because of that clamping,
and so some of our code may get confused. But fixing this shall
wait a for now.
Ville Syrjälä [Wed, 17 Sep 2025 20:34:43 +0000 (23:34 +0300)]
drm/i915/vrr: Readout framestart_delay earlier
In order to pretend that ICL/TGL VRR hardware has a similar guardband
as on ADL+ we'll need access to framestart_delay already during
intel_vrr_get_config(). Hoist the framestart_delay to an earlier point
to make that possible.
Ville Syrjälä [Wed, 17 Sep 2025 20:34:42 +0000 (23:34 +0300)]
drm/i915/vrr: Extract helpers to convert between guardband and pipeline_full values
I'd like to move towards a world where we can't more or less
pretend that the ICl/TGL VRR hardware works the same way as
ADL+. To that end extract some helpers to convert between
the guardband and pipeline_full representations.
Ville Syrjälä [Fri, 12 Sep 2025 13:59:26 +0000 (16:59 +0300)]
drm/i915: Defeature DRRS on LNL+
DRRS has been defeatured on LNL+. Adjust HAS_DOUBLE_BUFFERED_M_N()
to match.
Note that the M/N registers still appear to be double buffered under
the hood but the double buffer update point is now documented to be
just the last register write to the M/N registers, so it no longer
happens synchronously with the vblank/MSA transmission. We should
perhaps rename HAS_DOUBLE_BUFFERED_M_N() to more accurately reflect
reality, but couldn't come up with a decent name right now...
Jonathan Cavitt [Tue, 16 Sep 2025 17:43:19 +0000 (17:43 +0000)]
drm/i915/gvt: Remove unnecessary check in reg_is_mmio
The reg >= 0 check in reg_is_mmio is unnecessary because reg is always
greater than zero in all current use cases. This is obvious when
checking 'offset' by itself (as offset is defined as an unsigned
integer), but it's also true for the offset + bytes - 1 use case in
intel_vgpu_emulate_mmio_read because bytes > 0.
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Reviewed-by: Zhenyu Wang <zhenyuw.linux@gmail.com> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> Link: https://lore.kernel.org/r/20250916174317.76521-5-jonathan.cavitt@intel.com
Jani Nikula [Wed, 17 Sep 2025 13:52:00 +0000 (16:52 +0300)]
drm/i915: add note on VLV/CHV hpll_freq and czclk_freq caching
The caching at the initial read is a bit fragile in case, say, a further
refactoring starts reading the frequencies at a time where it's not
possible. Add a note about it.
drm/i915/alpm: Remove error handling from get_lfps_cycle_min_max_time
Getter for LFPS cycle min/max times is unnecessarily checking faulty port
clock value. This doesn't make sense as erroneous port clock value would
have been noticed already at this point. Remove this check and use 140/800
ns always when port clock > 540000.
Jani Nikula [Fri, 12 Sep 2025 14:48:51 +0000 (17:48 +0300)]
drm/i915: remove intel_update_czclk() as unnecessary
With vlv_clock_get_czclk() caching the result on first use, we no longer
need a separate initializer. Remove intel_update_czclk() as
unnecessary. Log the CZCLK in vlv_clock_get_czclk() instead.
Aaron Ma [Sat, 23 Aug 2025 12:16:47 +0000 (20:16 +0800)]
drm/i915/backlight: Honor VESA eDP backlight luminance control capability
The VESA AUX backlight fails to be enable luminance based backlight
mainpulation becaused luminance_set is false by default.
Fix it by using luminance support control capabitliy.
Fixes: e13af5166a359 ("drm/i915/backlight: Use drm helper to initialize edp backlight") Signed-off-by: Aaron Ma <aaron.ma@canonical.com> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> Link: https://lore.kernel.org/r/20250823121647.275834-1-aaron.ma@canonical.com
drm/i915: Drop unused struct_mutex from drm_i915_private
The struct_mutex field in drm_i915_private is no longer used anywhere in
the driver. This patch removes it completely to clean up unused code and
avoid confusion.
The struct_mutex will be removed from the DRM subsystem, as it was a
legacy BKL that was only used by i915 driver. After review, it was
concluded that its usage was no longer necessary
This patch updates various comments in the i915 codebase to
either remove or clarify references to struct_mutex, in order to
prevent future misunderstandings.
* i915_drv.h: Removed the statement that stolen_lock is the inner lock
when overlaps with struct_mutex, since struct_mutex is no longer used
in the driver.
* i915_gem.c: Removed parentheses suggesting usage of struct_mutex, which
which is no longer used.
The struct_mutex will be removed from the DRM subsystem, as it was a
legacy BKL that was only used by i915 driver. After review, it was
concluded that its usage was no longer necessary
This patch update a comment about struct_mutex in i915/display, in order
to prevent future misunderstandings.
* intel_fbc.c: Removed the statement that intel_fbc->lock is the inner
lock when overlapping with struct_mutex, since struct_mutex is no
longer used anywhere in the driver.
The struct_mutex will be removed from the DRM subsystem, as it was a
legacy BKL that was only used by i915 driver. After review, it was
concluded that its usage was no longer necessary
This patch updates various comments in the i915/gem and i915/gt
codebase to either remove or clarify references to struct_mutex, in
order to prevent future misunderstandings.
* i915_gem_execbuffer.c: Replace reference to struct_mutex with
vm->mutex, as noted in the eb_reserve() function, which states that
vm->mutex handles deadlocks.
* i915_gem_object.c: Change struct_mutex by
drm_i915_gem_object->vma.lock. i915_gem_object_unbind() in i915_gem.c
states that this lock is who actually protects the unbind.
* i915_gem_shrinker.c: The correct lock is actually i915->mm.obj, as
already documented in its declaration.
* i915_gem_wait.c: The existing comment already mentioned that
struct_mutex was no longer necessary. Updated to refer to a generic
global lock instead.
* intel_reset_types.h: Cleaned up the comment text. Updated to refer to
a generic global lock instead.
Remove the use of struct_mutex from intel_guc_log.c and replace it with
a dedicated lock, guc_lock, defined within the intel_guc_log struct.
The struct_mutex was previously used to protect concurrent access and
modification of intel_guc_log->level in intel_guc_log_set_level().
However, it was suggested that the lock should reside within the
intel_guc_log struct itself.
Initialize the new guc_lock in intel_guc_log_init_early(), alongside the
existing relay.lock. The lock is initialized using drmm_mutex_init(),
which also ensures it is properly destroyed when the driver is unloaded.
drm/i915: Change mutex initialization in intel_guc_log
The intel_guc_log->relay.lock is currently initialized in
intel_guc_log_init_early(), but it lacks a corresponding destructor,
which can lead to a memory leak.
This patch replaces the use of mutex_init() with drmm_mutex_init(),
which ensures the lock is properly destroyed when the driver is
unloaded.
Add a proper function for display && HAS_DISPLAY(display) to hide
indirect struct intel_display access via the macro from a number of
places outside of display. This makes struct intel_display * an opaque
pointer in these places. All HAS_DISPLAY() usage is now constrained
within display.
Jani Nikula [Tue, 2 Sep 2025 17:51:52 +0000 (20:51 +0300)]
drm/{i915,xe}/panic: move framebuffer allocation where it belongs
The struct intel_framebuffer allocation naturally belongs in intel_fb.c,
not hidden inside panic implementation. Separate the panic
allocation. Drop the unnecessary struct i915_framebuffer and struct
xe_framebuffer types.
Jani Nikula [Tue, 2 Sep 2025 17:51:51 +0000 (20:51 +0300)]
drm/{i915,xe}/panic: rename struct {i915,xe}_panic_data to struct intel_panic
Prepare for better shared interfaces between panic implementations. The
struct intel_panic remains an opaque data type, with unique
implementations in i915 and xe.
This allows us to change the panic data pointer from void * to struct
intel_panic *, helping type safety.
Jani Nikula [Tue, 2 Sep 2025 17:51:50 +0000 (20:51 +0300)]
drm/{i915,xe}/fb: add panic pointer member to struct intel_framebuffer
Add a panic data pointer member in struct intel_framebuffer in
preparation for breaking the artificial subclassing between
intel_framebuffer and panic structures.
Jani Nikula [Tue, 2 Sep 2025 17:51:48 +0000 (20:51 +0300)]
drm/{i915,xe}/panic: split out intel_panic.[ch]
intel_bo.[ch] is not the appropriate location for the panic
functionality. Split out intel_panic.[ch] and xe_panic.c in i915 and
xe. Keep the function names for now.
Uma Shankar [Thu, 4 Sep 2025 09:53:38 +0000 (15:23 +0530)]
drm/i915/display: Remove FBC modulo 4 restriction for ADL-P+
WA:22010751166 does not apply past display version 12. Or, in
other words, the FBC restriction where FBC is disabled for
non-modulo 4 plane sizes (including plane size + yoffset) is fixed
from display version 13 and onwards. Relax the restriction for the same.
v4: Dropped redundant commit message
v3: Update comments for clarity (Jonathan Cavitt)
v2: Update the macro for display version check (Vinod)
Suggested-by: Vidya Srinivas <vidya.srinivas@intel.com> Signed-off-by: Uma Shankar <uma.shankar@intel.com> Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> Link: https://lore.kernel.org/r/20250904095338.300813-2-uma.shankar@intel.com
For ternary operators in the form of "a ? false : true", if 'a' itself
returns a boolean result, the ternary operator can be omitted. Remove
redundant ternary operators to clean up the code.
Jani Nikula [Wed, 3 Sep 2025 10:10:50 +0000 (13:10 +0300)]
drm/i915/ddi: abstract figuring out encoder name
The encoder name deduction has become a bit cumbersome within
intel_ddi_init(). Split it out to a separate function to declutter
intel_ddi_init(), even if that means having to use a temp seq buffer for
the name.
Jani Nikula [Fri, 5 Sep 2025 10:41:49 +0000 (13:41 +0300)]
drm/i915/power: fix size for for_each_set_bit() in abox iteration
for_each_set_bit() expects size to be in bits, not bytes. The abox mask
iteration uses bytes, but it works by coincidence, because the local
variable holding the mask is unsigned long, and the mask only ever has
bit 2 as the highest bit. Using a smaller type could lead to subtle and
very hard to track bugs.
Fixes: 62afef2811e4 ("drm/i915/rkl: RKL uses ABOX0 for pixel transfers") Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: stable@vger.kernel.org # v5.9+ Reviewed-by: Matt Roper <matthew.d.roper@intel.com> Link: https://lore.kernel.org/r/20250905104149.1144751-1-jani.nikula@intel.com Signed-off-by: Jani Nikula <jani.nikula@intel.com>
drm/i915/psr: Add poll for checking PSR is idle before starting update
We are currently observing crc failures after we started using dsb for PSR
updates as well. This seems to happen because PSR HW is still sending
couple of updates using old framebuffers on wake-up.
Fix this by adding poll ensuring PSR is idle before starting update.
v2: pass new_crtc_state->dsb_commit to intel_psr_wait_for_idle_dsb
drm/i915/psr: New interface adding PSR idle poll into dsb commit
We are currently observing crc failures after we started using dsb for PSR
updates as well. This seems to happen because PSR HW is still sending
couple of updates using old framebuffers on wake-up.
This patch is preparing to fix that by adding interface which can be used
to add poll ensuring PSR HW is idle into dsb commit.
v3: add intel_dsb as a parameter to intel_psr_wait_for_idle_dsb
v2: add pass crtc_state->dsb_commit as parameter
if __waitfor timeout, ret will have -ETIMEDOUT. Then if condition
was met, and read_ret will have error that's handled.
Then if ret was zero, read_ret was zero ksv_ready must have value.
Ville Syrjälä [Tue, 2 Sep 2025 13:31:13 +0000 (16:31 +0300)]
drm/i915/dram: Print memory details even if something went wrong
Print the memory details even if the detection failed in some way
but we continued the driver initialization anyway. It'll be easier
to debug issues if we at least know what the final results were.
And while at it also print the number of PSF GV points. Previously
we only printed the QGV points.
Ville Syrjälä [Tue, 2 Sep 2025 13:31:12 +0000 (16:31 +0300)]
drm/i915/dram: Don't call skl_get_dram_info()/skl_get_dram_type() on icl
Currently the icl codepaths first determine the memory type from the
memory controller registers (via skl_get_dram_info()->skl_get_dram_type())
and then overwrite the results with icl_pcode_read_mem_global_info().
Get rid of the pointless (and potentially incorrect) skl_get_dram_type()
stuff.
Ville Syrjälä [Tue, 2 Sep 2025 13:31:11 +0000 (16:31 +0300)]
drm/i915/dram: Fix some spelling around the 16Gb DIMM w/a
Use consistent spelling when talking about the 16Gb DIMM w/a.
Even currently language is a bit off as the w/a is actually
about DIMMs with 16Gb DRAM devices on them, not the total capacity
of the whole DIMM. But this language does more or less match how
Bspec talks about this stuff.
Ville Syrjälä [Tue, 2 Sep 2025 13:31:10 +0000 (16:31 +0300)]
drm/i915/dram: Move 16Gb DIMM detection fully to the skl/icl codepaths
We are incorrectly applying the 16Gb DIMM w/a (adding 1 extra
usec to WM0 latency) on MTL+ even though the w/a is only needed
for SKL/ICL.
The current way of setting this is up is a bit of a disaster:
1. always set has_16gb_dimms=true for all platforms except BXT/GLK
2. has_16gb_dimms potentially gets overwritten with something else
* BXT/GLK don't do anything since we never set has_16gb_dimms to
begin with
* skl_get_dram_info() overwrites has_16gb_dimms with the actual
detection results for SKL/ICL/derivatives
* gen12_get_dram_info() (correctly) resets has_16gb_dimms
for TGL/ADL/derivatives
* xelpdp_get_dram_info() doesn't do anything, leaving
has_16gb_dimms incorrectly set for MTL+
Clean up the whole mess by only setting has_16gb_dimms in the
SKL/ICL codepaths where we have the actual detection code for
it. This avois applying the w/a incorrectly on MTL+.
Ville Syrjälä [Tue, 2 Sep 2025 13:31:09 +0000 (16:31 +0300)]
drm/i915/dram: s/wm_lv0.../has_16gb_dimms/
The DRAM code shouldn't know anything about watermarks. Rename
wm_lv_0_adjust_needed to has_16gb_dimms. How this gets used is
up to the watermark code.