[PATCH 00/19] Ultrajoiner basic functionality series

[PATCH 00/19] Ultrajoiner basic functionality series

[Ankit Nautiyal]

This patch series attempts to implement basic support
for Ultrajoiner functionality.

Rev6:
-Upgrade the debugfs functionality to enable the joining of a
specified number of pipes.
-Modify the display helpers reliant on the pipe joiner mechanism
to use number of pipes joined, instead of joiner flag.
-Checkpatch fixes.

Rev7:
-Use struct intel_display, minor refactoring, and rebase.

Rev8:
-Address comments from Ville.
-Simplified debugfs for forcing joiner, and added option to disable
joiner.
-Modified the ultra/bigjoiner helpers as suggested by Ville. 
-Split few of the bigger patches as suggested.

Test-with: 20240911072204.22825-1-karthik.b.s@intel.com

Ankit Nautiyal (11):
  drm/i915/display: Check whether platform supports joiner
  drm/i915/display: Modify debugfs for joiner to force n pipes
  drm/i915/display_debugfs: Allow force joiner only if supported
  drm/i915/dp: Add helper to compute num pipes joined
  drm/i915/display: Add debugfs support to avoid joiner
  drm/i915/display: Simplify intel_joiner_num_pipes and its usage
  drm/i915/display: Use joined pipes in intel_dp_joiner_needs_dsc
  drm/i915/display: Use joined pipes in intel_mode_valid_max_plane_size
  drm/i915/display: Use joined pipes in dsc helpers for slices, bpp
  drm/i915/display: Consider ultrajoiner for computing maxdotclock
  drm/i915/intel_dp: Add support for forcing ultrajoiner

Stanislav Lisovskiy (8):
  drm/i915: Add some essential functionality for joiners
  drm/i915: Split current joiner hw state readout
  drm/i915: Add bigjoiner and uncompressed joiner hw readout sanity
    checks
  drm/i915: Implement hw state readout and checks for ultrajoiner
  drm/i915/display: Percolate ultrajoiner info to get_joiner_config
  drm/i915/display/vdsc: Add ultrajoiner support with DSC
  drm/i915: Add new abstraction layer to handle pipe order for different
    joiners
  drm/i915: Compute config and mode valid changes for ultrajoiner

 drivers/gpu/drm/i915/display/intel_ddi.c      |  19 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 423 +++++++++++++++---
 drivers/gpu/drm/i915/display/intel_display.h  |  17 +-
 .../drm/i915/display/intel_display_debugfs.c  |  84 +++-
 .../drm/i915/display/intel_display_types.h    |   2 +-
 drivers/gpu/drm/i915/display/intel_dp.c       | 138 ++++--
 drivers/gpu/drm/i915/display/intel_dp.h       |  14 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  54 ++-
 drivers/gpu/drm/i915/display/intel_dsi.c      |   2 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c     |   2 +-
 drivers/gpu/drm/i915/display/intel_vdsc.c     |  30 +-
 .../gpu/drm/i915/display/intel_vdsc_regs.h    |   3 +
 12 files changed, 646 insertions(+), 142 deletions(-)

-- 
2.45.2

[PATCH 01/19] drm/i915/display: Check whether platform supports joiner

[Ankit Nautiyal]

Add a helper to check if platform supports joiner. Use it before readout
for joiner stuff and to calculate the maxdotclock supported.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b4ec9bf12aa7..486bade9e927 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3536,14 +3536,23 @@ static bool transcoder_ddi_func_is_enabled(struct drm_i915_private *dev_priv,
 	return tmp & TRANS_DDI_FUNC_ENABLE;
 }
 
+static bool intel_display_can_use_joiner(struct intel_display *display)
+{
+	return (DISPLAY_VER(display) >= 11);
+}
+
 static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 				 u8 *primary_pipes, u8 *secondary_pipes)
 {
+	struct intel_display *display = to_intel_display(&dev_priv->drm);
 	struct intel_crtc *crtc;
 
 	*primary_pipes = 0;
 	*secondary_pipes = 0;
 
+	if (!intel_display_can_use_joiner(display))
+		return;
+
 	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc,
 					 joiner_pipes(dev_priv)) {
 		enum intel_display_power_domain power_domain;
@@ -7961,10 +7970,11 @@ void intel_setup_outputs(struct drm_i915_private *dev_priv)
 
 static int max_dotclock(struct drm_i915_private *i915)
 {
+	struct intel_display *display = to_intel_display(&i915->drm);
 	int max_dotclock = i915->display.cdclk.max_dotclk_freq;
 
 	/* icl+ might use joiner */
-	if (DISPLAY_VER(i915) >= 11)
+	if (intel_display_can_use_joiner(display))
 		max_dotclock *= 2;
 
 	return max_dotclock;
-- 
2.45.2

[PATCH 02/19] drm/i915/display: Modify debugfs for joiner to force n pipes

[Ankit Nautiyal]

At the moment, the debugfs for joiner allows only to force enable/disable
pipe joiner for 2 pipes. Modify it to force join 'n' number of pipes,
where n is a valid pipe joiner configuration.
This will help in case of ultra joiner where 4 pipes are joined.

v2:
-Fix commit message to state that only valid joiner config can be
forced. (Suraj)
-Rename the identifiers to have INTEL_BIG/NONE_JOINER_PIPES. (Suraj)

v3:
-Avoid enum for joiner pipe counts, use bare numbers for better
readability. (Ville)
-Remove redundant prints from debugfs. (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 61 ++++++++++++++++++-
 .../drm/i915/display/intel_display_types.h    |  2 +-
 drivers/gpu/drm/i915/display/intel_dp.c       |  2 +-
 3 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 830b9eb60976..d7651f6f80e1 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1504,6 +1504,63 @@ static int intel_crtc_pipe_show(struct seq_file *m, void *unused)
 }
 DEFINE_SHOW_ATTRIBUTE(intel_crtc_pipe);
 
+static int i915_joiner_show(struct seq_file *m, void *data)
+{
+	struct intel_connector *connector = m->private;
+
+	seq_printf(m, "%d\n", connector->force_joined_pipes);
+
+	return 0;
+}
+
+static ssize_t i915_joiner_write(struct file *file,
+				 const char __user *ubuf,
+				 size_t len, loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	struct intel_connector *connector = m->private;
+	struct intel_display *display = to_intel_display(connector);
+	int force_join_pipes = 0;
+	int ret;
+
+	if (len == 0)
+		return 0;
+
+	ret = kstrtoint_from_user(ubuf, len, 0, &force_join_pipes);
+	if (ret < 0)
+		return ret;
+
+	switch (force_join_pipes) {
+	case 0:
+		fallthrough;
+	case 2:
+		connector->force_joined_pipes = force_join_pipes;
+		break;
+	default:
+		drm_dbg(display->drm, "Ignoring Invalid num of pipes %d for force joining\n",
+			force_join_pipes);
+		connector->force_joined_pipes = 0;
+	}
+
+	*offp += len;
+
+	return len;
+}
+
+static int i915_joiner_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, i915_joiner_show, inode->i_private);
+}
+
+static const struct file_operations i915_joiner_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_joiner_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+	.write = i915_joiner_write
+};
+
 /**
  * intel_connector_debugfs_add - add i915 specific connector debugfs files
  * @connector: pointer to a registered intel_connector
@@ -1553,8 +1610,8 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
 	if (DISPLAY_VER(i915) >= 11 &&
 	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
 	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
-		debugfs_create_bool("i915_bigjoiner_force_enable", 0644, root,
-				    &connector->force_bigjoiner_enable);
+		debugfs_create_file("i915_joiner_force_enable", 0644, root,
+				    connector, &i915_joiner_fops);
 	}
 
 	if (connector_type == DRM_MODE_CONNECTOR_DSI ||
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 000ab373c887..2ac2068aefa4 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -524,7 +524,7 @@ struct intel_connector {
 
 	struct intel_dp *mst_port;
 
-	bool force_bigjoiner_enable;
+	int force_joined_pipes;
 
 	struct {
 		struct drm_dp_aux *dsc_decompression_aux;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index a1fcedfd404b..5c30d4488141 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1271,7 +1271,7 @@ bool intel_dp_need_joiner(struct intel_dp *intel_dp,
 		return false;
 
 	return clock > i915->display.cdclk.max_dotclk_freq || hdisplay > 5120 ||
-	       connector->force_bigjoiner_enable;
+	       connector->force_joined_pipes == 2;
 }
 
 bool intel_dp_has_dsc(const struct intel_connector *connector)
-- 
2.45.2

[PATCH 03/19] drm/i915/display_debugfs: Allow force joiner only if supported

[Ankit Nautiyal]

Currently joiner is only supported for DP encoder.
Allow force joiner only for DP for supported platforms.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 20 ++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index d7651f6f80e1..7ceaf01a884a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1520,6 +1520,9 @@ static ssize_t i915_joiner_write(struct file *file,
 	struct seq_file *m = file->private_data;
 	struct intel_connector *connector = m->private;
 	struct intel_display *display = to_intel_display(connector);
+	struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp;
+	int connector_type = connector->base.connector_type;
 	int force_join_pipes = 0;
 	int ret;
 
@@ -1530,16 +1533,27 @@ static ssize_t i915_joiner_write(struct file *file,
 	if (ret < 0)
 		return ret;
 
+	/* Currently joiner is only supported for eDP/DP */
+	if (connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
+	    connector_type != DRM_MODE_CONNECTOR_eDP)
+		return 0;
+
+	intel_dp = enc_to_intel_dp(intel_encoder);
+
+	connector->force_joined_pipes = 0;
+
 	switch (force_join_pipes) {
 	case 0:
-		fallthrough;
+		break;
 	case 2:
-		connector->force_joined_pipes = force_join_pipes;
+		if (intel_dp_has_joiner(intel_dp))
+			connector->force_joined_pipes = force_join_pipes;
+		else
+			drm_dbg(display->drm, "Force joiner not supported for the config\n");
 		break;
 	default:
 		drm_dbg(display->drm, "Ignoring Invalid num of pipes %d for force joining\n",
 			force_join_pipes);
-		connector->force_joined_pipes = 0;
 	}
 
 	*offp += len;
-- 
2.45.2

[PATCH 04/19] drm/i915/dp: Add helper to compute num pipes joined

[Ankit Nautiyal]

Add a helper to compute the number of pipes to be joined.
The num of pipes joined will depend on whether the joiner is required or
is forced through the debugfs.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c     | 46 +++++++++++++++------
 drivers/gpu/drm/i915/display/intel_dp.h     |  6 +--
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 14 ++++---
 3 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 5c30d4488141..333624fcebd7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1261,17 +1261,33 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
 	return MODE_OK;
 }
 
-bool intel_dp_need_joiner(struct intel_dp *intel_dp,
-			  struct intel_connector *connector,
-			  int hdisplay, int clock)
+static
+bool intel_dp_needs_bigjoiner(struct intel_dp *intel_dp,
+			      struct intel_connector *connector,
+			      int hdisplay, int clock)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 
-	if (!intel_dp_has_joiner(intel_dp))
-		return false;
+	return clock > i915->display.cdclk.max_dotclk_freq || hdisplay > 5120;
+}
 
-	return clock > i915->display.cdclk.max_dotclk_freq || hdisplay > 5120 ||
-	       connector->force_joined_pipes == 2;
+int intel_dp_compute_joiner_pipes(struct intel_dp *intel_dp,
+				  struct intel_connector *connector,
+				  int hdisplay, int clock)
+{
+	switch (connector->force_joined_pipes) {
+	case 2:
+		return connector->force_joined_pipes;
+	default:
+		MISSING_CASE(connector->force_joined_pipes);
+		fallthrough;
+	case 0:
+		if (intel_dp_has_joiner(intel_dp) &&
+		    intel_dp_needs_bigjoiner(intel_dp, connector, hdisplay, clock))
+			return 2;
+	}
+
+	return 0;
 }
 
 bool intel_dp_has_dsc(const struct intel_connector *connector)
@@ -1309,6 +1325,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 	u8 dsc_slice_count = 0;
 	enum drm_mode_status status;
 	bool dsc = false, joiner = false;
+	int num_joined_pipes;
 
 	status = intel_cpu_transcoder_mode_valid(dev_priv, mode);
 	if (status != MODE_OK)
@@ -1329,11 +1346,14 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 		target_clock = fixed_mode->clock;
 	}
 
-	if (intel_dp_need_joiner(intel_dp, connector,
-				 mode->hdisplay, target_clock)) {
+	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
+							 mode->hdisplay, target_clock);
+
+	if (num_joined_pipes == 2) {
 		joiner = true;
 		max_dotclk *= 2;
 	}
+
 	if (target_clock > max_dotclk)
 		return MODE_CLOCK_HIGH;
 
@@ -2523,15 +2543,17 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	struct link_config_limits limits;
 	bool dsc_needed, joiner_needs_dsc;
+	int num_joined_pipes;
 	int ret = 0;
 
 	if (pipe_config->fec_enable &&
 	    !intel_dp_supports_fec(intel_dp, connector, pipe_config))
 		return -EINVAL;
 
-	if (intel_dp_need_joiner(intel_dp, connector,
-				 adjusted_mode->crtc_hdisplay,
-				 adjusted_mode->crtc_clock))
+	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
+							 adjusted_mode->crtc_hdisplay,
+							 adjusted_mode->crtc_clock);
+	if (num_joined_pipes == 2)
 		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
 
 	joiner_needs_dsc = intel_dp_joiner_needs_dsc(i915, pipe_config->joiner_pipes);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 1b9aaddd8c35..bc9a82d82df2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -153,9 +153,9 @@ int intel_dp_dsc_sink_max_compressed_bpp(const struct intel_connector *connector
 u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
 				int mode_clock, int mode_hdisplay,
 				bool bigjoiner);
-bool intel_dp_need_joiner(struct intel_dp *intel_dp,
-			  struct intel_connector *connector,
-			  int hdisplay, int clock);
+int intel_dp_compute_joiner_pipes(struct intel_dp *intel_dp,
+				  struct intel_connector *connector,
+				  int hdisplay, int clock);
 
 static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 15541932b809..383b3e38df52 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -568,6 +568,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
 		&pipe_config->hw.adjusted_mode;
 	struct link_config_limits limits;
 	bool dsc_needed, joiner_needs_dsc;
+	int num_joined_pipes;
 	int ret = 0;
 
 	if (pipe_config->fec_enable &&
@@ -577,9 +578,10 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return -EINVAL;
 
-	if (intel_dp_need_joiner(intel_dp, connector,
-				 adjusted_mode->crtc_hdisplay,
-				 adjusted_mode->crtc_clock))
+	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
+							 adjusted_mode->crtc_hdisplay,
+							 adjusted_mode->crtc_clock);
+	if (num_joined_pipes == 2)
 		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
 
 	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
@@ -1426,6 +1428,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 	u16 dsc_max_compressed_bpp = 0;
 	u8 dsc_slice_count = 0;
 	int target_clock = mode->clock;
+	int num_joined_pipes;
 
 	if (drm_connector_is_unregistered(connector)) {
 		*status = MODE_ERROR;
@@ -1465,8 +1468,9 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 	 *   corresponding link capabilities of the sink) in case the
 	 *   stream is uncompressed for it by the last branch device.
 	 */
-	if (intel_dp_need_joiner(intel_dp, intel_connector,
-				 mode->hdisplay, target_clock)) {
+	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, intel_connector,
+							 mode->hdisplay, target_clock);
+	if (num_joined_pipes == 2) {
 		joiner = true;
 		max_dotclk *= 2;
 	}
-- 
2.45.2

[PATCH 05/19] drm/i915/display: Add debugfs support to avoid joiner

[Ankit Nautiyal]

Currently debugfs for joiner can take a value of 0->dont care and
2->join 2 pipes. Add option to force to use only 1 pipe.

If debugfs is set to 1, force to exactly one pipe (ie. no
joiner despite what the automagic logic is saying).

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_debugfs.c | 3 +++
 drivers/gpu/drm/i915/display/intel_dp.c              | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 7ceaf01a884a..8b0cb750d9ae 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1545,6 +1545,9 @@ static ssize_t i915_joiner_write(struct file *file,
 	switch (force_join_pipes) {
 	case 0:
 		break;
+	case 1:
+		connector->force_joined_pipes = force_join_pipes;
+		break;
 	case 2:
 		if (intel_dp_has_joiner(intel_dp))
 			connector->force_joined_pipes = force_join_pipes;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 333624fcebd7..65044f63d1f5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1276,6 +1276,8 @@ int intel_dp_compute_joiner_pipes(struct intel_dp *intel_dp,
 				  int hdisplay, int clock)
 {
 	switch (connector->force_joined_pipes) {
+	case 1:
+		fallthrough;
 	case 2:
 		return connector->force_joined_pipes;
 	default:
@@ -1287,7 +1289,7 @@ int intel_dp_compute_joiner_pipes(struct intel_dp *intel_dp,
 			return 2;
 	}
 
-	return 0;
+	return 1;
 }
 
 bool intel_dp_has_dsc(const struct intel_connector *connector)
-- 
2.45.2

[PATCH 06/19] drm/i915/display: Simplify intel_joiner_num_pipes and its usage

[Ankit Nautiyal]

Currently intel_joiner_num_pipes is used to get num of pipes wrt num of
pipes joined. Simplify this by returning 1 when no joiner is used and
update the checks for no joiner case.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 486bade9e927..4751ee20216d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -280,7 +280,7 @@ bool intel_crtc_is_joiner_primary(const struct intel_crtc_state *crtc_state)
 
 static int intel_joiner_num_pipes(const struct intel_crtc_state *crtc_state)
 {
-	return hweight8(crtc_state->joiner_pipes);
+	return hweight8(crtc_state->joiner_pipes) ?: 1;
 }
 
 u8 intel_crtc_joined_pipe_mask(const struct intel_crtc_state *crtc_state)
@@ -2347,7 +2347,7 @@ static void intel_joiner_adjust_timings(const struct intel_crtc_state *crtc_stat
 {
 	int num_pipes = intel_joiner_num_pipes(crtc_state);
 
-	if (num_pipes < 2)
+	if (num_pipes == 1)
 		return;
 
 	mode->crtc_clock /= num_pipes;
@@ -2409,7 +2409,7 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
 	drm_mode_copy(mode, pipe_mode);
 	intel_mode_from_crtc_timings(mode, mode);
 	mode->hdisplay = drm_rect_width(&crtc_state->pipe_src) *
-		(intel_joiner_num_pipes(crtc_state) ?: 1);
+		intel_joiner_num_pipes(crtc_state);
 	mode->vdisplay = drm_rect_height(&crtc_state->pipe_src);
 
 	/* Derive per-pipe timings in case joiner is used */
@@ -2432,7 +2432,7 @@ static void intel_joiner_compute_pipe_src(struct intel_crtc_state *crtc_state)
 	int num_pipes = intel_joiner_num_pipes(crtc_state);
 	int width, height;
 
-	if (num_pipes < 2)
+	if (num_pipes == 1)
 		return;
 
 	width = drm_rect_width(&crtc_state->pipe_src);
@@ -2893,7 +2893,7 @@ static void intel_joiner_adjust_pipe_src(struct intel_crtc_state *crtc_state)
 	enum pipe primary_pipe, pipe = crtc->pipe;
 	int width;
 
-	if (num_pipes < 2)
+	if (num_pipes == 1)
 		return;
 
 	primary_pipe = joiner_primary_pipe(crtc_state);
-- 
2.45.2

[PATCH 07/19] drm/i915/display: Use joined pipes in intel_dp_joiner_needs_dsc

[Ankit Nautiyal]

In preparation of ultrajoiner, use number of joined pipes in the
intel_dp_joiner_needs_dsc helper, instead of joiner flag.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c     | 9 +++++----
 drivers/gpu/drm/i915/display/intel_dp.h     | 3 ++-
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 +++--
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 65044f63d1f5..6c503b5968d2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1412,7 +1412,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 		dsc = dsc_max_compressed_bpp && dsc_slice_count;
 	}
 
-	if (intel_dp_joiner_needs_dsc(dev_priv, joiner) && !dsc)
+	if (intel_dp_joiner_needs_dsc(dev_priv, num_joined_pipes) && !dsc)
 		return MODE_CLOCK_HIGH;
 
 	if (mode_rate > max_rate && !dsc)
@@ -2520,14 +2520,15 @@ int intel_dp_config_required_rate(const struct intel_crtc_state *crtc_state)
 	return intel_dp_link_required(adjusted_mode->crtc_clock, bpp);
 }
 
-bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915, bool use_joiner)
+bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915,
+			       int num_joined_pipes)
 {
 	/*
 	 * Pipe joiner needs compression up to display 12 due to bandwidth
 	 * limitation. DG2 onwards pipe joiner can be enabled without
 	 * compression.
 	 */
-	return DISPLAY_VER(i915) < 13 && use_joiner;
+	return DISPLAY_VER(i915) < 13 && (num_joined_pipes == 2);
 }
 
 static int
@@ -2558,7 +2559,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	if (num_joined_pipes == 2)
 		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
 
-	joiner_needs_dsc = intel_dp_joiner_needs_dsc(i915, pipe_config->joiner_pipes);
+	joiner_needs_dsc = intel_dp_joiner_needs_dsc(i915, num_joined_pipes);
 
 	dsc_needed = joiner_needs_dsc || intel_dp->force_dsc_en ||
 		     !intel_dp_compute_config_limits(intel_dp, pipe_config,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index bc9a82d82df2..cc08a309eb78 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -123,7 +123,8 @@ int intel_dp_effective_data_rate(int pixel_clock, int bpp_x16,
 				 int bw_overhead);
 int intel_dp_max_link_data_rate(struct intel_dp *intel_dp,
 				int max_dprx_rate, int max_dprx_lanes);
-bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915, bool use_joiner);
+bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915,
+			       int num_joined_pipes);
 bool intel_dp_has_joiner(struct intel_dp *intel_dp);
 bool intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state,
 			    const struct drm_connector_state *conn_state);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 383b3e38df52..fb067749f3a0 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -588,7 +588,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->has_pch_encoder = false;
 
-	joiner_needs_dsc = intel_dp_joiner_needs_dsc(dev_priv, pipe_config->joiner_pipes);
+	joiner_needs_dsc = intel_dp_joiner_needs_dsc(dev_priv, num_joined_pipes);
 
 	dsc_needed = joiner_needs_dsc || intel_dp->force_dsc_en ||
 		     !intel_dp_mst_compute_config_limits(intel_dp,
@@ -1472,6 +1472,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 							 mode->hdisplay, target_clock);
 	if (num_joined_pipes == 2) {
 		joiner = true;
+		num_joined_pipes = 2;
 		max_dotclk *= 2;
 	}
 
@@ -1512,7 +1513,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 		dsc = dsc_max_compressed_bpp && dsc_slice_count;
 	}
 
-	if (intel_dp_joiner_needs_dsc(dev_priv, joiner) && !dsc) {
+	if (intel_dp_joiner_needs_dsc(dev_priv, num_joined_pipes) && !dsc) {
 		*status = MODE_CLOCK_HIGH;
 		return 0;
 	}
-- 
2.45.2

[PATCH 08/19] drm/i915/display: Use joined pipes in intel_mode_valid_max_plane_size

[Ankit Nautiyal]

In preparation of ultrajoiner, use number of pipes in the
intel_mode_valid_max_plane_size helper, instead of joiner flag.

v2: Use num_pipes 1 where there are no joined pipes (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 4 ++--
 drivers/gpu/drm/i915/display/intel_display.h | 2 +-
 drivers/gpu/drm/i915/display/intel_dp.c      | 2 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c  | 2 +-
 drivers/gpu/drm/i915/display/intel_dsi.c     | 2 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c    | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 4751ee20216d..decfc72312fa 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8098,7 +8098,7 @@ enum drm_mode_status intel_cpu_transcoder_mode_valid(struct drm_i915_private *de
 enum drm_mode_status
 intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
 				const struct drm_display_mode *mode,
-				bool joiner)
+				int num_pipes)
 {
 	int plane_width_max, plane_height_max;
 
@@ -8115,7 +8115,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
 	 * too big for that.
 	 */
 	if (DISPLAY_VER(dev_priv) >= 11) {
-		plane_width_max = 5120 << joiner;
+		plane_width_max = 5120 * num_pipes;
 		plane_height_max = 4320;
 	} else {
 		plane_width_max = 5120;
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index b21d9578d5db..d6af9e3f52a9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -415,7 +415,7 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
 enum drm_mode_status
 intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
 				const struct drm_display_mode *mode,
-				bool joiner);
+				int num_pipes);
 enum drm_mode_status
 intel_cpu_transcoder_mode_valid(struct drm_i915_private *i915,
 				const struct drm_display_mode *mode);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 6c503b5968d2..63eef9d97f25 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1422,7 +1422,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 	if (status != MODE_OK)
 		return status;
 
-	return intel_mode_valid_max_plane_size(dev_priv, mode, joiner);
+	return intel_mode_valid_max_plane_size(dev_priv, mode, num_joined_pipes);
 }
 
 bool intel_dp_source_supports_tps3(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index fb067749f3a0..bd069c34c5e6 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -1523,7 +1523,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 		return 0;
 	}
 
-	*status = intel_mode_valid_max_plane_size(dev_priv, mode, joiner);
+	*status = intel_mode_valid_max_plane_size(dev_priv, mode, num_joined_pipes);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_dsi.c b/drivers/gpu/drm/i915/display/intel_dsi.c
index bd5888ce4852..0be46c6c9611 100644
--- a/drivers/gpu/drm/i915/display/intel_dsi.c
+++ b/drivers/gpu/drm/i915/display/intel_dsi.c
@@ -76,7 +76,7 @@ enum drm_mode_status intel_dsi_mode_valid(struct drm_connector *connector,
 	if (fixed_mode->clock > max_dotclk)
 		return MODE_CLOCK_HIGH;
 
-	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
+	return intel_mode_valid_max_plane_size(dev_priv, mode, 1);
 }
 
 struct intel_dsi_host *intel_dsi_host_init(struct intel_dsi *intel_dsi,
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 869fa00f7ef2..72ac910bf6ec 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2057,7 +2057,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 			return status;
 	}
 
-	return intel_mode_valid_max_plane_size(dev_priv, mode, false);
+	return intel_mode_valid_max_plane_size(dev_priv, mode, 1);
 }
 
 bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
-- 
2.45.2

[PATCH 09/19] drm/i915/display: Use joined pipes in dsc helpers for slices, bpp

[Ankit Nautiyal]

In preparation of ultrajoiner, use number of joined pipes in the dsc
helpers to compute dsc slices and max compressed bpp, instead of using
the joiner flag.

v2: Adjust the formulae to use num of pipes as 1 (no joiner) or 2
(bigjoiner). (Ankit)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> (v1)
---
 drivers/gpu/drm/i915/display/intel_display.c |  2 +-
 drivers/gpu/drm/i915/display/intel_display.h |  1 +
 drivers/gpu/drm/i915/display/intel_dp.c      | 30 ++++++++++----------
 drivers/gpu/drm/i915/display/intel_dp.h      |  5 ++--
 drivers/gpu/drm/i915/display/intel_dp_mst.c  | 15 +++++-----
 5 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index decfc72312fa..a4201e96d6d9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -278,7 +278,7 @@ bool intel_crtc_is_joiner_primary(const struct intel_crtc_state *crtc_state)
 		crtc->pipe == joiner_primary_pipe(crtc_state);
 }
 
-static int intel_joiner_num_pipes(const struct intel_crtc_state *crtc_state)
+int intel_joiner_num_pipes(const struct intel_crtc_state *crtc_state)
 {
 	return hweight8(crtc_state->joiner_pipes) ?: 1;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index d6af9e3f52a9..366f300a5169 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -586,5 +586,6 @@ bool assert_port_valid(struct drm_i915_private *i915, enum port port);
 })
 
 bool intel_scanout_needs_vtd_wa(struct drm_i915_private *i915);
+int intel_joiner_num_pipes(const struct intel_crtc_state *crtc_state);
 
 #endif
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 63eef9d97f25..4b05a717cce3 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -859,14 +859,14 @@ u32 intel_dp_dsc_nearest_valid_bpp(struct drm_i915_private *i915, u32 bpp, u32 p
 static
 u32 get_max_compressed_bpp_with_joiner(struct drm_i915_private *i915,
 				       u32 mode_clock, u32 mode_hdisplay,
-				       bool bigjoiner)
+				       int num_joined_pipes)
 {
 	u32 max_bpp_small_joiner_ram;
 
 	/* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
 	max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) / mode_hdisplay;
 
-	if (bigjoiner) {
+	if (num_joined_pipes == 2) {
 		int bigjoiner_interface_bits = DISPLAY_VER(i915) >= 14 ? 36 : 24;
 		/* With bigjoiner multiple dsc engines are used in parallel so PPC is 2 */
 		int ppc = 2;
@@ -885,7 +885,7 @@ u32 get_max_compressed_bpp_with_joiner(struct drm_i915_private *i915,
 u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
 					u32 link_clock, u32 lane_count,
 					u32 mode_clock, u32 mode_hdisplay,
-					bool bigjoiner,
+					int num_joined_pipes,
 					enum intel_output_format output_format,
 					u32 pipe_bpp,
 					u32 timeslots)
@@ -931,7 +931,7 @@ u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
 				intel_dp_mode_to_fec_clock(mode_clock));
 
 	joiner_max_bpp = get_max_compressed_bpp_with_joiner(i915, mode_clock,
-							    mode_hdisplay, bigjoiner);
+							    mode_hdisplay, num_joined_pipes);
 	bits_per_pixel = min(bits_per_pixel, joiner_max_bpp);
 
 	bits_per_pixel = intel_dp_dsc_nearest_valid_bpp(i915, bits_per_pixel, pipe_bpp);
@@ -941,7 +941,7 @@ u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
 
 u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
 				int mode_clock, int mode_hdisplay,
-				bool bigjoiner)
+				int num_pipes)
 {
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 	u8 min_slice_count, i;
@@ -975,14 +975,14 @@ u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
 
 	/* Find the closest match to the valid slice count values */
 	for (i = 0; i < ARRAY_SIZE(valid_dsc_slicecount); i++) {
-		u8 test_slice_count = valid_dsc_slicecount[i] << bigjoiner;
+		u8 test_slice_count = valid_dsc_slicecount[i] * num_pipes;
 
 		if (test_slice_count >
 		    drm_dp_dsc_sink_max_slice_count(connector->dp.dsc_dpcd, false))
 			break;
 
 		/* big joiner needs small joiner to be enabled */
-		if (bigjoiner && test_slice_count < 4)
+		if (num_pipes == 2 && test_slice_count < 4)
 			continue;
 
 		if (min_slice_count <= test_slice_count)
@@ -1326,7 +1326,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 	u16 dsc_max_compressed_bpp = 0;
 	u8 dsc_slice_count = 0;
 	enum drm_mode_status status;
-	bool dsc = false, joiner = false;
+	bool dsc = false;
 	int num_joined_pipes;
 
 	status = intel_cpu_transcoder_mode_valid(dev_priv, mode);
@@ -1351,10 +1351,8 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
 							 mode->hdisplay, target_clock);
 
-	if (num_joined_pipes == 2) {
-		joiner = true;
+	if (num_joined_pipes == 2)
 		max_dotclk *= 2;
-	}
 
 	if (target_clock > max_dotclk)
 		return MODE_CLOCK_HIGH;
@@ -1399,14 +1397,14 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 								    max_lanes,
 								    target_clock,
 								    mode->hdisplay,
-								    joiner,
+								    num_joined_pipes,
 								    output_format,
 								    pipe_bpp, 64);
 			dsc_slice_count =
 				intel_dp_dsc_get_slice_count(connector,
 							     target_clock,
 							     mode->hdisplay,
-							     joiner);
+							     num_joined_pipes);
 		}
 
 		dsc = dsc_max_compressed_bpp && dsc_slice_count;
@@ -2122,6 +2120,7 @@ static int dsc_compute_compressed_bpp(struct intel_dp *intel_dp,
 	int dsc_src_min_bpp, dsc_sink_min_bpp, dsc_min_bpp;
 	int dsc_src_max_bpp, dsc_sink_max_bpp, dsc_max_bpp;
 	int dsc_joiner_max_bpp;
+	int num_joined_pipes = intel_joiner_num_pipes(pipe_config);
 
 	dsc_src_min_bpp = dsc_src_min_compressed_bpp();
 	dsc_sink_min_bpp = intel_dp_dsc_sink_min_compressed_bpp(pipe_config);
@@ -2136,7 +2135,7 @@ static int dsc_compute_compressed_bpp(struct intel_dp *intel_dp,
 
 	dsc_joiner_max_bpp = get_max_compressed_bpp_with_joiner(i915, adjusted_mode->clock,
 								adjusted_mode->hdisplay,
-								pipe_config->joiner_pipes);
+								num_joined_pipes);
 	dsc_max_bpp = min(dsc_max_bpp, dsc_joiner_max_bpp);
 	dsc_max_bpp = min(dsc_max_bpp, fxp_q4_to_int(limits->link.max_bpp_x16));
 
@@ -2321,6 +2320,7 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 		to_intel_connector(conn_state->connector);
 	const struct drm_display_mode *adjusted_mode =
 		&pipe_config->hw.adjusted_mode;
+	int num_joined_pipes = intel_joiner_num_pipes(pipe_config);
 	int ret;
 
 	pipe_config->fec_enable = pipe_config->fec_enable ||
@@ -2370,7 +2370,7 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 			intel_dp_dsc_get_slice_count(connector,
 						     adjusted_mode->crtc_clock,
 						     adjusted_mode->crtc_hdisplay,
-						     pipe_config->joiner_pipes);
+						     num_joined_pipes);
 		if (!dsc_dp_slice_count) {
 			drm_dbg_kms(&dev_priv->drm,
 				    "Compressed Slice Count not supported\n");
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index cc08a309eb78..4aa8d1c8b7fc 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -143,7 +143,7 @@ int intel_dp_dsc_compute_max_bpp(const struct intel_connector *connector,
 u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
 					u32 link_clock, u32 lane_count,
 					u32 mode_clock, u32 mode_hdisplay,
-					bool bigjoiner,
+					int num_joined_pipes,
 					enum intel_output_format output_format,
 					u32 pipe_bpp,
 					u32 timeslots);
@@ -153,11 +153,10 @@ int intel_dp_dsc_sink_max_compressed_bpp(const struct intel_connector *connector
 					 int bpc);
 u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
 				int mode_clock, int mode_hdisplay,
-				bool bigjoiner);
+				int num_pipes);
 int intel_dp_compute_joiner_pipes(struct intel_dp *intel_dp,
 				  struct intel_connector *connector,
 				  int hdisplay, int clock);
-
 static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
 {
 	return ~((1 << lane_count) - 1) & 0xf;
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index bd069c34c5e6..be79783ce09b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -102,11 +102,13 @@ static int intel_dp_mst_bw_overhead(const struct intel_crtc_state *crtc_state,
 	flags |= crtc_state->fec_enable ? DRM_DP_BW_OVERHEAD_FEC : 0;
 
 	if (dsc) {
+		int num_joined_pipes = intel_joiner_num_pipes(crtc_state);
+
 		flags |= DRM_DP_BW_OVERHEAD_DSC;
 		dsc_slice_count = intel_dp_dsc_get_slice_count(connector,
 							       adjusted_mode->clock,
 							       adjusted_mode->hdisplay,
-							       crtc_state->joiner_pipes);
+							       num_joined_pipes);
 	}
 
 	overhead = drm_dp_bw_overhead(crtc_state->lane_count,
@@ -1424,7 +1426,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 	int max_dotclk = to_i915(connector->dev)->display.cdclk.max_dotclk_freq;
 	int max_rate, mode_rate, max_lanes, max_link_clock;
 	int ret;
-	bool dsc = false, joiner = false;
+	bool dsc = false;
 	u16 dsc_max_compressed_bpp = 0;
 	u8 dsc_slice_count = 0;
 	int target_clock = mode->clock;
@@ -1470,11 +1472,8 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 	 */
 	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, intel_connector,
 							 mode->hdisplay, target_clock);
-	if (num_joined_pipes == 2) {
-		joiner = true;
-		num_joined_pipes = 2;
+	if (num_joined_pipes == 2)
 		max_dotclk *= 2;
-	}
 
 	ret = drm_modeset_lock(&mgr->base.lock, ctx);
 	if (ret)
@@ -1500,14 +1499,14 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 								    max_lanes,
 								    target_clock,
 								    mode->hdisplay,
-								    joiner,
+								    num_joined_pipes,
 								    INTEL_OUTPUT_FORMAT_RGB,
 								    pipe_bpp, 64);
 			dsc_slice_count =
 				intel_dp_dsc_get_slice_count(intel_connector,
 							     target_clock,
 							     mode->hdisplay,
-							     joiner);
+							     num_joined_pipes);
 		}
 
 		dsc = dsc_max_compressed_bpp && dsc_slice_count;
-- 
2.45.2

[PATCH 10/19] drm/i915: Add some essential functionality for joiners

[Ankit Nautiyal]

From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

In most of the cases we now try to avoid mentioning things like
"bigjoiner" or "ultrajoiner" trying to unify the API and refer
mostly to all this functionality as "joiner".
In majority cases that should be way to go.
However in some cases we still need to distinguish between
bigjoiner primaries and secondaries(such as DSC register programming).

Create correspondent helper functions and start using them,
in order be prepared for adding ultrajoiner functionality.

v2: Fixed checkpatch warnings (Ankit)
v3: Introduce ultrajoiner helpers in next patch.
v4: Streamlink the helpers and add few more. (Ville)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 41 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_display.h |  2 +
 drivers/gpu/drm/i915/display/intel_vdsc.c    |  4 +-
 3 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a4201e96d6d9..248ad63b0ba8 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -254,6 +254,47 @@ static enum pipe joiner_primary_pipe(const struct intel_crtc_state *crtc_state)
 	return ffs(crtc_state->joiner_pipes) - 1;
 }
 
+static bool is_bigjoiner(const struct intel_crtc_state *crtc_state)
+{
+	return hweight8(crtc_state->joiner_pipes) >= 2;
+}
+
+static u8 bigjoiner_primary_pipes(const struct intel_crtc_state *crtc_state)
+{
+	if (!is_bigjoiner(crtc_state))
+		return 0;
+
+	return crtc_state->joiner_pipes & (0b01010101 << joiner_primary_pipe(crtc_state));
+}
+
+static unsigned int bigjoiner_secondary_pipes(const struct intel_crtc_state *crtc_state)
+{
+	if (!is_bigjoiner(crtc_state))
+		return 0;
+
+	return crtc_state->joiner_pipes & (0b10101010 << joiner_primary_pipe(crtc_state));
+}
+
+bool intel_crtc_is_bigjoiner_primary(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+	if (!is_bigjoiner(crtc_state))
+		return false;
+
+	return BIT(crtc->pipe) & bigjoiner_primary_pipes(crtc_state);
+}
+
+bool intel_crtc_is_bigjoiner_secondary(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+	if (!is_bigjoiner(crtc_state))
+		return false;
+
+	return BIT(crtc->pipe) & bigjoiner_secondary_pipes(crtc_state);
+}
+
 u8 intel_crtc_joiner_secondary_pipes(const struct intel_crtc_state *crtc_state)
 {
 	if (crtc_state->joiner_pipes)
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 366f300a5169..17d8b9df3916 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -425,6 +425,8 @@ bool is_trans_port_sync_master(const struct intel_crtc_state *state);
 u8 intel_crtc_joined_pipe_mask(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_is_joiner_secondary(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_is_joiner_primary(const struct intel_crtc_state *crtc_state);
+bool intel_crtc_is_bigjoiner_primary(const struct intel_crtc_state *crtc_state);
+bool intel_crtc_is_bigjoiner_secondary(const struct intel_crtc_state *crtc_state);
 u8 intel_crtc_joiner_secondary_pipes(const struct intel_crtc_state *crtc_state);
 struct intel_crtc *intel_primary_crtc(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
index 2e849b015e74..8158e3702ed5 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -742,7 +742,7 @@ void intel_uncompressed_joiner_enable(const struct intel_crtc_state *crtc_state)
 	u32 dss_ctl1_val = 0;
 
 	if (crtc_state->joiner_pipes && !crtc_state->dsc.compression_enable) {
-		if (intel_crtc_is_joiner_secondary(crtc_state))
+		if (intel_crtc_is_bigjoiner_secondary(crtc_state))
 			dss_ctl1_val |= UNCOMPRESSED_JOINER_SECONDARY;
 		else
 			dss_ctl1_val |= UNCOMPRESSED_JOINER_PRIMARY;
@@ -771,7 +771,7 @@ void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
 	}
 	if (crtc_state->joiner_pipes) {
 		dss_ctl1_val |= BIG_JOINER_ENABLE;
-		if (!intel_crtc_is_joiner_secondary(crtc_state))
+		if (intel_crtc_is_bigjoiner_primary(crtc_state))
 			dss_ctl1_val |= PRIMARY_BIG_JOINER_ENABLE;
 	}
 	intel_de_write(dev_priv, dss_ctl1_reg(crtc, crtc_state->cpu_transcoder), dss_ctl1_val);
-- 
2.45.2

[PATCH 11/19] drm/i915: Split current joiner hw state readout

[Ankit Nautiyal]

From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

We need to add a new sanity checks and also do
some preparations for adding ultrajoiner hw state readout.
Lets first split reading of the uncompressed joiner and bigjoiner
bit masks into separate functions.

v2: Fixed checkpatch warnings (Ankit)
v3: Use struct intel_display in the new functions. (Ankit)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 75 ++++++++++++++------
 1 file changed, 55 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 248ad63b0ba8..e93af02aa859 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3582,27 +3582,54 @@ static bool intel_display_can_use_joiner(struct intel_display *display)
 	return (DISPLAY_VER(display) >= 11);
 }
 
-static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
-				 u8 *primary_pipes, u8 *secondary_pipes)
+static void enabled_uncompressed_joiner_pipes(struct intel_display *display,
+					      u8 *primary_pipes, u8 *secondary_pipes)
 {
-	struct intel_display *display = to_intel_display(&dev_priv->drm);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	struct intel_crtc *crtc;
 
 	*primary_pipes = 0;
 	*secondary_pipes = 0;
 
-	if (!intel_display_can_use_joiner(display))
+	if (DISPLAY_VER(display) < 13)
 		return;
 
-	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc,
-					 joiner_pipes(dev_priv)) {
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc,
+					 joiner_pipes(i915)) {
 		enum intel_display_power_domain power_domain;
 		enum pipe pipe = crtc->pipe;
 		intel_wakeref_t wakeref;
 
-		power_domain = intel_dsc_power_domain(crtc, (enum transcoder) pipe);
-		with_intel_display_power_if_enabled(dev_priv, power_domain, wakeref) {
-			u32 tmp = intel_de_read(dev_priv, ICL_PIPE_DSS_CTL1(pipe));
+		power_domain = POWER_DOMAIN_PIPE(pipe);
+		with_intel_display_power_if_enabled(i915, power_domain, wakeref) {
+			u32 tmp = intel_de_read(display, ICL_PIPE_DSS_CTL1(pipe));
+
+			if (tmp & UNCOMPRESSED_JOINER_PRIMARY)
+				*primary_pipes |= BIT(pipe);
+			if (tmp & UNCOMPRESSED_JOINER_SECONDARY)
+				*secondary_pipes |= BIT(pipe);
+		}
+	}
+}
+
+static void enabled_bigjoiner_pipes(struct intel_display *display,
+				    u8 *primary_pipes, u8 *secondary_pipes)
+{
+	struct drm_i915_private *i915 = to_i915(display->drm);
+	struct intel_crtc *crtc;
+
+	*primary_pipes = 0;
+	*secondary_pipes = 0;
+
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc,
+					 joiner_pipes(i915)) {
+		enum intel_display_power_domain power_domain;
+		enum pipe pipe = crtc->pipe;
+		intel_wakeref_t wakeref;
+
+		power_domain = intel_dsc_power_domain(crtc, (enum transcoder)pipe);
+		with_intel_display_power_if_enabled(i915, power_domain, wakeref) {
+			u32 tmp = intel_de_read(display, ICL_PIPE_DSS_CTL1(pipe));
 
 			if (!(tmp & BIG_JOINER_ENABLE))
 				continue;
@@ -3612,20 +3639,28 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 			else
 				*secondary_pipes |= BIT(pipe);
 		}
+	}
+}
 
-		if (DISPLAY_VER(dev_priv) < 13)
-			continue;
+static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
+				 u8 *primary_pipes, u8 *secondary_pipes)
+{
+	struct intel_display *display = to_intel_display(&dev_priv->drm);
+	u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes;
+	u8 secondary_uncompressed_joiner_pipes, secondary_bigjoiner_pipes;
 
-		power_domain = POWER_DOMAIN_PIPE(pipe);
-		with_intel_display_power_if_enabled(dev_priv, power_domain, wakeref) {
-			u32 tmp = intel_de_read(dev_priv, ICL_PIPE_DSS_CTL1(pipe));
+	if (!intel_display_can_use_joiner(display))
+		return;
 
-			if (tmp & UNCOMPRESSED_JOINER_PRIMARY)
-				*primary_pipes |= BIT(pipe);
-			if (tmp & UNCOMPRESSED_JOINER_SECONDARY)
-				*secondary_pipes |= BIT(pipe);
-		}
-	}
+	enabled_uncompressed_joiner_pipes(display, &primary_uncompressed_joiner_pipes,
+					  &secondary_uncompressed_joiner_pipes);
+
+	enabled_bigjoiner_pipes(display, &primary_bigjoiner_pipes,
+				&secondary_bigjoiner_pipes);
+
+	*primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes;
+
+	*secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes;
 
 	/* Joiner pipes should always be consecutive primary and secondary */
 	drm_WARN(&dev_priv->drm, *secondary_pipes != *primary_pipes << 1,
-- 
2.45.2

[PATCH 12/19] drm/i915: Add bigjoiner and uncompressed joiner hw readout sanity checks

[Ankit Nautiyal]

From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

Adding sanity checks for primary and secondary bigjoiner/uncompressed
bitmasks, should make it easier to spot possible issues.

v2:
-Streamline the expected masks and add few more drm_WARNs. (Ville)
-Use %#x format specifier for printing joiner masks. (Ville)
-Use struct intel_display instead of struct drm_i915_private. (Ankit)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> (v1)
---
 drivers/gpu/drm/i915/display/intel_display.c | 49 +++++++++++++++++++-
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e93af02aa859..b66685a9843f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3642,12 +3642,33 @@ static void enabled_bigjoiner_pipes(struct intel_display *display,
 	}
 }
 
+static u8 expected_secondary_pipes(u8 primary_pipes, int num_pipes)
+{
+	u8 secondary_pipes = 0;
+
+	for (int i = 1; i < num_pipes; i++)
+		secondary_pipes |= primary_pipes << i;
+
+	return secondary_pipes;
+}
+
+static u8 expected_uncompjoiner_secondary_pipes(u8 uncompjoiner_primary_pipes)
+{
+	return expected_secondary_pipes(uncompjoiner_primary_pipes, 2);
+}
+
+static u8 expected_bigjoiner_secondary_pipes(u8 bigjoiner_primary_pipes)
+{
+	return expected_secondary_pipes(bigjoiner_primary_pipes, 2);
+}
+
 static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 				 u8 *primary_pipes, u8 *secondary_pipes)
 {
 	struct intel_display *display = to_intel_display(&dev_priv->drm);
 	u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes;
 	u8 secondary_uncompressed_joiner_pipes, secondary_bigjoiner_pipes;
+	u8 uncompressed_joiner_pipes, bigjoiner_pipes;
 
 	if (!intel_display_can_use_joiner(display))
 		return;
@@ -3655,16 +3676,40 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 	enabled_uncompressed_joiner_pipes(display, &primary_uncompressed_joiner_pipes,
 					  &secondary_uncompressed_joiner_pipes);
 
+	drm_WARN_ON(display->drm,
+		    (primary_uncompressed_joiner_pipes & secondary_uncompressed_joiner_pipes) != 0);
+
 	enabled_bigjoiner_pipes(display, &primary_bigjoiner_pipes,
 				&secondary_bigjoiner_pipes);
 
+	drm_WARN_ON(display->drm,
+		    (primary_bigjoiner_pipes & secondary_bigjoiner_pipes) != 0);
+
+	uncompressed_joiner_pipes = primary_uncompressed_joiner_pipes |
+				    secondary_uncompressed_joiner_pipes;
+	bigjoiner_pipes = primary_bigjoiner_pipes | secondary_bigjoiner_pipes;
+
+	drm_WARN(display->drm, (uncompressed_joiner_pipes & bigjoiner_pipes) != 0,
+		 "Uncomressed joiner pipes(%#x) and bigjoiner pipes(%#x) can't intersect\n",
+		 uncompressed_joiner_pipes, bigjoiner_pipes);
+	drm_WARN(display->drm, secondary_bigjoiner_pipes !=
+		 expected_bigjoiner_secondary_pipes(primary_bigjoiner_pipes),
+		 "Wrong secondary bigjoiner pipes(expected %#x, current %#x)\n",
+		 expected_bigjoiner_secondary_pipes(primary_bigjoiner_pipes),
+		 secondary_bigjoiner_pipes);
+	drm_WARN(display->drm, secondary_uncompressed_joiner_pipes !=
+		 expected_uncompjoiner_secondary_pipes(primary_uncompressed_joiner_pipes),
+		 "Wrong secondary uncompressed joiner pipes(expected %#x, current %#x)\n",
+		 expected_uncompjoiner_secondary_pipes(primary_uncompressed_joiner_pipes),
+		 secondary_uncompressed_joiner_pipes);
+
 	*primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes;
 
 	*secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes;
 
 	/* Joiner pipes should always be consecutive primary and secondary */
-	drm_WARN(&dev_priv->drm, *secondary_pipes != *primary_pipes << 1,
-		 "Joiner misconfigured (primary pipes 0x%x, secondary pipes 0x%x)\n",
+	drm_WARN(display->drm, *secondary_pipes != *primary_pipes << 1,
+		 "Joiner misconfigured (primary pipes 0x%#x, secondary pipes 0x%#x)\n",
 		 *primary_pipes, *secondary_pipes);
 }
 
-- 
2.45.2

[PATCH 13/19] drm/i915: Implement hw state readout and checks for ultrajoiner

[Ankit Nautiyal]

From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

Ultrajoiner mode has some new bits and states to be
read out from the hw. Lets make changes accordingly.

v2: Fix checkpatch warnings. (Ankit)
v3: Add separate functions for computing expected secondary_big/ultrajoiner
pipes. (Ankit)
v4: Streamline the helpers for ultrajoiner. (Ville)
-Add fixup to accommodate PIPED check for ultrajoiner. (Ville)
-Add more Ultrajoiner drm_WARNs. (Ville)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  | 91 ++++++++++++++++++-
 .../gpu/drm/i915/display/intel_vdsc_regs.h    |  3 +
 2 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b66685a9843f..02926a8ef7c5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3662,13 +3662,68 @@ static u8 expected_bigjoiner_secondary_pipes(u8 bigjoiner_primary_pipes)
 	return expected_secondary_pipes(bigjoiner_primary_pipes, 2);
 }
 
+static u8 expected_ultrajoiner_secondary_pipes(u8 ultrajoiner_primary_pipes)
+{
+	return expected_secondary_pipes(ultrajoiner_primary_pipes, 4);
+}
+
+static u8 fixup_ultrajoiner_secondary_pipes(u8 ultrajoiner_primary_pipes,
+					    u8 ultrajoiner_secondary_pipes)
+{
+	return ultrajoiner_secondary_pipes | ultrajoiner_primary_pipes << 3;
+}
+
+static
+bool intel_display_can_use_ultrajoiner(struct intel_display *display)
+{
+	struct drm_i915_private *i915 = to_i915(display->drm);
+
+	return ((DISPLAY_VER(display) == 14 && IS_DGFX(i915)) ||
+		DISPLAY_VER(display) > 14);
+}
+
+static void enabled_ultrajoiner_pipes(struct drm_i915_private *i915,
+				      u8 *primary_pipes, u8 *secondary_pipes)
+{
+	struct intel_display *display = to_intel_display(&i915->drm);
+	struct intel_crtc *crtc;
+
+	*primary_pipes = 0;
+	*secondary_pipes = 0;
+
+	if (!intel_display_can_use_ultrajoiner(display))
+		return;
+
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc,
+					 joiner_pipes(i915)) {
+		enum intel_display_power_domain power_domain;
+		enum pipe pipe = crtc->pipe;
+		intel_wakeref_t wakeref;
+
+		power_domain = intel_dsc_power_domain(crtc, (enum transcoder)pipe);
+		with_intel_display_power_if_enabled(i915, power_domain, wakeref) {
+			u32 tmp = intel_de_read(i915, ICL_PIPE_DSS_CTL1(pipe));
+
+			if (!(tmp & ULTRA_JOINER_ENABLE))
+				continue;
+
+			if (tmp & PRIMARY_ULTRA_JOINER_ENABLE)
+				*primary_pipes |= BIT(pipe);
+			else
+				*secondary_pipes |= BIT(pipe);
+		}
+	}
+}
+
 static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 				 u8 *primary_pipes, u8 *secondary_pipes)
 {
 	struct intel_display *display = to_intel_display(&dev_priv->drm);
 	u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes;
+	u8 primary_ultrajoiner_pipes;
 	u8 secondary_uncompressed_joiner_pipes, secondary_bigjoiner_pipes;
-	u8 uncompressed_joiner_pipes, bigjoiner_pipes;
+	u8 secondary_ultrajoiner_pipes;
+	u8 uncompressed_joiner_pipes, bigjoiner_pipes, ultrajoiner_pipes;
 
 	if (!intel_display_can_use_joiner(display))
 		return;
@@ -3685,13 +3740,33 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 	drm_WARN_ON(display->drm,
 		    (primary_bigjoiner_pipes & secondary_bigjoiner_pipes) != 0);
 
+	enabled_ultrajoiner_pipes(dev_priv, &primary_ultrajoiner_pipes,
+				  &secondary_ultrajoiner_pipes);
+	/*
+	 * For some strange reason the last pipe in the set of four
+	 * shouldn't have ultrajoiner enable bit set in hardware.
+	 * Set the bit anyway to make life easier.
+	 */
+	drm_WARN_ON(&dev_priv->drm,
+		    expected_secondary_pipes(primary_ultrajoiner_pipes, 3) !=
+		    secondary_ultrajoiner_pipes);
+	secondary_ultrajoiner_pipes =
+		fixup_ultrajoiner_secondary_pipes(primary_ultrajoiner_pipes,
+						  secondary_ultrajoiner_pipes);
+
+	drm_WARN_ON(&dev_priv->drm, (primary_ultrajoiner_pipes & secondary_ultrajoiner_pipes) != 0);
+
 	uncompressed_joiner_pipes = primary_uncompressed_joiner_pipes |
 				    secondary_uncompressed_joiner_pipes;
 	bigjoiner_pipes = primary_bigjoiner_pipes | secondary_bigjoiner_pipes;
+	ultrajoiner_pipes = primary_ultrajoiner_pipes | secondary_ultrajoiner_pipes;
 
 	drm_WARN(display->drm, (uncompressed_joiner_pipes & bigjoiner_pipes) != 0,
 		 "Uncomressed joiner pipes(%#x) and bigjoiner pipes(%#x) can't intersect\n",
 		 uncompressed_joiner_pipes, bigjoiner_pipes);
+	drm_WARN(display->drm, (ultrajoiner_pipes & bigjoiner_pipes) != ultrajoiner_pipes,
+		 "Ultrajoiner pipes(%#x) should be bigjoiner pipes(%#x)\n",
+		 ultrajoiner_pipes, bigjoiner_pipes);
 	drm_WARN(display->drm, secondary_bigjoiner_pipes !=
 		 expected_bigjoiner_secondary_pipes(primary_bigjoiner_pipes),
 		 "Wrong secondary bigjoiner pipes(expected %#x, current %#x)\n",
@@ -3702,10 +3777,16 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 		 "Wrong secondary uncompressed joiner pipes(expected %#x, current %#x)\n",
 		 expected_uncompjoiner_secondary_pipes(primary_uncompressed_joiner_pipes),
 		 secondary_uncompressed_joiner_pipes);
-
-	*primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes;
-
-	*secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes;
+	drm_WARN(display->drm, secondary_ultrajoiner_pipes !=
+		 expected_ultrajoiner_secondary_pipes(primary_ultrajoiner_pipes),
+		 "Wrong secondary ultrajoiner pipes(expected %x, current %x)\n",
+		 expected_ultrajoiner_secondary_pipes(primary_ultrajoiner_pipes),
+		 secondary_ultrajoiner_pipes);
+
+	*primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes |
+			 primary_ultrajoiner_pipes;
+	*secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes |
+			 secondary_ultrajoiner_pipes;
 
 	/* Joiner pipes should always be consecutive primary and secondary */
 	drm_WARN(display->drm, *secondary_pipes != *primary_pipes << 1,
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc_regs.h b/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
index f921ad67b587..db07c9775892 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
@@ -37,6 +37,9 @@
 #define  SPLITTER_CONFIGURATION_MASK		REG_GENMASK(26, 25)
 #define  SPLITTER_CONFIGURATION_2_SEGMENT	REG_FIELD_PREP(SPLITTER_CONFIGURATION_MASK, 0)
 #define  SPLITTER_CONFIGURATION_4_SEGMENT	REG_FIELD_PREP(SPLITTER_CONFIGURATION_MASK, 1)
+
+#define  ULTRA_JOINER_ENABLE			REG_BIT(23)
+#define  PRIMARY_ULTRA_JOINER_ENABLE		REG_BIT(22)
 #define  UNCOMPRESSED_JOINER_PRIMARY		(1 << 21)
 #define  UNCOMPRESSED_JOINER_SECONDARY		(1 << 20)
 
-- 
2.45.2

[PATCH 14/19] drm/i915/display: Percolate ultrajoiner info to get_joiner_config

[Ankit Nautiyal]

From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

Modify the helpers get_primary/secondary_pipes to account for
ultrajoiner usage. Use the modified helpers to retrieve ultrajoiner
information in get_joiner_config.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 57 +++++++++++++-------
 1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 02926a8ef7c5..24698d8ed5d6 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3716,7 +3716,8 @@ static void enabled_ultrajoiner_pipes(struct drm_i915_private *i915,
 }
 
 static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
-				 u8 *primary_pipes, u8 *secondary_pipes)
+				 u8 *primary_pipes, u8 *secondary_pipes,
+				 bool *ultrajoiner_used)
 {
 	struct intel_display *display = to_intel_display(&dev_priv->drm);
 	u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes;
@@ -3760,6 +3761,7 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 				    secondary_uncompressed_joiner_pipes;
 	bigjoiner_pipes = primary_bigjoiner_pipes | secondary_bigjoiner_pipes;
 	ultrajoiner_pipes = primary_ultrajoiner_pipes | secondary_ultrajoiner_pipes;
+	*ultrajoiner_used = ultrajoiner_pipes != 0;
 
 	drm_WARN(display->drm, (uncompressed_joiner_pipes & bigjoiner_pipes) != 0,
 		 "Uncomressed joiner pipes(%#x) and bigjoiner pipes(%#x) can't intersect\n",
@@ -3794,7 +3796,8 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 		 *primary_pipes, *secondary_pipes);
 }
 
-static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
+static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes,
+					 u8 secondary_pipes, bool ultrajoiner_used)
 {
 	if ((secondary_pipes & BIT(pipe)) == 0)
 		return pipe;
@@ -3802,27 +3805,37 @@ static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 se
 	/* ignore everything above our pipe */
 	primary_pipes &= ~GENMASK(7, pipe);
 
-	/* highest remaining bit should be our primary pipe */
-	return fls(primary_pipes) - 1;
+	if (!ultrajoiner_used)
+		/* highest remaining bit should be our master pipe */
+		return fls(primary_pipes) - 1;
+
+	/* lowest remaining bit should be our primary master pipe */
+	return ffs(primary_pipes) - 1;
 }
 
-static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
+static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes,
+				     u8 secondary_pipes, bool ultrajoiner_used)
 {
 	enum pipe primary_pipe, next_primary_pipe;
 
-	primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes);
+	primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes,
+					       secondary_pipes, ultrajoiner_used);
 
 	if ((primary_pipes & BIT(primary_pipe)) == 0)
 		return 0;
 
-	/* ignore our primary pipe and everything below it */
-	primary_pipes &= ~GENMASK(primary_pipe, 0);
-	/* make sure a high bit is set for the ffs() */
-	primary_pipes |= BIT(7);
-	/* lowest remaining bit should be the next primary pipe */
-	next_primary_pipe = ffs(primary_pipes) - 1;
+	if (!ultrajoiner_used) {
+		/* ignore our primary pipe and everything below it */
+		primary_pipes &= ~GENMASK(primary_pipe, 0);
+		/* make sure a high bit is set for the ffs() */
+		primary_pipes |= BIT(7);
+		/* lowest remaining bit should be the next primary pipe */
+		next_primary_pipe = ffs(primary_pipes) - 1;
 
-	return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe);
+		return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe);
+	} else {
+		return (secondary_pipes | primary_pipes) & ~BIT(primary_pipe);
+	}
 }
 
 static u8 hsw_panel_transcoders(struct drm_i915_private *i915)
@@ -3843,6 +3856,7 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc)
 	enum transcoder cpu_transcoder;
 	u8 primary_pipes, secondary_pipes;
 	u8 enabled_transcoders = 0;
+	bool ultrajoiner_used;
 
 	/*
 	 * XXX: Do intel_display_power_get_if_enabled before reading this (for
@@ -3893,11 +3907,12 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc)
 	if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder))
 		enabled_transcoders |= BIT(cpu_transcoder);
 
-	/* joiner secondary -> consider the primary pipe's transcoder as well */
-	enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes);
+	/* joiner slave -> consider the master pipe's transcoder as well */
+	enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes, &ultrajoiner_used);
 	if (secondary_pipes & BIT(crtc->pipe)) {
 		cpu_transcoder = (enum transcoder)
-			get_joiner_primary_pipe(crtc->pipe, primary_pipes, secondary_pipes);
+			get_joiner_primary_pipe(crtc->pipe, primary_pipes,
+						secondary_pipes, ultrajoiner_used);
 		if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder))
 			enabled_transcoders |= BIT(cpu_transcoder);
 	}
@@ -4029,16 +4044,18 @@ static void intel_joiner_get_config(struct intel_crtc_state *crtc_state)
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
 	u8 primary_pipes, secondary_pipes;
+	bool ultrajoiner_used;
 	enum pipe pipe = crtc->pipe;
 
-	enabled_joiner_pipes(i915, &primary_pipes, &secondary_pipes);
+	enabled_joiner_pipes(i915, &primary_pipes, &secondary_pipes, &ultrajoiner_used);
 
 	if (((primary_pipes | secondary_pipes) & BIT(pipe)) == 0)
 		return;
 
-	crtc_state->joiner_pipes =
-		BIT(get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes)) |
-		get_joiner_secondary_pipes(pipe, primary_pipes, secondary_pipes);
+	crtc_state->joiner_pipes = BIT(get_joiner_primary_pipe(pipe, primary_pipes,
+							       secondary_pipes, ultrajoiner_used)) |
+				   get_joiner_secondary_pipes(pipe, primary_pipes,
+							      secondary_pipes, ultrajoiner_used);
 }
 
 static bool hsw_get_pipe_config(struct intel_crtc *crtc,
-- 
2.45.2

[PATCH 15/19] drm/i915/display/vdsc: Add ultrajoiner support with DSC

[Ankit Nautiyal]

From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

Add changes to DSC which are required for Ultrajoiner.

v2:
-Use correct helper for setting bits for bigjoiner secondary. (Ankit)
-Use enum for joiner pipe count instead magic numbers. (Suraj)
-Use primary/secondary instead of master/slave. (Suraj)

v3:
-Add the ultrajoiner helpers and use it for setting ultrajoiner bits
(Ankit)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++
 drivers/gpu/drm/i915/display/intel_display.h |  2 ++
 drivers/gpu/drm/i915/display/intel_vdsc.c    | 26 ++++++++++++++++++--
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 24698d8ed5d6..db27850b2c36 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -295,6 +295,27 @@ bool intel_crtc_is_bigjoiner_secondary(const struct intel_crtc_state *crtc_state
 	return BIT(crtc->pipe) & bigjoiner_secondary_pipes(crtc_state);
 }
 
+bool intel_crtc_is_ultrajoiner(const struct intel_crtc_state *crtc_state)
+{
+	return intel_joiner_num_pipes(crtc_state) >= 4;
+}
+
+static u8 ultrajoiner_primary_pipes(const struct intel_crtc_state *crtc_state)
+{
+	if (!intel_crtc_is_ultrajoiner(crtc_state))
+		return 0;
+
+	return crtc_state->joiner_pipes & (0b00010001 << joiner_primary_pipe(crtc_state));
+}
+
+bool intel_crtc_is_ultrajoiner_primary(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+	return intel_crtc_is_ultrajoiner(crtc_state) &&
+	       BIT(crtc->pipe) & ultrajoiner_primary_pipes(crtc_state);
+}
+
 u8 intel_crtc_joiner_secondary_pipes(const struct intel_crtc_state *crtc_state)
 {
 	if (crtc_state->joiner_pipes)
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 17d8b9df3916..dbbe23ea14fc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -427,6 +427,8 @@ bool intel_crtc_is_joiner_secondary(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_is_joiner_primary(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_is_bigjoiner_primary(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_is_bigjoiner_secondary(const struct intel_crtc_state *crtc_state);
+bool intel_crtc_is_ultrajoiner(const struct intel_crtc_state *crtc_state);
+bool intel_crtc_is_ultrajoiner_primary(const struct intel_crtc_state *crtc_state);
 u8 intel_crtc_joiner_secondary_pipes(const struct intel_crtc_state *crtc_state);
 struct intel_crtc *intel_primary_crtc(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
index 8158e3702ed5..b95dd7a7111e 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -379,9 +379,11 @@ static int intel_dsc_get_vdsc_per_pipe(const struct intel_crtc_state *crtc_state
 int intel_dsc_get_num_vdsc_instances(const struct intel_crtc_state *crtc_state)
 {
 	int num_vdsc_instances = intel_dsc_get_vdsc_per_pipe(crtc_state);
+	int num_joined_pipes = intel_joiner_num_pipes(crtc_state);
 
-	if (crtc_state->joiner_pipes)
-		num_vdsc_instances *= 2;
+	if (num_joined_pipes == 2 ||
+	    num_joined_pipes == 4)
+		num_vdsc_instances *= num_joined_pipes;
 
 	return num_vdsc_instances;
 }
@@ -751,6 +753,14 @@ void intel_uncompressed_joiner_enable(const struct intel_crtc_state *crtc_state)
 	}
 }
 
+static bool intel_crtc_ultrajoiner_enable_needed(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+	return intel_crtc_is_ultrajoiner(crtc_state) &&
+	       crtc->pipe != PIPE_D;
+}
+
 void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -770,7 +780,19 @@ void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
 		dss_ctl1_val |= JOINER_ENABLE;
 	}
 	if (crtc_state->joiner_pipes) {
+		/*
+		 * This bit doesn't seem to follow primary/secondary logic or
+		 * any other logic, so lets just add helper function to
+		 * at least hide this hassle..
+		 */
+		if (intel_crtc_ultrajoiner_enable_needed(crtc_state))
+			dss_ctl1_val |= ULTRA_JOINER_ENABLE;
+
+		if (intel_crtc_is_ultrajoiner_primary(crtc_state))
+			dss_ctl1_val |= PRIMARY_ULTRA_JOINER_ENABLE;
+
 		dss_ctl1_val |= BIG_JOINER_ENABLE;
+
 		if (intel_crtc_is_bigjoiner_primary(crtc_state))
 			dss_ctl1_val |= PRIMARY_BIG_JOINER_ENABLE;
 	}
-- 
2.45.2

[PATCH 16/19] drm/i915: Add new abstraction layer to handle pipe order for different joiners

[Ankit Nautiyal]

From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

Ultrajoiner case requires special treatment where both reverse and
staight order iteration doesn't work(for instance disabling case requires
order to be: primary master, slaves, secondary master).

Lets unify our approach by using not only pipe masks for iterating required
pipes based on joiner type used, but also using different "priority" arrays
for each of those.

v2: Fix checkpatch warnings. (Ankit)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c     | 19 +++--
 drivers/gpu/drm/i915/display/intel_display.c | 83 ++++++++++++++++----
 drivers/gpu/drm/i915/display/intel_display.h |  7 ++
 drivers/gpu/drm/i915/display/intel_dp_mst.c  | 18 +++--
 4 files changed, 96 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 00fbe9f8c03a..2c064b6c6d01 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3116,10 +3116,11 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
 					       const struct drm_connector_state *old_conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct intel_crtc *pipe_crtc;
+	struct intel_crtc *pipe_crtc; enum pipe pipe;
 
-	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
-					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
+	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(old_crtc_state),
+					     intel_get_pipe_order_disable(old_crtc_state)) {
 		const struct intel_crtc_state *old_pipe_crtc_state =
 			intel_atomic_get_old_crtc_state(state, pipe_crtc);
 
@@ -3130,8 +3131,9 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
 
 	intel_ddi_disable_transcoder_func(old_crtc_state);
 
-	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
-					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
+	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(old_crtc_state),
+					     intel_get_pipe_order_disable(old_crtc_state)) {
 		const struct intel_crtc_state *old_pipe_crtc_state =
 			intel_atomic_get_old_crtc_state(state, pipe_crtc);
 
@@ -3383,7 +3385,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
 			     const struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
-	struct intel_crtc *pipe_crtc;
+	struct intel_crtc *pipe_crtc; enum pipe pipe;
 
 	intel_ddi_enable_transcoder_func(encoder, crtc_state);
 
@@ -3394,8 +3396,9 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
 
 	intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
 
-	for_each_intel_crtc_in_pipe_mask_reverse(&i915->drm, pipe_crtc,
-						 intel_crtc_joined_pipe_mask(crtc_state)) {
+	for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(crtc_state),
+					     intel_get_pipe_order_enable(crtc_state)) {
 		const struct intel_crtc_state *pipe_crtc_state =
 			intel_atomic_get_new_crtc_state(state, pipe_crtc);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index db27850b2c36..27622d51a473 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1737,6 +1737,50 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
 	hsw_set_transconf(crtc_state);
 }
 
+static
+bool intel_crtc_is_bigjoiner(const struct intel_crtc_state *pipe_config)
+{
+	return hweight8(pipe_config->joiner_pipes) == 2;
+}
+
+const enum pipe *intel_get_pipe_order_enable(const struct intel_crtc_state *crtc_state)
+{
+	static const enum pipe ultrajoiner_pipe_order_enable[I915_MAX_PIPES] = {
+		PIPE_B, PIPE_D, PIPE_C, PIPE_A
+	};
+	static const enum pipe bigjoiner_pipe_order_enable[I915_MAX_PIPES] = {
+		PIPE_B, PIPE_A, PIPE_D, PIPE_C
+	};
+	static const enum pipe nojoiner_pipe_order_enable[I915_MAX_PIPES] = {
+		PIPE_A, PIPE_B, PIPE_C, PIPE_D
+	};
+
+	if (intel_crtc_is_ultrajoiner(crtc_state))
+		return ultrajoiner_pipe_order_enable;
+	else if (intel_crtc_is_bigjoiner(crtc_state))
+		return bigjoiner_pipe_order_enable;
+	return nojoiner_pipe_order_enable;
+}
+
+const enum pipe *intel_get_pipe_order_disable(const struct intel_crtc_state *crtc_state)
+{
+	static const enum pipe ultrajoiner_pipe_order_disable[I915_MAX_PIPES] = {
+		PIPE_A, PIPE_B, PIPE_D, PIPE_C
+	};
+	static const enum pipe bigjoiner_pipe_order_disable[I915_MAX_PIPES] = {
+		PIPE_A, PIPE_B, PIPE_C, PIPE_D
+	};
+	static const enum pipe nojoiner_pipe_order_disable[I915_MAX_PIPES] = {
+		PIPE_A, PIPE_B, PIPE_C, PIPE_D
+	};
+
+	if (intel_crtc_is_ultrajoiner(crtc_state))
+		return ultrajoiner_pipe_order_disable;
+	else if (intel_crtc_is_bigjoiner(crtc_state))
+		return bigjoiner_pipe_order_disable;
+	return nojoiner_pipe_order_disable;
+}
+
 static void hsw_crtc_enable(struct intel_atomic_state *state,
 			    struct intel_crtc *crtc)
 {
@@ -1745,19 +1789,21 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder;
-	struct intel_crtc *pipe_crtc;
+	struct intel_crtc *pipe_crtc; enum pipe pipe;
 
 	if (drm_WARN_ON(&dev_priv->drm, crtc->active))
 		return;
 
-	for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
-						 intel_crtc_joined_pipe_mask(new_crtc_state))
+	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(new_crtc_state),
+					     intel_get_pipe_order_enable(new_crtc_state))
 		intel_dmc_enable_pipe(dev_priv, pipe_crtc->pipe);
 
 	intel_encoders_pre_pll_enable(state, crtc);
 
-	for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
-						 intel_crtc_joined_pipe_mask(new_crtc_state)) {
+	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(new_crtc_state),
+					     intel_get_pipe_order_enable(new_crtc_state)) {
 		const struct intel_crtc_state *pipe_crtc_state =
 			intel_atomic_get_new_crtc_state(state, pipe_crtc);
 
@@ -1767,8 +1813,9 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
 
 	intel_encoders_pre_enable(state, crtc);
 
-	for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
-						 intel_crtc_joined_pipe_mask(new_crtc_state)) {
+	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(new_crtc_state),
+					     intel_get_pipe_order_enable(new_crtc_state)) {
 		const struct intel_crtc_state *pipe_crtc_state =
 			intel_atomic_get_new_crtc_state(state, pipe_crtc);
 
@@ -1786,8 +1833,9 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
 	if (!transcoder_is_dsi(cpu_transcoder))
 		hsw_configure_cpu_transcoder(new_crtc_state);
 
-	for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
-						 intel_crtc_joined_pipe_mask(new_crtc_state)) {
+	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(new_crtc_state),
+					     intel_get_pipe_order_enable(new_crtc_state)) {
 		const struct intel_crtc_state *pipe_crtc_state =
 			intel_atomic_get_new_crtc_state(state, pipe_crtc);
 
@@ -1822,8 +1870,9 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
 
 	intel_encoders_enable(state, crtc);
 
-	for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
-						 intel_crtc_joined_pipe_mask(new_crtc_state)) {
+	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(new_crtc_state),
+					     intel_get_pipe_order_enable(new_crtc_state)) {
 		const struct intel_crtc_state *pipe_crtc_state =
 			intel_atomic_get_new_crtc_state(state, pipe_crtc);
 		enum pipe hsw_workaround_pipe;
@@ -1908,7 +1957,7 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
 	const struct intel_crtc_state *old_crtc_state =
 		intel_atomic_get_old_crtc_state(state, crtc);
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
-	struct intel_crtc *pipe_crtc;
+	struct intel_crtc *pipe_crtc; enum pipe pipe;
 
 	/*
 	 * FIXME collapse everything to one hook.
@@ -1917,8 +1966,9 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
 	intel_encoders_disable(state, crtc);
 	intel_encoders_post_disable(state, crtc);
 
-	for_each_intel_crtc_in_pipe_mask(&i915->drm, pipe_crtc,
-					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
+	for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(old_crtc_state),
+					     intel_get_pipe_order_disable(old_crtc_state)) {
 		const struct intel_crtc_state *old_pipe_crtc_state =
 			intel_atomic_get_old_crtc_state(state, pipe_crtc);
 
@@ -1927,8 +1977,9 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
 
 	intel_encoders_post_pll_disable(state, crtc);
 
-	for_each_intel_crtc_in_pipe_mask(&i915->drm, pipe_crtc,
-					 intel_crtc_joined_pipe_mask(old_crtc_state))
+	for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(old_crtc_state),
+					     intel_get_pipe_order_disable(old_crtc_state))
 		intel_dmc_disable_pipe(i915, pipe_crtc->pipe);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index dbbe23ea14fc..72dc495c645c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -274,6 +274,11 @@ enum phy_fia {
 			    &(dev)->mode_config.crtc_list,		\
 			    base.head)
 
+#define for_each_intel_crtc_in_mask_priority(__dev_priv, intel_crtc, __p, __mask, __priolist) \
+	for_each_pipe(__dev_priv, __p) \
+		for_each_if((__mask) & BIT(__priolist[__p])) \
+			for_each_if(intel_crtc = intel_crtc_for_pipe(to_intel_display(&__dev_priv->drm), __priolist[__p]))
+
 #define for_each_intel_crtc_in_pipe_mask(dev, intel_crtc, pipe_mask)	\
 	list_for_each_entry(intel_crtc,					\
 			    &(dev)->mode_config.crtc_list,		\
@@ -431,6 +436,8 @@ bool intel_crtc_is_ultrajoiner(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_is_ultrajoiner_primary(const struct intel_crtc_state *crtc_state);
 u8 intel_crtc_joiner_secondary_pipes(const struct intel_crtc_state *crtc_state);
 struct intel_crtc *intel_primary_crtc(const struct intel_crtc_state *crtc_state);
+const enum pipe *intel_get_pipe_order_enable(const struct intel_crtc_state *crtc_state);
+const enum pipe *intel_get_pipe_order_disable(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state);
 bool intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 			       const struct intel_crtc_state *pipe_config,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index be79783ce09b..1c87f81568c8 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -1003,7 +1003,7 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
 	struct drm_dp_mst_atomic_payload *new_payload =
 		drm_atomic_get_mst_payload_state(new_mst_state, connector->port);
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
-	struct intel_crtc *pipe_crtc;
+	struct intel_crtc *pipe_crtc; enum pipe pipe;
 	bool last_mst_stream;
 
 	intel_dp->active_mst_links--;
@@ -1012,8 +1012,9 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
 		    DISPLAY_VER(dev_priv) >= 12 && last_mst_stream &&
 		    !intel_dp_mst_is_master_trans(old_crtc_state));
 
-	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
-					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
+	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(old_crtc_state),
+					     intel_get_pipe_order_disable(old_crtc_state)) {
 		const struct intel_crtc_state *old_pipe_crtc_state =
 			intel_atomic_get_old_crtc_state(state, pipe_crtc);
 
@@ -1037,8 +1038,9 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
 
 	intel_ddi_disable_transcoder_func(old_crtc_state);
 
-	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
-					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
+	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(old_crtc_state),
+					     intel_get_pipe_order_disable(old_crtc_state)) {
 		const struct intel_crtc_state *old_pipe_crtc_state =
 			intel_atomic_get_old_crtc_state(state, pipe_crtc);
 
@@ -1257,6 +1259,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
 	enum transcoder trans = pipe_config->cpu_transcoder;
 	bool first_mst_stream = intel_dp->active_mst_links == 1;
 	struct intel_crtc *pipe_crtc;
+	enum pipe pipe;
 	int ret;
 
 	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
@@ -1304,8 +1307,9 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
 
 	intel_enable_transcoder(pipe_config);
 
-	for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
-						 intel_crtc_joined_pipe_mask(pipe_config)) {
+	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
+					     intel_crtc_joined_pipe_mask(pipe_config),
+					     intel_get_pipe_order_enable(pipe_config)) {
 		const struct intel_crtc_state *pipe_crtc_state =
 			intel_atomic_get_new_crtc_state(state, pipe_crtc);
 
-- 
2.45.2

[PATCH 17/19] drm/i915: Compute config and mode valid changes for ultrajoiner

[Ankit Nautiyal]

From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

Implement required changes for mode validation and compute config,
to support Ultrajoiner.
This also includes required DSC changes and checks.

v2:
-Use enum for Ultrajoiner pipes.
-Drop changes for HDMI.
-Separate out DSC changes into another patch.

v3:
-Fix check in can_ultrajoiner. (Ankit)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  1 -
 drivers/gpu/drm/i915/display/intel_display.h |  3 +
 drivers/gpu/drm/i915/display/intel_dp.c      | 59 ++++++++++++++++----
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |  8 ++-
 4 files changed, 58 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 27622d51a473..bab336d8702f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3745,7 +3745,6 @@ static u8 fixup_ultrajoiner_secondary_pipes(u8 ultrajoiner_primary_pipes,
 	return ultrajoiner_secondary_pipes | ultrajoiner_primary_pipes << 3;
 }
 
-static
 bool intel_display_can_use_ultrajoiner(struct intel_display *display)
 {
 	struct drm_i915_private *i915 = to_i915(display->drm);
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 72dc495c645c..6aa3aac8c927 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -52,6 +52,7 @@ struct intel_atomic_state;
 struct intel_crtc;
 struct intel_crtc_state;
 struct intel_digital_port;
+struct intel_display;
 struct intel_dp;
 struct intel_encoder;
 struct intel_initial_plane_config;
@@ -599,4 +600,6 @@ bool assert_port_valid(struct drm_i915_private *i915, enum port port);
 bool intel_scanout_needs_vtd_wa(struct drm_i915_private *i915);
 int intel_joiner_num_pipes(const struct intel_crtc_state *crtc_state);
 
+bool intel_display_can_use_ultrajoiner(struct intel_display *display);
+
 #endif
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 4b05a717cce3..9160614d74c6 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -862,24 +862,34 @@ u32 get_max_compressed_bpp_with_joiner(struct drm_i915_private *i915,
 				       int num_joined_pipes)
 {
 	u32 max_bpp_small_joiner_ram;
+	u32 max_bpp_joiner;
 
 	/* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
 	max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) / mode_hdisplay;
+	max_bpp_joiner = max_bpp_small_joiner_ram;
 
-	if (num_joined_pipes == 2) {
-		int bigjoiner_interface_bits = DISPLAY_VER(i915) >= 14 ? 36 : 24;
+	/* if ultra joiner is enabled, we have 2 bigjoiners enabled */
+	if (num_joined_pipes == 2 ||
+	    num_joined_pipes == 4) {
+		int joiner_interface_bits = DISPLAY_VER(i915) >= 14 ? 36 : 24;
 		/* With bigjoiner multiple dsc engines are used in parallel so PPC is 2 */
 		int ppc = 2;
-		u32 max_bpp_bigjoiner =
-			i915->display.cdclk.max_cdclk_freq * ppc * bigjoiner_interface_bits /
+		max_bpp_joiner =
+			i915->display.cdclk.max_cdclk_freq * ppc * joiner_interface_bits /
 			intel_dp_mode_to_fec_clock(mode_clock);
 
 		max_bpp_small_joiner_ram *= 2;
 
-		return min(max_bpp_small_joiner_ram, max_bpp_bigjoiner);
 	}
+	if (num_joined_pipes == 4) {
+		/* TODO: Check for ultrajoiner ram constraints */
 
-	return max_bpp_small_joiner_ram;
+		/* both get multiplied by 2, because ram bits/ppc now doubled */
+		max_bpp_small_joiner_ram *= 2;
+		max_bpp_joiner *= 2;
+	}
+
+	return min(max_bpp_small_joiner_ram, max_bpp_joiner);
 }
 
 u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
@@ -985,6 +995,10 @@ u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
 		if (num_pipes == 2 && test_slice_count < 4)
 			continue;
 
+		/* ultrajoiner needs 2 bigjoiners to be enabled */
+		if (num_pipes == 4 && test_slice_count < 8)
+			continue;
+
 		if (min_slice_count <= test_slice_count)
 			return test_slice_count;
 	}
@@ -1261,6 +1275,19 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
 	return MODE_OK;
 }
 
+static
+bool intel_dp_needs_ultrajoiner(struct intel_dp *dp, int clock)
+{
+	const struct intel_encoder *encoder = &dp_to_dig_port(dp)->base;
+	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+	struct intel_display *display = to_intel_display(encoder);
+
+	if (!intel_display_can_use_ultrajoiner(display))
+		return false;
+
+	return clock > (i915->display.cdclk.max_dotclk_freq * 2);
+}
+
 static
 bool intel_dp_needs_bigjoiner(struct intel_dp *intel_dp,
 			      struct intel_connector *connector,
@@ -1275,6 +1302,8 @@ int intel_dp_compute_joiner_pipes(struct intel_dp *intel_dp,
 				  struct intel_connector *connector,
 				  int hdisplay, int clock)
 {
+	struct intel_display *display = to_intel_display(intel_dp);
+
 	switch (connector->force_joined_pipes) {
 	case 1:
 		fallthrough;
@@ -1284,8 +1313,11 @@ int intel_dp_compute_joiner_pipes(struct intel_dp *intel_dp,
 		MISSING_CASE(connector->force_joined_pipes);
 		fallthrough;
 	case 0:
-		if (intel_dp_has_joiner(intel_dp) &&
-		    intel_dp_needs_bigjoiner(intel_dp, connector, hdisplay, clock))
+		if (intel_display_can_use_ultrajoiner(display) &&
+		    intel_dp_needs_ultrajoiner(intel_dp, clock))
+			return 4;
+		else if (intel_dp_has_joiner(intel_dp) &&
+			 intel_dp_needs_bigjoiner(intel_dp, connector, hdisplay, clock))
 			return 2;
 	}
 
@@ -1353,6 +1385,8 @@ intel_dp_mode_valid(struct drm_connector *_connector,
 
 	if (num_joined_pipes == 2)
 		max_dotclk *= 2;
+	else if (num_joined_pipes == 4)
+		max_dotclk *= 4;
 
 	if (target_clock > max_dotclk)
 		return MODE_CLOCK_HIGH;
@@ -2527,8 +2561,10 @@ bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915,
 	 * Pipe joiner needs compression up to display 12 due to bandwidth
 	 * limitation. DG2 onwards pipe joiner can be enabled without
 	 * compression.
+	 * Ultrajoiner always needs compression.
 	 */
-	return DISPLAY_VER(i915) < 13 && (num_joined_pipes == 2);
+	return (DISPLAY_VER(i915) < 13 && (num_joined_pipes == 2)) ||
+		num_joined_pipes == 4;
 }
 
 static int
@@ -2556,7 +2592,10 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
 							 adjusted_mode->crtc_hdisplay,
 							 adjusted_mode->crtc_clock);
-	if (num_joined_pipes == 2)
+
+	if (num_joined_pipes == 4)
+		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 3, crtc->pipe);
+	else if (num_joined_pipes == 2)
 		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
 
 	joiner_needs_dsc = intel_dp_joiner_needs_dsc(i915, num_joined_pipes);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 1c87f81568c8..cfa30d5e26ca 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -583,7 +583,9 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
 							 adjusted_mode->crtc_hdisplay,
 							 adjusted_mode->crtc_clock);
-	if (num_joined_pipes == 2)
+	if (num_joined_pipes == 4)
+		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 3, crtc->pipe);
+	else if (num_joined_pipes == 2)
 		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
 
 	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
@@ -1476,7 +1478,9 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
 	 */
 	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, intel_connector,
 							 mode->hdisplay, target_clock);
-	if (num_joined_pipes == 2)
+	if (num_joined_pipes == 4)
+		max_dotclk *= 4;
+	else if (num_joined_pipes == 2)
 		max_dotclk *= 2;
 
 	ret = drm_modeset_lock(&mgr->base.lock, ctx);
-- 
2.45.2

[PATCH 18/19] drm/i915/display: Consider ultrajoiner for computing maxdotclock

[Ankit Nautiyal]

Use the check for ultrajoiner while computing maxdotclock.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index bab336d8702f..709b55fc0e07 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8267,6 +8267,9 @@ static int max_dotclock(struct drm_i915_private *i915)
 	if (intel_display_can_use_joiner(display))
 		max_dotclock *= 2;
 
+	if (intel_display_can_use_ultrajoiner(display))
+		max_dotclock *= 2;
+
 	return max_dotclock;
 }
 
-- 
2.45.2

[PATCH 19/19] drm/i915/intel_dp: Add support for forcing ultrajoiner

[Ankit Nautiyal]

Allow forcing ultrajoiner through debugfs.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_debugfs.c | 6 ++++++
 drivers/gpu/drm/i915/display/intel_dp.c              | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 8b0cb750d9ae..5d94ce6060f6 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1554,6 +1554,12 @@ static ssize_t i915_joiner_write(struct file *file,
 		else
 			drm_dbg(display->drm, "Force joiner not supported for the config\n");
 		break;
+	case 4:
+		if (intel_display_can_use_ultrajoiner(display))
+			connector->force_joined_pipes = force_join_pipes;
+		else
+			drm_dbg(display->drm, "Force ultrajoiner not supported for the config\n");
+		break;
 	default:
 		drm_dbg(display->drm, "Ignoring Invalid num of pipes %d for force joining\n",
 			force_join_pipes);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 9160614d74c6..149fdcfb297d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1308,6 +1308,8 @@ int intel_dp_compute_joiner_pipes(struct intel_dp *intel_dp,
 	case 1:
 		fallthrough;
 	case 2:
+		fallthrough;
+	case 4:
 		return connector->force_joined_pipes;
 	default:
 		MISSING_CASE(connector->force_joined_pipes);
-- 
2.45.2

✗ Fi.CI.BUILD: failure for Ultrajoiner basic functionality series (rev8)

[Patchwork]

== Series Details ==

Series: Ultrajoiner basic functionality series (rev8)
URL   : https://patchwork.freedesktop.org/series/133800/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/133800/revisions/8/mbox/ not applied
Applying: drm/i915/display: Check whether platform supports joiner
Applying: drm/i915/display: Modify debugfs for joiner to force n pipes
Applying: drm/i915/display_debugfs: Allow force joiner only if supported
Applying: drm/i915/dp: Add helper to compute num pipes joined
Applying: drm/i915/display: Add debugfs support to avoid joiner
Applying: drm/i915/display: Simplify intel_joiner_num_pipes and its usage
Applying: drm/i915/display: Use joined pipes in intel_dp_joiner_needs_dsc
Applying: drm/i915/display: Use joined pipes in intel_mode_valid_max_plane_size
Applying: drm/i915/display: Use joined pipes in dsc helpers for slices, bpp
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/display/intel_display.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0009 drm/i915/display: Use joined pipes in dsc helpers for slices, bpp
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced

Re: [PATCH 03/19] drm/i915/display_debugfs: Allow force joiner only if supported

[Ville Syrjälä]

On Wed, Sep 11, 2024 at 06:43:33PM +0530, Ankit Nautiyal wrote:
> Currently joiner is only supported for DP encoder.
> Allow force joiner only for DP for supported platforms.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 20 ++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index d7651f6f80e1..7ceaf01a884a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1520,6 +1520,9 @@ static ssize_t i915_joiner_write(struct file *file,
>  	struct seq_file *m = file->private_data;
>  	struct intel_connector *connector = m->private;
>  	struct intel_display *display = to_intel_display(connector);
> +	struct intel_encoder *intel_encoder = intel_attached_encoder(connector);

That won't work for MST. You'll want to use intel_attached_dp().

> +	struct intel_dp *intel_dp;
> +	int connector_type = connector->base.connector_type;
>  	int force_join_pipes = 0;
>  	int ret;
>  
> @@ -1530,16 +1533,27 @@ static ssize_t i915_joiner_write(struct file *file,
>  	if (ret < 0)
>  		return ret;
>  
> +	/* Currently joiner is only supported for eDP/DP */
> +	if (connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
> +	    connector_type != DRM_MODE_CONNECTOR_eDP)
> +		return 0;

We shouldn't be adding the debugfs file to a non-DP connector so this
seems redundant.

> +
> +	intel_dp = enc_to_intel_dp(intel_encoder);
> +
> +	connector->force_joined_pipes = 0;
> +
>  	switch (force_join_pipes) {
>  	case 0:
> -		fallthrough;
> +		break;
>  	case 2:
> -		connector->force_joined_pipes = force_join_pipes;
> +		if (intel_dp_has_joiner(intel_dp))
> +			connector->force_joined_pipes = force_join_pipes;
> +		else
> +			drm_dbg(display->drm, "Force joiner not supported for the config\n");

That should return an error. Hmm, or perhaps even better to not
even create the debugfs file in the first place?

>  		break;
>  	default:
>  		drm_dbg(display->drm, "Ignoring Invalid num of pipes %d for force joining\n",
>  			force_join_pipes);
> -		connector->force_joined_pipes = 0;
>  	}
>  
>  	*offp += len;
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 04/19] drm/i915/dp: Add helper to compute num pipes joined

[Ville Syrjälä]

On Wed, Sep 11, 2024 at 06:43:34PM +0530, Ankit Nautiyal wrote:
> Add a helper to compute the number of pipes to be joined.
> The num of pipes joined will depend on whether the joiner is required or
> is forced through the debugfs.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c     | 46 +++++++++++++++------
>  drivers/gpu/drm/i915/display/intel_dp.h     |  6 +--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 14 ++++---
>  3 files changed, 46 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 5c30d4488141..333624fcebd7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1261,17 +1261,33 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
>  	return MODE_OK;
>  }
>  
> -bool intel_dp_need_joiner(struct intel_dp *intel_dp,
> -			  struct intel_connector *connector,
> -			  int hdisplay, int clock)
> +static
> +bool intel_dp_needs_bigjoiner(struct intel_dp *intel_dp,
> +			      struct intel_connector *connector,
> +			      int hdisplay, int clock)
>  {
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  
> -	if (!intel_dp_has_joiner(intel_dp))
> -		return false;
> +	return clock > i915->display.cdclk.max_dotclk_freq || hdisplay > 5120;
> +}
>  
> -	return clock > i915->display.cdclk.max_dotclk_freq || hdisplay > 5120 ||
> -	       connector->force_joined_pipes == 2;
> +int intel_dp_compute_joiner_pipes(struct intel_dp *intel_dp,
> +				  struct intel_connector *connector,
> +				  int hdisplay, int clock)

s/joiner_pipes/num_pipes/ would seem a more appropriate name
if we want to make i return 1 for the non-joiner cases...

> +{
> +	switch (connector->force_joined_pipes) {
> +	case 2:
> +		return connector->force_joined_pipes;
> +	default:
> +		MISSING_CASE(connector->force_joined_pipes);
> +		fallthrough;
> +	case 0:
> +		if (intel_dp_has_joiner(intel_dp) &&
> +		    intel_dp_needs_bigjoiner(intel_dp, connector, hdisplay, clock))
> +			return 2;
> +	}
> +
> +	return 0;

... which I thought we did, but here we are returning 0?

>  }
>  
>  bool intel_dp_has_dsc(const struct intel_connector *connector)
> @@ -1309,6 +1325,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  	u8 dsc_slice_count = 0;
>  	enum drm_mode_status status;
>  	bool dsc = false, joiner = false;
> +	int num_joined_pipes;
>  
>  	status = intel_cpu_transcoder_mode_valid(dev_priv, mode);
>  	if (status != MODE_OK)
> @@ -1329,11 +1346,14 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  		target_clock = fixed_mode->clock;
>  	}
>  
> -	if (intel_dp_need_joiner(intel_dp, connector,
> -				 mode->hdisplay, target_clock)) {
> +	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
> +							 mode->hdisplay, target_clock);
> +
> +	if (num_joined_pipes == 2) {
>  		joiner = true;
>  		max_dotclk *= 2;
>  	}

This could become nan unconditional
	max_dotclock *= num_pipes;
if we did the return 1 for non-joiner cases.

> +
>  	if (target_clock > max_dotclk)
>  		return MODE_CLOCK_HIGH;
>  
> @@ -2523,15 +2543,17 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  	struct link_config_limits limits;
>  	bool dsc_needed, joiner_needs_dsc;
> +	int num_joined_pipes;
>  	int ret = 0;
>  
>  	if (pipe_config->fec_enable &&
>  	    !intel_dp_supports_fec(intel_dp, connector, pipe_config))
>  		return -EINVAL;
>  
> -	if (intel_dp_need_joiner(intel_dp, connector,
> -				 adjusted_mode->crtc_hdisplay,
> -				 adjusted_mode->crtc_clock))
> +	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
> +							 adjusted_mode->crtc_hdisplay,
> +							 adjusted_mode->crtc_clock);
> +	if (num_joined_pipes == 2)
>  		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);

This could be a bit more generic with something like:
	if (num_pipes > 1)
		joiner_pipes = GENNMASK(pipe + num_pipes - 1, pipe);

But maybe you're doing some that stuff later. I'll keep reading...

>  
>  	joiner_needs_dsc = intel_dp_joiner_needs_dsc(i915, pipe_config->joiner_pipes);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 1b9aaddd8c35..bc9a82d82df2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -153,9 +153,9 @@ int intel_dp_dsc_sink_max_compressed_bpp(const struct intel_connector *connector
>  u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
>  				int mode_clock, int mode_hdisplay,
>  				bool bigjoiner);
> -bool intel_dp_need_joiner(struct intel_dp *intel_dp,
> -			  struct intel_connector *connector,
> -			  int hdisplay, int clock);
> +int intel_dp_compute_joiner_pipes(struct intel_dp *intel_dp,
> +				  struct intel_connector *connector,
> +				  int hdisplay, int clock);
>  
>  static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 15541932b809..383b3e38df52 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -568,6 +568,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  		&pipe_config->hw.adjusted_mode;
>  	struct link_config_limits limits;
>  	bool dsc_needed, joiner_needs_dsc;
> +	int num_joined_pipes;
>  	int ret = 0;
>  
>  	if (pipe_config->fec_enable &&
> @@ -577,9 +578,10 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return -EINVAL;
>  
> -	if (intel_dp_need_joiner(intel_dp, connector,
> -				 adjusted_mode->crtc_hdisplay,
> -				 adjusted_mode->crtc_clock))
> +	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, connector,
> +							 adjusted_mode->crtc_hdisplay,
> +							 adjusted_mode->crtc_clock);
> +	if (num_joined_pipes == 2)
>  		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
>  
>  	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
> @@ -1426,6 +1428,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>  	u16 dsc_max_compressed_bpp = 0;
>  	u8 dsc_slice_count = 0;
>  	int target_clock = mode->clock;
> +	int num_joined_pipes;
>  
>  	if (drm_connector_is_unregistered(connector)) {
>  		*status = MODE_ERROR;
> @@ -1465,8 +1468,9 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>  	 *   corresponding link capabilities of the sink) in case the
>  	 *   stream is uncompressed for it by the last branch device.
>  	 */
> -	if (intel_dp_need_joiner(intel_dp, intel_connector,
> -				 mode->hdisplay, target_clock)) {
> +	num_joined_pipes = intel_dp_compute_joiner_pipes(intel_dp, intel_connector,
> +							 mode->hdisplay, target_clock);
> +	if (num_joined_pipes == 2) {
>  		joiner = true;
>  		max_dotclk *= 2;
>  	}
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 03/19] drm/i915/display_debugfs: Allow force joiner only if supported

[Ville Syrjälä]

On Wed, Sep 11, 2024 at 06:43:33PM +0530, Ankit Nautiyal wrote:
> Currently joiner is only supported for DP encoder.
> Allow force joiner only for DP for supported platforms.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 20 ++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index d7651f6f80e1..7ceaf01a884a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1520,6 +1520,9 @@ static ssize_t i915_joiner_write(struct file *file,
>  	struct seq_file *m = file->private_data;
>  	struct intel_connector *connector = m->private;
>  	struct intel_display *display = to_intel_display(connector);
> +	struct intel_encoder *intel_encoder = intel_attached_encoder(connector);
> +	struct intel_dp *intel_dp;
> +	int connector_type = connector->base.connector_type;
>  	int force_join_pipes = 0;
>  	int ret;
>  
> @@ -1530,16 +1533,27 @@ static ssize_t i915_joiner_write(struct file *file,
>  	if (ret < 0)
>  		return ret;
>  
> +	/* Currently joiner is only supported for eDP/DP */
> +	if (connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
> +	    connector_type != DRM_MODE_CONNECTOR_eDP)
> +		return 0;
> +
> +	intel_dp = enc_to_intel_dp(intel_encoder);
> +
> +	connector->force_joined_pipes = 0;
> +
>  	switch (force_join_pipes) {
>  	case 0:
> -		fallthrough;
> +		break;

That 'break' looks wrong.

>  	case 2:
> -		connector->force_joined_pipes = force_join_pipes;
> +		if (intel_dp_has_joiner(intel_dp))
> +			connector->force_joined_pipes = force_join_pipes;
> +		else
> +			drm_dbg(display->drm, "Force joiner not supported for the config\n");
>  		break;
>  	default:
>  		drm_dbg(display->drm, "Ignoring Invalid num of pipes %d for force joining\n",
>  			force_join_pipes);
> -		connector->force_joined_pipes = 0;
>  	}
>  
>  	*offp += len;
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 06/19] drm/i915/display: Simplify intel_joiner_num_pipes and its usage

[Ville Syrjälä]

On Wed, Sep 11, 2024 at 06:43:36PM +0530, Ankit Nautiyal wrote:
> Currently intel_joiner_num_pipes is used to get num of pipes wrt num of
> pipes joined. Simplify this by returning 1 when no joiner is used and
> update the checks for no joiner case.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 486bade9e927..4751ee20216d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -280,7 +280,7 @@ bool intel_crtc_is_joiner_primary(const struct intel_crtc_state *crtc_state)
>  
>  static int intel_joiner_num_pipes(const struct intel_crtc_state *crtc_state)
>  {
> -	return hweight8(crtc_state->joiner_pipes);
> +	return hweight8(crtc_state->joiner_pipes) ?: 1;

If we want to deal with this here, then the function probably needs
to be renamed to something like intel_crtc_num_joined_pipes().

In fact we could implement it using the already exising
intel_crtc_joined_pipe_mask() and avoid the ?:1 stuff.

>  }
>  
>  u8 intel_crtc_joined_pipe_mask(const struct intel_crtc_state *crtc_state)
> @@ -2347,7 +2347,7 @@ static void intel_joiner_adjust_timings(const struct intel_crtc_state *crtc_stat
>  {
>  	int num_pipes = intel_joiner_num_pipes(crtc_state);
>  
> -	if (num_pipes < 2)
> +	if (num_pipes == 1)
>  		return;
>  
>  	mode->crtc_clock /= num_pipes;
> @@ -2409,7 +2409,7 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
>  	drm_mode_copy(mode, pipe_mode);
>  	intel_mode_from_crtc_timings(mode, mode);
>  	mode->hdisplay = drm_rect_width(&crtc_state->pipe_src) *
> -		(intel_joiner_num_pipes(crtc_state) ?: 1);
> +		intel_joiner_num_pipes(crtc_state);
>  	mode->vdisplay = drm_rect_height(&crtc_state->pipe_src);
>  
>  	/* Derive per-pipe timings in case joiner is used */
> @@ -2432,7 +2432,7 @@ static void intel_joiner_compute_pipe_src(struct intel_crtc_state *crtc_state)
>  	int num_pipes = intel_joiner_num_pipes(crtc_state);
>  	int width, height;
>  
> -	if (num_pipes < 2)
> +	if (num_pipes == 1)
>  		return;
>  
>  	width = drm_rect_width(&crtc_state->pipe_src);
> @@ -2893,7 +2893,7 @@ static void intel_joiner_adjust_pipe_src(struct intel_crtc_state *crtc_state)
>  	enum pipe primary_pipe, pipe = crtc->pipe;
>  	int width;
>  
> -	if (num_pipes < 2)
> +	if (num_pipes == 1)
>  		return;
>  
>  	primary_pipe = joiner_primary_pipe(crtc_state);
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 07/19] drm/i915/display: Use joined pipes in intel_dp_joiner_needs_dsc

[Ville Syrjälä]

On Wed, Sep 11, 2024 at 06:43:37PM +0530, Ankit Nautiyal wrote:
> In preparation of ultrajoiner, use number of joined pipes in the
> intel_dp_joiner_needs_dsc helper, instead of joiner flag.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c     | 9 +++++----
>  drivers/gpu/drm/i915/display/intel_dp.h     | 3 ++-
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 +++--
>  3 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 65044f63d1f5..6c503b5968d2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1412,7 +1412,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  		dsc = dsc_max_compressed_bpp && dsc_slice_count;
>  	}
>  
> -	if (intel_dp_joiner_needs_dsc(dev_priv, joiner) && !dsc)
> +	if (intel_dp_joiner_needs_dsc(dev_priv, num_joined_pipes) && !dsc)
>  		return MODE_CLOCK_HIGH;
>  
>  	if (mode_rate > max_rate && !dsc)
> @@ -2520,14 +2520,15 @@ int intel_dp_config_required_rate(const struct intel_crtc_state *crtc_state)
>  	return intel_dp_link_required(adjusted_mode->crtc_clock, bpp);
>  }
>  
> -bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915, bool use_joiner)
> +bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915,
> +			       int num_joined_pipes)
>  {
>  	/*
>  	 * Pipe joiner needs compression up to display 12 due to bandwidth
>  	 * limitation. DG2 onwards pipe joiner can be enabled without
>  	 * compression.
>  	 */
> -	return DISPLAY_VER(i915) < 13 && use_joiner;
> +	return DISPLAY_VER(i915) < 13 && (num_joined_pipes == 2);

Pointless parens.

>  }
>  
>  static int
> @@ -2558,7 +2559,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  	if (num_joined_pipes == 2)
>  		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
>  
> -	joiner_needs_dsc = intel_dp_joiner_needs_dsc(i915, pipe_config->joiner_pipes);
> +	joiner_needs_dsc = intel_dp_joiner_needs_dsc(i915, num_joined_pipes);
>  
>  	dsc_needed = joiner_needs_dsc || intel_dp->force_dsc_en ||
>  		     !intel_dp_compute_config_limits(intel_dp, pipe_config,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index bc9a82d82df2..cc08a309eb78 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -123,7 +123,8 @@ int intel_dp_effective_data_rate(int pixel_clock, int bpp_x16,
>  				 int bw_overhead);
>  int intel_dp_max_link_data_rate(struct intel_dp *intel_dp,
>  				int max_dprx_rate, int max_dprx_lanes);
> -bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915, bool use_joiner);
> +bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915,
> +			       int num_joined_pipes);
>  bool intel_dp_has_joiner(struct intel_dp *intel_dp);
>  bool intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state,
>  			    const struct drm_connector_state *conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 383b3e38df52..fb067749f3a0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -588,7 +588,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->has_pch_encoder = false;
>  
> -	joiner_needs_dsc = intel_dp_joiner_needs_dsc(dev_priv, pipe_config->joiner_pipes);
> +	joiner_needs_dsc = intel_dp_joiner_needs_dsc(dev_priv, num_joined_pipes);
>  
>  	dsc_needed = joiner_needs_dsc || intel_dp->force_dsc_en ||
>  		     !intel_dp_mst_compute_config_limits(intel_dp,
> @@ -1472,6 +1472,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>  							 mode->hdisplay, target_clock);
>  	if (num_joined_pipes == 2) {
>  		joiner = true;
> +		num_joined_pipes = 2;
>  		max_dotclk *= 2;
>  	}
>  
> @@ -1512,7 +1513,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>  		dsc = dsc_max_compressed_bpp && dsc_slice_count;
>  	}
>  
> -	if (intel_dp_joiner_needs_dsc(dev_priv, joiner) && !dsc) {
> +	if (intel_dp_joiner_needs_dsc(dev_priv, num_joined_pipes) && !dsc) {
>  		*status = MODE_CLOCK_HIGH;
>  		return 0;
>  	}
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 11/19] drm/i915: Split current joiner hw state readout

[Ville Syrjälä]

On Wed, Sep 11, 2024 at 06:43:41PM +0530, Ankit Nautiyal wrote:
> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> 
> We need to add a new sanity checks and also do
> some preparations for adding ultrajoiner hw state readout.
> Lets first split reading of the uncompressed joiner and bigjoiner
> bit masks into separate functions.
> 
> v2: Fixed checkpatch warnings (Ankit)
> v3: Use struct intel_display in the new functions. (Ankit)
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 75 ++++++++++++++------
>  1 file changed, 55 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 248ad63b0ba8..e93af02aa859 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3582,27 +3582,54 @@ static bool intel_display_can_use_joiner(struct intel_display *display)
>  	return (DISPLAY_VER(display) >= 11);
>  }
>  
> -static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
> -				 u8 *primary_pipes, u8 *secondary_pipes)
> +static void enabled_uncompressed_joiner_pipes(struct intel_display *display,
> +					      u8 *primary_pipes, u8 *secondary_pipes)
>  {
> -	struct intel_display *display = to_intel_display(&dev_priv->drm);
> +	struct drm_i915_private *i915 = to_i915(display->drm);
>  	struct intel_crtc *crtc;
>  
>  	*primary_pipes = 0;
>  	*secondary_pipes = 0;
>  
> -	if (!intel_display_can_use_joiner(display))
> +	if (DISPLAY_VER(display) < 13)
>  		return;
>  
> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc,
> -					 joiner_pipes(dev_priv)) {
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc,
> +					 joiner_pipes(i915)) {
>  		enum intel_display_power_domain power_domain;
>  		enum pipe pipe = crtc->pipe;
>  		intel_wakeref_t wakeref;
>  
> -		power_domain = intel_dsc_power_domain(crtc, (enum transcoder) pipe);
> -		with_intel_display_power_if_enabled(dev_priv, power_domain, wakeref) {
> -			u32 tmp = intel_de_read(dev_priv, ICL_PIPE_DSS_CTL1(pipe));
> +		power_domain = POWER_DOMAIN_PIPE(pipe);
> +		with_intel_display_power_if_enabled(i915, power_domain, wakeref) {
> +			u32 tmp = intel_de_read(display, ICL_PIPE_DSS_CTL1(pipe));
> +
> +			if (tmp & UNCOMPRESSED_JOINER_PRIMARY)
> +				*primary_pipes |= BIT(pipe);
> +			if (tmp & UNCOMPRESSED_JOINER_SECONDARY)
> +				*secondary_pipes |= BIT(pipe);
> +		}
> +	}
> +}
> +
> +static void enabled_bigjoiner_pipes(struct intel_display *display,
> +				    u8 *primary_pipes, u8 *secondary_pipes)
> +{
> +	struct drm_i915_private *i915 = to_i915(display->drm);
> +	struct intel_crtc *crtc;
> +
> +	*primary_pipes = 0;
> +	*secondary_pipes = 0;
> +
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc,
> +					 joiner_pipes(i915)) {
> +		enum intel_display_power_domain power_domain;
> +		enum pipe pipe = crtc->pipe;
> +		intel_wakeref_t wakeref;
> +
> +		power_domain = intel_dsc_power_domain(crtc, (enum transcoder)pipe);
> +		with_intel_display_power_if_enabled(i915, power_domain, wakeref) {
> +			u32 tmp = intel_de_read(display, ICL_PIPE_DSS_CTL1(pipe));
>  
>  			if (!(tmp & BIG_JOINER_ENABLE))
>  				continue;
> @@ -3612,20 +3639,28 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
>  			else
>  				*secondary_pipes |= BIT(pipe);
>  		}
> +	}
> +}
>  
> -		if (DISPLAY_VER(dev_priv) < 13)
> -			continue;
> +static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
> +				 u8 *primary_pipes, u8 *secondary_pipes)
> +{
> +	struct intel_display *display = to_intel_display(&dev_priv->drm);
> +	u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes;
> +	u8 secondary_uncompressed_joiner_pipes, secondary_bigjoiner_pipes;
>  
> -		power_domain = POWER_DOMAIN_PIPE(pipe);
> -		with_intel_display_power_if_enabled(dev_priv, power_domain, wakeref) {
> -			u32 tmp = intel_de_read(dev_priv, ICL_PIPE_DSS_CTL1(pipe));
> +	if (!intel_display_can_use_joiner(display))
> +		return;

I think we should just have the check in enabled_bigjoiner_pipes()
and this function shouldn't really have to care whether joiners
are supported or not (or which types are supported).

>  
> -			if (tmp & UNCOMPRESSED_JOINER_PRIMARY)
> -				*primary_pipes |= BIT(pipe);
> -			if (tmp & UNCOMPRESSED_JOINER_SECONDARY)
> -				*secondary_pipes |= BIT(pipe);
> -		}
> -	}
> +	enabled_uncompressed_joiner_pipes(display, &primary_uncompressed_joiner_pipes,
> +					  &secondary_uncompressed_joiner_pipes);
> +
> +	enabled_bigjoiner_pipes(display, &primary_bigjoiner_pipes,
> +				&secondary_bigjoiner_pipes);
> +
> +	*primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes;
> +
> +	*secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes;
>  
>  	/* Joiner pipes should always be consecutive primary and secondary */
>  	drm_WARN(&dev_priv->drm, *secondary_pipes != *primary_pipes << 1,
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 13/19] drm/i915: Implement hw state readout and checks for ultrajoiner

[Ville Syrjälä]

On Wed, Sep 11, 2024 at 06:43:43PM +0530, Ankit Nautiyal wrote:
> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> 
> Ultrajoiner mode has some new bits and states to be
> read out from the hw. Lets make changes accordingly.
> 
> v2: Fix checkpatch warnings. (Ankit)
> v3: Add separate functions for computing expected secondary_big/ultrajoiner
> pipes. (Ankit)
> v4: Streamline the helpers for ultrajoiner. (Ville)
> -Add fixup to accommodate PIPED check for ultrajoiner. (Ville)
> -Add more Ultrajoiner drm_WARNs. (Ville)
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  | 91 ++++++++++++++++++-
>  .../gpu/drm/i915/display/intel_vdsc_regs.h    |  3 +
>  2 files changed, 89 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index b66685a9843f..02926a8ef7c5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3662,13 +3662,68 @@ static u8 expected_bigjoiner_secondary_pipes(u8 bigjoiner_primary_pipes)
>  	return expected_secondary_pipes(bigjoiner_primary_pipes, 2);
>  }
>  
> +static u8 expected_ultrajoiner_secondary_pipes(u8 ultrajoiner_primary_pipes)
> +{
> +	return expected_secondary_pipes(ultrajoiner_primary_pipes, 4);
> +}
> +
> +static u8 fixup_ultrajoiner_secondary_pipes(u8 ultrajoiner_primary_pipes,
> +					    u8 ultrajoiner_secondary_pipes)
> +{
> +	return ultrajoiner_secondary_pipes | ultrajoiner_primary_pipes << 3;
> +}
> +
> +static
> +bool intel_display_can_use_ultrajoiner(struct intel_display *display)
> +{
> +	struct drm_i915_private *i915 = to_i915(display->drm);
> +
> +	return ((DISPLAY_VER(display) == 14 && IS_DGFX(i915)) ||
> +		DISPLAY_VER(display) > 14);
> +}
> +
> +static void enabled_ultrajoiner_pipes(struct drm_i915_private *i915,
> +				      u8 *primary_pipes, u8 *secondary_pipes)
> +{
> +	struct intel_display *display = to_intel_display(&i915->drm);
> +	struct intel_crtc *crtc;
> +
> +	*primary_pipes = 0;
> +	*secondary_pipes = 0;
> +
> +	if (!intel_display_can_use_ultrajoiner(display))
> +		return;
> +
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc,
> +					 joiner_pipes(i915)) {
> +		enum intel_display_power_domain power_domain;
> +		enum pipe pipe = crtc->pipe;
> +		intel_wakeref_t wakeref;
> +
> +		power_domain = intel_dsc_power_domain(crtc, (enum transcoder)pipe);
> +		with_intel_display_power_if_enabled(i915, power_domain, wakeref) {
> +			u32 tmp = intel_de_read(i915, ICL_PIPE_DSS_CTL1(pipe));
> +
> +			if (!(tmp & ULTRA_JOINER_ENABLE))
> +				continue;
> +
> +			if (tmp & PRIMARY_ULTRA_JOINER_ENABLE)
> +				*primary_pipes |= BIT(pipe);
> +			else
> +				*secondary_pipes |= BIT(pipe);
> +		}
> +	}
> +}
> +
>  static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
>  				 u8 *primary_pipes, u8 *secondary_pipes)
>  {
>  	struct intel_display *display = to_intel_display(&dev_priv->drm);
>  	u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes;
> +	u8 primary_ultrajoiner_pipes;
>  	u8 secondary_uncompressed_joiner_pipes, secondary_bigjoiner_pipes;
> -	u8 uncompressed_joiner_pipes, bigjoiner_pipes;
> +	u8 secondary_ultrajoiner_pipes;
> +	u8 uncompressed_joiner_pipes, bigjoiner_pipes, ultrajoiner_pipes;
>  
>  	if (!intel_display_can_use_joiner(display))
>  		return;
> @@ -3685,13 +3740,33 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
>  	drm_WARN_ON(display->drm,
>  		    (primary_bigjoiner_pipes & secondary_bigjoiner_pipes) != 0);
>  
> +	enabled_ultrajoiner_pipes(dev_priv, &primary_ultrajoiner_pipes,
> +				  &secondary_ultrajoiner_pipes);
> +	/*
> +	 * For some strange reason the last pipe in the set of four
> +	 * shouldn't have ultrajoiner enable bit set in hardware.
> +	 * Set the bit anyway to make life easier.
> +	 */
> +	drm_WARN_ON(&dev_priv->drm,
> +		    expected_secondary_pipes(primary_ultrajoiner_pipes, 3) !=
> +		    secondary_ultrajoiner_pipes);
> +	secondary_ultrajoiner_pipes =
> +		fixup_ultrajoiner_secondary_pipes(primary_ultrajoiner_pipes,
> +						  secondary_ultrajoiner_pipes);

Did we find out if the spec is right or wrong about this?

> +
> +	drm_WARN_ON(&dev_priv->drm, (primary_ultrajoiner_pipes & secondary_ultrajoiner_pipes) != 0);
> +
>  	uncompressed_joiner_pipes = primary_uncompressed_joiner_pipes |
>  				    secondary_uncompressed_joiner_pipes;
>  	bigjoiner_pipes = primary_bigjoiner_pipes | secondary_bigjoiner_pipes;
> +	ultrajoiner_pipes = primary_ultrajoiner_pipes | secondary_ultrajoiner_pipes;
>  
>  	drm_WARN(display->drm, (uncompressed_joiner_pipes & bigjoiner_pipes) != 0,
>  		 "Uncomressed joiner pipes(%#x) and bigjoiner pipes(%#x) can't intersect\n",
>  		 uncompressed_joiner_pipes, bigjoiner_pipes);
> +	drm_WARN(display->drm, (ultrajoiner_pipes & bigjoiner_pipes) != ultrajoiner_pipes,
> +		 "Ultrajoiner pipes(%#x) should be bigjoiner pipes(%#x)\n",
> +		 ultrajoiner_pipes, bigjoiner_pipes);
>  	drm_WARN(display->drm, secondary_bigjoiner_pipes !=
>  		 expected_bigjoiner_secondary_pipes(primary_bigjoiner_pipes),
>  		 "Wrong secondary bigjoiner pipes(expected %#x, current %#x)\n",
> @@ -3702,10 +3777,16 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
>  		 "Wrong secondary uncompressed joiner pipes(expected %#x, current %#x)\n",
>  		 expected_uncompjoiner_secondary_pipes(primary_uncompressed_joiner_pipes),
>  		 secondary_uncompressed_joiner_pipes);
> -
> -	*primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes;
> -
> -	*secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes;
> +	drm_WARN(display->drm, secondary_ultrajoiner_pipes !=
> +		 expected_ultrajoiner_secondary_pipes(primary_ultrajoiner_pipes),
> +		 "Wrong secondary ultrajoiner pipes(expected %x, current %x)\n",
> +		 expected_ultrajoiner_secondary_pipes(primary_ultrajoiner_pipes),
> +		 secondary_ultrajoiner_pipes);
> +
> +	*primary_pipes = primary_uncompressed_joiner_pipes | primary_bigjoiner_pipes |
> +			 primary_ultrajoiner_pipes;
> +	*secondary_pipes = secondary_uncompressed_joiner_pipes | secondary_bigjoiner_pipes |
> +			 secondary_ultrajoiner_pipes;
>  
>  	/* Joiner pipes should always be consecutive primary and secondary */
>  	drm_WARN(display->drm, *secondary_pipes != *primary_pipes << 1,
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc_regs.h b/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
> index f921ad67b587..db07c9775892 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
> @@ -37,6 +37,9 @@
>  #define  SPLITTER_CONFIGURATION_MASK		REG_GENMASK(26, 25)
>  #define  SPLITTER_CONFIGURATION_2_SEGMENT	REG_FIELD_PREP(SPLITTER_CONFIGURATION_MASK, 0)
>  #define  SPLITTER_CONFIGURATION_4_SEGMENT	REG_FIELD_PREP(SPLITTER_CONFIGURATION_MASK, 1)
> +

spurious newline

> +#define  ULTRA_JOINER_ENABLE			REG_BIT(23)
> +#define  PRIMARY_ULTRA_JOINER_ENABLE		REG_BIT(22)
>  #define  UNCOMPRESSED_JOINER_PRIMARY		(1 << 21)
>  #define  UNCOMPRESSED_JOINER_SECONDARY		(1 << 20)
>  
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 14/19] drm/i915/display: Percolate ultrajoiner info to get_joiner_config

[Ville Syrjälä]

On Wed, Sep 11, 2024 at 06:43:44PM +0530, Ankit Nautiyal wrote:
> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> 
> Modify the helpers get_primary/secondary_pipes to account for
> ultrajoiner usage. Use the modified helpers to retrieve ultrajoiner
> information in get_joiner_config.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 57 +++++++++++++-------
>  1 file changed, 37 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 02926a8ef7c5..24698d8ed5d6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3716,7 +3716,8 @@ static void enabled_ultrajoiner_pipes(struct drm_i915_private *i915,
>  }
>  
>  static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
> -				 u8 *primary_pipes, u8 *secondary_pipes)
> +				 u8 *primary_pipes, u8 *secondary_pipes,
> +				 bool *ultrajoiner_used)

I think life will be simpler if we just pass the current pipe into
enabled_joiner_pipes(), and let it figure out the proper bitmasks
for us.

We should be able to just do somehting like:

if (ultrajoiner_pipes & BIT(pipe)) {
	*primary_pipes = BIT(get_primary_pipe(ultrajoiner_primary_pipes, pipe));
	*secondary_pipes = ultrajoiner_secondary_pipes &
                       expected_ultrajoiner_secondary_pipes(*primary_pipes);
	return;
}
// same for uncompressed joiner
// same for bigjoiner

to get the approriate primary pipe and secondary pipes for the current
pipe.

>  {
>  	struct intel_display *display = to_intel_display(&dev_priv->drm);
>  	u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes;
> @@ -3760,6 +3761,7 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
>  				    secondary_uncompressed_joiner_pipes;
>  	bigjoiner_pipes = primary_bigjoiner_pipes | secondary_bigjoiner_pipes;
>  	ultrajoiner_pipes = primary_ultrajoiner_pipes | secondary_ultrajoiner_pipes;
> +	*ultrajoiner_used = ultrajoiner_pipes != 0;
>  
>  	drm_WARN(display->drm, (uncompressed_joiner_pipes & bigjoiner_pipes) != 0,
>  		 "Uncomressed joiner pipes(%#x) and bigjoiner pipes(%#x) can't intersect\n",
> @@ -3794,7 +3796,8 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
>  		 *primary_pipes, *secondary_pipes);
>  }
>  
> -static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
> +static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes,
> +					 u8 secondary_pipes, bool ultrajoiner_used)
>  {
>  	if ((secondary_pipes & BIT(pipe)) == 0)
>  		return pipe;
> @@ -3802,27 +3805,37 @@ static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 se
>  	/* ignore everything above our pipe */
>  	primary_pipes &= ~GENMASK(7, pipe);
>  
> -	/* highest remaining bit should be our primary pipe */
> -	return fls(primary_pipes) - 1;
> +	if (!ultrajoiner_used)
> +		/* highest remaining bit should be our master pipe */
> +		return fls(primary_pipes) - 1;
> +
> +	/* lowest remaining bit should be our primary master pipe */
> +	return ffs(primary_pipes) - 1;
>  }
>  
> -static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
> +static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes,
> +				     u8 secondary_pipes, bool ultrajoiner_used)
>  {
>  	enum pipe primary_pipe, next_primary_pipe;
>  
> -	primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes);
> +	primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes,
> +					       secondary_pipes, ultrajoiner_used);
>  
>  	if ((primary_pipes & BIT(primary_pipe)) == 0)
>  		return 0;
>  
> -	/* ignore our primary pipe and everything below it */
> -	primary_pipes &= ~GENMASK(primary_pipe, 0);
> -	/* make sure a high bit is set for the ffs() */
> -	primary_pipes |= BIT(7);
> -	/* lowest remaining bit should be the next primary pipe */
> -	next_primary_pipe = ffs(primary_pipes) - 1;
> +	if (!ultrajoiner_used) {
> +		/* ignore our primary pipe and everything below it */
> +		primary_pipes &= ~GENMASK(primary_pipe, 0);
> +		/* make sure a high bit is set for the ffs() */
> +		primary_pipes |= BIT(7);
> +		/* lowest remaining bit should be the next primary pipe */
> +		next_primary_pipe = ffs(primary_pipes) - 1;
>  
> -	return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe);
> +		return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe);
> +	} else {
> +		return (secondary_pipes | primary_pipes) & ~BIT(primary_pipe);
> +	}
>  }
>  
>  static u8 hsw_panel_transcoders(struct drm_i915_private *i915)
> @@ -3843,6 +3856,7 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc)
>  	enum transcoder cpu_transcoder;
>  	u8 primary_pipes, secondary_pipes;
>  	u8 enabled_transcoders = 0;
> +	bool ultrajoiner_used;
>  
>  	/*
>  	 * XXX: Do intel_display_power_get_if_enabled before reading this (for
> @@ -3893,11 +3907,12 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc)
>  	if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder))
>  		enabled_transcoders |= BIT(cpu_transcoder);
>  
> -	/* joiner secondary -> consider the primary pipe's transcoder as well */
> -	enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes);
> +	/* joiner slave -> consider the master pipe's transcoder as well */
> +	enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes, &ultrajoiner_used);
>  	if (secondary_pipes & BIT(crtc->pipe)) {
>  		cpu_transcoder = (enum transcoder)
> -			get_joiner_primary_pipe(crtc->pipe, primary_pipes, secondary_pipes);
> +			get_joiner_primary_pipe(crtc->pipe, primary_pipes,
> +						secondary_pipes, ultrajoiner_used);
>  		if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder))
>  			enabled_transcoders |= BIT(cpu_transcoder);
>  	}
> @@ -4029,16 +4044,18 @@ static void intel_joiner_get_config(struct intel_crtc_state *crtc_state)
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>  	u8 primary_pipes, secondary_pipes;
> +	bool ultrajoiner_used;
>  	enum pipe pipe = crtc->pipe;
>  
> -	enabled_joiner_pipes(i915, &primary_pipes, &secondary_pipes);
> +	enabled_joiner_pipes(i915, &primary_pipes, &secondary_pipes, &ultrajoiner_used);
>  
>  	if (((primary_pipes | secondary_pipes) & BIT(pipe)) == 0)
>  		return;
>  
> -	crtc_state->joiner_pipes =
> -		BIT(get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes)) |
> -		get_joiner_secondary_pipes(pipe, primary_pipes, secondary_pipes);
> +	crtc_state->joiner_pipes = BIT(get_joiner_primary_pipe(pipe, primary_pipes,
> +							       secondary_pipes, ultrajoiner_used)) |
> +				   get_joiner_secondary_pipes(pipe, primary_pipes,
> +							      secondary_pipes, ultrajoiner_used);
>  }
>  
>  static bool hsw_get_pipe_config(struct intel_crtc *crtc,
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 15/19] drm/i915/display/vdsc: Add ultrajoiner support with DSC

[Ville Syrjälä]

On Wed, Sep 11, 2024 at 06:43:45PM +0530, Ankit Nautiyal wrote:
> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> 
> Add changes to DSC which are required for Ultrajoiner.
> 
> v2:
> -Use correct helper for setting bits for bigjoiner secondary. (Ankit)
> -Use enum for joiner pipe count instead magic numbers. (Suraj)
> -Use primary/secondary instead of master/slave. (Suraj)
> 
> v3:
> -Add the ultrajoiner helpers and use it for setting ultrajoiner bits
> (Ankit)
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 21 ++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_display.h |  2 ++
>  drivers/gpu/drm/i915/display/intel_vdsc.c    | 26 ++++++++++++++++++--
>  3 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 24698d8ed5d6..db27850b2c36 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -295,6 +295,27 @@ bool intel_crtc_is_bigjoiner_secondary(const struct intel_crtc_state *crtc_state
>  	return BIT(crtc->pipe) & bigjoiner_secondary_pipes(crtc_state);
>  }
>  
> +bool intel_crtc_is_ultrajoiner(const struct intel_crtc_state *crtc_state)
> +{
> +	return intel_joiner_num_pipes(crtc_state) >= 4;
> +}
> +
> +static u8 ultrajoiner_primary_pipes(const struct intel_crtc_state *crtc_state)
> +{
> +	if (!intel_crtc_is_ultrajoiner(crtc_state))
> +		return 0;
> +
> +	return crtc_state->joiner_pipes & (0b00010001 << joiner_primary_pipe(crtc_state));
> +}
> +
> +bool intel_crtc_is_ultrajoiner_primary(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +
> +	return intel_crtc_is_ultrajoiner(crtc_state) &&
> +	       BIT(crtc->pipe) & ultrajoiner_primary_pipes(crtc_state);
> +}
> +
>  u8 intel_crtc_joiner_secondary_pipes(const struct intel_crtc_state *crtc_state)
>  {
>  	if (crtc_state->joiner_pipes)
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 17d8b9df3916..dbbe23ea14fc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -427,6 +427,8 @@ bool intel_crtc_is_joiner_secondary(const struct intel_crtc_state *crtc_state);
>  bool intel_crtc_is_joiner_primary(const struct intel_crtc_state *crtc_state);
>  bool intel_crtc_is_bigjoiner_primary(const struct intel_crtc_state *crtc_state);
>  bool intel_crtc_is_bigjoiner_secondary(const struct intel_crtc_state *crtc_state);
> +bool intel_crtc_is_ultrajoiner(const struct intel_crtc_state *crtc_state);
> +bool intel_crtc_is_ultrajoiner_primary(const struct intel_crtc_state *crtc_state);
>  u8 intel_crtc_joiner_secondary_pipes(const struct intel_crtc_state *crtc_state);
>  struct intel_crtc *intel_primary_crtc(const struct intel_crtc_state *crtc_state);
>  bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 8158e3702ed5..b95dd7a7111e 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -379,9 +379,11 @@ static int intel_dsc_get_vdsc_per_pipe(const struct intel_crtc_state *crtc_state
>  int intel_dsc_get_num_vdsc_instances(const struct intel_crtc_state *crtc_state)
>  {
>  	int num_vdsc_instances = intel_dsc_get_vdsc_per_pipe(crtc_state);
> +	int num_joined_pipes = intel_joiner_num_pipes(crtc_state);
>  
> -	if (crtc_state->joiner_pipes)
> -		num_vdsc_instances *= 2;
> +	if (num_joined_pipes == 2 ||
> +	    num_joined_pipes == 4)
> +		num_vdsc_instances *= num_joined_pipes;

Since num_joined_pipes >= 1 this sort of stuff can become unconditional.

>  
>  	return num_vdsc_instances;
>  }
> @@ -751,6 +753,14 @@ void intel_uncompressed_joiner_enable(const struct intel_crtc_state *crtc_state)
>  	}
>  }
>  
> +static bool intel_crtc_ultrajoiner_enable_needed(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +
> +	return intel_crtc_is_ultrajoiner(crtc_state) &&
> +	       crtc->pipe != PIPE_D;
> +}
> +
>  void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> @@ -770,7 +780,19 @@ void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
>  		dss_ctl1_val |= JOINER_ENABLE;
>  	}
>  	if (crtc_state->joiner_pipes) {
> +		/*
> +		 * This bit doesn't seem to follow primary/secondary logic or
> +		 * any other logic, so lets just add helper function to
> +		 * at least hide this hassle..
> +		 */
> +		if (intel_crtc_ultrajoiner_enable_needed(crtc_state))
> +			dss_ctl1_val |= ULTRA_JOINER_ENABLE;
> +
> +		if (intel_crtc_is_ultrajoiner_primary(crtc_state))
> +			dss_ctl1_val |= PRIMARY_ULTRA_JOINER_ENABLE;
> +
>  		dss_ctl1_val |= BIG_JOINER_ENABLE;
> +
>  		if (intel_crtc_is_bigjoiner_primary(crtc_state))
>  			dss_ctl1_val |= PRIMARY_BIG_JOINER_ENABLE;
>  	}
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 16/19] drm/i915: Add new abstraction layer to handle pipe order for different joiners

[Ville Syrjälä]

On Wed, Sep 11, 2024 at 06:43:46PM +0530, Ankit Nautiyal wrote:
> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> 
> Ultrajoiner case requires special treatment where both reverse and
> staight order iteration doesn't work(for instance disabling case requires
> order to be: primary master, slaves, secondary master).
> 
> Lets unify our approach by using not only pipe masks for iterating required
> pipes based on joiner type used, but also using different "priority" arrays
> for each of those.
> 
> v2: Fix checkpatch warnings. (Ankit)
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 19 +++--
>  drivers/gpu/drm/i915/display/intel_display.c | 83 ++++++++++++++++----
>  drivers/gpu/drm/i915/display/intel_display.h |  7 ++
>  drivers/gpu/drm/i915/display/intel_dp_mst.c  | 18 +++--
>  4 files changed, 96 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 00fbe9f8c03a..2c064b6c6d01 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3116,10 +3116,11 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
>  					       const struct drm_connector_state *old_conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	struct intel_crtc *pipe_crtc;
> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
>  
> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
> +					     intel_get_pipe_order_disable(old_crtc_state)) {
>  		const struct intel_crtc_state *old_pipe_crtc_state =
>  			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>  
> @@ -3130,8 +3131,9 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
>  
>  	intel_ddi_disable_transcoder_func(old_crtc_state);
>  
> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
> +					     intel_get_pipe_order_disable(old_crtc_state)) {
>  		const struct intel_crtc_state *old_pipe_crtc_state =
>  			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>  
> @@ -3383,7 +3385,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
>  			     const struct drm_connector_state *conn_state)
>  {
>  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> -	struct intel_crtc *pipe_crtc;
> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
>  
>  	intel_ddi_enable_transcoder_func(encoder, crtc_state);
>  
> @@ -3394,8 +3396,9 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
>  
>  	intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
>  
> -	for_each_intel_crtc_in_pipe_mask_reverse(&i915->drm, pipe_crtc,
> -						 intel_crtc_joined_pipe_mask(crtc_state)) {
> +	for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
> +					     intel_crtc_joined_pipe_mask(crtc_state),
> +					     intel_get_pipe_order_enable(crtc_state)) {
>  		const struct intel_crtc_state *pipe_crtc_state =
>  			intel_atomic_get_new_crtc_state(state, pipe_crtc);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index db27850b2c36..27622d51a473 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1737,6 +1737,50 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
>  	hsw_set_transconf(crtc_state);
>  }
>  
> +static
> +bool intel_crtc_is_bigjoiner(const struct intel_crtc_state *pipe_config)
> +{
> +	return hweight8(pipe_config->joiner_pipes) == 2;
> +}
> +
> +const enum pipe *intel_get_pipe_order_enable(const struct intel_crtc_state *crtc_state)
> +{
> +	static const enum pipe ultrajoiner_pipe_order_enable[I915_MAX_PIPES] = {
> +		PIPE_B, PIPE_D, PIPE_C, PIPE_A
> +	};
> +	static const enum pipe bigjoiner_pipe_order_enable[I915_MAX_PIPES] = {
> +		PIPE_B, PIPE_A, PIPE_D, PIPE_C
> +	};
> +	static const enum pipe nojoiner_pipe_order_enable[I915_MAX_PIPES] = {
> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> +	};
> +
> +	if (intel_crtc_is_ultrajoiner(crtc_state))
> +		return ultrajoiner_pipe_order_enable;
> +	else if (intel_crtc_is_bigjoiner(crtc_state))
> +		return bigjoiner_pipe_order_enable;
> +	return nojoiner_pipe_order_enable;
> +}
> +
> +const enum pipe *intel_get_pipe_order_disable(const struct intel_crtc_state *crtc_state)
> +{
> +	static const enum pipe ultrajoiner_pipe_order_disable[I915_MAX_PIPES] = {
> +		PIPE_A, PIPE_B, PIPE_D, PIPE_C
> +	};
> +	static const enum pipe bigjoiner_pipe_order_disable[I915_MAX_PIPES] = {
> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> +	};
> +	static const enum pipe nojoiner_pipe_order_disable[I915_MAX_PIPES] = {
> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> +	};
> +
> +	if (intel_crtc_is_ultrajoiner(crtc_state))
> +		return ultrajoiner_pipe_order_disable;
> +	else if (intel_crtc_is_bigjoiner(crtc_state))
> +		return bigjoiner_pipe_order_disable;
> +	return nojoiner_pipe_order_disable;

I don't think we should need all those diffrent order array. Technically
one should do. Though having two might make sense.

Another problem is the hardcoded pipes. If we eg. get hardware that
would support ultrajoiner on pipes B-E in the future this would no
longer  work.

> +}
<snip>
> +#define for_each_intel_crtc_in_mask_priority(__dev_priv, intel_crtc, __p, __mask, __priolist) \
> +	for_each_pipe(__dev_priv, __p) \
> +		for_each_if((__mask) & BIT(__priolist[__p])) \
> +			for_each_if(intel_crtc = intel_crtc_for_pipe(to_intel_display(&__dev_priv->drm), __priolist[__p]))


I think something like:

const u8 intel_pipe_order_enable[4] = {
        3, 1, 2, 0,
};

const u8 intel_pipe_order_disable[4] = {
        0, 2, 1, 3,
};

#define for_each_intel_crtc_in_pipe_mask_ordered(crtc, pipe_masks, order, i) \
        for ((i) = 0; \
             (i) < ARRAY_SIZE(order) && \
             ((crtc) = intel_crtc_for_pipe(joiner_primary_pipe(pipe_mask) + (order)[(i)]), 1); \
             (i)++) \
                for_each_if((crtc) && (pipe_mask) & BIT((crtc)->pipe))

would let us avoid that hardcoded pipe stuff, and everything is
just based on the relative order between the pipes. The same orders
also work for bigjoiner and non-joined cases (it just skips the pipes
that are't in the mask).


The alternative would be to just use the bigjoiner primary+secondary masks
and come up with a a way to iterate two bitmask in either forward or reverse
order. Hmm, I suppose one might just combine the bigjoiner primary and
secondary masks into one, with one of them shifted up to some high bits,
and then iterate the combined bitmask either forward or backward.

Something like this should work:
#define for_each_crtc_in_masks(crtc, first_pipes, second_pipes, pipes, i) \
        for ((i) = 0, (pipes) = (second_pipes) << 16 | (first_pipes); \
             (i) < 32 && ((crtc) = intel_crtc_for_pipe((i) & 15), 1); \
             (i)++) \
                for_each_if((crtc) && (pipes) & BIT(i))

#define for_each_crtc_in_masks_reverse(crtc, first_pipes, second_pipes, pipes, i) \
        for ((i) = 31, (pipes) = (first_pipes) << 16 | (second_pipes); \
             (i) >= 0 && ((crtc) = intel_crtc_for_pipe((i) & 15), 1); \
             (i)--) \
                for_each_if((crtc) && (pipes) & BIT(i))

(could reduce the constants a bit given we don't have 16 pipes).

And then we'd just use them like so:
for_each_crtc_in_masks_reverse(crtc, bigjoiner_secondary_pipes(),
				bigjoiner_primary_pipes(), pipes, i)
	enable();

for_each_crtc_in_masks(crtc, bigjoiner_primary_pipes(),
			bigjoiner_secondary_pipes(), pipes, i)
	disable();

Would at least be very close to what we already have, just splitting
the pipe masks into two essentially.

> +
>  #define for_each_intel_crtc_in_pipe_mask(dev, intel_crtc, pipe_mask)	\
>  	list_for_each_entry(intel_crtc,					\
>  			    &(dev)->mode_config.crtc_list,		\
> @@ -431,6 +436,8 @@ bool intel_crtc_is_ultrajoiner(const struct intel_crtc_state *crtc_state);
>  bool intel_crtc_is_ultrajoiner_primary(const struct intel_crtc_state *crtc_state);
>  u8 intel_crtc_joiner_secondary_pipes(const struct intel_crtc_state *crtc_state);
>  struct intel_crtc *intel_primary_crtc(const struct intel_crtc_state *crtc_state);
> +const enum pipe *intel_get_pipe_order_enable(const struct intel_crtc_state *crtc_state);
> +const enum pipe *intel_get_pipe_order_disable(const struct intel_crtc_state *crtc_state);
>  bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state);
>  bool intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  			       const struct intel_crtc_state *pipe_config,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index be79783ce09b..1c87f81568c8 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1003,7 +1003,7 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  	struct drm_dp_mst_atomic_payload *new_payload =
>  		drm_atomic_get_mst_payload_state(new_mst_state, connector->port);
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> -	struct intel_crtc *pipe_crtc;
> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
>  	bool last_mst_stream;
>  
>  	intel_dp->active_mst_links--;
> @@ -1012,8 +1012,9 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  		    DISPLAY_VER(dev_priv) >= 12 && last_mst_stream &&
>  		    !intel_dp_mst_is_master_trans(old_crtc_state));
>  
> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
> +					     intel_get_pipe_order_disable(old_crtc_state)) {
>  		const struct intel_crtc_state *old_pipe_crtc_state =
>  			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>  
> @@ -1037,8 +1038,9 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  
>  	intel_ddi_disable_transcoder_func(old_crtc_state);
>  
> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
> +					     intel_get_pipe_order_disable(old_crtc_state)) {
>  		const struct intel_crtc_state *old_pipe_crtc_state =
>  			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>  
> @@ -1257,6 +1259,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
>  	enum transcoder trans = pipe_config->cpu_transcoder;
>  	bool first_mst_stream = intel_dp->active_mst_links == 1;
>  	struct intel_crtc *pipe_crtc;
> +	enum pipe pipe;
>  	int ret;
>  
>  	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
> @@ -1304,8 +1307,9 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
>  
>  	intel_enable_transcoder(pipe_config);
>  
> -	for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
> -						 intel_crtc_joined_pipe_mask(pipe_config)) {
> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> +					     intel_crtc_joined_pipe_mask(pipe_config),
> +					     intel_get_pipe_order_enable(pipe_config)) {
>  		const struct intel_crtc_state *pipe_crtc_state =
>  			intel_atomic_get_new_crtc_state(state, pipe_crtc);
>  
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 00/19] Ultrajoiner basic functionality series

[Ville Syrjälä]

On Wed, Sep 11, 2024 at 06:43:30PM +0530, Ankit Nautiyal wrote:
> This patch series attempts to implement basic support
> for Ultrajoiner functionality.
> 
> Rev6:
> -Upgrade the debugfs functionality to enable the joining of a
> specified number of pipes.
> -Modify the display helpers reliant on the pipe joiner mechanism
> to use number of pipes joined, instead of joiner flag.
> -Checkpatch fixes.
> 
> Rev7:
> -Use struct intel_display, minor refactoring, and rebase.
> 
> Rev8:
> -Address comments from Ville.
> -Simplified debugfs for forcing joiner, and added option to disable
> joiner.
> -Modified the ultra/bigjoiner helpers as suggested by Ville. 
> -Split few of the bigger patches as suggested.
> 
> Test-with: 20240911072204.22825-1-karthik.b.s@intel.com
> 
> Ankit Nautiyal (11):
>   drm/i915/display: Simplify intel_joiner_num_pipes and its usage
>   drm/i915/display: Use joined pipes in intel_dp_joiner_needs_dsc
>   drm/i915/display: Use joined pipes in intel_mode_valid_max_plane_size
>   drm/i915/display: Use joined pipes in dsc helpers for slices, bpp

I have a feeling those four at least could be merged ahead of
time, and thus shrink what's left quite decently already.
Granted, you probably won't have the num_pipes value passed
in all the way from the top yet, so might have to do some
'joiner ? 2 : 1' here and there as a temporary measure.

I think getting this stuff out of the way would help
avoid confusion with the debugfs/compute_config changes
to some degree at least, and might help with bisectability,
Presumably it builds, but I'm not really convinced all the
intermediate steps would actually work 100% correctly.

>   drm/i915/display: Check whether platform supports joiner

For this stuff I'd like to see a HAS_BIGJOINER() and
HAS_UNCOMPRESSED_JOINER() like we have for most platform
features. This also seems like good material for stuff to
merge on its own, ahead of the rest of the more complicated 
stuff below.

>   drm/i915/display: Modify debugfs for joiner to force n pipes
>   drm/i915/display_debugfs: Allow force joiner only if supported
>   drm/i915/dp: Add helper to compute num pipes joined
>   drm/i915/display: Add debugfs support to avoid joiner
>   drm/i915/display: Consider ultrajoiner for computing maxdotclock
>   drm/i915/intel_dp: Add support for forcing ultrajoiner
> 
> Stanislav Lisovskiy (8):
>   drm/i915: Add some essential functionality for joiners
>   drm/i915: Split current joiner hw state readout
>   drm/i915: Add bigjoiner and uncompressed joiner hw readout sanity
>     checks
>   drm/i915: Implement hw state readout and checks for ultrajoiner
>   drm/i915/display: Percolate ultrajoiner info to get_joiner_config
>   drm/i915/display/vdsc: Add ultrajoiner support with DSC
>   drm/i915: Add new abstraction layer to handle pipe order for different
>     joiners
>   drm/i915: Compute config and mode valid changes for ultrajoiner
> 
>  drivers/gpu/drm/i915/display/intel_ddi.c      |  19 +-
>  drivers/gpu/drm/i915/display/intel_display.c  | 423 +++++++++++++++---
>  drivers/gpu/drm/i915/display/intel_display.h  |  17 +-
>  .../drm/i915/display/intel_display_debugfs.c  |  84 +++-
>  .../drm/i915/display/intel_display_types.h    |   2 +-
>  drivers/gpu/drm/i915/display/intel_dp.c       | 138 ++++--
>  drivers/gpu/drm/i915/display/intel_dp.h       |  14 +-
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  54 ++-
>  drivers/gpu/drm/i915/display/intel_dsi.c      |   2 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.c     |   2 +-
>  drivers/gpu/drm/i915/display/intel_vdsc.c     |  30 +-
>  .../gpu/drm/i915/display/intel_vdsc_regs.h    |   3 +
>  12 files changed, 646 insertions(+), 142 deletions(-)
> 
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 06/19] drm/i915/display: Simplify intel_joiner_num_pipes and its usage

[Nautiyal, Ankit K]

On 9/12/2024 1:44 AM, Ville Syrjälä wrote:
> On Wed, Sep 11, 2024 at 06:43:36PM +0530, Ankit Nautiyal wrote:
>> Currently intel_joiner_num_pipes is used to get num of pipes wrt num of
>> pipes joined. Simplify this by returning 1 when no joiner is used and
>> update the checks for no joiner case.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 486bade9e927..4751ee20216d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -280,7 +280,7 @@ bool intel_crtc_is_joiner_primary(const struct intel_crtc_state *crtc_state)
>>   
>>   static int intel_joiner_num_pipes(const struct intel_crtc_state *crtc_state)
>>   {
>> -	return hweight8(crtc_state->joiner_pipes);
>> +	return hweight8(crtc_state->joiner_pipes) ?: 1;
> If we want to deal with this here, then the function probably needs
> to be renamed to something like intel_crtc_num_joined_pipes().
>
> In fact we could implement it using the already exising
> intel_crtc_joined_pipe_mask() and avoid the ?:1 stuff.

Alright I will rename the function as suggested and use:

int intel_crtc_num_joined_pipes(const struct intel_crtc_state *crtc_state)
{
         return hweight8(intel_crtc_joined_pipe_mask(crtc_state));
}

Regards,

Ankit

>
>>   }
>>   
>>   u8 intel_crtc_joined_pipe_mask(const struct intel_crtc_state *crtc_state)
>> @@ -2347,7 +2347,7 @@ static void intel_joiner_adjust_timings(const struct intel_crtc_state *crtc_stat
>>   {
>>   	int num_pipes = intel_joiner_num_pipes(crtc_state);
>>   
>> -	if (num_pipes < 2)
>> +	if (num_pipes == 1)
>>   		return;
>>   
>>   	mode->crtc_clock /= num_pipes;
>> @@ -2409,7 +2409,7 @@ static void intel_crtc_readout_derived_state(struct intel_crtc_state *crtc_state
>>   	drm_mode_copy(mode, pipe_mode);
>>   	intel_mode_from_crtc_timings(mode, mode);
>>   	mode->hdisplay = drm_rect_width(&crtc_state->pipe_src) *
>> -		(intel_joiner_num_pipes(crtc_state) ?: 1);
>> +		intel_joiner_num_pipes(crtc_state);
>>   	mode->vdisplay = drm_rect_height(&crtc_state->pipe_src);
>>   
>>   	/* Derive per-pipe timings in case joiner is used */
>> @@ -2432,7 +2432,7 @@ static void intel_joiner_compute_pipe_src(struct intel_crtc_state *crtc_state)
>>   	int num_pipes = intel_joiner_num_pipes(crtc_state);
>>   	int width, height;
>>   
>> -	if (num_pipes < 2)
>> +	if (num_pipes == 1)
>>   		return;
>>   
>>   	width = drm_rect_width(&crtc_state->pipe_src);
>> @@ -2893,7 +2893,7 @@ static void intel_joiner_adjust_pipe_src(struct intel_crtc_state *crtc_state)
>>   	enum pipe primary_pipe, pipe = crtc->pipe;
>>   	int width;
>>   
>> -	if (num_pipes < 2)
>> +	if (num_pipes == 1)
>>   		return;
>>   
>>   	primary_pipe = joiner_primary_pipe(crtc_state);
>> -- 
>> 2.45.2

Re: [PATCH 07/19] drm/i915/display: Use joined pipes in intel_dp_joiner_needs_dsc

[Nautiyal, Ankit K]

On 9/12/2024 1:47 AM, Ville Syrjälä wrote:
> On Wed, Sep 11, 2024 at 06:43:37PM +0530, Ankit Nautiyal wrote:
>> In preparation of ultrajoiner, use number of joined pipes in the
>> intel_dp_joiner_needs_dsc helper, instead of joiner flag.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c     | 9 +++++----
>>   drivers/gpu/drm/i915/display/intel_dp.h     | 3 ++-
>>   drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 +++--
>>   3 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 65044f63d1f5..6c503b5968d2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -1412,7 +1412,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>>   		dsc = dsc_max_compressed_bpp && dsc_slice_count;
>>   	}
>>   
>> -	if (intel_dp_joiner_needs_dsc(dev_priv, joiner) && !dsc)
>> +	if (intel_dp_joiner_needs_dsc(dev_priv, num_joined_pipes) && !dsc)
>>   		return MODE_CLOCK_HIGH;
>>   
>>   	if (mode_rate > max_rate && !dsc)
>> @@ -2520,14 +2520,15 @@ int intel_dp_config_required_rate(const struct intel_crtc_state *crtc_state)
>>   	return intel_dp_link_required(adjusted_mode->crtc_clock, bpp);
>>   }
>>   
>> -bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915, bool use_joiner)
>> +bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915,
>> +			       int num_joined_pipes)
>>   {
>>   	/*
>>   	 * Pipe joiner needs compression up to display 12 due to bandwidth
>>   	 * limitation. DG2 onwards pipe joiner can be enabled without
>>   	 * compression.
>>   	 */
>> -	return DISPLAY_VER(i915) < 13 && use_joiner;
>> +	return DISPLAY_VER(i915) < 13 && (num_joined_pipes == 2);
> Pointless parens.

Noted. Will remove these.

Besides this, I am wondering if I should change the variable 
num_joined_pipes to num_pipes or num_pipes_used? So its values 1, 2, 4 
make more sense?


Regards,

Ankit

>
>>   }
>>   
>>   static int
>> @@ -2558,7 +2559,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>>   	if (num_joined_pipes == 2)
>>   		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
>>   
>> -	joiner_needs_dsc = intel_dp_joiner_needs_dsc(i915, pipe_config->joiner_pipes);
>> +	joiner_needs_dsc = intel_dp_joiner_needs_dsc(i915, num_joined_pipes);
>>   
>>   	dsc_needed = joiner_needs_dsc || intel_dp->force_dsc_en ||
>>   		     !intel_dp_compute_config_limits(intel_dp, pipe_config,
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
>> index bc9a82d82df2..cc08a309eb78 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>> @@ -123,7 +123,8 @@ int intel_dp_effective_data_rate(int pixel_clock, int bpp_x16,
>>   				 int bw_overhead);
>>   int intel_dp_max_link_data_rate(struct intel_dp *intel_dp,
>>   				int max_dprx_rate, int max_dprx_lanes);
>> -bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915, bool use_joiner);
>> +bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915,
>> +			       int num_joined_pipes);
>>   bool intel_dp_has_joiner(struct intel_dp *intel_dp);
>>   bool intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state,
>>   			    const struct drm_connector_state *conn_state);
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> index 383b3e38df52..fb067749f3a0 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> @@ -588,7 +588,7 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
>>   	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>>   	pipe_config->has_pch_encoder = false;
>>   
>> -	joiner_needs_dsc = intel_dp_joiner_needs_dsc(dev_priv, pipe_config->joiner_pipes);
>> +	joiner_needs_dsc = intel_dp_joiner_needs_dsc(dev_priv, num_joined_pipes);
>>   
>>   	dsc_needed = joiner_needs_dsc || intel_dp->force_dsc_en ||
>>   		     !intel_dp_mst_compute_config_limits(intel_dp,
>> @@ -1472,6 +1472,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>>   							 mode->hdisplay, target_clock);
>>   	if (num_joined_pipes == 2) {
>>   		joiner = true;
>> +		num_joined_pipes = 2;
>>   		max_dotclk *= 2;
>>   	}
>>   
>> @@ -1512,7 +1513,7 @@ intel_dp_mst_mode_valid_ctx(struct drm_connector *connector,
>>   		dsc = dsc_max_compressed_bpp && dsc_slice_count;
>>   	}
>>   
>> -	if (intel_dp_joiner_needs_dsc(dev_priv, joiner) && !dsc) {
>> +	if (intel_dp_joiner_needs_dsc(dev_priv, num_joined_pipes) && !dsc) {
>>   		*status = MODE_CLOCK_HIGH;
>>   		return 0;
>>   	}
>> -- 
>> 2.45.2

Re: [PATCH 07/19] drm/i915/display: Use joined pipes in intel_dp_joiner_needs_dsc

[Ville Syrjälä]

On Thu, Sep 12, 2024 at 03:50:34PM +0530, Nautiyal, Ankit K wrote:
> 
> On 9/12/2024 1:47 AM, Ville Syrjälä wrote:
> > On Wed, Sep 11, 2024 at 06:43:37PM +0530, Ankit Nautiyal wrote:
> >> In preparation of ultrajoiner, use number of joined pipes in the
> >> intel_dp_joiner_needs_dsc helper, instead of joiner flag.
> >>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_dp.c     | 9 +++++----
> >>   drivers/gpu/drm/i915/display/intel_dp.h     | 3 ++-
> >>   drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 +++--
> >>   3 files changed, 10 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >> index 65044f63d1f5..6c503b5968d2 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> @@ -1412,7 +1412,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
> >>   		dsc = dsc_max_compressed_bpp && dsc_slice_count;
> >>   	}
> >>   
> >> -	if (intel_dp_joiner_needs_dsc(dev_priv, joiner) && !dsc)
> >> +	if (intel_dp_joiner_needs_dsc(dev_priv, num_joined_pipes) && !dsc)
> >>   		return MODE_CLOCK_HIGH;
> >>   
> >>   	if (mode_rate > max_rate && !dsc)
> >> @@ -2520,14 +2520,15 @@ int intel_dp_config_required_rate(const struct intel_crtc_state *crtc_state)
> >>   	return intel_dp_link_required(adjusted_mode->crtc_clock, bpp);
> >>   }
> >>   
> >> -bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915, bool use_joiner)
> >> +bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915,
> >> +			       int num_joined_pipes)
> >>   {
> >>   	/*
> >>   	 * Pipe joiner needs compression up to display 12 due to bandwidth
> >>   	 * limitation. DG2 onwards pipe joiner can be enabled without
> >>   	 * compression.
> >>   	 */
> >> -	return DISPLAY_VER(i915) < 13 && use_joiner;
> >> +	return DISPLAY_VER(i915) < 13 && (num_joined_pipes == 2);
> > Pointless parens.
> 
> Noted. Will remove these.
> 
> Besides this, I am wondering if I should change the variable 
> num_joined_pipes to num_pipes or num_pipes_used? So its values 1, 2, 4 
> make more sense?

I think 'num_joined_pipes' might be the best since it reminds the
casual reader that this can have something do with joining, as
opposed to just about the number of pipes generally available on
the platform/etc.

-- 
Ville Syrjälä
Intel

Re: [PATCH 00/19] Ultrajoiner basic functionality series

[Nautiyal, Ankit K]

On 9/12/2024 4:35 AM, Ville Syrjälä wrote:
> On Wed, Sep 11, 2024 at 06:43:30PM +0530, Ankit Nautiyal wrote:
>> This patch series attempts to implement basic support
>> for Ultrajoiner functionality.
>>
>> Rev6:
>> -Upgrade the debugfs functionality to enable the joining of a
>> specified number of pipes.
>> -Modify the display helpers reliant on the pipe joiner mechanism
>> to use number of pipes joined, instead of joiner flag.
>> -Checkpatch fixes.
>>
>> Rev7:
>> -Use struct intel_display, minor refactoring, and rebase.
>>
>> Rev8:
>> -Address comments from Ville.
>> -Simplified debugfs for forcing joiner, and added option to disable
>> joiner.
>> -Modified the ultra/bigjoiner helpers as suggested by Ville.
>> -Split few of the bigger patches as suggested.
>>
>> Test-with: 20240911072204.22825-1-karthik.b.s@intel.com
>>
>> Ankit Nautiyal (11):
>>    drm/i915/display: Simplify intel_joiner_num_pipes and its usage
>>    drm/i915/display: Use joined pipes in intel_dp_joiner_needs_dsc
>>    drm/i915/display: Use joined pipes in intel_mode_valid_max_plane_size
>>    drm/i915/display: Use joined pipes in dsc helpers for slices, bpp
> I have a feeling those four at least could be merged ahead of
> time, and thus shrink what's left quite decently already.
> Granted, you probably won't have the num_pipes value passed
> in all the way from the top yet, so might have to do some
> 'joiner ? 2 : 1' here and there as a temporary measure.
>
> I think getting this stuff out of the way would help
> avoid confusion with the debugfs/compute_config changes
> to some degree at least, and might help with bisectability,
> Presumably it builds, but I'm not really convinced all the
> intermediate steps would actually work 100% correctly.

Thanks for the comments and suggestions.

Agreed, will send this as a separate patch series shortly.

>
>>    drm/i915/display: Check whether platform supports joiner
> For this stuff I'd like to see a HAS_BIGJOINER() and
> HAS_UNCOMPRESSED_JOINER() like we have for most platform
> features. This also seems like good material for stuff to
> merge on its own, ahead of the rest of the more complicated
> stuff below.

Alright will make the changes accordingly, and send separately.


Thanks & Regards,

Ankit

>
>>    drm/i915/display: Modify debugfs for joiner to force n pipes
>>    drm/i915/display_debugfs: Allow force joiner only if supported
>>    drm/i915/dp: Add helper to compute num pipes joined
>>    drm/i915/display: Add debugfs support to avoid joiner
>>    drm/i915/display: Consider ultrajoiner for computing maxdotclock
>>    drm/i915/intel_dp: Add support for forcing ultrajoiner
>>
>> Stanislav Lisovskiy (8):
>>    drm/i915: Add some essential functionality for joiners
>>    drm/i915: Split current joiner hw state readout
>>    drm/i915: Add bigjoiner and uncompressed joiner hw readout sanity
>>      checks
>>    drm/i915: Implement hw state readout and checks for ultrajoiner
>>    drm/i915/display: Percolate ultrajoiner info to get_joiner_config
>>    drm/i915/display/vdsc: Add ultrajoiner support with DSC
>>    drm/i915: Add new abstraction layer to handle pipe order for different
>>      joiners
>>    drm/i915: Compute config and mode valid changes for ultrajoiner
>>
>>   drivers/gpu/drm/i915/display/intel_ddi.c      |  19 +-
>>   drivers/gpu/drm/i915/display/intel_display.c  | 423 +++++++++++++++---
>>   drivers/gpu/drm/i915/display/intel_display.h  |  17 +-
>>   .../drm/i915/display/intel_display_debugfs.c  |  84 +++-
>>   .../drm/i915/display/intel_display_types.h    |   2 +-
>>   drivers/gpu/drm/i915/display/intel_dp.c       | 138 ++++--
>>   drivers/gpu/drm/i915/display/intel_dp.h       |  14 +-
>>   drivers/gpu/drm/i915/display/intel_dp_mst.c   |  54 ++-
>>   drivers/gpu/drm/i915/display/intel_dsi.c      |   2 +-
>>   drivers/gpu/drm/i915/display/intel_hdmi.c     |   2 +-
>>   drivers/gpu/drm/i915/display/intel_vdsc.c     |  30 +-
>>   .../gpu/drm/i915/display/intel_vdsc_regs.h    |   3 +
>>   12 files changed, 646 insertions(+), 142 deletions(-)
>>
>> -- 
>> 2.45.2

Re: [PATCH 07/19] drm/i915/display: Use joined pipes in intel_dp_joiner_needs_dsc

[Nautiyal, Ankit K]

On 9/12/2024 4:28 PM, Ville Syrjälä wrote:
> On Thu, Sep 12, 2024 at 03:50:34PM +0530, Nautiyal, Ankit K wrote:
>> On 9/12/2024 1:47 AM, Ville Syrjälä wrote:
>>> On Wed, Sep 11, 2024 at 06:43:37PM +0530, Ankit Nautiyal wrote:
>>>> In preparation of ultrajoiner, use number of joined pipes in the
>>>> intel_dp_joiner_needs_dsc helper, instead of joiner flag.
>>>>
>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/display/intel_dp.c     | 9 +++++----
>>>>    drivers/gpu/drm/i915/display/intel_dp.h     | 3 ++-
>>>>    drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 +++--
>>>>    3 files changed, 10 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>>>> index 65044f63d1f5..6c503b5968d2 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>>> @@ -1412,7 +1412,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>>>>    		dsc = dsc_max_compressed_bpp && dsc_slice_count;
>>>>    	}
>>>>    
>>>> -	if (intel_dp_joiner_needs_dsc(dev_priv, joiner) && !dsc)
>>>> +	if (intel_dp_joiner_needs_dsc(dev_priv, num_joined_pipes) && !dsc)
>>>>    		return MODE_CLOCK_HIGH;
>>>>    
>>>>    	if (mode_rate > max_rate && !dsc)
>>>> @@ -2520,14 +2520,15 @@ int intel_dp_config_required_rate(const struct intel_crtc_state *crtc_state)
>>>>    	return intel_dp_link_required(adjusted_mode->crtc_clock, bpp);
>>>>    }
>>>>    
>>>> -bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915, bool use_joiner)
>>>> +bool intel_dp_joiner_needs_dsc(struct drm_i915_private *i915,
>>>> +			       int num_joined_pipes)
>>>>    {
>>>>    	/*
>>>>    	 * Pipe joiner needs compression up to display 12 due to bandwidth
>>>>    	 * limitation. DG2 onwards pipe joiner can be enabled without
>>>>    	 * compression.
>>>>    	 */
>>>> -	return DISPLAY_VER(i915) < 13 && use_joiner;
>>>> +	return DISPLAY_VER(i915) < 13 && (num_joined_pipes == 2);
>>> Pointless parens.
>> Noted. Will remove these.
>>
>> Besides this, I am wondering if I should change the variable
>> num_joined_pipes to num_pipes or num_pipes_used? So its values 1, 2, 4
>> make more sense?
> I think 'num_joined_pipes' might be the best since it reminds the
> casual reader that this can have something do with joining, as
> opposed to just about the number of pipes generally available on
> the platform/etc.

Got it! Thanks again!

Regards,

Ankit

>

Re: [PATCH 16/19] drm/i915: Add new abstraction layer to handle pipe order for different joiners

[Nautiyal, Ankit K]

On 9/12/2024 4:08 AM, Ville Syrjälä wrote:
> On Wed, Sep 11, 2024 at 06:43:46PM +0530, Ankit Nautiyal wrote:
>> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>>
>> Ultrajoiner case requires special treatment where both reverse and
>> staight order iteration doesn't work(for instance disabling case requires
>> order to be: primary master, slaves, secondary master).
>>
>> Lets unify our approach by using not only pipe masks for iterating required
>> pipes based on joiner type used, but also using different "priority" arrays
>> for each of those.
>>
>> v2: Fix checkpatch warnings. (Ankit)
>>
>> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_ddi.c     | 19 +++--
>>   drivers/gpu/drm/i915/display/intel_display.c | 83 ++++++++++++++++----
>>   drivers/gpu/drm/i915/display/intel_display.h |  7 ++
>>   drivers/gpu/drm/i915/display/intel_dp_mst.c  | 18 +++--
>>   4 files changed, 96 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index 00fbe9f8c03a..2c064b6c6d01 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -3116,10 +3116,11 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
>>   					       const struct drm_connector_state *old_conn_state)
>>   {
>>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> -	struct intel_crtc *pipe_crtc;
>> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
>>   
>> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
>> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
>> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
>> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
>> +					     intel_get_pipe_order_disable(old_crtc_state)) {
>>   		const struct intel_crtc_state *old_pipe_crtc_state =
>>   			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>>   
>> @@ -3130,8 +3131,9 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
>>   
>>   	intel_ddi_disable_transcoder_func(old_crtc_state);
>>   
>> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
>> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
>> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
>> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
>> +					     intel_get_pipe_order_disable(old_crtc_state)) {
>>   		const struct intel_crtc_state *old_pipe_crtc_state =
>>   			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>>   
>> @@ -3383,7 +3385,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
>>   			     const struct drm_connector_state *conn_state)
>>   {
>>   	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> -	struct intel_crtc *pipe_crtc;
>> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
>>   
>>   	intel_ddi_enable_transcoder_func(encoder, crtc_state);
>>   
>> @@ -3394,8 +3396,9 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
>>   
>>   	intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
>>   
>> -	for_each_intel_crtc_in_pipe_mask_reverse(&i915->drm, pipe_crtc,
>> -						 intel_crtc_joined_pipe_mask(crtc_state)) {
>> +	for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
>> +					     intel_crtc_joined_pipe_mask(crtc_state),
>> +					     intel_get_pipe_order_enable(crtc_state)) {
>>   		const struct intel_crtc_state *pipe_crtc_state =
>>   			intel_atomic_get_new_crtc_state(state, pipe_crtc);
>>   
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index db27850b2c36..27622d51a473 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -1737,6 +1737,50 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
>>   	hsw_set_transconf(crtc_state);
>>   }
>>   
>> +static
>> +bool intel_crtc_is_bigjoiner(const struct intel_crtc_state *pipe_config)
>> +{
>> +	return hweight8(pipe_config->joiner_pipes) == 2;
>> +}
>> +
>> +const enum pipe *intel_get_pipe_order_enable(const struct intel_crtc_state *crtc_state)
>> +{
>> +	static const enum pipe ultrajoiner_pipe_order_enable[I915_MAX_PIPES] = {
>> +		PIPE_B, PIPE_D, PIPE_C, PIPE_A
>> +	};
>> +	static const enum pipe bigjoiner_pipe_order_enable[I915_MAX_PIPES] = {
>> +		PIPE_B, PIPE_A, PIPE_D, PIPE_C
>> +	};
>> +	static const enum pipe nojoiner_pipe_order_enable[I915_MAX_PIPES] = {
>> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
>> +	};
>> +
>> +	if (intel_crtc_is_ultrajoiner(crtc_state))
>> +		return ultrajoiner_pipe_order_enable;
>> +	else if (intel_crtc_is_bigjoiner(crtc_state))
>> +		return bigjoiner_pipe_order_enable;
>> +	return nojoiner_pipe_order_enable;
>> +}
>> +
>> +const enum pipe *intel_get_pipe_order_disable(const struct intel_crtc_state *crtc_state)
>> +{
>> +	static const enum pipe ultrajoiner_pipe_order_disable[I915_MAX_PIPES] = {
>> +		PIPE_A, PIPE_B, PIPE_D, PIPE_C
>> +	};
>> +	static const enum pipe bigjoiner_pipe_order_disable[I915_MAX_PIPES] = {
>> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
>> +	};
>> +	static const enum pipe nojoiner_pipe_order_disable[I915_MAX_PIPES] = {
>> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
>> +	};
>> +
>> +	if (intel_crtc_is_ultrajoiner(crtc_state))
>> +		return ultrajoiner_pipe_order_disable;
>> +	else if (intel_crtc_is_bigjoiner(crtc_state))
>> +		return bigjoiner_pipe_order_disable;
>> +	return nojoiner_pipe_order_disable;
> I don't think we should need all those diffrent order array. Technically
> one should do. Though having two might make sense.
>
> Another problem is the hardcoded pipes. If we eg. get hardware that
> would support ultrajoiner on pipes B-E in the future this would no
> longer  work.
>
>> +}
> <snip>
>> +#define for_each_intel_crtc_in_mask_priority(__dev_priv, intel_crtc, __p, __mask, __priolist) \
>> +	for_each_pipe(__dev_priv, __p) \
>> +		for_each_if((__mask) & BIT(__priolist[__p])) \
>> +			for_each_if(intel_crtc = intel_crtc_for_pipe(to_intel_display(&__dev_priv->drm), __priolist[__p]))
>
> I think something like:
>
> const u8 intel_pipe_order_enable[4] = {
>          3, 1, 2, 0,
> };
>
> const u8 intel_pipe_order_disable[4] = {
>          0, 2, 1, 3,
> };
>
> #define for_each_intel_crtc_in_pipe_mask_ordered(crtc, pipe_masks, order, i) \
>          for ((i) = 0; \
>               (i) < ARRAY_SIZE(order) && \
>               ((crtc) = intel_crtc_for_pipe(joiner_primary_pipe(pipe_mask) + (order)[(i)]), 1); \
>               (i)++) \
>                  for_each_if((crtc) && (pipe_mask) & BIT((crtc)->pipe))
>
> would let us avoid that hardcoded pipe stuff, and everything is
> just based on the relative order between the pipes. The same orders
> also work for bigjoiner and non-joined cases (it just skips the pipes
> that are't in the mask).
>
>
> The alternative would be to just use the bigjoiner primary+secondary masks
> and come up with a a way to iterate two bitmask in either forward or reverse
> order. Hmm, I suppose one might just combine the bigjoiner primary and
> secondary masks into one, with one of them shifted up to some high bits,
> and then iterate the combined bitmask either forward or backward.
>
> Something like this should work:
> #define for_each_crtc_in_masks(crtc, first_pipes, second_pipes, pipes, i) \
>          for ((i) = 0, (pipes) = (second_pipes) << 16 | (first_pipes); \
>               (i) < 32 && ((crtc) = intel_crtc_for_pipe((i) & 15), 1); \
>               (i)++) \
>                  for_each_if((crtc) && (pipes) & BIT(i))
>
> #define for_each_crtc_in_masks_reverse(crtc, first_pipes, second_pipes, pipes, i) \
>          for ((i) = 31, (pipes) = (first_pipes) << 16 | (second_pipes); \
>               (i) >= 0 && ((crtc) = intel_crtc_for_pipe((i) & 15), 1); \
>               (i)--) \
>                  for_each_if((crtc) && (pipes) & BIT(i))
>
> (could reduce the constants a bit given we don't have 16 pipes).

This looks good to me. changed for 4 pipes, as below:


#define for_each_crtc_in_masks(crtc, first_pipes, second_pipes, pipes, i) \
         for ((i) = 0, (pipes) = (first_pipes) | ((second_pipes) << 4); \
              (i) < 8 && ((crtc) = intel_crtc_for_pipe((i & 3)), 1); \
              (i)++) \
                 for_each_if((crtc) && (pipes) & BIT(i))

#define for_each_crtc_in_masks_reverse(crtc, first_pipes, second_pipes, 
pipes, i) \
         for ((i) = 7, (pipes) = (first_pipes) | ((second_pipes) << 4); \
              (i) >= 0 && ((crtc) = intel_crtc_for_pipe((i & 3)), 1); \
              (i)--) \
                 for_each_if((crtc) && (pipes) & BIT(i))

But, for non joiner case, when the bigjoiner_primary/secondary_pipes are 
0 so pipes will be 0.

How to work with non joiner case?

what if we do : (pipes) = ((first_pipes) | ((second_pipes) << 4)) ? 
((first_pipes) | ((second_pipes) << 4)) : 0xF; \

So if pipes is computed to be 0, we use pipe:0xF

Regards,

Ankit

>
> And then we'd just use them like so:
> for_each_crtc_in_masks_reverse(crtc, bigjoiner_secondary_pipes(),
> 				bigjoiner_primary_pipes(), pipes, i)
> 	enable();
>
> for_each_crtc_in_masks(crtc, bigjoiner_primary_pipes(),
> 			bigjoiner_secondary_pipes(), pipes, i)
> 	disable();
>
> Would at least be very close to what we already have, just splitting
> the pipe masks into two essentially.

>
>> +
>>   #define for_each_intel_crtc_in_pipe_mask(dev, intel_crtc, pipe_mask)	\
>>   	list_for_each_entry(intel_crtc,					\
>>   			    &(dev)->mode_config.crtc_list,		\
>> @@ -431,6 +436,8 @@ bool intel_crtc_is_ultrajoiner(const struct intel_crtc_state *crtc_state);
>>   bool intel_crtc_is_ultrajoiner_primary(const struct intel_crtc_state *crtc_state);
>>   u8 intel_crtc_joiner_secondary_pipes(const struct intel_crtc_state *crtc_state);
>>   struct intel_crtc *intel_primary_crtc(const struct intel_crtc_state *crtc_state);
>> +const enum pipe *intel_get_pipe_order_enable(const struct intel_crtc_state *crtc_state);
>> +const enum pipe *intel_get_pipe_order_disable(const struct intel_crtc_state *crtc_state);
>>   bool intel_crtc_get_pipe_config(struct intel_crtc_state *crtc_state);
>>   bool intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>>   			       const struct intel_crtc_state *pipe_config,
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> index be79783ce09b..1c87f81568c8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> @@ -1003,7 +1003,7 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>>   	struct drm_dp_mst_atomic_payload *new_payload =
>>   		drm_atomic_get_mst_payload_state(new_mst_state, connector->port);
>>   	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>> -	struct intel_crtc *pipe_crtc;
>> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
>>   	bool last_mst_stream;
>>   
>>   	intel_dp->active_mst_links--;
>> @@ -1012,8 +1012,9 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>>   		    DISPLAY_VER(dev_priv) >= 12 && last_mst_stream &&
>>   		    !intel_dp_mst_is_master_trans(old_crtc_state));
>>   
>> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
>> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
>> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
>> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
>> +					     intel_get_pipe_order_disable(old_crtc_state)) {
>>   		const struct intel_crtc_state *old_pipe_crtc_state =
>>   			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>>   
>> @@ -1037,8 +1038,9 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>>   
>>   	intel_ddi_disable_transcoder_func(old_crtc_state);
>>   
>> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
>> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
>> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
>> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
>> +					     intel_get_pipe_order_disable(old_crtc_state)) {
>>   		const struct intel_crtc_state *old_pipe_crtc_state =
>>   			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>>   
>> @@ -1257,6 +1259,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
>>   	enum transcoder trans = pipe_config->cpu_transcoder;
>>   	bool first_mst_stream = intel_dp->active_mst_links == 1;
>>   	struct intel_crtc *pipe_crtc;
>> +	enum pipe pipe;
>>   	int ret;
>>   
>>   	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
>> @@ -1304,8 +1307,9 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
>>   
>>   	intel_enable_transcoder(pipe_config);
>>   
>> -	for_each_intel_crtc_in_pipe_mask_reverse(&dev_priv->drm, pipe_crtc,
>> -						 intel_crtc_joined_pipe_mask(pipe_config)) {
>> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
>> +					     intel_crtc_joined_pipe_mask(pipe_config),
>> +					     intel_get_pipe_order_enable(pipe_config)) {
>>   		const struct intel_crtc_state *pipe_crtc_state =
>>   			intel_atomic_get_new_crtc_state(state, pipe_crtc);
>>   
>> -- 
>> 2.45.2

Re: [PATCH 16/19] drm/i915: Add new abstraction layer to handle pipe order for different joiners

[Ville Syrjälä]

On Mon, Sep 16, 2024 at 01:09:42PM +0530, Nautiyal, Ankit K wrote:
> 
> On 9/12/2024 4:08 AM, Ville Syrjälä wrote:
> > On Wed, Sep 11, 2024 at 06:43:46PM +0530, Ankit Nautiyal wrote:
> >> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> >>
> >> Ultrajoiner case requires special treatment where both reverse and
> >> staight order iteration doesn't work(for instance disabling case requires
> >> order to be: primary master, slaves, secondary master).
> >>
> >> Lets unify our approach by using not only pipe masks for iterating required
> >> pipes based on joiner type used, but also using different "priority" arrays
> >> for each of those.
> >>
> >> v2: Fix checkpatch warnings. (Ankit)
> >>
> >> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_ddi.c     | 19 +++--
> >>   drivers/gpu/drm/i915/display/intel_display.c | 83 ++++++++++++++++----
> >>   drivers/gpu/drm/i915/display/intel_display.h |  7 ++
> >>   drivers/gpu/drm/i915/display/intel_dp_mst.c  | 18 +++--
> >>   4 files changed, 96 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> >> index 00fbe9f8c03a..2c064b6c6d01 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> >> @@ -3116,10 +3116,11 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
> >>   					       const struct drm_connector_state *old_conn_state)
> >>   {
> >>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >> -	struct intel_crtc *pipe_crtc;
> >> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
> >>   
> >> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> >> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
> >> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> >> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
> >> +					     intel_get_pipe_order_disable(old_crtc_state)) {
> >>   		const struct intel_crtc_state *old_pipe_crtc_state =
> >>   			intel_atomic_get_old_crtc_state(state, pipe_crtc);
> >>   
> >> @@ -3130,8 +3131,9 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
> >>   
> >>   	intel_ddi_disable_transcoder_func(old_crtc_state);
> >>   
> >> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> >> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
> >> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> >> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
> >> +					     intel_get_pipe_order_disable(old_crtc_state)) {
> >>   		const struct intel_crtc_state *old_pipe_crtc_state =
> >>   			intel_atomic_get_old_crtc_state(state, pipe_crtc);
> >>   
> >> @@ -3383,7 +3385,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
> >>   			     const struct drm_connector_state *conn_state)
> >>   {
> >>   	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >> -	struct intel_crtc *pipe_crtc;
> >> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
> >>   
> >>   	intel_ddi_enable_transcoder_func(encoder, crtc_state);
> >>   
> >> @@ -3394,8 +3396,9 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
> >>   
> >>   	intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
> >>   
> >> -	for_each_intel_crtc_in_pipe_mask_reverse(&i915->drm, pipe_crtc,
> >> -						 intel_crtc_joined_pipe_mask(crtc_state)) {
> >> +	for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
> >> +					     intel_crtc_joined_pipe_mask(crtc_state),
> >> +					     intel_get_pipe_order_enable(crtc_state)) {
> >>   		const struct intel_crtc_state *pipe_crtc_state =
> >>   			intel_atomic_get_new_crtc_state(state, pipe_crtc);
> >>   
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >> index db27850b2c36..27622d51a473 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> @@ -1737,6 +1737,50 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
> >>   	hsw_set_transconf(crtc_state);
> >>   }
> >>   
> >> +static
> >> +bool intel_crtc_is_bigjoiner(const struct intel_crtc_state *pipe_config)
> >> +{
> >> +	return hweight8(pipe_config->joiner_pipes) == 2;
> >> +}
> >> +
> >> +const enum pipe *intel_get_pipe_order_enable(const struct intel_crtc_state *crtc_state)
> >> +{
> >> +	static const enum pipe ultrajoiner_pipe_order_enable[I915_MAX_PIPES] = {
> >> +		PIPE_B, PIPE_D, PIPE_C, PIPE_A
> >> +	};
> >> +	static const enum pipe bigjoiner_pipe_order_enable[I915_MAX_PIPES] = {
> >> +		PIPE_B, PIPE_A, PIPE_D, PIPE_C
> >> +	};
> >> +	static const enum pipe nojoiner_pipe_order_enable[I915_MAX_PIPES] = {
> >> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> >> +	};
> >> +
> >> +	if (intel_crtc_is_ultrajoiner(crtc_state))
> >> +		return ultrajoiner_pipe_order_enable;
> >> +	else if (intel_crtc_is_bigjoiner(crtc_state))
> >> +		return bigjoiner_pipe_order_enable;
> >> +	return nojoiner_pipe_order_enable;
> >> +}
> >> +
> >> +const enum pipe *intel_get_pipe_order_disable(const struct intel_crtc_state *crtc_state)
> >> +{
> >> +	static const enum pipe ultrajoiner_pipe_order_disable[I915_MAX_PIPES] = {
> >> +		PIPE_A, PIPE_B, PIPE_D, PIPE_C
> >> +	};
> >> +	static const enum pipe bigjoiner_pipe_order_disable[I915_MAX_PIPES] = {
> >> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> >> +	};
> >> +	static const enum pipe nojoiner_pipe_order_disable[I915_MAX_PIPES] = {
> >> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> >> +	};
> >> +
> >> +	if (intel_crtc_is_ultrajoiner(crtc_state))
> >> +		return ultrajoiner_pipe_order_disable;
> >> +	else if (intel_crtc_is_bigjoiner(crtc_state))
> >> +		return bigjoiner_pipe_order_disable;
> >> +	return nojoiner_pipe_order_disable;
> > I don't think we should need all those diffrent order array. Technically
> > one should do. Though having two might make sense.
> >
> > Another problem is the hardcoded pipes. If we eg. get hardware that
> > would support ultrajoiner on pipes B-E in the future this would no
> > longer  work.
> >
> >> +}
> > <snip>
> >> +#define for_each_intel_crtc_in_mask_priority(__dev_priv, intel_crtc, __p, __mask, __priolist) \
> >> +	for_each_pipe(__dev_priv, __p) \
> >> +		for_each_if((__mask) & BIT(__priolist[__p])) \
> >> +			for_each_if(intel_crtc = intel_crtc_for_pipe(to_intel_display(&__dev_priv->drm), __priolist[__p]))
> >
> > I think something like:
> >
> > const u8 intel_pipe_order_enable[4] = {
> >          3, 1, 2, 0,
> > };
> >
> > const u8 intel_pipe_order_disable[4] = {
> >          0, 2, 1, 3,
> > };
> >
> > #define for_each_intel_crtc_in_pipe_mask_ordered(crtc, pipe_masks, order, i) \
> >          for ((i) = 0; \
> >               (i) < ARRAY_SIZE(order) && \
> >               ((crtc) = intel_crtc_for_pipe(joiner_primary_pipe(pipe_mask) + (order)[(i)]), 1); \
> >               (i)++) \
> >                  for_each_if((crtc) && (pipe_mask) & BIT((crtc)->pipe))
> >
> > would let us avoid that hardcoded pipe stuff, and everything is
> > just based on the relative order between the pipes. The same orders
> > also work for bigjoiner and non-joined cases (it just skips the pipes
> > that are't in the mask).
> >
> >
> > The alternative would be to just use the bigjoiner primary+secondary masks
> > and come up with a a way to iterate two bitmask in either forward or reverse
> > order. Hmm, I suppose one might just combine the bigjoiner primary and
> > secondary masks into one, with one of them shifted up to some high bits,
> > and then iterate the combined bitmask either forward or backward.
> >
> > Something like this should work:
> > #define for_each_crtc_in_masks(crtc, first_pipes, second_pipes, pipes, i) \
> >          for ((i) = 0, (pipes) = (second_pipes) << 16 | (first_pipes); \
> >               (i) < 32 && ((crtc) = intel_crtc_for_pipe((i) & 15), 1); \
> >               (i)++) \
> >                  for_each_if((crtc) && (pipes) & BIT(i))
> >
> > #define for_each_crtc_in_masks_reverse(crtc, first_pipes, second_pipes, pipes, i) \
> >          for ((i) = 31, (pipes) = (first_pipes) << 16 | (second_pipes); \
> >               (i) >= 0 && ((crtc) = intel_crtc_for_pipe((i) & 15), 1); \
> >               (i)--) \
> >                  for_each_if((crtc) && (pipes) & BIT(i))
> >
> > (could reduce the constants a bit given we don't have 16 pipes).
> 
> This looks good to me. changed for 4 pipes, as below:
> 
> 
> #define for_each_crtc_in_masks(crtc, first_pipes, second_pipes, pipes, i) \
>          for ((i) = 0, (pipes) = (first_pipes) | ((second_pipes) << 4); \
>               (i) < 8 && ((crtc) = intel_crtc_for_pipe((i & 3)), 1); \

We could probably use a single internal define for the magic
number to avoid things going out of sync by accident.

Hmm, maybe even define it as something like
#define _INTEL_MAX_PIPES_POT roundup_power_of_two(I915_MAX_PIPES)
?

O, I suppose we don't really need it to be POT, so we could
just replace the '&' with '%', and then we can just use
I915_MAX_PIPES directly.

>               (i)++) \
>                  for_each_if((crtc) && (pipes) & BIT(i))
> 
> #define for_each_crtc_in_masks_reverse(crtc, first_pipes, second_pipes, 
> pipes, i) \
>          for ((i) = 7, (pipes) = (first_pipes) | ((second_pipes) << 4); \
>               (i) >= 0 && ((crtc) = intel_crtc_for_pipe((i & 3)), 1); \
>               (i)--) \
>                  for_each_if((crtc) && (pipes) & BIT(i))
> 
> But, for non joiner case, when the bigjoiner_primary/secondary_pipes are 
> 0 so pipes will be 0.

Hmm. I think we just need to make bigjoiner_primary_pipes()
return BIT(crtc->pipe) for the non-joiner cases.

Maybe we should rename these to something like
_modeset_{primary,secondary}_pipes() so that people
don't get tempted to use them for anything else?

And then we could hide all this into something like
#define for_each_pipe_crtc_modeset_disable(...) \ 
	for_each_crtc_in_masks(..., _modeset_primary_pipes(), \
			       _modeset_secondary_pipes(), ...)
#define for_each_pipe_crtc_modeset_enable(...) \ 
	for_each_crtc_in_masks_reverse(..., _modeset_secondary_pipes(), \
				      _modeset_primary_pipes(), ...)

-- 
Ville Syrjälä
Intel

Re: [PATCH 16/19] drm/i915: Add new abstraction layer to handle pipe order for different joiners

[Ville Syrjälä]

On Mon, Sep 16, 2024 at 05:54:12PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 16, 2024 at 01:09:42PM +0530, Nautiyal, Ankit K wrote:
> > 
> > On 9/12/2024 4:08 AM, Ville Syrjälä wrote:
> > > On Wed, Sep 11, 2024 at 06:43:46PM +0530, Ankit Nautiyal wrote:
> > >> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > >>
> > >> Ultrajoiner case requires special treatment where both reverse and
> > >> staight order iteration doesn't work(for instance disabling case requires
> > >> order to be: primary master, slaves, secondary master).
> > >>
> > >> Lets unify our approach by using not only pipe masks for iterating required
> > >> pipes based on joiner type used, but also using different "priority" arrays
> > >> for each of those.
> > >>
> > >> v2: Fix checkpatch warnings. (Ankit)
> > >>
> > >> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > >> ---
> > >>   drivers/gpu/drm/i915/display/intel_ddi.c     | 19 +++--
> > >>   drivers/gpu/drm/i915/display/intel_display.c | 83 ++++++++++++++++----
> > >>   drivers/gpu/drm/i915/display/intel_display.h |  7 ++
> > >>   drivers/gpu/drm/i915/display/intel_dp_mst.c  | 18 +++--
> > >>   4 files changed, 96 insertions(+), 31 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > >> index 00fbe9f8c03a..2c064b6c6d01 100644
> > >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > >> @@ -3116,10 +3116,11 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
> > >>   					       const struct drm_connector_state *old_conn_state)
> > >>   {
> > >>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > >> -	struct intel_crtc *pipe_crtc;
> > >> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
> > >>   
> > >> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> > >> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
> > >> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> > >> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
> > >> +					     intel_get_pipe_order_disable(old_crtc_state)) {
> > >>   		const struct intel_crtc_state *old_pipe_crtc_state =
> > >>   			intel_atomic_get_old_crtc_state(state, pipe_crtc);
> > >>   
> > >> @@ -3130,8 +3131,9 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
> > >>   
> > >>   	intel_ddi_disable_transcoder_func(old_crtc_state);
> > >>   
> > >> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> > >> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
> > >> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> > >> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
> > >> +					     intel_get_pipe_order_disable(old_crtc_state)) {
> > >>   		const struct intel_crtc_state *old_pipe_crtc_state =
> > >>   			intel_atomic_get_old_crtc_state(state, pipe_crtc);
> > >>   
> > >> @@ -3383,7 +3385,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
> > >>   			     const struct drm_connector_state *conn_state)
> > >>   {
> > >>   	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > >> -	struct intel_crtc *pipe_crtc;
> > >> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
> > >>   
> > >>   	intel_ddi_enable_transcoder_func(encoder, crtc_state);
> > >>   
> > >> @@ -3394,8 +3396,9 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
> > >>   
> > >>   	intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
> > >>   
> > >> -	for_each_intel_crtc_in_pipe_mask_reverse(&i915->drm, pipe_crtc,
> > >> -						 intel_crtc_joined_pipe_mask(crtc_state)) {
> > >> +	for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
> > >> +					     intel_crtc_joined_pipe_mask(crtc_state),
> > >> +					     intel_get_pipe_order_enable(crtc_state)) {
> > >>   		const struct intel_crtc_state *pipe_crtc_state =
> > >>   			intel_atomic_get_new_crtc_state(state, pipe_crtc);
> > >>   
> > >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > >> index db27850b2c36..27622d51a473 100644
> > >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> > >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > >> @@ -1737,6 +1737,50 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
> > >>   	hsw_set_transconf(crtc_state);
> > >>   }
> > >>   
> > >> +static
> > >> +bool intel_crtc_is_bigjoiner(const struct intel_crtc_state *pipe_config)
> > >> +{
> > >> +	return hweight8(pipe_config->joiner_pipes) == 2;
> > >> +}
> > >> +
> > >> +const enum pipe *intel_get_pipe_order_enable(const struct intel_crtc_state *crtc_state)
> > >> +{
> > >> +	static const enum pipe ultrajoiner_pipe_order_enable[I915_MAX_PIPES] = {
> > >> +		PIPE_B, PIPE_D, PIPE_C, PIPE_A
> > >> +	};
> > >> +	static const enum pipe bigjoiner_pipe_order_enable[I915_MAX_PIPES] = {
> > >> +		PIPE_B, PIPE_A, PIPE_D, PIPE_C
> > >> +	};
> > >> +	static const enum pipe nojoiner_pipe_order_enable[I915_MAX_PIPES] = {
> > >> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> > >> +	};
> > >> +
> > >> +	if (intel_crtc_is_ultrajoiner(crtc_state))
> > >> +		return ultrajoiner_pipe_order_enable;
> > >> +	else if (intel_crtc_is_bigjoiner(crtc_state))
> > >> +		return bigjoiner_pipe_order_enable;
> > >> +	return nojoiner_pipe_order_enable;
> > >> +}
> > >> +
> > >> +const enum pipe *intel_get_pipe_order_disable(const struct intel_crtc_state *crtc_state)
> > >> +{
> > >> +	static const enum pipe ultrajoiner_pipe_order_disable[I915_MAX_PIPES] = {
> > >> +		PIPE_A, PIPE_B, PIPE_D, PIPE_C
> > >> +	};
> > >> +	static const enum pipe bigjoiner_pipe_order_disable[I915_MAX_PIPES] = {
> > >> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> > >> +	};
> > >> +	static const enum pipe nojoiner_pipe_order_disable[I915_MAX_PIPES] = {
> > >> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> > >> +	};
> > >> +
> > >> +	if (intel_crtc_is_ultrajoiner(crtc_state))
> > >> +		return ultrajoiner_pipe_order_disable;
> > >> +	else if (intel_crtc_is_bigjoiner(crtc_state))
> > >> +		return bigjoiner_pipe_order_disable;
> > >> +	return nojoiner_pipe_order_disable;
> > > I don't think we should need all those diffrent order array. Technically
> > > one should do. Though having two might make sense.
> > >
> > > Another problem is the hardcoded pipes. If we eg. get hardware that
> > > would support ultrajoiner on pipes B-E in the future this would no
> > > longer  work.
> > >
> > >> +}
> > > <snip>
> > >> +#define for_each_intel_crtc_in_mask_priority(__dev_priv, intel_crtc, __p, __mask, __priolist) \
> > >> +	for_each_pipe(__dev_priv, __p) \
> > >> +		for_each_if((__mask) & BIT(__priolist[__p])) \
> > >> +			for_each_if(intel_crtc = intel_crtc_for_pipe(to_intel_display(&__dev_priv->drm), __priolist[__p]))
> > >
> > > I think something like:
> > >
> > > const u8 intel_pipe_order_enable[4] = {
> > >          3, 1, 2, 0,
> > > };
> > >
> > > const u8 intel_pipe_order_disable[4] = {
> > >          0, 2, 1, 3,
> > > };
> > >
> > > #define for_each_intel_crtc_in_pipe_mask_ordered(crtc, pipe_masks, order, i) \
> > >          for ((i) = 0; \
> > >               (i) < ARRAY_SIZE(order) && \
> > >               ((crtc) = intel_crtc_for_pipe(joiner_primary_pipe(pipe_mask) + (order)[(i)]), 1); \
> > >               (i)++) \
> > >                  for_each_if((crtc) && (pipe_mask) & BIT((crtc)->pipe))
> > >
> > > would let us avoid that hardcoded pipe stuff, and everything is
> > > just based on the relative order between the pipes. The same orders
> > > also work for bigjoiner and non-joined cases (it just skips the pipes
> > > that are't in the mask).
> > >
> > >
> > > The alternative would be to just use the bigjoiner primary+secondary masks
> > > and come up with a a way to iterate two bitmask in either forward or reverse
> > > order. Hmm, I suppose one might just combine the bigjoiner primary and
> > > secondary masks into one, with one of them shifted up to some high bits,
> > > and then iterate the combined bitmask either forward or backward.
> > >
> > > Something like this should work:
> > > #define for_each_crtc_in_masks(crtc, first_pipes, second_pipes, pipes, i) \
> > >          for ((i) = 0, (pipes) = (second_pipes) << 16 | (first_pipes); \
> > >               (i) < 32 && ((crtc) = intel_crtc_for_pipe((i) & 15), 1); \
> > >               (i)++) \
> > >                  for_each_if((crtc) && (pipes) & BIT(i))
> > >
> > > #define for_each_crtc_in_masks_reverse(crtc, first_pipes, second_pipes, pipes, i) \
> > >          for ((i) = 31, (pipes) = (first_pipes) << 16 | (second_pipes); \
> > >               (i) >= 0 && ((crtc) = intel_crtc_for_pipe((i) & 15), 1); \
> > >               (i)--) \
> > >                  for_each_if((crtc) && (pipes) & BIT(i))
> > >
> > > (could reduce the constants a bit given we don't have 16 pipes).
> > 
> > This looks good to me. changed for 4 pipes, as below:
> > 
> > 
> > #define for_each_crtc_in_masks(crtc, first_pipes, second_pipes, pipes, i) \
> >          for ((i) = 0, (pipes) = (first_pipes) | ((second_pipes) << 4); \
> >               (i) < 8 && ((crtc) = intel_crtc_for_pipe((i & 3)), 1); \
> 
> We could probably use a single internal define for the magic
> number to avoid things going out of sync by accident.
> 
> Hmm, maybe even define it as something like
> #define _INTEL_MAX_PIPES_POT roundup_power_of_two(I915_MAX_PIPES)
> ?
> 
> O, I suppose we don't really need it to be POT, so we could
> just replace the '&' with '%', and then we can just use
> I915_MAX_PIPES directly.
> 
> >               (i)++) \
> >                  for_each_if((crtc) && (pipes) & BIT(i))
> > 
> > #define for_each_crtc_in_masks_reverse(crtc, first_pipes, second_pipes, 
> > pipes, i) \
> >          for ((i) = 7, (pipes) = (first_pipes) | ((second_pipes) << 4); \
> >               (i) >= 0 && ((crtc) = intel_crtc_for_pipe((i & 3)), 1); \
> >               (i)--) \
> >                  for_each_if((crtc) && (pipes) & BIT(i))
> > 
> > But, for non joiner case, when the bigjoiner_primary/secondary_pipes are 
> > 0 so pipes will be 0.
> 
> Hmm. I think we just need to make bigjoiner_primary_pipes()
> return BIT(crtc->pipe) for the non-joiner cases.
> 
> Maybe we should rename these to something like
> _modeset_{primary,secondary}_pipes() so that people
> don't get tempted to use them for anything else?
> 
> And then we could hide all this into something like
> #define for_each_pipe_crtc_modeset_disable(...) \ 
> 	for_each_crtc_in_masks(..., _modeset_primary_pipes(), \
> 			       _modeset_secondary_pipes(), ...)
> #define for_each_pipe_crtc_modeset_enable(...) \ 
> 	for_each_crtc_in_masks_reverse(..., _modeset_secondary_pipes(), \
> 				      _modeset_primary_pipes(), ...)

These last two macros you could already implement right
now using the current code, and then we can replace them
with the ultrajoiner capable stuff in another patch and
not touch any of the actual modeset code anymore.

-- 
Ville Syrjälä
Intel

Re: [PATCH 16/19] drm/i915: Add new abstraction layer to handle pipe order for different joiners

[Nautiyal, Ankit K]

On 9/16/2024 8:36 PM, Ville Syrjälä wrote:
> On Mon, Sep 16, 2024 at 05:54:12PM +0300, Ville Syrjälä wrote:
>> On Mon, Sep 16, 2024 at 01:09:42PM +0530, Nautiyal, Ankit K wrote:
>>> On 9/12/2024 4:08 AM, Ville Syrjälä wrote:
>>>> On Wed, Sep 11, 2024 at 06:43:46PM +0530, Ankit Nautiyal wrote:
>>>>> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>>>>>
>>>>> Ultrajoiner case requires special treatment where both reverse and
>>>>> staight order iteration doesn't work(for instance disabling case requires
>>>>> order to be: primary master, slaves, secondary master).
>>>>>
>>>>> Lets unify our approach by using not only pipe masks for iterating required
>>>>> pipes based on joiner type used, but also using different "priority" arrays
>>>>> for each of those.
>>>>>
>>>>> v2: Fix checkpatch warnings. (Ankit)
>>>>>
>>>>> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/display/intel_ddi.c     | 19 +++--
>>>>>    drivers/gpu/drm/i915/display/intel_display.c | 83 ++++++++++++++++----
>>>>>    drivers/gpu/drm/i915/display/intel_display.h |  7 ++
>>>>>    drivers/gpu/drm/i915/display/intel_dp_mst.c  | 18 +++--
>>>>>    4 files changed, 96 insertions(+), 31 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>>>>> index 00fbe9f8c03a..2c064b6c6d01 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>>>>> @@ -3116,10 +3116,11 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
>>>>>    					       const struct drm_connector_state *old_conn_state)
>>>>>    {
>>>>>    	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>>>> -	struct intel_crtc *pipe_crtc;
>>>>> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
>>>>>    
>>>>> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
>>>>> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
>>>>> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
>>>>> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
>>>>> +					     intel_get_pipe_order_disable(old_crtc_state)) {
>>>>>    		const struct intel_crtc_state *old_pipe_crtc_state =
>>>>>    			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>>>>>    
>>>>> @@ -3130,8 +3131,9 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
>>>>>    
>>>>>    	intel_ddi_disable_transcoder_func(old_crtc_state);
>>>>>    
>>>>> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
>>>>> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
>>>>> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
>>>>> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
>>>>> +					     intel_get_pipe_order_disable(old_crtc_state)) {
>>>>>    		const struct intel_crtc_state *old_pipe_crtc_state =
>>>>>    			intel_atomic_get_old_crtc_state(state, pipe_crtc);
>>>>>    
>>>>> @@ -3383,7 +3385,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
>>>>>    			     const struct drm_connector_state *conn_state)
>>>>>    {
>>>>>    	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>>>>> -	struct intel_crtc *pipe_crtc;
>>>>> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
>>>>>    
>>>>>    	intel_ddi_enable_transcoder_func(encoder, crtc_state);
>>>>>    
>>>>> @@ -3394,8 +3396,9 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
>>>>>    
>>>>>    	intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
>>>>>    
>>>>> -	for_each_intel_crtc_in_pipe_mask_reverse(&i915->drm, pipe_crtc,
>>>>> -						 intel_crtc_joined_pipe_mask(crtc_state)) {
>>>>> +	for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
>>>>> +					     intel_crtc_joined_pipe_mask(crtc_state),
>>>>> +					     intel_get_pipe_order_enable(crtc_state)) {
>>>>>    		const struct intel_crtc_state *pipe_crtc_state =
>>>>>    			intel_atomic_get_new_crtc_state(state, pipe_crtc);
>>>>>    
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>>>> index db27850b2c36..27622d51a473 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>>>> @@ -1737,6 +1737,50 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
>>>>>    	hsw_set_transconf(crtc_state);
>>>>>    }
>>>>>    
>>>>> +static
>>>>> +bool intel_crtc_is_bigjoiner(const struct intel_crtc_state *pipe_config)
>>>>> +{
>>>>> +	return hweight8(pipe_config->joiner_pipes) == 2;
>>>>> +}
>>>>> +
>>>>> +const enum pipe *intel_get_pipe_order_enable(const struct intel_crtc_state *crtc_state)
>>>>> +{
>>>>> +	static const enum pipe ultrajoiner_pipe_order_enable[I915_MAX_PIPES] = {
>>>>> +		PIPE_B, PIPE_D, PIPE_C, PIPE_A
>>>>> +	};
>>>>> +	static const enum pipe bigjoiner_pipe_order_enable[I915_MAX_PIPES] = {
>>>>> +		PIPE_B, PIPE_A, PIPE_D, PIPE_C
>>>>> +	};
>>>>> +	static const enum pipe nojoiner_pipe_order_enable[I915_MAX_PIPES] = {
>>>>> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
>>>>> +	};
>>>>> +
>>>>> +	if (intel_crtc_is_ultrajoiner(crtc_state))
>>>>> +		return ultrajoiner_pipe_order_enable;
>>>>> +	else if (intel_crtc_is_bigjoiner(crtc_state))
>>>>> +		return bigjoiner_pipe_order_enable;
>>>>> +	return nojoiner_pipe_order_enable;
>>>>> +}
>>>>> +
>>>>> +const enum pipe *intel_get_pipe_order_disable(const struct intel_crtc_state *crtc_state)
>>>>> +{
>>>>> +	static const enum pipe ultrajoiner_pipe_order_disable[I915_MAX_PIPES] = {
>>>>> +		PIPE_A, PIPE_B, PIPE_D, PIPE_C
>>>>> +	};
>>>>> +	static const enum pipe bigjoiner_pipe_order_disable[I915_MAX_PIPES] = {
>>>>> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
>>>>> +	};
>>>>> +	static const enum pipe nojoiner_pipe_order_disable[I915_MAX_PIPES] = {
>>>>> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
>>>>> +	};
>>>>> +
>>>>> +	if (intel_crtc_is_ultrajoiner(crtc_state))
>>>>> +		return ultrajoiner_pipe_order_disable;
>>>>> +	else if (intel_crtc_is_bigjoiner(crtc_state))
>>>>> +		return bigjoiner_pipe_order_disable;
>>>>> +	return nojoiner_pipe_order_disable;
>>>> I don't think we should need all those diffrent order array. Technically
>>>> one should do. Though having two might make sense.
>>>>
>>>> Another problem is the hardcoded pipes. If we eg. get hardware that
>>>> would support ultrajoiner on pipes B-E in the future this would no
>>>> longer  work.
>>>>
>>>>> +}
>>>> <snip>
>>>>> +#define for_each_intel_crtc_in_mask_priority(__dev_priv, intel_crtc, __p, __mask, __priolist) \
>>>>> +	for_each_pipe(__dev_priv, __p) \
>>>>> +		for_each_if((__mask) & BIT(__priolist[__p])) \
>>>>> +			for_each_if(intel_crtc = intel_crtc_for_pipe(to_intel_display(&__dev_priv->drm), __priolist[__p]))
>>>> I think something like:
>>>>
>>>> const u8 intel_pipe_order_enable[4] = {
>>>>           3, 1, 2, 0,
>>>> };
>>>>
>>>> const u8 intel_pipe_order_disable[4] = {
>>>>           0, 2, 1, 3,
>>>> };


I just realized that as per bspec:54142 sequence for ultrajoiner doesnt 
follow this.

its 1, 3, 2, 0 for enabling and 0, 1, 3, 2 for disabling :(


Regards,

Ankit

>>>>
>>>> #define for_each_intel_crtc_in_pipe_mask_ordered(crtc, pipe_masks, order, i) \
>>>>           for ((i) = 0; \
>>>>                (i) < ARRAY_SIZE(order) && \
>>>>                ((crtc) = intel_crtc_for_pipe(joiner_primary_pipe(pipe_mask) + (order)[(i)]), 1); \
>>>>                (i)++) \
>>>>                   for_each_if((crtc) && (pipe_mask) & BIT((crtc)->pipe))
>>>>
>>>> would let us avoid that hardcoded pipe stuff, and everything is
>>>> just based on the relative order between the pipes. The same orders
>>>> also work for bigjoiner and non-joined cases (it just skips the pipes
>>>> that are't in the mask).
>>>>
>>>>
>>>> The alternative would be to just use the bigjoiner primary+secondary masks
>>>> and come up with a a way to iterate two bitmask in either forward or reverse
>>>> order. Hmm, I suppose one might just combine the bigjoiner primary and
>>>> secondary masks into one, with one of them shifted up to some high bits,
>>>> and then iterate the combined bitmask either forward or backward.
>>>>
>>>> Something like this should work:
>>>> #define for_each_crtc_in_masks(crtc, first_pipes, second_pipes, pipes, i) \
>>>>           for ((i) = 0, (pipes) = (second_pipes) << 16 | (first_pipes); \
>>>>                (i) < 32 && ((crtc) = intel_crtc_for_pipe((i) & 15), 1); \
>>>>                (i)++) \
>>>>                   for_each_if((crtc) && (pipes) & BIT(i))
>>>>
>>>> #define for_each_crtc_in_masks_reverse(crtc, first_pipes, second_pipes, pipes, i) \
>>>>           for ((i) = 31, (pipes) = (first_pipes) << 16 | (second_pipes); \
>>>>                (i) >= 0 && ((crtc) = intel_crtc_for_pipe((i) & 15), 1); \
>>>>                (i)--) \
>>>>                   for_each_if((crtc) && (pipes) & BIT(i))
>>>>
>>>> (could reduce the constants a bit given we don't have 16 pipes).
>>> This looks good to me. changed for 4 pipes, as below:
>>>
>>>
>>> #define for_each_crtc_in_masks(crtc, first_pipes, second_pipes, pipes, i) \
>>>           for ((i) = 0, (pipes) = (first_pipes) | ((second_pipes) << 4); \
>>>                (i) < 8 && ((crtc) = intel_crtc_for_pipe((i & 3)), 1); \
>> We could probably use a single internal define for the magic
>> number to avoid things going out of sync by accident.
>>
>> Hmm, maybe even define it as something like
>> #define _INTEL_MAX_PIPES_POT roundup_power_of_two(I915_MAX_PIPES)
>> ?
>>
>> O, I suppose we don't really need it to be POT, so we could
>> just replace the '&' with '%', and then we can just use
>> I915_MAX_PIPES directly.
>>
>>>                (i)++) \
>>>                   for_each_if((crtc) && (pipes) & BIT(i))
>>>
>>> #define for_each_crtc_in_masks_reverse(crtc, first_pipes, second_pipes,
>>> pipes, i) \
>>>           for ((i) = 7, (pipes) = (first_pipes) | ((second_pipes) << 4); \
>>>                (i) >= 0 && ((crtc) = intel_crtc_for_pipe((i & 3)), 1); \
>>>                (i)--) \
>>>                   for_each_if((crtc) && (pipes) & BIT(i))
>>>
>>> But, for non joiner case, when the bigjoiner_primary/secondary_pipes are
>>> 0 so pipes will be 0.
>> Hmm. I think we just need to make bigjoiner_primary_pipes()
>> return BIT(crtc->pipe) for the non-joiner cases.
>>
>> Maybe we should rename these to something like
>> _modeset_{primary,secondary}_pipes() so that people
>> don't get tempted to use them for anything else?
>>
>> And then we could hide all this into something like
>> #define for_each_pipe_crtc_modeset_disable(...) \
>> 	for_each_crtc_in_masks(..., _modeset_primary_pipes(), \
>> 			       _modeset_secondary_pipes(), ...)
>> #define for_each_pipe_crtc_modeset_enable(...) \
>> 	for_each_crtc_in_masks_reverse(..., _modeset_secondary_pipes(), \
>> 				      _modeset_primary_pipes(), ...)
> These last two macros you could already implement right
> now using the current code, and then we can replace them
> with the ultrajoiner capable stuff in another patch and
> not touch any of the actual modeset code anymore.
>

Re: [PATCH 16/19] drm/i915: Add new abstraction layer to handle pipe order for different joiners

[Ville Syrjälä]

On Tue, Sep 17, 2024 at 02:52:10PM +0530, Nautiyal, Ankit K wrote:
> 
> On 9/16/2024 8:36 PM, Ville Syrjälä wrote:
> > On Mon, Sep 16, 2024 at 05:54:12PM +0300, Ville Syrjälä wrote:
> >> On Mon, Sep 16, 2024 at 01:09:42PM +0530, Nautiyal, Ankit K wrote:
> >>> On 9/12/2024 4:08 AM, Ville Syrjälä wrote:
> >>>> On Wed, Sep 11, 2024 at 06:43:46PM +0530, Ankit Nautiyal wrote:
> >>>>> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> >>>>>
> >>>>> Ultrajoiner case requires special treatment where both reverse and
> >>>>> staight order iteration doesn't work(for instance disabling case requires
> >>>>> order to be: primary master, slaves, secondary master).
> >>>>>
> >>>>> Lets unify our approach by using not only pipe masks for iterating required
> >>>>> pipes based on joiner type used, but also using different "priority" arrays
> >>>>> for each of those.
> >>>>>
> >>>>> v2: Fix checkpatch warnings. (Ankit)
> >>>>>
> >>>>> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> >>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >>>>> ---
> >>>>>    drivers/gpu/drm/i915/display/intel_ddi.c     | 19 +++--
> >>>>>    drivers/gpu/drm/i915/display/intel_display.c | 83 ++++++++++++++++----
> >>>>>    drivers/gpu/drm/i915/display/intel_display.h |  7 ++
> >>>>>    drivers/gpu/drm/i915/display/intel_dp_mst.c  | 18 +++--
> >>>>>    4 files changed, 96 insertions(+), 31 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> >>>>> index 00fbe9f8c03a..2c064b6c6d01 100644
> >>>>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> >>>>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> >>>>> @@ -3116,10 +3116,11 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
> >>>>>    					       const struct drm_connector_state *old_conn_state)
> >>>>>    {
> >>>>>    	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>>>> -	struct intel_crtc *pipe_crtc;
> >>>>> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
> >>>>>    
> >>>>> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> >>>>> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
> >>>>> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> >>>>> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
> >>>>> +					     intel_get_pipe_order_disable(old_crtc_state)) {
> >>>>>    		const struct intel_crtc_state *old_pipe_crtc_state =
> >>>>>    			intel_atomic_get_old_crtc_state(state, pipe_crtc);
> >>>>>    
> >>>>> @@ -3130,8 +3131,9 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
> >>>>>    
> >>>>>    	intel_ddi_disable_transcoder_func(old_crtc_state);
> >>>>>    
> >>>>> -	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, pipe_crtc,
> >>>>> -					 intel_crtc_joined_pipe_mask(old_crtc_state)) {
> >>>>> +	for_each_intel_crtc_in_mask_priority(dev_priv, pipe_crtc, pipe,
> >>>>> +					     intel_crtc_joined_pipe_mask(old_crtc_state),
> >>>>> +					     intel_get_pipe_order_disable(old_crtc_state)) {
> >>>>>    		const struct intel_crtc_state *old_pipe_crtc_state =
> >>>>>    			intel_atomic_get_old_crtc_state(state, pipe_crtc);
> >>>>>    
> >>>>> @@ -3383,7 +3385,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
> >>>>>    			     const struct drm_connector_state *conn_state)
> >>>>>    {
> >>>>>    	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> >>>>> -	struct intel_crtc *pipe_crtc;
> >>>>> +	struct intel_crtc *pipe_crtc; enum pipe pipe;
> >>>>>    
> >>>>>    	intel_ddi_enable_transcoder_func(encoder, crtc_state);
> >>>>>    
> >>>>> @@ -3394,8 +3396,9 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
> >>>>>    
> >>>>>    	intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
> >>>>>    
> >>>>> -	for_each_intel_crtc_in_pipe_mask_reverse(&i915->drm, pipe_crtc,
> >>>>> -						 intel_crtc_joined_pipe_mask(crtc_state)) {
> >>>>> +	for_each_intel_crtc_in_mask_priority(i915, pipe_crtc, pipe,
> >>>>> +					     intel_crtc_joined_pipe_mask(crtc_state),
> >>>>> +					     intel_get_pipe_order_enable(crtc_state)) {
> >>>>>    		const struct intel_crtc_state *pipe_crtc_state =
> >>>>>    			intel_atomic_get_new_crtc_state(state, pipe_crtc);
> >>>>>    
> >>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >>>>> index db27850b2c36..27622d51a473 100644
> >>>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >>>>> @@ -1737,6 +1737,50 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
> >>>>>    	hsw_set_transconf(crtc_state);
> >>>>>    }
> >>>>>    
> >>>>> +static
> >>>>> +bool intel_crtc_is_bigjoiner(const struct intel_crtc_state *pipe_config)
> >>>>> +{
> >>>>> +	return hweight8(pipe_config->joiner_pipes) == 2;
> >>>>> +}
> >>>>> +
> >>>>> +const enum pipe *intel_get_pipe_order_enable(const struct intel_crtc_state *crtc_state)
> >>>>> +{
> >>>>> +	static const enum pipe ultrajoiner_pipe_order_enable[I915_MAX_PIPES] = {
> >>>>> +		PIPE_B, PIPE_D, PIPE_C, PIPE_A
> >>>>> +	};
> >>>>> +	static const enum pipe bigjoiner_pipe_order_enable[I915_MAX_PIPES] = {
> >>>>> +		PIPE_B, PIPE_A, PIPE_D, PIPE_C
> >>>>> +	};
> >>>>> +	static const enum pipe nojoiner_pipe_order_enable[I915_MAX_PIPES] = {
> >>>>> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> >>>>> +	};
> >>>>> +
> >>>>> +	if (intel_crtc_is_ultrajoiner(crtc_state))
> >>>>> +		return ultrajoiner_pipe_order_enable;
> >>>>> +	else if (intel_crtc_is_bigjoiner(crtc_state))
> >>>>> +		return bigjoiner_pipe_order_enable;
> >>>>> +	return nojoiner_pipe_order_enable;
> >>>>> +}
> >>>>> +
> >>>>> +const enum pipe *intel_get_pipe_order_disable(const struct intel_crtc_state *crtc_state)
> >>>>> +{
> >>>>> +	static const enum pipe ultrajoiner_pipe_order_disable[I915_MAX_PIPES] = {
> >>>>> +		PIPE_A, PIPE_B, PIPE_D, PIPE_C
> >>>>> +	};
> >>>>> +	static const enum pipe bigjoiner_pipe_order_disable[I915_MAX_PIPES] = {
> >>>>> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> >>>>> +	};
> >>>>> +	static const enum pipe nojoiner_pipe_order_disable[I915_MAX_PIPES] = {
> >>>>> +		PIPE_A, PIPE_B, PIPE_C, PIPE_D
> >>>>> +	};
> >>>>> +
> >>>>> +	if (intel_crtc_is_ultrajoiner(crtc_state))
> >>>>> +		return ultrajoiner_pipe_order_disable;
> >>>>> +	else if (intel_crtc_is_bigjoiner(crtc_state))
> >>>>> +		return bigjoiner_pipe_order_disable;
> >>>>> +	return nojoiner_pipe_order_disable;
> >>>> I don't think we should need all those diffrent order array. Technically
> >>>> one should do. Though having two might make sense.
> >>>>
> >>>> Another problem is the hardcoded pipes. If we eg. get hardware that
> >>>> would support ultrajoiner on pipes B-E in the future this would no
> >>>> longer  work.
> >>>>
> >>>>> +}
> >>>> <snip>
> >>>>> +#define for_each_intel_crtc_in_mask_priority(__dev_priv, intel_crtc, __p, __mask, __priolist) \
> >>>>> +	for_each_pipe(__dev_priv, __p) \
> >>>>> +		for_each_if((__mask) & BIT(__priolist[__p])) \
> >>>>> +			for_each_if(intel_crtc = intel_crtc_for_pipe(to_intel_display(&__dev_priv->drm), __priolist[__p]))
> >>>> I think something like:
> >>>>
> >>>> const u8 intel_pipe_order_enable[4] = {
> >>>>           3, 1, 2, 0,
> >>>> };
> >>>>
> >>>> const u8 intel_pipe_order_disable[4] = {
> >>>>           0, 2, 1, 3,
> >>>> };
> 
> 
> I just realized that as per bspec:54142 sequence for ultrajoiner doesnt 
> follow this.
> 
> its 1, 3, 2, 0 for enabling and 0, 1, 3, 2 for disabling :(

The bigjoiner secondary pipes are listed as one step in the sequence.
I believe that means that there is no ordering requirement between
them.

As for the funny C vs. BD order in the disable sequence, bspec:68847
has it the other way around as ACBD (a more proper mirror image of the
enable sequence). So either the hardware really has changed between
these two and the order is thus different, or one of them is simply
wrong, or the order doesn't really matter here either.

I suspect we can just always follow the more sensible order from
bspec:68847. Should probably ask for clarification though, and
test on actual hardware to confirm ofc.

-- 
Ville Syrjälä
Intel