[Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

[Intel-gfx] [PATCH] 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.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 71 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |  2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  3 +-
 3 files changed, 75 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 f6d7c4d45fae..5855c6de91a7 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1399,6 +1399,35 @@ out:	drm_modeset_unlock(&dev->mode_config.connection_mutex);
 	return ret;
 }
 
+static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
+{
+	struct drm_connector *connector = m->private;
+	struct drm_device *dev = connector->dev;
+	struct drm_crtc *crtc;
+	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	int ret;
+
+	if (!encoder)
+		return -ENODEV;
+
+	ret = drm_modeset_lock_single_interruptible(&dev->mode_config.connection_mutex);
+	if (ret)
+		return ret;
+
+	crtc = connector->state->crtc;
+	if (connector->status != connector_status_connected || !crtc) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_en);
+
+out:	drm_modeset_unlock(&dev->mode_config.connection_mutex);
+
+	return ret;
+}
+
 static ssize_t i915_dsc_output_format_write(struct file *file,
 					    const char __user *ubuf,
 					    size_t len, loff_t *offp)
@@ -1420,12 +1449,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 drm_connector *connector =
+		((struct seq_file *)file->private_data)->private;
+	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	int bigjoiner_en = 0;
+	int ret;
+
+	ret = kstrtoint_from_user(ubuf, len, 0, &bigjoiner_en);
+	if (ret < 0)
+		return ret;
+
+	intel_dp->force_bigjoiner_en = 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,
@@ -1435,6 +1491,15 @@ static const struct file_operations i915_dsc_output_format_fops = {
 	.write = i915_dsc_output_format_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
@@ -1514,6 +1579,12 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
 				    connector, &i915_dsc_output_format_fops);
 	}
 
+	if (intel_dp_can_bigjoiner(enc_to_intel_dp(intel_connector->encoder)) &&
+	    connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
+		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
+				    connector, &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 8d8b2f8d37a9..ecec8a25838e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1753,6 +1753,8 @@ struct intel_dp {
 	bool is_mst;
 	int active_mst_links;
 
+	bool force_bigjoiner_en;
+
 	/* 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 f0f43aeabd21..6665214bd332 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1153,7 +1153,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_en;
 }
 
 static enum drm_mode_status
-- 
2.37.3

[Intel-gfx] [PATCH] 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.

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 71 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |  2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  6 +-
 3 files changed, 78 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 f6d7c4d45fae..c806957cb902 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1399,6 +1399,35 @@ out:	drm_modeset_unlock(&dev->mode_config.connection_mutex);
 	return ret;
 }
 
+static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
+{
+	struct drm_connector *connector = m->private;
+	struct drm_device *dev = connector->dev;
+	struct drm_crtc *crtc;
+	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	int ret;
+
+	if (!encoder)
+		return -ENODEV;
+
+	ret = drm_modeset_lock_single_interruptible(&dev->mode_config.connection_mutex);
+	if (ret)
+		return ret;
+
+	crtc = connector->state->crtc;
+	if (connector->status != connector_status_connected || !crtc) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_en);
+
+out:	drm_modeset_unlock(&dev->mode_config.connection_mutex);
+
+	return ret;
+}
+
 static ssize_t i915_dsc_output_format_write(struct file *file,
 					    const char __user *ubuf,
 					    size_t len, loff_t *offp)
@@ -1420,12 +1449,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 drm_connector *connector =
+		((struct seq_file *)file->private_data)->private;
+	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	int bigjoiner_en = 0;
+	int ret;
+
+	ret = kstrtoint_from_user(ubuf, len, 0, &bigjoiner_en);
+	if (ret < 0)
+		return ret;
+
+	intel_dp->force_bigjoiner_en = 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,
@@ -1435,6 +1491,15 @@ static const struct file_operations i915_dsc_output_format_fops = {
 	.write = i915_dsc_output_format_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
@@ -1514,6 +1579,12 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
 				    connector, &i915_dsc_output_format_fops);
 	}
 
+	if (DISPLAY_VER(dev_priv) >= 11 &&
+	    connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
+		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
+				    connector, &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 8d8b2f8d37a9..ecec8a25838e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1753,6 +1753,8 @@ struct intel_dp {
 	bool is_mst;
 	int active_mst_links;
 
+	bool force_bigjoiner_en;
+
 	/* 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 f0f43aeabd21..7e553cb7ecbb 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1153,7 +1153,11 @@ 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;
+	if (intel_dp->force_bigjoiner_en)
+		drm_dbg_kms(&i915->drm, "Forcing bigjoiner mode");
+
+	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
+	       intel_dp->force_bigjoiner_en;
 }
 
 static enum drm_mode_status
-- 
2.37.3

Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

[Jani Nikula]

On Mon, 09 Oct 2023, 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.
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 71 +++++++++++++++++++
>  .../drm/i915/display/intel_display_types.h    |  2 +
>  drivers/gpu/drm/i915/display/intel_dp.c       |  6 +-
>  3 files changed, 78 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 f6d7c4d45fae..c806957cb902 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1399,6 +1399,35 @@ out:	drm_modeset_unlock(&dev->mode_config.connection_mutex);
>  	return ret;
>  }
>  
> +static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
> +{
> +	struct drm_connector *connector = m->private;

struct intel_connector *connector, please. Yeah, this is copy-paste from
other files, but they should be changed too.

> +	struct drm_device *dev = connector->dev;

struct drm_i915_private *i915, please.

> +	struct drm_crtc *crtc;
> +	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +	int ret;
> +
> +	if (!encoder)
> +		return -ENODEV;
> +
> +	ret = drm_modeset_lock_single_interruptible(&dev->mode_config.connection_mutex);
> +	if (ret)
> +		return ret;

Why does show need locking but write doesn't?

> +
> +	crtc = connector->state->crtc;
> +	if (connector->status != connector_status_connected || !crtc) {
> +		ret = -ENODEV;
> +		goto out;
> +	}

I guess because of the above, but... do we need it? Or is this just
copy-pasted from some other debugfs files, which copy-pasted from other
debugfs files, which... ;)

> +
> +	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_en);
> +
> +out:	drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +
> +	return ret;
> +}
> +
>  static ssize_t i915_dsc_output_format_write(struct file *file,
>  					    const char __user *ubuf,
>  					    size_t len, loff_t *offp)
> @@ -1420,12 +1449,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 drm_connector *connector =
> +		((struct seq_file *)file->private_data)->private;

I think this reads better with

	struct seq_file *m = file->private_data;
        struct intel_connector *connector = m->private;

> +	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +	int bigjoiner_en = 0;
> +	int ret;
> +
> +	ret = kstrtoint_from_user(ubuf, len, 0, &bigjoiner_en);
> +	if (ret < 0)
> +		return ret;

If it's a bool, why kstrtoint? Yeah, because copy-paste, but why keep
percolating the same mistakes?

> +
> +	intel_dp->force_bigjoiner_en = 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,
> @@ -1435,6 +1491,15 @@ static const struct file_operations i915_dsc_output_format_fops = {
>  	.write = i915_dsc_output_format_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
> @@ -1514,6 +1579,12 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
>  				    connector, &i915_dsc_output_format_fops);
>  	}
>  
> +	if (DISPLAY_VER(dev_priv) >= 11 &&
> +	    connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
> +		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
> +				    connector, &i915_bigjoiner_enable_fops);

All of these should just pass struct intel_connector.

> +	}
> +
>  	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 8d8b2f8d37a9..ecec8a25838e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1753,6 +1753,8 @@ struct intel_dp {
>  	bool is_mst;
>  	int active_mst_links;
>  
> +	bool force_bigjoiner_en;

No need to try to shorten this, just spell out 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 f0f43aeabd21..7e553cb7ecbb 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1153,7 +1153,11 @@ 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;
> +	if (intel_dp->force_bigjoiner_en)
> +		drm_dbg_kms(&i915->drm, "Forcing bigjoiner mode");

Missing \n.

> +
> +	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
> +	       intel_dp->force_bigjoiner_en;
>  }
>  
>  static enum drm_mode_status

-- 
Jani Nikula, Intel

Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

[Lisovskiy, Stanislav]

On Wed, Oct 11, 2023 at 11:49:38AM +0300, Jani Nikula wrote:
> On Mon, 09 Oct 2023, 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.
> >
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_debugfs.c  | 71 +++++++++++++++++++
> >  .../drm/i915/display/intel_display_types.h    |  2 +
> >  drivers/gpu/drm/i915/display/intel_dp.c       |  6 +-
> >  3 files changed, 78 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 f6d7c4d45fae..c806957cb902 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -1399,6 +1399,35 @@ out:	drm_modeset_unlock(&dev->mode_config.connection_mutex);
> >  	return ret;
> >  }
> >  
> > +static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
> > +{
> > +	struct drm_connector *connector = m->private;
> 
> struct intel_connector *connector, please. Yeah, this is copy-paste from
> other files, but they should be changed too.
> 
> > +	struct drm_device *dev = connector->dev;
> 
> struct drm_i915_private *i915, please.
> 
> > +	struct drm_crtc *crtc;
> > +	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +	int ret;
> > +
> > +	if (!encoder)
> > +		return -ENODEV;
> > +
> > +	ret = drm_modeset_lock_single_interruptible(&dev->mode_config.connection_mutex);
> > +	if (ret)
> > +		return ret;
> 
> Why does show need locking but write doesn't?
> 
> > +
> > +	crtc = connector->state->crtc;
> > +	if (connector->status != connector_status_connected || !crtc) {
> > +		ret = -ENODEV;
> > +		goto out;
> > +	}
> 
> I guess because of the above, but... do we need it? Or is this just
> copy-pasted from some other debugfs files, which copy-pasted from other
> debugfs files, which... ;)

Yeah, I would be honest here, had same question both for bool/int and crtc locking
however decided just to copy same approach, from other debugfs entries, assuming
that was done with some purpose :)

> 
> > +
> > +	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_en);
> > +
> > +out:	drm_modeset_unlock(&dev->mode_config.connection_mutex);
> > +
> > +	return ret;
> > +}
> > +
> >  static ssize_t i915_dsc_output_format_write(struct file *file,
> >  					    const char __user *ubuf,
> >  					    size_t len, loff_t *offp)
> > @@ -1420,12 +1449,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 drm_connector *connector =
> > +		((struct seq_file *)file->private_data)->private;
> 
> I think this reads better with
> 
> 	struct seq_file *m = file->private_data;
>         struct intel_connector *connector = m->private;
> 
> > +	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +	int bigjoiner_en = 0;
> > +	int ret;
> > +
> > +	ret = kstrtoint_from_user(ubuf, len, 0, &bigjoiner_en);
> > +	if (ret < 0)
> > +		return ret;
> 
> If it's a bool, why kstrtoint? Yeah, because copy-paste, but why keep
> percolating the same mistakes?

Yeah, as I said above..

> 
> > +
> > +	intel_dp->force_bigjoiner_en = 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,
> > @@ -1435,6 +1491,15 @@ static const struct file_operations i915_dsc_output_format_fops = {
> >  	.write = i915_dsc_output_format_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
> > @@ -1514,6 +1579,12 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
> >  				    connector, &i915_dsc_output_format_fops);
> >  	}
> >  
> > +	if (DISPLAY_VER(dev_priv) >= 11 &&
> > +	    connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
> > +		debugfs_create_file("i915_bigjoiner_force_enable", 0644, root,
> > +				    connector, &i915_bigjoiner_enable_fops);
> 
> All of these should just pass struct intel_connector.

Do you mean intel_connector_debugfs_add? I think it does that already, will check.

> 
> > +	}
> > +
> >  	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 8d8b2f8d37a9..ecec8a25838e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1753,6 +1753,8 @@ struct intel_dp {
> >  	bool is_mst;
> >  	int active_mst_links;
> >  
> > +	bool force_bigjoiner_en;
> 
> No need to try to shorten this, just spell out force_bigjoiner_enable.

Again just tried to match same things done before, but for me personally
doesn't really matter at all.

> 
> > +
> >  	/* 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 f0f43aeabd21..7e553cb7ecbb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1153,7 +1153,11 @@ 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;
> > +	if (intel_dp->force_bigjoiner_en)
> > +		drm_dbg_kms(&i915->drm, "Forcing bigjoiner mode");
> 
> Missing \n.

Yes, will fix, thank you for comments!

Stan

> 
> > +
> > +	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
> > +	       intel_dp->force_bigjoiner_en;
> >  }
> >  
> >  static enum drm_mode_status
> 
> -- 
> Jani Nikula, Intel

[Intel-gfx] [PATCH] 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)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 72 ++++++++++++++++++-
 .../drm/i915/display/intel_display_types.h    |  2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  6 +-
 3 files changed, 76 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 fbe75d47a165..dea7bbea83ba 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1398,13 +1398,37 @@ 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 drm_crtc *crtc;
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	int ret;
+
+	if (!encoder)
+		return -ENODEV;
+
+	crtc = connector->base.state->crtc;
+	if (connector->base.status != connector_status_connected || !crtc) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
+
+out:
+
+	return ret;
+}
+
 static ssize_t i915_dsc_output_format_write(struct file *file,
 					    const char __user *ubuf,
 					    size_t len, loff_t *offp)
 {
-	struct drm_connector *connector =
-		((struct seq_file *)file->private_data)->private;
-	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
+	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);
 	int dsc_output_format = 0;
 	int ret;
@@ -1419,12 +1443,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 drm_connector *connector =
+		((struct seq_file *)file->private_data)->private;
+	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(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,
@@ -1434,6 +1485,15 @@ static const struct file_operations i915_dsc_output_format_fops = {
 	.write = i915_dsc_output_format_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
@@ -1513,6 +1573,12 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
 				    connector, &i915_dsc_output_format_fops);
 	}
 
+	if (DISPLAY_VER(dev_priv) >= 11 &&
+	    intel_connector->base.connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
+		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 8d8b2f8d37a9..e0de6eeaf59e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1753,6 +1753,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 0ef7cb8134b6..daf9bc0d6838 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1153,7 +1153,11 @@ 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;
+	if (intel_dp->force_bigjoiner_enable)
+		drm_dbg_kms(&i915->drm, "Forcing bigjoiner mode\n");
+
+	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
+	       intel_dp->force_bigjoiner_enable;
 }
 
 static enum drm_mode_status
-- 
2.37.3

Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

[Jani Nikula]

On Thu, 12 Oct 2023, 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)
>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 72 ++++++++++++++++++-
>  .../drm/i915/display/intel_display_types.h    |  2 +
>  drivers/gpu/drm/i915/display/intel_dp.c       |  6 +-
>  3 files changed, 76 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 fbe75d47a165..dea7bbea83ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1398,13 +1398,37 @@ 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 drm_crtc *crtc;
> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +	int ret;
> +
> +	if (!encoder)
> +		return -ENODEV;
> +
> +	crtc = connector->base.state->crtc;
> +	if (connector->base.status != connector_status_connected || !crtc) {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
> +
> +out:
> +
> +	return ret;
> +}
> +
>  static ssize_t i915_dsc_output_format_write(struct file *file,
>  					    const char __user *ubuf,
>  					    size_t len, loff_t *offp)
>  {
> -	struct drm_connector *connector =
> -		((struct seq_file *)file->private_data)->private;
> -	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
> +	struct seq_file *m = file->private_data;
> +        struct intel_connector *connector = m->private;
> +	struct intel_encoder *encoder = intel_attached_encoder(connector);

Ooops, I think you made those changes to the wrong function there. ;)

>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  	int dsc_output_format = 0;
>  	int ret;
> @@ -1419,12 +1443,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 drm_connector *connector =
> +		((struct seq_file *)file->private_data)->private;
> +	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));

Should be here.

> +	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,
> @@ -1434,6 +1485,15 @@ static const struct file_operations i915_dsc_output_format_fops = {
>  	.write = i915_dsc_output_format_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
> @@ -1513,6 +1573,12 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
>  				    connector, &i915_dsc_output_format_fops);
>  	}
>  
> +	if (DISPLAY_VER(dev_priv) >= 11 &&
> +	    intel_connector->base.connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
> +		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 8d8b2f8d37a9..e0de6eeaf59e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1753,6 +1753,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 0ef7cb8134b6..daf9bc0d6838 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1153,7 +1153,11 @@ 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;
> +	if (intel_dp->force_bigjoiner_enable)
> +		drm_dbg_kms(&i915->drm, "Forcing bigjoiner mode\n");
> +
> +	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
> +	       intel_dp->force_bigjoiner_enable;
>  }
>  
>  static enum drm_mode_status

-- 
Jani Nikula, Intel

Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

[Lisovskiy, Stanislav]

On Thu, Oct 12, 2023 at 05:59:18PM +0300, Jani Nikula wrote:
> On Thu, 12 Oct 2023, 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)
> >
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_debugfs.c  | 72 ++++++++++++++++++-
> >  .../drm/i915/display/intel_display_types.h    |  2 +
> >  drivers/gpu/drm/i915/display/intel_dp.c       |  6 +-
> >  3 files changed, 76 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 fbe75d47a165..dea7bbea83ba 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -1398,13 +1398,37 @@ 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 drm_crtc *crtc;
> > +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > +	int ret;
> > +
> > +	if (!encoder)
> > +		return -ENODEV;
> > +
> > +	crtc = connector->base.state->crtc;
> > +	if (connector->base.status != connector_status_connected || !crtc) {
> > +		ret = -ENODEV;
> > +		goto out;
> > +	}
> > +
> > +	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
> > +
> > +out:
> > +
> > +	return ret;
> > +}
> > +
> >  static ssize_t i915_dsc_output_format_write(struct file *file,
> >  					    const char __user *ubuf,
> >  					    size_t len, loff_t *offp)
> >  {
> > -	struct drm_connector *connector =
> > -		((struct seq_file *)file->private_data)->private;
> > -	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
> > +	struct seq_file *m = file->private_data;
> > +        struct intel_connector *connector = m->private;
> > +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> 
> Ooops, I think you made those changes to the wrong function there. ;)

Ouch, they looked exactly identical! I would say this could have been changed too,
but not in that patch indeed :)))
Thanks for spotting..probably really time to think about starting using glasses.

> 
> >  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >  	int dsc_output_format = 0;
> >  	int ret;
> > @@ -1419,12 +1443,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 drm_connector *connector =
> > +		((struct seq_file *)file->private_data)->private;
> > +	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
> 
> Should be here.
> 
> > +	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,
> > @@ -1434,6 +1485,15 @@ static const struct file_operations i915_dsc_output_format_fops = {
> >  	.write = i915_dsc_output_format_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
> > @@ -1513,6 +1573,12 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
> >  				    connector, &i915_dsc_output_format_fops);
> >  	}
> >  
> > +	if (DISPLAY_VER(dev_priv) >= 11 &&
> > +	    intel_connector->base.connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
> > +		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 8d8b2f8d37a9..e0de6eeaf59e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1753,6 +1753,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 0ef7cb8134b6..daf9bc0d6838 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1153,7 +1153,11 @@ 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;
> > +	if (intel_dp->force_bigjoiner_enable)
> > +		drm_dbg_kms(&i915->drm, "Forcing bigjoiner mode\n");
> > +
> > +	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
> > +	       intel_dp->force_bigjoiner_enable;
> >  }
> >  
> >  static enum drm_mode_status
> 
> -- 
> Jani Nikula, Intel

Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

[kernel test robot]

Hi Stanislav,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-tip/drm-tip]

url:    https://github.com/intel-lab-lkp/linux/commits/Stanislav-Lisovskiy/drm-i915-Add-bigjoiner-force-enable-option-to-debugfs/20231017-105841
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link:    https://lore.kernel.org/r/20231012123411.17241-1-stanislav.lisovskiy%40intel.com
patch subject: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231017/202310171756.yzfTKqav-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231017/202310171756.yzfTKqav-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310171756.yzfTKqav-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/i915/display/intel_display_debugfs.c:1413:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           if (connector->base.status != connector_status_connected || !crtc) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_display_debugfs.c:1422:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   drivers/gpu/drm/i915/display/intel_display_debugfs.c:1413:2: note: remove the 'if' if its condition is always true
           if (connector->base.status != connector_status_connected || !crtc) {
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/display/intel_display_debugfs.c:1407:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +1413 drivers/gpu/drm/i915/display/intel_display_debugfs.c

  1400	
  1401	static int i915_bigjoiner_enable_show(struct seq_file *m, void *data)
  1402	{
  1403		struct intel_connector *connector = to_intel_connector(m->private);
  1404		struct drm_crtc *crtc;
  1405		struct intel_encoder *encoder = intel_attached_encoder(connector);
  1406		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
  1407		int ret;
  1408	
  1409		if (!encoder)
  1410			return -ENODEV;
  1411	
  1412		crtc = connector->base.state->crtc;
> 1413		if (connector->base.status != connector_status_connected || !crtc) {
  1414			ret = -ENODEV;
  1415			goto out;
  1416		}
  1417	
  1418		seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
  1419	
  1420	out:
  1421	
  1422		return ret;
  1423	}
  1424	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[Intel-gfx] [PATCH] 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)

Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 66 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |  2 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  6 +-
 3 files changed, 73 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 fbe75d47a165..9b810c6f96ea 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -1398,6 +1398,30 @@ 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 drm_crtc *crtc;
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	int ret = 0;
+
+	if (!encoder)
+		return -ENODEV;
+
+	crtc = connector->base.state->crtc;
+	if (connector->base.status != connector_status_connected || !crtc) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
+
+out:
+
+	return ret;
+}
+
 static ssize_t i915_dsc_output_format_write(struct file *file,
 					    const char __user *ubuf,
 					    size_t len, loff_t *offp)
@@ -1419,12 +1443,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,
@@ -1434,6 +1485,15 @@ static const struct file_operations i915_dsc_output_format_fops = {
 	.write = i915_dsc_output_format_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
@@ -1513,6 +1573,12 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
 				    connector, &i915_dsc_output_format_fops);
 	}
 
+	if (DISPLAY_VER(dev_priv) >= 11 &&
+	    intel_connector->base.connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
+		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 8d8b2f8d37a9..e0de6eeaf59e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1753,6 +1753,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 4f6835a7578e..6a9148232a9c 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1153,7 +1153,11 @@ 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;
+	if (intel_dp->force_bigjoiner_enable)
+		drm_dbg_kms(&i915->drm, "Forcing bigjoiner mode\n");
+
+	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
+	       intel_dp->force_bigjoiner_enable;
 }
 
 static enum drm_mode_status
-- 
2.37.3

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Add bigjoiner force enable option to debugfs (rev4)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 9045 bytes --]

== Series Details ==

Series: drm/i915: Add bigjoiner force enable option to debugfs (rev4)
URL   : https://patchwork.freedesktop.org/series/124730/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13773 -> Patchwork_124730v4
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_124730v4 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_124730v4, please notify your bug team (lgci.bug.filing@intel.com) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/index.html

Participating hosts (32 -> 33)
------------------------------

  Additional (2): bat-adln-1 bat-adlm-1 
  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_124730v4:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - bat-jsl-3:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13773/bat-jsl-3/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-jsl-3/igt@i915_module_load@load.html

  
Known issues
------------

  Here are the changes found in Patchwork_124730v4 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-adln-1:         NOTRUN -> [SKIP][3] ([i915#9318])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adln-1/igt@debugfs_test@basic-hwmon.html
    - bat-adlm-1:         NOTRUN -> [SKIP][4] ([i915#3826])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@debugfs_test@basic-hwmon.html

  * igt@fbdev@eof:
    - bat-adlm-1:         NOTRUN -> [SKIP][5] ([i915#2582]) +3 other tests skip
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@fbdev@eof.html

  * igt@fbdev@info:
    - bat-adlm-1:         NOTRUN -> [SKIP][6] ([i915#1849] / [i915#2582])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@fbdev@info.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - bat-adlm-1:         NOTRUN -> [SKIP][7] ([i915#4613]) +3 other tests skip
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@gem_tiled_pread_basic:
    - bat-adln-1:         NOTRUN -> [SKIP][8] ([i915#3282])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adln-1/igt@gem_tiled_pread_basic.html
    - bat-adlm-1:         NOTRUN -> [SKIP][9] ([i915#3282])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rps@basic-api:
    - bat-adlm-1:         NOTRUN -> [SKIP][10] ([i915#6621])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@i915_pm_rps@basic-api.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - bat-adln-1:         NOTRUN -> [SKIP][11] ([i915#4213]) +1 other test skip
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adln-1/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size:
    - bat-adlm-1:         NOTRUN -> [SKIP][12] ([i915#1845]) +17 other tests skip
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@kms_cursor_legacy@basic-flip-after-cursor-varying-size.html

  * igt@kms_dsc@dsc-basic:
    - bat-adln-1:         NOTRUN -> [SKIP][13] ([i915#3555] / [i915#3840])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adln-1/igt@kms_dsc@dsc-basic.html

  * igt@kms_flip@basic-plain-flip:
    - bat-adlm-1:         NOTRUN -> [SKIP][14] ([i915#3637]) +3 other tests skip
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@kms_flip@basic-plain-flip.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-adln-1:         NOTRUN -> [SKIP][15] ([fdo#109285])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adln-1/igt@kms_force_connector_basic@force-load-detect.html
    - bat-adlm-1:         NOTRUN -> [SKIP][16] ([fdo#109285])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_frontbuffer_tracking@basic:
    - bat-adlm-1:         NOTRUN -> [SKIP][17] ([i915#1849])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-c-dp-5:
    - bat-adlp-11:        [PASS][18] -> [ABORT][19] ([i915#8668] / [i915#9451])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13773/bat-adlp-11/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-c-dp-5.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlp-11/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-c-dp-5.html

  * igt@kms_psr@cursor_plane_move:
    - bat-adlm-1:         NOTRUN -> [SKIP][20] ([i915#1072]) +3 other tests skip
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@kms_psr@cursor_plane_move.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-adlm-1:         NOTRUN -> [SKIP][21] ([i915#3555])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-adlm-1:         NOTRUN -> [SKIP][22] ([i915#1845] / [i915#3708])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-write:
    - bat-adlm-1:         NOTRUN -> [SKIP][23] ([i915#3708]) +2 other tests skip
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlm-1/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-dp-5:
    - bat-adlp-11:        [ABORT][24] ([i915#8668]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13773/bat-adlp-11/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-dp-5.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-adlp-11/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-dp-5.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
    - bat-rplp-1:         [ABORT][26] ([i915#8668]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13773/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3826]: https://gitlab.freedesktop.org/drm/intel/issues/3826
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
  [i915#9318]: https://gitlab.freedesktop.org/drm/intel/issues/9318
  [i915#9451]: https://gitlab.freedesktop.org/drm/intel/issues/9451


Build changes
-------------

  * Linux: CI_DRM_13773 -> Patchwork_124730v4

  CI-20190529: 20190529
  CI_DRM_13773: 730155121b4ad393d5bc4b5ea06aac28116d5097 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7545: 4aac61139d076775a173a75a15156e408a366546 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_124730v4: 730155121b4ad393d5bc4b5ea06aac28116d5097 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

0e03bdaa536a drm/i915: Add bigjoiner force enable option to debugfs

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124730v4/index.html

[-- Attachment #2: Type: text/html, Size: 10778 bytes --]

Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

[Ville Syrjälä]

On Wed, Oct 18, 2023 at 04:24:00PM +0300, 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)
> 
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 66 +++++++++++++++++++
>  .../drm/i915/display/intel_display_types.h    |  2 +
>  drivers/gpu/drm/i915/display/intel_dp.c       |  6 +-
>  3 files changed, 73 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 fbe75d47a165..9b810c6f96ea 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1398,6 +1398,30 @@ 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 drm_crtc *crtc;
> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +	int ret = 0;
> +
> +	if (!encoder)
> +		return -ENODEV;
> +
> +	crtc = connector->base.state->crtc;
> +	if (connector->base.status != connector_status_connected || !crtc) {
> +		ret = -ENODEV;

Why do we need this extra complication?

> +		goto out;
> +	}
> +
> +	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
> +
> +out:
> +
> +	return ret;
> +}
> +
>  static ssize_t i915_dsc_output_format_write(struct file *file,
>  					    const char __user *ubuf,
>  					    size_t len, loff_t *offp)
> @@ -1419,12 +1443,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,
> @@ -1434,6 +1485,15 @@ static const struct file_operations i915_dsc_output_format_fops = {
>  	.write = i915_dsc_output_format_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
> @@ -1513,6 +1573,12 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
>  				    connector, &i915_dsc_output_format_fops);
>  	}
>  
> +	if (DISPLAY_VER(dev_priv) >= 11 &&
> +	    intel_connector->base.connector_type == DRM_MODE_CONNECTOR_DisplayPort) {

eDP missing

> +		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 8d8b2f8d37a9..e0de6eeaf59e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1753,6 +1753,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 4f6835a7578e..6a9148232a9c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1153,7 +1153,11 @@ 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;
> +	if (intel_dp->force_bigjoiner_enable)
> +		drm_dbg_kms(&i915->drm, "Forcing bigjoiner mode\n");

That's going to cause excessive dmesg spam.

> +
> +	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

[Intel-gfx] [PATCH] 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

Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

[Modem, Bhanuprakash]

On Thu-16-11-2023 04:07 pm, 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.

Do we have any IGT to validate this?

- Bhanu

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

Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

[Sharma, Swati2]

Hi Ville,

On 25-Oct-23 6:29 PM, Ville Syrjälä wrote:
> On Wed, Oct 18, 2023 at 04:24:00PM +0300, 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)
>>
>> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>> ---
>>   .../drm/i915/display/intel_display_debugfs.c  | 66 +++++++++++++++++++
>>   .../drm/i915/display/intel_display_types.h    |  2 +
>>   drivers/gpu/drm/i915/display/intel_dp.c       |  6 +-
>>   3 files changed, 73 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 fbe75d47a165..9b810c6f96ea 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> @@ -1398,6 +1398,30 @@ 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 drm_crtc *crtc;
>> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
>> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>> +	int ret = 0;
>> +
>> +	if (!encoder)
>> +		return -ENODEV;
>> +
>> +	crtc = connector->base.state->crtc;
>> +	if (connector->base.status != connector_status_connected || !crtc) {
>> +		ret = -ENODEV;
> 
> Why do we need this extra complication?
> 
>> +		goto out;
>> +	}
>> +
>> +	seq_printf(m, "Bigjoiner enable: %d\n", intel_dp->force_bigjoiner_enable);
>> +
>> +out:
>> +
>> +	return ret;
>> +}
>> +
>>   static ssize_t i915_dsc_output_format_write(struct file *file,
>>   					    const char __user *ubuf,
>>   					    size_t len, loff_t *offp)
>> @@ -1419,12 +1443,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,
>> @@ -1434,6 +1485,15 @@ static const struct file_operations i915_dsc_output_format_fops = {
>>   	.write = i915_dsc_output_format_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
>> @@ -1513,6 +1573,12 @@ void intel_connector_debugfs_add(struct intel_connector *intel_connector)
>>   				    connector, &i915_dsc_output_format_fops);
>>   	}
>>   
>> +	if (DISPLAY_VER(dev_priv) >= 11 &&
>> +	    intel_connector->base.connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
> 
> eDP missing
> 
>> +		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 8d8b2f8d37a9..e0de6eeaf59e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1753,6 +1753,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 4f6835a7578e..6a9148232a9c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -1153,7 +1153,11 @@ 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;
>> +	if (intel_dp->force_bigjoiner_enable)
>> +		drm_dbg_kms(&i915->drm, "Forcing bigjoiner mode\n");
> 
> That's going to cause excessive dmesg spam.

Then how from dmesg we will get to know, big joiner was forced? 
Shouldn't we have atleast one debug print to know this?

> 
>> +
>> +	return clock > i915->max_dotclk_freq || hdisplay > 5120 ||
>> +	       intel_dp->force_bigjoiner_enable;
>>   }
>>   
>>   static enum drm_mode_status
>> -- 
>> 2.37.3
> 

Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

[Jani Nikula]

On Wed, 17 Jan 2024, "Sharma, Swati2" <swati2.sharma@intel.com> wrote:
> Hi Ville,
>
> On 25-Oct-23 6:29 PM, Ville Syrjälä wrote:
>> On Wed, Oct 18, 2023 at 04:24:00PM +0300, Stanislav Lisovskiy wrote:
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>>> index 4f6835a7578e..6a9148232a9c 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> @@ -1153,7 +1153,11 @@ 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;
>>> +	if (intel_dp->force_bigjoiner_enable)
>>> +		drm_dbg_kms(&i915->drm, "Forcing bigjoiner mode\n");
>> 
>> That's going to cause excessive dmesg spam.
>
> Then how from dmesg we will get to know, big joiner was forced? 
> Shouldn't we have atleast one debug print to know this?

Yeah. But this gets called for every single display mode via
intel_dp_mode_valid().

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs

[Sharma, Swati2]

On 17-Jan-24 11:31 PM, Jani Nikula wrote:
> On Wed, 17 Jan 2024, "Sharma, Swati2" <swati2.sharma@intel.com> wrote:
>> Hi Ville,
>>
>> On 25-Oct-23 6:29 PM, Ville Syrjälä wrote:
>>> On Wed, Oct 18, 2023 at 04:24:00PM +0300, Stanislav Lisovskiy wrote:
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>>>> index 4f6835a7578e..6a9148232a9c 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>>> @@ -1153,7 +1153,11 @@ 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;
>>>> +	if (intel_dp->force_bigjoiner_enable)
>>>> +		drm_dbg_kms(&i915->drm, "Forcing bigjoiner mode\n");
>>>
>>> That's going to cause excessive dmesg spam.
>>
>> Then how from dmesg we will get to know, big joiner was forced?
>> Shouldn't we have atleast one debug print to know this?
> 
> Yeah. But this gets called for every single display mode via
> intel_dp_mode_valid().

ohh okay, can we add dbg print in intel_dp_compute_link_config()
from where intel_dp_need_bigjoiner() is called?

> 
> BR,
> Jani.
> 
>