* [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
@ 2023-10-18 13:24 Stanislav Lisovskiy
2023-10-18 23:17 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Add bigjoiner force enable option to debugfs (rev4) Patchwork
2023-10-25 12:59 ` [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs Ville Syrjälä
0 siblings, 2 replies; 16+ messages in thread
From: Stanislav Lisovskiy @ 2023-10-18 13:24 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Add bigjoiner force enable option to debugfs (rev4)
2023-10-18 13:24 [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs Stanislav Lisovskiy
@ 2023-10-18 23:17 ` Patchwork
2023-10-25 12:59 ` [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs Ville Syrjälä
1 sibling, 0 replies; 16+ messages in thread
From: Patchwork @ 2023-10-18 23:17 UTC (permalink / raw)
To: Stanislav Lisovskiy; +Cc: intel-gfx
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
2023-10-18 13:24 [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs Stanislav Lisovskiy
2023-10-18 23:17 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Add bigjoiner force enable option to debugfs (rev4) Patchwork
@ 2023-10-25 12:59 ` Ville Syrjälä
2024-01-17 16:46 ` Sharma, Swati2
1 sibling, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2023-10-25 12:59 UTC (permalink / raw)
To: Stanislav Lisovskiy; +Cc: jani.nikula, intel-gfx
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
2023-10-25 12:59 ` [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs Ville Syrjälä
@ 2024-01-17 16:46 ` Sharma, Swati2
2024-01-17 18:01 ` Jani Nikula
0 siblings, 1 reply; 16+ messages in thread
From: Sharma, Swati2 @ 2024-01-17 16:46 UTC (permalink / raw)
To: Ville Syrjälä, Stanislav Lisovskiy; +Cc: jani.nikula, intel-gfx
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
2024-01-17 16:46 ` Sharma, Swati2
@ 2024-01-17 18:01 ` Jani Nikula
2024-01-18 6:01 ` Sharma, Swati2
0 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2024-01-17 18:01 UTC (permalink / raw)
To: Sharma, Swati2, Ville Syrjälä, Stanislav Lisovskiy; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
2024-01-17 18:01 ` Jani Nikula
@ 2024-01-18 6:01 ` Sharma, Swati2
0 siblings, 0 replies; 16+ messages in thread
From: Sharma, Swati2 @ 2024-01-18 6:01 UTC (permalink / raw)
To: Jani Nikula, Ville Syrjälä, Stanislav Lisovskiy; +Cc: intel-gfx
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.
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
@ 2023-11-16 10:37 Stanislav Lisovskiy
2023-11-16 13:48 ` Modem, Bhanuprakash
0 siblings, 1 reply; 16+ messages in thread
From: Stanislav Lisovskiy @ 2023-11-16 10:37 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
2023-11-16 10:37 Stanislav Lisovskiy
@ 2023-11-16 13:48 ` Modem, Bhanuprakash
0 siblings, 0 replies; 16+ messages in thread
From: Modem, Bhanuprakash @ 2023-11-16 13:48 UTC (permalink / raw)
To: Stanislav Lisovskiy, intel-gfx
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
@ 2023-10-12 12:34 Stanislav Lisovskiy
2023-10-12 14:59 ` Jani Nikula
2023-10-17 9:49 ` kernel test robot
0 siblings, 2 replies; 16+ messages in thread
From: Stanislav Lisovskiy @ 2023-10-12 12:34 UTC (permalink / raw)
To: intel-gfx; +Cc: jani.nikula
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
2023-10-12 12:34 Stanislav Lisovskiy
@ 2023-10-12 14:59 ` Jani Nikula
2023-10-13 9:54 ` Lisovskiy, Stanislav
2023-10-17 9:49 ` kernel test robot
1 sibling, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2023-10-12 14:59 UTC (permalink / raw)
To: Stanislav Lisovskiy, intel-gfx
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
2023-10-12 14:59 ` Jani Nikula
@ 2023-10-13 9:54 ` Lisovskiy, Stanislav
0 siblings, 0 replies; 16+ messages in thread
From: Lisovskiy, Stanislav @ 2023-10-13 9:54 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
2023-10-12 12:34 Stanislav Lisovskiy
2023-10-12 14:59 ` Jani Nikula
@ 2023-10-17 9:49 ` kernel test robot
1 sibling, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-10-17 9:49 UTC (permalink / raw)
To: Stanislav Lisovskiy, intel-gfx; +Cc: jani.nikula, llvm, oe-kbuild-all
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
@ 2023-10-09 13:30 Stanislav Lisovskiy
2023-10-11 8:49 ` Jani Nikula
0 siblings, 1 reply; 16+ messages in thread
From: Stanislav Lisovskiy @ 2023-10-09 13:30 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
2023-10-09 13:30 Stanislav Lisovskiy
@ 2023-10-11 8:49 ` Jani Nikula
2023-10-11 8:59 ` Lisovskiy, Stanislav
0 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2023-10-11 8:49 UTC (permalink / raw)
To: Stanislav Lisovskiy, intel-gfx
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
2023-10-11 8:49 ` Jani Nikula
@ 2023-10-11 8:59 ` Lisovskiy, Stanislav
0 siblings, 0 replies; 16+ messages in thread
From: Lisovskiy, Stanislav @ 2023-10-11 8:59 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs
@ 2023-10-06 14:15 Stanislav Lisovskiy
0 siblings, 0 replies; 16+ messages in thread
From: Stanislav Lisovskiy @ 2023-10-06 14:15 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-01-18 6:01 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-18 13:24 [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs Stanislav Lisovskiy
2023-10-18 23:17 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Add bigjoiner force enable option to debugfs (rev4) Patchwork
2023-10-25 12:59 ` [Intel-gfx] [PATCH] drm/i915: Add bigjoiner force enable option to debugfs Ville Syrjälä
2024-01-17 16:46 ` Sharma, Swati2
2024-01-17 18:01 ` Jani Nikula
2024-01-18 6:01 ` Sharma, Swati2
-- strict thread matches above, loose matches on Subject: below --
2023-11-16 10:37 Stanislav Lisovskiy
2023-11-16 13:48 ` Modem, Bhanuprakash
2023-10-12 12:34 Stanislav Lisovskiy
2023-10-12 14:59 ` Jani Nikula
2023-10-13 9:54 ` Lisovskiy, Stanislav
2023-10-17 9:49 ` kernel test robot
2023-10-09 13:30 Stanislav Lisovskiy
2023-10-11 8:49 ` Jani Nikula
2023-10-11 8:59 ` Lisovskiy, Stanislav
2023-10-06 14:15 Stanislav Lisovskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox