[PATCH 00/15] Ultrajoiner basic functionality series

[PATCH 00/15] 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.

Rev9:
Rebase

Rev10:
-Rebase
-Fix enable_joined_pipes as suggested by Ville, to avoid propagate
is_ultrajoiner.
-Enhance the mask to iterate over joiner pipes, as suggested by Ville.

Rev11:
-Address review comments from Ville
-Split changes to dsc helpers to separate patches

Rev12:
-Move 'Refactor enable_joiner_pipes' earlier than 'Implement hw state
readout and checks for ultrajoiner'.
-Few improvements as suggested in review comments.
-Use --patience for Patch #8 "drm/i915/display: Refactor
enable_joiner_pipes"

Ankit Nautiyal (10):
  drm/i915/display_device: Add Check HAS_DSC for bigjoiner
  drm/i915/display_debugfs: Allow force joiner only if supported
  drm/i915/display: Modify debugfs for joiner to force n pipes
  drm/i915/dp: Add helper to compute num pipes required
  drm/i915: Split current joiner hw state readout
  drm/i915/display: Add macro HAS_ULTRAJOINER()
  drm/i915/display: Refactor enable_joiner_pipes
  drm/i915/dp: Modify compressed bpp limitations for ultrajoiner
  drm/i915/display: Consider ultrajoiner for computing maxdotclock
  drm/i915/intel_dp: Add support for forcing ultrajoiner

Stanislav Lisovskiy (5):
  drm/i915: Add bigjoiner and uncompressed joiner hw readout sanity
    checks
  drm/i915: Implement hw state readout and checks for ultrajoiner
  drm/i915/display/vdsc: Add ultrajoiner support with DSC
  drm/i915/dp: Simplify helper to get slice count with joiner
  drm/i915: Compute config and mode valid changes for ultrajoiner

 drivers/gpu/drm/i915/display/intel_display.c  | 311 +++++++++++++++---
 drivers/gpu/drm/i915/display/intel_display.h  |   3 +
 .../drm/i915/display/intel_display_debugfs.c  |  73 +++-
 .../drm/i915/display/intel_display_device.h   |   5 +-
 .../drm/i915/display/intel_display_types.h    |   2 +-
 drivers/gpu/drm/i915/display/intel_dp.c       |  97 ++++--
 drivers/gpu/drm/i915/display/intel_dp.h       |   6 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  23 +-
 drivers/gpu/drm/i915/display/intel_vdsc.c     |  11 +-
 .../gpu/drm/i915/display/intel_vdsc_regs.h    |   2 +
 10 files changed, 430 insertions(+), 103 deletions(-)

-- 
2.45.2

[PATCH 01/15] drm/i915/display_device: Add Check HAS_DSC for bigjoiner

[Ankit Nautiyal]

Bigjoiner needs DSC, but DSC might be disabled on some platforms.
The platform check itself is not sufficient, so add a check for
DSC to reflect that.

v2: Modify the commit message to address the DSC fuse case.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_device.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 5306bbd13e59..6a5bee59e6aa 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -118,7 +118,7 @@ enum intel_display_subplatform {
 
 #define HAS_4TILE(i915)			(IS_DG2(i915) || DISPLAY_VER(i915) >= 14)
 #define HAS_ASYNC_FLIPS(i915)		(DISPLAY_VER(i915) >= 5)
-#define HAS_BIGJOINER(i915)		(DISPLAY_VER(i915) >= 11)
+#define HAS_BIGJOINER(i915)		(DISPLAY_VER(i915) >= 11 && HAS_DSC(i915))
 #define HAS_CDCLK_CRAWL(i915)		(DISPLAY_INFO(i915)->has_cdclk_crawl)
 #define HAS_CDCLK_SQUASH(i915)		(DISPLAY_INFO(i915)->has_cdclk_squash)
 #define HAS_CUR_FBC(i915)		(!HAS_GMCH(i915) && IS_DISPLAY_VER(i915, 7, 13))
-- 
2.45.2

[PATCH 02/15] drm/i915/display_debugfs: Allow force joiner only if supported

[Ankit Nautiyal]

Currently we support joiner only for DP encoder.
Do not create the debugfs for joiner if DP does not support the joiner.
This will also help avoiding cases where config has eDP MSO, with which
we do not support joiner.

v2: Check for intel_dp_has_joiner and avoid creating debugfs if not
supported. (Ville)
v3: Remove HAS_BIGJOINER check. (Ville)
v4: Reverse checks for connector type and intel_dp_has_joiner(). (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_debugfs.c | 7 ++++---
 1 file changed, 4 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 890ef7067b77..08adeaa2e87f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1328,6 +1328,7 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 	struct dentry *root = connector->base.debugfs_entry;
 	int connector_type = connector->base.connector_type;
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
 
 	/* The connector must have been registered beforehands. */
 	if (!root)
@@ -1362,9 +1363,9 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
 				    connector, &i915_dsc_fractional_bpp_fops);
 	}
 
-	if (HAS_BIGJOINER(i915) &&
-	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
-	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
+	if ((connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	     connector_type == DRM_MODE_CONNECTOR_eDP) &&
+	    intel_dp_has_joiner(intel_dp)) {
 		debugfs_create_bool("i915_bigjoiner_force_enable", 0644, root,
 				    &connector->force_bigjoiner_enable);
 	}
-- 
2.45.2

[PATCH 03/15] 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)
v4: Return -EINVAL if joiner forced to an invalid value.
v5: Remove extra debug message. (Ville)
v6: Minor fix in switch case. (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 57 ++++++++++++++++++-
 .../drm/i915/display/intel_display_types.h    |  2 +-
 drivers/gpu/drm/i915/display/intel_dp.c       |  2 +-
 3 files changed, 57 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 08adeaa2e87f..29590cc4429c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1316,6 +1316,59 @@ 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;
+	int force_joined_pipes = 0;
+	int ret;
+
+	if (len == 0)
+		return 0;
+
+	ret = kstrtoint_from_user(ubuf, len, 0, &force_joined_pipes);
+	if (ret < 0)
+		return ret;
+
+	switch (force_joined_pipes) {
+	case 0:
+	case 2:
+		connector->force_joined_pipes = force_joined_pipes;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	*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
@@ -1366,8 +1419,8 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
 	if ((connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
 	     connector_type == DRM_MODE_CONNECTOR_eDP) &&
 	    intel_dp_has_joiner(intel_dp)) {
-		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 7ff97e5b83dd..7fb3eeb0e0f2 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 16dc1d26d2a2..a1a64758d30d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1274,7 +1274,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 04/15] drm/i915/dp: Add helper to compute num pipes required

[Ankit Nautiyal]

Add a helper to compute the number of pipes required.
This will depend on whether the joiner is required or is forced through
the debugfs. If no joiner is required the helper returns 1.

v2:
-Return 1 if no joiner is required. (Ville)
-Change the suffix from joined_pipes to num_pipes. (Ville)
-Use number of pipes while calculating joined_pipe masks and
max_dotclk. (Ville)
v3: Simplify and rename the helper to intel_dp_num_joined_pipes(). Ville
v4: Remove redundant 'fallthrough' statement. (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  |  1 +
 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   | 23 ++++------
 4 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 29590cc4429c..14bfda9f2d5c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1343,6 +1343,7 @@ static ssize_t i915_joiner_write(struct file *file,
 
 	switch (force_joined_pipes) {
 	case 0:
+	case 1:
 	case 2:
 		connector->force_joined_pipes = force_joined_pipes;
 		break;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index a1a64758d30d..f2a2541c1091 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1264,17 +1264,30 @@ 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 ||
-	       connector->force_joined_pipes == 2;
+	return clock > i915->display.cdclk.max_dotclk_freq || hdisplay > 5120;
+}
+
+int intel_dp_num_joined_pipes(struct intel_dp *intel_dp,
+			      struct intel_connector *connector,
+			      int hdisplay, int clock)
+{
+	if (connector->force_joined_pipes)
+		return connector->force_joined_pipes;
+
+	if (intel_dp_needs_bigjoiner(intel_dp, connector, hdisplay, clock))
+		return 2;
+
+	return 1;
 }
 
 bool intel_dp_has_dsc(const struct intel_connector *connector)
@@ -1311,7 +1324,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);
@@ -1333,13 +1346,9 @@ 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)) {
-		joiner = true;
-		max_dotclk *= 2;
-	}
-
-	num_joined_pipes = joiner ? 2 : 1;
+	num_joined_pipes = intel_dp_num_joined_pipes(intel_dp, connector,
+						     mode->hdisplay, target_clock);
+	max_dotclk *= num_joined_pipes;
 
 	if (target_clock > max_dotclk)
 		return MODE_CLOCK_HIGH;
@@ -2507,12 +2516,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	    !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))
-		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
-
-	num_joined_pipes = intel_crtc_num_joined_pipes(pipe_config);
+	num_joined_pipes = intel_dp_num_joined_pipes(intel_dp, connector,
+						     adjusted_mode->crtc_hdisplay,
+						     adjusted_mode->crtc_clock);
+	if (num_joined_pipes > 1)
+		pipe_config->joiner_pipes = GENMASK(crtc->pipe + num_joined_pipes - 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.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 3b869429e575..53d1217800ef 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -151,9 +151,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,
 				int num_joined_pipes);
-bool intel_dp_need_joiner(struct intel_dp *intel_dp,
-			  struct intel_connector *connector,
-			  int hdisplay, int clock);
+int intel_dp_num_joined_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 732d7543ad06..4765bda154c1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -581,12 +581,11 @@ 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))
-		pipe_config->joiner_pipes = GENMASK(crtc->pipe + 1, crtc->pipe);
-
-	num_joined_pipes = intel_crtc_num_joined_pipes(pipe_config);
+	num_joined_pipes = intel_dp_num_joined_pipes(intel_dp, connector,
+						     adjusted_mode->crtc_hdisplay,
+						     adjusted_mode->crtc_clock);
+	if (num_joined_pipes > 1)
+		pipe_config->joiner_pipes = GENMASK(crtc->pipe + num_joined_pipes - 1, crtc->pipe);
 
 	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
 	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
@@ -1428,7 +1427,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;
@@ -1472,13 +1471,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)) {
-		joiner = true;
-		max_dotclk *= 2;
-	}
-
-	num_joined_pipes = joiner ? 2 : 1;
+	num_joined_pipes = intel_dp_num_joined_pipes(intel_dp, intel_connector,
+						     mode->hdisplay, target_clock);
+	max_dotclk *= num_joined_pipes;
 
 	ret = drm_modeset_lock(&mgr->base.lock, ctx);
 	if (ret)
-- 
2.45.2

[PATCH 05/15] drm/i915: Split current joiner hw state readout

[Ankit Nautiyal]

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)
v4: Use check for bigjoiner before reading the regs. (Ville)

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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 74 +++++++++++++++-----
 1 file changed, 55 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f7667931f9d9..8d37973864ee 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3580,26 +3580,57 @@ static bool transcoder_ddi_func_is_enabled(struct drm_i915_private *dev_priv,
 	return tmp & TRANS_DDI_FUNC_ENABLE;
 }
 
-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 drm_i915_private *i915 = to_i915(display->drm);
 	struct intel_crtc *crtc;
 
 	*primary_pipes = 0;
 	*secondary_pipes = 0;
 
-	if (!HAS_BIGJOINER(dev_priv))
+	if (!HAS_UNCOMPRESSED_JOINER(display))
 		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;
+
+	if (!HAS_BIGJOINER(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(display, ICL_PIPE_DSS_CTL1(pipe));
 
 			if (!(tmp & BIG_JOINER_ENABLE))
 				continue;
@@ -3609,20 +3640,25 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 			else
 				*secondary_pipes |= BIT(pipe);
 		}
+	}
+}
 
-		if (!HAS_UNCOMPRESSED_JOINER(dev_priv))
-			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));
+	enabled_uncompressed_joiner_pipes(display, &primary_uncompressed_joiner_pipes,
+					  &secondary_uncompressed_joiner_pipes);
 
-			if (tmp & UNCOMPRESSED_JOINER_PRIMARY)
-				*primary_pipes |= BIT(pipe);
-			if (tmp & UNCOMPRESSED_JOINER_SECONDARY)
-				*secondary_pipes |= BIT(pipe);
-		}
-	}
+	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 06/15] drm/i915: Add bigjoiner and uncompressed joiner hw readout sanity checks

[Ankit Nautiyal]

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

Add 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)

v3:
-Rename helper to get expected uncompressed joiner pipes. (Ville)

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 8d37973864ee..2d6260c3bca5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3643,26 +3643,73 @@ 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_uncompressed_joiner_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;
 
 	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_uncompressed_joiner_secondary_pipes(primary_uncompressed_joiner_pipes),
+		 "Wrong secondary uncompressed joiner pipes(expected %#x, current %#x)\n",
+		 expected_uncompressed_joiner_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 %#x, secondary pipes %#x)\n",
 		 *primary_pipes, *secondary_pipes);
 }
 
-- 
2.45.2

[PATCH 07/15] drm/i915/display: Add macro HAS_ULTRAJOINER()

[Ankit Nautiyal]

Add macro to check if platform supports Ultrajoiner.

v2:
-Use check for DISPLAY_VER >= 20, and add bmg as a special case. (Ville)
-Add check for HAS_DSC. (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_device.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 6a5bee59e6aa..220cca6333ee 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -154,6 +154,9 @@ enum intel_display_subplatform {
 #define HAS_TRANSCODER(i915, trans)	((DISPLAY_RUNTIME_INFO(i915)->cpu_transcoder_mask & \
 					  BIT(trans)) != 0)
 #define HAS_UNCOMPRESSED_JOINER(i915)	(DISPLAY_VER(i915) >= 13)
+#define HAS_ULTRAJOINER(i915)		((DISPLAY_VER(i915) >= 20 || \
+					  (IS_DGFX(i915) && DISPLAY_VER(i915) == 14)) && \
+					 HAS_DSC(i915))
 #define HAS_VRR(i915)			(DISPLAY_VER(i915) >= 11)
 #define HAS_AS_SDP(i915)		(DISPLAY_VER(i915) >= 13)
 #define HAS_CMRR(i915)			(DISPLAY_VER(i915) >= 20)
-- 
2.45.2

[PATCH 08/15] drm/i915/display: Refactor enable_joiner_pipes

[Ankit Nautiyal]

Pass the current pipe into enabled_joiner_pipes(), and let it figure out
the proper bitmasks for us. Since the enabled_joiner_pipes now gets the
primary and secondary pipe wrt a given pipe, the helpers
to get primary pipe and secondary pipes are no longer required.

v2:
-Simplify helper get_joiner_primary_pipes. (Ville)
-Nuke get_joiner_secondary_pipes. (Ville)
-Add more drm_WARNs final primary/secondary pipes. (Ville)
v3:
-Drop ultrajoiner stuff and add it in subsequent patches. (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 89 ++++++++++----------
 1 file changed, 44 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 2d6260c3bca5..ea259b142786 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3663,7 +3663,15 @@ static u8 expected_bigjoiner_secondary_pipes(u8 bigjoiner_primary_pipes)
 	return expected_secondary_pipes(bigjoiner_primary_pipes, 2);
 }
 
+static u8 get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes)
+{
+	primary_pipes &= GENMASK(pipe, 0);
+
+	return primary_pipes ? BIT(fls(primary_pipes) - 1) : 0;
+}
+
 static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
+				 enum pipe pipe,
 				 u8 *primary_pipes, u8 *secondary_pipes)
 {
 	struct intel_display *display = to_intel_display(&dev_priv->drm);
@@ -3703,45 +3711,38 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 		 expected_uncompressed_joiner_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(display->drm, *secondary_pipes != *primary_pipes << 1,
-		 "Joiner misconfigured (primary pipes %#x, secondary pipes %#x)\n",
-		 *primary_pipes, *secondary_pipes);
-}
-
-static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
-{
-	if ((secondary_pipes & BIT(pipe)) == 0)
-		return pipe;
-
-	/* ignore everything above our pipe */
-	primary_pipes &= ~GENMASK(7, pipe);
-
-	/* highest remaining bit should be our primary pipe */
-	return fls(primary_pipes) - 1;
-}
-
-static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
-{
-	enum pipe primary_pipe, next_primary_pipe;
-
-	primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes);
-
-	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;
-
-	return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe);
+	*primary_pipes = 0;
+	*secondary_pipes = 0;
+
+	if (uncompressed_joiner_pipes & BIT(pipe)) {
+		*primary_pipes = get_joiner_primary_pipe(pipe, primary_uncompressed_joiner_pipes);
+		*secondary_pipes = secondary_uncompressed_joiner_pipes &
+				   expected_uncompressed_joiner_secondary_pipes(*primary_pipes);
+
+		drm_WARN(display->drm,
+			 expected_uncompressed_joiner_secondary_pipes(*primary_pipes) !=
+			 *secondary_pipes,
+			 "Wrong uncompressed joiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n",
+			 *primary_pipes,
+			 expected_uncompressed_joiner_secondary_pipes(*primary_pipes),
+			 *secondary_pipes);
+		return;
+	}
+
+	if (bigjoiner_pipes & BIT(pipe)) {
+		*primary_pipes = get_joiner_primary_pipe(pipe, primary_bigjoiner_pipes);
+		*secondary_pipes = secondary_bigjoiner_pipes &
+				   expected_bigjoiner_secondary_pipes(*primary_pipes);
+
+		drm_WARN(display->drm,
+			 expected_bigjoiner_secondary_pipes(*primary_pipes) !=
+			 *secondary_pipes,
+			 "Wrong bigjoiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n",
+			 *primary_pipes,
+			 expected_bigjoiner_secondary_pipes(*primary_pipes),
+			 *secondary_pipes);
+		return;
+	}
 }
 
 static u8 hsw_panel_transcoders(struct drm_i915_private *i915)
@@ -3813,10 +3814,10 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc)
 		enabled_transcoders |= BIT(cpu_transcoder);
 
 	/* joiner secondary -> consider the primary pipe's transcoder as well */
-	enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes);
+	enabled_joiner_pipes(dev_priv, crtc->pipe, &primary_pipes, &secondary_pipes);
 	if (secondary_pipes & BIT(crtc->pipe)) {
 		cpu_transcoder = (enum transcoder)
-			get_joiner_primary_pipe(crtc->pipe, primary_pipes, secondary_pipes);
+			ffs(get_joiner_primary_pipe(crtc->pipe, primary_pipes)) - 1;
 		if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder))
 			enabled_transcoders |= BIT(cpu_transcoder);
 	}
@@ -3950,14 +3951,12 @@ static void intel_joiner_get_config(struct intel_crtc_state *crtc_state)
 	u8 primary_pipes, secondary_pipes;
 	enum pipe pipe = crtc->pipe;
 
-	enabled_joiner_pipes(i915, &primary_pipes, &secondary_pipes);
+	enabled_joiner_pipes(i915, pipe, &primary_pipes, &secondary_pipes);
 
 	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 = primary_pipes | secondary_pipes;
 }
 
 static bool hsw_get_pipe_config(struct intel_crtc *crtc,
-- 
2.45.2

[PATCH 09/15] 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)
v5: Remove spurious newline. (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  | 88 +++++++++++++++++++
 .../gpu/drm/i915/display/intel_vdsc_regs.h    |  2 +
 2 files changed, 90 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ea259b142786..6d8143f62110 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3670,15 +3670,77 @@ static u8 get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes)
 	return primary_pipes ? BIT(fls(primary_pipes) - 1) : 0;
 }
 
+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 void enabled_ultrajoiner_pipes(struct drm_i915_private *i915,
+				      u8 *primary_pipes, u8 *secondary_pipes)
+{
+	struct intel_crtc *crtc;
+
+	*primary_pipes = 0;
+	*secondary_pipes = 0;
+
+	if (!HAS_ULTRAJOINER(i915))
+		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,
 				 enum pipe pipe,
 				 u8 *primary_pipes, u8 *secondary_pipes)
 {
 	struct intel_display *display = to_intel_display(&dev_priv->drm);
+	u8 primary_ultrajoiner_pipes;
 	u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes;
+	u8 secondary_ultrajoiner_pipes;
 	u8 secondary_uncompressed_joiner_pipes, secondary_bigjoiner_pipes;
+	u8 ultrajoiner_pipes;
 	u8 uncompressed_joiner_pipes, bigjoiner_pipes;
 
+	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);
+
 	enabled_uncompressed_joiner_pipes(display, &primary_uncompressed_joiner_pipes,
 					  &secondary_uncompressed_joiner_pipes);
 
@@ -3691,10 +3753,21 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 	drm_WARN_ON(display->drm,
 		    (primary_bigjoiner_pipes & secondary_bigjoiner_pipes) != 0);
 
+	ultrajoiner_pipes = primary_ultrajoiner_pipes | secondary_ultrajoiner_pipes;
 	uncompressed_joiner_pipes = primary_uncompressed_joiner_pipes |
 				    secondary_uncompressed_joiner_pipes;
 	bigjoiner_pipes = primary_bigjoiner_pipes | secondary_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_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);
+
 	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);
@@ -3714,6 +3787,21 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
 	*primary_pipes = 0;
 	*secondary_pipes = 0;
 
+	if (ultrajoiner_pipes & BIT(pipe)) {
+		*primary_pipes = get_joiner_primary_pipe(pipe, primary_ultrajoiner_pipes);
+		*secondary_pipes = secondary_ultrajoiner_pipes &
+				   expected_ultrajoiner_secondary_pipes(*primary_pipes);
+
+		drm_WARN(display->drm,
+			 expected_ultrajoiner_secondary_pipes(*primary_pipes) !=
+			 *secondary_pipes,
+			 "Wrong ultrajoiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n",
+			 *primary_pipes,
+			 expected_ultrajoiner_secondary_pipes(*primary_pipes),
+			 *secondary_pipes);
+		return;
+	}
+
 	if (uncompressed_joiner_pipes & BIT(pipe)) {
 		*primary_pipes = get_joiner_primary_pipe(pipe, primary_uncompressed_joiner_pipes);
 		*secondary_pipes = secondary_uncompressed_joiner_pipes &
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc_regs.h b/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
index f921ad67b587..bf32a3b46fb1 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
@@ -37,6 +37,8 @@
 #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 10/15] 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 primary/secondary instead of master/slave. (Suraj)
v3: Add the ultrajoiner helpers and use it for setting ultrajoiner
bits (Ankit)
v4: Use num_vdsc_instances *= num_joined_pipes (Ville)
v5: Align the helper to get ultrajoiner enabled pipes with other helpers
(Ville)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 42 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_display.h |  3 ++
 drivers/gpu/drm/i915/display/intel_vdsc.c    | 11 ++++-
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 6d8143f62110..4f969e526bdd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -312,6 +312,48 @@ u8 _intel_modeset_secondary_pipes(const struct intel_crtc_state *crtc_state)
 	return bigjoiner_secondary_pipes(crtc_state);
 }
 
+bool intel_crtc_is_ultrajoiner(const struct intel_crtc_state *crtc_state)
+{
+	return intel_crtc_num_joined_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);
+}
+
+/*
+ * The ultrajoiner enable 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..
+ */
+static u8 ultrajoiner_enable_pipes(const struct intel_crtc_state *crtc_state)
+{
+	if (!intel_crtc_is_ultrajoiner(crtc_state))
+		return 0;
+
+	return crtc_state->joiner_pipes & (0b01110111 << joiner_primary_pipe(crtc_state));
+}
+
+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) &&
+	       BIT(crtc->pipe) & ultrajoiner_enable_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 1f0fed5ea7bc..61e1df878de9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -441,6 +441,9 @@ 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);
+bool intel_crtc_ultrajoiner_enable_needed(const struct intel_crtc_state *crtc_state);
 u8 intel_crtc_joiner_secondary_pipes(const struct intel_crtc_state *crtc_state);
 u8 _intel_modeset_primary_pipes(const struct intel_crtc_state *crtc_state);
 u8 _intel_modeset_secondary_pipes(const 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..c3405234dc51 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -379,9 +379,9 @@ 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_crtc_num_joined_pipes(crtc_state);
 
-	if (crtc_state->joiner_pipes)
-		num_vdsc_instances *= 2;
+	num_vdsc_instances *= num_joined_pipes;
 
 	return num_vdsc_instances;
 }
@@ -770,7 +770,14 @@ void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
 		dss_ctl1_val |= JOINER_ENABLE;
 	}
 	if (crtc_state->joiner_pipes) {
+		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 11/15] drm/i915/dp: Modify compressed bpp limitations for ultrajoiner

[Ankit Nautiyal]

Add compressed bpp limitations for ultrajoiner.

v2: Fix the case for 1 pipe. (Ankit)

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

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index f2a2541c1091..a0afb4991334 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -865,24 +865,39 @@ 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_bigjoiner;
+	u32 max_bpp;
 
 	/* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
 	max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) / mode_hdisplay;
 
-	if (num_joined_pipes == 2) {
+	if (num_joined_pipes == 1)
+		return max_bpp_small_joiner_ram;
+
+	if (num_joined_pipes > 1) {
 		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;
-		u32 max_bpp_bigjoiner =
-			i915->display.cdclk.max_cdclk_freq * ppc * bigjoiner_interface_bits /
+		int num_bigjoiners = num_joined_pipes / 2;
+
+		max_bpp_bigjoiner =
+			i915->display.cdclk.max_cdclk_freq * ppc * bigjoiner_interface_bits *
 			intel_dp_mode_to_fec_clock(mode_clock);
 
-		max_bpp_small_joiner_ram *= 2;
+		max_bpp_bigjoiner *= num_bigjoiners;
+
+		max_bpp_small_joiner_ram *= num_joined_pipes;
+	}
+
+	max_bpp = min(max_bpp_small_joiner_ram, max_bpp_bigjoiner);
+
+	if (num_joined_pipes == 4) {
+		u32 max_bpp_ultrajoiner_ram = (4 * 72 * 512) / mode_hdisplay;
 
-		return min(max_bpp_small_joiner_ram, max_bpp_bigjoiner);
+		max_bpp = min(max_bpp, max_bpp_ultrajoiner_ram);
 	}
 
-	return max_bpp_small_joiner_ram;
+	return max_bpp;
 }
 
 u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
-- 
2.45.2

[PATCH 12/15] drm/i915/dp: Simplify helper to get slice count with joiner

[Ankit Nautiyal]

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

When bigjoiner is used, we need at least 2 dsc slices per pipe.
Modify the condition in intel_dp_dsc_get_slice_count() to reflect the
same.

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_dp.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index a0afb4991334..eb92fda21e87 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -999,8 +999,12 @@ u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
 		    drm_dp_dsc_sink_max_slice_count(connector->dp.dsc_dpcd, false))
 			break;
 
-		/* big joiner needs small joiner to be enabled */
-		if (num_joined_pipes == 2 && test_slice_count < 4)
+		 /*
+		  * Bigjoiner needs small joiner to be enabled.
+		  * So there should be at least 2 dsc slices per pipe,
+		  * whenever bigjoiner is enabled.
+		  */
+		if (num_joined_pipes > 1 && valid_dsc_slicecount[i] < 2)
 			continue;
 
 		if (min_slice_count <= test_slice_count)
-- 
2.45.2

[PATCH 13/15] 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.

v2:
-Drop changes for HDMI.
-Separate out DSC changes into another patch.
v3: Fix check in can_ultrajoiner. (Ankit)
v4:
-Unify helper to check joiner requirement. (Ville)
-Split patches for ultrajoiner changes for max dsc slices and compressed
bpp.(Ankit)
v5: Fix check for joiner. (Ville)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 26 +++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index eb92fda21e87..d0609d007743 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1284,26 +1284,38 @@ intel_dp_mode_valid_downstream(struct intel_connector *connector,
 }
 
 static
-bool intel_dp_needs_bigjoiner(struct intel_dp *intel_dp,
-			      struct intel_connector *connector,
-			      int hdisplay, int clock)
+bool intel_dp_needs_joiner(struct intel_dp *intel_dp,
+			   struct intel_connector *connector,
+			   int hdisplay, int clock,
+			   int num_joined_pipes)
 {
 	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;
+	num_joined_pipes /= 2;
+
+	return clock > num_joined_pipes * i915->display.cdclk.max_dotclk_freq ||
+	       hdisplay > num_joined_pipes * 5120;
 }
 
 int intel_dp_num_joined_pipes(struct intel_dp *intel_dp,
 			      struct intel_connector *connector,
 			      int hdisplay, int clock)
 {
+	struct intel_display *display = to_intel_display(intel_dp);
+	struct drm_i915_private *i915 = to_i915(display->drm);
+
 	if (connector->force_joined_pipes)
 		return connector->force_joined_pipes;
 
-	if (intel_dp_needs_bigjoiner(intel_dp, connector, hdisplay, clock))
+	if (HAS_ULTRAJOINER(i915) &&
+	    intel_dp_needs_joiner(intel_dp, connector, hdisplay, clock, 4))
+		return 4;
+
+	if ((HAS_BIGJOINER(i915) || HAS_UNCOMPRESSED_JOINER(i915)) &&
+	    intel_dp_needs_joiner(intel_dp, connector, hdisplay, clock, 2))
 		return 2;
 
 	return 1;
@@ -2509,8 +2521,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 !HAS_UNCOMPRESSED_JOINER(i915) && num_joined_pipes == 2;
+	return (!HAS_UNCOMPRESSED_JOINER(i915) && num_joined_pipes == 2) ||
+		num_joined_pipes == 4;
 }
 
 static int
-- 
2.45.2

[PATCH 14/15] drm/i915/display: Consider ultrajoiner for computing maxdotclock

[Ankit Nautiyal]

Use the check for ultrajoiner while computing maxdotclock.

v2: Add Check for HAS_UNCOMPRESSED_JOINER. (Ville)
v3: Remove extraneous newline. (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 4f969e526bdd..ea0875256083 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8187,8 +8187,9 @@ static int max_dotclock(struct drm_i915_private *i915)
 {
 	int max_dotclock = i915->display.cdclk.max_dotclk_freq;
 
-	/* icl+ might use joiner */
-	if (HAS_BIGJOINER(i915))
+	if (HAS_ULTRAJOINER(i915))
+		max_dotclock *= 4;
+	else if (HAS_UNCOMPRESSED_JOINER(i915) || HAS_BIGJOINER(i915))
 		max_dotclock *= 2;
 
 	return max_dotclock;
-- 
2.45.2

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

[Ankit Nautiyal]

Allow forcing ultrajoiner through debugfs.

v2: Minor refactoring of switch case logic. (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_debugfs.c | 8 ++++++++
 1 file 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 14bfda9f2d5c..820c6859c9bf 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1331,6 +1331,7 @@ static ssize_t i915_joiner_write(struct file *file,
 {
 	struct seq_file *m = file->private_data;
 	struct intel_connector *connector = m->private;
+	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 	int force_joined_pipes = 0;
 	int ret;
 
@@ -1347,6 +1348,13 @@ static ssize_t i915_joiner_write(struct file *file,
 	case 2:
 		connector->force_joined_pipes = force_joined_pipes;
 		break;
+	case 4:
+		if (HAS_ULTRAJOINER(i915)) {
+			connector->force_joined_pipes = force_joined_pipes;
+			break;
+		}
+
+		fallthrough;
 	default:
 		return -EINVAL;
 	}
-- 
2.45.2

Re: [PATCH 02/15] drm/i915/display_debugfs: Allow force joiner only if supported

[Ville Syrjälä]

On Thu, Sep 26, 2024 at 12:56:25PM +0530, Ankit Nautiyal wrote:
> Currently we support joiner only for DP encoder.
> Do not create the debugfs for joiner if DP does not support the joiner.
> This will also help avoiding cases where config has eDP MSO, with which
> we do not support joiner.
> 
> v2: Check for intel_dp_has_joiner and avoid creating debugfs if not
> supported. (Ville)
> v3: Remove HAS_BIGJOINER check. (Ville)
> v4: Reverse checks for connector type and intel_dp_has_joiner(). (Ville)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_debugfs.c | 7 ++++---
>  1 file changed, 4 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 890ef7067b77..08adeaa2e87f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1328,6 +1328,7 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>  	struct drm_i915_private *i915 = to_i915(connector->base.dev);
>  	struct dentry *root = connector->base.debugfs_entry;
>  	int connector_type = connector->base.connector_type;
> +	struct intel_dp *intel_dp = intel_attached_dp(connector);

I'd probably drop the local variable entirely since it
can give us garbage for non-dp stuff.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	/* The connector must have been registered beforehands. */
>  	if (!root)
> @@ -1362,9 +1363,9 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>  				    connector, &i915_dsc_fractional_bpp_fops);
>  	}
>  
> -	if (HAS_BIGJOINER(i915) &&
> -	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> -	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
> +	if ((connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> +	     connector_type == DRM_MODE_CONNECTOR_eDP) &&
> +	    intel_dp_has_joiner(intel_dp)) {
>  		debugfs_create_bool("i915_bigjoiner_force_enable", 0644, root,
>  				    &connector->force_bigjoiner_enable);
>  	}
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 08/15] drm/i915/display: Refactor enable_joiner_pipes

[Ville Syrjälä]

On Thu, Sep 26, 2024 at 12:56:31PM +0530, Ankit Nautiyal wrote:
> Pass the current pipe into enabled_joiner_pipes(), and let it figure out
> the proper bitmasks for us. Since the enabled_joiner_pipes now gets the
> primary and secondary pipe wrt a given pipe, the helpers
> to get primary pipe and secondary pipes are no longer required.
> 
> v2:
> -Simplify helper get_joiner_primary_pipes. (Ville)
> -Nuke get_joiner_secondary_pipes. (Ville)
> -Add more drm_WARNs final primary/secondary pipes. (Ville)
> v3:
> -Drop ultrajoiner stuff and add it in subsequent patches. (Ville)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 89 ++++++++++----------
>  1 file changed, 44 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 2d6260c3bca5..ea259b142786 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3663,7 +3663,15 @@ static u8 expected_bigjoiner_secondary_pipes(u8 bigjoiner_primary_pipes)
>  	return expected_secondary_pipes(bigjoiner_primary_pipes, 2);
>  }
>  
> +static u8 get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes)
> +{
> +	primary_pipes &= GENMASK(pipe, 0);
> +
> +	return primary_pipes ? BIT(fls(primary_pipes) - 1) : 0;
> +}
> +
>  static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
> +				 enum pipe pipe,
>  				 u8 *primary_pipes, u8 *secondary_pipes)
>  {
>  	struct intel_display *display = to_intel_display(&dev_priv->drm);
> @@ -3703,45 +3711,38 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
>  		 expected_uncompressed_joiner_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(display->drm, *secondary_pipes != *primary_pipes << 1,
> -		 "Joiner misconfigured (primary pipes %#x, secondary pipes %#x)\n",
> -		 *primary_pipes, *secondary_pipes);
> -}
> -
> -static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
> -{
> -	if ((secondary_pipes & BIT(pipe)) == 0)
> -		return pipe;
> -
> -	/* ignore everything above our pipe */
> -	primary_pipes &= ~GENMASK(7, pipe);
> -
> -	/* highest remaining bit should be our primary pipe */
> -	return fls(primary_pipes) - 1;
> -}
> -
> -static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
> -{
> -	enum pipe primary_pipe, next_primary_pipe;
> -
> -	primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes);
> -
> -	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;
> -
> -	return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe);
> +	*primary_pipes = 0;
> +	*secondary_pipes = 0;
> +
> +	if (uncompressed_joiner_pipes & BIT(pipe)) {
> +		*primary_pipes = get_joiner_primary_pipe(pipe, primary_uncompressed_joiner_pipes);
> +		*secondary_pipes = secondary_uncompressed_joiner_pipes &
> +				   expected_uncompressed_joiner_secondary_pipes(*primary_pipes);
> +
> +		drm_WARN(display->drm,
> +			 expected_uncompressed_joiner_secondary_pipes(*primary_pipes) !=
> +			 *secondary_pipes,
> +			 "Wrong uncompressed joiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n",
> +			 *primary_pipes,
> +			 expected_uncompressed_joiner_secondary_pipes(*primary_pipes),
> +			 *secondary_pipes);
> +		return;
> +	}
> +
> +	if (bigjoiner_pipes & BIT(pipe)) {
> +		*primary_pipes = get_joiner_primary_pipe(pipe, primary_bigjoiner_pipes);
> +		*secondary_pipes = secondary_bigjoiner_pipes &
> +				   expected_bigjoiner_secondary_pipes(*primary_pipes);
> +
> +		drm_WARN(display->drm,
> +			 expected_bigjoiner_secondary_pipes(*primary_pipes) !=
> +			 *secondary_pipes,
> +			 "Wrong bigjoiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n",
> +			 *primary_pipes,
> +			 expected_bigjoiner_secondary_pipes(*primary_pipes),
> +			 *secondary_pipes);
> +		return;
> +	}
>  }
>  
>  static u8 hsw_panel_transcoders(struct drm_i915_private *i915)
> @@ -3813,10 +3814,10 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc)
>  		enabled_transcoders |= BIT(cpu_transcoder);
>  
>  	/* joiner secondary -> consider the primary pipe's transcoder as well */
> -	enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes);
> +	enabled_joiner_pipes(dev_priv, crtc->pipe, &primary_pipes, &secondary_pipes);
>  	if (secondary_pipes & BIT(crtc->pipe)) {
>  		cpu_transcoder = (enum transcoder)
> -			get_joiner_primary_pipe(crtc->pipe, primary_pipes, secondary_pipes);
> +			ffs(get_joiner_primary_pipe(crtc->pipe, primary_pipes)) - 1;

The get_joiner_primary_pipe() shouldn't be needed here since
enabled_joiner_pipes() guarantees that only one bit is set.

With that
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  		if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder))
>  			enabled_transcoders |= BIT(cpu_transcoder);
>  	}
> @@ -3950,14 +3951,12 @@ static void intel_joiner_get_config(struct intel_crtc_state *crtc_state)
>  	u8 primary_pipes, secondary_pipes;
>  	enum pipe pipe = crtc->pipe;
>  
> -	enabled_joiner_pipes(i915, &primary_pipes, &secondary_pipes);
> +	enabled_joiner_pipes(i915, pipe, &primary_pipes, &secondary_pipes);
>  
>  	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 = primary_pipes | secondary_pipes;
>  }
>  
>  static bool hsw_get_pipe_config(struct intel_crtc *crtc,
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

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

[Ville Syrjälä]

On Thu, Sep 26, 2024 at 12:56:32PM +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)
> v5: Remove spurious newline. (Ville)
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display.c  | 88 +++++++++++++++++++
>  .../gpu/drm/i915/display/intel_vdsc_regs.h    |  2 +
>  2 files changed, 90 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index ea259b142786..6d8143f62110 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3670,15 +3670,77 @@ static u8 get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes)
>  	return primary_pipes ? BIT(fls(primary_pipes) - 1) : 0;
>  }
>  
> +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 void enabled_ultrajoiner_pipes(struct drm_i915_private *i915,
> +				      u8 *primary_pipes, u8 *secondary_pipes)
> +{
> +	struct intel_crtc *crtc;
> +
> +	*primary_pipes = 0;
> +	*secondary_pipes = 0;
> +
> +	if (!HAS_ULTRAJOINER(i915))
> +		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,
>  				 enum pipe pipe,
>  				 u8 *primary_pipes, u8 *secondary_pipes)
>  {
>  	struct intel_display *display = to_intel_display(&dev_priv->drm);
> +	u8 primary_ultrajoiner_pipes;
>  	u8 primary_uncompressed_joiner_pipes, primary_bigjoiner_pipes;
> +	u8 secondary_ultrajoiner_pipes;
>  	u8 secondary_uncompressed_joiner_pipes, secondary_bigjoiner_pipes;
> +	u8 ultrajoiner_pipes;
>  	u8 uncompressed_joiner_pipes, bigjoiner_pipes;
>  
> +	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);
> +
>  	enabled_uncompressed_joiner_pipes(display, &primary_uncompressed_joiner_pipes,
>  					  &secondary_uncompressed_joiner_pipes);
>  
> @@ -3691,10 +3753,21 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
>  	drm_WARN_ON(display->drm,
>  		    (primary_bigjoiner_pipes & secondary_bigjoiner_pipes) != 0);
>  
> +	ultrajoiner_pipes = primary_ultrajoiner_pipes | secondary_ultrajoiner_pipes;
>  	uncompressed_joiner_pipes = primary_uncompressed_joiner_pipes |
>  				    secondary_uncompressed_joiner_pipes;
>  	bigjoiner_pipes = primary_bigjoiner_pipes | secondary_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_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);
> +
>  	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);
> @@ -3714,6 +3787,21 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
>  	*primary_pipes = 0;
>  	*secondary_pipes = 0;
>  
> +	if (ultrajoiner_pipes & BIT(pipe)) {
> +		*primary_pipes = get_joiner_primary_pipe(pipe, primary_ultrajoiner_pipes);
> +		*secondary_pipes = secondary_ultrajoiner_pipes &
> +				   expected_ultrajoiner_secondary_pipes(*primary_pipes);
> +
> +		drm_WARN(display->drm,
> +			 expected_ultrajoiner_secondary_pipes(*primary_pipes) !=
> +			 *secondary_pipes,
> +			 "Wrong ultrajoiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n",
> +			 *primary_pipes,
> +			 expected_ultrajoiner_secondary_pipes(*primary_pipes),
> +			 *secondary_pipes);
> +		return;
> +	}
> +
>  	if (uncompressed_joiner_pipes & BIT(pipe)) {
>  		*primary_pipes = get_joiner_primary_pipe(pipe, primary_uncompressed_joiner_pipes);
>  		*secondary_pipes = secondary_uncompressed_joiner_pipes &
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc_regs.h b/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
> index f921ad67b587..bf32a3b46fb1 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc_regs.h
> @@ -37,6 +37,8 @@
>  #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

-- 
Ville Syrjälä
Intel

Re: [PATCH 12/15] drm/i915/dp: Simplify helper to get slice count with joiner

[Ville Syrjälä]

On Thu, Sep 26, 2024 at 12:56:35PM +0530, Ankit Nautiyal wrote:
> From: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> 
> When bigjoiner is used, we need at least 2 dsc slices per pipe.
> Modify the condition in intel_dp_dsc_get_slice_count() to reflect the
> same.
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index a0afb4991334..eb92fda21e87 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -999,8 +999,12 @@ u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
>  		    drm_dp_dsc_sink_max_slice_count(connector->dp.dsc_dpcd, false))
>  			break;
>  
> -		/* big joiner needs small joiner to be enabled */
> -		if (num_joined_pipes == 2 && test_slice_count < 4)
> +		 /*
> +		  * Bigjoiner needs small joiner to be enabled.
> +		  * So there should be at least 2 dsc slices per pipe,
> +		  * whenever bigjoiner is enabled.
> +		  */
> +		if (num_joined_pipes > 1 && valid_dsc_slicecount[i] < 2)
>  			continue;
>  
>  		if (min_slice_count <= test_slice_count)
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 02/15] drm/i915/display_debugfs: Allow force joiner only if supported

[Nautiyal, Ankit K]

On 9/26/2024 4:44 PM, Ville Syrjälä wrote:
> On Thu, Sep 26, 2024 at 12:56:25PM +0530, Ankit Nautiyal wrote:
>> Currently we support joiner only for DP encoder.
>> Do not create the debugfs for joiner if DP does not support the joiner.
>> This will also help avoiding cases where config has eDP MSO, with which
>> we do not support joiner.
>>
>> v2: Check for intel_dp_has_joiner and avoid creating debugfs if not
>> supported. (Ville)
>> v3: Remove HAS_BIGJOINER check. (Ville)
>> v4: Reverse checks for connector type and intel_dp_has_joiner(). (Ville)
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display_debugfs.c | 7 ++++---
>>   1 file changed, 4 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 890ef7067b77..08adeaa2e87f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> @@ -1328,6 +1328,7 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>>   	struct drm_i915_private *i915 = to_i915(connector->base.dev);
>>   	struct dentry *root = connector->base.debugfs_entry;
>>   	int connector_type = connector->base.connector_type;
>> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> I'd probably drop the local variable entirely since it
> can give us garbage for non-dp stuff.

Yeah, can directly use intel_attached_dp(connector) to avoid having 
intel_dp with some garbage values for non DP connectors.

Thanks for the review.

As an aside, now that the first 4 patches (that are dealing with the 
debugfs) are reviewed, can I send them as separate series and merge them?

This will help get the IGT changes merge for debugfs changes.

Thanks & Regards,

Ankit

>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>>   
>>   	/* The connector must have been registered beforehands. */
>>   	if (!root)
>> @@ -1362,9 +1363,9 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>>   				    connector, &i915_dsc_fractional_bpp_fops);
>>   	}
>>   
>> -	if (HAS_BIGJOINER(i915) &&
>> -	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
>> -	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
>> +	if ((connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
>> +	     connector_type == DRM_MODE_CONNECTOR_eDP) &&
>> +	    intel_dp_has_joiner(intel_dp)) {
>>   		debugfs_create_bool("i915_bigjoiner_force_enable", 0644, root,
>>   				    &connector->force_bigjoiner_enable);
>>   	}
>> -- 
>> 2.45.2

Re: [PATCH 08/15] drm/i915/display: Refactor enable_joiner_pipes

[Nautiyal, Ankit K]

On 9/26/2024 4:49 PM, Ville Syrjälä wrote:
> On Thu, Sep 26, 2024 at 12:56:31PM +0530, Ankit Nautiyal wrote:
>> Pass the current pipe into enabled_joiner_pipes(), and let it figure out
>> the proper bitmasks for us. Since the enabled_joiner_pipes now gets the
>> primary and secondary pipe wrt a given pipe, the helpers
>> to get primary pipe and secondary pipes are no longer required.
>>
>> v2:
>> -Simplify helper get_joiner_primary_pipes. (Ville)
>> -Nuke get_joiner_secondary_pipes. (Ville)
>> -Add more drm_WARNs final primary/secondary pipes. (Ville)
>> v3:
>> -Drop ultrajoiner stuff and add it in subsequent patches. (Ville)
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c | 89 ++++++++++----------
>>   1 file changed, 44 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 2d6260c3bca5..ea259b142786 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -3663,7 +3663,15 @@ static u8 expected_bigjoiner_secondary_pipes(u8 bigjoiner_primary_pipes)
>>   	return expected_secondary_pipes(bigjoiner_primary_pipes, 2);
>>   }
>>   
>> +static u8 get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes)
>> +{
>> +	primary_pipes &= GENMASK(pipe, 0);
>> +
>> +	return primary_pipes ? BIT(fls(primary_pipes) - 1) : 0;
>> +}
>> +
>>   static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
>> +				 enum pipe pipe,
>>   				 u8 *primary_pipes, u8 *secondary_pipes)
>>   {
>>   	struct intel_display *display = to_intel_display(&dev_priv->drm);
>> @@ -3703,45 +3711,38 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
>>   		 expected_uncompressed_joiner_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(display->drm, *secondary_pipes != *primary_pipes << 1,
>> -		 "Joiner misconfigured (primary pipes %#x, secondary pipes %#x)\n",
>> -		 *primary_pipes, *secondary_pipes);
>> -}
>> -
>> -static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
>> -{
>> -	if ((secondary_pipes & BIT(pipe)) == 0)
>> -		return pipe;
>> -
>> -	/* ignore everything above our pipe */
>> -	primary_pipes &= ~GENMASK(7, pipe);
>> -
>> -	/* highest remaining bit should be our primary pipe */
>> -	return fls(primary_pipes) - 1;
>> -}
>> -
>> -static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
>> -{
>> -	enum pipe primary_pipe, next_primary_pipe;
>> -
>> -	primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes);
>> -
>> -	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;
>> -
>> -	return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe);
>> +	*primary_pipes = 0;
>> +	*secondary_pipes = 0;
>> +
>> +	if (uncompressed_joiner_pipes & BIT(pipe)) {
>> +		*primary_pipes = get_joiner_primary_pipe(pipe, primary_uncompressed_joiner_pipes);
>> +		*secondary_pipes = secondary_uncompressed_joiner_pipes &
>> +				   expected_uncompressed_joiner_secondary_pipes(*primary_pipes);
>> +
>> +		drm_WARN(display->drm,
>> +			 expected_uncompressed_joiner_secondary_pipes(*primary_pipes) !=
>> +			 *secondary_pipes,
>> +			 "Wrong uncompressed joiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n",
>> +			 *primary_pipes,
>> +			 expected_uncompressed_joiner_secondary_pipes(*primary_pipes),
>> +			 *secondary_pipes);
>> +		return;
>> +	}
>> +
>> +	if (bigjoiner_pipes & BIT(pipe)) {
>> +		*primary_pipes = get_joiner_primary_pipe(pipe, primary_bigjoiner_pipes);
>> +		*secondary_pipes = secondary_bigjoiner_pipes &
>> +				   expected_bigjoiner_secondary_pipes(*primary_pipes);
>> +
>> +		drm_WARN(display->drm,
>> +			 expected_bigjoiner_secondary_pipes(*primary_pipes) !=
>> +			 *secondary_pipes,
>> +			 "Wrong bigjoiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n",
>> +			 *primary_pipes,
>> +			 expected_bigjoiner_secondary_pipes(*primary_pipes),
>> +			 *secondary_pipes);
>> +		return;
>> +	}
>>   }
>>   
>>   static u8 hsw_panel_transcoders(struct drm_i915_private *i915)
>> @@ -3813,10 +3814,10 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc)
>>   		enabled_transcoders |= BIT(cpu_transcoder);
>>   
>>   	/* joiner secondary -> consider the primary pipe's transcoder as well */
>> -	enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes);
>> +	enabled_joiner_pipes(dev_priv, crtc->pipe, &primary_pipes, &secondary_pipes);
>>   	if (secondary_pipes & BIT(crtc->pipe)) {
>>   		cpu_transcoder = (enum transcoder)
>> -			get_joiner_primary_pipe(crtc->pipe, primary_pipes, secondary_pipes);
>> +			ffs(get_joiner_primary_pipe(crtc->pipe, primary_pipes)) - 1;
> The get_joiner_primary_pipe() shouldn't be needed here since
> enabled_joiner_pipes() guarantees that only one bit is set.


I agree. Additionally, I was considering changing the input variable 
name from `primary_pipes` to `primary_pipe` in enabled_joiner_pipes for 
clarity. Does that make sense?

Regards,

Ankit

>
> With that
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>>   		if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder))
>>   			enabled_transcoders |= BIT(cpu_transcoder);
>>   	}
>> @@ -3950,14 +3951,12 @@ static void intel_joiner_get_config(struct intel_crtc_state *crtc_state)
>>   	u8 primary_pipes, secondary_pipes;
>>   	enum pipe pipe = crtc->pipe;
>>   
>> -	enabled_joiner_pipes(i915, &primary_pipes, &secondary_pipes);
>> +	enabled_joiner_pipes(i915, pipe, &primary_pipes, &secondary_pipes);
>>   
>>   	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 = primary_pipes | secondary_pipes;
>>   }
>>   
>>   static bool hsw_get_pipe_config(struct intel_crtc *crtc,
>> -- 
>> 2.45.2

Re: [PATCH 02/15] drm/i915/display_debugfs: Allow force joiner only if supported

[Ville Syrjälä]

On Thu, Sep 26, 2024 at 06:37:46PM +0530, Nautiyal, Ankit K wrote:
> 
> On 9/26/2024 4:44 PM, Ville Syrjälä wrote:
> > On Thu, Sep 26, 2024 at 12:56:25PM +0530, Ankit Nautiyal wrote:
> >> Currently we support joiner only for DP encoder.
> >> Do not create the debugfs for joiner if DP does not support the joiner.
> >> This will also help avoiding cases where config has eDP MSO, with which
> >> we do not support joiner.
> >>
> >> v2: Check for intel_dp_has_joiner and avoid creating debugfs if not
> >> supported. (Ville)
> >> v3: Remove HAS_BIGJOINER check. (Ville)
> >> v4: Reverse checks for connector type and intel_dp_has_joiner(). (Ville)
> >>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_display_debugfs.c | 7 ++++---
> >>   1 file changed, 4 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 890ef7067b77..08adeaa2e87f 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> >> @@ -1328,6 +1328,7 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
> >>   	struct drm_i915_private *i915 = to_i915(connector->base.dev);
> >>   	struct dentry *root = connector->base.debugfs_entry;
> >>   	int connector_type = connector->base.connector_type;
> >> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> > I'd probably drop the local variable entirely since it
> > can give us garbage for non-dp stuff.
> 
> Yeah, can directly use intel_attached_dp(connector) to avoid having 
> intel_dp with some garbage values for non DP connectors.
> 
> Thanks for the review.
> 
> As an aside, now that the first 4 patches (that are dealing with the 
> debugfs) are reviewed, can I send them as separate series and merge them?
> 
> This will help get the IGT changes merge for debugfs changes.

Sure. Though CI is bogged down still, so dunno when you can
expect results form there.

> 
> Thanks & Regards,
> 
> Ankit
> 
> >
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >>   
> >>   	/* The connector must have been registered beforehands. */
> >>   	if (!root)
> >> @@ -1362,9 +1363,9 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
> >>   				    connector, &i915_dsc_fractional_bpp_fops);
> >>   	}
> >>   
> >> -	if (HAS_BIGJOINER(i915) &&
> >> -	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> >> -	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
> >> +	if ((connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> >> +	     connector_type == DRM_MODE_CONNECTOR_eDP) &&
> >> +	    intel_dp_has_joiner(intel_dp)) {
> >>   		debugfs_create_bool("i915_bigjoiner_force_enable", 0644, root,
> >>   				    &connector->force_bigjoiner_enable);
> >>   	}
> >> -- 
> >> 2.45.2

-- 
Ville Syrjälä
Intel

Re: [PATCH 08/15] drm/i915/display: Refactor enable_joiner_pipes

[Ville Syrjälä]

On Thu, Sep 26, 2024 at 06:45:14PM +0530, Nautiyal, Ankit K wrote:
> 
> On 9/26/2024 4:49 PM, Ville Syrjälä wrote:
> > On Thu, Sep 26, 2024 at 12:56:31PM +0530, Ankit Nautiyal wrote:
> >> Pass the current pipe into enabled_joiner_pipes(), and let it figure out
> >> the proper bitmasks for us. Since the enabled_joiner_pipes now gets the
> >> primary and secondary pipe wrt a given pipe, the helpers
> >> to get primary pipe and secondary pipes are no longer required.
> >>
> >> v2:
> >> -Simplify helper get_joiner_primary_pipes. (Ville)
> >> -Nuke get_joiner_secondary_pipes. (Ville)
> >> -Add more drm_WARNs final primary/secondary pipes. (Ville)
> >> v3:
> >> -Drop ultrajoiner stuff and add it in subsequent patches. (Ville)
> >>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_display.c | 89 ++++++++++----------
> >>   1 file changed, 44 insertions(+), 45 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >> index 2d6260c3bca5..ea259b142786 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> @@ -3663,7 +3663,15 @@ static u8 expected_bigjoiner_secondary_pipes(u8 bigjoiner_primary_pipes)
> >>   	return expected_secondary_pipes(bigjoiner_primary_pipes, 2);
> >>   }
> >>   
> >> +static u8 get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes)
> >> +{
> >> +	primary_pipes &= GENMASK(pipe, 0);
> >> +
> >> +	return primary_pipes ? BIT(fls(primary_pipes) - 1) : 0;
> >> +}
> >> +
> >>   static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
> >> +				 enum pipe pipe,
> >>   				 u8 *primary_pipes, u8 *secondary_pipes)
> >>   {
> >>   	struct intel_display *display = to_intel_display(&dev_priv->drm);
> >> @@ -3703,45 +3711,38 @@ static void enabled_joiner_pipes(struct drm_i915_private *dev_priv,
> >>   		 expected_uncompressed_joiner_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(display->drm, *secondary_pipes != *primary_pipes << 1,
> >> -		 "Joiner misconfigured (primary pipes %#x, secondary pipes %#x)\n",
> >> -		 *primary_pipes, *secondary_pipes);
> >> -}
> >> -
> >> -static enum pipe get_joiner_primary_pipe(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
> >> -{
> >> -	if ((secondary_pipes & BIT(pipe)) == 0)
> >> -		return pipe;
> >> -
> >> -	/* ignore everything above our pipe */
> >> -	primary_pipes &= ~GENMASK(7, pipe);
> >> -
> >> -	/* highest remaining bit should be our primary pipe */
> >> -	return fls(primary_pipes) - 1;
> >> -}
> >> -
> >> -static u8 get_joiner_secondary_pipes(enum pipe pipe, u8 primary_pipes, u8 secondary_pipes)
> >> -{
> >> -	enum pipe primary_pipe, next_primary_pipe;
> >> -
> >> -	primary_pipe = get_joiner_primary_pipe(pipe, primary_pipes, secondary_pipes);
> >> -
> >> -	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;
> >> -
> >> -	return secondary_pipes & GENMASK(next_primary_pipe - 1, primary_pipe);
> >> +	*primary_pipes = 0;
> >> +	*secondary_pipes = 0;
> >> +
> >> +	if (uncompressed_joiner_pipes & BIT(pipe)) {
> >> +		*primary_pipes = get_joiner_primary_pipe(pipe, primary_uncompressed_joiner_pipes);
> >> +		*secondary_pipes = secondary_uncompressed_joiner_pipes &
> >> +				   expected_uncompressed_joiner_secondary_pipes(*primary_pipes);
> >> +
> >> +		drm_WARN(display->drm,
> >> +			 expected_uncompressed_joiner_secondary_pipes(*primary_pipes) !=
> >> +			 *secondary_pipes,
> >> +			 "Wrong uncompressed joiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n",
> >> +			 *primary_pipes,
> >> +			 expected_uncompressed_joiner_secondary_pipes(*primary_pipes),
> >> +			 *secondary_pipes);
> >> +		return;
> >> +	}
> >> +
> >> +	if (bigjoiner_pipes & BIT(pipe)) {
> >> +		*primary_pipes = get_joiner_primary_pipe(pipe, primary_bigjoiner_pipes);
> >> +		*secondary_pipes = secondary_bigjoiner_pipes &
> >> +				   expected_bigjoiner_secondary_pipes(*primary_pipes);
> >> +
> >> +		drm_WARN(display->drm,
> >> +			 expected_bigjoiner_secondary_pipes(*primary_pipes) !=
> >> +			 *secondary_pipes,
> >> +			 "Wrong bigjoiner secondary pipes for primary_pipes %#x (expected %#x, current %#x)\n",
> >> +			 *primary_pipes,
> >> +			 expected_bigjoiner_secondary_pipes(*primary_pipes),
> >> +			 *secondary_pipes);
> >> +		return;
> >> +	}
> >>   }
> >>   
> >>   static u8 hsw_panel_transcoders(struct drm_i915_private *i915)
> >> @@ -3813,10 +3814,10 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc)
> >>   		enabled_transcoders |= BIT(cpu_transcoder);
> >>   
> >>   	/* joiner secondary -> consider the primary pipe's transcoder as well */
> >> -	enabled_joiner_pipes(dev_priv, &primary_pipes, &secondary_pipes);
> >> +	enabled_joiner_pipes(dev_priv, crtc->pipe, &primary_pipes, &secondary_pipes);
> >>   	if (secondary_pipes & BIT(crtc->pipe)) {
> >>   		cpu_transcoder = (enum transcoder)
> >> -			get_joiner_primary_pipe(crtc->pipe, primary_pipes, secondary_pipes);
> >> +			ffs(get_joiner_primary_pipe(crtc->pipe, primary_pipes)) - 1;
> > The get_joiner_primary_pipe() shouldn't be needed here since
> > enabled_joiner_pipes() guarantees that only one bit is set.
> 
> 
> I agree. Additionally, I was considering changing the input variable 
> name from `primary_pipes` to `primary_pipe` in enabled_joiner_pipes for 
> clarity. Does that make sense?

Probably a good idea. It'll be a bit weird to have it as a bitmask,
but feels like returning just a single enum pipe could make
things more complicated. So yeah, 'u8 *primary_pipe' seems perhaps the
best compromise.

-- 
Ville Syrjälä
Intel

Re: [PATCH 11/15] drm/i915/dp: Modify compressed bpp limitations for ultrajoiner

[Ville Syrjälä]

On Thu, Sep 26, 2024 at 12:56:34PM +0530, Ankit Nautiyal wrote:
> Add compressed bpp limitations for ultrajoiner.
> 
> v2: Fix the case for 1 pipe. (Ankit)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 27 +++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index f2a2541c1091..a0afb4991334 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -865,24 +865,39 @@ 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_bigjoiner;
> +	u32 max_bpp;
>  
>  	/* Small Joiner Check: output bpp <= joiner RAM (bits) / Horiz. width */
>  	max_bpp_small_joiner_ram = small_joiner_ram_size_bits(i915) / mode_hdisplay;
>  
> -	if (num_joined_pipes == 2) {
> +	if (num_joined_pipes == 1)
> +		return max_bpp_small_joiner_ram;

Hmm. This seems to assume that small joiner will be
enabled. I can't immediately see anything that would
guarantee that is the case. But I suppose it's a safe
assumption in that we can then freely choose whether to
use small joiner or not based on other constraints.

> +
> +	if (num_joined_pipes > 1) {
>  		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;
> -		u32 max_bpp_bigjoiner =
> -			i915->display.cdclk.max_cdclk_freq * ppc * bigjoiner_interface_bits /
> +		int num_bigjoiners = num_joined_pipes / 2;
> +
> +		max_bpp_bigjoiner =
> +			i915->display.cdclk.max_cdclk_freq * ppc * bigjoiner_interface_bits *

The the '/' seems to have turned into a '*'.

>  			intel_dp_mode_to_fec_clock(mode_clock);
>  
> -		max_bpp_small_joiner_ram *= 2;
> +		max_bpp_bigjoiner *= num_bigjoiners;
> +
> +		max_bpp_small_joiner_ram *= num_joined_pipes;

I get the feeling we're not handling the MSO overlap properly in
this code. But that's not directly related to this patch I guess.

I think we need to split this function up into its
constituent parts. Right now it's mixing it all into
a big mush that's very hard to follow. Once that is
done this function should just collapse into:
 max_bpp = min(max_bpp, smalljoiner_ram_max_bpp())
 max_bpp = min(max_bpp, bigjoiner_bw_max_bpp())
 max_bpp = min(max_bpp, ultrajoiner_ram_max_bpp())

We should also extract functions for bigjoiner_interface_bits()
and ultrajoiner_ram_size_bits() so that we don't have to be
distracted by the actual numbers.

> +	}
> +
> +	max_bpp = min(max_bpp_small_joiner_ram, max_bpp_bigjoiner);
> +
> +	if (num_joined_pipes == 4) {
> +		u32 max_bpp_ultrajoiner_ram = (4 * 72 * 512) / mode_hdisplay;
>  
> -		return min(max_bpp_small_joiner_ram, max_bpp_bigjoiner);
> +		max_bpp = min(max_bpp, max_bpp_ultrajoiner_ram);
>  	}
>  
> -	return max_bpp_small_joiner_ram;
> +	return max_bpp;
>  }
>  
>  u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel