I'm seeing underruns with these 64bpp YUV formats on TGL.
The weird details:
- only happens on pipe B/C/D SDR planes, pipe A SDR planes
seem fine, as do all HDR planes
- somehow CDCLK related, higher CDCLK allows for bigger plane
with these formats without underruns. With 300MHz CDCLK I
can only go up to 1200 pixels wide or so, with 650MHz even
a 3840 pixel wide plane was OK
- ICL and ADL so far appear unaffected
So not really sure what's the deal with this, but bspec does
state "64-bit formats supported only on the HDR planes" so
let's just drop these formats from the SDR planes. We already
disallow 64bpp RGB formats.
Cc: stable@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241218173650.19782-2-ville.syrjala@linux.intel.com
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
The CMTG is a timing generator that runs in parallel with transcoders
timing generators and can be used as a reference for synchronization.
We have observed that we are inheriting from GOP a display configuration
with the CMTG enabled. Because our driver doesn't currently implement
any CMTG sequences, the CMTG ends up still enabled after our driver
takes over.
We need to make sure that the CMTG is not enabled if we are not going to
use it. For that, let's add a partial implementation in our driver that
only cares about disabling the CMTG if it was found enabled during
initial hardware readout. In the future, we can also implement sequences
for using the CMTG if that becomes a needed feature.
For now, we only deal with cases when it is possible to disable the CMTG
without requiring a modeset. For earlier display versions, we simply
skip if we find the CMTG enabled and we can't disable it without a
proper modeset. In the future, we need to properly handle that case.
v2:
- DG2 does not have the CMTG. Update HAS_CMTG() accordingly.
- Update logic to force disabling of CMTG only for initial commit.
v3:
- Add missing changes for v2 that were staged but not committed.
v4:
- Avoid if/else duplication in intel_cmtg_dump_state() by using "n/a"
for CMTG B enabled/disabled string for platforms without it. (Jani)
- Prefer intel_cmtg_readout_hw_state() over intel_cmtg_readout_state().
(Jani)
- Use display struct instead of i915 as first parameter for
TRANS_DDI_FUNC_CTL2(). (Jani)
- Fewer continuation lines in variable declaration/initialization for
better readability. (Jani)
- Coding style improvements. (Jani)
- Use drm_dbg_kms() instead of drm_info() for logging the disabling
of the CMTG.
- Make struct intel_cmtg_state entirely private to intel_cmtg.c.
v5:
- Do the disable sequence as part of the sanitization step after
hardware readout instead of initial modeset commit. (Jani)
- Adapt to commit 1513358246 ("drm/i915/display: convert global state
to struct intel_display") by using a display struct instead of i915
as argument for intel_atomic_global_obj_init().
v6:
- Do not track CMTG state as a global state. (Ville)
- Simplify the driver logic by only disabling the CMTG only on cases
when a modeset is not required. (Ville)
v7:
- Remove the call to drm_WARN_ON() when checking
intel_cmtg_disable_requires_modeset() and use a FIXME in the comment
instead.
- Remove the !HAS_CMTG() guard from intel_cmtg_get_config(), which is
static and its caller is already protected by that same condition.
- Also take the opportunity to put some Bspec references in the commit
trailers section.
v8:
- Use HAS_TRANSCODER() instead of intel_crtc_for_pipe(). (Ville)
- Ensure transcoder power well is enabled before reading
TRANS_DDI_FUNC_CTL2. (Ville)
Bspec: 68915, 49262
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250124173956.46534-1-gustavo.sousa@intel.com
High refresh rate panels which may have small line times
and vblank sizes, Check if vblank size is sufficient for
dsc prefill latency.
--v2:
- Consider chroma downscaling factor in latency calculation. [Ankit]
- Replace with appropriate function name.
--v3:
- Remove FIXME tag.[Ankit]
- Replace Ycbcr444 to Ycbcr420.[Ankit]
- Correct precision. [Ankit]
- Use some local valiables like linetime_factor and latency to
adjust precision.
- Declare latency to 0 initially to avoid returning any garbage values.
- Account for second scaler downscaling factor as well. [Ankit]
--v4:
- Improvise hscale and vscale calculation. [Ankit]
- Use appropriate name for number of scaler users. [Ankit]
- Update commit message and rebase.
- Add linetime and cdclk prefill adjustment calculation. [Ankit]
--v5:
- Update bspec link in trailer. [Ankit]
- Correct hscale, vscale datatype. [Ankit]
- Use intel_crtc_compute_min_cdclk. [Ankit]
--v6:
- Use cdclk_state->logical.cdclk instead of
intel_crtc_compute_min_cdclk. [Ankit]
--v7:
- Fix linetime calculation. [Ankit]
- Reduce redandancy use of variables. [Ankit]
- Fix typos. [Ankit]
- Update calculation for precision. [Ankit]
--v8:
- Initialise variable to return garbage later. [Ankit]
- Initialise few variables to use at local loop, where
it is used. [Ankit]
Bspec: 70151
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250120172209.188488-8-mitulkumar.ajitkumar.golani@intel.com
High refresh rate panels which may have small line times
and vblank sizes, Check if vblank size is sufficient for
enabled scaler users.
--v2:
- Use hweight* family of functions for counting bits. [Jani]
- Update precision handling for hscale and vscale. [Ankit]
- Consider chroma downscaling factor during latency
calculation. [Ankit]
- Replace function name from scaler_prefill_time to
scaler_prefill_latency.
--v3:
- hscale_k and vscale_k values are already left shifted
by 16, after multiplying by 1000, those need to be right
shifted to 16. [Ankit]
- Replace YCBCR444 to YCBCR420. [Ankit]
- Divide by 1000 * 1000 in end to get correct precision. [Ankit]
- Initialise latency to 0 to avoid any garbage.
--v4:
- Elaborate commit message and add Bspec number. [Ankit]
- Improvise latency calculation. [Ankit]
- Use ceiling value for down scaling factor when less than 1
as per bspec. [Ankit]
- Correct linetime calculation. [Ankit]
- Consider cdclk prefill adjustment while prefill
computation.[Ankit]
--v5:
- Add Bspec link in commit message trailer. [Ankit]
- Correct hscale, vscale data type.
- Use intel_crtc_compute_min_cdclk. [Ankit]
--v6:
- Update FIXME comment.
- Use cdclk_state->logical.cdclk instead of
intel_crtc_compute_min_cdclk. [Ankit]
--v7:
- Handle error return from cdclk_prefill_adjustment. [Ankit]
- Avoid incorrect round off for linetime. [Ankit]
- Correct precision. [Ankit]
--v8:
- Remove redundancy calculation added from previous patch. [Ankit]
Bspec: 70151
Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250120172209.188488-7-mitulkumar.ajitkumar.golani@intel.com
Add helpers to calculate the necessary parameters for configuring the
HDMI PLL for SNPS MPLLB and C10 PHY.
The pll parameters are computed for desired pixel clock, curve data
and other inputs used for interpolation and finally stored in the
pll_state.
Currently the helper is used to compute PLLs for DG2 SNPS PHY.
Support for computing Plls for C10 PHY is added in subsequent patches.
v2:
-Used kernel types instead of C99 types. (Jani)
-Fixed styling issues and renamed few variables to more meaningful
names. (Jani)
-Added Xe make file changes. (Jani)
-Fixed build errors reported by kernel test robot
v3:
-Renamed helper to align with file name. (Jani)
v4:
-Removed erroraneous comment, and added Bspec# as part of trailer. (Suraj)
-Fixed warning flagged by kernel test robot.
Bspec: 54032
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Tested-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250120042122.1029481-2-ankit.k.nautiyal@intel.com
While setting the bounds for compressed bpp, we ensure that the
compressed bpp is less than the pipe bpp.
This causes an issue with the 420 output format, where the effective
link bpp (or output bpp) is half that of the pipe bpp. Therefore instead
of using pipe bpp, use the output bpp to set the bounds for the
compressed bpp.
v2: Use identifier output_bpp instead of link_bpp (Imre)
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250117050713.152012-1-ankit.k.nautiyal@intel.com
The scale() functions detects invalid parameters, but continues
its calculations anyway. This causes bad results if negative values
are used for unsigned operations. Worst case, a division by 0 error
will be seen if source_min == source_max.
On top of that, after v6.13, the sequence of WARN_ON() followed by clamp()
may result in a build error with gcc 13.x.
drivers/gpu/drm/i915/display/intel_backlight.c: In function 'scale':
include/linux/compiler_types.h:542:45: error:
call to '__compiletime_assert_415' declared with attribute error:
clamp() low limit source_min greater than high limit source_max
This happens if the compiler decides to rearrange the code as follows.
if (source_min > source_max) {
WARN(..);
/* Do the clamp() knowing that source_min > source_max */
source_val = clamp(source_val, source_min, source_max);
} else {
/* Do the clamp knowing that source_min <= source_max */
source_val = clamp(source_val, source_min, source_max);
}
Fix the problem by evaluating the return values from WARN_ON and returning
immediately after a warning. While at it, fix divide by zero error seen
if source_min == source_max.
Analyzed-by: Linus Torvalds <torvalds@linux-foundation.org>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Suggested-by: David Laight <david.laight.linux@gmail.com>
Cc: David Laight <david.laight.linux@gmail.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250121145203.2851237-1-linux@roeck-us.net
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
We have at least two options for how to do the
TRANS_PUSH_SEND + commit completion signalling
with the DSB:
Option A)
1. trigger TRANS_PUSH_SEND
2. wait for "safe window"
3. signal the interrupt
In this cases step 2 should not do anything if we were already
between vmin and vmax decision boundaries. Otherwise we'll wait
until the next start of the vblank period.
Option B)
1. wait for "safe window"
2. trigger TRANS_PUSH_SEND
3. signal the interrupt
This option is perhaps a bit less racy, but if we do somehow
screw up and the wait is a nop but the push gets deferred
until the next frame then we'll end up completing the commit
a frame too early.
So for now I'm leaning towards option A since losing the race
won't have any drastic consequences. To deal with the race we
can give the DSB a bit more time to start step 2 before the
hardware has started the vblank termination properly. Often
times it seems to be fast enough to make it in time even without
any extra vblank delay (the push is issued somewhere within a
scanline and it latches on the next scanline).
v2: Use intel_vrr_possible() to determine if we need some
vblank delay (also avoids adding it for DSI which doens't
actually program the transcoder registers correctly for it)
Cc: Paz Zcharya <pazz@chromium.org>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250116201637.22486-8-ville.syrjala@linux.intel.com
Currently we are forcing full modeset if Panel Replay mode is changed. This
is not necessary as long as we are not changing sink PANEL REPLAY ENABLE
bit in PANEL REPLAY ENABLE AND CONFIGURATION 1 register. This can be
achieved by entering Panel Replay inactive mode (Live Frame mode) when
Panel Replay is disabled and keep PANEL REPLAY ENABLE bit in PANEL REPLAY
ENABLE AND CONFIGURATION 1 enabled always if panel is just supporting Panel
Replay.
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250109103532.2093356-5-jouni.hogander@intel.com
Currently we are configuring Panel Replay on sink when it get's
enabled. This means we need to do full modeset when enabling Panel
Replay. This is required as DP specification is saying sink Panel Replay
needs to be configured before link training. Avoid full modeset by enabling
Panel Replay on sink always when it's supported by the sink and the
source.
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250109103532.2093356-3-jouni.hogander@intel.com
