Age | Commit message (Collapse) | Author |
|
It would be best to have self-explanatory code, but lacking that, add
some comments about the way the DDI encoder hooks get called from DP MST
stream encoders.
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/5941b8ef48cac4d61de2e2806a80645de48e856c.1731409802.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
The fake encoders pretty much match individual MST streams. The encoders
remain as fake as ever, but change the naming to MST stream
encoders. Rename all the encoder hooks and related functions called from
them to mst_stream_* to clarify what type of encoders the hooks are
called on.
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/37d9572e884ca2fdd84ea6722b4b7cefde04eed9.1731409802.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
struct intel_display will replace struct drm_i915_private as the main
display device data structure. Convert the DP MST code to struct
intel_display as much as possible.
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/84c9923fccdd779f0ca713cab1d26a0c802b140c.1731409802.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Making register macros platform or display version aware is not exactly
something I want to promote widely, but in this case it's the lesser of
two evils. hsw_chicken_trans_reg() is not pretty, and it doesn't have a
suitable home.
v2: Rebase
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/712c17ee22537b0628aa32695743bc017b3fe332.1731409802.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Use a primary_encoder local variable in
intel_dp_create_fake_mst_encoder() for clarity.
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/7c9ad1db97550b7eacb1f010521704f623b0c689.1731409802.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Add helpers to_primary_encoder() and to_primary_dp() to convert fake MST
encoder pointers to primary encoder and DP pointers, respectively, and
use them.
The main point is to highlight the primary encoder and DP usage. Very
few places actually need the struct intel_dp_mst_encoder pointer, or the
primary struct intel_digital_port, so ditch them where possible for
clarity.
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/ff8786c04bae053055783f720f89a4d9f9815c70.1731409802.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Switch to the modern style in fake mst encoder creation.
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/4d29387fba64b925c6ab4c108782a7b5ac22628a.1731409802.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Pass the primary encoder to the primary encoder hooks. This is
pedantically correct, but intel_ddi_post_pll_disable() also works with
the fake encoder by coincidence.
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/a0151fddfdd5ec11a26345232cdd3ae59c8cf56a.1731409802.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Identify mobile platforms separately in display, using the platform
group mechanism. This enables dropping the dependency on i915_drv.h
IS_MOBILE() from display code.
v2: Make snb_display static (kernel test robot)
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/2389fd58446e83a79ec57bc0b037679662037b9e.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert intel_display_device_probe() to accept struct pci_dev * instead
of struct drm_i915_private *. Return struct intel_display * in
preparation of allocating the memory of it later.
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/ab4e960e3fff46cbeba185882b1e554f0ccd5877.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert intel_display_device.[ch] to struct intel_display, including
callers, but excluding intel_display_device_probe() which will be
handled in follow-up.
v2: fix display->drm = display->drm goof-up
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/865b27b66f599e707081d46fca9f679e19a4e8aa.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
All the feature check helpers now support (and some require) passing
struct intel_display. Rename the parameter to __display to reflect the
fact.
v2: Rebase
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/4933ded2efa9f333c560b6e634f60d4808614030.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert HAS_ULTRAJOINER() to struct intel_display. Do minimal drive-by
conversions to struct intel_display in the callers while at it.
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/f562d35c94cf7ef18d261a9867d959a58f165f1e.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert HAS_HW_SAGV_WM() to struct intel_display. Do minimal drive-by
conversions to struct intel_display in the callers while at it.
v2: Rebase
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com> # v1
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> # v1
Link: https://patchwork.freedesktop.org/patch/msgid/8babfd6f09df054d33d604a02e213200a3783737.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert HAS_SAGV() to struct intel_display. Do minimal drive-by
conversions to struct intel_display in the callers while at it.
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/c0f65566d08c4ab5296740367979beb1670d1c06.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert HAS_MBUS_JOINING() to struct intel_display. Do minimal drive-by
conversions to struct intel_display in the callers while at it.
v2: Rebase
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com> # v1
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> # v1
Link: https://patchwork.freedesktop.org/patch/msgid/c198dd5a0ab0a5f37edce3d18ddb4991cf3d4792.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert HAS_IPS() to struct intel_display. Do minimal drive-by
conversions to struct intel_display in the callers while at it.
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/55fd6063b30ca227483a479cff0012e5a915196d.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert HAS_GMBUS_BURST_READ() to struct intel_display. Do minimal
drive-by conversions to struct intel_display in the callers while at it.
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/34dfafeccf6f8ddd5437c79e18978151bfd96c24.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert HAS_DP20() to struct intel_display. Do minimal drive-by
conversions to struct intel_display in the callers while at it.
v2: Rebase
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com> # v1
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> # v1
Link: https://patchwork.freedesktop.org/patch/msgid/757d91d2a57052b11cbd9f4f40d836d9245ca926.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert HAS_DOUBLE_BUFFERED_M_N() to struct intel_display. Do minimal
drive-by conversions to struct intel_display in the callers while at it.
v2: Rebase
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/d313d32ae411b86eedb86c4a4949dc84588362df.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert HAS_4TILE() to struct intel_display. Do minimal drive-by
conversions to struct intel_display in the callers while at it.
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/123799e7c96d03d8eea7d9bea3d450e82c88ae5d.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Convert HAS_D12_PLANE_MINIMIZATION() to struct intel_display. Do minimal
drive-by conversions to struct intel_display in the callers while at it.
Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/ed9402eb83ab301f3c1d60884376d715b2190fd3.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Identify discrete graphics separately in display, using the platform
group mechanism. This enables dropping the dependency on i915_drv.h
IS_DGFX() from display code.
Start grouping platform groups separately in INTEL_DISPLAY_PLATFORMS()
in anticipation of more groups to come.
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/c02230d26cc0d9dbd7ddcc064661b2ad03739b6a.1731321183.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
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: 9d5a05f86d2f ("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
|
|
intel_atomic_setup_scaler() is a mess. Make it a bit less so
by moving the scaler allocation loop into its own function.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107122658.21901-9-ville.syrjala@linux.intel.com
|
|
Make scaler in_use a boolean since that's how it's used.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107122658.21901-8-ville.syrjala@linux.intel.com
|
|
Declutter intel_atomic_setup_scalers() a bit by splitting
the crtc scaling/pfit vs. plane scaling cases into their
own functions.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107122658.21901-7-ville.syrjala@linux.intel.com
|
|
struct intel_display will replace struct drm_i915_private as
the main thing for display code. Convert the scaler code to
use it (as much as possible at this stage).
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107122658.21901-6-ville.syrjala@linux.intel.com
|
|
intel_atomic_setup_scalers() is currently messing around with
the internals of the atomic states. Stop doing that and instead
use the regular interfaces so that we don't need to know any ugly
implementation details.
Reviewed-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107122658.21901-5-ville.syrjala@linux.intel.com
|
|
intel_atomic_setup_scalers() currently digs out the full atomic
state from the crtc state. Flip that on its head so that we instead
pass in the full atomic state and dig out the crtc state (and whatever
else we need). This is generallte the better approach as it works
in all phases of the atomic commit, whereas the other apporoach only
really works during .atomic_check().
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107122658.21901-4-ville.syrjala@linux.intel.com
|
|
Nuke some redundant includes and forward declarations from
skl_scaler.h.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107122658.21901-3-ville.syrjala@linux.intel.com
|
|
Rename some variables from the intel_foo to just foo, to match
the more modern style used throughout the driver.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107122658.21901-2-ville.syrjala@linux.intel.com
|
|
crt->connector is never used, nuke it.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107161123.16269-11-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
|
|
Rename a bunch of local variables to the preferred
encoder/connector from intel_encoder/intel_connector.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107161123.16269-10-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
|
|
Remove a bunch of pointless 'struct drm_device *dev' local variables.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107161123.16269-9-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
|
|
Call the crtc state 'crtc_state' rather than 'pipe_config',
as is the modern style.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107161123.16269-8-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
|
|
Move the analog port register definitions into their
own file.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107161123.16269-7-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@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>
|
|
Follow the modern style and use REG_BIT() & co. for the analog
port register definitions.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107161123.16269-5-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
|
|
The ADPA_DPMS bit definitions are just an alias for the
sync disable bits, and unused one at that. Drop the
pointless definitions.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107161123.16269-4-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
|
|
Split an overly long line in the CRT code.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107161123.16269-3-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
|
|
Grab the intel_display from 'encoder' rather than 'state'
in the encoder hooks to avoid the massive footgun that is
intel_sanitize_encoder(), which passes NULL as the 'state'
argument to encoder .disable() and .post_disable().
TODO: figure out how to actually fix intel_sanitize_encoder()...
Fixes: 40eb34c3f491 ("drm/i915/crt: convert to struct intel_display")
Fixes: ab0b0eb5c85c ("drm/i915/tv: convert to struct intel_display")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241107161123.16269-2-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
|
|
Bitwise OR for booleans is a bit of an abuse. We can trivially drop the
usage.
Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/f7ea5854c9a306f56f4142f8d37d567ee2f768a7.1730201504.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Drop the wa_16013835468_bit_get() function in favour of the register
macro. It doesn't have to be so complicated, and we don't have to use
the workaround name in everything that's related to it.
Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/22934fee1ea37c777c35e4b520d5f11b6cd953d0.1730201504.git.jani.nikula@intel.com
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
|
|
Commit 1f12d63a14d7 ("drm/i915/dp: Write the source OUI for non-eDP
sinks as well") started writing source OUI for non-eDP sinks as well,
increasing the possibilities of hitting read/write failures either due
to the sink behaviour or hotplug or whatever.
Even before that, commit 3fb0501f0c07 ("drm/i915/display/dp: Reduce log
level for SOURCE OUI write failures") already reduced write failures to
info level when source OUI was just for eDP.
Further reduce the log level to just debug. Switch to struct
intel_display while at it.
Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/3372
Cc: Clint Taylor <clinton.a.taylor@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241106162325.4065078-1-jani.nikula@intel.com
Signed-off-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
|
|
Instead of checking for HAS_DMC_WAKELOCK() multiple times, let's use it
to sanitize the enable_dmc_wl parameter and use that variable when
necessary.
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-15-gustavo.sousa@intel.com
|
|
A HAS_DMC_WAKELOCK() macro gives more semantic than openly checking the
display version. Define it and use it where appropriate.
v2:
- Make this patch contain only the non-functional refactor. Functional
changes related to including HAS_DMC() in the macro are done in
upcoming changes. (Jani)
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-14-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
|