Re: [PATCH 2/3] drm/msm/disp/dpu1: allow dspp selection for all the interfaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 17/01/2023 18:21, Kalyan Thota wrote:
Allow dspps to be populated as a requirement for all the encoder
types it need not be just DSI. If for any encoder the dspp
allocation doesn't go through then there can be an option to
fallback for color features.

Signed-off-by: Kalyan Thota <quic_kalyant@xxxxxxxxxxx>
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 18 +++++++++---------
  1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 9c6817b..e39b345 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -545,7 +545,8 @@ bool dpu_encoder_use_dsc_merge(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_display_mode *mode,
+			struct drm_crtc_state *crtc_state)

Is this new argument used at all?

  {
  	struct msm_display_topology topology = {0};
  	int i, intf_count = 0;
@@ -563,8 +564,9 @@ static struct msm_display_topology dpu_encoder_get_topology(
  	 * 1 LM, 1 INTF
  	 * 2 LM, 1 INTF (stream merge to support high resolution interfaces)
  	 *
-	 * Adding color blocks only to primary interface if available in
-	 * sufficient number
+	 * dspp blocks are made optional. If RM manager cannot allocate
+	 * dspp blocks, then reservations will still go through with non dspp LM's
+	 * so as to allow color management support via composer fallbacks
  	 */

No, this is not the way to go.

First, RM should prefer non-DSPP-enabled LMs if DSPP blocks are not required. Right now your patch makes it possible to allocate LMs, that have DSPP attached, for non-CTM-enabled encoder and later fail allocation of DSPP for the CRTC which has CTM blob attached.

Second, the decision on using DSPPs should come from dpu_crtc_atomic_check(). Pass 'bool need_dspp' to this function from dpu_atomic_check(). Fail if the need_dspp constraint can't be fulfilled.


  	if (intf_count == 2)
  		topology.num_lm = 2;
@@ -573,11 +575,9 @@ static struct msm_display_topology dpu_encoder_get_topology(
  	else
  		topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
- if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {
-		if (dpu_kms->catalog->dspp &&
-			(dpu_kms->catalog->dspp_count >= topology.num_lm))
-			topology.num_dspp = topology.num_lm;
-	}
+	if (dpu_kms->catalog->dspp &&
+	    (dpu_kms->catalog->dspp_count >= topology.num_lm))
+		topology.num_dspp = topology.num_lm;
topology.num_enc = 0;
  	topology.num_intf = intf_count;
@@ -643,7 +643,7 @@ static int dpu_encoder_virt_atomic_check(
  		}
  	}
- topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
+	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state);
/* Reserve dynamic resources now. */
  	if (!ret) {

--
With best wishes
Dmitry




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux