Apparently I failed to test the unload case properly and
thus didn't notice that the i915_overlay_is_active() needs
i915->overlay after fetch_and_zero() already cleared it.
Stop using fetch_and_zero() and only clear the pointer at
the end to avoid the oops.
Fixes: 38d9a352c4 ("drm/i915/overlay: Extract i915_overlay_is_active()")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260303101417.14409-1-ville.syrjala@linux.intel.com
Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>
Ack only those SST link service IRQs that will be handled, similarly to
device service IRQs. While at it add asserts that only the known/acked
link service IRQs are handled both in the MST and SST case.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-21-imre.deak@intel.com
Only those IRQs should be acked that are handled, however for SST all
IRQs triggered by the sink are acked. This can be a problem for flags
that are reserved/reading zero at a given moment, but become used for
some purpose - with a side-effect if set - in a future DPCD revision.
Fix the above by acking only those device service IRQs that will be
handled. While at it add asserts that only the known/acked device
service IRQs are handled both in the MST and SST case.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-20-imre.deak@intel.com
Move checking the link status for SST to
intel_dp_handle_link_service_irq(). This is the logical place for the
check which should only happen in response to a LINK_STATUS_CHANGE sink
IRQ. This IRQ is only supported by DPCD revision >= 1.2, so for sinks
with an older DPCD revision the link status is checked in response to
any HPD IRQ. For newer DPCD revisions however the link status check can
be made conditional on LINK_STATUS_CHANGE.
For now keep the current behavior of always forcing a link status check
regardless of LINK_STATUS_CHANGE being set or not.
This also prepares for a follow-up change sharing the link service IRQ
handler for SST and MST (on MST the link status check only happening in
response to a LINK_STATUS_CHANGE IRQ).
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-18-imre.deak@intel.com
So far the CONNECTED_OFF_ENTRY_REQUESTED request was accepted only
implicitly, by acking all the IRQs raised by the sink. Make this
explicit by printing a debug message.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-17-imre.deak@intel.com
Read and ack the sink count, sink device and link service IRQs for SST
the same way this is done for MST, the read/ack happening in separate
steps via an ESI (Event Status Indicator) vector.
The above way is more efficient, since on newer (DPCD_REV >= 1.2) sinks
the DP_SINK_COUNT_ESI..DP_LINK_SERVICE_IRQ_VECTOR_ESI0 registers can be
read out in one AUX transaction - and the 3 last one written in one
transaction. Also this allows sharing more of the SST and MST IRQ
handling code (done as a follow-up).
For now keep the current behavior of always reading the legacy
DP_SINK_COUNT, DP_DEVICE_SERVICE_IRQ_VECTOR registers and not reading
the DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1 register.
v2: Document the ESI vector get/ack helper fnuctions' return value.
(Jani, Luca)
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Luca Coelho <luciano.coelho@intel.com>
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-16-imre.deak@intel.com
If getting/acking the link service IRQs fail, the short HPD handler
should bail out, falling back to a full connector detection as in case
of any AUX access failures during the HPD handling. Do this by
separating the getting/acking and handling steps of the IRQs.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-15-imre.deak@intel.com
If getting/acking the device service IRQs fail, the short HPD handler
should bail out, falling back to a full connector detection as in case
of any AUX access failures during the HPD handling. Do this by
separating the getting/acking and handling steps of the IRQs.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-14-imre.deak@intel.com
An AUX access failure during HPD IRQ handling should be handled by
falling back to a full connector detection, ensure that if the failure
happens while reading/acking a link service IRQ.
While at it add code comment documenting the return value of
intel_dp_check_link_service_irq().
v2: Docuement intel_dp_check_link_service_irq()'s return value. (Jani)
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-13-imre.deak@intel.com
An AUX access failure during HPD IRQ handling should be handled by
falling back to a full connector detection, ensure that if the failure
happens while reading/acking a device service IRQ.
v2: Document intel_dp_check_device_service_irq()'s return value.
(Jani, Luca)
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Luca Coelho <luciano.coelho@intel.com>
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-12-imre.deak@intel.com
The DP_LINK_SERVICE_IRQ_VECTOR_ESI0 DPCD register is supported only
since DPCD REV 1.2, so fix the revision check accordingly.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-11-imre.deak@intel.com
The DP_DEVICE_SERVICE_IRQ_VECTOR DPCD register is supported since DPCD
REV 1.0, so read it out always. Flags added only by later DPCD revisions
are defined as reserved/must-be-zero by earlier DP Standard versions.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-10-imre.deak@intel.com
The device service IRQ handling was added to the connector detect
function by
commit 09b1eb130e ("drm/i915: Move Displayport test request and sink
IRQ logic to intel_dp_detect()")
since some Automated Test Request IRQs couldn't be handled in the short
HPD IRQ handler context. This has been fixed meanwhile by deferring the
handling of all test request events from the IRQ handler to the hotplug
handler (intel_dp_short_pulse() -> intel_dp_test_short_pulse() ->
reprobe) and by handling all hotplug events (both for short and long HPD
pulses) in the test application.
Handling device IRQs during connector detection is not standard
compliant (the IRQs should be handled when an HPD IRQ is raised) and it
happens in a racy way with the same device IRQ handling happening from
the HPD IRQ handler (since the detect and HPD IRQ handler can run in
parallel).
Based on the above, remove the redundant call from the detect function.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-9-imre.deak@intel.com
The intel_dp_get_dpcd() function called from an HPD IRQ handler reads
out the DPRX capabilities from the sink and updates these in the
intel_dp encoder state. Since the IRQ handler can run in parallel with
the encoder/connector detection (intel_dp_detect()) which also calls
intel_dp_get_dpcd(), the encoder state can get corrupted, since the two
updates happen in a racy way.
Fix the above by checking only for any change in the sink count value in
the HPD IRQ handler, without updating the encoder state.
Note that any state change in the sink requiring an update of the
encoder state is handled via the sink's SINK_COUNT change,
RX_CAPS_CHANGED, DOWNSTREAM_PORT_STATUS_CHANGED signaling, which all
should result in a full connector detection.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-8-imre.deak@intel.com
Handle the DOWNSTREAM_PORT_STATUS_CHANGED event a branch device can use
to indicate the state change of a DFP connector on the branch device.
The event is signaled in the DP_LANE_ALIGN_STATUS_UPDATED DPCD register
setting a clear-on-read flag and triggering an HPD IRQ. Accordingly keep
a cached version of the flag, updating it whenever
DP_LANE_ALIGN_STATUS_UPDATED is read. Schedule a full connector
detection from the HPD IRQ handler if the cached flag is set and clear
the cached flag at the start of detection.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-7-imre.deak@intel.com
Handle the RX_CAP_CHANGED IRQ, which a sink can use to indicate a DPRX
capability change without disconnecting and reconnecting itself (i.e.
through a short vs. long HPD pulse). Handle the IRQ by doing a full
connector detection.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-6-imre.deak@intel.com
HPD IRQs in general should be handled after acking them. The
1. Read IRQ register (read DP_DEVICE_SERVICE_IRQ_VECTOR,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0)
2. Handle IRQ
3. Ack IRQ (write DP_DEVICE_SERVICE_IRQ_VECTOR,
DP_LINK_SERVICE_IRQ_VECTOR_ESI0)
sequence would miss a new interrupt triggered after 2. and before 3.,
since the flag set in the IRQ register for this interrupt would be
cleared in step 3.
Fix the above by handling the IRQ after acking it.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-5-imre.deak@intel.com
The MST link status should be always verified from the same DPCD
registers after link training. Atm, both the legacy (0x202 - 0x205) and
the ESI (0x200C - 0x200F) link status registers are used. Use always the
latter ESI version of link status registers.
v2: Propagate error from intel_dp_read_link_status(). (Jani, Luca)
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-3-imre.deak@intel.com
An AUX access failure during HPD IRQ handling should be handled by
falling back to a full connector detection, do so.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260225164618.1261368-2-imre.deak@intel.com
Move compat i915_vma.h to xe_display_vma.h, and remove all extra
cruft. Drop the i915_ggtt_offset() wrapper in favour of using
xe_ggtt_node_addr() directly.
The usefulness of the I915_TILING_X and I915_TILING_Y undef/define is
unclear, since uapi/drm/i915_drm.h is included in other paths as well.
The naming of struct i915_vma is a bit unfortunate in xe, but (at least
for now) a necessity for maintaining type safety on the opaque type.
Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>
Link: https://patch.msgid.link/ecd5d75981b4b21c3da3b1831faceccfe385d898.1772212579.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
It's unclear what the direction of the VMA abstraction in the parent
interface should be, but convert i915_vma_fence_id() to parent interface
for starters. This paves the way for making struct i915_vma opaque
towards display.
Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>
Link: https://patch.msgid.link/036f4b2d20cc1b0a7ab814beb5bb914c53b6eb53.1772212579.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
When computing the encoder/CRTC state multiple times, such as during
iteration over the possible pipe joiner configurations, it must be
ensured that all state is explicitly initialized each time. At the
moment, this is not guaranteed for the DSC/FEC state within the
encoder/CRTC state. In one iteration trying a pipe joiner configuration,
the DSC state may get initialized without computing the full
CRTC/encoder state due to the given joiner configuration being
impossible. When the same CRTC/encoder state is recomputed in a
subsequent iteration, the previously set non-zero - now stale - DSC/FEC
state may still be present, which is unexpected if a non-DSC
configuration is being computed.
This can lead to a DSC state mismatch error if multiple joiner
configurations are evaluated and the working configuration ultimately
turns out to be a non-DSC one.
Follow the existing pattern and compute the full (DSC/FEC) state on all
code paths (including now the non-DSC path as well) to fix the issue.
Fixes: 1f1e3e5c65 ("drm/i915/dp: Rework pipe joiner logic in compute_config")
Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/7512
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patch.msgid.link/20260302102838.1522499-1-imre.deak@intel.com
Currently it takes us multiple log prints to arrive at the conclusion
on how we are actually controlling backlight level. Make the logging
concise.
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
Link: https://patch.msgid.link/20260302040613.3324049-8-suraj.kandpal@intel.com
With luminance_set which represents PANEL_LUMINANCE_OVERRIDE, we
have another variable to decide if we use PWM or DPCD.
Make drm_dbg_kms log represent that.
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
Link: https://patch.msgid.link/20260302040613.3324049-7-suraj.kandpal@intel.com
intel_dp_aux_supports_hdr_backlight() prints debug message for
intel HDR backlight version. This is fine when dealing with eDP 1.4b
and lower. When we are talking about eDP 1.5 it causes confusion in
logs since we need to use VESA AUX backlight functions but this
print causes confusion as to which path code take.
Short circuit this function with a eDP version check. Make sure this
is only called if eDP <= 1.4b
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
Link: https://patch.msgid.link/20260302040613.3324049-6-suraj.kandpal@intel.com
Convert the direct i915_overlay_*() calls from the display
side to go over a new parent interface instead.
v2: Correctly handle the ERR_PTR returned by
i915_overlay_obj_lookup() (Jani)
v3: Rebase due to the NULL check in intel_overlay_cleanup()
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226130150.16816-1-ville.syrjala@linux.intel.com
Relocate the i915 driver specific parts of the overlay code
into i915_overlay.c. This leaves intel_overlay.c with just
the display specific code.
The one annoyance here is the DSPCLK_GATE_D register access
being done from i830_overlay_clock_gating(). The register
definition lives on the display side as we do need to access
it on other platforms there. Since it's just one register
and bit, I decided to just duplicate that part in i915_reg.h.
v2: Use kzalloc_obj()
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226133855.17690-1-ville.syrjala@linux.intel.com
We don't generally want fetch_and_zero() on the display side, so
stop using it in the display side intel_overlay_cleanup().
Fortunately we don't really have anything to do here apart from
freeing the data. And we'll keep on clearing the pointer, just
in case something somewhere cares about it.
Note that once i915_overlay_cleanup() is converted to the parent
interface we can't call it unconditionally (as xe won't have it).
So we need to keep the early bailout for overlay==NULL.
v2: Adjust the commit message since we do apparently
have fetch_and_zero() in display code as well (Jani)
v3: Skip i915_overlay_cleanup() if the overlay wasn't initialized
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226130113.16714-1-ville.syrjala@linux.intel.com
Split the i915 driver specific bits from 'struct intel_overlay'
into a seaarate 'struct i915_overlay'. The latter will move to
the i915 side of the parent vs. display driver split.
The display side will also need to know the virtual address of
the register map. That now gets passed as the return value
from i915_overlay_setup(), so that the display side doesn't
need to know how the mapping was achieved.
v2: Use kzalloc_obj()
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-17-ville.syrjala@linux.intel.com
i830_overlay_clock_gating() will remain on the i915 side of the
parent vs. display driver split. Stop using display specific stuff
inside it.
The one annoyance here is access to the display engine's
DSPCLK_GATE_D register. The proper way to deal with that might
be to move it to the display side, but that seems a bit hard right
now. So leave it where it is for now.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-15-ville.syrjala@linux.intel.com
Adjust the names ("i915_overlay_" prefix) and calling
convention (pass the driver agnostic 'struct drm_device'()
of the functions that will provide the remainder of the
parent driver interface to be used by the overlay display
code.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-14-ville.syrjala@linux.intel.com
Some of the lower level functions in the overlay code will
move to the i915 side of the upcoming parent vs. display
driver split. Move all such functions to the "i915_overlay_"
namespace to make it easier to see what belongs where.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-13-ville.syrjala@linux.intel.com
Make the buffer (un)pinning a bit more abstract so that
the display side of the code doesn't need to know about i915
specific things (i915_ggtt_offset() and i915_vma_unpin()).
In preparation for the full parent vs. display driver split
we'll also pass in the drm_device to these functions, although
not strictly needed yet.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-12-ville.syrjala@linux.intel.com
Pull the i915 specific bits of the overlay cleanup into
a separate function (i915_overlay_cleanup()) to accommodate
the upcoming parent vs. display driver split.
For now we'll also have to pass in the overlay struct, but
that will disappear once the i915 vs. display split is completed.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-11-ville.syrjala@linux.intel.com
Pull the gem/gt related bits of the overlay setup into
a separate function (i915_overlay_setup()) that will eventually
move to the i915 side of the parent vs. display driver split.
For now we'll also have to pass in the overlay struct, but
that will disappear once the i915 vs. display split is completed.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-10-ville.syrjala@linux.intel.com
overlay->frontbuffer_bits tracking will move to the i915 side
of the parent vs. display driver split, so extract the reset
part of that into a new function (i915_overlay_reset()).
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-9-ville.syrjala@linux.intel.com
Use 'struct drm_gem_object' for the BO instead of 'struct
drm_i915_gem_object', to avoid having the display side
know anything about the i915 specific BO type.
v2: Correctly handle the ERR_PTR returned by
i915_overlay_obj_lookup() (Jani)
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-8-ville.syrjala@linux.intel.com
Extract the BO lookup and tiling check into a new
helper called i915_overlay_obj_lookup(). This will have to
move to the i915 side of the parent vs. display driver split.
There is a slight change here in that we now do the tiling
check before taking the modeset locks, but those locks don't
protect the BO tiling stuff in any way, so nothing is really
different here.
Note that the hardware should support X-tiled scanout also
for the overlay, but I guess no one ever bothered to hook
it up and test it. So the check should stay at least for now.
v2: Correctly handle the ERR_PTR returned by
i915_overlay_obj_lookup() (Jani)
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-7-ville.syrjala@linux.intel.com
Move the underrun check out from intel_overlay_continue()
so that the DOVSTA register access can stay on the display
side of the parent vs. display driver split.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-6-ville.syrjala@linux.intel.com
Pull the "is the overlay active?" check to a helper
(i915_overlay_is_active()). This will have to move to the
i915 side of the parent vs. display driver split.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patch.msgid.link/20260226100738.29997-4-ville.syrjala@linux.intel.com
