Rob Clark [Mon, 17 Mar 2025 15:00:06 +0000 (08:00 -0700)]
drm/msm/a6xx+: Don't let IB_SIZE overflow
IB_SIZE is only b0..b19. Starting with a6xx gen3, additional fields
were added above the IB_SIZE. Accidentially setting them can cause
badness. Fix this by properly defining the CP_INDIRECT_BUFFER packet
and using the generated builder macro to ensure unintended bits are not
set.
v2: add missing type attribute for IB_BASE
v3: fix offset attribute in xml
Reported-by: Connor Abbott <cwabbott0@gmail.com> Fixes: a83366ef19ea ("drm/msm/a6xx: add A640/A650 to gpulist") Signed-off-by: Rob Clark <robdclark@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/643396/
The commit 5a9d50150c2c ("drm/msm/dpu: shift IRQ indices by 1") shifted
IRQ indices by 1, making 'NO_IRQ' to be 0 rather than -1 (and allowing
to skip the definition if the IRQ is not present).
Several platform files were sketched before that commit, but got applied
afterwards. As such, they inherited historical (and currently incorrect)
setting of .intr_tear_rd_ptr = -1 for 'NO_IRQ' value.
Drop that setting for all the affected platforms.
Fixes: 62af6e1cb596 ("drm/msm/dpu: Add support for MSM8917") Fixes: c079680bb0fa ("drm/msm/dpu: Add support for MSM8937") Fixes: 7a6109ce1c2c ("drm/msm/dpu: Add support for MSM8953") Fixes: daf9a92daeb8 ("drm/msm/dpu: Add support for MSM8996") Fixes: 7204df5e7e68 ("drm/msm/dpu: add support for SDM660 and SDM630 platforms") Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/647486/ Link: https://lore.kernel.org/r/20250408-dpu-drop-intr-rd-ptr-v1-1-eeac337d88f8@oss.qualcomm.com Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
drm/msm/dpu: reorder pointer operations after sanity checks to avoid NULL deref
_dpu_encoder_trigger_start dereferences "struct dpu_encoder_phys *phys"
before the sanity checks which can lead to a NULL pointer dereference if
phys is NULL.
Fix this by reordering the dereference after the sanity checks.
Chenyuan Yang [Fri, 14 Mar 2025 01:10:04 +0000 (20:10 -0500)]
drm/msm/dpu: Fix error pointers in dpu_plane_virtual_atomic_check
The function dpu_plane_virtual_atomic_check was dereferencing pointers
returned by drm_atomic_get_plane_state without checking for errors. This
could lead to undefined behavior if the function returns an error pointer.
This commit adds checks using IS_ERR to ensure that plane_state is
valid before dereferencing them.
Similar to commit da29abe71e16
("drm/amd/display: Fix error pointers in amdgpu_dm_crtc_mem_type_changed").
Fixes: 774bcfb73176 ("drm/msm/dpu: add support for virtual planes") Signed-off-by: Chenyuan Yang <chenyuan0y@gmail.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/643132/ Link: https://lore.kernel.org/r/20250314011004.663804-1-chenyuan0y@gmail.com Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Jun Nie [Mon, 3 Mar 2025 15:14:30 +0000 (23:14 +0800)]
drm/msm/dpu: check every pipe per capability
The capability stored in sblk and pipe_hw_caps is checked only for
SSPP of the first pipe in the pair with current implementation. That
of the 2nd pipe, r_pipe, is not checked and may violate hardware
capability. Move requirement check to dpu_plane_atomic_check_pipe()
for the check of every pipe.
Specific constrain in if:then: blocks for variable lists, like clocks
and clock-names, should have a fixed upper and lower size. Older
dtschema implied minItems, but that's not true since 2024 and missing
minItems means that lower bound is not set.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Patchwork: https://patchwork.freedesktop.org/patch/638751/ Link: https://lore.kernel.org/r/20250221-b4-sm8750-display-v3-2-3ea95b1630ea@linaro.org Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Correct commit 20972609d12c ("drm/msm/dpu: Require modeset if clone mode
status changes") and describe old_crtc_state and new_crtc_state params
instead of the single previously used parameter crtc_state.
Fixes: 20972609d12c ("drm/msm/dpu: Require modeset if clone mode status changes") Signed-off-by: Dmitry Baryshkov <lumag@kernel.org> Reviewed-by: Rob Clark <robdclark@gmail.com>
Patchwork: https://patchwork.freedesktop.org/patch/641313/ Link: https://lore.kernel.org/r/20250306-dpu-fix-docs-v1-1-e51b71e8ad84@kernel.org Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Rob Clark [Fri, 28 Feb 2025 21:31:24 +0000 (13:31 -0800)]
drm/msm/a6xx: Fix a6xx indexed-regs in devcoreduump
Somehow, possibly as a result of rebase gone badly, setting
nr_indexed_regs for pre-a650 a6xx devices lost the setting of
nr_indexed_regs, resulting in values getting snapshot, but omitted
from the devcoredump.
Fixes: e997ae5f45ca ("drm/msm/a6xx: Mostly implement A7xx gpu_state") Signed-off-by: Rob Clark <robdclark@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/640289/
Dmitry Baryshkov [Sun, 9 Feb 2025 03:21:18 +0000 (05:21 +0200)]
drm/msm/dpu: handle perf mode in _dpu_core_perf_crtc_update_bus()
Move perf mode handling for the bandwidth to
_dpu_core_perf_crtc_update_bus() rather than overriding per-CRTC data
and then aggregating known values.
Note, this changes the fix_core_ab_vote. Previously it would be
multiplied per the CRTC number, now it will be used directly for
interconnect voting. This better reflects user requirements in the case
of different resolutions being set on different CRTCs: instead of using
the same bandwidth for each CRTC (which is incorrect) user can now
calculate overall bandwidth required by all outputs and use that value.
Note #2: this also disables threshold checks for user-entered bandwidth
values. First of all, it doesn't make sense to fail atomic commits
because of the debugfs input. Compositors have no way to correlate
failing commits with debugfs settings. Second, it makes sense to allow
users to go beyond these values and check whether this makes any
difference or fixes the issue.
Dmitry Baryshkov [Sun, 9 Feb 2025 03:21:14 +0000 (05:21 +0200)]
drm/msm/dpu: make fix_core_ab_vote consistent with fix_core_ib_vote
The fix_core_ab_vote is an average bandwidth value, used for bandwidth
overrides in several cases. However there is an internal inconsistency:
fix_core_ib_vote is defined in KBps, while fix_core_ab_vote is defined
in Bps.
Fix that by changing the type of the variable to u32 and using * 1000ULL
multiplier when setting up the dpu_core_perf_params::bw_ctl value.
Dmitry Baryshkov [Sun, 9 Feb 2025 03:21:13 +0000 (05:21 +0200)]
drm/msm/dpu: change ib values to u32
The IB values in core_perf calculations (max_per_pipe_ib,
fix_core_ib_vote) are expressed in KBps and are passed to icc_set_bw
without additional division. Change type of those values to u32.
The Qualcomm SA8775p MDSS display block comes with eDP phy, already used
in DTS and already documented in phy/qcom,edp-phy.yaml binding. Add the
missing device node in the binding and extend example to silence
dtbs_check warnings like:
sa8775p-ride.dtb: display-subsystem@ae00000: Unevaluated properties are not allowed ('phy@aec2a00', 'phy@aec5a00' were unexpected)
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Patchwork: https://patchwork.freedesktop.org/patch/638744/ Link: https://lore.kernel.org/r/20250221151311.138755-1-krzysztof.kozlowski@linaro.org Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Dmitry Baryshkov [Wed, 26 Feb 2025 08:59:30 +0000 (10:59 +0200)]
drm/msm/hdmi: use DRM HDMI Audio framework
In order to simplify the driver even further and to remove the
boilerplate code, rewrite the audio interface to use the DRM HDMI Audio
framework.
Audio InfoFames are controlled centrally via the DRM HDMI framework.
Correct InfoFrame data is programmed at the atomic_pre_enable() time (if
it was set before, drm_atomic_helper_connector_hdmi_update_infoframes()
takes care of writing all InfoFrames, including the Audio one.) or
during msm_hdmi_bridge_audio_prepare() when the new stream is started.
All audio data frame management is deferred to
msm_hdmi_bridge_audio_prepare() and msm_hdmi_bridge_audio_shutdown().
Dmitry Baryshkov [Wed, 26 Feb 2025 08:59:29 +0000 (10:59 +0200)]
drm/msm/hdmi: also send the SPD and HDMI Vendor Specific InfoFrames
Extend the driver to send SPD and HDMI Vendor Specific InfoFrames.
While the HDMI block has special block to send HVS InfoFrame, use
GENERIC0 block instead. VENSPEC_INFO registers pack frame data in a way
that requires manual repacking in the driver, while GENERIC0 doesn't
have such format requirements. The msm-4.4 kernel uses GENERIC0 to send
HDR InfoFrame which we do not at this point anyway.
Dmitry Baryshkov [Wed, 26 Feb 2025 08:59:25 +0000 (10:59 +0200)]
drm/msm/hdmi: program HDMI timings during atomic_pre_enable
The mode_set callback is deprecated, it doesn't get the
drm_bridge_state, just mode-related argumetns. Also Abhinav pointed out
that HDMI timings should be programmed before setting up HDMI PHY and
PLL. Rework the code to program HDMI timings at the start of
atomic_pre_enable().
Jessica Zhang [Sat, 15 Feb 2025 00:14:36 +0000 (16:14 -0800)]
drm/msm/dpu: Reorder encoder kickoff for CWB
Add a helper that will handle the correct order of the encoder kickoffs
for concurrent writeback.
For concurrent writeback, the realtime encoder must always kickoff last
as it will call the trigger flush and start.
This avoids the following scenario where the writeback encoder
increments the pending kickoff count after the WB_DONE interrupt is
fired:
If the realtime encoder is kicked off first, the encoder kickoff will
flush/start the encoder and increment the pending kickoff count. The
WB_DONE interrupt then fires (before the writeback encoder is kicked
off). When the writeback encoder enters its kickoff, it will skip the
flush/start (due to CWB being enabled) and hit a frame done timeout
as the frame was kicked off (and the WB_DONE interrupt fired) without
the pending kickoff count being incremented.
In addition, the writeback timer should only start after the realtime
encoder is kicked off to ensure that we don't get timeouts when the
system has a heavy load (ex. when debug logs are enabled)
Jessica Zhang [Sat, 15 Feb 2025 00:14:35 +0000 (16:14 -0800)]
drm/msm/dpu: Skip trigger flush and start for CWB
For concurrent writeback, the real time encoder is responsible for
trigger flush and trigger start. Return early for trigger start and
trigger flush for the concurrent writeback encoders.
Jessica Zhang [Sat, 15 Feb 2025 00:14:34 +0000 (16:14 -0800)]
drm/msm/dpu: Start frame done timer after encoder kickoff
Starting the frame done timer before the encoder is finished kicking off
can lead to unnecessary frame done timeouts when the device is
experiencing heavy load (ex. when debug logs are enabled).
Thus, create a separate API for starting the encoder frame done timer and
call it after the encoder kickoff is finished
Jessica Zhang [Sat, 15 Feb 2025 00:14:30 +0000 (16:14 -0800)]
drm/msm/dpu: Reserve resources for CWB
Add support for RM to reserve dedicated CWB PINGPONGs and CWB muxes
For concurrent writeback, even-indexed CWB muxes must be assigned to
even-indexed LMs and odd-indexed CWB muxes for odd-indexed LMs. The same
even/odd rule applies for dedicated CWB PINGPONGs.
Track the CWB muxes in the global state and add a CWB-specific helper to
reserve the correct CWB muxes and dedicated PINGPONGs following the
even/odd rule.
Jessica Zhang [Sat, 15 Feb 2025 00:14:29 +0000 (16:14 -0800)]
drm/msm/dpu: Fail atomic_check if multiple outputs request CDM block
Currently, our hardware only supports a single output using CDM block at
most. Because of this, we cannot support cases where both writeback and DP
output request CDM simultaneously
To avoid this happening when CWB is enabled, change
msm_display_topoloy.needs_cdm into a num_cdm counter to track how many
outputs are requesting CDM block. Return EINVAL if multiple outputs are
trying to reserve CDM.
Jessica Zhang [Sat, 15 Feb 2025 00:14:27 +0000 (16:14 -0800)]
drm/msm/dpu: Add CWB to msm_display_topology
Currently, the topology is calculated based on the assumption that the
user cannot request real-time and writeback simultaneously. For example,
the number of LMs and CTLs are currently based off the number of phys
encoders under the assumption there will be at least 1 LM/CTL per phys
encoder.
This will not hold true for concurrent writeback as both phys encoders
(1 real-time and 1 writeback) must be driven by 1 LM/CTL when concurrent
writeback is enabled.
To account for this, add a cwb_enabled flag and only adjust the number of
CTL/LMs needed by a given topology based on the number of phys encoders
only if CWB is not enabled.
Dmitry Baryshkov [Sat, 15 Feb 2025 00:14:25 +0000 (16:14 -0800)]
drm/msm/dpu: move resource allocation to CRTC
All resource allocation is centered around the LMs. Then other blocks
(except DSCs) are allocated basing on the LMs that was selected, and LM
powers up the CRTC rather than the encoder.
Moreover if at some point the driver supports encoder cloning,
allocating resources from the encoder will be incorrect, as all clones
will have different encoder IDs, while LMs are to be shared by these
encoders.
In addition, move mode_changed() to dpu_crtc as encoder no longer has
access to topology information
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
[quic_abhinavk@quicinc.com: Refactored resource allocation for CDM] Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
[quic_jesszhan@quicinc.com: Changed to grabbing exising global state] Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
[DB: rebased on top of msm-next]
[DB: fixed resource allcoation to ignore the active_changed flag]
Patchwork: https://patchwork.freedesktop.org/patch/637487/ Link: https://lore.kernel.org/r/20250214-concurrent-wb-v6-2-a44c293cf422@quicinc.com Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Dmitry Baryshkov [Thu, 23 Jan 2025 12:43:36 +0000 (14:43 +0200)]
drm/msm/dpu: don't set crtc_state->mode_changed from atomic_check()
The MSM driver uses drm_atomic_helper_check() which mandates that none
of the atomic_check() callbacks toggles crtc_state->mode_changed.
Perform corresponding check before calling the drm_atomic_helper_check()
function.
Fixes: 8b45a26f2ba9 ("drm/msm/dpu: reserve cdm blocks for writeback in case of YUV output") Reported-by: Simona Vetter <simona.vetter@ffwll.ch> Closes: https://lore.kernel.org/dri-devel/ZtW_S0j5AEr4g0QW@phenom.ffwll.local/ Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
[DB: dropped the WARN_ON] Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Patchwork: https://patchwork.freedesktop.org/patch/633400/ Link: https://lore.kernel.org/r/20250123-drm-dirty-modeset-v2-4-bbfd3a6cd1a4@linaro.org
Jie Zhang [Thu, 27 Feb 2025 20:07:52 +0000 (01:37 +0530)]
dt-bindings: display/msm/gmu: Add Adreno 623 GMU
Document Adreno 623 GMU in the dt-binding specification.
Signed-off-by: Jie Zhang <quic_jiezh@quicinc.com> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Patchwork: https://patchwork.freedesktop.org/patch/640058/ Signed-off-by: Rob Clark <robdclark@chromium.org>
Jie Zhang [Thu, 27 Feb 2025 20:07:51 +0000 (01:37 +0530)]
drm/msm/a6xx: Add support for Adreno 623
Add support for Adreno 623 GPU found in QCS8300 chipsets.
Signed-off-by: Jie Zhang <quic_jiezh@quicinc.com> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/640056/ Signed-off-by: Rob Clark <robdclark@chromium.org>
Jie Zhang [Thu, 27 Feb 2025 20:07:50 +0000 (01:37 +0530)]
drm/msm/a6xx: Fix gpucc register block for A621
Adreno 621 has a different memory map for GPUCC block. So update
a6xx_gpu_state code to dump the correct set of gpucc registers.
Signed-off-by: Jie Zhang <quic_jiezh@quicinc.com> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/640055/ Signed-off-by: Rob Clark <robdclark@chromium.org>
Jie Zhang [Thu, 27 Feb 2025 20:07:49 +0000 (01:37 +0530)]
drm/msm/a6xx: Split out gpucc register block
Some GPUs have different memory map for GPUCC block. So split out the
gpucc range from a6xx_gmu_cx_registers to a separate block to
accommodate those GPUs.
Signed-off-by: Jie Zhang <quic_jiezh@quicinc.com> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/640052/ Signed-off-by: Rob Clark <robdclark@chromium.org>
Dan Carpenter [Mon, 17 Feb 2025 07:32:11 +0000 (10:32 +0300)]
drm/msm/gem: Fix error code msm_parse_deps()
The SUBMIT_ERROR() macro turns the error code negative. This extra '-'
operation turns it back to positive EINVAL again. The error code is
passed to ERR_PTR() and since positive values are not an IS_ERR() it
eventually will lead to an oops. Delete the '-'.
Fixes: 866e43b945bf ("drm/msm: UAPI error reporting") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Patchwork: https://patchwork.freedesktop.org/patch/637625/ Signed-off-by: Rob Clark <robdclark@chromium.org>
Rob Clark [Tue, 10 Dec 2024 16:51:22 +0000 (08:51 -0800)]
drm/msm: Extend gpu devcore dumps with pgtbl info
In the case of iova fault triggered devcore dumps, include additional
debug information based on what we think is the current page tables,
including the TTBR0 value (which should match what we have in
adreno_smmu_fault_info unless things have gone horribly wrong), and
the pagetable entries traversed in the process of resolving the
faulting iova.
Signed-off-by: Rob Clark <robdclark@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/628117/
Rob Clark [Tue, 25 Feb 2025 22:52:43 +0000 (14:52 -0800)]
drm/msm: Expose DRIVER_SYNCOBJ_TIMELINE
Initially we didn't want to expose the cap, as it would expose a bug
in the vk driver (turnip) with older mesa versions. This was fixed over
a year ago (and cherry-picked to stable release branches at the time),
see https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25981.
So let's go ahead and expose it now.
Signed-off-by: Rob Clark <robdclark@chromium.org> Reviewed-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/639610/
Akhil P Oommen [Tue, 25 Feb 2025 19:52:14 +0000 (01:22 +0530)]
drm/msm/a6xx: Fix stale rpmh votes from GPU
It was observed on sc7180 (A618 gpu) that GPU votes for GX rail and CNOC
BCM nodes were not removed after GPU suspend. This was because we
skipped sending 'prepare-slumber' request to gmu during suspend sequence
in some cases. So, make sure we always call prepare-slumber hfi during
suspend. Also, calling prepare-slumber without a prior oob-gpu handshake
messes up gmu firmware's internal state. So, do that when required.
Fixes: 4b565ca5a2cb ("drm/msm: Add A6XX device support") Cc: stable@vger.kernel.org Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/639569/ Signed-off-by: Rob Clark <robdclark@chromium.org>
Abhinav Kumar [Wed, 19 Feb 2025 19:49:21 +0000 (11:49 -0800)]
drm/msm/dpu: rate limit snapshot capture for mmu faults
There is no recovery mechanism in place yet to recover from mmu
faults for DPU. We can only prevent the faults by making sure there
is no misconfiguration.
Rate-limit the snapshot capture for mmu faults to once per
msm_atomic_commit_tail() as that should be sufficient to capture
the snapshot for debugging otherwise there will be a lot of DPU
snapshots getting captured for the same fault which is redundant
and also might affect capturing even one snapshot accurately.
Abhinav Kumar [Wed, 19 Feb 2025 19:49:17 +0000 (11:49 -0800)]
drm/msm: register a fault handler for display mmu faults
In preparation to register a iommu fault handler for display
related modules, register a fault handler for the backing
mmu object of msm_kms.
Currently, the fault handler only captures the display snapshot
but we can expand this later if more information needs to be
added to debug display mmu faults.
Krzysztof Kozlowski [Wed, 19 Feb 2025 16:23:32 +0000 (17:23 +0100)]
drm/msm/dsi/phy: Use dsi_pll_cmn_clk_cfg1_update() when registering PLL
Newly added dsi_pll_cmn_clk_cfg1_update() wrapper protects concurrent
updates to PHY_CMN_CLK_CFG1 register between driver and Common Clock
Framework. pll_7nm_register() still used in one place previous
readl+writel, which can be simplified with this new wrapper.
This is purely for readability and simplification and should have no
functional impact, because the code touched here is before clock is
registered via CCF, so there is no concurrency issue.
Eugene Lepshy [Mon, 17 Feb 2025 22:24:30 +0000 (01:24 +0300)]
drm/msm/dsi: Allow values of 10 and 12 for bits per component
The DRM DSC helper has various bits_per_component values - not just 8.
But the DSC 1.1 block supports only 8, 10, and 12. Extend the guard
accordingly and add a comment noting this limitation.
Marijn Suijten [Mon, 17 Feb 2025 11:17:43 +0000 (12:17 +0100)]
drm/msm/dpu: Remove arbitrary limit of 1 interface in DSC topology
When DSC is enabled the number of interfaces is forced to be 1, and
documented that it is a "power-optimal" layout to use two DSC encoders
together with two Layer Mixers. However, the same layout (two DSC
hard-slice encoders with two LMs) is also used when the display is
fed with data over two instead of one interface (common on 4k@120Hz
smartphone panels with Dual-DSI). Solve this by simply removing the
num_intf = 1 assignment as the count is already calculated by computing
the number of physical encoders within the virtual encoder.
Fixes: 7e9cc175b159 ("drm/msm/disp/dpu1: Add support for DSC in topology") Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
Patchwork: https://patchwork.freedesktop.org/patch/637649/ Link: https://lore.kernel.org/r/20250217-drm-msm-initial-dualpipe-dsc-fixes-v3-3-913100d6103f@somainline.org Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Marijn Suijten [Mon, 17 Feb 2025 11:17:42 +0000 (12:17 +0100)]
drm/msm/dsi: Set PHY usescase (and mode) before registering DSI host
Ordering issues here cause an uninitialized (default STANDALONE)
usecase to be programmed (which appears to be a MUX) in some cases
when msm_dsi_host_register() is called, leading to the slave PLL in
bonded-DSI mode to source from a clock parent (dsi1vco) that is off.
This should seemingly not be a problem as the actual dispcc clocks from
DSI1 that are muxed in the clock tree of DSI0 are way further down, this
bit still seems to have an effect on them somehow and causes the right
side of the panel controlled by DSI1 to not function.
In an ideal world this code is refactored to no longer have such
error-prone calls "across subsystems", and instead model the "PLL src"
register field as a regular mux so that changing the clock parents
programmatically or in DTS via `assigned-clock-parents` has the
desired effect.
But for the avid reader, the clocks that we *are* muxing into DSI0's
tree are way further down, so if this bit turns out to be a simple mux
between dsiXvco and out_div, that shouldn't have any effect as this
whole tree is off anyway.
Fixes: 57bf43389337 ("drm/msm/dsi: Pass down use case to PHY") Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
Patchwork: https://patchwork.freedesktop.org/patch/637650/ Link: https://lore.kernel.org/r/20250217-drm-msm-initial-dualpipe-dsc-fixes-v3-2-913100d6103f@somainline.org Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Marijn Suijten [Mon, 17 Feb 2025 11:17:41 +0000 (12:17 +0100)]
drm/msm/dsi: Use existing per-interface slice count in DSC timing
When configuring the timing of DSI hosts (interfaces) in
dsi_timing_setup() all values written to registers are taking
bonded-mode into account by dividing the original mode width by 2
(half the data is sent over each of the two DSI hosts), but the full
width instead of the interface width is passed as hdisplay parameter to
dsi_update_dsc_timing().
Currently only msm_dsc_get_slices_per_intf() is called within
dsi_update_dsc_timing() with the `hdisplay` argument which clearly
documents that it wants the width of a single interface (which, again,
in bonded DSI mode is half the total width of the mode) resulting in all
subsequent values to be completely off.
However, as soon as we start to pass the halved hdisplay
into dsi_update_dsc_timing() we might as well discard
msm_dsc_get_slices_per_intf() since the value it calculates is already
available in dsc->slice_count which is per-interface by the current
design of MSM DPU/DSI implementations and their use of the DRM DSC
helpers.
Krzysztof Kozlowski [Fri, 14 Feb 2025 13:17:46 +0000 (14:17 +0100)]
drm/msm/dsi: Minor whitespace and style cleanup
Cleanup few obvious kernel coding style violations: missing or
unnecessary braces in 'if-else', unnecessary break lines, incorrect
breaking of long function declarations, unnecessary 'else' after a
'return'. No functional impact expected.
Krzysztof Kozlowski [Fri, 14 Feb 2025 13:17:45 +0000 (14:17 +0100)]
drm/msm/dsi: Simplify with dev_err_probe()
dsi_get_config(), dsi_clk_init() and msm_dsi_host_init() are called only
from platform driver probe function, so using dev_err_probe() is both
appropriate and beneficial:
- Properly marks device deferred probe status,
- Avoids dmesg flood on probe deferrals,
- Already incorporates printing ERR value,
- Shows device name (in contrast to pr_err()),
- Makes code smaller and simpler.
Krzysztof Kozlowski [Fri, 14 Feb 2025 13:17:44 +0000 (14:17 +0100)]
drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths
dsi_clk_init(), which gets the clocks, is called only through platform
driver probe and its failure is a failure of the probe. Therefore
NULL-ifying specific clocks is pointless and redundant - the PTR_ERR
value stored there won't be used/dereferenced afterwards. What's more,
variant-specific clock init calls like dsi_clk_init_6g_v2() are not
doing this cleanup. Dropping redundant code allows later to make this a
bit simpler.
Neil Armstrong [Thu, 13 Feb 2025 16:27:57 +0000 (17:27 +0100)]
dt-bindings: display: qcom,sm8650-mdss: explicitly document mdp0-mem and cpu-cfg interconnect paths
The mdp1-mem is not supported on the SM8550 SoCs, and having maxItems=2
makes the bindings not clear if mdp0-mem/mdp1-mem or mdp0-mem/cpu-cfg
is required, so explicitly document the mdp0-mem/cpu-cfg interconnect
paths and complete the example with the missing interconnect paths.
Neil Armstrong [Thu, 13 Feb 2025 16:27:56 +0000 (17:27 +0100)]
dt-bindings: display: qcom,sm8550-mdss: explicitly document mdp0-mem and cpu-cfg interconnect paths
The mdp1-mem is not supported on the SM8550 SoCs, and having maxItems=2
makes the bindings not clear if mdp0-mem/mdp1-mem or mdp0-mem/cpu-cfg is
required, so explicitly document the mdp0-mem/cpu-cfg interconnect and
add the cpu-cfg path in the example.
Krzysztof Kozlowski [Wed, 29 Jan 2025 11:55:04 +0000 (12:55 +0100)]
drm/msm/dsi/phy: Program clock inverters in correct register
Since SM8250 all downstream sources program clock inverters in
PLL_CLOCK_INVERTERS_1 register and leave the PLL_CLOCK_INVERTERS as
reset value (0x0). The most recent Hardware Programming Guide for 3 nm,
4 nm, 5 nm and 7 nm PHYs also mention PLL_CLOCK_INVERTERS_1.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Fixes: 1ef7c99d145c ("drm/msm/dsi: add support for 7nm DSI PHY/PLL") Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reported-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/634489/ Link: https://lore.kernel.org/r/20250129115504.40080-1-krzysztof.kozlowski@linaro.org Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Krzysztof Kozlowski [Mon, 27 Jan 2025 13:21:04 +0000 (14:21 +0100)]
dt-bindings: display/msm/dsi-phy: Add header with exposed clock IDs
DSI phys, from earliest (28 nm) up to newest (3 nm) generation, provide
two clocks. The respective clock ID is used by drivers and DTS, so it
should be documented as explicit ABI.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Acked-by: Stephen Boyd <sboyd@kernel.org> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Patchwork: https://patchwork.freedesktop.org/patch/634146/ Link: https://lore.kernel.org/r/20250127132105.107138-1-krzysztof.kozlowski@linaro.org Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
As a preparation for calling dpu_encoder_get_topology() from different
code paths, simplify its calling interface, obtaining some data pointers
internally instead passing them via arguments.
Dmitry Baryshkov [Thu, 23 Jan 2025 12:43:34 +0000 (14:43 +0200)]
drm/msm/dpu: move needs_cdm setting to dpu_encoder_get_topology()
As a preparation for calling dpu_encoder_get_topology() from different
places, move the code setting topology->needs_cdm to that function
(instead of patching topology separately).
Dmitry Baryshkov [Thu, 23 Jan 2025 12:43:33 +0000 (14:43 +0200)]
drm/msm/dpu: don't use active in atomic_check()
The driver isn't supposed to consult crtc_state->active/active_check for
resource allocation. Instead all resources should be allocated if
crtc_state->enabled is set. Stop consulting active / active_changed in
order to determine whether the hardware resources should be
(re)allocated.
Marijn Suijten [Wed, 22 Jan 2025 16:23:44 +0000 (17:23 +0100)]
drm/msm/dpu: Fall back to a single DSC encoder (1:1:1) on small SoCs
Some SoCs such as SC7280 (used in the Fairphone 5) have only a single
DSC "hard slice" encoder. The current hardcoded use of 2:2:1 topology
(2 LM and 2 DSC for a single interface) make it impossible to use
Display Stream Compression panels with mainline, which is exactly what's
installed on the Fairphone 5.
By loosening the hardcoded `num_dsc = 2` to fall back to `num_dsc =
1` when the catalog only contains one entry, we can trivially support
this phone and unblock further panel enablement on mainline. A few
more supporting changes in this patch ensure hardcoded constants of 2
DSC encoders are replaced to count or read back the actual number of
DSC hardware blocks that are enabled for the given virtual encoder.
Likewise DSC_MODE_SPLIT_PANEL can no longer be unconditionally enabled.
Krzysztof Kozlowski [Tue, 14 Jan 2025 19:17:24 +0000 (20:17 +0100)]
drm/msm: Use str_enable_disable-like helpers
Replace ternary (condition ? "enable" : "disable") syntax with helpers
from string_choices.h because:
1. Simple function call with one argument is easier to read. Ternary
operator has three arguments and with wrapping might lead to quite
long code.
2. Is slightly shorter thus also easier to read.
3. It brings uniformity in the text - same string.
4. Allows deduping by the linker, which results in a smaller binary
file.
Abel Vesa [Mon, 3 Feb 2025 10:57:59 +0000 (12:57 +0200)]
drm/msm/dp: Add support for LTTPR handling
Link Training Tunable PHY Repeaters (LTTPRs) are defined in DisplayPort
1.4a specification. As the name suggests, these PHY repeaters are
capable of adjusting their output for link training purposes.
According to the DisplayPort standard, LTTPRs have two operating
modes:
- non-transparent - it replies to DPCD LTTPR field specific AUX
requests, while passes through all other AUX requests
- transparent - it passes through all AUX requests.
Switching between these two modes is done by the DPTX by issuing
an AUX write to the DPCD PHY_REPEATER_MODE register.
The msm DP driver is currently lacking any handling of LTTPRs.
This means that if at least one LTTPR is found between DPTX and DPRX,
the link training would fail if that LTTPR was not already configured
in transparent mode.
The section 3.6.6.1 from the DisplayPort v2.0 specification mandates
that before link training with the LTTPR is started, the DPTX may place
the LTTPR in non-transparent mode by first switching to transparent mode
and then to non-transparent mode. This operation seems to be needed only
on first link training and doesn't need to be done again until device is
unplugged.
It has been observed on a few X Elite-based platforms which have
such LTTPRs in their board design that the DPTX needs to follow the
procedure described above in order for the link training to be successful.
So add support for reading the LTTPR DPCD caps to figure out the number
of such LTTPRs first. Then, for platforms (or Type-C dongles) that have
at least one such an LTTPR, set its operation mode to transparent mode
first and then to non-transparent, just like the mentioned section of
the specification mandates.
Tested-by: Johan Hovold <johan+linaro@kernel.org> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Johan Hovold <johan+linaro@kernel.org> Signed-off-by: Abel Vesa <abel.vesa@linaro.org> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Acked-by: Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20250203-drm-dp-msm-add-lttpr-transparent-mode-set-v5-4-c865d0e56d6e@linaro.org Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Abel Vesa [Mon, 3 Feb 2025 10:57:58 +0000 (12:57 +0200)]
drm/i915/dp: Use the generic helper to control LTTPR transparent mode
LTTPRs operating modes are defined by the DisplayPort standard and the
generic framework now provides a helper to switch between them, which
is handling the explicit disabling of non-transparent mode and its
disable->enable sequence mentioned in the DP Standard v2.0 section
3.6.6.1.
So use the new drm generic helper instead as it makes the code a bit
cleaner. Since the driver specific implementation holds the
lttrp_common_caps, if the call to the drm generic helper fails, the
lttrp_common_caps need to be updated as the helper has already rolled
back to transparent mode.
Abel Vesa [Mon, 3 Feb 2025 10:57:57 +0000 (12:57 +0200)]
drm/nouveau/dp: Use the generic helper to control LTTPR transparent mode
LTTPRs operating modes are defined by the DisplayPort standard and the
generic framework now provides a helper to switch between them, which
is handling the explicit disabling of non-transparent mode and its
disable->enable sequence mentioned in the DP Standard v2.0 section
3.6.6.1.
So use the new drm generic helper instead as it makes the code a bit
cleaner.
Abel Vesa [Mon, 3 Feb 2025 10:57:56 +0000 (12:57 +0200)]
drm/dp: Add helper to set LTTPRs in transparent mode
According to the DisplayPort standard, LTTPRs have two operating
modes:
- non-transparent - it replies to DPCD LTTPR field specific AUX
requests, while passes through all other AUX requests
- transparent - it passes through all AUX requests.
Switching between this two modes is done by the DPTX by issuing
an AUX write to the DPCD PHY_REPEATER_MODE register.
Add a generic helper that allows switching between these modes.
Also add a generic wrapper for the helper that handles the explicit
disabling of non-transparent mode and its disable->enable sequence
mentioned in the DP Standard v2.0 section 3.6.6.1. Do this in order
to move this handling out of the vendor specific driver implementation
into the generic framework.
Tested-by: Johan Hovold <johan+linaro@kernel.org> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Johan Hovold <johan+linaro@kernel.org> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com> Signed-off-by: Abel Vesa <abel.vesa@linaro.org> Acked-by: Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20250203-drm-dp-msm-add-lttpr-transparent-mode-set-v5-1-c865d0e56d6e@linaro.org Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>