intel_gmch_vga_set_state() is a complete lie on ILK+ because
the GMCH_CTRL register is locked and can't actually be written.
But we still need to remove the iGPU from the VGA arbitration
on iGPU+dGPU systems, or else Xorg performance will tank due
to the constant VGA arbiter accesses.
For VGA memory decode we can't turn off the PCI_COMMAND
memory deocde as that would disable even normal MMIO.
Instead we can disable just the VGA memory decode via
the VGA MSR register. And we can do that just once
when disabling the VGA plane. That way we don't have
to touch VGA registers anywhere else.
We can also inform the arbiter that we're no longer decoding
VGA memory. This will stop the arbiter from disabling all
memory decode for the iGPU via PCI_COMMAND (and thus breaking
everything) whenever some other GPU wants to own the VGA memory
accesses.
For IO we can disable all IO decode via the PCI_COMMAND
register, except around the few VGA register accesses that
we need to do in intel_vga_disable(). Unfortunately we can't
disable IO decode permanently as it makes some laptops (eg.
Dell Latitude E5400) hang during reboot/shutdown. One option
would be to re-enable IO decode from the poweroff hooks, but
that won't help the sysrq emergency reboot/shutdown since it
won't call said hooks. So let's try to keep IO decode in its
original setting unless we really need to disable it to
exclude the GPU from VGA arbitration.
I suppose we could keep frobbing GMCH_CTRL on pre-ILK, but
it seems better to not do it since it has other side effects
such as changing the class code of the PCI device.
For discrete GPUs we'll rely on the bridge control instead.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20251208182637.334-9-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Avoid using the VGA arbiter during intel_vga_get() for iGPUs because
that will clobber the VGA routing for whatever external GPU is the
current VGA device. That will cause all reads from VGA memory to
come back as 0xff/white, and thus we get a white rectangle on screen
when the external GPU switches from vgacon to fbcon.
The iGPU has the highest VGA decode priority so it will steal all
VGA register accesses whenever its IO decoding is enabled. We'll only
keep the IO decode enabled for a short time so hopefully we don't
end up eating too many unrelated VGA register accesses.
For discrete GPUs we need all the bridges to have their VGA forwarding
bits correctly configured so we can't really avoid the VGA arbiter
there. Although we only do this stuff on dGPUs when the VGA plane was
actaully enabled, so the dGPU should be the current VGA device
and thus have VGA routed to it already anyway.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20251208182637.334-8-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Turns out at least some systems (eg. HSW Lenovo ThinkCentre E73)
configure the VGA registers even when booting in UEFI mode. So
in order to avoid any issues with the MSR register we should
clean up the VGA registers anyway.
For now this mostly avoids the potential for unclaimed register
accesses due to the power well vs. MDA/CGA selection. But this
will become more important soon as we'll start to rely on the
MSR register to control VGA memory decode as well.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20251208182637.334-7-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
On some systems the BIOS will disable the VGA decode logic in the
iGPU (via GMCH_CTRL) when an external GPU is used as the primary
VGA device. In that case the iGPU will never claim any VGA register
accesses, and any access we do will in fact end up on the external
GPU. Don't go poking around in the other GPUs' registers.
Note that (at least on the g4x board where I tested this) the BIOS
forgets to set the VGACNTR VGA_DISP_DISABLE bit, and the reset
value for said bit is 0. That apparently prevents the pipes from
running, so we must still remember to set the bit, despite the VGA
plane was never actually enabled. On more modern platforms (hsw+
maybe?) the reset value for VGACNTR was changed to have
VGA_DISP_DISABLE already set.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20251208182637.334-6-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Remove the MSR VGA register access from the power well hook, and
just do it once in intel_vga_disable().
Turns out that the hardware has two levels of MDA vs. CGA decode
logic: GPU level and display engine level. When we write the MSR
register MDA/CGA mode selection bit both decode logics are updated
accordingly, so that whichever register accessed the GPU claims
will also be claimed by the display engine on the RMbus. If the
two get out of sync the GPU will claim the register accesses but
the display engine will not, leading to RMbus NoClaim errors.
The way to get the two decode logics out of sync is by resetting
the power well housing the VGA stuff, while we are in CGA mode.
At that point only the display engine level decode logic will be
updated with the new MSR register reset value (which selects MDA
mode), and the GPU level decode logic will retain its previous
state (GGA mode). To avoid the mismatch we just have to switch
to MDA mode with an explicit MSR register write.
Currently this is being done in a somewhat dodgy manner whenever
the power well gets enabled. But doing if from the power well
hook is not actually necessary since the GPU level decode logic
will retain its state even when the power well is disabled. Ie.
doing it just the one time is sufficient, and that can be done
when we're anyway writing other VGA registers while disabling
the VGA plane.
See commit f9dcb0dfee ("drm/i915: touch VGA MSR after we
enable the power well") for the original details.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20251208182637.334-3-ville.syrjala@linux.intel.com
Acked-by: Jani Nikula <jani.nikula@intel.com>
Currently we register to vgaarb way too early. Thus it is
possible that the entire VGA decode logic gets nuked via
GMCH_CTRL before intel_vga_disable() has even disabled the
VGA plane. This could even cause a system hang because
we'll be unable to turn off the VGA plane gracefully.
Move the vgaarb registration into intel_display_driver_register().
I suppose we could do it a bit earlier (after intel_vga_disable()),
but not convinced there's any point.
Also the error handling here is pointless since the
registration can't fail (unless the device isn't a VGA class
in which case all VGA decode logic should aleady be disabled
by the BIOS via GMCH_CTRL). But let's toss in a WARN to catch
any future breakage of vga_client_register().
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20251208182637.334-2-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Static analysis reveals a potential integer underflow in
intel_fbc_stolen_end. This can apparently occur if
intel_parent_stolen_area_size returns zero (or, theoretically, any value
less than 2^23), as 2^23 is subtracted from the return value and stored
in a u64. While this doesn't appear to cause any issues due to the use
of the min() function to clamp the return values from the
intel_fbc_stolen_end function, it would be best practice to avoid
undeflowing values like this on principle. So, rework the function to
prevent the underflow from occurring. Note that the underflow at
present would result in the value of intel_fbc_cfb_base_max being
returned at the end of intel_fbc_stolen_end, so just return that if the
value of intel_parent_stolen_area_size is too small.
While we're here, fix the other comments here and modify the execution
path for readability.
v2: (Jani)
- Fix the comments in intel_fbc_stolen_end
- Use check_sub_overflow
- Remove macro that mirrors SZ_8M, as it is now only referenced once
- Misc. formatting fixes
Fixes: a9da512b3e ("drm/i915: avoid the last 8mb of stolen on BDW/SKL")
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patch.msgid.link/20260107162935.8123-2-jonathan.cavitt@intel.com
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
PLL state structure has a member .clock. This is not needed as
the port clock is possible to calculate from the pll dividers.
Remove the encoder from being passed to the port clock calculation
function.
v2: Keep the pll_state->clock assignment in
intel_snps_hdmi_pll_compute_mpllb().
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Link: https://patch.msgid.link/20260119093757.2850233-16-mika.kahola@intel.com
Add verification for lt phy pll dividers during boot. The port clock
is calculated from pll dividers and compared against the requested
port clock value. If there are a difference exceeding +-1 kHz an
drm_warn() is thrown out to indicate possible pll divider mismatch.
v2:
- Move the LT_PHY_PLL_PARAMS -> LT_PHY_PLL_DP/HDMI_PARAMS change
earlier.
- Use tables[i].name != NULL as a terminating condition.
- Use state vs. params term consistently in intel_c10pll_verify_clock()
and intel_c20pll_verify_clock().
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Link: https://patch.msgid.link/20260119093757.2850233-13-mika.kahola@intel.com
Add verification for pll table dividers. The port clock
is computed based on pll tables and, for hdmi case, the
algorithmic model is applied to calculate pll dividers.
If port clock differs more than +-1 kHz from expected value
an drm_warn() is thrown and pll divider differences are
printed out for debugging purposes.
v2:
- Move clock derivation from dividers in intel_cx0pll_enable()
earlier in the patchset.
- Keep intel_cx0_pll_power_save_wa() in intel_dpll_sanitize_state()
- Use tables[i].name != NULL as a terminating condition.
- Drop duplicate intel_cx0pll_clock_matches() declaration in header.
- Use state vs. params term consistently in intel_c10pll_verify_clock()
and intel_c20pll_verify_clock().
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Link: https://patch.msgid.link/20260119093757.2850233-12-mika.kahola@intel.com
The hard coded clock rate stored in the PLL state will be removed by
a follow-up change. The clock is calculated instead of
using clock from the PLL divider values. Since this calculated clock
may vary due to fixed point rounding issues, a +-1 kHz variation is
allowed with the request clock rate against the calculated clock rate.
v2:
- Use the stricter +-1 kHz allowed difference.
- Derive the clock from PLL dividers in intel_cx0pll_enable().
- Move corresponding fuzzy check for LT PHY PLLs to this patch.
v3: Reword commit message (Suraj)
Move clock check to intel_dpll and rename it (Suraj)
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Link: https://patch.msgid.link/20260119093757.2850233-9-mika.kahola@intel.com
Drop crtc_state from HDMI TMDS calculation and replace with the
parameters that are only required. Follow-up changes will call
these functions without a crtc_state available.
v2: Keep required crtc_state param for intel_c20_pll_tables_get()
and other functions calling this one.
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Link: https://patch.msgid.link/20260119093757.2850233-4-mika.kahola@intel.com
With some panels informing support for Panel Replay we are observing
problems if having Panel Replay enable bit set on sink when forced to use
PSR instead of Panel Replay. Avoid these problems by not setting Panel
Replay enable bit in sink when Panel Replay is globally disabled during
link training. I.e. disabled by module parameter.
The enable bit is still set when disabling Panel Replay via debugfs
interface. Added note comment about this.
Fixes: 68f3a505b3 ("drm/i915/psr: Enable Panel Replay on sink always when it's supported")
Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: <stable@vger.kernel.org> # v6.15+
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>
Link: https://patch.msgid.link/20260115070039.368965-1-jouni.hogander@intel.com
Simplify things by computing the detailed slice configuration using
intel_dp_dsc_get_slice_config(), instead of open-coding the same.
While at it add a TODO comment to intel_dp_dsc_compute_config() to
explore if it's worth increasing the number of VDSC stream engines used,
in order to reduce the minimum CDCLK required.
v2: Add a TODO comment to intel_dp_dsc_compute_config() to explore if
it's worth increasing the number of slices in order to use a lower
CDCLK. (Jouni)
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260114162232.92731-16-imre.deak@intel.com
Unify the DP and eDP slices-per-line computation. Atm eDP simply returns
the maximum slices-per-line value supported by the sink, but using the
same helper function for both cases still makes sense, since a follow-up
change will compute the detailed slice config for both cases.
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260114162232.92731-14-imre.deak@intel.com
Simplify the slice config loop in intel_dp_dsc_get_slice_count(), using
the loop iterator as the slices-per-pipe value directly, instead of
looking up the same value from an array.
While at it move the code comment about the slice configuration closer
to where the configuration is determined and clarify it a bit.
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260114162232.92731-11-imre.deak@intel.com
By now all the places are updated to track the DSC slice configuration
in intel_crtc_state::dsc.slice_config, so calculate the slices-per-line
value using that config, instead of using
intel_crtc_state::dsc.slice_count caching the same value and remove
the cached slice_count.
v2: Rebase on latest drm-tip, converting another user of dsc.slice_count
in intel_vdsc_min_cdclk().
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Jouni Högander <jouni.hogander@intel.com> # v1
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260114162232.92731-7-imre.deak@intel.com
