]> www.infradead.org Git - users/jedix/linux-maple.git/commit
drm/i915/dmc_wl: Deal with existing references when disabling
authorGustavo Sousa <gustavo.sousa@intel.com>
Fri, 8 Nov 2024 12:57:16 +0000 (09:57 -0300)
committerMatt Roper <matthew.d.roper@intel.com>
Fri, 8 Nov 2024 17:54:05 +0000 (09:54 -0800)
commit9fe9cd95feacce9ae1c30c88a5513f556623d24d
treebea7a24b838029cbac72877adbcde6c4087f5cc0
parent02e2224796a3b609d47bc3a1b78cc833289d7d1f
drm/i915/dmc_wl: Deal with existing references when disabling

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
drivers/gpu/drm/i915/display/intel_dmc_wl.c