* [PATCH v2 0/5] drm/msm/dsi: Add DSC support to 2 panels in dual DSI mode
@ 2025-02-20 10:07 Jun Nie
2025-02-20 10:07 ` [PATCH v2 1/5] drm/msm/dsi: add support VBIF_CTRL_PRIORITY to v2.8.0 controller Jun Nie
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Jun Nie @ 2025-02-20 10:07 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Jessica Zhang, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree,
Jun Nie, Jonathan Marek
The 2 DSI interfaces may be connected to 2 independent panels in dual-DSI
mode. Device tree binging is added and frame width for DSC is changed to
support the usage case. Support to multiple slice per packet is added for
the device setup to test the usage case.
This patch set is split from the quad-pipe patch set v1. It is also dependent
on Marijn's patch:
https://lore.kernel.org/all/20250209-drm-msm-initial-dualpipe-dsc-fixes-v2-1-9a60184fdc36@somainline.org/
The change vs v1:
- Add device tree binding for dual panel case in handling frame width for
DSC to avoid breaking existing dual-DSI case.
- Leverage Marijn's patch to configure proper slice per interface in
dsi_update_dsc_timing().
- Polish commit comments.
- Link to v1: https://lore.kernel.org/all/20240829-sm8650-v6-11-hmd-pocf-mdss-quad-upstream-8-v1-0-bdb05b4b5a2e@linaro.org/
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
Jun Nie (5):
drm/msm/dsi: add support VBIF_CTRL_PRIORITY to v2.8.0 controller
drm/msm/dsi: check DSC width for the bonded DSI case
drm/msm/dsi: support DSC configurations with slice_per_pkt > 1
dt-bindings: display/msm: dsi-controller-main: Document dual panel property
drm/msm/dsi: Support DSC for dual panel case
.../bindings/display/msm/dsi-controller-main.yaml | 8 +++-
drivers/gpu/drm/msm/dsi/dsi.h | 6 ++-
drivers/gpu/drm/msm/dsi/dsi_host.c | 54 ++++++++++++++--------
drivers/gpu/drm/msm/dsi/dsi_manager.c | 12 +++--
include/drm/drm_mipi_dsi.h | 2 +
5 files changed, 56 insertions(+), 26 deletions(-)
---
base-commit: 53d2d43787aa9a7daf91d2421033528c2e186be8
change-id: 20250220-dual-dsi-c5660c22a661
Best regards,
--
Jun Nie <jun.nie@linaro.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/5] drm/msm/dsi: add support VBIF_CTRL_PRIORITY to v2.8.0 controller
2025-02-20 10:07 [PATCH v2 0/5] drm/msm/dsi: Add DSC support to 2 panels in dual DSI mode Jun Nie
@ 2025-02-20 10:07 ` Jun Nie
2025-02-20 10:27 ` Dmitry Baryshkov
2025-02-20 10:07 ` [PATCH v2 2/5] drm/msm/dsi: check DSC width for the bonded DSI case Jun Nie
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Jun Nie @ 2025-02-20 10:07 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Jessica Zhang, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree,
Jun Nie, Jonathan Marek
This change originates from the Qualcomm Android Linux driver. It is
essential to support a dual-DSI configuration with two panels in
some circumstances per testing. As the name suggests, this modification
may enhance the bandwidth robustness of a bus.
Co-developed-by: Jonathan Marek <jonathan@marek.ca>
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 42e100a8adca09d7b55afce0e2553e76d898744f..f59c4cd6bc8cdb31c1302f8e3ff395486c0b4898 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -2238,13 +2238,23 @@ int msm_dsi_host_cmd_rx(struct mipi_dsi_host *host,
return ret;
}
+#define DSI_VBIF_CTRL (0x01CC - 4)
+#define DSI_VBIF_CTRL_PRIORITY 0x07
+
void msm_dsi_host_cmd_xfer_commit(struct mipi_dsi_host *host, u32 dma_base,
u32 len)
{
struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
+ const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
+ u32 reg;
dsi_write(msm_host, REG_DSI_DMA_BASE, dma_base);
dsi_write(msm_host, REG_DSI_DMA_LEN, len);
+ if (cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V2_8_0) {
+ reg = dsi_read(msm_host, DSI_VBIF_CTRL);
+ reg |= (DSI_VBIF_CTRL_PRIORITY & 0x7);
+ dsi_write(msm_host, DSI_VBIF_CTRL, reg);
+ }
dsi_write(msm_host, REG_DSI_TRIG_DMA, 1);
/* Make sure trigger happens */
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/5] drm/msm/dsi: check DSC width for the bonded DSI case
2025-02-20 10:07 [PATCH v2 0/5] drm/msm/dsi: Add DSC support to 2 panels in dual DSI mode Jun Nie
2025-02-20 10:07 ` [PATCH v2 1/5] drm/msm/dsi: add support VBIF_CTRL_PRIORITY to v2.8.0 controller Jun Nie
@ 2025-02-20 10:07 ` Jun Nie
2025-02-20 10:07 ` [PATCH v2 3/5] drm/msm/dsi: support DSC configurations with slice_per_pkt > 1 Jun Nie
` (2 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Jun Nie @ 2025-02-20 10:07 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Jessica Zhang, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree,
Jun Nie, Jonathan Marek
The frame width is validated to ensure it is a multiple of the DSC slice.
In the case of bonded DSI, the frame is divided horizontally in half for
compression and is delivered through two DSI interfaces. Therefore, the
width for each DSI interface should also be a multiple of the slice.
Currently, the implementation only validates this requirement against the
entire frame width. Use half of the frame width for validation.
Co-developed-by: Jonathan Marek <jonathan@marek.ca>
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
drivers/gpu/drm/msm/dsi/dsi.h | 3 ++-
drivers/gpu/drm/msm/dsi/dsi_host.c | 6 +++++-
drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 87496db203d6c7582eadcb74e94eb56a219df292..35b90c462f637111159b204269ce908614a21586 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -79,7 +79,8 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host);
int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
const struct drm_display_mode *mode);
enum drm_mode_status msm_dsi_host_check_dsc(struct mipi_dsi_host *host,
- const struct drm_display_mode *mode);
+ const struct drm_display_mode *mode,
+ bool is_bonded_dsi);
unsigned long msm_dsi_host_get_mode_flags(struct mipi_dsi_host *host);
int msm_dsi_host_register(struct mipi_dsi_host *host);
void msm_dsi_host_unregister(struct mipi_dsi_host *host);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index f59c4cd6bc8cdb31c1302f8e3ff395486c0b4898..908f5f1649d650f1cf152fc0b263541dc566ac68 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -2489,7 +2489,8 @@ int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
}
enum drm_mode_status msm_dsi_host_check_dsc(struct mipi_dsi_host *host,
- const struct drm_display_mode *mode)
+ const struct drm_display_mode *mode,
+ bool is_bonded_dsi)
{
struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
struct drm_dsc_config *dsc = msm_host->dsc;
@@ -2499,6 +2500,9 @@ enum drm_mode_status msm_dsi_host_check_dsc(struct mipi_dsi_host *host,
if (!msm_host->dsc)
return MODE_OK;
+ if (is_bonded_dsi)
+ pic_width = mode->hdisplay / 2;
+
if (pic_width % dsc->slice_width) {
pr_err("DSI: pic_width %d has to be multiple of slice %d\n",
pic_width, dsc->slice_width);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index b93205c034e4acc73d536deeddce6ebd694b4a80..be13bf682a9601484c9c14e8419563f37c2281ee 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -428,7 +428,7 @@ static enum drm_mode_status dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,
return MODE_ERROR;
}
- return msm_dsi_host_check_dsc(host, mode);
+ return msm_dsi_host_check_dsc(host, mode, IS_BONDED_DSI());
}
static int dsi_mgr_bridge_attach(struct drm_bridge *bridge,
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 3/5] drm/msm/dsi: support DSC configurations with slice_per_pkt > 1
2025-02-20 10:07 [PATCH v2 0/5] drm/msm/dsi: Add DSC support to 2 panels in dual DSI mode Jun Nie
2025-02-20 10:07 ` [PATCH v2 1/5] drm/msm/dsi: add support VBIF_CTRL_PRIORITY to v2.8.0 controller Jun Nie
2025-02-20 10:07 ` [PATCH v2 2/5] drm/msm/dsi: check DSC width for the bonded DSI case Jun Nie
@ 2025-02-20 10:07 ` Jun Nie
2025-02-20 10:07 ` [PATCH v2 4/5] dt-bindings: display/msm: dsi-controller-main: Document dual panel property Jun Nie
2025-02-20 10:07 ` [PATCH v2 5/5] drm/msm/dsi: Support DSC for dual panel case Jun Nie
4 siblings, 0 replies; 17+ messages in thread
From: Jun Nie @ 2025-02-20 10:07 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Jessica Zhang, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree,
Jun Nie, Jonathan Marek
Some panels support multiple slice to be sent in a single DSC packet. And
this feature is a must for specific panels, such as JDI LPM026M648C. Add a
dsc_slice_per_pkt member into struct mipi_dsi_device and support the
feature in msm mdss driver.
Co-developed-by: Jonathan Marek <jonathan@marek.ca>
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 25 ++++++++++---------------
include/drm/drm_mipi_dsi.h | 2 ++
2 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 908f5f1649d650f1cf152fc0b263541dc566ac68..976c5d82a2efa0fc51657b8534675890be7c33a6 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -157,6 +157,7 @@ struct msm_dsi_host {
struct drm_display_mode *mode;
struct drm_dsc_config *dsc;
+ unsigned int dsc_slice_per_pkt;
/* connected device info */
unsigned int channel;
@@ -861,17 +862,10 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
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; */
+ bytes_per_pkt = dsc->slice_chunk_size * msm_host->dsc_slice_per_pkt;
eol_byte_num = total_bytes_per_intf % 3;
-
- /*
- * Typically, pkt_per_line = slice_per_intf * slice_per_pkt.
- *
- * Since the current driver only supports slice_per_pkt = 1,
- * pkt_per_line will be equal to slice per intf for now.
- */
- pkt_per_line = slice_per_intf;
+ pkt_per_line = slice_per_intf / msm_host->dsc_slice_per_pkt;
if (is_cmd_mode) /* packet data type */
reg = DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(MIPI_DSI_DCS_LONG_WRITE);
@@ -1020,12 +1014,8 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
else
/*
* When DSC is enabled, WC = slice_chunk_size * slice_per_pkt + 1.
- * Currently, the driver only supports default value of slice_per_pkt = 1
- *
- * TODO: Expand mipi_dsi_device struct to hold slice_per_pkt info
- * and adjust DSC math to account for slice_per_pkt.
*/
- wc = msm_host->dsc->slice_chunk_size + 1;
+ wc = msm_host->dsc->slice_chunk_size * msm_host->dsc_slice_per_pkt + 1;
dsi_write(msm_host, REG_DSI_CMD_MDP_STREAM0_CTRL,
DSI_CMD_MDP_STREAM0_CTRL_WORD_COUNT(wc) |
@@ -1630,8 +1620,13 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
msm_host->lanes = dsi->lanes;
msm_host->format = dsi->format;
msm_host->mode_flags = dsi->mode_flags;
- if (dsi->dsc)
+ if (dsi->dsc) {
msm_host->dsc = dsi->dsc;
+ msm_host->dsc_slice_per_pkt = dsi->dsc_slice_per_pkt;
+ /* for backwards compatibility, assume 1 if not set */
+ if (!msm_host->dsc_slice_per_pkt)
+ msm_host->dsc_slice_per_pkt = 1;
+ }
ret = dsi_dev_attach(msm_host->pdev);
if (ret)
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 94400a78031f1b5f515c4a1519f604c0df7f3e0c..f13ed73b54b0af6d93456f4b8c8257c4f8419023 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -182,6 +182,7 @@ struct mipi_dsi_device_info {
* be set to the real limits of the hardware, zero is only accepted for
* legacy drivers
* @dsc: panel/bridge DSC pps payload to be sent
+ * @dsc_slice_per_pkt: number of DSC slices to be sent as in a single packet
*/
struct mipi_dsi_device {
struct mipi_dsi_host *host;
@@ -196,6 +197,7 @@ struct mipi_dsi_device {
unsigned long hs_rate;
unsigned long lp_rate;
struct drm_dsc_config *dsc;
+ unsigned int dsc_slice_per_pkt;
};
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 4/5] dt-bindings: display/msm: dsi-controller-main: Document dual panel property
2025-02-20 10:07 [PATCH v2 0/5] drm/msm/dsi: Add DSC support to 2 panels in dual DSI mode Jun Nie
` (2 preceding siblings ...)
2025-02-20 10:07 ` [PATCH v2 3/5] drm/msm/dsi: support DSC configurations with slice_per_pkt > 1 Jun Nie
@ 2025-02-20 10:07 ` Jun Nie
2025-02-20 10:33 ` Dmitry Baryshkov
2025-02-20 10:07 ` [PATCH v2 5/5] drm/msm/dsi: Support DSC for dual panel case Jun Nie
4 siblings, 1 reply; 17+ messages in thread
From: Jun Nie @ 2025-02-20 10:07 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Jessica Zhang, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree,
Jun Nie
The DSI interface can be connected to a panel that has a dual DSI channel,
or to two separate panels, each equipped with a single DSI channel. To
prevent the DSC configuration for the dual panel setup from disrupting the
current configuration of a single panel with a dual DSI channel, add a dual
panel property to support the use of two panels.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
.../devicetree/bindings/display/msm/dsi-controller-main.yaml | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index ffbd1dc9470e2091b477b0c88392d81802119f48..e3f2eabde27609a66d6d81fafcb14e1bc014613c 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -88,9 +88,15 @@ properties:
qcom,dual-dsi-mode:
type: boolean
description: |
- Indicates if the DSI controller is driving a panel which needs
+ Indicates if the DSI controller is driving display device which needs
2 DSI links.
+ qcom,dual-panel:
+ type: boolean
+ description: |
+ Indicates if the DSI controller is driving display device that composed
+ with 2 independent panels and needs 2 DSI links.
+
qcom,master-dsi:
type: boolean
description: |
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 5/5] drm/msm/dsi: Support DSC for dual panel case
2025-02-20 10:07 [PATCH v2 0/5] drm/msm/dsi: Add DSC support to 2 panels in dual DSI mode Jun Nie
` (3 preceding siblings ...)
2025-02-20 10:07 ` [PATCH v2 4/5] dt-bindings: display/msm: dsi-controller-main: Document dual panel property Jun Nie
@ 2025-02-20 10:07 ` Jun Nie
2025-02-20 10:38 ` Dmitry Baryshkov
4 siblings, 1 reply; 17+ messages in thread
From: Jun Nie @ 2025-02-20 10:07 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Jessica Zhang, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree,
Jun Nie
There is dual DSI case that every DSI link is connected to an independent
panel. In this dual panel case, the frame width for DSC on each link should
be halved to support the usage case.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
drivers/gpu/drm/msm/dsi/dsi.h | 3 ++-
drivers/gpu/drm/msm/dsi/dsi_host.c | 13 +++++++++----
drivers/gpu/drm/msm/dsi/dsi_manager.c | 10 ++++++++--
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 35b90c462f637111159b204269ce908614a21586..5a8978bed9f4ca897b418ced60194042d9dd8d05 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -74,7 +74,8 @@ void msm_dsi_host_enable_irq(struct mipi_dsi_host *host);
void msm_dsi_host_disable_irq(struct mipi_dsi_host *host);
int msm_dsi_host_power_on(struct mipi_dsi_host *host,
struct msm_dsi_phy_shared_timings *phy_shared_timings,
- bool is_bonded_dsi, struct msm_dsi_phy *phy);
+ bool is_bonded_dsi, bool is_dual_panel,
+ struct msm_dsi_phy *phy);
int msm_dsi_host_power_off(struct mipi_dsi_host *host);
int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
const struct drm_display_mode *mode);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 976c5d82a2efa0fc51657b8534675890be7c33a6..752a97f7181c30dade0a7745492bf16649b3197b 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -902,7 +902,8 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
}
}
-static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
+static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi,
+ bool is_dual_panel)
{
struct drm_display_mode *mode = msm_host->mode;
u32 hs_start = 0, vs_start = 0; /* take sync start as 0 */
@@ -947,7 +948,10 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
return;
}
- dsc->pic_width = mode->hdisplay;
+ if (is_dual_panel)
+ dsc->pic_width = hdisplay;
+ else
+ dsc->pic_width = mode->hdisplay;
dsc->pic_height = mode->vdisplay;
DBG("Mode %dx%d\n", dsc->pic_width, dsc->pic_height);
@@ -2369,7 +2373,8 @@ static void msm_dsi_sfpb_config(struct msm_dsi_host *msm_host, bool enable)
int msm_dsi_host_power_on(struct mipi_dsi_host *host,
struct msm_dsi_phy_shared_timings *phy_shared_timings,
- bool is_bonded_dsi, struct msm_dsi_phy *phy)
+ bool is_bonded_dsi, bool is_dual_panel,
+ struct msm_dsi_phy *phy)
{
struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
@@ -2412,7 +2417,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
goto fail_disable_clk;
}
- dsi_timing_setup(msm_host, is_bonded_dsi);
+ dsi_timing_setup(msm_host, is_bonded_dsi, is_dual_panel);
dsi_sw_reset(msm_host);
dsi_ctrl_enable(msm_host, phy_shared_timings, phy);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index be13bf682a9601484c9c14e8419563f37c2281ee..158b6cc907cb39cc3b182d3088b793d322a3527c 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -24,6 +24,7 @@ struct msm_dsi_manager {
struct msm_dsi *dsi[DSI_MAX];
bool is_bonded_dsi;
+ bool is_dual_panel;
bool is_sync_needed;
int master_dsi_link_id;
};
@@ -31,6 +32,7 @@ struct msm_dsi_manager {
static struct msm_dsi_manager msm_dsim_glb;
#define IS_BONDED_DSI() (msm_dsim_glb.is_bonded_dsi)
+#define IS_DUAL_PANEL() (msm_dsim_glb.is_dual_panel)
#define IS_SYNC_NEEDED() (msm_dsim_glb.is_sync_needed)
#define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id == id)
@@ -55,6 +57,7 @@ static int dsi_mgr_parse_of(struct device_node *np, int id)
msm_dsim->is_bonded_dsi = of_property_read_bool(np, "qcom,dual-dsi-mode");
if (msm_dsim->is_bonded_dsi) {
+ msm_dsim->is_dual_panel = of_property_read_bool(np, "qcom,dual-panel");
if (of_property_read_bool(np, "qcom,master-dsi"))
msm_dsim->master_dsi_link_id = id;
if (!msm_dsim->is_sync_needed)
@@ -214,6 +217,7 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
struct mipi_dsi_host *host = msm_dsi->host;
struct msm_dsi_phy_shared_timings phy_shared_timings[DSI_MAX];
bool is_bonded_dsi = IS_BONDED_DSI();
+ bool is_dual_panel = IS_DUAL_PANEL();
int ret;
DBG("id=%d", id);
@@ -222,7 +226,8 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
if (ret)
goto phy_en_fail;
- ret = msm_dsi_host_power_on(host, &phy_shared_timings[id], is_bonded_dsi, msm_dsi->phy);
+ ret = msm_dsi_host_power_on(host, &phy_shared_timings[id],
+ is_bonded_dsi, is_dual_panel, msm_dsi->phy);
if (ret) {
pr_err("%s: power on host %d failed, %d\n", __func__, id, ret);
goto host_on_fail;
@@ -230,7 +235,8 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
if (is_bonded_dsi && msm_dsi1) {
ret = msm_dsi_host_power_on(msm_dsi1->host,
- &phy_shared_timings[DSI_1], is_bonded_dsi, msm_dsi1->phy);
+ &phy_shared_timings[DSI_1], is_bonded_dsi,
+ is_dual_panel, msm_dsi1->phy);
if (ret) {
pr_err("%s: power on host1 failed, %d\n",
__func__, ret);
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/5] drm/msm/dsi: add support VBIF_CTRL_PRIORITY to v2.8.0 controller
2025-02-20 10:07 ` [PATCH v2 1/5] drm/msm/dsi: add support VBIF_CTRL_PRIORITY to v2.8.0 controller Jun Nie
@ 2025-02-20 10:27 ` Dmitry Baryshkov
2025-02-20 15:45 ` Jun Nie
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2025-02-20 10:27 UTC (permalink / raw)
To: Jun Nie
Cc: Rob Clark, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan,
linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree,
Jonathan Marek
On Thu, Feb 20, 2025 at 06:07:52PM +0800, Jun Nie wrote:
> This change originates from the Qualcomm Android Linux driver. It is
> essential to support a dual-DSI configuration with two panels in
> some circumstances per testing. As the name suggests, this modification
> may enhance the bandwidth robustness of a bus.
Please start by describing the problem and the result of the changes.
Otherwise it reads as it "may enhance or may worsen" the robustness.
>
> Co-developed-by: Jonathan Marek <jonathan@marek.ca>
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 42e100a8adca09d7b55afce0e2553e76d898744f..f59c4cd6bc8cdb31c1302f8e3ff395486c0b4898 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -2238,13 +2238,23 @@ int msm_dsi_host_cmd_rx(struct mipi_dsi_host *host,
> return ret;
> }
>
> +#define DSI_VBIF_CTRL (0x01CC - 4)
> +#define DSI_VBIF_CTRL_PRIORITY 0x07
> +
> void msm_dsi_host_cmd_xfer_commit(struct mipi_dsi_host *host, u32 dma_base,
> u32 len)
> {
> struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> + const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
> + u32 reg;
>
> dsi_write(msm_host, REG_DSI_DMA_BASE, dma_base);
> dsi_write(msm_host, REG_DSI_DMA_LEN, len);
> + if (cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V2_8_0) {
> + reg = dsi_read(msm_host, DSI_VBIF_CTRL);
> + reg |= (DSI_VBIF_CTRL_PRIORITY & 0x7);
> + dsi_write(msm_host, DSI_VBIF_CTRL, reg);
> + }
> dsi_write(msm_host, REG_DSI_TRIG_DMA, 1);
>
> /* Make sure trigger happens */
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] dt-bindings: display/msm: dsi-controller-main: Document dual panel property
2025-02-20 10:07 ` [PATCH v2 4/5] dt-bindings: display/msm: dsi-controller-main: Document dual panel property Jun Nie
@ 2025-02-20 10:33 ` Dmitry Baryshkov
2025-02-20 15:40 ` Jun Nie
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2025-02-20 10:33 UTC (permalink / raw)
To: Jun Nie
Cc: Rob Clark, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan,
linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree
On Thu, Feb 20, 2025 at 06:07:55PM +0800, Jun Nie wrote:
> The DSI interface can be connected to a panel that has a dual DSI channel,
> or to two separate panels, each equipped with a single DSI channel. To
> prevent the DSC configuration for the dual panel setup from disrupting the
> current configuration of a single panel with a dual DSI channel, add a dual
> panel property to support the use of two panels.
Please use the terms from the standard. The "channel" is mostly used for
the "Virtual Channel" or the "logical channel".
Also I don't follow how DSC configuration for a dual panel setup can
disrupt current (?) configuration of a single panel.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
> .../devicetree/bindings/display/msm/dsi-controller-main.yaml | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index ffbd1dc9470e2091b477b0c88392d81802119f48..e3f2eabde27609a66d6d81fafcb14e1bc014613c 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -88,9 +88,15 @@ properties:
> qcom,dual-dsi-mode:
> type: boolean
> description: |
> - Indicates if the DSI controller is driving a panel which needs
> + Indicates if the DSI controller is driving display device which needs
Unrelated change
> 2 DSI links.
>
> + qcom,dual-panel:
> + type: boolean
> + description: |
> + Indicates if the DSI controller is driving display device that composed
> + with 2 independent panels and needs 2 DSI links.
How is tht different from qcom,dual-dsi-mode?
> +
> qcom,master-dsi:
> type: boolean
> description: |
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/5] drm/msm/dsi: Support DSC for dual panel case
2025-02-20 10:07 ` [PATCH v2 5/5] drm/msm/dsi: Support DSC for dual panel case Jun Nie
@ 2025-02-20 10:38 ` Dmitry Baryshkov
2025-02-20 15:42 ` Jun Nie
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2025-02-20 10:38 UTC (permalink / raw)
To: Jun Nie
Cc: Rob Clark, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan,
linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree
On Thu, Feb 20, 2025 at 06:07:56PM +0800, Jun Nie wrote:
> There is dual DSI case that every DSI link is connected to an independent
> panel. In this dual panel case, the frame width for DSC on each link should
> be halved to support the usage case.
Isn't it the case for the DSI panel utilizing two DSI links?
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
> drivers/gpu/drm/msm/dsi/dsi.h | 3 ++-
> drivers/gpu/drm/msm/dsi/dsi_host.c | 13 +++++++++----
> drivers/gpu/drm/msm/dsi/dsi_manager.c | 10 ++++++++--
> 3 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> index 35b90c462f637111159b204269ce908614a21586..5a8978bed9f4ca897b418ced60194042d9dd8d05 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.h
> +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> @@ -74,7 +74,8 @@ void msm_dsi_host_enable_irq(struct mipi_dsi_host *host);
> void msm_dsi_host_disable_irq(struct mipi_dsi_host *host);
> int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> struct msm_dsi_phy_shared_timings *phy_shared_timings,
> - bool is_bonded_dsi, struct msm_dsi_phy *phy);
> + bool is_bonded_dsi, bool is_dual_panel,
> + struct msm_dsi_phy *phy);
> int msm_dsi_host_power_off(struct mipi_dsi_host *host);
> int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
> const struct drm_display_mode *mode);
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 976c5d82a2efa0fc51657b8534675890be7c33a6..752a97f7181c30dade0a7745492bf16649b3197b 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -902,7 +902,8 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
> }
> }
>
> -static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> +static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi,
> + bool is_dual_panel)
> {
> struct drm_display_mode *mode = msm_host->mode;
> u32 hs_start = 0, vs_start = 0; /* take sync start as 0 */
> @@ -947,7 +948,10 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> return;
> }
>
> - dsc->pic_width = mode->hdisplay;
> + if (is_dual_panel)
> + dsc->pic_width = hdisplay;
> + else
> + dsc->pic_width = mode->hdisplay;
> dsc->pic_height = mode->vdisplay;
> DBG("Mode %dx%d\n", dsc->pic_width, dsc->pic_height);
>
> @@ -2369,7 +2373,8 @@ static void msm_dsi_sfpb_config(struct msm_dsi_host *msm_host, bool enable)
>
> int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> struct msm_dsi_phy_shared_timings *phy_shared_timings,
> - bool is_bonded_dsi, struct msm_dsi_phy *phy)
> + bool is_bonded_dsi, bool is_dual_panel,
> + struct msm_dsi_phy *phy)
> {
> struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
> @@ -2412,7 +2417,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> goto fail_disable_clk;
> }
>
> - dsi_timing_setup(msm_host, is_bonded_dsi);
> + dsi_timing_setup(msm_host, is_bonded_dsi, is_dual_panel);
> dsi_sw_reset(msm_host);
> dsi_ctrl_enable(msm_host, phy_shared_timings, phy);
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index be13bf682a9601484c9c14e8419563f37c2281ee..158b6cc907cb39cc3b182d3088b793d322a3527c 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -24,6 +24,7 @@ struct msm_dsi_manager {
> struct msm_dsi *dsi[DSI_MAX];
>
> bool is_bonded_dsi;
> + bool is_dual_panel;
> bool is_sync_needed;
> int master_dsi_link_id;
> };
> @@ -31,6 +32,7 @@ struct msm_dsi_manager {
> static struct msm_dsi_manager msm_dsim_glb;
>
> #define IS_BONDED_DSI() (msm_dsim_glb.is_bonded_dsi)
> +#define IS_DUAL_PANEL() (msm_dsim_glb.is_dual_panel)
> #define IS_SYNC_NEEDED() (msm_dsim_glb.is_sync_needed)
> #define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id == id)
>
> @@ -55,6 +57,7 @@ static int dsi_mgr_parse_of(struct device_node *np, int id)
> msm_dsim->is_bonded_dsi = of_property_read_bool(np, "qcom,dual-dsi-mode");
>
> if (msm_dsim->is_bonded_dsi) {
> + msm_dsim->is_dual_panel = of_property_read_bool(np, "qcom,dual-panel");
> if (of_property_read_bool(np, "qcom,master-dsi"))
> msm_dsim->master_dsi_link_id = id;
> if (!msm_dsim->is_sync_needed)
> @@ -214,6 +217,7 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> struct mipi_dsi_host *host = msm_dsi->host;
> struct msm_dsi_phy_shared_timings phy_shared_timings[DSI_MAX];
> bool is_bonded_dsi = IS_BONDED_DSI();
> + bool is_dual_panel = IS_DUAL_PANEL();
> int ret;
>
> DBG("id=%d", id);
> @@ -222,7 +226,8 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> if (ret)
> goto phy_en_fail;
>
> - ret = msm_dsi_host_power_on(host, &phy_shared_timings[id], is_bonded_dsi, msm_dsi->phy);
> + ret = msm_dsi_host_power_on(host, &phy_shared_timings[id],
> + is_bonded_dsi, is_dual_panel, msm_dsi->phy);
> if (ret) {
> pr_err("%s: power on host %d failed, %d\n", __func__, id, ret);
> goto host_on_fail;
> @@ -230,7 +235,8 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
>
> if (is_bonded_dsi && msm_dsi1) {
> ret = msm_dsi_host_power_on(msm_dsi1->host,
> - &phy_shared_timings[DSI_1], is_bonded_dsi, msm_dsi1->phy);
> + &phy_shared_timings[DSI_1], is_bonded_dsi,
> + is_dual_panel, msm_dsi1->phy);
> if (ret) {
> pr_err("%s: power on host1 failed, %d\n",
> __func__, ret);
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] dt-bindings: display/msm: dsi-controller-main: Document dual panel property
2025-02-20 10:33 ` Dmitry Baryshkov
@ 2025-02-20 15:40 ` Jun Nie
2025-02-20 16:01 ` Dmitry Baryshkov
0 siblings, 1 reply; 17+ messages in thread
From: Jun Nie @ 2025-02-20 15:40 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan,
linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2025年2月20日周四 18:33写道:
>
> On Thu, Feb 20, 2025 at 06:07:55PM +0800, Jun Nie wrote:
> > The DSI interface can be connected to a panel that has a dual DSI channel,
> > or to two separate panels, each equipped with a single DSI channel. To
> > prevent the DSC configuration for the dual panel setup from disrupting the
> > current configuration of a single panel with a dual DSI channel, add a dual
> > panel property to support the use of two panels.
>
> Please use the terms from the standard. The "channel" is mostly used for
> the "Virtual Channel" or the "logical channel".
OK, will use DSI link for all later description.
>
> Also I don't follow how DSC configuration for a dual panel setup can
> disrupt current (?) configuration of a single panel.
For the disruption, Marijn mentioned it in the last post.
https://gitlab.freedesktop.org/drm/msm/-/issues/41#note_2411541
>
> >
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > ---
> > .../devicetree/bindings/display/msm/dsi-controller-main.yaml | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > index ffbd1dc9470e2091b477b0c88392d81802119f48..e3f2eabde27609a66d6d81fafcb14e1bc014613c 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > @@ -88,9 +88,15 @@ properties:
> > qcom,dual-dsi-mode:
> > type: boolean
> > description: |
> > - Indicates if the DSI controller is driving a panel which needs
> > + Indicates if the DSI controller is driving display device which needs
>
> Unrelated change
>
> > 2 DSI links.
> >
> > + qcom,dual-panel:
> > + type: boolean
> > + description: |
> > + Indicates if the DSI controller is driving display device that composed
> > + with 2 independent panels and needs 2 DSI links.
>
> How is tht different from qcom,dual-dsi-mode?
Your questioning is right. The dual panel case is a subset of
dual-dsi-mode, not parallel with
dual-dsi-mode. It is single panel with 2 DSI link by default, and 2
panel with 1 DSI link in
each panel if property dual-panel is present.
>
> > +
> > qcom,master-dsi:
> > type: boolean
> > description: |
> >
> > --
> > 2.34.1
> >
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/5] drm/msm/dsi: Support DSC for dual panel case
2025-02-20 10:38 ` Dmitry Baryshkov
@ 2025-02-20 15:42 ` Jun Nie
2025-02-20 16:06 ` Dmitry Baryshkov
0 siblings, 1 reply; 17+ messages in thread
From: Jun Nie @ 2025-02-20 15:42 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan,
linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2025年2月20日周四 18:39写道:
>
> On Thu, Feb 20, 2025 at 06:07:56PM +0800, Jun Nie wrote:
> > There is dual DSI case that every DSI link is connected to an independent
> > panel. In this dual panel case, the frame width for DSC on each link should
> > be halved to support the usage case.
>
> Isn't it the case for the DSI panel utilizing two DSI links?
The added case here is 2 DSI panel utilizing two DSI links, 1 DSI link
in each panel.
I assume default case is 1 panel with 2 DSI link, such as Marijn's case.
>
> >
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > ---
> > drivers/gpu/drm/msm/dsi/dsi.h | 3 ++-
> > drivers/gpu/drm/msm/dsi/dsi_host.c | 13 +++++++++----
> > drivers/gpu/drm/msm/dsi/dsi_manager.c | 10 ++++++++--
> > 3 files changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> > index 35b90c462f637111159b204269ce908614a21586..5a8978bed9f4ca897b418ced60194042d9dd8d05 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi.h
> > +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> > @@ -74,7 +74,8 @@ void msm_dsi_host_enable_irq(struct mipi_dsi_host *host);
> > void msm_dsi_host_disable_irq(struct mipi_dsi_host *host);
> > int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> > struct msm_dsi_phy_shared_timings *phy_shared_timings,
> > - bool is_bonded_dsi, struct msm_dsi_phy *phy);
> > + bool is_bonded_dsi, bool is_dual_panel,
> > + struct msm_dsi_phy *phy);
> > int msm_dsi_host_power_off(struct mipi_dsi_host *host);
> > int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
> > const struct drm_display_mode *mode);
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > index 976c5d82a2efa0fc51657b8534675890be7c33a6..752a97f7181c30dade0a7745492bf16649b3197b 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -902,7 +902,8 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
> > }
> > }
> >
> > -static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> > +static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi,
> > + bool is_dual_panel)
> > {
> > struct drm_display_mode *mode = msm_host->mode;
> > u32 hs_start = 0, vs_start = 0; /* take sync start as 0 */
> > @@ -947,7 +948,10 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> > return;
> > }
> >
> > - dsc->pic_width = mode->hdisplay;
> > + if (is_dual_panel)
> > + dsc->pic_width = hdisplay;
> > + else
> > + dsc->pic_width = mode->hdisplay;
> > dsc->pic_height = mode->vdisplay;
> > DBG("Mode %dx%d\n", dsc->pic_width, dsc->pic_height);
> >
> > @@ -2369,7 +2373,8 @@ static void msm_dsi_sfpb_config(struct msm_dsi_host *msm_host, bool enable)
> >
> > int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> > struct msm_dsi_phy_shared_timings *phy_shared_timings,
> > - bool is_bonded_dsi, struct msm_dsi_phy *phy)
> > + bool is_bonded_dsi, bool is_dual_panel,
> > + struct msm_dsi_phy *phy)
> > {
> > struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> > const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
> > @@ -2412,7 +2417,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> > goto fail_disable_clk;
> > }
> >
> > - dsi_timing_setup(msm_host, is_bonded_dsi);
> > + dsi_timing_setup(msm_host, is_bonded_dsi, is_dual_panel);
> > dsi_sw_reset(msm_host);
> > dsi_ctrl_enable(msm_host, phy_shared_timings, phy);
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > index be13bf682a9601484c9c14e8419563f37c2281ee..158b6cc907cb39cc3b182d3088b793d322a3527c 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > @@ -24,6 +24,7 @@ struct msm_dsi_manager {
> > struct msm_dsi *dsi[DSI_MAX];
> >
> > bool is_bonded_dsi;
> > + bool is_dual_panel;
> > bool is_sync_needed;
> > int master_dsi_link_id;
> > };
> > @@ -31,6 +32,7 @@ struct msm_dsi_manager {
> > static struct msm_dsi_manager msm_dsim_glb;
> >
> > #define IS_BONDED_DSI() (msm_dsim_glb.is_bonded_dsi)
> > +#define IS_DUAL_PANEL() (msm_dsim_glb.is_dual_panel)
> > #define IS_SYNC_NEEDED() (msm_dsim_glb.is_sync_needed)
> > #define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id == id)
> >
> > @@ -55,6 +57,7 @@ static int dsi_mgr_parse_of(struct device_node *np, int id)
> > msm_dsim->is_bonded_dsi = of_property_read_bool(np, "qcom,dual-dsi-mode");
> >
> > if (msm_dsim->is_bonded_dsi) {
> > + msm_dsim->is_dual_panel = of_property_read_bool(np, "qcom,dual-panel");
> > if (of_property_read_bool(np, "qcom,master-dsi"))
> > msm_dsim->master_dsi_link_id = id;
> > if (!msm_dsim->is_sync_needed)
> > @@ -214,6 +217,7 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> > struct mipi_dsi_host *host = msm_dsi->host;
> > struct msm_dsi_phy_shared_timings phy_shared_timings[DSI_MAX];
> > bool is_bonded_dsi = IS_BONDED_DSI();
> > + bool is_dual_panel = IS_DUAL_PANEL();
> > int ret;
> >
> > DBG("id=%d", id);
> > @@ -222,7 +226,8 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> > if (ret)
> > goto phy_en_fail;
> >
> > - ret = msm_dsi_host_power_on(host, &phy_shared_timings[id], is_bonded_dsi, msm_dsi->phy);
> > + ret = msm_dsi_host_power_on(host, &phy_shared_timings[id],
> > + is_bonded_dsi, is_dual_panel, msm_dsi->phy);
> > if (ret) {
> > pr_err("%s: power on host %d failed, %d\n", __func__, id, ret);
> > goto host_on_fail;
> > @@ -230,7 +235,8 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> >
> > if (is_bonded_dsi && msm_dsi1) {
> > ret = msm_dsi_host_power_on(msm_dsi1->host,
> > - &phy_shared_timings[DSI_1], is_bonded_dsi, msm_dsi1->phy);
> > + &phy_shared_timings[DSI_1], is_bonded_dsi,
> > + is_dual_panel, msm_dsi1->phy);
> > if (ret) {
> > pr_err("%s: power on host1 failed, %d\n",
> > __func__, ret);
> >
> > --
> > 2.34.1
> >
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/5] drm/msm/dsi: add support VBIF_CTRL_PRIORITY to v2.8.0 controller
2025-02-20 10:27 ` Dmitry Baryshkov
@ 2025-02-20 15:45 ` Jun Nie
2025-02-20 15:55 ` Dmitry Baryshkov
0 siblings, 1 reply; 17+ messages in thread
From: Jun Nie @ 2025-02-20 15:45 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan,
linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree,
Jonathan Marek
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2025年2月20日周四 18:27写道:
>
> On Thu, Feb 20, 2025 at 06:07:52PM +0800, Jun Nie wrote:
> > This change originates from the Qualcomm Android Linux driver. It is
> > essential to support a dual-DSI configuration with two panels in
> > some circumstances per testing. As the name suggests, this modification
> > may enhance the bandwidth robustness of a bus.
>
> Please start by describing the problem and the result of the changes.
> Otherwise it reads as it "may enhance or may worsen" the robustness.
OK, will re-test it with older branch. I remember it cause LCD to go
to black without
this patch, but not fully confident with my memory. With latest code,
there is no
difference to have this patch or not.
>
> >
> > Co-developed-by: Jonathan Marek <jonathan@marek.ca>
> > Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > ---
> > drivers/gpu/drm/msm/dsi/dsi_host.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > index 42e100a8adca09d7b55afce0e2553e76d898744f..f59c4cd6bc8cdb31c1302f8e3ff395486c0b4898 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -2238,13 +2238,23 @@ int msm_dsi_host_cmd_rx(struct mipi_dsi_host *host,
> > return ret;
> > }
> >
> > +#define DSI_VBIF_CTRL (0x01CC - 4)
> > +#define DSI_VBIF_CTRL_PRIORITY 0x07
> > +
> > void msm_dsi_host_cmd_xfer_commit(struct mipi_dsi_host *host, u32 dma_base,
> > u32 len)
> > {
> > struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> > + const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
> > + u32 reg;
> >
> > dsi_write(msm_host, REG_DSI_DMA_BASE, dma_base);
> > dsi_write(msm_host, REG_DSI_DMA_LEN, len);
> > + if (cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V2_8_0) {
> > + reg = dsi_read(msm_host, DSI_VBIF_CTRL);
> > + reg |= (DSI_VBIF_CTRL_PRIORITY & 0x7);
> > + dsi_write(msm_host, DSI_VBIF_CTRL, reg);
> > + }
> > dsi_write(msm_host, REG_DSI_TRIG_DMA, 1);
> >
> > /* Make sure trigger happens */
> >
> > --
> > 2.34.1
> >
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/5] drm/msm/dsi: add support VBIF_CTRL_PRIORITY to v2.8.0 controller
2025-02-20 15:45 ` Jun Nie
@ 2025-02-20 15:55 ` Dmitry Baryshkov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Baryshkov @ 2025-02-20 15:55 UTC (permalink / raw)
To: Jun Nie
Cc: Rob Clark, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan,
linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree,
Jonathan Marek
On Thu, Feb 20, 2025 at 11:45:18PM +0800, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2025年2月20日周四 18:27写道:
> >
> > On Thu, Feb 20, 2025 at 06:07:52PM +0800, Jun Nie wrote:
> > > This change originates from the Qualcomm Android Linux driver. It is
> > > essential to support a dual-DSI configuration with two panels in
> > > some circumstances per testing. As the name suggests, this modification
> > > may enhance the bandwidth robustness of a bus.
> >
> > Please start by describing the problem and the result of the changes.
> > Otherwise it reads as it "may enhance or may worsen" the robustness.
>
> OK, will re-test it with older branch. I remember it cause LCD to go
> to black without
> this patch, but not fully confident with my memory. With latest code,
> there is no
> difference to have this patch or not.
For the reference, this is the description from the display drivers
techpack:
disp: msm: dsi: Adjust DSI priority level
Sets DSI priority level to 7 before any commands are triggered.
This DSI priority setting is recommended by systems team as DSI
and Lutdma uses same Xin for fetch.
Maybe Abhinav can help with the proper problem description.
Also, see below.
> >
> > >
> > > Co-developed-by: Jonathan Marek <jonathan@marek.ca>
> > > Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> > > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > > ---
> > > drivers/gpu/drm/msm/dsi/dsi_host.c | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > index 42e100a8adca09d7b55afce0e2553e76d898744f..f59c4cd6bc8cdb31c1302f8e3ff395486c0b4898 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > @@ -2238,13 +2238,23 @@ int msm_dsi_host_cmd_rx(struct mipi_dsi_host *host,
> > > return ret;
> > > }
> > >
> > > +#define DSI_VBIF_CTRL (0x01CC - 4)
Register definition should go to the dsi.xml file. And anyway it should
have used lower case hex.
> > > +#define DSI_VBIF_CTRL_PRIORITY 0x07
> > > +
> > > void msm_dsi_host_cmd_xfer_commit(struct mipi_dsi_host *host, u32 dma_base,
> > > u32 len)
> > > {
> > > struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> > > + const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
> > > + u32 reg;
> > >
> > > dsi_write(msm_host, REG_DSI_DMA_BASE, dma_base);
> > > dsi_write(msm_host, REG_DSI_DMA_LEN, len);
> > > + if (cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V2_8_0) {
> > > + reg = dsi_read(msm_host, DSI_VBIF_CTRL);
> > > + reg |= (DSI_VBIF_CTRL_PRIORITY & 0x7);
> > > + dsi_write(msm_host, DSI_VBIF_CTRL, reg);
> > > + }
> > > dsi_write(msm_host, REG_DSI_TRIG_DMA, 1);
> > >
> > > /* Make sure trigger happens */
> > >
> > > --
> > > 2.34.1
> > >
> >
> > --
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/5] dt-bindings: display/msm: dsi-controller-main: Document dual panel property
2025-02-20 15:40 ` Jun Nie
@ 2025-02-20 16:01 ` Dmitry Baryshkov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Baryshkov @ 2025-02-20 16:01 UTC (permalink / raw)
To: Jun Nie
Cc: Rob Clark, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan,
linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree
On Thu, Feb 20, 2025 at 11:40:03PM +0800, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2025年2月20日周四 18:33写道:
> >
> > On Thu, Feb 20, 2025 at 06:07:55PM +0800, Jun Nie wrote:
> > > The DSI interface can be connected to a panel that has a dual DSI channel,
> > > or to two separate panels, each equipped with a single DSI channel. To
> > > prevent the DSC configuration for the dual panel setup from disrupting the
> > > current configuration of a single panel with a dual DSI channel, add a dual
> > > panel property to support the use of two panels.
> >
> > Please use the terms from the standard. The "channel" is mostly used for
> > the "Virtual Channel" or the "logical channel".
>
> OK, will use DSI link for all later description.
> >
> > Also I don't follow how DSC configuration for a dual panel setup can
> > disrupt current (?) configuration of a single panel.
>
> For the disruption, Marijn mentioned it in the last post.
> https://gitlab.freedesktop.org/drm/msm/-/issues/41#note_2411541
So, why is it happening?
>
> >
> > >
> > > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > > ---
> > > .../devicetree/bindings/display/msm/dsi-controller-main.yaml | 8 +++++++-
> > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > > index ffbd1dc9470e2091b477b0c88392d81802119f48..e3f2eabde27609a66d6d81fafcb14e1bc014613c 100644
> > > --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > > +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> > > @@ -88,9 +88,15 @@ properties:
> > > qcom,dual-dsi-mode:
> > > type: boolean
> > > description: |
> > > - Indicates if the DSI controller is driving a panel which needs
> > > + Indicates if the DSI controller is driving display device which needs
> >
> > Unrelated change
> >
> > > 2 DSI links.
> > >
> > > + qcom,dual-panel:
> > > + type: boolean
> > > + description: |
> > > + Indicates if the DSI controller is driving display device that composed
> > > + with 2 independent panels and needs 2 DSI links.
> >
> > How is tht different from qcom,dual-dsi-mode?
>
> Your questioning is right. The dual panel case is a subset of
> dual-dsi-mode, not parallel with
> dual-dsi-mode. It is single panel with 2 DSI link by default, and 2
> panel with 1 DSI link in
> each panel if property dual-panel is present.
And what if it is one panel having two 'logical' panels inside?
I'm trying to point out that this should be a property of the
struct mipi_dsi_device (or an option in the struct drm_dsc_config).
There is no need to describe this in DT.
> >
> > > +
> > > qcom,master-dsi:
> > > type: boolean
> > > description: |
> > >
> > > --
> > > 2.34.1
> > >
> >
> > --
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/5] drm/msm/dsi: Support DSC for dual panel case
2025-02-20 15:42 ` Jun Nie
@ 2025-02-20 16:06 ` Dmitry Baryshkov
2025-02-20 21:50 ` Marijn Suijten
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2025-02-20 16:06 UTC (permalink / raw)
To: Jun Nie
Cc: Rob Clark, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Krishna Manikandan,
linux-arm-msm, dri-devel, freedreno, linux-kernel, devicetree
On Thu, Feb 20, 2025 at 11:42:28PM +0800, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2025年2月20日周四 18:39写道:
> >
> > On Thu, Feb 20, 2025 at 06:07:56PM +0800, Jun Nie wrote:
> > > There is dual DSI case that every DSI link is connected to an independent
> > > panel. In this dual panel case, the frame width for DSC on each link should
> > > be halved to support the usage case.
> >
> > Isn't it the case for the DSI panel utilizing two DSI links?
>
> The added case here is 2 DSI panel utilizing two DSI links, 1 DSI link
> in each panel.
> I assume default case is 1 panel with 2 DSI link, such as Marijn's case.
So it should be halved in your case, but not in Marijn's case? I can
suspect that if you are describing two DSI panels as a single instance,
you should also adjust drm_dsc_config accordingly (on the panel's side?)
Maybe drm_dsc_config.pic_width and drm_dsc_config.pic_height should be
set on the panel's side? But then, how will that function for the DSI
panels or bridges which can change the mode?
> > > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > > ---
> > > drivers/gpu/drm/msm/dsi/dsi.h | 3 ++-
> > > drivers/gpu/drm/msm/dsi/dsi_host.c | 13 +++++++++----
> > > drivers/gpu/drm/msm/dsi/dsi_manager.c | 10 ++++++++--
> > > 3 files changed, 19 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> > > index 35b90c462f637111159b204269ce908614a21586..5a8978bed9f4ca897b418ced60194042d9dd8d05 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi.h
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> > > @@ -74,7 +74,8 @@ void msm_dsi_host_enable_irq(struct mipi_dsi_host *host);
> > > void msm_dsi_host_disable_irq(struct mipi_dsi_host *host);
> > > int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> > > struct msm_dsi_phy_shared_timings *phy_shared_timings,
> > > - bool is_bonded_dsi, struct msm_dsi_phy *phy);
> > > + bool is_bonded_dsi, bool is_dual_panel,
> > > + struct msm_dsi_phy *phy);
> > > int msm_dsi_host_power_off(struct mipi_dsi_host *host);
> > > int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
> > > const struct drm_display_mode *mode);
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > index 976c5d82a2efa0fc51657b8534675890be7c33a6..752a97f7181c30dade0a7745492bf16649b3197b 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > @@ -902,7 +902,8 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
> > > }
> > > }
> > >
> > > -static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> > > +static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi,
> > > + bool is_dual_panel)
> > > {
> > > struct drm_display_mode *mode = msm_host->mode;
> > > u32 hs_start = 0, vs_start = 0; /* take sync start as 0 */
> > > @@ -947,7 +948,10 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> > > return;
> > > }
> > >
> > > - dsc->pic_width = mode->hdisplay;
> > > + if (is_dual_panel)
> > > + dsc->pic_width = hdisplay;
> > > + else
> > > + dsc->pic_width = mode->hdisplay;
> > > dsc->pic_height = mode->vdisplay;
> > > DBG("Mode %dx%d\n", dsc->pic_width, dsc->pic_height);
> > >
> > > @@ -2369,7 +2373,8 @@ static void msm_dsi_sfpb_config(struct msm_dsi_host *msm_host, bool enable)
> > >
> > > int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> > > struct msm_dsi_phy_shared_timings *phy_shared_timings,
> > > - bool is_bonded_dsi, struct msm_dsi_phy *phy)
> > > + bool is_bonded_dsi, bool is_dual_panel,
> > > + struct msm_dsi_phy *phy)
> > > {
> > > struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> > > const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
> > > @@ -2412,7 +2417,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> > > goto fail_disable_clk;
> > > }
> > >
> > > - dsi_timing_setup(msm_host, is_bonded_dsi);
> > > + dsi_timing_setup(msm_host, is_bonded_dsi, is_dual_panel);
> > > dsi_sw_reset(msm_host);
> > > dsi_ctrl_enable(msm_host, phy_shared_timings, phy);
> > >
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > index be13bf682a9601484c9c14e8419563f37c2281ee..158b6cc907cb39cc3b182d3088b793d322a3527c 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > @@ -24,6 +24,7 @@ struct msm_dsi_manager {
> > > struct msm_dsi *dsi[DSI_MAX];
> > >
> > > bool is_bonded_dsi;
> > > + bool is_dual_panel;
> > > bool is_sync_needed;
> > > int master_dsi_link_id;
> > > };
> > > @@ -31,6 +32,7 @@ struct msm_dsi_manager {
> > > static struct msm_dsi_manager msm_dsim_glb;
> > >
> > > #define IS_BONDED_DSI() (msm_dsim_glb.is_bonded_dsi)
> > > +#define IS_DUAL_PANEL() (msm_dsim_glb.is_dual_panel)
> > > #define IS_SYNC_NEEDED() (msm_dsim_glb.is_sync_needed)
> > > #define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id == id)
> > >
> > > @@ -55,6 +57,7 @@ static int dsi_mgr_parse_of(struct device_node *np, int id)
> > > msm_dsim->is_bonded_dsi = of_property_read_bool(np, "qcom,dual-dsi-mode");
> > >
> > > if (msm_dsim->is_bonded_dsi) {
> > > + msm_dsim->is_dual_panel = of_property_read_bool(np, "qcom,dual-panel");
> > > if (of_property_read_bool(np, "qcom,master-dsi"))
> > > msm_dsim->master_dsi_link_id = id;
> > > if (!msm_dsim->is_sync_needed)
> > > @@ -214,6 +217,7 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> > > struct mipi_dsi_host *host = msm_dsi->host;
> > > struct msm_dsi_phy_shared_timings phy_shared_timings[DSI_MAX];
> > > bool is_bonded_dsi = IS_BONDED_DSI();
> > > + bool is_dual_panel = IS_DUAL_PANEL();
> > > int ret;
> > >
> > > DBG("id=%d", id);
> > > @@ -222,7 +226,8 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> > > if (ret)
> > > goto phy_en_fail;
> > >
> > > - ret = msm_dsi_host_power_on(host, &phy_shared_timings[id], is_bonded_dsi, msm_dsi->phy);
> > > + ret = msm_dsi_host_power_on(host, &phy_shared_timings[id],
> > > + is_bonded_dsi, is_dual_panel, msm_dsi->phy);
> > > if (ret) {
> > > pr_err("%s: power on host %d failed, %d\n", __func__, id, ret);
> > > goto host_on_fail;
> > > @@ -230,7 +235,8 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> > >
> > > if (is_bonded_dsi && msm_dsi1) {
> > > ret = msm_dsi_host_power_on(msm_dsi1->host,
> > > - &phy_shared_timings[DSI_1], is_bonded_dsi, msm_dsi1->phy);
> > > + &phy_shared_timings[DSI_1], is_bonded_dsi,
> > > + is_dual_panel, msm_dsi1->phy);
> > > if (ret) {
> > > pr_err("%s: power on host1 failed, %d\n",
> > > __func__, ret);
> > >
> > > --
> > > 2.34.1
> > >
> >
> > --
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/5] drm/msm/dsi: Support DSC for dual panel case
2025-02-20 16:06 ` Dmitry Baryshkov
@ 2025-02-20 21:50 ` Marijn Suijten
2025-02-21 0:20 ` Dmitry Baryshkov
0 siblings, 1 reply; 17+ messages in thread
From: Marijn Suijten @ 2025-02-20 21:50 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Jun Nie, Rob Clark, Abhinav Kumar, Jessica Zhang, Sean Paul,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Krishna Manikandan, linux-arm-msm, dri-devel, freedreno,
linux-kernel, devicetree
On 2025-02-20 18:06:01, Dmitry Baryshkov wrote:
> On Thu, Feb 20, 2025 at 11:42:28PM +0800, Jun Nie wrote:
> > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2025年2月20日周四 18:39写道:
> > >
> > > On Thu, Feb 20, 2025 at 06:07:56PM +0800, Jun Nie wrote:
> > > > There is dual DSI case that every DSI link is connected to an independent
There is a dual-DSI case where every DSI link ...
> > > > panel. In this dual panel case, the frame width for DSC on each link should
> > > > be halved to support the usage case.
use* case. Also, it shouldn't be "halved" just... because? It should be
"halved" because apparently hdisplay here is the width of the two panels
together, while the width coded in pic_width should contain that of a single
panel (since they're independent), which is equal to the width of a single
interface.
Tl;dr for below: this needs a *lot* more text to explain the setup and
possibilities. How is a DSI panel driver supposed to configure this on their
end? Hint: look at my previous drm/msm patches that explain how we expect to
interface with the parameters set by the panel driver.
> > >
> > > Isn't it the case for the DSI panel utilizing two DSI links?
> >
> > The added case here is 2 DSI panel utilizing two DSI links, 1 DSI link
> > in each panel.
> > I assume default case is 1 panel with 2 DSI link, such as Marijn's case.
>
> So it should be halved in your case, but not in Marijn's case? I can
> suspect that if you are describing two DSI panels as a single instance,
> you should also adjust drm_dsc_config accordingly (on the panel's side?)
>
> Maybe drm_dsc_config.pic_width and drm_dsc_config.pic_height should be
> set on the panel's side? But then, how will that function for the DSI
> panels or bridges which can change the mode?
It appears that these patches are missing a proper description of the setup
or use-case. I previously NAK'd those "dual DSI" patches because of this, but
reading between the lines I think I came to understand the reason without anyone
else explaining it, unfortunately. Needless to say that this needs very careful
documentation and wording in both code (DT and/or header structs) and commit
messages.
In my case I have a single high-resolution high-refresh-rate panel that can
simply not be driven over a single DSI link. A dual-DSI link is used in bonded
mode, most likely to keep the clocks and other things in sync, and to make it
easier to be represented by one virtual encoder in DPU? All control commands
only need the sent over one DSI link, not over both.
In this case pic_width is equal to the entire width of the panel, hence it is
double the width of a single interface.
Jun seems to have a strangely different use-case for bonded-DSI / dual-DSI that
isn't explained: two "independent" panels. It is clear to me that pic_width
here has to contain the width of the entire panel, and is hence equal to the
entire width of a single interface.
(And in the future, it seems the quad setup can drive two "bonded" panels with
two DSI-merge's each)
But what we're missing here is what the **advantages and limitations** are
of this setup: why would one run two DSI links for "independent" panels in
bonded-DSI mode? Is it more power-optimal? Will userspace see this as one
panel that's simply twice as wide? Do these panels have to be "identical"
so that they behave and are clocked the same? How is the driver expected to
prepare the mode and DSC configuration parameters to make this work?
Perhaps it's possible to scrape this info from [1] and prior commits but I
rather have a more digestible description in the commit message, thanks.
- Marijn
[1]: https://gitlab.com/jun.nie/linux/-/commit/98c0f411a85d68a76be500f8421df467d6cc53f3
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/5] drm/msm/dsi: Support DSC for dual panel case
2025-02-20 21:50 ` Marijn Suijten
@ 2025-02-21 0:20 ` Dmitry Baryshkov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Baryshkov @ 2025-02-21 0:20 UTC (permalink / raw)
To: Marijn Suijten
Cc: Jun Nie, Rob Clark, Abhinav Kumar, Jessica Zhang, Sean Paul,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Krishna Manikandan, linux-arm-msm, dri-devel, freedreno,
linux-kernel, devicetree
On Thu, Feb 20, 2025 at 10:50:57PM +0100, Marijn Suijten wrote:
> On 2025-02-20 18:06:01, Dmitry Baryshkov wrote:
> > On Thu, Feb 20, 2025 at 11:42:28PM +0800, Jun Nie wrote:
> > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2025年2月20日周四 18:39写道:
> > > >
> > > > On Thu, Feb 20, 2025 at 06:07:56PM +0800, Jun Nie wrote:
> > > > > There is dual DSI case that every DSI link is connected to an independent
>
> There is a dual-DSI case where every DSI link ...
>
> > > > > panel. In this dual panel case, the frame width for DSC on each link should
> > > > > be halved to support the usage case.
>
> use* case. Also, it shouldn't be "halved" just... because? It should be
> "halved" because apparently hdisplay here is the width of the two panels
> together, while the width coded in pic_width should contain that of a single
> panel (since they're independent), which is equal to the width of a single
> interface.
>
> Tl;dr for below: this needs a *lot* more text to explain the setup and
> possibilities. How is a DSI panel driver supposed to configure this on their
> end? Hint: look at my previous drm/msm patches that explain how we expect to
> interface with the parameters set by the panel driver.
>
> > > >
> > > > Isn't it the case for the DSI panel utilizing two DSI links?
> > >
> > > The added case here is 2 DSI panel utilizing two DSI links, 1 DSI link
> > > in each panel.
> > > I assume default case is 1 panel with 2 DSI link, such as Marijn's case.
> >
> > So it should be halved in your case, but not in Marijn's case? I can
> > suspect that if you are describing two DSI panels as a single instance,
> > you should also adjust drm_dsc_config accordingly (on the panel's side?)
> >
> > Maybe drm_dsc_config.pic_width and drm_dsc_config.pic_height should be
> > set on the panel's side? But then, how will that function for the DSI
> > panels or bridges which can change the mode?
>
> It appears that these patches are missing a proper description of the setup
> or use-case. I previously NAK'd those "dual DSI" patches because of this, but
> reading between the lines I think I came to understand the reason without anyone
> else explaining it, unfortunately. Needless to say that this needs very careful
> documentation and wording in both code (DT and/or header structs) and commit
> messages.
>
> In my case I have a single high-resolution high-refresh-rate panel that can
> simply not be driven over a single DSI link. A dual-DSI link is used in bonded
> mode, most likely to keep the clocks and other things in sync, and to make it
> easier to be represented by one virtual encoder in DPU? All control commands
> only need the sent over one DSI link, not over both.
>
> In this case pic_width is equal to the entire width of the panel, hence it is
> double the width of a single interface.
>
> Jun seems to have a strangely different use-case for bonded-DSI / dual-DSI that
> isn't explained: two "independent" panels. It is clear to me that pic_width
> here has to contain the width of the entire panel, and is hence equal to the
> entire width of a single interface.
> (And in the future, it seems the quad setup can drive two "bonded" panels with
> two DSI-merge's each)
>
> But what we're missing here is what the **advantages and limitations** are
> of this setup: why would one run two DSI links for "independent" panels in
> bonded-DSI mode? Is it more power-optimal? Will userspace see this as one
> panel that's simply twice as wide? Do these panels have to be "identical"
> so that they behave and are clocked the same? How is the driver expected to
> prepare the mode and DSC configuration parameters to make this work?
Fair enough. Maybe you will suggest how to handle it in a more efficient
way. We have been running such configurations (2 independent panels over
a bonded DSI link) in order to get a single synchronous CRTC vblank for
both panels. This is a nice hack for the software that outputs data for
both panels, but doesn't want to be concerned with separate vblanks.
> Perhaps it's possible to scrape this info from [1] and prior commits but I
> rather have a more digestible description in the commit message, thanks.
>
> - Marijn
>
> [1]: https://gitlab.com/jun.nie/linux/-/commit/98c0f411a85d68a76be500f8421df467d6cc53f3
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2025-02-21 0:21 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-20 10:07 [PATCH v2 0/5] drm/msm/dsi: Add DSC support to 2 panels in dual DSI mode Jun Nie
2025-02-20 10:07 ` [PATCH v2 1/5] drm/msm/dsi: add support VBIF_CTRL_PRIORITY to v2.8.0 controller Jun Nie
2025-02-20 10:27 ` Dmitry Baryshkov
2025-02-20 15:45 ` Jun Nie
2025-02-20 15:55 ` Dmitry Baryshkov
2025-02-20 10:07 ` [PATCH v2 2/5] drm/msm/dsi: check DSC width for the bonded DSI case Jun Nie
2025-02-20 10:07 ` [PATCH v2 3/5] drm/msm/dsi: support DSC configurations with slice_per_pkt > 1 Jun Nie
2025-02-20 10:07 ` [PATCH v2 4/5] dt-bindings: display/msm: dsi-controller-main: Document dual panel property Jun Nie
2025-02-20 10:33 ` Dmitry Baryshkov
2025-02-20 15:40 ` Jun Nie
2025-02-20 16:01 ` Dmitry Baryshkov
2025-02-20 10:07 ` [PATCH v2 5/5] drm/msm/dsi: Support DSC for dual panel case Jun Nie
2025-02-20 10:38 ` Dmitry Baryshkov
2025-02-20 15:42 ` Jun Nie
2025-02-20 16:06 ` Dmitry Baryshkov
2025-02-20 21:50 ` Marijn Suijten
2025-02-21 0:20 ` Dmitry Baryshkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).