From b6090ffb30f3301d3831774f9c3e2f1b1141a399 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Wed, 22 Jan 2025 17:23:44 +0100 Subject: [PATCH 01/16] 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. Cc: Luca Weiss Signed-off-by: Marijn Suijten Reviewed-by: Dmitry Baryshkov Tested-by: Luca Weiss Reviewed-by: Jessica Zhang Tested-by: Danila Tikhonov Patchwork: https://patchwork.freedesktop.org/patch/633318/ Link: https://lore.kernel.org/r/20250122-dpu-111-topology-v2-1-505e95964af9@somainline.org Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 47 +++++++++++---------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 503dfd79b8f2..e3cbd65d2b13 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -622,9 +622,9 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc) if (dpu_enc->phys_encs[i]) intf_count++; - /* See dpu_encoder_get_topology, we only support 2:2:1 topology */ - if (dpu_enc->dsc) - num_dsc = 2; + for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) + if (dpu_enc->hw_dsc[i]) + num_dsc++; return (num_dsc > 0) && (num_dsc > intf_count); } @@ -686,13 +686,19 @@ static struct msm_display_topology dpu_encoder_get_topology( if (dsc) { /* - * In case of Display Stream Compression (DSC), we would use - * 2 DSC encoders, 2 layer mixers and 1 interface - * this is power optimal and can drive up to (including) 4k - * screens + * Use 2 DSC encoders and 2 layer mixers per single interface + * when Display Stream Compression (DSC) is enabled, + * and when enough DSC blocks are available. + * This is power-optimal and can drive up to (including) 4k + * screens. */ - topology.num_dsc = 2; - topology.num_lm = 2; + if (dpu_kms->catalog->dsc_count >= 2) { + topology.num_dsc = 2; + topology.num_lm = 2; + } else { + topology.num_dsc = 1; + topology.num_lm = 1; + } topology.num_intf = 1; } @@ -2020,7 +2026,6 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl, static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, struct drm_dsc_config *dsc) { - /* coding only for 2LM, 2enc, 1 dsc config */ struct dpu_encoder_phys *enc_master = dpu_enc->cur_master; struct dpu_hw_ctl *ctl = enc_master->hw_ctl; struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC]; @@ -2030,22 +2035,24 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, int dsc_common_mode; int pic_width; u32 initial_lines; + int num_dsc = 0; int i; for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) { hw_pp[i] = dpu_enc->hw_pp[i]; hw_dsc[i] = dpu_enc->hw_dsc[i]; - if (!hw_pp[i] || !hw_dsc[i]) { - DPU_ERROR_ENC(dpu_enc, "invalid params for DSC\n"); - return; - } + if (!hw_pp[i] || !hw_dsc[i]) + break; + + num_dsc++; } - dsc_common_mode = 0; pic_width = dsc->pic_width; - dsc_common_mode = DSC_MODE_SPLIT_PANEL; + dsc_common_mode = 0; + if (num_dsc > 1) + dsc_common_mode |= DSC_MODE_SPLIT_PANEL; if (dpu_encoder_use_dsc_merge(enc_master->parent)) dsc_common_mode |= DSC_MODE_MULTIPLEX; if (enc_master->intf_mode == INTF_MODE_VIDEO) @@ -2054,14 +2061,10 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc, this_frame_slices = pic_width / dsc->slice_width; intf_ip_w = this_frame_slices * dsc->slice_width; - /* - * dsc merge case: when using 2 encoders for the same stream, - * no. of slices need to be same on both the encoders. - */ - enc_ip_w = intf_ip_w / 2; + enc_ip_w = intf_ip_w / num_dsc; initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w); - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) + for (i = 0; i < num_dsc; i++) dpu_encoder_dsc_pipe_cfg(ctl, hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, initial_lines); } -- 2.51.0 From 25b4614843bcc56ba150f7c99905125a019e656c Mon Sep 17 00:00:00 2001 From: Dmitry Baryshkov Date: Thu, 23 Jan 2025 14:43:33 +0200 Subject: [PATCH 02/16] 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. Fixes: ccc862b957c6 ("drm/msm/dpu: Fix reservation failures in modeset") Reported-by: Simona Vetter Closes: https://lore.kernel.org/dri-devel/ZtW_S0j5AEr4g0QW@phenom.ffwll.local/ Reviewed-by: Simona Vetter Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov Patchwork: https://patchwork.freedesktop.org/patch/633393/ Link: https://lore.kernel.org/r/20250123-drm-dirty-modeset-v2-1-bbfd3a6cd1a4@linaro.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 ---- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 +-- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index e5dcd41a361f..29485e76f531 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1262,10 +1262,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc, DRM_DEBUG_ATOMIC("%s: check\n", dpu_crtc->name); - /* force a full mode set if active state changed */ - if (crtc_state->active_changed) - crtc_state->mode_changed = true; - if (cstate->num_mixers) { rc = _dpu_crtc_check_and_setup_lm_bounds(crtc, crtc_state); if (rc) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index e3cbd65d2b13..aeec5a5ab8ff 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -799,12 +799,11 @@ static int dpu_encoder_virt_atomic_check( crtc_state->mode_changed = true; /* * Release and Allocate resources on every modeset - * Dont allocate when active is false. */ if (drm_atomic_crtc_needs_modeset(crtc_state)) { dpu_rm_release(global_state, drm_enc); - if (!crtc_state->active_changed || crtc_state->enable) + if (crtc_state->enable) ret = dpu_rm_reserve(&dpu_kms->rm, global_state, drm_enc, crtc_state, &topology); if (!ret) -- 2.51.0 From 7d39f5bb82c0d7155037982dd0ff583a68db1c34 Mon Sep 17 00:00:00 2001 From: Dmitry Baryshkov Date: Thu, 23 Jan 2025 14:43:34 +0200 Subject: [PATCH 03/16] 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). Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov Patchwork: https://patchwork.freedesktop.org/patch/633395/ Link: https://lore.kernel.org/r/20250123-drm-dirty-modeset-v2-2-bbfd3a6cd1a4@linaro.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 41 +++++++++++---------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index aeec5a5ab8ff..de329761cdb6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -652,8 +652,11 @@ static struct msm_display_topology dpu_encoder_get_topology( struct dpu_kms *dpu_kms, struct drm_display_mode *mode, struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state, struct drm_dsc_config *dsc) { + struct msm_drm_private *priv = dpu_enc->base.dev->dev_private; + struct msm_display_info *disp_info = &dpu_enc->disp_info; struct msm_display_topology topology = {0}; int i, intf_count = 0; @@ -702,6 +705,23 @@ static struct msm_display_topology dpu_encoder_get_topology( topology.num_intf = 1; } + /* + * Use CDM only for writeback or DP at the moment as other interfaces cannot handle it. + * If writeback itself cannot handle cdm for some reason it will fail in its atomic_check() + * earlier. + */ + if (disp_info->intf_type == INTF_WB && conn_state->writeback_job) { + struct drm_framebuffer *fb; + + fb = conn_state->writeback_job->fb; + + if (fb && MSM_FORMAT_IS_YUV(msm_framebuffer_format(fb))) + topology.needs_cdm = true; + } else if (disp_info->intf_type == INTF_DP) { + if (msm_dp_is_yuv_420_enabled(priv->dp[disp_info->h_tile_instance[0]], mode)) + topology.needs_cdm = true; + } + return topology; } @@ -749,9 +769,7 @@ static int dpu_encoder_virt_atomic_check( struct dpu_kms *dpu_kms; struct drm_display_mode *adj_mode; struct msm_display_topology topology; - struct msm_display_info *disp_info; struct dpu_global_state *global_state; - struct drm_framebuffer *fb; struct drm_dsc_config *dsc; int ret = 0; @@ -765,7 +783,6 @@ static int dpu_encoder_virt_atomic_check( DPU_DEBUG_ENC(dpu_enc, "\n"); priv = drm_enc->dev->dev_private; - disp_info = &dpu_enc->disp_info; dpu_kms = to_dpu_kms(priv->kms); adj_mode = &crtc_state->adjusted_mode; global_state = dpu_kms_get_global_state(crtc_state->state); @@ -776,22 +793,8 @@ static int dpu_encoder_virt_atomic_check( dsc = dpu_encoder_get_dsc_config(drm_enc); - topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, dsc); - - /* - * Use CDM only for writeback or DP at the moment as other interfaces cannot handle it. - * If writeback itself cannot handle cdm for some reason it will fail in its atomic_check() - * earlier. - */ - if (disp_info->intf_type == INTF_WB && conn_state->writeback_job) { - fb = conn_state->writeback_job->fb; - - if (fb && MSM_FORMAT_IS_YUV(msm_framebuffer_format(fb))) - topology.needs_cdm = true; - } else if (disp_info->intf_type == INTF_DP) { - if (msm_dp_is_yuv_420_enabled(priv->dp[disp_info->h_tile_instance[0]], adj_mode)) - topology.needs_cdm = true; - } + topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, conn_state, + dsc); if (topology.needs_cdm && !dpu_enc->cur_master->hw_cdm) crtc_state->mode_changed = true; -- 2.51.0 From 41921f231abf9a3a95550b2b565df8e4329319cb Mon Sep 17 00:00:00 2001 From: Dmitry Baryshkov Date: Thu, 23 Jan 2025 14:43:35 +0200 Subject: [PATCH 04/16] drm/msm/dpu: simplify dpu_encoder_get_topology() interface 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. Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov Patchwork: https://patchwork.freedesktop.org/patch/633396/ Link: https://lore.kernel.org/r/20250123-drm-dirty-modeset-v2-3-bbfd3a6cd1a4@linaro.org --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index de329761cdb6..938d8b05a4cd 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -649,14 +649,14 @@ struct drm_dsc_config *dpu_encoder_get_dsc_config(struct drm_encoder *drm_enc) static struct msm_display_topology dpu_encoder_get_topology( struct dpu_encoder_virt *dpu_enc, - struct dpu_kms *dpu_kms, struct drm_display_mode *mode, struct drm_crtc_state *crtc_state, - struct drm_connector_state *conn_state, - struct drm_dsc_config *dsc) + struct drm_connector_state *conn_state) { struct msm_drm_private *priv = dpu_enc->base.dev->dev_private; struct msm_display_info *disp_info = &dpu_enc->disp_info; + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); + struct drm_dsc_config *dsc = dpu_encoder_get_dsc_config(&dpu_enc->base); struct msm_display_topology topology = {0}; int i, intf_count = 0; @@ -770,7 +770,6 @@ static int dpu_encoder_virt_atomic_check( struct drm_display_mode *adj_mode; struct msm_display_topology topology; struct dpu_global_state *global_state; - struct drm_dsc_config *dsc; int ret = 0; if (!drm_enc || !crtc_state || !conn_state) { @@ -791,10 +790,7 @@ static int dpu_encoder_virt_atomic_check( trace_dpu_enc_atomic_check(DRMID(drm_enc)); - dsc = dpu_encoder_get_dsc_config(drm_enc); - - topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, conn_state, - dsc); + topology = dpu_encoder_get_topology(dpu_enc, adj_mode, crtc_state, conn_state); if (topology.needs_cdm && !dpu_enc->cur_master->hw_cdm) crtc_state->mode_changed = true; -- 2.51.0 From d1f28e30a525107cd3b7927b73c69fbab9e826a5 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Mon, 27 Jan 2025 14:21:04 +0100 Subject: [PATCH 05/16] 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 Acked-by: Stephen Boyd Acked-by: Dmitry Baryshkov 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 --- .../devicetree/bindings/display/msm/dsi-phy-common.yaml | 2 ++ MAINTAINERS | 1 + include/dt-bindings/clock/qcom,dsi-phy-28nm.h | 9 +++++++++ 3 files changed, 12 insertions(+) create mode 100644 include/dt-bindings/clock/qcom,dsi-phy-28nm.h diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-common.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-common.yaml index 6b57ce41c95f..d0ce85a08b6d 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-common.yaml +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-common.yaml @@ -15,6 +15,8 @@ description: properties: "#clock-cells": const: 1 + description: + See include/dt-bindings/clock/qcom,dsi-phy-28nm.h for clock IDs. "#phy-cells": const: 0 diff --git a/MAINTAINERS b/MAINTAINERS index 43b55429f0fc..9f29ec77a654 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7392,6 +7392,7 @@ T: git https://gitlab.freedesktop.org/drm/msm.git F: Documentation/devicetree/bindings/display/msm/ F: drivers/gpu/drm/ci/xfails/msm* F: drivers/gpu/drm/msm/ +F: include/dt-bindings/clock/qcom,dsi-phy-28nm.h F: include/uapi/drm/msm_drm.h DRM DRIVER FOR NOVATEK NT35510 PANELS diff --git a/include/dt-bindings/clock/qcom,dsi-phy-28nm.h b/include/dt-bindings/clock/qcom,dsi-phy-28nm.h new file mode 100644 index 000000000000..ab94d58377a1 --- /dev/null +++ b/include/dt-bindings/clock/qcom,dsi-phy-28nm.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */ + +#ifndef _DT_BINDINGS_CLK_QCOM_DSI_PHY_28NM_H +#define _DT_BINDINGS_CLK_QCOM_DSI_PHY_28NM_H + +#define DSI_BYTE_PLL_CLK 0 +#define DSI_PIXEL_PLL_CLK 1 + +#endif -- 2.51.0 From 5100ae76b5ab6afab33f38bc5850da2d076e5732 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Mon, 27 Jan 2025 14:21:05 +0100 Subject: [PATCH 06/16] drm/msm/dsi/phy: Use the header with clock IDs Use the header with clock IDs to bind the interface between driver and DTS. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Dmitry Baryshkov Patchwork: https://patchwork.freedesktop.org/patch/634149/ Link: https://lore.kernel.org/r/20250127132105.107138-2-krzysztof.kozlowski@linaro.org Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 5 ++--- drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 1 + drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 1 + drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 1 + drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 1 + drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 1 + 6 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h index 8985818bb2e0..1925418d9999 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h @@ -6,6 +6,7 @@ #ifndef __DSI_PHY_H__ #define __DSI_PHY_H__ +#include #include #include #include @@ -84,9 +85,7 @@ struct msm_dsi_dphy_timing { u8 hs_halfbyte_en_ckln; }; -#define DSI_BYTE_PLL_CLK 0 -#define DSI_PIXEL_PLL_CLK 1 -#define NUM_PROVIDED_CLKS 2 +#define NUM_PROVIDED_CLKS (DSI_PIXEL_PLL_CLK + 1) #define DSI_LANE_MAX 5 diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c index 677c62571811..9812b4d69197 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c @@ -3,6 +3,7 @@ * Copyright (c) 2018, The Linux Foundation */ +#include #include #include #include diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c index 2c3cbe0f2870..3a1c8ece6657 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c @@ -3,6 +3,7 @@ * Copyright (c) 2016, The Linux Foundation. All rights reserved. */ +#include #include #include #include diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c index 1383e3a4e050..90348a2af3e9 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c @@ -3,6 +3,7 @@ * Copyright (c) 2015, The Linux Foundation. All rights reserved. */ +#include #include #include diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c index 5311ab7f3c70..f3643320ff2f 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c @@ -3,6 +3,7 @@ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved. */ +#include #include #include diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c index 798168180c1a..e496a95c34e9 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c @@ -3,6 +3,7 @@ * Copyright (c) 2018, The Linux Foundation */ +#include #include #include #include -- 2.51.0 From baf49072877726616c7f5943a6b45eb86bfeca0a Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Wed, 29 Jan 2025 12:55:04 +0100 Subject: [PATCH 07/16] 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 Fixes: 1ef7c99d145c ("drm/msm/dsi: add support for 7nm DSI PHY/PLL") Reviewed-by: Dmitry Baryshkov Reported-by: Abhinav Kumar 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 --- drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c index e496a95c34e9..3332399c7fd7 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c @@ -306,7 +306,7 @@ static void dsi_pll_commit(struct dsi_pll_7nm *pll, struct dsi_pll_config *confi writel(pll->phy->cphy_mode ? 0x00 : 0x10, base + REG_DSI_7nm_PHY_PLL_CMODE_1); writel(config->pll_clock_inverters, - base + REG_DSI_7nm_PHY_PLL_CLOCK_INVERTERS); + base + REG_DSI_7nm_PHY_PLL_CLOCK_INVERTERS_1); } static int dsi_pll_7nm_vco_set_rate(struct clk_hw *hw, unsigned long rate, -- 2.51.0 From e05b233ae13b2ee6ea30d8c9f445dc5efbde6ce6 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Thu, 13 Feb 2025 17:27:56 +0100 Subject: [PATCH 08/16] 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. Suggested-by: Dmitry Baryshkov Reviewed-by: Krzysztof Kozlowski Signed-off-by: Neil Armstrong Patchwork: https://patchwork.freedesktop.org/patch/637050/ Link: https://lore.kernel.org/r/20250213-topic-sm8x50-mdss-interconnect-bindings-fix-v4-1-3fa0bc42dd38@linaro.org Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/qcom,sm8550-mdss.yaml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml index 1ea50a2c7c8e..59192c59ddb9 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml @@ -30,10 +30,14 @@ properties: maxItems: 1 interconnects: - maxItems: 2 + items: + - description: Interconnect path from mdp0 port to the data bus + - description: Interconnect path from CPU to the reg bus interconnect-names: - maxItems: 2 + items: + - const: mdp0-mem + - const: cpu-cfg patternProperties: "^display-controller@[0-9a-f]+$": @@ -91,9 +95,9 @@ examples: reg = <0x0ae00000 0x1000>; reg-names = "mdss"; - interconnects = <&mmss_noc MASTER_MDP 0 &gem_noc SLAVE_LLCC 0>, - <&mc_virt MASTER_LLCC 0 &mc_virt SLAVE_EBI1 0>; - interconnect-names = "mdp0-mem", "mdp1-mem"; + interconnects = <&mmss_noc MASTER_MDP 0 &mc_virt SLAVE_EBI1 0>, + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_DISPLAY_CFG 0>; + interconnect-names = "mdp0-mem", "cpu-cfg"; resets = <&dispcc DISP_CC_MDSS_CORE_BCR>; -- 2.51.0 From 162c57b8e7a1089d7db5a9ee8c1bff73edec1695 Mon Sep 17 00:00:00 2001 From: Neil Armstrong Date: Thu, 13 Feb 2025 17:27:57 +0100 Subject: [PATCH 09/16] 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. Suggested-by: Dmitry Baryshkov Reviewed-by: Krzysztof Kozlowski Signed-off-by: Neil Armstrong Patchwork: https://patchwork.freedesktop.org/patch/637051/ Link: https://lore.kernel.org/r/20250213-topic-sm8x50-mdss-interconnect-bindings-fix-v4-2-3fa0bc42dd38@linaro.org Signed-off-by: Dmitry Baryshkov --- .../bindings/display/msm/qcom,sm8650-mdss.yaml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm8650-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sm8650-mdss.yaml index 24cece1e888b..a1c53e191033 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,sm8650-mdss.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8650-mdss.yaml @@ -29,10 +29,14 @@ properties: maxItems: 1 interconnects: - maxItems: 2 + items: + - description: Interconnect path from mdp0 port to the data bus + - description: Interconnect path from CPU to the reg bus interconnect-names: - maxItems: 2 + items: + - const: mdp0-mem + - const: cpu-cfg patternProperties: "^display-controller@[0-9a-f]+$": @@ -75,12 +79,17 @@ examples: #include #include #include + #include display-subsystem@ae00000 { compatible = "qcom,sm8650-mdss"; reg = <0x0ae00000 0x1000>; reg-names = "mdss"; + interconnects = <&mmss_noc MASTER_MDP 0 &mc_virt SLAVE_EBI1 0>, + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_DISPLAY_CFG 0>; + interconnect-names = "mdp0-mem", "cpu-cfg"; + resets = <&dispcc_core_bcr>; power-domains = <&dispcc_gdsc>; -- 2.51.0 From 709cc0620107bd87e48e7d697f97ccc00c98c47d Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Fri, 14 Feb 2025 14:17:44 +0100 Subject: [PATCH 10/16] 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. Reviewed-by: Abhinav Kumar Signed-off-by: Krzysztof Kozlowski Reviewed-by: Dmitry Baryshkov Patchwork: https://patchwork.freedesktop.org/patch/637303/ Link: https://lore.kernel.org/r/20250214-drm-msm-cleanups-v2-1-1bec50f37dc1@linaro.org Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dsi/dsi_host.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 007311c21fda..397c9f1f5885 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -292,7 +292,6 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host) ret = PTR_ERR(msm_host->byte_clk); pr_err("%s: can't find dsi_byte clock. ret=%d\n", __func__, ret); - msm_host->byte_clk = NULL; goto exit; } @@ -301,7 +300,6 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host) ret = PTR_ERR(msm_host->pixel_clk); pr_err("%s: can't find dsi_pixel clock. ret=%d\n", __func__, ret); - msm_host->pixel_clk = NULL; goto exit; } @@ -310,7 +308,6 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host) ret = PTR_ERR(msm_host->esc_clk); pr_err("%s: can't find dsi_esc clock. ret=%d\n", __func__, ret); - msm_host->esc_clk = NULL; goto exit; } -- 2.51.0 From d5bc3c3389d7850a3207f2a638966db6ecd30a5e Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Fri, 14 Feb 2025 14:17:45 +0100 Subject: [PATCH 11/16] 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. Reviewed-by: Abhinav Kumar Signed-off-by: Krzysztof Kozlowski Reviewed-by: Dmitry Baryshkov Patchwork: https://patchwork.freedesktop.org/patch/637306/ Link: https://lore.kernel.org/r/20250214-drm-msm-cleanups-v2-2-1bec50f37dc1@linaro.org Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dsi/dsi_host.c | 100 ++++++++++++----------------- 1 file changed, 41 insertions(+), 59 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 397c9f1f5885..8fc9f5486aeb 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -200,7 +200,8 @@ static const struct msm_dsi_cfg_handler *dsi_get_config( ahb_clk = msm_clk_get(msm_host->pdev, "iface"); if (IS_ERR(ahb_clk)) { - pr_err("%s: cannot get interface clock\n", __func__); + dev_err_probe(dev, PTR_ERR(ahb_clk), "%s: cannot get interface clock\n", + __func__); goto exit; } @@ -208,13 +209,13 @@ static const struct msm_dsi_cfg_handler *dsi_get_config( ret = clk_prepare_enable(ahb_clk); if (ret) { - pr_err("%s: unable to enable ahb_clk\n", __func__); + dev_err_probe(dev, ret, "%s: unable to enable ahb_clk\n", __func__); goto runtime_put; } ret = dsi_get_version(msm_host->ctrl_base, &major, &minor); if (ret) { - pr_err("%s: Invalid version\n", __func__); + dev_err_probe(dev, ret, "%s: Invalid version\n", __func__); goto disable_clks; } @@ -281,39 +282,31 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host) msm_host->num_bus_clks = cfg->num_bus_clks; ret = devm_clk_bulk_get(&pdev->dev, msm_host->num_bus_clks, msm_host->bus_clks); - if (ret < 0) { - dev_err(&pdev->dev, "Unable to get clocks, ret = %d\n", ret); - goto exit; - } + if (ret < 0) + return dev_err_probe(&pdev->dev, ret, "Unable to get clocks\n"); /* get link and source clocks */ msm_host->byte_clk = msm_clk_get(pdev, "byte"); - if (IS_ERR(msm_host->byte_clk)) { - ret = PTR_ERR(msm_host->byte_clk); - pr_err("%s: can't find dsi_byte clock. ret=%d\n", - __func__, ret); - goto exit; - } + if (IS_ERR(msm_host->byte_clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(msm_host->byte_clk), + "%s: can't find dsi_byte clock\n", + __func__); msm_host->pixel_clk = msm_clk_get(pdev, "pixel"); - if (IS_ERR(msm_host->pixel_clk)) { - ret = PTR_ERR(msm_host->pixel_clk); - pr_err("%s: can't find dsi_pixel clock. ret=%d\n", - __func__, ret); - goto exit; - } + if (IS_ERR(msm_host->pixel_clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(msm_host->pixel_clk), + "%s: can't find dsi_pixel clock\n", + __func__); msm_host->esc_clk = msm_clk_get(pdev, "core"); - if (IS_ERR(msm_host->esc_clk)) { - ret = PTR_ERR(msm_host->esc_clk); - pr_err("%s: can't find dsi_esc clock. ret=%d\n", - __func__, ret); - goto exit; - } + if (IS_ERR(msm_host->esc_clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(msm_host->esc_clk), + "%s: can't find dsi_esc clock\n", + __func__); if (cfg_hnd->ops->clk_init_ver) ret = cfg_hnd->ops->clk_init_ver(msm_host); -exit: + return ret; } @@ -1879,31 +1872,28 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) msm_dsi->host = &msm_host->base; ret = dsi_host_parse_dt(msm_host); - if (ret) { - pr_err("%s: failed to parse dt\n", __func__); - return ret; - } + if (ret) + return dev_err_probe(&pdev->dev, ret, "%s: failed to parse dt\n", + __func__); msm_host->ctrl_base = msm_ioremap_size(pdev, "dsi_ctrl", &msm_host->ctrl_size); - if (IS_ERR(msm_host->ctrl_base)) { - pr_err("%s: unable to map Dsi ctrl base\n", __func__); - return PTR_ERR(msm_host->ctrl_base); - } + if (IS_ERR(msm_host->ctrl_base)) + return dev_err_probe(&pdev->dev, PTR_ERR(msm_host->ctrl_base), + "%s: unable to map Dsi ctrl base\n", __func__); pm_runtime_enable(&pdev->dev); msm_host->cfg_hnd = dsi_get_config(msm_host); - if (!msm_host->cfg_hnd) { - pr_err("%s: get config failed\n", __func__); - return -EINVAL; - } + if (!msm_host->cfg_hnd) + return dev_err_probe(&pdev->dev, -EINVAL, + "%s: get config failed\n", __func__); cfg = msm_host->cfg_hnd->cfg; msm_host->id = dsi_host_get_id(msm_host); - if (msm_host->id < 0) { - pr_err("%s: unable to identify DSI host index\n", __func__); - return msm_host->id; - } + if (msm_host->id < 0) + return dev_err_probe(&pdev->dev, msm_host->id, + "%s: unable to identify DSI host index\n", + __func__); /* fixup base address by io offset */ msm_host->ctrl_base += cfg->io_offset; @@ -1915,10 +1905,8 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) return ret; ret = dsi_clk_init(msm_host); - if (ret) { - pr_err("%s: unable to initialize dsi clks\n", __func__); - return ret; - } + if (ret) + return dev_err_probe(&pdev->dev, ret, "%s: unable to initialize dsi clks\n", __func__); msm_host->rx_buf = devm_kzalloc(&pdev->dev, SZ_4K, GFP_KERNEL); if (!msm_host->rx_buf) { @@ -1931,26 +1919,20 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) return ret; /* OPP table is optional */ ret = devm_pm_opp_of_add_table(&pdev->dev); - if (ret && ret != -ENODEV) { - dev_err(&pdev->dev, "invalid OPP table in device tree\n"); - return ret; - } + if (ret && ret != -ENODEV) + return dev_err_probe(&pdev->dev, ret, "invalid OPP table in device tree\n"); msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0); - if (!msm_host->irq) { - dev_err(&pdev->dev, "failed to get irq\n"); - return -EINVAL; - } + if (!msm_host->irq) + return dev_err_probe(&pdev->dev, -EINVAL, "failed to get irq\n"); /* do not autoenable, will be enabled later */ ret = devm_request_irq(&pdev->dev, msm_host->irq, dsi_host_irq, IRQF_TRIGGER_HIGH | IRQF_NO_AUTOEN, "dsi_isr", msm_host); - if (ret < 0) { - dev_err(&pdev->dev, "failed to request IRQ%u: %d\n", - msm_host->irq, ret); - return ret; - } + if (ret < 0) + return dev_err_probe(&pdev->dev, ret, "failed to request IRQ%u\n", + msm_host->irq); init_completion(&msm_host->dma_comp); init_completion(&msm_host->video_comp); -- 2.51.0 From cce156257ed3414ab104dc2360c5c4eb03ce7ed3 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Fri, 14 Feb 2025 14:17:46 +0100 Subject: [PATCH 12/16] 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. Reviewed-by: Abhinav Kumar Signed-off-by: Krzysztof Kozlowski Reviewed-by: Dmitry Baryshkov Patchwork: https://patchwork.freedesktop.org/patch/637305/ Link: https://lore.kernel.org/r/20250214-drm-msm-cleanups-v2-3-1bec50f37dc1@linaro.org Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dsi/dsi_host.c | 51 +++++++++++++++--------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 8fc9f5486aeb..051e26ae1b7f 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -179,18 +179,18 @@ struct msm_dsi_host { int irq; }; - static inline u32 dsi_read(struct msm_dsi_host *msm_host, u32 reg) { return readl(msm_host->ctrl_base + reg); } + static inline void dsi_write(struct msm_dsi_host *msm_host, u32 reg, u32 data) { writel(data, msm_host->ctrl_base + reg); } -static const struct msm_dsi_cfg_handler *dsi_get_config( - struct msm_dsi_host *msm_host) +static const struct msm_dsi_cfg_handler * +dsi_get_config(struct msm_dsi_host *msm_host) { const struct msm_dsi_cfg_handler *cfg_hnd = NULL; struct device *dev = &msm_host->pdev->dev; @@ -370,7 +370,6 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host) return 0; } - int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host) { int ret; @@ -588,7 +587,6 @@ static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool is_bonded_dsi) DBG("pclk=%lu, bclk=%lu", msm_host->pixel_clk_rate, msm_host->byte_clk_rate); - } int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool is_bonded_dsi) @@ -677,8 +675,8 @@ static inline enum dsi_traffic_mode dsi_get_traffic_mode(const u32 mode_flags) return NON_BURST_SYNCH_EVENT; } -static inline enum dsi_vid_dst_format dsi_get_vid_fmt( - const enum mipi_dsi_pixel_format mipi_fmt) +static inline enum dsi_vid_dst_format +dsi_get_vid_fmt(const enum mipi_dsi_pixel_format mipi_fmt) { switch (mipi_fmt) { case MIPI_DSI_FMT_RGB888: return VID_DST_FORMAT_RGB888; @@ -689,8 +687,8 @@ static inline enum dsi_vid_dst_format dsi_get_vid_fmt( } } -static inline enum dsi_cmd_dst_format dsi_get_cmd_fmt( - const enum mipi_dsi_pixel_format mipi_fmt) +static inline enum dsi_cmd_dst_format +dsi_get_cmd_fmt(const enum mipi_dsi_pixel_format mipi_fmt) { switch (mipi_fmt) { case MIPI_DSI_FMT_RGB888: return CMD_DST_FORMAT_RGB888; @@ -1282,14 +1280,15 @@ static int dsi_cmd_dma_add(struct msm_dsi_host *msm_host, static int dsi_short_read1_resp(u8 *buf, const struct mipi_dsi_msg *msg) { u8 *data = msg->rx_buf; + if (data && (msg->rx_len >= 1)) { *data = buf[1]; /* strip out dcs type */ return 1; - } else { - pr_err("%s: read data does not match with rx_buf len %zu\n", - __func__, msg->rx_len); - return -EINVAL; } + + pr_err("%s: read data does not match with rx_buf len %zu\n", + __func__, msg->rx_len); + return -EINVAL; } /* @@ -1298,15 +1297,16 @@ static int dsi_short_read1_resp(u8 *buf, const struct mipi_dsi_msg *msg) static int dsi_short_read2_resp(u8 *buf, const struct mipi_dsi_msg *msg) { u8 *data = msg->rx_buf; + if (data && (msg->rx_len >= 2)) { data[0] = buf[1]; /* strip out dcs type */ data[1] = buf[2]; return 2; - } else { - pr_err("%s: read data does not match with rx_buf len %zu\n", - __func__, msg->rx_len); - return -EINVAL; } + + pr_err("%s: read data does not match with rx_buf len %zu\n", + __func__, msg->rx_len); + return -EINVAL; } static int dsi_long_read_resp(u8 *buf, const struct mipi_dsi_msg *msg) @@ -1366,8 +1366,9 @@ static int dsi_cmd_dma_tx(struct msm_dsi_host *msm_host, int len) ret = -ETIMEDOUT; else ret = len; - } else + } else { ret = len; + } return ret; } @@ -1435,11 +1436,12 @@ static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host, return len; } - /* for video mode, do not send cmds more than - * one pixel line, since it only transmit it - * during BLLP. - */ - /* TODO: if the command is sent in LP mode, the bit rate is only + /* + * for video mode, do not send cmds more than + * one pixel line, since it only transmit it + * during BLLP. + * + * TODO: if the command is sent in LP mode, the bit rate is only * half of esc clk rate. In this case, if the video is already * actively streaming, we need to check more carefully if the * command can be fit into one BLLP. @@ -1864,9 +1866,8 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) int ret; msm_host = devm_kzalloc(&pdev->dev, sizeof(*msm_host), GFP_KERNEL); - if (!msm_host) { + if (!msm_host) return -ENOMEM; - } msm_host->pdev = pdev; msm_dsi->host = &msm_host->base; -- 2.51.0 From b39e7014ed3121f980d55097ec2bd9ecee3adc83 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Fri, 14 Feb 2025 14:17:47 +0100 Subject: [PATCH 13/16] drm/msm/dsi: Drop unnecessary -ENOMEM message Kernel core already prints detailed report about memory allocation failures, so drivers should not have their own error messages. Reviewed-by: Abhinav Kumar Signed-off-by: Krzysztof Kozlowski Reviewed-by: Dmitry Baryshkov Patchwork: https://patchwork.freedesktop.org/patch/637308/ Link: https://lore.kernel.org/r/20250214-drm-msm-cleanups-v2-4-1bec50f37dc1@linaro.org Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dsi/dsi_host.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 051e26ae1b7f..2218d4f0c513 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1910,10 +1910,8 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) return dev_err_probe(&pdev->dev, ret, "%s: unable to initialize dsi clks\n", __func__); msm_host->rx_buf = devm_kzalloc(&pdev->dev, SZ_4K, GFP_KERNEL); - if (!msm_host->rx_buf) { - pr_err("%s: alloc rx temp buf failed\n", __func__); + if (!msm_host->rx_buf) return -ENOMEM; - } ret = devm_pm_opp_set_clkname(&pdev->dev, "byte"); if (ret) -- 2.51.0 From 14ad809ceb66d0874cbe4bd5ca9edf0de8d9ad96 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Mon, 17 Feb 2025 12:17:41 +0100 Subject: [PATCH 14/16] 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. Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration") Reviewed-by: Dmitry Baryshkov Reviewed-by: Jessica Zhang Signed-off-by: Marijn Suijten Patchwork: https://patchwork.freedesktop.org/patch/637648/ Link: https://lore.kernel.org/r/20250217-drm-msm-initial-dualpipe-dsc-fixes-v3-1-913100d6103f@somainline.org Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dsi/dsi_host.c | 8 ++++---- drivers/gpu/drm/msm/msm_dsc_helper.h | 11 ----------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 2218d4f0c513..b6ae9717a5d3 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -834,7 +834,7 @@ static void dsi_ctrl_enable(struct msm_dsi_host *msm_host, dsi_write(msm_host, REG_DSI_CPHY_MODE_CTRL, BIT(0)); } -static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mode, u32 hdisplay) +static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mode) { struct drm_dsc_config *dsc = msm_host->dsc; u32 reg, reg_ctrl, reg_ctrl2; @@ -846,7 +846,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod /* first calculate dsc parameters and then program * compress mode registers */ - slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay); + slice_per_intf = dsc->slice_count; total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf; bytes_per_pkt = dsc->slice_chunk_size; /* * slice_per_pkt; */ @@ -979,7 +979,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) if (msm_host->mode_flags & MIPI_DSI_MODE_VIDEO) { if (msm_host->dsc) - dsi_update_dsc_timing(msm_host, false, mode->hdisplay); + dsi_update_dsc_timing(msm_host, false); dsi_write(msm_host, REG_DSI_ACTIVE_H, DSI_ACTIVE_H_START(ha_start) | @@ -1000,7 +1000,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi) DSI_ACTIVE_VSYNC_VPOS_END(vs_end)); } else { /* command mode */ if (msm_host->dsc) - dsi_update_dsc_timing(msm_host, true, mode->hdisplay); + dsi_update_dsc_timing(msm_host, true); /* image data and 1 byte write_memory_start cmd */ if (!msm_host->dsc) diff --git a/drivers/gpu/drm/msm/msm_dsc_helper.h b/drivers/gpu/drm/msm/msm_dsc_helper.h index b9049fe1e279..63f95523b2cb 100644 --- a/drivers/gpu/drm/msm/msm_dsc_helper.h +++ b/drivers/gpu/drm/msm/msm_dsc_helper.h @@ -12,17 +12,6 @@ #include #include -/** - * msm_dsc_get_slices_per_intf() - calculate number of slices per interface - * @dsc: Pointer to drm dsc config struct - * @intf_width: interface width in pixels - * Returns: Integer representing the number of slices for the given interface - */ -static inline u32 msm_dsc_get_slices_per_intf(const struct drm_dsc_config *dsc, u32 intf_width) -{ - return DIV_ROUND_UP(intf_width, dsc->slice_width); -} - /** * msm_dsc_get_bytes_per_line() - calculate bytes per line * @dsc: Pointer to drm dsc config struct -- 2.51.0 From 660c396c98c061f9696bebacc178b74072e80054 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Mon, 17 Feb 2025 12:17:42 +0100 Subject: [PATCH 15/16] 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 Reviewed-by: Dmitry Baryshkov Signed-off-by: Marijn Suijten 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 --- drivers/gpu/drm/msm/dsi/dsi_manager.c | 32 ++++++++++++++++++--------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c index a210b7c9e5ca..4fabb01345aa 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c @@ -74,17 +74,35 @@ static int dsi_mgr_setup_components(int id) int ret; if (!IS_BONDED_DSI()) { + /* + * Set the usecase before calling msm_dsi_host_register(), which would + * already program the PLL source mux based on a default usecase. + */ + msm_dsi_phy_set_usecase(msm_dsi->phy, MSM_DSI_PHY_STANDALONE); + msm_dsi_host_set_phy_mode(msm_dsi->host, msm_dsi->phy); + ret = msm_dsi_host_register(msm_dsi->host); if (ret) return ret; - - msm_dsi_phy_set_usecase(msm_dsi->phy, MSM_DSI_PHY_STANDALONE); - msm_dsi_host_set_phy_mode(msm_dsi->host, msm_dsi->phy); } else if (other_dsi) { struct msm_dsi *master_link_dsi = IS_MASTER_DSI_LINK(id) ? msm_dsi : other_dsi; struct msm_dsi *slave_link_dsi = IS_MASTER_DSI_LINK(id) ? other_dsi : msm_dsi; + + /* + * PLL0 is to drive both DSI link clocks in bonded DSI mode. + * + * Set the usecase before calling msm_dsi_host_register(), which would + * already program the PLL source mux based on a default usecase. + */ + msm_dsi_phy_set_usecase(clk_master_dsi->phy, + MSM_DSI_PHY_MASTER); + msm_dsi_phy_set_usecase(clk_slave_dsi->phy, + MSM_DSI_PHY_SLAVE); + msm_dsi_host_set_phy_mode(msm_dsi->host, msm_dsi->phy); + msm_dsi_host_set_phy_mode(other_dsi->host, other_dsi->phy); + /* Register slave host first, so that slave DSI device * has a chance to probe, and do not block the master * DSI device's probe. @@ -98,14 +116,6 @@ static int dsi_mgr_setup_components(int id) ret = msm_dsi_host_register(master_link_dsi->host); if (ret) return ret; - - /* PLL0 is to drive both 2 DSI link clocks in bonded DSI mode. */ - msm_dsi_phy_set_usecase(clk_master_dsi->phy, - MSM_DSI_PHY_MASTER); - msm_dsi_phy_set_usecase(clk_slave_dsi->phy, - MSM_DSI_PHY_SLAVE); - msm_dsi_host_set_phy_mode(msm_dsi->host, msm_dsi->phy); - msm_dsi_host_set_phy_mode(other_dsi->host, other_dsi->phy); } return 0; -- 2.51.0 From d245ce568929e30f650e260631f7ad14970d7c2c Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Mon, 17 Feb 2025 12:17:43 +0100 Subject: [PATCH 16/16] 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 Reviewed-by: Jessica Zhang Signed-off-by: Marijn Suijten 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 --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 938d8b05a4cd..32992e952553 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -689,20 +689,21 @@ static struct msm_display_topology dpu_encoder_get_topology( if (dsc) { /* - * Use 2 DSC encoders and 2 layer mixers per single interface + * Use 2 DSC encoders, 2 layer mixers and 1 or 2 interfaces * when Display Stream Compression (DSC) is enabled, * and when enough DSC blocks are available. * This is power-optimal and can drive up to (including) 4k * screens. */ - if (dpu_kms->catalog->dsc_count >= 2) { + WARN(topology.num_intf > 2, + "DSC topology cannot support more than 2 interfaces\n"); + if (intf_count >= 2 || dpu_kms->catalog->dsc_count >= 2) { topology.num_dsc = 2; topology.num_lm = 2; } else { topology.num_dsc = 1; topology.num_lm = 1; } - topology.num_intf = 1; } /* -- 2.51.0