Make sure that the first read/write in hdcp2_authentication_key_exchange
are only retried when we have either DP/DPMST encoder connected,
since we do this to give docks and dp encoders some extra time to
get their HDCP DPCD registers ready only for DP/DPMST encoders as
this issue is only observed here no need to burden other encoders
with extra retries as this causes the HDMI connector to have some
other timing issue and fails HDCP authentication.
--v2
-Add intent of patch [Chaitanya]
-Add reasoning for loop [Jani]
-Make sure we forfiet the 50ms wait for non DP/DPMST encoders.
--v3
-Remove the is_dp_encoder check [Jani/Chaitanya]
-Make the commit message more clearer [Jani]
Fixes: 9d5a05f86d ("drm/i915/hdcp: Retry first read and writes to downstream")
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241104035951.517837-1-suraj.kandpal@intel.com
ADPA_HOTPLUG_BITS is defined in terms of the individual
register bits and is defined in intel_crt.c, whereas the
counterpart mask (ADPA_CRT_HOTPLUG_MASK) is just defined
as a raw hex constant and lives in i915_reg.h. Just define
both the same way (with unified name to boot) and move both
to intel_crt.c since they are more an implementation detail
rather than anything to do with the actual register definitions.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107161123.16269-6-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Although Bspec doesn't explicitly mentions that, as of Xe3_LPD, using
DMC wakelock is the officially recommended way of accessing registers
that would be off during DC5/DC6 and the legacy method (where the DMC
intercepts MMIO to wake up the hardware) is to be avoided.
As such, update the driver to use the DMC wakelock by default starting
with Xe3_LPD. Since the feature is somewhat new to the driver, also
allow disabling it via a module parameter for debugging purposes.
For that, make the existing parameter allow values -1 (per-chip
default), 0 (disabled) and 1 (enabled), similarly to what is done for
other parameters.
v2:
- Describe -1 in the same area where 0 and 1 are described. (Luca)
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241108130218.24125-16-gustavo.sousa@intel.com
Enabling and disabling the DMC wakelock should be done as part of
enabling and disabling of dynamic DC states, respectively. We should not
enable or disable DMC wakelock independently of DC states, otherwise we
would risk ending up with an inconsistent state where dynamic DC states
are enabled and the DMC wakelock is disabled, going against current
recommendations and making MMIO transactions potentially slower. In
future display IPs that could have a worse outcome if DMC trap
implementation is completely removed.
So, let's make things safer by tying stuff together, removing the
independent calls, and also put warnings in place to detect inconsistent
calls.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241108130218.24125-13-gustavo.sousa@intel.com
It is possible that there are active wakelock references at the time we
are disabling the DMC wakelock mechanism. We need to deal with that in
two ways:
(A) Implement the missing step from Bspec:
The Bspec instructs us to clear any existing wakelock request bit
after disabling the mechanism. That gives a clue that it is okay to
disable while there are locks held and we do not need to wait for
them. However, since the spec is not explicit about it, we need
still to get confirmation with the hardware team. Let's thus
implement the spec and add a TODO note.
(B) Ensure a consistent driver state:
The enable/disable logic would be problematic if the following
sequence of events would happen:
1. Function A calls intel_dmc_wl_get();
2. Some function calls intel_dmc_wl_disable();
3. Some function calls intel_dmc_wl_enable();
4. Function A is done and calls intel_dmc_wl_put().
At (2), the refcount becomes zero and then (4) causes an invalid
decrement to the refcount. That would cause some issues:
- At the time between (3) and (4), function A would think that
the hardware lock is held but it could not be really held
until intel_dmc_wl_get() is called by something else.
- The call made to (4) could cause the refcount to become zero
and consequently the hardware lock to be released while there
could be innocent paths trusting they still have the lock.
To fix that, we need to keep the refcount correctly in sync with
intel_dmc_wl_{get,put}() calls and retake the hardware lock when
enabling the DMC wakelock with a non-zero refcount.
One missing piece left to be handled here is the following scenario:
1. Function A calls intel_dmc_wl_get();
2. Some function calls intel_dmc_wl_disable();
3. Some function calls intel_dmc_wl_enable();
4. Concurrently with (3), function A performs the MMIO in between
setting DMC_WAKELOCK_CFG_ENABLE and asserting the lock with
__intel_dmc_wl_take().
I'm mostly sure this would cause issues future display IPs if DMC
trap implementation was completely removed. We need to check with
the hardware team whether it would be safe to assert the hardware
lock before setting DMC_WAKELOCK_CFG_ENABLE to avoid this scenario.
If not, then we would have to deal with that via software
synchronization.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241108130218.24125-12-gustavo.sousa@intel.com
There are extra registers that require the DMC wakelock when specific
dynamic DC states are in place. Those are registers that are touched by
the DMC and require DC exit for proper access. Add the range tables for
them and use the correct one depending on the enabled DC state.
v2:
- Do not look into power domains guts (i.e.
display->power.domains.dc_state). (Jani)
- Come up with better names for variables containing register ranges.
(Luca)
- Keep a copy of dc_state in struct intel_dmc_wl.
- Update commit message for a clearer explanation for the need of
these new tables.
Bspec: 71583
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241108130218.24125-10-gustavo.sousa@intel.com
We are currently using ARRAY_SIZE() to iterate address ranges in
intel_dmc_wl_check_range(). In upcoming changes, we will be using more
than a single table and will extract the range checking logic into a
dedicated function that takes a range table as argument. As we will not
able to use ARRAY_SIZE() then, let's make range tables contain a
sentinel item at the end and use that instead of having to pass the size
as parameter in this future function.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241108130218.24125-7-gustavo.sousa@intel.com
When the DMC wakelock refcount reaches zero, we know that there are no
users and that we can do the actual release operation on the hardware,
which is queued with a delayed work. The idea of the delayed work is to
avoid performing the release if a new lock user appears (i.e. refcount
gets incremented) in a very short period of time.
Based on the above, the release work should bail out if refcount is
non-zero (meaning new lock users appeared in the meantime), but our
current code actually does the opposite: it bails when refcount is zero.
That means that the wakelock is not released when it should be; and
that, when the work is not canceled in time, it ends up being releasing
when it should not.
Fix that by inverting the condition.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241108130218.24125-5-gustavo.sousa@intel.com
Some display MMIO transactions for offsets in the range that requires
the DMC wakelock happen in atomic context (this has been confirmed
during tests on PTL). That means that we need to use a non-sleeping
variant of MMIO waiting function.
Implement __intel_de_wait_for_register_atomic_nowl() and use it when
waiting for acknowledgment of acquire/release.
v2:
- No __intel_de_wait_for_register_atomic_nowl() wrapper to convert
i915 to display. (Jani)
- Add a quick explanation why DMC_WAKELOCK_CTL_TIMEOUT_US is defined
in microseconds. (Luca)
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241108130218.24125-4-gustavo.sousa@intel.com
In upcoming display changes, we will modify the DMC wakelock MMIO
waiting code to choose a non-sleeping variant implementation, because
the wakelock is also taking in atomic context.
While xe provides an explicit parameter (namely "atomic") to prevent
xe_mmio_wait32() from sleeping, i915 does not and implements that
behavior when slow_timeout_ms is zero.
So, for now, let's mimic what i915 does to allow for display to use
non-sleeping MMIO wait. In the future, we should come up with a better
and explicit interface for this behavior in i915, at least while display
code is not an independent entity with proper interfaces between xe and
i915.
v2:
- Make the tone in comment the comment added in
__intel_wait_for_register() more explanatory than a FIXME-like text.
(Luca)
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241108130218.24125-3-gustavo.sousa@intel.com
Having Panel Replay enabled together with VRR is causing following errors:
xe 0000:00:02.0: [drm] *ERROR* Timed out waiting PSR idle state
xe 0000:00:02.0: [drm] *ERROR* Fault errors on pipe A: 0x00000080
xe 0000:00:02.0: [drm] *ERROR* CPU pipe A FIFO underrun
xe 0000:00:02.0: [drm] *ERROR* Fault errors on pipe A: 0x00040080
xe 0000:00:02.0: [drm] *ERROR* Fault errors on pipe A: 0x00040080
Let's disable Panel Replay as well if VRR is enabled.
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241031081025.3215966-1-jouni.hogander@intel.com
TRANS_DDI_FUNC_CTL asks us to disable hdcp line rekeying when not in
hdcp 2.2 and we are not using an hdmi transcoder and it need to be
enabled when we are using an HDMI transcoder to enable HDCP 1.4.
We use intel_de_rmw cycles to update TRANS_DDI_FUNC_CTL register so
we cannot depend on the value being 0 by default everytime which calls
for seprate handling of HDCP 1.4 case.
--v2
-Use the exising function and modify it based on a bool rather than
have a different function [Matt]
--v3
-No need for kzalloc [Jani]
--v4
-i915_reg_t should not be pointer [Jani]
-use rekey_bit to ensure when writing to reg everything is initialized
Bspec: 69964, 50493, 50054
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107022807.869744-1-suraj.kandpal@intel.com
