Accessing the TypeC DKL PHY registers during modeset-commit,
-verification, DP link-retraining and AUX power well toggling is racy
due to these code paths being concurrent and the PHY register bank
selection register (HIP_INDEX_REG) being shared between PHY instances
(aka TC ports) and the bank selection being not atomic wrt. the actual
PHY register access.
Add the required locking around each PHY register bank selection->
register access sequence.
Kudos to Ville for noticing the race conditions.
v2:
- Add the DKL PHY register accessors to intel_dkl_phy.[ch]. (Jani)
- Make the DKL_REG_TC_PORT macro independent of PHY internals.
- Move initing the DKL PHY lock to a more logical place.
v3:
- Fix parameter reuse in the DKL_REG_TC_PORT definition.
- Document the usage of phy_lock.
v4:
- Fix adding TC_PORT_1 offset in the DKL_REG_TC_PORT definition.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: <stable@vger.kernel.org> # v5.5+
Acked-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221025114457.2191004-1-imre.deak@intel.com
The connector->override_edid flag is strictly for EDID override debugfs
management, and drivers have no business using it.
The check for override_edid was added in commit 3019062905 ("drm/i915:
Ignore TMDS clock limit for DP++ when EDID override is set") to
facilitate mode list cross-checking against modes in override EDID when
the connector in question isn't even connected. The dual mode detect
fallback would do VBT based limiting in this case.
Instead of override EDID, check for connector forcing in the fallback.
v2: Simply use !connector->force (Ville)
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/c8b45867cf37134ab40be23e22825ca45adc6041.1666614699.git.jani.nikula@intel.com
For normal connector detect, there's really no point in trying dual mode
detect if the connector is disconnected. We can simplify the detect
sequence by skipping it. Since intel_hdmi_dp_dual_mode_detect() is only
called when EDID is present, we can drop the has_edid parameter.
The functional effect is speeding up disconnected connector detection
ever so slightly, and, combined with firmware EDID, also stop logging
about assuming dual mode adaptor.
It's a bit subtle, but this will also skip dual mode detect if the
connector is force connected and a) there's no EDID of any kind, normal
or override/firmware or b) there's EDID but it does not indicate
digital. These are corner cases no matter what, and arguably forcing
should not be limited by dual mode detect.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/f8f2a4a147e1c87ba93269a607f71fc29c4b59f6.1666614699.git.jani.nikula@intel.com
Make glk_load_luts() a bit lighter for the common case
where neither the degamma LUT nor pipe CSC are enabled
by not loading the linear degamma LUT. Making .load_luts()
as lightweight as possible is a good idea since it may need
to execute from a vblank worker under tight deadlines.
My earlier reasoning for always loading the linear degamma LUT
was to avoid an extra LUT load when just enabling/disabling the
pipe CSC, but that is nonsense since we load the LUTs on every
flagged color management change/modeset anyway (either of which
is needed for a pipe CSC toggle).
We can also get rid of the glk_can_preload_luts() special
case since the presence of the degamma LUT will now always
match csc_enable.
v2: Fix typos (Uma)
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221024161514.5340-6-ville.syrjala@linux.intel.com
Add an extra remapping step between the logical state of the LUTs
(hw.(de)gamma_lut) as specified via uapi/bigjoiner copy vs.
the actual state of the LUTs programmed into the hardware.
With this we should be finally able finish the (de)gamma
readout/state checker support for the remaining platforms
(ilk-skl) where the same hardware LUT can be positioned
either before or after the pipe CSC unit. Where we position
it depends on factors such as presence of the logical degamma
LUT, RGB vs. YCbCr output, full vs. limited RGB quantization
range.
Without the extra remapping step the state readout doesn't
really know whether the LUT read from the hardware is the
degamma or gamma LUT, and so we is unable to accurately store
it into our crtc state. With the remapping step we know
exactly where to put it given the order of the LUT vs. CSC
in the hardware state.
Only the initial hw->uapi state readout done during driver
load/resume still has the problem of not really knowing
what to do with the LUT(s). But we can just assume 1:1
mapping there and let subsequent commits fix things up.
Another benefit is that we now have a place for purely
internal LUTs, without complicating the bigjoiner uapi->hw
copy logic. This should prove useful for streamlining
glk degamma LUT handling.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221024161514.5340-3-ville.syrjala@linux.intel.com
Be consistent in whether we flag a full modeset or a
fastset for the pipe. intel_modeset_all_pipes() would
seem to be the only codepath not getting this right.
The other case is when we flag the fastset initially,
currently we just clear the mode_changed flag and set
the update_pipe flag. But we could still have
connectors_changed==true or active_changed==true forcing
a full modeset anyway. So check for that after clearing
the mode_changed flag.
And let's add a WARN to make sure we did get it right.
v2: Deal with {connectors,active}_changed
Reviewed-by: Jani Nikula <jani.nikula@intel.com> #v1
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221021162442.27283-4-ville.syrjala@linux.intel.com
On BDW+ we have just the one set of DP M/N registers. The
values we write into said registers depends on whether we
want DRRS to be in high or low gear. This causes issues
for the state checker which currently has to assume either
set of M/N (high or low refresh rate) values may appear there.
That sort of works for M/N itself, but all other values
derived from the M/N (dotclock, pixel rate) are not handled
correctly, leading to potential for state checker mismatches.
Let's avoid all those problems by simply keeping DRRS in
high gear until the state checker has done its hardware
state readout.
Note that hitting this issue presumable became very hard
after commit 1b333c679a ("drm/i915: Do DRRS disable/enable
during pre/post_plane_update()") since the state check would
have to laze about for one full second (delay used by
intel_drrs_schedule_work()) to see the low refresh rate.
But it is still theoretically possible.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221020120706.25728-1-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
We use all kinds of weird names for our base address registers.
Take the names from the spec and stick to them to avoid confusing
everyone.
The only exceptions are IOBAR and LMEMBAR since naming them
IOBAR_BAR and LMEMBAR_BAR looks too funny, and yet I think
that adding the _BAR to GTTMMADR & co. (which don't have one
in the spec name) does make it more clear what they are.
And IOBAR vs. GTTMMADR_BAR also looks a bit too inconsistent
for my taste.
v2: Fix gvt build
v3: Add GEN2_IO_BAR for completeness
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221005195646.17201-1-ville.syrjala@linux.intel.com
Acked-by: Matthew Auld <matthew.auld@intel.com>
MTL and dgfx use the same DC5 counter.
While at it, this patch also adds the corresponding
debugfs entries. Some cleanup wrt dc3co register
which makes the code more readable.
Driver loads all firmware that it finds in the firmware
binary but platform doesn't *need* all of them. Cleaning the
previous debugs entries to reflect which firmware is needed
and if the needed firmware is loaded or not.
MTL needs both Pipe A and Pipe B DMC to be loaded
along with Main DMC.
BSpec: 49788
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221010202135.28388-1-anusha.srivatsa@intel.com
For these types of display buffers, we need to able to CPU access some
part of the backing memory in prepare_plane_clear_colors(). As a result
we need to ensure we always place in the mappable part of lmem, which
becomes necessary on small-bar systems.
v2(Nirmoy & Ville):
- Add some commentary for why we need to CPU access the buffer.
- Split out the other changes, so we just consider the display change
here.
v3:
- Handle this in the dpt path.
v4(Ville):
- Drop the intel_fb_rc_ccs_cc_plane() sanity check in
pin_and_fence_fb_obj(), since we can also trigger this on DG1 it
seems.
Fixes: eb1c535f0d ("drm/i915: turn on small BAR support")
Reported-by: Jianshui Yu <jianshui.yu@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Nirmoy Das <nirmoy.das@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221004131916.233474-4-matthew.auld@intel.com
(cherry picked from commit e3afc69018)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
We've excluded gmch platforms from writing the final watermarks
for any disabled pipe. IIRC the reason was perhaps some lingering
issue with the watermark merging across the pipes. But I can't
really see any reason for this anymore, so let's unify this behaviour.
The main benefit being more consistency in register dumps when
we don't have stale watermarks hanging around in the registers.
Functionally there should be no difference as the hardware just
ignore all of it when the pipe is disabled.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220622155452.32587-9-ville.syrjala@linux.intel.com
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
On g4x/vlv/chv the hardware seems incapable of changing the pixel
format, rotation, or YUV->RGB CSC matrix while in CxSR.
Additionally on VLV/CHV the sprites seem incapable of tiling
changes while in CxSR. On g4x CxSR is not even possible with
the sprite enabled. Curiously the primary plane seems perfectly
happy when changing tiling during CxSR.
Pimp up the code to account for these when determining whether
CxSR needs to be disabled. Since it looks like most of the plane
control register bits are affected let's just compare that.
But in the name of efficiency we'll make an exception for the
primary plane tiling changes (avoids some extra vblank waits).
v2: Just use the pre-computed plane control register values
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220622155452.32587-7-ville.syrjala@linux.intel.com
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
For these types of display buffers, we need to able to CPU access some
part of the backing memory in prepare_plane_clear_colors(). As a result
we need to ensure we always place in the mappable part of lmem, which
becomes necessary on small-bar systems.
v2(Nirmoy & Ville):
- Add some commentary for why we need to CPU access the buffer.
- Split out the other changes, so we just consider the display change
here.
v3:
- Handle this in the dpt path.
v4(Ville):
- Drop the intel_fb_rc_ccs_cc_plane() sanity check in
pin_and_fence_fb_obj(), since we can also trigger this on DG1 it
seems.
Fixes: eb1c535f0d ("drm/i915: turn on small BAR support")
Reported-by: Jianshui Yu <jianshui.yu@intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Nirmoy Das <nirmoy.das@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Nirmoy Das <nirmoy.das@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221004131916.233474-4-matthew.auld@intel.com
Make sure modes with crazy big dotclocks are rejected early,
so as to not cause problems for subsequent code via integer
overflows and whatnot.
These would eventually be rejected in intel_crtc_compute_pipe_mode()
but that is now too late as we do the clock computations a bit
earlier than that. And we don't want to just reorder the two since
we still want to check the final computed dotclock against the
hardware limit to make sure we didn't end up above the limit due
to rounding/etc.
Fixes: 0ff0e219d9 ("drm/i915: Compute clocks earlier")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220927182455.3422-1-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
(cherry picked from commit df2f59c585)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Current PSR code is supposed to use TRANSCODER_EDP to force 0 shift for
bits in PSR_IMR/IIR registers:
/*
* gen12+ has registers relative to transcoder and one per transcoder
* using the same bit definition: handle it as TRANSCODER_EDP to force
* 0 shift in bit definition
*/
At the time of writing the code assumption "TRANSCODER_EDP == 0" was made.
This is not the case and all fields in PSR_IMR and PSR_IIR are shifted
incorrectly if DISPLAY_VER >= 12.
Fix this by adding separate register field defines for >=12 and add bit
getter functions to keep code readability.
v4:
- Remove EDP from TGL definitions (José)
- Use REG_BIT and REG_GENMASK (José)
v3:
- Add separate register field defines (José)
- Add bit getter functions (José)
v2:
- Improve commit message (José)
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Fixes: 8241cfbe67 ("drm/i915/tgl: Access the right register when handling PSR interruptions")
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221003072011.72408-1-jouni.hogander@intel.com
(cherry picked from commit 8da8e32e0b)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Daniele needs 84d4333c1e ("misc/mei: Add NULL check to component match
callback functions") in order to merge the DG2 HuC patches.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
