[PATCH 0/3] Bigjoiner refactoring

[PATCH 0/3] Bigjoiner refactoring

[Stanislav Lisovskiy]

There are few things we need to do for bigjoiner, in order
to improve code maintenance and also make testing for Bigjoiner
easier.
Those series contain addition of bigjoiner force debugfs option,
in order to be able to force bigjoiner even if there is no display
support, also we refactor pipe vs transcoder logic, as currently
it is a bit scattered between *_commit_modeset_enables/disables
and *_crtc_enable/disable functions. Same applies to encoders.
We made a decision to handle all the slaves in correspondent master
hook, so slaves and slave checks no longer would be in modesetting
level logic.

Stanislav Lisovskiy (3):
  drm/i915: Add bigjoiner force enable option to debugfs
  drm/i915/bigjoiner: Refactor bigjoiner state readout
  Start separating pipe vs transcoder set logic for bigjoiner during
    modeset

 drivers/gpu/drm/i915/display/intel_ddi.c      |   3 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 170 ++++++++++++++----
 .../drm/i915/display/intel_display_debugfs.c  |  59 ++++++
 .../drm/i915/display/intel_display_types.h    |   2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |   3 +-
 5 files changed, 202 insertions(+), 35 deletions(-)

-- 
2.37.3

[PATCH 1/3] drm/i915: Add bigjoiner force enable option to debugfs

[Stanislav Lisovskiy]

For validation purposes, it might be useful to be able to
force Bigjoiner mode, even if current dotclock/resolution
do not require that.
Lets add such to option to debugfs.

v2: - Apparently intel_dp_need_bigjoiner can't be used, when
      debugfs entry is created so lets just check manually
      the DISPLAY_VER.

v3: - Switch to intel_connector from drm_connector(Jani Nikula)
    - Remove redundant modeset lock(Jani Nikula)
    - Use kstrtobool_from_user for boolean value(Jani Nikula)

v4: - Apply the changes to proper function(Jani Nikula)

v5: - Removed unnecessary check from i915_bigjoiner_enable_show
      (Ville Syrjälä)
    - Added eDP connector check to intel_connector_debugfs_add
      (Ville Syrjälä)
    - Removed debug message in order to prevent dmesg flooding
      (Ville Syrjälä)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 59 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |  2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  3 +-
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 915420d0cef8f..aa95ecf2458ee 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1414,6 +1414,22 @@ out:	drm_modeset_unlock(&dev->mode_config.connection_mutex);
 	return ret;
 }
 
+static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
+{
+	struct intel_connector *connector = to_intel_connector(m->private);
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp;
+
+	if (!encoder)
+		return -ENODEV;
+
+	intel_dp = enc_to_intel_dp(encoder);
+
+	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
+
+	return 0;
+}
+
 static ssize_t i915_dsc_output_format_write(struct file *file,
 					    const char __user *ubuf,
 					    size_t len, loff_t *offp)
@@ -1435,12 +1451,39 @@ static ssize_t i915_dsc_output_format_write(struct file *file,
 	return len;
 }
 
+static ssize_t i915_bigjoiner_enable_fops_write(struct file *file,
+						const char __user *ubuf,
+						size_t len, loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	struct intel_connector *connector = m->private;
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	bool bigjoiner_en = 0;
+	int ret;
+
+	ret = kstrtobool_from_user(ubuf, len, &bigjoiner_en);
+	if (ret < 0)
+		return ret;
+
+	intel_dp->force_bigjoiner_enable = bigjoiner_en;
+	*offp += len;
+
+	return len;
+}
+
 static int i915_dsc_output_format_open(struct inode *inode,
 				       struct file *file)
 {
 	return single_open(file, i915_dsc_output_format_show, inode->i_private);
 }
 
+static int i915_bigjoiner_enable_open(struct inode *inode,
+				      struct file *file)
+{
+	return single_open(file, i915_bigjoiner_enable_show, inode->i_private);
+}
+
 static const struct file_operations i915_dsc_output_format_fops = {
 	.owner = THIS_MODULE,
 	.open = i915_dsc_output_format_open,
@@ -1529,6 +1572,15 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = {
 	.write = i915_dsc_fractional_bpp_write
 };
 
+static const struct file_operations i915_bigjoiner_enable_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_bigjoiner_enable_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+	.write = i915_bigjoiner_enable_fops_write
+};
+
 /*
  * Returns the Current CRTC's bpc.
  * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
@@ -1611,6 +1663,13 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
 				    connector, &i915_dsc_fractional_bpp_fops);
 	}
 
+	if (DISPLAY_VER(dev_priv) >= 11 &&
+	    (intel_connector->base.connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	     intel_connector->base.connector_type == DRM_MODE_CONNECTOR_eDP)) {
+		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
+				    &intel_connector->base, &i915_bigjoiner_enable_fops);
+	}
+
 	if (connector->connector_type == DRM_MODE_CONNECTOR_DSI ||
 	    connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
 	    connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 9a44350ba05dd..559302ff79c1a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1764,6 +1764,8 @@ struct intel_dp {
 	bool is_mst;
 	int active_mst_links;
 
+	bool force_bigjoiner_enable;
+
 	/* connector directly attached - won't be use for modeset in mst world */
 	struct intel_connector *attached_connector;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 54bd0bffa9f08..5b8411b9fc30b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1172,7 +1172,8 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
 	if (!intel_dp_can_bigjoiner(intel_dp))
 		return false;
 
-	return clock > i915->max_dotclk_freq || hdisplay > 5120;
+	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
+	       intel_dp->force_bigjoiner_enable;
 }
 
 static enum drm_mode_status
-- 
2.37.3

[PATCH 2/3] drm/i915/bigjoiner: Refactor bigjoiner state readout

[Stanislav Lisovskiy]

Don't call enabled_bigjoiner_pipes twice, lets just move
intel_get_bigjoiner_config earlier, because it is anyway
calling same function.
Also cleanup hsw_enabled_transcoders from irrelevant bigjoiner code.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 125903007a292..29cb5dfa852b7 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3535,7 +3535,6 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	u8 panel_transcoder_mask = hsw_panel_transcoders(dev_priv);
 	enum transcoder cpu_transcoder;
-	u8 master_pipes, slave_pipes;
 	u8 enabled_transcoders = 0;
 
 	/*
@@ -3586,15 +3585,6 @@ static u8 hsw_enabled_transcoders(struct intel_crtc *crtc)
 	if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder))
 		enabled_transcoders |= BIT(cpu_transcoder);
 
-	/* bigjoiner slave -> consider the master pipe's transcoder as well */
-	enabled_bigjoiner_pipes(dev_priv, &master_pipes, &slave_pipes);
-	if (slave_pipes & BIT(crtc->pipe)) {
-		cpu_transcoder = (enum transcoder)
-			get_bigjoiner_master_pipe(crtc->pipe, master_pipes, slave_pipes);
-		if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder))
-			enabled_transcoders |= BIT(cpu_transcoder);
-	}
-
 	return enabled_transcoders;
 }
 
@@ -3641,6 +3631,15 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
 	u32 tmp;
 
 	enabled_transcoders = hsw_enabled_transcoders(crtc);
+
+	/* bigjoiner slave -> consider the master pipe's transcoder as well */
+	if (intel_crtc_is_bigjoiner_slave(pipe_config)) {
+		unsigned long cpu_transcoder = (enum transcoder)
+			bigjoiner_master_pipe(pipe_config);
+		if (transcoder_ddi_func_is_enabled(dev_priv, cpu_transcoder))
+			enabled_transcoders |= BIT(cpu_transcoder);
+	}
+
 	if (!enabled_transcoders)
 		return false;
 
@@ -3745,6 +3744,8 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
 
 	pipe_config->shared_dpll = NULL;
 
+	intel_bigjoiner_get_config(pipe_config);
+
 	active = hsw_get_transcoder_state(crtc, pipe_config, &crtc->hw_readout_power_domains);
 
 	if ((IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) &&
@@ -3757,7 +3758,6 @@ static bool hsw_get_pipe_config(struct intel_crtc *crtc,
 		goto out;
 
 	intel_dsc_get_config(pipe_config);
-	intel_bigjoiner_get_config(pipe_config);
 
 	if (!transcoder_is_dsi(pipe_config->cpu_transcoder) ||
 	    DISPLAY_VER(dev_priv) >= 11)
-- 
2.37.3

[PATCH 3/3] Start separating pipe vs transcoder set logic for bigjoiner during modeset

[Stanislav Lisovskiy]

Handle only bigjoiner masters in skl_commit_modeset_enables/disables,
slave crtcs should be handled by master hooks. Same for encoders.
That way we can also remove a bunch of checks like intel_crtc_is_bigjoiner_slave.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c     |   3 +-
 drivers/gpu/drm/i915/display/intel_display.c | 148 ++++++++++++++++---
 2 files changed, 128 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index f70af660dfcfa..ec643010452ca 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3344,8 +3344,7 @@ static void intel_enable_ddi(struct intel_atomic_state *state,
 {
 	drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
 
-	if (!intel_crtc_is_bigjoiner_slave(crtc_state))
-		intel_ddi_enable_transcoder_func(encoder, crtc_state);
+	intel_ddi_enable_transcoder_func(encoder, crtc_state);
 
 	/* Enable/Disable DP2.0 SDP split config before transcoder */
 	intel_audio_sdp_split_update(crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 29cb5dfa852b7..53e5ccfdf062a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1551,6 +1551,93 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
 	hsw_set_transconf(crtc_state);
 }
 
+static void hsw_crtc_enable_slave(struct intel_atomic_state *state,
+				  struct intel_crtc *crtc)
+{
+	const struct intel_crtc_state *new_crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe, hsw_workaround_pipe;
+	bool psl_clkgate_wa;
+
+	if (drm_WARN_ON(&dev_priv->drm, crtc->active))
+		return;
+
+	intel_dmc_enable_pipe(dev_priv, crtc->pipe);
+
+	if (!new_crtc_state->bigjoiner_pipes) {
+		intel_encoders_pre_pll_enable(state, crtc);
+
+		if (new_crtc_state->shared_dpll)
+			intel_enable_shared_dpll(new_crtc_state);
+
+		intel_encoders_pre_enable(state, crtc);
+	} else {
+		icl_ddi_bigjoiner_pre_enable(state, new_crtc_state);
+	}
+
+	intel_dsc_enable(new_crtc_state);
+
+	if (DISPLAY_VER(dev_priv) >= 13)
+		intel_uncompressed_joiner_enable(new_crtc_state);
+
+	intel_set_pipe_src_size(new_crtc_state);
+	if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
+		bdw_set_pipe_misc(new_crtc_state);
+
+	crtc->active = true;
+
+	/* Display WA #1180: WaDisableScalarClockGating: glk */
+	psl_clkgate_wa = DISPLAY_VER(dev_priv) == 10 &&
+		new_crtc_state->pch_pfit.enabled;
+	if (psl_clkgate_wa)
+		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
+
+	if (DISPLAY_VER(dev_priv) >= 9)
+		skl_pfit_enable(new_crtc_state);
+	else
+		ilk_pfit_enable(new_crtc_state);
+
+	/*
+	 * On ILK+ LUT must be loaded before the pipe is running but with
+	 * clocks enabled
+	 */
+	intel_color_load_luts(new_crtc_state);
+	intel_color_commit_noarm(new_crtc_state);
+	intel_color_commit_arm(new_crtc_state);
+	/* update DSPCNTR to configure gamma/csc for pipe bottom color */
+	if (DISPLAY_VER(dev_priv) < 9)
+		intel_disable_primary_plane(new_crtc_state);
+
+	hsw_set_linetime_wm(new_crtc_state);
+
+	if (DISPLAY_VER(dev_priv) >= 11)
+		icl_set_pipe_chicken(new_crtc_state);
+
+	intel_initial_watermarks(state, crtc);
+
+	intel_crtc_vblank_on(new_crtc_state);
+
+	intel_encoders_enable(state, crtc);
+
+	if (psl_clkgate_wa) {
+		intel_crtc_wait_for_next_vblank(crtc);
+		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, false);
+	}
+
+	/* If we change the relative order between pipe/planes enabling, we need
+	 * to change the workaround. */
+	hsw_workaround_pipe = new_crtc_state->hsw_workaround_pipe;
+	if (IS_HASWELL(dev_priv) && hsw_workaround_pipe != INVALID_PIPE) {
+		struct intel_crtc *wa_crtc;
+
+		wa_crtc = intel_crtc_for_pipe(dev_priv, hsw_workaround_pipe);
+
+		intel_crtc_wait_for_next_vblank(wa_crtc);
+		intel_crtc_wait_for_next_vblank(wa_crtc);
+	}
+}
+
 static void hsw_crtc_enable(struct intel_atomic_state *state,
 			    struct intel_crtc *crtc)
 {
@@ -1560,10 +1647,16 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
 	enum pipe pipe = crtc->pipe, hsw_workaround_pipe;
 	enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder;
 	bool psl_clkgate_wa;
+	struct intel_crtc *slave_crtc;
 
 	if (drm_WARN_ON(&dev_priv->drm, crtc->active))
 		return;
 
+	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, slave_crtc,
+					 intel_crtc_bigjoiner_slave_pipes(new_crtc_state)) {
+		hsw_crtc_enable_slave(state, slave_crtc);
+	}
+
 	intel_dmc_enable_pipe(dev_priv, crtc->pipe);
 
 	if (!new_crtc_state->bigjoiner_pipes) {
@@ -1586,8 +1679,7 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
 	if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
 		bdw_set_pipe_misc(new_crtc_state);
 
-	if (!intel_crtc_is_bigjoiner_slave(new_crtc_state) &&
-	    !transcoder_is_dsi(cpu_transcoder))
+	if (!transcoder_is_dsi(cpu_transcoder))
 		hsw_configure_cpu_transcoder(new_crtc_state);
 
 	crtc->active = true;
@@ -1621,9 +1713,6 @@ static void hsw_crtc_enable(struct intel_atomic_state *state,
 
 	intel_initial_watermarks(state, crtc);
 
-	if (intel_crtc_is_bigjoiner_slave(new_crtc_state))
-		intel_crtc_vblank_on(new_crtc_state);
-
 	intel_encoders_enable(state, crtc);
 
 	if (psl_clkgate_wa) {
@@ -1698,6 +1787,15 @@ static void ilk_crtc_disable(struct intel_atomic_state *state,
 	intel_disable_shared_dpll(old_crtc_state);
 }
 
+static void hsw_crtc_disable_slave(struct intel_atomic_state *state,
+				   struct intel_crtc *crtc)
+{
+	const struct intel_crtc_state *old_crtc_state =
+		intel_atomic_get_old_crtc_state(state, crtc);
+
+	intel_disable_shared_dpll(old_crtc_state);
+}
+
 static void hsw_crtc_disable(struct intel_atomic_state *state,
 			     struct intel_crtc *crtc)
 {
@@ -1709,23 +1807,21 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
 	 * FIXME collapse everything to one hook.
 	 * Need care with mst->ddi interactions.
 	 */
-	if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) {
-		intel_encoders_disable(state, crtc);
-		intel_encoders_post_disable(state, crtc);
-	}
+	intel_encoders_disable(state, crtc);
+	intel_encoders_post_disable(state, crtc);
 
 	intel_disable_shared_dpll(old_crtc_state);
 
-	if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) {
-		struct intel_crtc *slave_crtc;
+	struct intel_crtc *slave_crtc;
 
-		intel_encoders_post_pll_disable(state, crtc);
+	intel_encoders_post_pll_disable(state, crtc);
 
-		intel_dmc_disable_pipe(i915, crtc->pipe);
+	intel_dmc_disable_pipe(i915, crtc->pipe);
 
-		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
-						 intel_crtc_bigjoiner_slave_pipes(old_crtc_state))
-			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
+					 intel_crtc_bigjoiner_slave_pipes(old_crtc_state)) {
+		intel_dmc_disable_pipe(i915, slave_crtc->pipe);
+		hsw_crtc_disable_slave(state, slave_crtc);
 	}
 }
 
@@ -6900,8 +6996,10 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		 * Slave vblanks are masked till Master Vblanks.
 		 */
 		if (!is_trans_port_sync_slave(old_crtc_state) &&
-		    !intel_dp_mst_is_slave_trans(old_crtc_state) &&
-		    !intel_crtc_is_bigjoiner_slave(old_crtc_state))
+		    !intel_dp_mst_is_slave_trans(old_crtc_state))
+			continue;
+
+		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
 			continue;
 
 		intel_old_crtc_state_disables(state, old_crtc_state,
@@ -6919,6 +7017,9 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		if (!old_crtc_state->hw.active)
 			continue;
 
+		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
+			continue;
+
 		intel_old_crtc_state_disables(state, old_crtc_state,
 					      new_crtc_state, crtc);
 	}
@@ -7031,8 +7132,10 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 			continue;
 
 		if (intel_dp_mst_is_slave_trans(new_crtc_state) ||
-		    is_trans_port_sync_master(new_crtc_state) ||
-		    intel_crtc_is_bigjoiner_master(new_crtc_state))
+		    is_trans_port_sync_master(new_crtc_state))
+			continue;
+
+		if (intel_crtc_is_bigjoiner_slave(new_crtc_state))
 			continue;
 
 		modeset_pipes &= ~BIT(pipe);
@@ -7042,7 +7145,7 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 
 	/*
 	 * Then we enable all remaining pipes that depend on other
-	 * pipes: MST slaves and port sync masters, big joiner master
+	 * pipes: MST slaves and port sync masters
 	 */
 	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
 		enum pipe pipe = crtc->pipe;
@@ -7050,6 +7153,9 @@ static void skl_commit_modeset_enables(struct intel_atomic_state *state)
 		if ((modeset_pipes & BIT(pipe)) == 0)
 			continue;
 
+		if (intel_crtc_is_bigjoiner_slave(new_crtc_state))
+			continue;
+
 		modeset_pipes &= ~BIT(pipe);
 
 		intel_enable_crtc(state, crtc);
-- 
2.37.3

Re: [PATCH 1/3] drm/i915: Add bigjoiner force enable option to debugfs

[Jani Nikula]

On Mon, 08 Jan 2024, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote:
> For validation purposes, it might be useful to be able to
> force Bigjoiner mode, even if current dotclock/resolution
> do not require that.
> Lets add such to option to debugfs.
>
> v2: - Apparently intel_dp_need_bigjoiner can't be used, when
>       debugfs entry is created so lets just check manually
>       the DISPLAY_VER.
>
> v3: - Switch to intel_connector from drm_connector(Jani Nikula)
>     - Remove redundant modeset lock(Jani Nikula)
>     - Use kstrtobool_from_user for boolean value(Jani Nikula)
>
> v4: - Apply the changes to proper function(Jani Nikula)
>
> v5: - Removed unnecessary check from i915_bigjoiner_enable_show
>       (Ville Syrjälä)
>     - Added eDP connector check to intel_connector_debugfs_add
>       (Ville Syrjälä)
>     - Removed debug message in order to prevent dmesg flooding
>       (Ville Syrjälä)
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 59 +++++++++++++++++++
>  .../drm/i915/display/intel_display_types.h    |  2 +
>  drivers/gpu/drm/i915/display/intel_dp.c       |  3 +-
>  3 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 915420d0cef8f..aa95ecf2458ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1414,6 +1414,22 @@ out:	drm_modeset_unlock(&dev->mode_config.connection_mutex);
>  	return ret;
>  }
>  
> +static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
> +{
> +	struct intel_connector *connector = to_intel_connector(m->private);

This expects m->private to be struct drm_connector *.

> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> +	struct intel_dp *intel_dp;
> +
> +	if (!encoder)
> +		return -ENODEV;
> +
> +	intel_dp = enc_to_intel_dp(encoder);
> +
> +	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
> +
> +	return 0;
> +}
> +
>  static ssize_t i915_dsc_output_format_write(struct file *file,
>  					    const char __user *ubuf,
>  					    size_t len, loff_t *offp)
> @@ -1435,12 +1451,39 @@ static ssize_t i915_dsc_output_format_write(struct file *file,
>  	return len;
>  }
>  
> +static ssize_t i915_bigjoiner_enable_fops_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;

While this one expects it to be struct intel_connector *.

> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +	bool bigjoiner_en = 0;

Please initialize bools with true/false, not 0.

> +	int ret;
> +
> +	ret = kstrtobool_from_user(ubuf, len, &bigjoiner_en);
> +	if (ret < 0)
> +		return ret;
> +
> +	intel_dp->force_bigjoiner_enable = bigjoiner_en;
> +	*offp += len;
> +
> +	return len;
> +}
> +
>  static int i915_dsc_output_format_open(struct inode *inode,
>  				       struct file *file)
>  {
>  	return single_open(file, i915_dsc_output_format_show, inode->i_private);
>  }
>  
> +static int i915_bigjoiner_enable_open(struct inode *inode,
> +				      struct file *file)
> +{
> +	return single_open(file, i915_bigjoiner_enable_show, inode->i_private);
> +}
> +
>  static const struct file_operations i915_dsc_output_format_fops = {
>  	.owner = THIS_MODULE,
>  	.open = i915_dsc_output_format_open,
> @@ -1529,6 +1572,15 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = {
>  	.write = i915_dsc_fractional_bpp_write
>  };
>  
> +static const struct file_operations i915_bigjoiner_enable_fops = {
> +	.owner = THIS_MODULE,
> +	.open = i915_bigjoiner_enable_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +	.write = i915_bigjoiner_enable_fops_write
> +};
> +
>  /*
>   * Returns the Current CRTC's bpc.
>   * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
> @@ -1611,6 +1663,13 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
>  				    connector, &i915_dsc_fractional_bpp_fops);
>  	}
>  
> +	if (DISPLAY_VER(dev_priv) >= 11 &&
> +	    (intel_connector->base.connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> +	     intel_connector->base.connector_type == DRM_MODE_CONNECTOR_eDP)) {
> +		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
> +				    &intel_connector->base, &i915_bigjoiner_enable_fops);
> +	}
> +
>  	if (connector->connector_type == DRM_MODE_CONNECTOR_DSI ||
>  	    connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
>  	    connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||

You're on some old baseline. Please rebase.

> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 9a44350ba05dd..559302ff79c1a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1764,6 +1764,8 @@ struct intel_dp {
>  	bool is_mst;
>  	int active_mst_links;
>  
> +	bool force_bigjoiner_enable;
> +
>  	/* connector directly attached - won't be use for modeset in mst world */
>  	struct intel_connector *attached_connector;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 54bd0bffa9f08..5b8411b9fc30b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1172,7 +1172,8 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
>  	if (!intel_dp_can_bigjoiner(intel_dp))
>  		return false;
>  
> -	return clock > i915->max_dotclk_freq || hdisplay > 5120;
> +	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
> +	       intel_dp->force_bigjoiner_enable;
>  }
>  
>  static enum drm_mode_status

-- 
Jani Nikula, Intel

✗ Fi.CI.BUILD: failure for Bigjoiner refactoring

[Patchwork]

== Series Details ==

Series: Bigjoiner refactoring
URL   : https://patchwork.freedesktop.org/series/128311/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/128311/revisions/1/mbox/ not applied
Applying: drm/i915: Add bigjoiner force enable option to debugfs
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/display/intel_display_debugfs.c
M	drivers/gpu/drm/i915/display/intel_display_types.h
M	drivers/gpu/drm/i915/display/intel_dp.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/display/intel_dp.c
Auto-merging drivers/gpu/drm/i915/display/intel_display_types.h
Auto-merging drivers/gpu/drm/i915/display/intel_display_debugfs.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_display_debugfs.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915: Add bigjoiner force enable option to debugfs
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced

[PATCH 1/3] drm/i915: Add bigjoiner force enable option to debugfs

[Stanislav Lisovskiy]

For validation purposes, it might be useful to be able to
force Bigjoiner mode, even if current dotclock/resolution
do not require that.
Lets add such to option to debugfs.

v2: - Apparently intel_dp_need_bigjoiner can't be used, when
      debugfs entry is created so lets just check manually
      the DISPLAY_VER.

v3: - Switch to intel_connector from drm_connector(Jani Nikula)
    - Remove redundant modeset lock(Jani Nikula)
    - Use kstrtobool_from_user for boolean value(Jani Nikula)

v4: - Apply the changes to proper function(Jani Nikula)

v5: - Removed unnecessary check from i915_bigjoiner_enable_show
      (Ville Syrjälä)
    - Added eDP connector check to intel_connector_debugfs_add
      (Ville Syrjälä)
    - Removed debug message in order to prevent dmesg flooding
      (Ville Syrjälä)

v6: - Assume now always that m->private is intel_connector
    - Fixed other similar conflicts

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 59 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |  2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  3 +-
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index d951edb366871..353e71b4e1db2 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1413,6 +1413,22 @@ out:	drm_modeset_unlock(&i915->drm.mode_config.connection_mutex);
 	return ret;
 }
 
+static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
+{
+	struct intel_connector *connector = m->private;
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp;
+
+	if (!encoder)
+		return -ENODEV;
+
+	intel_dp = enc_to_intel_dp(encoder);
+
+	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
+
+	return 0;
+}
+
 static ssize_t i915_dsc_output_format_write(struct file *file,
 					    const char __user *ubuf,
 					    size_t len, loff_t *offp)
@@ -1434,12 +1450,39 @@ static ssize_t i915_dsc_output_format_write(struct file *file,
 	return len;
 }
 
+static ssize_t i915_bigjoiner_enable_fops_write(struct file *file,
+						const char __user *ubuf,
+						size_t len, loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	struct intel_connector *connector = m->private;
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	bool bigjoiner_en = 0;
+	int ret;
+
+	ret = kstrtobool_from_user(ubuf, len, &bigjoiner_en);
+	if (ret < 0)
+		return ret;
+
+	intel_dp->force_bigjoiner_enable = bigjoiner_en;
+	*offp += len;
+
+	return len;
+}
+
 static int i915_dsc_output_format_open(struct inode *inode,
 				       struct file *file)
 {
 	return single_open(file, i915_dsc_output_format_show, inode->i_private);
 }
 
+static int i915_bigjoiner_enable_open(struct inode *inode,
+				      struct file *file)
+{
+	return single_open(file, i915_bigjoiner_enable_show, inode->i_private);
+}
+
 static const struct file_operations i915_dsc_output_format_fops = {
 	.owner = THIS_MODULE,
 	.open = i915_dsc_output_format_open,
@@ -1527,6 +1570,15 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = {
 	.write = i915_dsc_fractional_bpp_write
 };
 
+static const struct file_operations i915_bigjoiner_enable_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_bigjoiner_enable_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+	.write = i915_bigjoiner_enable_fops_write
+};
+
 /*
  * Returns the Current CRTC's bpc.
  * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
@@ -1608,6 +1660,13 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
 				    connector, &i915_dsc_fractional_bpp_fops);
 	}
 
+	if (DISPLAY_VER(i915) >= 11 &&
+	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
+		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
+				    &connector->base, &i915_bigjoiner_enable_fops);
+	}
+
 	if (connector_type == DRM_MODE_CONNECTOR_DSI ||
 	    connector_type == DRM_MODE_CONNECTOR_eDP ||
 	    connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index b9b9d9f2bc0ba..e4c5a44dd02f5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1768,6 +1768,8 @@ struct intel_dp {
 	bool is_mst;
 	int active_mst_links;
 
+	bool force_bigjoiner_enable;
+
 	/* connector directly attached - won't be use for modeset in mst world */
 	struct intel_connector *attached_connector;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 9ff0cbd9c0df5..525ab926582d5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1208,7 +1208,8 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
 	if (!intel_dp_can_bigjoiner(intel_dp))
 		return false;
 
-	return clock > i915->max_dotclk_freq || hdisplay > 5120;
+	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
+	       intel_dp->force_bigjoiner_enable;
 }
 
 static enum drm_mode_status
-- 
2.37.3

Re: [PATCH 1/3] drm/i915: Add bigjoiner force enable option to debugfs

[Ville Syrjälä]

On Mon, Jan 08, 2024 at 02:07:23PM +0200, Stanislav Lisovskiy wrote:
> For validation purposes, it might be useful to be able to
> force Bigjoiner mode, even if current dotclock/resolution
> do not require that.
> Lets add such to option to debugfs.
> 
> v2: - Apparently intel_dp_need_bigjoiner can't be used, when
>       debugfs entry is created so lets just check manually
>       the DISPLAY_VER.
> 
> v3: - Switch to intel_connector from drm_connector(Jani Nikula)
>     - Remove redundant modeset lock(Jani Nikula)
>     - Use kstrtobool_from_user for boolean value(Jani Nikula)
> 
> v4: - Apply the changes to proper function(Jani Nikula)
> 
> v5: - Removed unnecessary check from i915_bigjoiner_enable_show
>       (Ville Syrjälä)
>     - Added eDP connector check to intel_connector_debugfs_add
>       (Ville Syrjälä)
>     - Removed debug message in order to prevent dmesg flooding
>       (Ville Syrjälä)
> 
> v6: - Assume now always that m->private is intel_connector
>     - Fixed other similar conflicts
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 59 +++++++++++++++++++
>  .../drm/i915/display/intel_display_types.h    |  2 +
>  drivers/gpu/drm/i915/display/intel_dp.c       |  3 +-
>  3 files changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index d951edb366871..353e71b4e1db2 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1413,6 +1413,22 @@ out:	drm_modeset_unlock(&i915->drm.mode_config.connection_mutex);
>  	return ret;
>  }
>  
> +static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
> +{
> +	struct intel_connector *connector = m->private;
> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> +	struct intel_dp *intel_dp;
> +
> +	if (!encoder)
> +		return -ENODEV;
> +
> +	intel_dp = enc_to_intel_dp(encoder);
> +
> +	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);

So it's a per-connector debugfs knob but we track it in the
SST DP encoder? That's rather odd, and not going to work for MST.

> +
> +	return 0;
> +}
> +
>  static ssize_t i915_dsc_output_format_write(struct file *file,
>  					    const char __user *ubuf,
>  					    size_t len, loff_t *offp)
> @@ -1434,12 +1450,39 @@ static ssize_t i915_dsc_output_format_write(struct file *file,
>  	return len;
>  }
>  
> +static ssize_t i915_bigjoiner_enable_fops_write(struct file *file,
> +						const char __user *ubuf,
> +						size_t len, loff_t *offp)
> +{
> +	struct seq_file *m = file->private_data;
> +	struct intel_connector *connector = m->private;
> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +	bool bigjoiner_en = 0;
> +	int ret;
> +
> +	ret = kstrtobool_from_user(ubuf, len, &bigjoiner_en);
> +	if (ret < 0)
> +		return ret;
> +
> +	intel_dp->force_bigjoiner_enable = bigjoiner_en;
> +	*offp += len;
> +
> +	return len;
> +}
> +
>  static int i915_dsc_output_format_open(struct inode *inode,
>  				       struct file *file)
>  {
>  	return single_open(file, i915_dsc_output_format_show, inode->i_private);
>  }
>  
> +static int i915_bigjoiner_enable_open(struct inode *inode,
> +				      struct file *file)
> +{
> +	return single_open(file, i915_bigjoiner_enable_show, inode->i_private);
> +}
> +
>  static const struct file_operations i915_dsc_output_format_fops = {
>  	.owner = THIS_MODULE,
>  	.open = i915_dsc_output_format_open,
> @@ -1527,6 +1570,15 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = {
>  	.write = i915_dsc_fractional_bpp_write
>  };
>  
> +static const struct file_operations i915_bigjoiner_enable_fops = {
> +	.owner = THIS_MODULE,
> +	.open = i915_bigjoiner_enable_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +	.write = i915_bigjoiner_enable_fops_write
> +};

Why are we implementing this long-hand for a simple boolean flag?

> +
>  /*
>   * Returns the Current CRTC's bpc.
>   * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
> @@ -1608,6 +1660,13 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>  				    connector, &i915_dsc_fractional_bpp_fops);
>  	}
>  
> +	if (DISPLAY_VER(i915) >= 11 &&
> +	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> +	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
> +		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
> +				    &connector->base, &i915_bigjoiner_enable_fops);
> +	}
> +
>  	if (connector_type == DRM_MODE_CONNECTOR_DSI ||
>  	    connector_type == DRM_MODE_CONNECTOR_eDP ||
>  	    connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index b9b9d9f2bc0ba..e4c5a44dd02f5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1768,6 +1768,8 @@ struct intel_dp {
>  	bool is_mst;
>  	int active_mst_links;
>  
> +	bool force_bigjoiner_enable;
> +
>  	/* connector directly attached - won't be use for modeset in mst world */
>  	struct intel_connector *attached_connector;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 9ff0cbd9c0df5..525ab926582d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1208,7 +1208,8 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
>  	if (!intel_dp_can_bigjoiner(intel_dp))
>  		return false;
>  
> -	return clock > i915->max_dotclk_freq || hdisplay > 5120;
> +	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
> +	       intel_dp->force_bigjoiner_enable;
>  }
>  
>  static enum drm_mode_status
> -- 
> 2.37.3

-- 
Ville Syrjälä
Intel

Re: [PATCH 1/3] drm/i915: Add bigjoiner force enable option to debugfs

[Lisovskiy, Stanislav]

On Fri, Jan 12, 2024 at 06:25:05PM +0200, Ville Syrjälä wrote:
> On Mon, Jan 08, 2024 at 02:07:23PM +0200, Stanislav Lisovskiy wrote:
> > For validation purposes, it might be useful to be able to
> > force Bigjoiner mode, even if current dotclock/resolution
> > do not require that.
> > Lets add such to option to debugfs.
> > 
> > v2: - Apparently intel_dp_need_bigjoiner can't be used, when
> >       debugfs entry is created so lets just check manually
> >       the DISPLAY_VER.
> > 
> > v3: - Switch to intel_connector from drm_connector(Jani Nikula)
> >     - Remove redundant modeset lock(Jani Nikula)
> >     - Use kstrtobool_from_user for boolean value(Jani Nikula)
> > 
> > v4: - Apply the changes to proper function(Jani Nikula)
> > 
> > v5: - Removed unnecessary check from i915_bigjoiner_enable_show
> >       (Ville Syrjälä)
> >     - Added eDP connector check to intel_connector_debugfs_add
> >       (Ville Syrjälä)
> >     - Removed debug message in order to prevent dmesg flooding
> >       (Ville Syrjälä)
> > 
> > v6: - Assume now always that m->private is intel_connector
> >     - Fixed other similar conflicts
> > 
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_debugfs.c  | 59 +++++++++++++++++++
> >  .../drm/i915/display/intel_display_types.h    |  2 +
> >  drivers/gpu/drm/i915/display/intel_dp.c       |  3 +-
> >  3 files changed, 63 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index d951edb366871..353e71b4e1db2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -1413,6 +1413,22 @@ out:	drm_modeset_unlock(&i915->drm.mode_config.connection_mutex);
> >  	return ret;
> >  }
> >  
> > +static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
> > +{
> > +	struct intel_connector *connector = m->private;
> > +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> > +	struct intel_dp *intel_dp;
> > +
> > +	if (!encoder)
> > +		return -ENODEV;
> > +
> > +	intel_dp = enc_to_intel_dp(encoder);
> > +
> > +	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
> 
> So it's a per-connector debugfs knob but we track it in the
> SST DP encoder? That's rather odd, and not going to work for MST.

I guess you mean, I should move it to the connector instead, makes sense.

> 
> > +
> > +	return 0;
> > +}
> > +
> >  static ssize_t i915_dsc_output_format_write(struct file *file,
> >  					    const char __user *ubuf,
> >  					    size_t len, loff_t *offp)
> > @@ -1434,12 +1450,39 @@ static ssize_t i915_dsc_output_format_write(struct file *file,
> >  	return len;
> >  }
> >  
> > +static ssize_t i915_bigjoiner_enable_fops_write(struct file *file,
> > +						const char __user *ubuf,
> > +						size_t len, loff_t *offp)
> > +{
> > +	struct seq_file *m = file->private_data;
> > +	struct intel_connector *connector = m->private;
> > +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +	bool bigjoiner_en = 0;
> > +	int ret;
> > +
> > +	ret = kstrtobool_from_user(ubuf, len, &bigjoiner_en);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	intel_dp->force_bigjoiner_enable = bigjoiner_en;
> > +	*offp += len;
> > +
> > +	return len;
> > +}
> > +
> >  static int i915_dsc_output_format_open(struct inode *inode,
> >  				       struct file *file)
> >  {
> >  	return single_open(file, i915_dsc_output_format_show, inode->i_private);
> >  }
> >  
> > +static int i915_bigjoiner_enable_open(struct inode *inode,
> > +				      struct file *file)
> > +{
> > +	return single_open(file, i915_bigjoiner_enable_show, inode->i_private);
> > +}
> > +
> >  static const struct file_operations i915_dsc_output_format_fops = {
> >  	.owner = THIS_MODULE,
> >  	.open = i915_dsc_output_format_open,
> > @@ -1527,6 +1570,15 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = {
> >  	.write = i915_dsc_fractional_bpp_write
> >  };
> >  
> > +static const struct file_operations i915_bigjoiner_enable_fops = {
> > +	.owner = THIS_MODULE,
> > +	.open = i915_bigjoiner_enable_open,
> > +	.read = seq_read,
> > +	.llseek = seq_lseek,
> > +	.release = single_release,
> > +	.write = i915_bigjoiner_enable_fops_write
> > +};
> 
> Why are we implementing this long-hand for a simple boolean flag?

Will check, thought thats the only way..

> 
> > +
> >  /*
> >   * Returns the Current CRTC's bpc.
> >   * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
> > @@ -1608,6 +1660,13 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
> >  				    connector, &i915_dsc_fractional_bpp_fops);
> >  	}
> >  
> > +	if (DISPLAY_VER(i915) >= 11 &&
> > +	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > +	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
> > +		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
> > +				    &connector->base, &i915_bigjoiner_enable_fops);
> > +	}
> > +
> >  	if (connector_type == DRM_MODE_CONNECTOR_DSI ||
> >  	    connector_type == DRM_MODE_CONNECTOR_eDP ||
> >  	    connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index b9b9d9f2bc0ba..e4c5a44dd02f5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1768,6 +1768,8 @@ struct intel_dp {
> >  	bool is_mst;
> >  	int active_mst_links;
> >  
> > +	bool force_bigjoiner_enable;
> > +
> >  	/* connector directly attached - won't be use for modeset in mst world */
> >  	struct intel_connector *attached_connector;
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 9ff0cbd9c0df5..525ab926582d5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1208,7 +1208,8 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
> >  	if (!intel_dp_can_bigjoiner(intel_dp))
> >  		return false;
> >  
> > -	return clock > i915->max_dotclk_freq || hdisplay > 5120;
> > +	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
> > +	       intel_dp->force_bigjoiner_enable;
> >  }
> >  
> >  static enum drm_mode_status
> > -- 
> > 2.37.3
> 
> -- 
> Ville Syrjälä
> Intel

Re: [PATCH 1/3] drm/i915: Add bigjoiner force enable option to debugfs

[Manasi Navare]

Thanks Stan for the patch.
I agree that since by forcing big joiner enable we are simulating a
higher mode/pixel clock on a connector, this should be a per connector
debugfs except for edp that can be exposed.

Manasi

On Mon, Jan 15, 2024 at 12:57 AM Lisovskiy, Stanislav
<stanislav.lisovskiy@intel.com> wrote:
>
> On Fri, Jan 12, 2024 at 06:25:05PM +0200, Ville Syrjälä wrote:
> > On Mon, Jan 08, 2024 at 02:07:23PM +0200, Stanislav Lisovskiy wrote:
> > > For validation purposes, it might be useful to be able to
> > > force Bigjoiner mode, even if current dotclock/resolution
> > > do not require that.
> > > Lets add such to option to debugfs.
> > >
> > > v2: - Apparently intel_dp_need_bigjoiner can't be used, when
> > >       debugfs entry is created so lets just check manually
> > >       the DISPLAY_VER.
> > >
> > > v3: - Switch to intel_connector from drm_connector(Jani Nikula)
> > >     - Remove redundant modeset lock(Jani Nikula)
> > >     - Use kstrtobool_from_user for boolean value(Jani Nikula)
> > >
> > > v4: - Apply the changes to proper function(Jani Nikula)
> > >
> > > v5: - Removed unnecessary check from i915_bigjoiner_enable_show
> > >       (Ville Syrjälä)
> > >     - Added eDP connector check to intel_connector_debugfs_add
> > >       (Ville Syrjälä)
> > >     - Removed debug message in order to prevent dmesg flooding
> > >       (Ville Syrjälä)
> > >
> > > v6: - Assume now always that m->private is intel_connector
> > >     - Fixed other similar conflicts
> > >
> > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > > ---
> > >  .../drm/i915/display/intel_display_debugfs.c  | 59 +++++++++++++++++++
> > >  .../drm/i915/display/intel_display_types.h    |  2 +
> > >  drivers/gpu/drm/i915/display/intel_dp.c       |  3 +-
> > >  3 files changed, 63 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > index d951edb366871..353e71b4e1db2 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > @@ -1413,6 +1413,22 @@ out: drm_modeset_unlock(&i915->drm.mode_config.connection_mutex);
> > >     return ret;
> > >  }
> > >
> > > +static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
> > > +{
> > > +   struct intel_connector *connector = m->private;
> > > +   struct intel_encoder *encoder = intel_attached_encoder(connector);
> > > +   struct intel_dp *intel_dp;
> > > +
> > > +   if (!encoder)
> > > +           return -ENODEV;
> > > +
> > > +   intel_dp = enc_to_intel_dp(encoder);
> > > +
> > > +   seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
> >
> > So it's a per-connector debugfs knob but we track it in the
> > SST DP encoder? That's rather odd, and not going to work for MST.
>
> I guess you mean, I should move it to the connector instead, makes sense.
>
> >
> > > +
> > > +   return 0;
> > > +}
> > > +
> > >  static ssize_t i915_dsc_output_format_write(struct file *file,
> > >                                         const char __user *ubuf,
> > >                                         size_t len, loff_t *offp)
> > > @@ -1434,12 +1450,39 @@ static ssize_t i915_dsc_output_format_write(struct file *file,
> > >     return len;
> > >  }
> > >
> > > +static ssize_t i915_bigjoiner_enable_fops_write(struct file *file,
> > > +                                           const char __user *ubuf,
> > > +                                           size_t len, loff_t *offp)
> > > +{
> > > +   struct seq_file *m = file->private_data;
> > > +   struct intel_connector *connector = m->private;
> > > +   struct intel_encoder *encoder = intel_attached_encoder(connector);
> > > +   struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > +   bool bigjoiner_en = 0;
> > > +   int ret;
> > > +
> > > +   ret = kstrtobool_from_user(ubuf, len, &bigjoiner_en);
> > > +   if (ret < 0)
> > > +           return ret;
> > > +
> > > +   intel_dp->force_bigjoiner_enable = bigjoiner_en;
> > > +   *offp += len;
> > > +
> > > +   return len;
> > > +}
> > > +
> > >  static int i915_dsc_output_format_open(struct inode *inode,
> > >                                    struct file *file)
> > >  {
> > >     return single_open(file, i915_dsc_output_format_show, inode->i_private);
> > >  }
> > >
> > > +static int i915_bigjoiner_enable_open(struct inode *inode,
> > > +                                 struct file *file)
> > > +{
> > > +   return single_open(file, i915_bigjoiner_enable_show, inode->i_private);
> > > +}
> > > +
> > >  static const struct file_operations i915_dsc_output_format_fops = {
> > >     .owner = THIS_MODULE,
> > >     .open = i915_dsc_output_format_open,
> > > @@ -1527,6 +1570,15 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = {
> > >     .write = i915_dsc_fractional_bpp_write
> > >  };
> > >
> > > +static const struct file_operations i915_bigjoiner_enable_fops = {
> > > +   .owner = THIS_MODULE,
> > > +   .open = i915_bigjoiner_enable_open,
> > > +   .read = seq_read,
> > > +   .llseek = seq_lseek,
> > > +   .release = single_release,
> > > +   .write = i915_bigjoiner_enable_fops_write
> > > +};
> >
> > Why are we implementing this long-hand for a simple boolean flag?
>
> Will check, thought thats the only way..
>
> >
> > > +
> > >  /*
> > >   * Returns the Current CRTC's bpc.
> > >   * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
> > > @@ -1608,6 +1660,13 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
> > >                                 connector, &i915_dsc_fractional_bpp_fops);
> > >     }
> > >
> > > +   if (DISPLAY_VER(i915) >= 11 &&
> > > +       (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > > +        connector_type == DRM_MODE_CONNECTOR_eDP)) {
> > > +           debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
> > > +                               &connector->base, &i915_bigjoiner_enable_fops);
> > > +   }
> > > +
> > >     if (connector_type == DRM_MODE_CONNECTOR_DSI ||
> > >         connector_type == DRM_MODE_CONNECTOR_eDP ||
> > >         connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index b9b9d9f2bc0ba..e4c5a44dd02f5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -1768,6 +1768,8 @@ struct intel_dp {
> > >     bool is_mst;
> > >     int active_mst_links;
> > >
> > > +   bool force_bigjoiner_enable;
> > > +
> > >     /* connector directly attached - won't be use for modeset in mst world */
> > >     struct intel_connector *attached_connector;
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 9ff0cbd9c0df5..525ab926582d5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -1208,7 +1208,8 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
> > >     if (!intel_dp_can_bigjoiner(intel_dp))
> > >             return false;
> > >
> > > -   return clock > i915->max_dotclk_freq || hdisplay > 5120;
> > > +   return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
> > > +          intel_dp->force_bigjoiner_enable;
> > >  }
> > >
> > >  static enum drm_mode_status
> > > --
> > > 2.37.3
> >
> > --
> > Ville Syrjälä
> > Intel

[PATCH 1/3] drm/i915: Add bigjoiner force enable option to debugfs

[Stanislav Lisovskiy]

For validation purposes, it might be useful to be able to
force Bigjoiner mode, even if current dotclock/resolution
do not require that.
Lets add such to option to debugfs.

v2: - Apparently intel_dp_need_bigjoiner can't be used, when
      debugfs entry is created so lets just check manually
      the DISPLAY_VER.

v3: - Switch to intel_connector from drm_connector(Jani Nikula)
    - Remove redundant modeset lock(Jani Nikula)
    - Use kstrtobool_from_user for boolean value(Jani Nikula)

v4: - Apply the changes to proper function(Jani Nikula)

v5: - Removed unnecessary check from i915_bigjoiner_enable_show
      (Ville Syrjälä)
    - Added eDP connector check to intel_connector_debugfs_add
      (Ville Syrjälä)
    - Removed debug message in order to prevent dmesg flooding
      (Ville Syrjälä)

v6: - Assume now always that m->private is intel_connector
    - Fixed other similar conflicts

v7: - Move bigjoiner force option to intel_connector(Ville Syrjälä)
    - Use DEFINE_SHOW_STORE_ATTRIBUTE instead of defining fops
      manually.(Ville Syrjälä)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 47 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |  2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index d951edb366871..ff0c7dfae89a3 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1413,6 +1413,20 @@ out:	drm_modeset_unlock(&i915->drm.mode_config.connection_mutex);
 	return ret;
 }
 
+static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
+{
+	struct intel_connector *connector = m->private;
+	struct drm_crtc *crtc;
+
+	crtc = connector->base.state->crtc;
+	if (connector->base.status != connector_status_connected || !crtc)
+		return -ENODEV;
+
+	seq_printf(m, "Bigjoiner enable: %d\n", connector->force_bigjoiner_enable);
+
+	return 0;
+}
+
 static ssize_t i915_dsc_output_format_write(struct file *file,
 					    const char __user *ubuf,
 					    size_t len, loff_t *offp)
@@ -1434,6 +1448,30 @@ static ssize_t i915_dsc_output_format_write(struct file *file,
 	return len;
 }
 
+static ssize_t i915_bigjoiner_enable_write(struct file *file,
+						const char __user *ubuf,
+						size_t len, loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	struct intel_connector *connector = m->private;
+	struct drm_crtc *crtc;
+	bool bigjoiner_en = 0;
+	int ret;
+
+	crtc = connector->base.state->crtc;
+	if (connector->base.status != connector_status_connected || !crtc)
+		return -ENODEV;
+
+	ret = kstrtobool_from_user(ubuf, len, &bigjoiner_en);
+	if (ret < 0)
+		return ret;
+
+	connector->force_bigjoiner_enable = bigjoiner_en;
+	*offp += len;
+
+	return len;
+}
+
 static int i915_dsc_output_format_open(struct inode *inode,
 				       struct file *file)
 {
@@ -1527,6 +1565,8 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = {
 	.write = i915_dsc_fractional_bpp_write
 };
 
+DEFINE_SHOW_STORE_ATTRIBUTE(i915_bigjoiner_enable);
+
 /*
  * Returns the Current CRTC's bpc.
  * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
@@ -1608,6 +1648,13 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
 				    connector, &i915_dsc_fractional_bpp_fops);
 	}
 
+	if (DISPLAY_VER(i915) >= 11 &&
+	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
+		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
+				    &connector->base, &i915_bigjoiner_enable_fops);
+	}
+
 	if (connector_type == DRM_MODE_CONNECTOR_DSI ||
 	    connector_type == DRM_MODE_CONNECTOR_eDP ||
 	    connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index ae2e8cff9d691..db38f2b8178a8 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -626,6 +626,8 @@ struct intel_connector {
 
 	struct intel_dp *mst_port;
 
+	bool force_bigjoiner_enable;
+
 	struct {
 		struct drm_dp_aux *dsc_decompression_aux;
 		u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE];
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index ab415f41924d7..d70890ca2b47d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1205,11 +1205,13 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
 			     int hdisplay, int clock)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+	struct intel_connector *connector = intel_dp->attached_connector;
 
 	if (!intel_dp_can_bigjoiner(intel_dp))
 		return false;
 
-	return clock > i915->max_dotclk_freq || hdisplay > 5120;
+	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
+	       connector->force_bigjoiner_enable;
 }
 
 static enum drm_mode_status
-- 
2.37.3

Re: [PATCH 1/3] drm/i915: Add bigjoiner force enable option to debugfs

[Jani Nikula]

On Thu, 18 Jan 2024, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote:
> For validation purposes, it might be useful to be able to
> force Bigjoiner mode, even if current dotclock/resolution
> do not require that.
> Lets add such to option to debugfs.
>
> v2: - Apparently intel_dp_need_bigjoiner can't be used, when
>       debugfs entry is created so lets just check manually
>       the DISPLAY_VER.
>
> v3: - Switch to intel_connector from drm_connector(Jani Nikula)
>     - Remove redundant modeset lock(Jani Nikula)
>     - Use kstrtobool_from_user for boolean value(Jani Nikula)
>
> v4: - Apply the changes to proper function(Jani Nikula)
>
> v5: - Removed unnecessary check from i915_bigjoiner_enable_show
>       (Ville Syrjälä)
>     - Added eDP connector check to intel_connector_debugfs_add
>       (Ville Syrjälä)
>     - Removed debug message in order to prevent dmesg flooding
>       (Ville Syrjälä)
>
> v6: - Assume now always that m->private is intel_connector
>     - Fixed other similar conflicts
>
> v7: - Move bigjoiner force option to intel_connector(Ville Syrjälä)
>     - Use DEFINE_SHOW_STORE_ATTRIBUTE instead of defining fops
>       manually.(Ville Syrjälä)
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 47 +++++++++++++++++++
>  .../drm/i915/display/intel_display_types.h    |  2 +
>  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
>  3 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index d951edb366871..ff0c7dfae89a3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1413,6 +1413,20 @@ out:	drm_modeset_unlock(&i915->drm.mode_config.connection_mutex);
>  	return ret;
>  }
>  
> +static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
> +{
> +	struct intel_connector *connector = m->private;
> +	struct drm_crtc *crtc;
> +
> +	crtc = connector->base.state->crtc;
> +	if (connector->base.status != connector_status_connected || !crtc)
> +		return -ENODEV;
> +
> +	seq_printf(m, "Bigjoiner enable: %d\n", connector->force_bigjoiner_enable);
> +
> +	return 0;
> +}
> +
>  static ssize_t i915_dsc_output_format_write(struct file *file,
>  					    const char __user *ubuf,
>  					    size_t len, loff_t *offp)
> @@ -1434,6 +1448,30 @@ static ssize_t i915_dsc_output_format_write(struct file *file,
>  	return len;
>  }
>  
> +static ssize_t i915_bigjoiner_enable_write(struct file *file,
> +						const char __user *ubuf,
> +						size_t len, loff_t *offp)
> +{
> +	struct seq_file *m = file->private_data;
> +	struct intel_connector *connector = m->private;
> +	struct drm_crtc *crtc;
> +	bool bigjoiner_en = 0;
> +	int ret;
> +
> +	crtc = connector->base.state->crtc;
> +	if (connector->base.status != connector_status_connected || !crtc)
> +		return -ENODEV;
> +
> +	ret = kstrtobool_from_user(ubuf, len, &bigjoiner_en);
> +	if (ret < 0)
> +		return ret;
> +
> +	connector->force_bigjoiner_enable = bigjoiner_en;
> +	*offp += len;
> +
> +	return len;
> +}
> +
>  static int i915_dsc_output_format_open(struct inode *inode,
>  				       struct file *file)
>  {
> @@ -1527,6 +1565,8 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = {
>  	.write = i915_dsc_fractional_bpp_write
>  };
>  
> +DEFINE_SHOW_STORE_ATTRIBUTE(i915_bigjoiner_enable);
> +
>  /*
>   * Returns the Current CRTC's bpc.
>   * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
> @@ -1608,6 +1648,13 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>  				    connector, &i915_dsc_fractional_bpp_fops);
>  	}
>  
> +	if (DISPLAY_VER(i915) >= 11 &&
> +	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> +	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
> +		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
> +				    &connector->base, &i915_bigjoiner_enable_fops);

I've replied to an earlier version of this patch telling you to pass
struct intel_connector * to debugfs_create_file() when your show/write
hooks expect that. This only works because offset of base member is 0,
and the private pointer is void *.

BR,
Jani.

> +	}
> +
>  	if (connector_type == DRM_MODE_CONNECTOR_DSI ||
>  	    connector_type == DRM_MODE_CONNECTOR_eDP ||
>  	    connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index ae2e8cff9d691..db38f2b8178a8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -626,6 +626,8 @@ struct intel_connector {
>  
>  	struct intel_dp *mst_port;
>  
> +	bool force_bigjoiner_enable;
> +
>  	struct {
>  		struct drm_dp_aux *dsc_decompression_aux;
>  		u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE];
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index ab415f41924d7..d70890ca2b47d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1205,11 +1205,13 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
>  			     int hdisplay, int clock)
>  {
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +	struct intel_connector *connector = intel_dp->attached_connector;
>  
>  	if (!intel_dp_can_bigjoiner(intel_dp))
>  		return false;
>  
> -	return clock > i915->max_dotclk_freq || hdisplay > 5120;
> +	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
> +	       connector->force_bigjoiner_enable;
>  }
>  
>  static enum drm_mode_status

-- 
Jani Nikula, Intel

Re: [PATCH 1/3] drm/i915: Add bigjoiner force enable option to debugfs

[Lisovskiy, Stanislav]

On Thu, Jan 18, 2024 at 01:53:41PM +0200, Jani Nikula wrote:
> On Thu, 18 Jan 2024, Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> wrote:
> > For validation purposes, it might be useful to be able to
> > force Bigjoiner mode, even if current dotclock/resolution
> > do not require that.
> > Lets add such to option to debugfs.
> >
> > v2: - Apparently intel_dp_need_bigjoiner can't be used, when
> >       debugfs entry is created so lets just check manually
> >       the DISPLAY_VER.
> >
> > v3: - Switch to intel_connector from drm_connector(Jani Nikula)
> >     - Remove redundant modeset lock(Jani Nikula)
> >     - Use kstrtobool_from_user for boolean value(Jani Nikula)
> >
> > v4: - Apply the changes to proper function(Jani Nikula)
> >
> > v5: - Removed unnecessary check from i915_bigjoiner_enable_show
> >       (Ville Syrjälä)
> >     - Added eDP connector check to intel_connector_debugfs_add
> >       (Ville Syrjälä)
> >     - Removed debug message in order to prevent dmesg flooding
> >       (Ville Syrjälä)
> >
> > v6: - Assume now always that m->private is intel_connector
> >     - Fixed other similar conflicts
> >
> > v7: - Move bigjoiner force option to intel_connector(Ville Syrjälä)
> >     - Use DEFINE_SHOW_STORE_ATTRIBUTE instead of defining fops
> >       manually.(Ville Syrjälä)
> >
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_debugfs.c  | 47 +++++++++++++++++++
> >  .../drm/i915/display/intel_display_types.h    |  2 +
> >  drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
> >  3 files changed, 52 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index d951edb366871..ff0c7dfae89a3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -1413,6 +1413,20 @@ out:	drm_modeset_unlock(&i915->drm.mode_config.connection_mutex);
> >  	return ret;
> >  }
> >  
> > +static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
> > +{
> > +	struct intel_connector *connector = m->private;
> > +	struct drm_crtc *crtc;
> > +
> > +	crtc = connector->base.state->crtc;
> > +	if (connector->base.status != connector_status_connected || !crtc)
> > +		return -ENODEV;
> > +
> > +	seq_printf(m, "Bigjoiner enable: %d\n", connector->force_bigjoiner_enable);
> > +
> > +	return 0;
> > +}
> > +
> >  static ssize_t i915_dsc_output_format_write(struct file *file,
> >  					    const char __user *ubuf,
> >  					    size_t len, loff_t *offp)
> > @@ -1434,6 +1448,30 @@ static ssize_t i915_dsc_output_format_write(struct file *file,
> >  	return len;
> >  }
> >  
> > +static ssize_t i915_bigjoiner_enable_write(struct file *file,
> > +						const char __user *ubuf,
> > +						size_t len, loff_t *offp)
> > +{
> > +	struct seq_file *m = file->private_data;
> > +	struct intel_connector *connector = m->private;
> > +	struct drm_crtc *crtc;
> > +	bool bigjoiner_en = 0;
> > +	int ret;
> > +
> > +	crtc = connector->base.state->crtc;
> > +	if (connector->base.status != connector_status_connected || !crtc)
> > +		return -ENODEV;
> > +
> > +	ret = kstrtobool_from_user(ubuf, len, &bigjoiner_en);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	connector->force_bigjoiner_enable = bigjoiner_en;
> > +	*offp += len;
> > +
> > +	return len;
> > +}
> > +
> >  static int i915_dsc_output_format_open(struct inode *inode,
> >  				       struct file *file)
> >  {
> > @@ -1527,6 +1565,8 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = {
> >  	.write = i915_dsc_fractional_bpp_write
> >  };
> >  
> > +DEFINE_SHOW_STORE_ATTRIBUTE(i915_bigjoiner_enable);
> > +
> >  /*
> >   * Returns the Current CRTC's bpc.
> >   * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
> > @@ -1608,6 +1648,13 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
> >  				    connector, &i915_dsc_fractional_bpp_fops);
> >  	}
> >  
> > +	if (DISPLAY_VER(i915) >= 11 &&
> > +	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > +	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
> > +		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
> > +				    &connector->base, &i915_bigjoiner_enable_fops);
> 
> I've replied to an earlier version of this patch telling you to pass
> struct intel_connector * to debugfs_create_file() when your show/write
> hooks expect that. This only works because offset of base member is 0,
> and the private pointer is void *.
> 
> BR,
> Jani.

Ahh.. I've seen but forgot to change that.
Anyways I will fix that, will now wait for feedback from Ville as well here, if
something else pops up.

Stan

> 
> > +	}
> > +
> >  	if (connector_type == DRM_MODE_CONNECTOR_DSI ||
> >  	    connector_type == DRM_MODE_CONNECTOR_eDP ||
> >  	    connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index ae2e8cff9d691..db38f2b8178a8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -626,6 +626,8 @@ struct intel_connector {
> >  
> >  	struct intel_dp *mst_port;
> >  
> > +	bool force_bigjoiner_enable;
> > +
> >  	struct {
> >  		struct drm_dp_aux *dsc_decompression_aux;
> >  		u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE];
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index ab415f41924d7..d70890ca2b47d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1205,11 +1205,13 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
> >  			     int hdisplay, int clock)
> >  {
> >  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > +	struct intel_connector *connector = intel_dp->attached_connector;
> >  
> >  	if (!intel_dp_can_bigjoiner(intel_dp))
> >  		return false;
> >  
> > -	return clock > i915->max_dotclk_freq || hdisplay > 5120;
> > +	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
> > +	       connector->force_bigjoiner_enable;
> >  }
> >  
> >  static enum drm_mode_status
> 
> -- 
> Jani Nikula, Intel

[PATCH 1/3] drm/i915: Add bigjoiner force enable option to debugfs

[Stanislav Lisovskiy]

For validation purposes, it might be useful to be able to
force Bigjoiner mode, even if current dotclock/resolution
do not require that.
Lets add such to option to debugfs.

v2: - Apparently intel_dp_need_bigjoiner can't be used, when
      debugfs entry is created so lets just check manually
      the DISPLAY_VER.

v3: - Switch to intel_connector from drm_connector(Jani Nikula)
    - Remove redundant modeset lock(Jani Nikula)
    - Use kstrtobool_from_user for boolean value(Jani Nikula)

v4: - Apply the changes to proper function(Jani Nikula)

v5: - Removed unnecessary check from i915_bigjoiner_enable_show
      (Ville Syrjälä)
    - Added eDP connector check to intel_connector_debugfs_add
      (Ville Syrjälä)
    - Removed debug message in order to prevent dmesg flooding
      (Ville Syrjälä)

v6: - Assume now always that m->private is intel_connector
    - Fixed other similar conflicts

v7: - Move bigjoiner force option to intel_connector(Ville Syrjälä)
    - Use DEFINE_SHOW_STORE_ATTRIBUTE instead of defining fops
      manually.(Ville Syrjälä)

v8: - Pass intel_connector to debugfs_create_file, instead of drm_connector.
      (Jani Nikula)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 47 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |  2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  4 +-
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index d951edb366871..8bb56ba2902ac 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1413,6 +1413,20 @@ out:	drm_modeset_unlock(&i915->drm.mode_config.connection_mutex);
 	return ret;
 }
 
+static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
+{
+	struct intel_connector *connector = m->private;
+	struct drm_crtc *crtc;
+
+	crtc = connector->base.state->crtc;
+	if (connector->base.status != connector_status_connected || !crtc)
+		return -ENODEV;
+
+	seq_printf(m, "Bigjoiner enable: %d\n", connector->force_bigjoiner_enable);
+
+	return 0;
+}
+
 static ssize_t i915_dsc_output_format_write(struct file *file,
 					    const char __user *ubuf,
 					    size_t len, loff_t *offp)
@@ -1434,6 +1448,30 @@ static ssize_t i915_dsc_output_format_write(struct file *file,
 	return len;
 }
 
+static ssize_t i915_bigjoiner_enable_write(struct file *file,
+					   const char __user *ubuf,
+					   size_t len, loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	struct intel_connector *connector = m->private;
+	struct drm_crtc *crtc;
+	bool bigjoiner_en = 0;
+	int ret;
+
+	crtc = connector->base.state->crtc;
+	if (connector->base.status != connector_status_connected || !crtc)
+		return -ENODEV;
+
+	ret = kstrtobool_from_user(ubuf, len, &bigjoiner_en);
+	if (ret < 0)
+		return ret;
+
+	connector->force_bigjoiner_enable = bigjoiner_en;
+	*offp += len;
+
+	return len;
+}
+
 static int i915_dsc_output_format_open(struct inode *inode,
 				       struct file *file)
 {
@@ -1527,6 +1565,8 @@ static const struct file_operations i915_dsc_fractional_bpp_fops = {
 	.write = i915_dsc_fractional_bpp_write
 };
 
+DEFINE_SHOW_STORE_ATTRIBUTE(i915_bigjoiner_enable);
+
 /*
  * Returns the Current CRTC's bpc.
  * Example usage: cat /sys/kernel/debug/dri/0/crtc-0/i915_current_bpc
@@ -1608,6 +1648,13 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
 				    connector, &i915_dsc_fractional_bpp_fops);
 	}
 
+	if (DISPLAY_VER(i915) >= 11 &&
+	    (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	     connector_type == DRM_MODE_CONNECTOR_eDP)) {
+		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
+				    connector, &i915_bigjoiner_enable_fops);
+	}
+
 	if (connector_type == DRM_MODE_CONNECTOR_DSI ||
 	    connector_type == DRM_MODE_CONNECTOR_eDP ||
 	    connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index ae2e8cff9d691..db38f2b8178a8 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -626,6 +626,8 @@ struct intel_connector {
 
 	struct intel_dp *mst_port;
 
+	bool force_bigjoiner_enable;
+
 	struct {
 		struct drm_dp_aux *dsc_decompression_aux;
 		u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE];
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index ab415f41924d7..d70890ca2b47d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1205,11 +1205,13 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
 			     int hdisplay, int clock)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+	struct intel_connector *connector = intel_dp->attached_connector;
 
 	if (!intel_dp_can_bigjoiner(intel_dp))
 		return false;
 
-	return clock > i915->max_dotclk_freq || hdisplay > 5120;
+	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
+	       connector->force_bigjoiner_enable;
 }
 
 static enum drm_mode_status
-- 
2.37.3