On 1/3/2025 10:16 AM, Dmitry Baryshkov wrote:
On Fri, Jan 03, 2025 at 10:03:35AM -0800, Jessica Zhang wrote:
On 12/19/2024 9:03 PM, Dmitry Baryshkov wrote:
On Mon, Dec 16, 2024 at 04:43:26PM -0800, Jessica Zhang wrote:
Add the cwb_enabled flag to msm_display topology and adjust the toplogy
to account for concurrent writeback
Why?
Hi Dmitry,
This flag is necessary to specify that CWB mux(es) need to be assigned for
the given reqeusted topology.
Why is necessary? Please rephrase your statement (we need foo bar, so do
baz).
Ack, what do you think of rephrasing the commit msg to this:
```
Add support for adjusting topology based on if concurrent writeback is
enabled.
Currently, the topology is calculated based on the assumption that the
user cannot request real-time and writeback simultaneously. For example,
the number of LMs and CTLs are currently based off the number of phys
encoders under the assumption there will be at least 1 LM/CTL per phys
encoder.
This will not hold true for concurrent writeback as 2 phys encoders (1
real-time and 1 writeback) can be driven by 1 LM/CTL when concurrent
writeback is enabled.
To account for this, add a cwb_enabled flag and only adjust the number
of CTL/LMs needed by a given topology based on the number of phys
encoders only if CWB is not enabled.
```
Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 11 ++++++++++-
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 10 ++++++++--
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 ++
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index b4bfded3d53025853cee112ca598533ece290318..b063c8fe4c0594772d84401fa56c9c21afc0ad18 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1198,6 +1198,8 @@ static struct msm_display_topology dpu_crtc_get_topology(
dpu_encoder_update_topology(drm_enc, &topology, crtc_state->state,
&crtc_state->adjusted_mode);
+ topology.cwb_enabled = drm_crtc_in_clone_mode(crtc_state);
+
/*
* Datapath topology selection
*
@@ -1209,9 +1211,16 @@ static struct msm_display_topology dpu_crtc_get_topology(
* 2 LM, 1 INTF (stream merge to support high resolution interfaces)
*
* Add dspps to the reservation requirements if ctm is requested
+ *
+ * Only hardcode num_lm to 2 for cases where num_intf == 2 and CWB is not
+ * enabled. This is because in cases where CWB is enabled, num_intf will
+ * count both the WB and real-time phys encoders.
+ *
+ * For non-DSC CWB usecases, have the num_lm be decided by the
+ * (mode->hdisplay > MAX_HDISPLAY_SPLIT) check.
*/
- if (topology.num_intf == 2)
+ if (topology.num_intf == 2 && !topology.cwb_enabled)
topology.num_lm = 2;
else if (topology.num_dsc == 2)
topology.num_lm = 2;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index b763ef19f4c60ae8a35df6a6ffb19e8411bc63f8..85adaf256b2c705d2d7df378b6ffc0e578f52bc3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -382,8 +382,14 @@ static int _dpu_rm_reserve_ctls(
int i = 0, j, num_ctls;
bool needs_split_display;
- /* each hw_intf needs its own hw_ctrl to program its control path */
- num_ctls = top->num_intf;
+ /*
+ * For non-CWB mode, each hw_intf needs its own hw_ctl to program its
+ * control path. Hardcode num_ctls to 1 if CWB is enabled
+ */
Why?
This is because num_intf is based on the number of phys_encs. Since in the
CWB case, the WB and real-time encoders will be driven by the same CTL. I
can add this to the comment doc.
Why are they driven by the same CTL? Is it also the case for platforms
before DPU 5.x?
This is because the WB and real-time path for a given topology would be
driven by the same data path so the same CTL should enable the real-time
and WB active bits.
This is the same for pre-DPU 5.x.
Thanks,
Jessica Zhang
+ if (top->cwb_enabled)
+ num_ctls = 1;
+ else
+ num_ctls = top->num_intf;
needs_split_display = _dpu_rm_needs_split_display(top);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index b061dfdab52e04ab7d777e912a30173273cb3db7..12db21a2403ec6930894c36a58e898c5d94c2568 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -46,6 +46,7 @@ struct dpu_rm {
* @num_dspp: number of dspp blocks used
* @num_dsc: number of Display Stream Compression (DSC) blocks used
* @needs_cdm: indicates whether cdm block is needed for this display topology
+ * @cwb_enabled: indicates whether CWB is enabled for this display topology
*/
struct msm_display_topology {
u32 num_lm;
@@ -53,6 +54,7 @@ struct msm_display_topology {
u32 num_dspp;
u32 num_dsc;
bool needs_cdm;
+ bool cwb_enabled;
};
int dpu_rm_init(struct drm_device *dev,
--
2.34.1
--
With best wishes
Dmitry
--
With best wishes
Dmitry
