- 06 Oct, 2021 1 commit
-
-
Ville Syrjälä authored
Currently we consider the max vswing reached when we transmit a the max voltage level, but we don't consider pre-emphasis at all. This kinda matches older DP specs that only had some vague text about transmitting the maximum voltage swing. Latest versions now say something vague about consider the sum of the vswing and pre-emphasis fields in the ADJUST_REQUEST_LANE registers. Very vague, and super confusing especially the fact that it talks about transmitted voltgage swing in the same sentence as it say to look at the requested values. Also glanced at the link CTS spec, and that one seems to have tests that assume contradicting behaviour. Some say to consider just the vswing level we transmit, others say to check for sum of transmitted vswing+preemph being 3. So let's try to take some kind of sane middle ground here. I think what could make sense is only consider max vswing reached if MAX_SWING_REACHED==1 _and_ vswing+preemph==3. That will allow things to go all the way up to vswing 3 + pre-emph 0 or vswing 2 + pre-emph 1, depending on what the maximum supported vswing is. Only considering the sum of vswing+pre-emph doesn't make much sense to me since we could terminate too early if the sink requests eg. vswing 0 + pre-emph 3. And if we'd stick to the current code we could terminate too early of the sink asks for vswing 2 + pre-emph 0 when vswing level 3 is not supported. Side note: I don't really understand why any of this stuff is "specified" at all. There is already a limit of 5 attempts at the same vswing+pre-emph level, and a total limit of 10 attempts. So might as well stick to the same max 5 attempts across the board IMO. Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20211004170535.4173-2-ville.syrjala@linux.intel.comReviewed-by:
Imre Deak <imre.deak@intel.com>
-
- 05 Oct, 2021 1 commit
-
-
Dan Carpenter authored
The "digi_port" pointer can't be NULL and we have already dereferenced it so checking for NULL is not necessary. Delete the check. Signed-off-by:
Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by:
-
- 04 Oct, 2021 15 commits
-
-
Ville Syrjälä authored
Since the VT-d vs. async flip issues are plaguing a wider range of supported hw let's try to minimize the impact on normal operation by flipping the relevant chicken bits on and off as needed. I presume there is some power/perf impact on since this is reducing some prefetching I think. Cc: Karthik B S <karthik.b.s@intel.com> Signed-off-by:
Matt Roper <matthew.d.roper@intel.com>
-
Ville Syrjälä authored
Looks like skl/bxt/derivatives also need the plane stride stretch w/a when using async flips and VT-d is enabled, or else we get corruption on screen. To my surprise this was even documented in bspec, but only as a note on the CHICHKEN_PIPESL register description rather than on the w/a list. So very much the same thing as on HSW/BDW, except the bits moved yet again. Cc: stable@vger.kernel.org Cc: Karthik B S <karthik.b.s@intel.com> Fixes: 55ea1cb1 ("drm/i915: Enable async flips in i915") Signed-off-by:
-
Ville Syrjälä authored
LTTPRs should support per-lane drive settings I think, and even if they don't they should implement their own fallback logic to determine suitable common drive settings to use for all the lanes. v2: Actually check the correct thing Reviewed-by:
-
Ville Syrjälä authored
Adjust the link training code to accommodate per-lane drive settings, if supported by the platform. Actually enabling this will involve some changes to each platform's .set_signal_level() implementation, so for the moment all supported platforms will keep using the current codepath that just uses the same drive settings for all the lanes. v2: Fix min() vs. max() fumble v3: Compact the debug print to a single line Reviewed-by:
-
Ville Syrjälä authored
In order to have per-lane drive settings we need intel_ddi_level() to accept the lane as a parameter. That is, the eventual goal is to call intel_ddi_level() once for each lane. For now we just pass in a hardcoded 0 and use the same settings for every lane. Ie. no change in behaviour yet. Reviewed-by:
-
Ville Syrjälä authored
Since intel_ddi_level() now looks at the buf_trans table there's no point in having intel_ddi_hdmi_num_entries() around. Just roll the necessary bits of locic into intel_ddi_hdmi_level()/intel_ddi_level(). Reviewed-by:
-
Ville Syrjälä authored
All callers of intel_ddi_level() duplicate the check+WARN to make sure the returned level is actually present in the appropriate buf_trans table. Let's push that stuff into intel_ddi_level() so the callers don't have to worry about it. Reviewed-by:
-
Ville Syrjälä authored
Convert bxt_ddi_phy_set_signal_levels() to act as the full .set_signal_levels() hook instead of going through a pointless wrapper. Reviewed-by:
-
Ville Syrjälä authored
Now that .set_signal_levels() is used for HDMI as well, we can remove the extra level of indirection and just plug the correct stuff straight into .set_signal_levels(). Reviewed-by:
-
Ville Syrjälä authored
Currently .set_signal_levels() is only used by encoders in DP mode. For most modern platforms there is no essential difference between DP and HDMI, and both codepaths just end up calling the same function under the hood. Let's get remove the need for that extra indirection by moving .set_signal_levels() into the encoder from intel_dp. Since we already plumb the crtc_state/etc. into .set_signal_levels() the code will do the right thing for both DP and HDMI. HSW/BDW/SKL are the only platforms that need a bit of care on account of having to preload the hardware buf_trans register with the full set of values. So we must still remember to call hsw_prepare_{dp,hdmi}_ddi_buffers() to do said preloading, and .set_signal_levels() will just end up selecting the correct entry for DP, and also setting up the iboost magic for both DP and HDMI. Note that previously on HSW/BDW/SKL we did write to DDI_BUF_CTL to select the correct entry until link training started, now that we call .set_signal_levels() already from hsw_ddi_pre_enable_dp() that is no longer the case. But it's all safe now that the intel_ddi_init_dp_buf_reg() call was hoisted up and it no longer sets up the DDI_BUF_CTL_ENABLE bit (that is still deferred until link training). v2: Rebase due to has_{iboost,buf_trans_select}() Add some notes about the DDI_BUF_CTL situation on HSW/BDW/SKL (Imre) Reviewed-by: -
Ville Syrjälä authored
Add a small helper to determine if DDI_BUF_CTL uses the DDI_BUF_TRANS_SELECT field, and whether we have the accompanying DDI_BUF_TRANS table in the hardware. Cc: Imre Deak <imre.deak@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Suck the "do we have iboost?" platform checks into a small helper. Cc: Imre Deak <imre.deak@intel.com> Signed-off-by:
-
Ville Syrjälä authored
The DP spec says: "If the receiver keeps the same value in the ADJUST_REQUEST_LANEx_y register(s) while the LANEx_CR_DONE bits remain unset, the transmitter must loop four times with the same voltage swing. On the fifth time, the transmitter must down-shift to the lower bit rate and must repeat the CR-lock training sequence as described below." Lets fix the code to follow that instead of terminating after five times of transmitting the same signal levels. The text in spec feels a little bit ambiguous still, but this is my best guess at its meaning. As a bonus this also gets rid of the train_set[0] stuff which would not work for per-lane drive settings anyway. Cc: Imre Deak <imre.deak@intel.com> CC: Jani Nikula <jani.nikula@intel.com> Signed-off-by:
-
Jani Nikula authored
For controlling the audio SDP split. Bspec: 63837 Reviewed-by:
Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20211001100316.26441-1-jani.nikula@intel.com
-
Dave Airlie authored
This was causing infinite recursion on snb/ivb. Fixes: 5716c8c6 ("drm/i915/uncore: split the fw get function into separate vfunc") Signed-off-by:
Dave Airlie <airlied@redhat.com> Signed-off-by:
-
- 01 Oct, 2021 19 commits
-
-
Ville Syrjälä authored
While sanitizing the hardware state we're currently forcing the pipe bottom color legacy csc/gamma bits on. That is not a good idea as BIOSen are likely to leave gabage in the LUTs and so doing this causes ugly visual glitches if and when the planes covering the background get disabled. This was exactly the case on this Dell Precision 5560 tgl laptop. On icl+ we don't normally even use these legacy bits anymore and instead use their GAMMA_MODE counterparts. On earlier platforms the bits are used, but we still shouldn't force them on without knowing what's in the LUT. So two options, get rid of the whole thing, or do what intel_color_commit() does to make sure the bottom color state matches whatever out hardware readout produced. I chose the latter since it'll match what happens on older platforms when the primary plane gets turned off. In fact let's just call intel_color_commit(). It'll also do some CSC programming but since we don't have readout for that it'll actually just set to all zeros. So in the unlikely case of CSC actually being enabld by the BIOS we'll end up with all black until the first atomic commit happens. Still not totally sure what we should do about color management features here in general. Probably the safest thing would be to force everything off exactly at the same time when we disable the primary plane as there is no guarantees that whatever the LUTs/CSCs contain make any sense whatsoever without the specific pixel data in the BIOS fb. And if we preserve the primary plane then we should disable the color management features exactly when the primary plane fb contents first changes since the new content assumes more or less no transformations. But of course synchronizing front buffer rendering with anything else is a bit hard... Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3534Signed-off-by:
Uma Shankar <uma.shankar@intel.com>
-
Ville Syrjälä authored
Check for the zero length front porch already in intel_mode_valid() so that we get the same validation for both get_modes() and setcrtc()/etc. Signed-off-by:
-
Ville Syrjälä authored
"CRTC fixup failed" is probably leftovers from pre-atomic days when there was an actual fixup() function. Let's unify the debug messages between encoder vs. crtc compute_config() calls. Signed-off-by:
-
Ville Syrjälä authored
Unify how we check for -EDEADLK vs. other errors from crtc vs. encoder compute_config() calls. Signed-off-by:
-
Jani Nikula authored
Prefer the intel_ types. No functional changes. v2: Fix build. Reviewed-by:
Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by:
-
Jani Nikula authored
Failures to register debugfs should be ignored anyway, so stop propagating errors altogether for clarity and simplicity. No functional changes. Reviewed-by:
-
Jani Nikula authored
The debugfs file shows it's not capable, don't duplicate the info. Reviewed-by:
-
Jani Nikula authored
Prefer i915 over drm pointer. Reviewed-by:
-
Jani Nikula authored
Using standard -EAGAIN should be perfectly fine instead of using a special case value. Reviewed-by:
-
Jani Nikula authored
Avoid using the incidental -EPERM. Reviewed-by:
-
Jani Nikula authored
Avoid using the incidental -EPERM. Return the -EIO directly from i915_get_bridge_dev() instead of converting return values later. Reviewed-by:
-
Jani Nikula authored
Avoid using the incidental -EPERM. Reviewed-by:
-
Jani Nikula authored
Avoid using the incidental -EPERM. Also remove useless comment. Reviewed-by:
-
Jani Nikula authored
Having two functions for this seems like excess duplication and parameter juggling. Merge them together. While at it, drop the extra error message, as wait_for_payload_credits() already prints an error, and switch from incidental -EPERM (i.e. -1) to actual error codes. Reviewed-by:
-
Jani Nikula authored
Pass a const pointer instead of passing 32 bytes of struct mipi_dsi_packet by value. Reviewed-by:
-
Jani Nikula authored
Keep the functionality and the assert code together. Reviewed-by:
-
Jani Nikula authored
Keep the functionality and the assert code together. Reviewed-by:
-
Jani Nikula authored
Move assert_panel_unlocked() to intel_pps.c and rename assert_pps_unlocked(). Keep the functionality and the assert code together. There's still a bit of a split between the eDP PPS usage in intel_pps.c and all the other PPS usage, and assert_pps_unlocked() is arguably more related to the latter. However, intel_pps.c is the best fit for anything touching the PPS registers. Reviewed-by:
-
Jani Nikula authored
Keep the functionality and the assert code together. Reviewed-by:
-
- 30 Sep, 2021 4 commits
-
-
José Roberto de Souza authored
With all the past fixes now this feature is functional and can be enabled by default in desktop enviroments that uses compositor. Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by:
Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Signed-off-by:
José Roberto de Souza <jose.souza@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20210930001409.254817-8-jose.souza@intel.com
-
José Roberto de Souza authored
With all the recent fixes PSR2 is properly working in Alderlake-P but due to some issues that don't have software workarounds it will not be supported in display steppings older than B0. Even with this patch PSR2 will no be enabled by default in ADL-P, it still requires enable_psr2_sel_fetch to be set to true, what some of our tests does. Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Reviewed-by:
-
José Roberto de Souza authored
The Wa_14014971508 is required to fix scanout when a feature that i915 do not support is enabled and this feature is not planned to be enabled for adlp. Keeping this workaround enabled can badly hurt power-savings when a full frame fetch is required(see psr2_sel_fetch_plane_state_supported() and psr2_sel_fetch_pipe_state_supported()). Here a example that could badly hurt power-savings, userspace does a page flip to a rotated plane, so CONTINUOS_FULL_FRAME set. But then for a whole 30 seconds nothing in the screen requires updates but because CONTINUOS_FULL_FRAME is set, it will not go into DC5/DC6. Reverting Wa_14014971508 fixes that, as only a single frame will be sent and then display can go to DC5/DC6 for those 30 seconds of idleness. BSpec: 54369 Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Signed-off-by:
-
José Roberto de Souza authored
Legacy cursor APIs are handled by intel_legacy_cursor_update(), that calls drm_atomic_helper_update_plane() when going through the slow/atomic path to update cursor, what was the case for PSR2 selective fetch. drm_atomic_helper_update_plane() sets drm_atomic_state->legacy_cursor_update to true when updating the cursor plane, to allow several cursor updates to happen within the same frame, as userspace does that. If drivers waited for a vblank increment at the end of every cursor movement that would cause a visible lag in the cursor. But this optimization do not properly work with PSR2 selective fetch dirt area calculation, for example if within a single frame the cursor had 3 moves the final dirt area programmed to PSR2_MAN_TRK_CTL would be based in the second movement as old state and third movement as new state, not updating the area where cursor was in the first state. So here switching back to the fast path approach in intel_legacy_cursor_update() and handling cursor movements as frontbuffer rendering(psr_force_hw_tracking_exit()), that is not the most optimal for power-savings but is the solution that we have until mailbox style updates is implemented. Also removing the cursor workaround as not it is properly undestand the issue and is know that it will never cover all the cases. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Signed-off-by:
-
