All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
@ 2018-12-04  3:07 Manasi Navare
  2018-12-04  3:20 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Manasi Navare @ 2018-12-04  3:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

DSC can be supported per DP connector. This patch adds a per connector
debugfs node to expose DSC support capability by the kernel.
The same node can be used from userspace to force DSC enable.

force_dsc_en written through this debugfs node is used to force
DSC even for lower resolutions.

v5:
* Name it dsc sink support and also add
fec support in the same node (Ville)
v4:
* Add missed connector_status check (Manasi)
* Create i915_dsc_support node only for Gen >=10 (manasi)
* Access intel_dp->dsc_dpcd only if its not NULL (Manasi)
v3:
* Combine Force_dsc_en with this patch (Ville)
v2:
* Use kstrtobool_from_user to avoid explicit error checking (Lyude)
* Rebase on drm-tip (Manasi)

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 80 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c     |  3 +-
 drivers/gpu/drm/i915/intel_drv.h    |  3 ++
 3 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 129b9a6f8309..f2766004778d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -5086,6 +5086,79 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
 
+static int i915_dsc_fec_support_show(struct seq_file *m, void *data)
+{
+	struct drm_connector *connector = m->private;
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp =
+		enc_to_intel_dp(&encoder->base);
+	struct intel_crtc *crtc;
+	struct intel_crtc_state *crtc_state;
+
+	if (connector->status != connector_status_connected)
+		return -ENODEV;
+
+	crtc = to_intel_crtc(encoder->base.crtc);
+	crtc_state = to_intel_crtc_state(crtc->base.state);
+	drm_modeset_lock(&crtc->base.mutex, NULL);
+	seq_printf(m, "DSC_Enabled: %s\n",
+		   yesno(crtc_state->dsc_params.compression_enable));
+	if (intel_dp->dsc_dpcd)
+		seq_printf(m, "DSC_Sink_Support: %s\n",
+			   yesno(drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)));
+	if (intel_dp->fec_capable)
+		seq_printf(m, "FEC_Sink_Support: %s\n",
+			   yesno(drm_dp_sink_supports_fec(intel_dp->fec_capable)));
+	drm_modeset_unlock(&crtc->base.mutex);
+
+	return 0;
+}
+
+static ssize_t i915_dsc_fec_support_write(struct file *file,
+					  const char __user *ubuf,
+					  size_t len, loff_t *offp)
+{
+	bool dsc_enable = false;
+	int ret;
+	struct drm_connector *connector =
+		((struct seq_file *)file->private_data)->private;
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	if (len == 0)
+		return 0;
+
+	DRM_DEBUG_DRIVER("Copied %d bytes from user to force DSC\n",
+			 (unsigned int)len);
+
+	ret = kstrtobool_from_user(ubuf, len, &dsc_enable);
+	if (ret < 0)
+		return ret;
+
+	DRM_DEBUG_DRIVER("Got %s for DSC Enable\n",
+			 (dsc_enable) ? "true" : "false");
+	intel_dp->force_dsc_en = dsc_enable;
+
+	*offp += len;
+	return len;
+}
+
+static int i915_dsc_fec_support_open(struct inode *inode,
+				     struct file *file)
+{
+	return single_open(file, i915_dsc_fec_support_show,
+			   inode->i_private);
+}
+
+static const struct file_operations i915_dsc_fec_support_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_dsc_fec_support_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+	.write = i915_dsc_fec_support_write
+};
+
 /**
  * i915_debugfs_connector_add - add i915 specific connector debugfs files
  * @connector: pointer to a registered drm_connector
@@ -5098,6 +5171,7 @@ DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
 int i915_debugfs_connector_add(struct drm_connector *connector)
 {
 	struct dentry *root = connector->debugfs_entry;
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 
 	/* The connector must have been registered beforehands. */
 	if (!root)
@@ -5122,5 +5196,11 @@ int i915_debugfs_connector_add(struct drm_connector *connector)
 				    connector, &i915_hdcp_sink_capability_fops);
 	}
 
+	if (INTEL_GEN(dev_priv) >= 10 &&
+	    (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	     connector->connector_type == DRM_MODE_CONNECTOR_eDP))
+		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
+				    connector, &i915_dsc_fec_support_fops);
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a6907a1761ab..2e3097cae277 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2051,7 +2051,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 							&limits);
 
 	/* enable compression if the mode doesn't fit available BW */
-	if (!ret) {
+	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
+	if (!ret || intel_dp->force_dsc_en) {
 		if (!intel_dp_dsc_compute_config(intel_dp, pipe_config,
 						 conn_state, &limits))
 			return false;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f94a04b4ad87..0cedce438433 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1209,6 +1209,9 @@ struct intel_dp {
 
 	/* Displayport compliance testing */
 	struct intel_dp_compliance compliance;
+
+	/* Display stream compression testing */
+	bool force_dsc_en;
 };
 
 enum lspcon_vendor {
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-04  3:07 [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable Manasi Navare
@ 2018-12-04  3:20 ` Patchwork
  2018-12-04  3:45 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-12-04  3:20 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
URL   : https://patchwork.freedesktop.org/series/53449/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
3e6218c6f05e drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
-:132: WARNING:SYMBOLIC_PERMS: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
#132: FILE: drivers/gpu/drm/i915/i915_debugfs.c:5202:
+		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,

-:161: CHECK:BOOL_MEMBER: Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384
#161: FILE: drivers/gpu/drm/i915/intel_drv.h:1214:
+	bool force_dsc_en;

total: 0 errors, 1 warnings, 1 checks, 115 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-04  3:07 [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable Manasi Navare
  2018-12-04  3:20 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-12-04  3:45 ` Patchwork
  2018-12-04 19:55 ` [PATCH v6] " Manasi Navare
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-12-04  3:45 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
URL   : https://patchwork.freedesktop.org/series/53449/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5250 -> Patchwork_11006
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53449/revisions/1/mbox/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@debugfs_test@read_all_entries:
    - fi-icl-u2:          PASS -> INCOMPLETE

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-gvtdvm:      PASS -> INCOMPLETE [fdo#105600]

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@basic-small-copy-xy:
    - fi-glk-dsi:         INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

  
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#105600]: https://bugs.freedesktop.org/show_bug.cgi?id=105600
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (47 -> 42)
------------------------------

  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


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

    * Linux: CI_DRM_5250 -> Patchwork_11006

  CI_DRM_5250: 1e4e49c57969d1b53dea913c92e1d23ec23aee31 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11006: 3e6218c6f05e58875aa658bcff61f8f7ed4e9f6f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3e6218c6f05e drm/i915/dsc: Add Per connector debugfs node for DSC support/enable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11006/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-04  3:07 [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable Manasi Navare
  2018-12-04  3:20 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
  2018-12-04  3:45 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2018-12-04 19:55 ` Manasi Navare
  2018-12-05 19:00   ` Lyude Paul
  2018-12-05 22:57   ` [PATCH v7] " Manasi Navare
  2018-12-04 20:18 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev2) Patchwork
                   ` (6 subsequent siblings)
  9 siblings, 2 replies; 20+ messages in thread
From: Manasi Navare @ 2018-12-04 19:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

DSC can be supported per DP connector. This patch adds a per connector
debugfs node to expose DSC support capability by the kernel.
The same node can be used from userspace to force DSC enable.

force_dsc_en written through this debugfs node is used to force
DSC even for lower resolutions.

v6:
* Read fec_capable only for non edp (Manasi)
v5:
* Name it dsc sink support and also add
fec support in the same node (Ville)
v4:
* Add missed connector_status check (Manasi)
* Create i915_dsc_support node only for Gen >=10 (manasi)
* Access intel_dp->dsc_dpcd only if its not NULL (Manasi)
v3:
* Combine Force_dsc_en with this patch (Ville)
v2:
* Use kstrtobool_from_user to avoid explicit error checking (Lyude)
* Rebase on drm-tip (Manasi)

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 80 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c     |  3 +-
 drivers/gpu/drm/i915/intel_drv.h    |  3 ++
 3 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 129b9a6f8309..ec10ab027d18 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -5086,6 +5086,79 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
 
+static int i915_dsc_fec_support_show(struct seq_file *m, void *data)
+{
+	struct drm_connector *connector = m->private;
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp =
+		enc_to_intel_dp(&encoder->base);
+	struct intel_crtc *crtc;
+	struct intel_crtc_state *crtc_state;
+
+	if (connector->status != connector_status_connected)
+		return -ENODEV;
+
+	crtc = to_intel_crtc(encoder->base.crtc);
+	crtc_state = to_intel_crtc_state(crtc->base.state);
+	drm_modeset_lock(&crtc->base.mutex, NULL);
+	seq_printf(m, "DSC_Enabled: %s\n",
+		   yesno(crtc_state->dsc_params.compression_enable));
+	if (intel_dp->dsc_dpcd)
+		seq_printf(m, "DSC_Sink_Support: %s\n",
+			   yesno(drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)));
+	if (!intel_dp_is_edp(intel_dp))
+		seq_printf(m, "FEC_Sink_Support: %s\n",
+			   yesno(drm_dp_sink_supports_fec(intel_dp->fec_capable)));
+	drm_modeset_unlock(&crtc->base.mutex);
+
+	return 0;
+}
+
+static ssize_t i915_dsc_fec_support_write(struct file *file,
+					  const char __user *ubuf,
+					  size_t len, loff_t *offp)
+{
+	bool dsc_enable = false;
+	int ret;
+	struct drm_connector *connector =
+		((struct seq_file *)file->private_data)->private;
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	if (len == 0)
+		return 0;
+
+	DRM_DEBUG_DRIVER("Copied %d bytes from user to force DSC\n",
+			 (unsigned int)len);
+
+	ret = kstrtobool_from_user(ubuf, len, &dsc_enable);
+	if (ret < 0)
+		return ret;
+
+	DRM_DEBUG_DRIVER("Got %s for DSC Enable\n",
+			 (dsc_enable) ? "true" : "false");
+	intel_dp->force_dsc_en = dsc_enable;
+
+	*offp += len;
+	return len;
+}
+
+static int i915_dsc_fec_support_open(struct inode *inode,
+				     struct file *file)
+{
+	return single_open(file, i915_dsc_fec_support_show,
+			   inode->i_private);
+}
+
+static const struct file_operations i915_dsc_fec_support_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_dsc_fec_support_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+	.write = i915_dsc_fec_support_write
+};
+
 /**
  * i915_debugfs_connector_add - add i915 specific connector debugfs files
  * @connector: pointer to a registered drm_connector
@@ -5098,6 +5171,7 @@ DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
 int i915_debugfs_connector_add(struct drm_connector *connector)
 {
 	struct dentry *root = connector->debugfs_entry;
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 
 	/* The connector must have been registered beforehands. */
 	if (!root)
@@ -5122,5 +5196,11 @@ int i915_debugfs_connector_add(struct drm_connector *connector)
 				    connector, &i915_hdcp_sink_capability_fops);
 	}
 
+	if (INTEL_GEN(dev_priv) >= 10 &&
+	    (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	     connector->connector_type == DRM_MODE_CONNECTOR_eDP))
+		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
+				    connector, &i915_dsc_fec_support_fops);
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a6907a1761ab..2e3097cae277 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2051,7 +2051,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 							&limits);
 
 	/* enable compression if the mode doesn't fit available BW */
-	if (!ret) {
+	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
+	if (!ret || intel_dp->force_dsc_en) {
 		if (!intel_dp_dsc_compute_config(intel_dp, pipe_config,
 						 conn_state, &limits))
 			return false;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f94a04b4ad87..0cedce438433 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1209,6 +1209,9 @@ struct intel_dp {
 
 	/* Displayport compliance testing */
 	struct intel_dp_compliance compliance;
+
+	/* Display stream compression testing */
+	bool force_dsc_en;
 };
 
 enum lspcon_vendor {
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev2)
  2018-12-04  3:07 [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable Manasi Navare
                   ` (2 preceding siblings ...)
  2018-12-04 19:55 ` [PATCH v6] " Manasi Navare
@ 2018-12-04 20:18 ` Patchwork
  2018-12-04 20:44 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-12-04 20:18 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev2)
URL   : https://patchwork.freedesktop.org/series/53449/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
60eb08c39119 drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
-:134: WARNING:SYMBOLIC_PERMS: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
#134: FILE: drivers/gpu/drm/i915/i915_debugfs.c:5204:
+		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,

-:163: CHECK:BOOL_MEMBER: Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384
#163: FILE: drivers/gpu/drm/i915/intel_drv.h:1214:
+	bool force_dsc_en;

total: 0 errors, 1 warnings, 1 checks, 115 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev2)
  2018-12-04  3:07 [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable Manasi Navare
                   ` (3 preceding siblings ...)
  2018-12-04 20:18 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev2) Patchwork
@ 2018-12-04 20:44 ` Patchwork
  2018-12-05 23:25 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev3) Patchwork
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-12-04 20:44 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev2)
URL   : https://patchwork.freedesktop.org/series/53449/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5262 -> Patchwork_11013
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53449/revisions/2/mbox/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@debugfs_test@read_all_entries:
    - fi-icl-u2:          PASS -> INCOMPLETE

  * igt@kms_chamelium@hdmi-edid-read:
    - {fi-kbl-7567u}:     PASS -> FAIL +2

  
#### Warnings ####

  * igt@kms_busy@basic-flip-a:
    - {fi-kbl-7567u}:     PASS -> SKIP +6

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - {fi-kbl-7567u}:     PASS -> DMESG-WARN [fdo#103558] / [fdo#105079] / [fdo#105602] +1

  * igt@i915_module_load@reload-with-fault-injection:
    - {fi-kbl-7567u}:     PASS -> DMESG-WARN [fdo#105602] / [fdo#108529] +1

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - {fi-kbl-7567u}:     PASS -> DMESG-FAIL [fdo#105079]

  * igt@pm_rpm@module-reload:
    - {fi-kbl-7567u}:     PASS -> DMESG-WARN [fdo#108529]

  
#### Possible fixes ####

  * igt@gem_close_race@basic-threads:
    - fi-bsw-n3050:       FAIL [fdo#108656] -> PASS

  * igt@kms_chamelium@common-hpd-after-suspend:
    - {fi-kbl-7567u}:     DMESG-WARN [fdo#108473] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - {fi-icl-u3}:        DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108315]

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

  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
  [fdo#108473]: https://bugs.freedesktop.org/show_bug.cgi?id=108473
  [fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656


Participating hosts (50 -> 44)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y 


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

    * Linux: CI_DRM_5262 -> Patchwork_11013

  CI_DRM_5262: e870a77e9cf3f1c8a05cb60ed78919bd88ce6bd6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4740: dd8de0efa64e50bc06c2882a0028d98ad870e752 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11013: 60eb08c391192384ec99fefe9a837f04f7bcd10e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

60eb08c39119 drm/i915/dsc: Add Per connector debugfs node for DSC support/enable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11013/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-04 19:55 ` [PATCH v6] " Manasi Navare
@ 2018-12-05 19:00   ` Lyude Paul
  2018-12-05 19:29     ` Manasi Navare
  2018-12-05 22:57   ` [PATCH v7] " Manasi Navare
  1 sibling, 1 reply; 20+ messages in thread
From: Lyude Paul @ 2018-12-05 19:00 UTC (permalink / raw)
  To: Manasi Navare, intel-gfx; +Cc: Rodrigo Vivi

Looks good, some small nitpicks

On Tue, 2018-12-04 at 11:55 -0800, Manasi Navare wrote:
> DSC can be supported per DP connector. This patch adds a per connector
> debugfs node to expose DSC support capability by the kernel.
> The same node can be used from userspace to force DSC enable.
> 
> force_dsc_en written through this debugfs node is used to force
> DSC even for lower resolutions.
> 
> v6:
> * Read fec_capable only for non edp (Manasi)
> v5:
> * Name it dsc sink support and also add
> fec support in the same node (Ville)
> v4:
> * Add missed connector_status check (Manasi)
> * Create i915_dsc_support node only for Gen >=10 (manasi)
> * Access intel_dp->dsc_dpcd only if its not NULL (Manasi)
> v3:
> * Combine Force_dsc_en with this patch (Ville)
> v2:
> * Use kstrtobool_from_user to avoid explicit error checking (Lyude)
> * Rebase on drm-tip (Manasi)
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 80 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c     |  3 +-
>  drivers/gpu/drm/i915/intel_drv.h    |  3 ++
>  3 files changed, 85 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 129b9a6f8309..ec10ab027d18 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -5086,6 +5086,79 @@ static int i915_hdcp_sink_capability_show(struct
> seq_file *m, void *data)
>  }
>  DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
>  
> +static int i915_dsc_fec_support_show(struct seq_file *m, void *data)
> +{
> +	struct drm_connector *connector = m->private;
> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> +	struct intel_dp *intel_dp =
> +		enc_to_intel_dp(&encoder->base);
> +	struct intel_crtc *crtc;
> +	struct intel_crtc_state *crtc_state;
> +
> +	if (connector->status != connector_status_connected)
> +		return -ENODEV;
> +
> +	crtc = to_intel_crtc(encoder->base.crtc);
> +	crtc_state = to_intel_crtc_state(crtc->base.state);
> +	drm_modeset_lock(&crtc->base.mutex, NULL);
> +	seq_printf(m, "DSC_Enabled: %s\n",
> +		   yesno(crtc_state->dsc_params.compression_enable));
> +	if (intel_dp->dsc_dpcd)
> +		seq_printf(m, "DSC_Sink_Support: %s\n",
> +			   yesno(drm_dp_sink_supports_dsc(intel_dp-
> >dsc_dpcd)));
> +	if (!intel_dp_is_edp(intel_dp))
> +		seq_printf(m, "FEC_Sink_Support: %s\n",
> +			   yesno(drm_dp_sink_supports_fec(intel_dp-
> >fec_capable)));
> +	drm_modeset_unlock(&crtc->base.mutex);
> +
> +	return 0;
> +}
> +
> +static ssize_t i915_dsc_fec_support_write(struct file *file,
> +					  const char __user *ubuf,
> +					  size_t len, loff_t *offp)
> +{
> +	bool dsc_enable = false;
> +	int ret;
> +	struct drm_connector *connector =
> +		((struct seq_file *)file->private_data)->private;
> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
> +	if (len == 0)
> +		return 0;
> +
> +	DRM_DEBUG_DRIVER("Copied %d bytes from user to force DSC\n",
> +			 (unsigned int)len);
You can just use %zu instead of %d here, see:

https://01.org/linuxgraphics/gfx-docs/drm/core-api/printk-formats.html

Other then that, looks good to me! With that change:

Reviewed-by: Lyude Paul <lyude@redhat.com>

> +
> +	ret = kstrtobool_from_user(ubuf, len, &dsc_enable);
> +	if (ret < 0)
> +		return ret;
> +
> +	DRM_DEBUG_DRIVER("Got %s for DSC Enable\n",
> +			 (dsc_enable) ? "true" : "false");
> +	intel_dp->force_dsc_en = dsc_enable;
> +
> +	*offp += len;
> +	return len;
> +}
> +
> +static int i915_dsc_fec_support_open(struct inode *inode,
> +				     struct file *file)
> +{
> +	return single_open(file, i915_dsc_fec_support_show,
> +			   inode->i_private);
> +}
> +
> +static const struct file_operations i915_dsc_fec_support_fops = {
> +	.owner = THIS_MODULE,
> +	.open = i915_dsc_fec_support_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +	.write = i915_dsc_fec_support_write
> +};
> +
>  /**
>   * i915_debugfs_connector_add - add i915 specific connector debugfs files
>   * @connector: pointer to a registered drm_connector
> @@ -5098,6 +5171,7 @@ DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
>  int i915_debugfs_connector_add(struct drm_connector *connector)
>  {
>  	struct dentry *root = connector->debugfs_entry;
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>  
>  	/* The connector must have been registered beforehands. */
>  	if (!root)
> @@ -5122,5 +5196,11 @@ int i915_debugfs_connector_add(struct drm_connector
> *connector)
>  				    connector,
> &i915_hdcp_sink_capability_fops);
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 10 &&
> +	    (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> +	     connector->connector_type == DRM_MODE_CONNECTOR_eDP))
> +		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
> +				    connector, &i915_dsc_fec_support_fops);
> +
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index a6907a1761ab..2e3097cae277 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2051,7 +2051,8 @@ intel_dp_compute_link_config(struct intel_encoder
> *encoder,
>  							&limits);
>  
>  	/* enable compression if the mode doesn't fit available BW */
> -	if (!ret) {
> +	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> +	if (!ret || intel_dp->force_dsc_en) {
>  		if (!intel_dp_dsc_compute_config(intel_dp, pipe_config,
>  						 conn_state, &limits))
>  			return false;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index f94a04b4ad87..0cedce438433 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1209,6 +1209,9 @@ struct intel_dp {
>  
>  	/* Displayport compliance testing */
>  	struct intel_dp_compliance compliance;
> +
> +	/* Display stream compression testing */
> +	bool force_dsc_en;
>  };
>  
>  enum lspcon_vendor {
-- 
Cheers,
	Lyude Paul

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-05 19:00   ` Lyude Paul
@ 2018-12-05 19:29     ` Manasi Navare
  2018-12-05 21:24       ` Lyude Paul
  0 siblings, 1 reply; 20+ messages in thread
From: Manasi Navare @ 2018-12-05 19:29 UTC (permalink / raw)
  To: Lyude Paul; +Cc: intel-gfx, Rodrigo Vivi

On Wed, Dec 05, 2018 at 02:00:39PM -0500, Lyude Paul wrote:
> Looks good, some small nitpicks
> 
> On Tue, 2018-12-04 at 11:55 -0800, Manasi Navare wrote:
> > DSC can be supported per DP connector. This patch adds a per connector
> > debugfs node to expose DSC support capability by the kernel.
> > The same node can be used from userspace to force DSC enable.
> > 
> > force_dsc_en written through this debugfs node is used to force
> > DSC even for lower resolutions.
> > 
> > v6:
> > * Read fec_capable only for non edp (Manasi)
> > v5:
> > * Name it dsc sink support and also add
> > fec support in the same node (Ville)
> > v4:
> > * Add missed connector_status check (Manasi)
> > * Create i915_dsc_support node only for Gen >=10 (manasi)
> > * Access intel_dp->dsc_dpcd only if its not NULL (Manasi)
> > v3:
> > * Combine Force_dsc_en with this patch (Ville)
> > v2:
> > * Use kstrtobool_from_user to avoid explicit error checking (Lyude)
> > * Rebase on drm-tip (Manasi)
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Cc: Lyude Paul <lyude@redhat.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 80 +++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dp.c     |  3 +-
> >  drivers/gpu/drm/i915/intel_drv.h    |  3 ++
> >  3 files changed, 85 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 129b9a6f8309..ec10ab027d18 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -5086,6 +5086,79 @@ static int i915_hdcp_sink_capability_show(struct
> > seq_file *m, void *data)
> >  }
> >  DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
> >  
> > +static int i915_dsc_fec_support_show(struct seq_file *m, void *data)
> > +{
> > +	struct drm_connector *connector = m->private;
> > +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> > +	struct intel_dp *intel_dp =
> > +		enc_to_intel_dp(&encoder->base);
> > +	struct intel_crtc *crtc;
> > +	struct intel_crtc_state *crtc_state;
> > +
> > +	if (connector->status != connector_status_connected)
> > +		return -ENODEV;
> > +
> > +	crtc = to_intel_crtc(encoder->base.crtc);
> > +	crtc_state = to_intel_crtc_state(crtc->base.state);

I am seeing a kernel NULl pointer dereferencing here in CI possibly because
the crtc is NULL and as per Ville and Danvet I should be grabbing both connection mutex
and crtc mutex and use the acquire ctx and backoff stuff so I am trying to understand that part
and seeing how I can use that.

Let me know if you have any inputs/examples on that too..

> > +	drm_modeset_lock(&crtc->base.mutex, NULL);
> > +	seq_printf(m, "DSC_Enabled: %s\n",
> > +		   yesno(crtc_state->dsc_params.compression_enable));
> > +	if (intel_dp->dsc_dpcd)
> > +		seq_printf(m, "DSC_Sink_Support: %s\n",
> > +			   yesno(drm_dp_sink_supports_dsc(intel_dp-
> > >dsc_dpcd)));
> > +	if (!intel_dp_is_edp(intel_dp))
> > +		seq_printf(m, "FEC_Sink_Support: %s\n",
> > +			   yesno(drm_dp_sink_supports_fec(intel_dp-
> > >fec_capable)));
> > +	drm_modeset_unlock(&crtc->base.mutex);
> > +
> > +	return 0;
> > +}
> > +
> > +static ssize_t i915_dsc_fec_support_write(struct file *file,
> > +					  const char __user *ubuf,
> > +					  size_t len, loff_t *offp)
> > +{
> > +	bool dsc_enable = false;
> > +	int ret;
> > +	struct drm_connector *connector =
> > +		((struct seq_file *)file->private_data)->private;
> > +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > +
> > +	if (len == 0)
> > +		return 0;
> > +
> > +	DRM_DEBUG_DRIVER("Copied %d bytes from user to force DSC\n",
> > +			 (unsigned int)len);
> You can just use %zu instead of %d here, see:
> 
> https://01.org/linuxgraphics/gfx-docs/drm/core-api/printk-formats.html

Cool I will make this change. Thanks for the review.

Regards
Manasi

> 
> Other then that, looks good to me! With that change:
> 
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> 
> > +
> > +	ret = kstrtobool_from_user(ubuf, len, &dsc_enable);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	DRM_DEBUG_DRIVER("Got %s for DSC Enable\n",
> > +			 (dsc_enable) ? "true" : "false");
> > +	intel_dp->force_dsc_en = dsc_enable;
> > +
> > +	*offp += len;
> > +	return len;
> > +}
> > +
> > +static int i915_dsc_fec_support_open(struct inode *inode,
> > +				     struct file *file)
> > +{
> > +	return single_open(file, i915_dsc_fec_support_show,
> > +			   inode->i_private);
> > +}
> > +
> > +static const struct file_operations i915_dsc_fec_support_fops = {
> > +	.owner = THIS_MODULE,
> > +	.open = i915_dsc_fec_support_open,
> > +	.read = seq_read,
> > +	.llseek = seq_lseek,
> > +	.release = single_release,
> > +	.write = i915_dsc_fec_support_write
> > +};
> > +
> >  /**
> >   * i915_debugfs_connector_add - add i915 specific connector debugfs files
> >   * @connector: pointer to a registered drm_connector
> > @@ -5098,6 +5171,7 @@ DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
> >  int i915_debugfs_connector_add(struct drm_connector *connector)
> >  {
> >  	struct dentry *root = connector->debugfs_entry;
> > +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> >  
> >  	/* The connector must have been registered beforehands. */
> >  	if (!root)
> > @@ -5122,5 +5196,11 @@ int i915_debugfs_connector_add(struct drm_connector
> > *connector)
> >  				    connector,
> > &i915_hdcp_sink_capability_fops);
> >  	}
> >  
> > +	if (INTEL_GEN(dev_priv) >= 10 &&
> > +	    (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > +	     connector->connector_type == DRM_MODE_CONNECTOR_eDP))
> > +		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
> > +				    connector, &i915_dsc_fec_support_fops);
> > +
> >  	return 0;
> >  }
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index a6907a1761ab..2e3097cae277 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2051,7 +2051,8 @@ intel_dp_compute_link_config(struct intel_encoder
> > *encoder,
> >  							&limits);
> >  
> >  	/* enable compression if the mode doesn't fit available BW */
> > -	if (!ret) {
> > +	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> > +	if (!ret || intel_dp->force_dsc_en) {
> >  		if (!intel_dp_dsc_compute_config(intel_dp, pipe_config,
> >  						 conn_state, &limits))
> >  			return false;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index f94a04b4ad87..0cedce438433 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1209,6 +1209,9 @@ struct intel_dp {
> >  
> >  	/* Displayport compliance testing */
> >  	struct intel_dp_compliance compliance;
> > +
> > +	/* Display stream compression testing */
> > +	bool force_dsc_en;
> >  };
> >  
> >  enum lspcon_vendor {
> -- 
> Cheers,
> 	Lyude Paul
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v6] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-05 19:29     ` Manasi Navare
@ 2018-12-05 21:24       ` Lyude Paul
  0 siblings, 0 replies; 20+ messages in thread
From: Lyude Paul @ 2018-12-05 21:24 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx, Rodrigo Vivi

On Wed, 2018-12-05 at 11:29 -0800, Manasi Navare wrote:
> On Wed, Dec 05, 2018 at 02:00:39PM -0500, Lyude Paul wrote:
> > Looks good, some small nitpicks
> > 
> > On Tue, 2018-12-04 at 11:55 -0800, Manasi Navare wrote:
> > > DSC can be supported per DP connector. This patch adds a per connector
> > > debugfs node to expose DSC support capability by the kernel.
> > > The same node can be used from userspace to force DSC enable.
> > > 
> > > force_dsc_en written through this debugfs node is used to force
> > > DSC even for lower resolutions.
> > > 
> > > v6:
> > > * Read fec_capable only for non edp (Manasi)
> > > v5:
> > > * Name it dsc sink support and also add
> > > fec support in the same node (Ville)
> > > v4:
> > > * Add missed connector_status check (Manasi)
> > > * Create i915_dsc_support node only for Gen >=10 (manasi)
> > > * Access intel_dp->dsc_dpcd only if its not NULL (Manasi)
> > > v3:
> > > * Combine Force_dsc_en with this patch (Ville)
> > > v2:
> > > * Use kstrtobool_from_user to avoid explicit error checking (Lyude)
> > > * Rebase on drm-tip (Manasi)
> > > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > Cc: Lyude Paul <lyude@redhat.com>
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c | 80 +++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/i915/intel_dp.c     |  3 +-
> > >  drivers/gpu/drm/i915/intel_drv.h    |  3 ++
> > >  3 files changed, 85 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 129b9a6f8309..ec10ab027d18 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -5086,6 +5086,79 @@ static int i915_hdcp_sink_capability_show(struct
> > > seq_file *m, void *data)
> > >  }
> > >  DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
> > >  
> > > +static int i915_dsc_fec_support_show(struct seq_file *m, void *data)
> > > +{
> > > +	struct drm_connector *connector = m->private;
> > > +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> > > +	struct intel_dp *intel_dp =
> > > +		enc_to_intel_dp(&encoder->base);
> > > +	struct intel_crtc *crtc;
> > > +	struct intel_crtc_state *crtc_state;
> > > +
> > > +	if (connector->status != connector_status_connected)
> > > +		return -ENODEV;
> > > +
> > > +	crtc = to_intel_crtc(encoder->base.crtc);
> > > +	crtc_state = to_intel_crtc_state(crtc->base.state);
> 
> I am seeing a kernel NULl pointer dereferencing here in CI possibly because
> the crtc is NULL and as per Ville and Danvet I should be grabbing both
> connection mutex
> and crtc mutex and use the acquire ctx and backoff stuff so I am trying to
> understand that part
> and seeing how I can use that.
> 
> Let me know if you have any inputs/examples on that too..

Mhm, so here's why they are asking for each lock:

connection_mutex: Protects any connector->state, and connector->status.

(As an extra note: ignore connector->mutex for modesetting purposes, only
 dev->mode_config.connection_mutex is important.)

crtc->mutex: Anything in crtc->state for the given crtc

In order to avoid to avoid a NULL dereference due to connector->crtc->state
changing, you'd start grabbing locks with something like this:


  /* pretend we have 'connector' already set to the drm_connector */
  /* also pretend we have 'dev' set to the struct drm_device* */
  struct drm_crtc *crtc;
  struct drm_modeset_acquire_ctx ctx;
  bool try_again = false;
  int ret = 0;

  drm_modeset_acquire_init(&ctx, 0);

  do {
    /* We don't check the return code here because since this is
     * our first lock, so there isn't any chance of us deadlocking
     * yet.
     */
    drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);

    /* Cool! Now that we're holding connection_mutex, we can start
     * reading from the connector struct and friends
     */
    crtc = connector->state->crtc;
    if (connector->status != connector_status_connected || !crtc) {
      ret = -ENODEV;
      break;
    }

    /* Now we need to check the return code from
     * drm_modeset_lock(), because.......
     */
    ret = drm_modeset_lock(&crtc->mutex, &ctx);
    if (ret == -EDEADLK) {
      /* We could deadlock! If we're here, then this is
       * probably what happened:
       *
       * <CPU1> (us)                 <CPU2>
       * modeset_lock(&conn_mutex);  modeset_lock(&crtc->mutex);
       * modeset_lock(&crtc->mutex); modeset_lock(&conn_mutex);
       *
       * CPU1 is waiting for crtc->mutex which is held by CPU2, but
       * CPU2 is waiting for conn_mutex, which is held by CPU1
       *
       * Normally the kernel doesn't run into this issue
       * because a lot of care has been put into ordering
       * mutex_lock() calls correctly. But, with modesetting
       * we don't have the benefit of knowing what order these
       * locks will be acquired in. Since drm_modeset_lock()
       * has informed us of this scenario however, we can
       * avoid hanging the entire kernel by simply dropping
       * our locks, in this case that would just include
       * connection_mutex. We call this "backing off"
       */
      drm_modeset_backoff(&ctx);

      /* Now, things will look like this:
       *
       * <CPU1> (us)                    <CPU2>
       * modeset_lock(&conn_mutex);     modeset_lock(&crtc->mutex);
       * modeset_lock(&crtc->mutex);    modeset_lock(&conn_mutex);
       * (oh no, a deadlock!)           ...
       * modeset_backoff();             (Oh hey, I locked &conn_mutex)
       * modeset_lock(&conn_mutex);     (now I'm doing stuff, wee!)
       * ...                            modeset_drop_locks();
       * (Oh hey, I locked &conn_mutex) ...
       * (do stuff)                     ...
       * (etc. etc.)
       */
      go_again = true;
      continue;
    }

    /* Now if we're here, we managed to grab both locks correctly,
     * and we know neither connector->state or crtc->state will
     * change.
     */
    intel_dp = enc_to_intel_dp(intel_attached_encoder(connector));
    if (intel_dp->dsc_dpcd)
      /* etc. etc. */

    /* ... */

  } while (go_again);

  /* Now, we can finally drop any locks we're holding */
  drm_modeset_drop_locks(&ctx);
  /* And release our acquisition context */
  drm_modeset_acquire_fini(&ctx);

  return ret;


Does that help explain things?

> 
> > > +	drm_modeset_lock(&crtc->base.mutex, NULL);
> > > +	seq_printf(m, "DSC_Enabled: %s\n",
> > > +		   yesno(crtc_state->dsc_params.compression_enable));
> > > +	if (intel_dp->dsc_dpcd)
> > > +		seq_printf(m, "DSC_Sink_Support: %s\n",
> > > +			   yesno(drm_dp_sink_supports_dsc(intel_dp-
> > > > dsc_dpcd)));
> > > +	if (!intel_dp_is_edp(intel_dp))
> > > +		seq_printf(m, "FEC_Sink_Support: %s\n",
> > > +			   yesno(drm_dp_sink_supports_fec(intel_dp-
> > > > fec_capable)));
> > > +	drm_modeset_unlock(&crtc->base.mutex);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static ssize_t i915_dsc_fec_support_write(struct file *file,
> > > +					  const char __user *ubuf,
> > > +					  size_t len, loff_t *offp)
> > > +{
> > > +	bool dsc_enable = false;
> > > +	int ret;
> > > +	struct drm_connector *connector =
> > > +		((struct seq_file *)file->private_data)->private;
> > > +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> > > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > > +
> > > +	if (len == 0)
> > > +		return 0;
> > > +
> > > +	DRM_DEBUG_DRIVER("Copied %d bytes from user to force DSC\n",
> > > +			 (unsigned int)len);
> > You can just use %zu instead of %d here, see:
> > 
> > https://01.org/linuxgraphics/gfx-docs/drm/core-api/printk-formats.html
> 
> Cool I will make this change. Thanks for the review.
> 
> Regards
> Manasi
> 
> > Other then that, looks good to me! With that change:
> > 
> > Reviewed-by: Lyude Paul <lyude@redhat.com>
> > 
> > > +
> > > +	ret = kstrtobool_from_user(ubuf, len, &dsc_enable);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	DRM_DEBUG_DRIVER("Got %s for DSC Enable\n",
> > > +			 (dsc_enable) ? "true" : "false");
> > > +	intel_dp->force_dsc_en = dsc_enable;
> > > +
> > > +	*offp += len;
> > > +	return len;
> > > +}
> > > +
> > > +static int i915_dsc_fec_support_open(struct inode *inode,
> > > +				     struct file *file)
> > > +{
> > > +	return single_open(file, i915_dsc_fec_support_show,
> > > +			   inode->i_private);
> > > +}
> > > +
> > > +static const struct file_operations i915_dsc_fec_support_fops = {
> > > +	.owner = THIS_MODULE,
> > > +	.open = i915_dsc_fec_support_open,
> > > +	.read = seq_read,
> > > +	.llseek = seq_lseek,
> > > +	.release = single_release,
> > > +	.write = i915_dsc_fec_support_write
> > > +};
> > > +
> > >  /**
> > >   * i915_debugfs_connector_add - add i915 specific connector debugfs
> > > files
> > >   * @connector: pointer to a registered drm_connector
> > > @@ -5098,6 +5171,7 @@ DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
> > >  int i915_debugfs_connector_add(struct drm_connector *connector)
> > >  {
> > >  	struct dentry *root = connector->debugfs_entry;
> > > +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > >  
> > >  	/* The connector must have been registered beforehands. */
> > >  	if (!root)
> > > @@ -5122,5 +5196,11 @@ int i915_debugfs_connector_add(struct
> > > drm_connector
> > > *connector)
> > >  				    connector,
> > > &i915_hdcp_sink_capability_fops);
> > >  	}
> > >  
> > > +	if (INTEL_GEN(dev_priv) >= 10 &&
> > > +	    (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > > +	     connector->connector_type == DRM_MODE_CONNECTOR_eDP))
> > > +		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
> > > +				    connector, &i915_dsc_fec_support_fops);
> > > +
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index a6907a1761ab..2e3097cae277 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -2051,7 +2051,8 @@ intel_dp_compute_link_config(struct intel_encoder
> > > *encoder,
> > >  							&limits);
> > >  
> > >  	/* enable compression if the mode doesn't fit available BW */
> > > -	if (!ret) {
> > > +	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> > > +	if (!ret || intel_dp->force_dsc_en) {
> > >  		if (!intel_dp_dsc_compute_config(intel_dp, pipe_config,
> > >  						 conn_state, &limits))
> > >  			return false;
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index f94a04b4ad87..0cedce438433 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1209,6 +1209,9 @@ struct intel_dp {
> > >  
> > >  	/* Displayport compliance testing */
> > >  	struct intel_dp_compliance compliance;
> > > +
> > > +	/* Display stream compression testing */
> > > +	bool force_dsc_en;
> > >  };
> > >  
> > >  enum lspcon_vendor {
> > -- 
> > Cheers,
> > 	Lyude Paul
> > 
-- 
Cheers,
	Lyude Paul

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v7] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-04 19:55 ` [PATCH v6] " Manasi Navare
  2018-12-05 19:00   ` Lyude Paul
@ 2018-12-05 22:57   ` Manasi Navare
  2018-12-05 23:03     ` Lyude Paul
  2018-12-06  0:54     ` [PATCH v8] " Manasi Navare
  1 sibling, 2 replies; 20+ messages in thread
From: Manasi Navare @ 2018-12-05 22:57 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

DSC can be supported per DP connector. This patch adds a per connector
debugfs node to expose DSC support capability by the kernel.
The same node can be used from userspace to force DSC enable.

force_dsc_en written through this debugfs node is used to force
DSC even for lower resolutions.

Credits to Ville Syrjala for suggesting the proper locks to be used
and to Lyude Paul for explaining how to use them in this context

v7:
* Get crtc, crtc_state from connector atomic state
and add proper locks and backoff (Ville, Chris Wilson, Lyude)
(Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>)
* Use %zu for printing size_t variable (Lyude)
v6:
* Read fec_capable only for non edp (Manasi)
v5:
* Name it dsc sink support and also add
fec support in the same node (Ville)
v4:
* Add missed connector_status check (Manasi)
* Create i915_dsc_support node only for Gen >=10 (manasi)
* Access intel_dp->dsc_dpcd only if its not NULL (Manasi)
v3:
* Combine Force_dsc_en with this patch (Ville)
v2:
* Use kstrtobool_from_user to avoid explicit error checking (Lyude)
* Rebase on drm-tip (Manasi)

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 107 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c     |   3 +-
 drivers/gpu/drm/i915/intel_drv.h    |   3 +
 3 files changed, 112 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 129b9a6f8309..58f6636fffea 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -5086,6 +5086,106 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
 
+static int i915_dsc_fec_support_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_dp *intel_dp;
+	struct drm_modeset_acquire_ctx ctx;
+	struct intel_crtc_state *crtc_state = NULL;
+	int ret = 0;
+	bool try_again = false;
+
+	drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
+
+	do {
+		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
+				       &ctx);
+		if (ret) {
+			ret = -EINTR;
+			break;
+		}
+		crtc = connector->state->crtc;
+		if (connector->status != connector_status_connected || !crtc) {
+			ret = -ENODEV;
+			break;
+		}
+		ret = drm_modeset_lock(&crtc->mutex, &ctx);
+		if (ret == -EDEADLK) {
+			ret = drm_modeset_backoff(&ctx);
+			if (!ret) {
+				try_again = true;
+				continue;
+			}
+			break;
+		}
+		if (ret)
+			break;
+
+		intel_dp = enc_to_intel_dp(&intel_attached_encoder(connector)->base);
+		crtc_state = to_intel_crtc_state(crtc->state);
+		seq_printf(m, "DSC_Enabled: %s\n",
+			   yesno(crtc_state->dsc_params.compression_enable));
+		if (intel_dp->dsc_dpcd)
+			seq_printf(m, "DSC_Sink_Support: %s\n",
+				   yesno(drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)));
+		if (!intel_dp_is_edp(intel_dp))
+			seq_printf(m, "FEC_Sink_Support: %s\n",
+				   yesno(drm_dp_sink_supports_fec(intel_dp->fec_capable)));
+	} while (try_again);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	return ret;
+}
+
+static ssize_t i915_dsc_fec_support_write(struct file *file,
+					  const char __user *ubuf,
+					  size_t len, loff_t *offp)
+{
+	bool dsc_enable = false;
+	int ret;
+	struct drm_connector *connector =
+		((struct seq_file *)file->private_data)->private;
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	if (len == 0)
+		return 0;
+
+	DRM_DEBUG_DRIVER("Copied %zu bytes from user to force DSC\n",
+			 len);
+
+	ret = kstrtobool_from_user(ubuf, len, &dsc_enable);
+	if (ret < 0)
+		return ret;
+
+	DRM_DEBUG_DRIVER("Got %s for DSC Enable\n",
+			 (dsc_enable) ? "true" : "false");
+	intel_dp->force_dsc_en = dsc_enable;
+
+	*offp += len;
+	return len;
+}
+
+static int i915_dsc_fec_support_open(struct inode *inode,
+				     struct file *file)
+{
+	return single_open(file, i915_dsc_fec_support_show,
+			   inode->i_private);
+}
+
+static const struct file_operations i915_dsc_fec_support_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_dsc_fec_support_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+	.write = i915_dsc_fec_support_write
+};
+
 /**
  * i915_debugfs_connector_add - add i915 specific connector debugfs files
  * @connector: pointer to a registered drm_connector
@@ -5098,6 +5198,7 @@ DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
 int i915_debugfs_connector_add(struct drm_connector *connector)
 {
 	struct dentry *root = connector->debugfs_entry;
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 
 	/* The connector must have been registered beforehands. */
 	if (!root)
@@ -5122,5 +5223,11 @@ int i915_debugfs_connector_add(struct drm_connector *connector)
 				    connector, &i915_hdcp_sink_capability_fops);
 	}
 
+	if (INTEL_GEN(dev_priv) >= 10 &&
+	    (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	     connector->connector_type == DRM_MODE_CONNECTOR_eDP))
+		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
+				    connector, &i915_dsc_fec_support_fops);
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a6907a1761ab..2e3097cae277 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2051,7 +2051,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 							&limits);
 
 	/* enable compression if the mode doesn't fit available BW */
-	if (!ret) {
+	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
+	if (!ret || intel_dp->force_dsc_en) {
 		if (!intel_dp_dsc_compute_config(intel_dp, pipe_config,
 						 conn_state, &limits))
 			return false;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f94a04b4ad87..0cedce438433 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1209,6 +1209,9 @@ struct intel_dp {
 
 	/* Displayport compliance testing */
 	struct intel_dp_compliance compliance;
+
+	/* Display stream compression testing */
+	bool force_dsc_en;
 };
 
 enum lspcon_vendor {
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v7] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-05 22:57   ` [PATCH v7] " Manasi Navare
@ 2018-12-05 23:03     ` Lyude Paul
  2018-12-05 23:23       ` Manasi Navare
  2018-12-06  0:54     ` [PATCH v8] " Manasi Navare
  1 sibling, 1 reply; 20+ messages in thread
From: Lyude Paul @ 2018-12-05 23:03 UTC (permalink / raw)
  To: Manasi Navare, intel-gfx; +Cc: Rodrigo Vivi

On Wed, 2018-12-05 at 14:57 -0800, Manasi Navare wrote:
> DSC can be supported per DP connector. This patch adds a per connector
> debugfs node to expose DSC support capability by the kernel.
> The same node can be used from userspace to force DSC enable.
> 
> force_dsc_en written through this debugfs node is used to force
> DSC even for lower resolutions.
> 
> Credits to Ville Syrjala for suggesting the proper locks to be used
> and to Lyude Paul for explaining how to use them in this context
> 
> v7:
> * Get crtc, crtc_state from connector atomic state
> and add proper locks and backoff (Ville, Chris Wilson, Lyude)
> (Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>)
> * Use %zu for printing size_t variable (Lyude)
> v6:
> * Read fec_capable only for non edp (Manasi)
> v5:
> * Name it dsc sink support and also add
> fec support in the same node (Ville)
> v4:
> * Add missed connector_status check (Manasi)
> * Create i915_dsc_support node only for Gen >=10 (manasi)
> * Access intel_dp->dsc_dpcd only if its not NULL (Manasi)
> v3:
> * Combine Force_dsc_en with this patch (Ville)
> v2:
> * Use kstrtobool_from_user to avoid explicit error checking (Lyude)
> * Rebase on drm-tip (Manasi)
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 107 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c     |   3 +-
>  drivers/gpu/drm/i915/intel_drv.h    |   3 +
>  3 files changed, 112 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 129b9a6f8309..58f6636fffea 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -5086,6 +5086,106 @@ static int i915_hdcp_sink_capability_show(struct
> seq_file *m, void *data)
>  }
>  DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
>  
> +static int i915_dsc_fec_support_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_dp *intel_dp;
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct intel_crtc_state *crtc_state = NULL;
> +	int ret = 0;
> +	bool try_again = false;
> +
> +	drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> +
> +	do {
> +		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
> +				       &ctx);
> +		if (ret) {
> +			ret = -EINTR;
> +			break;
> +		}
> +		crtc = connector->state->crtc;
> +		if (connector->status != connector_status_connected || !crtc)
> {
> +			ret = -ENODEV;
> +			break;
> +		}
> +		ret = drm_modeset_lock(&crtc->mutex, &ctx);
> +		if (ret == -EDEADLK) {
> +			ret = drm_modeset_backoff(&ctx);
> +			if (!ret) {
> +				try_again = true;
> +				continue;
> +			}
> +			break;
> +		}
> +		if (ret)
> +			break;
Maybe maybe make this:

} else if (ret) {
	break;
}

Just for clarity. With that change:

Reviewed-by: Lyude Paul <lyude@redhat.com>
> +
> +		intel_dp = enc_to_intel_dp(&intel_attached_encoder(connector)-
> >base);
> +		crtc_state = to_intel_crtc_state(crtc->state);
> +		seq_printf(m, "DSC_Enabled: %s\n",
> +			   yesno(crtc_state->dsc_params.compression_enable));
> +		if (intel_dp->dsc_dpcd)
> +			seq_printf(m, "DSC_Sink_Support: %s\n",
> +				   yesno(drm_dp_sink_supports_dsc(intel_dp-
> >dsc_dpcd)));
> +		if (!intel_dp_is_edp(intel_dp))
> +			seq_printf(m, "FEC_Sink_Support: %s\n",
> +				   yesno(drm_dp_sink_supports_fec(intel_dp-
> >fec_capable)));
> +	} while (try_again);
> +
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +
> +	return ret;
> +}
> +
> +static ssize_t i915_dsc_fec_support_write(struct file *file,
> +					  const char __user *ubuf,
> +					  size_t len, loff_t *offp)
> +{
> +	bool dsc_enable = false;
> +	int ret;
> +	struct drm_connector *connector =
> +		((struct seq_file *)file->private_data)->private;
> +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
> +	if (len == 0)
> +		return 0;
> +
> +	DRM_DEBUG_DRIVER("Copied %zu bytes from user to force DSC\n",
> +			 len);
> +
> +	ret = kstrtobool_from_user(ubuf, len, &dsc_enable);
> +	if (ret < 0)
> +		return ret;
> +
> +	DRM_DEBUG_DRIVER("Got %s for DSC Enable\n",
> +			 (dsc_enable) ? "true" : "false");
> +	intel_dp->force_dsc_en = dsc_enable;
> +
> +	*offp += len;
> +	return len;
> +}
> +
> +static int i915_dsc_fec_support_open(struct inode *inode,
> +				     struct file *file)
> +{
> +	return single_open(file, i915_dsc_fec_support_show,
> +			   inode->i_private);
> +}
> +
> +static const struct file_operations i915_dsc_fec_support_fops = {
> +	.owner = THIS_MODULE,
> +	.open = i915_dsc_fec_support_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +	.write = i915_dsc_fec_support_write
> +};
> +
>  /**
>   * i915_debugfs_connector_add - add i915 specific connector debugfs files
>   * @connector: pointer to a registered drm_connector
> @@ -5098,6 +5198,7 @@ DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
>  int i915_debugfs_connector_add(struct drm_connector *connector)
>  {
>  	struct dentry *root = connector->debugfs_entry;
> +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
>  
>  	/* The connector must have been registered beforehands. */
>  	if (!root)
> @@ -5122,5 +5223,11 @@ int i915_debugfs_connector_add(struct drm_connector
> *connector)
>  				    connector,
> &i915_hdcp_sink_capability_fops);
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 10 &&
> +	    (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> +	     connector->connector_type == DRM_MODE_CONNECTOR_eDP))
> +		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
> +				    connector, &i915_dsc_fec_support_fops);
> +
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index a6907a1761ab..2e3097cae277 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2051,7 +2051,8 @@ intel_dp_compute_link_config(struct intel_encoder
> *encoder,
>  							&limits);
>  
>  	/* enable compression if the mode doesn't fit available BW */
> -	if (!ret) {
> +	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> +	if (!ret || intel_dp->force_dsc_en) {
>  		if (!intel_dp_dsc_compute_config(intel_dp, pipe_config,
>  						 conn_state, &limits))
>  			return false;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index f94a04b4ad87..0cedce438433 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1209,6 +1209,9 @@ struct intel_dp {
>  
>  	/* Displayport compliance testing */
>  	struct intel_dp_compliance compliance;
> +
> +	/* Display stream compression testing */
> +	bool force_dsc_en;
>  };
>  
>  enum lspcon_vendor {
-- 
Cheers,
	Lyude Paul

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v7] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-05 23:03     ` Lyude Paul
@ 2018-12-05 23:23       ` Manasi Navare
  0 siblings, 0 replies; 20+ messages in thread
From: Manasi Navare @ 2018-12-05 23:23 UTC (permalink / raw)
  To: Lyude Paul; +Cc: intel-gfx, Rodrigo Vivi

On Wed, Dec 05, 2018 at 06:03:08PM -0500, Lyude Paul wrote:
> On Wed, 2018-12-05 at 14:57 -0800, Manasi Navare wrote:
> > DSC can be supported per DP connector. This patch adds a per connector
> > debugfs node to expose DSC support capability by the kernel.
> > The same node can be used from userspace to force DSC enable.
> > 
> > force_dsc_en written through this debugfs node is used to force
> > DSC even for lower resolutions.
> > 
> > Credits to Ville Syrjala for suggesting the proper locks to be used
> > and to Lyude Paul for explaining how to use them in this context
> > 
> > v7:
> > * Get crtc, crtc_state from connector atomic state
> > and add proper locks and backoff (Ville, Chris Wilson, Lyude)
> > (Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>)
> > * Use %zu for printing size_t variable (Lyude)
> > v6:
> > * Read fec_capable only for non edp (Manasi)
> > v5:
> > * Name it dsc sink support and also add
> > fec support in the same node (Ville)
> > v4:
> > * Add missed connector_status check (Manasi)
> > * Create i915_dsc_support node only for Gen >=10 (manasi)
> > * Access intel_dp->dsc_dpcd only if its not NULL (Manasi)
> > v3:
> > * Combine Force_dsc_en with this patch (Ville)
> > v2:
> > * Use kstrtobool_from_user to avoid explicit error checking (Lyude)
> > * Rebase on drm-tip (Manasi)
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Cc: Lyude Paul <lyude@redhat.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 107 ++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dp.c     |   3 +-
> >  drivers/gpu/drm/i915/intel_drv.h    |   3 +
> >  3 files changed, 112 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 129b9a6f8309..58f6636fffea 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -5086,6 +5086,106 @@ static int i915_hdcp_sink_capability_show(struct
> > seq_file *m, void *data)
> >  }
> >  DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
> >  
> > +static int i915_dsc_fec_support_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_dp *intel_dp;
> > +	struct drm_modeset_acquire_ctx ctx;
> > +	struct intel_crtc_state *crtc_state = NULL;
> > +	int ret = 0;
> > +	bool try_again = false;
> > +
> > +	drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> > +
> > +	do {
> > +		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
> > +				       &ctx);
> > +		if (ret) {
> > +			ret = -EINTR;
> > +			break;
> > +		}
> > +		crtc = connector->state->crtc;
> > +		if (connector->status != connector_status_connected || !crtc)
> > {
> > +			ret = -ENODEV;
> > +			break;
> > +		}
> > +		ret = drm_modeset_lock(&crtc->mutex, &ctx);
> > +		if (ret == -EDEADLK) {
> > +			ret = drm_modeset_backoff(&ctx);
> > +			if (!ret) {
> > +				try_again = true;
> > +				continue;
> > +			}
> > +			break;
> > +		}
> > +		if (ret)
> > +			break;
> Maybe maybe make this:

Cool will do and add your r-b
Thanks for the reviews!

Manasi

> 
> } else if (ret) {
> 	break;
> }
> 
> Just for clarity. With that change:
> 
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> > +
> > +		intel_dp = enc_to_intel_dp(&intel_attached_encoder(connector)-
> > >base);
> > +		crtc_state = to_intel_crtc_state(crtc->state);
> > +		seq_printf(m, "DSC_Enabled: %s\n",
> > +			   yesno(crtc_state->dsc_params.compression_enable));
> > +		if (intel_dp->dsc_dpcd)
> > +			seq_printf(m, "DSC_Sink_Support: %s\n",
> > +				   yesno(drm_dp_sink_supports_dsc(intel_dp-
> > >dsc_dpcd)));
> > +		if (!intel_dp_is_edp(intel_dp))
> > +			seq_printf(m, "FEC_Sink_Support: %s\n",
> > +				   yesno(drm_dp_sink_supports_fec(intel_dp-
> > >fec_capable)));
> > +	} while (try_again);
> > +
> > +	drm_modeset_drop_locks(&ctx);
> > +	drm_modeset_acquire_fini(&ctx);
> > +
> > +	return ret;
> > +}
> > +
> > +static ssize_t i915_dsc_fec_support_write(struct file *file,
> > +					  const char __user *ubuf,
> > +					  size_t len, loff_t *offp)
> > +{
> > +	bool dsc_enable = false;
> > +	int ret;
> > +	struct drm_connector *connector =
> > +		((struct seq_file *)file->private_data)->private;
> > +	struct intel_encoder *encoder = intel_attached_encoder(connector);
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > +
> > +	if (len == 0)
> > +		return 0;
> > +
> > +	DRM_DEBUG_DRIVER("Copied %zu bytes from user to force DSC\n",
> > +			 len);
> > +
> > +	ret = kstrtobool_from_user(ubuf, len, &dsc_enable);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	DRM_DEBUG_DRIVER("Got %s for DSC Enable\n",
> > +			 (dsc_enable) ? "true" : "false");
> > +	intel_dp->force_dsc_en = dsc_enable;
> > +
> > +	*offp += len;
> > +	return len;
> > +}
> > +
> > +static int i915_dsc_fec_support_open(struct inode *inode,
> > +				     struct file *file)
> > +{
> > +	return single_open(file, i915_dsc_fec_support_show,
> > +			   inode->i_private);
> > +}
> > +
> > +static const struct file_operations i915_dsc_fec_support_fops = {
> > +	.owner = THIS_MODULE,
> > +	.open = i915_dsc_fec_support_open,
> > +	.read = seq_read,
> > +	.llseek = seq_lseek,
> > +	.release = single_release,
> > +	.write = i915_dsc_fec_support_write
> > +};
> > +
> >  /**
> >   * i915_debugfs_connector_add - add i915 specific connector debugfs files
> >   * @connector: pointer to a registered drm_connector
> > @@ -5098,6 +5198,7 @@ DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
> >  int i915_debugfs_connector_add(struct drm_connector *connector)
> >  {
> >  	struct dentry *root = connector->debugfs_entry;
> > +	struct drm_i915_private *dev_priv = to_i915(connector->dev);
> >  
> >  	/* The connector must have been registered beforehands. */
> >  	if (!root)
> > @@ -5122,5 +5223,11 @@ int i915_debugfs_connector_add(struct drm_connector
> > *connector)
> >  				    connector,
> > &i915_hdcp_sink_capability_fops);
> >  	}
> >  
> > +	if (INTEL_GEN(dev_priv) >= 10 &&
> > +	    (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> > +	     connector->connector_type == DRM_MODE_CONNECTOR_eDP))
> > +		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
> > +				    connector, &i915_dsc_fec_support_fops);
> > +
> >  	return 0;
> >  }
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index a6907a1761ab..2e3097cae277 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2051,7 +2051,8 @@ intel_dp_compute_link_config(struct intel_encoder
> > *encoder,
> >  							&limits);
> >  
> >  	/* enable compression if the mode doesn't fit available BW */
> > -	if (!ret) {
> > +	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> > +	if (!ret || intel_dp->force_dsc_en) {
> >  		if (!intel_dp_dsc_compute_config(intel_dp, pipe_config,
> >  						 conn_state, &limits))
> >  			return false;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index f94a04b4ad87..0cedce438433 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1209,6 +1209,9 @@ struct intel_dp {
> >  
> >  	/* Displayport compliance testing */
> >  	struct intel_dp_compliance compliance;
> > +
> > +	/* Display stream compression testing */
> > +	bool force_dsc_en;
> >  };
> >  
> >  enum lspcon_vendor {
> -- 
> Cheers,
> 	Lyude Paul
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev3)
  2018-12-04  3:07 [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable Manasi Navare
                   ` (4 preceding siblings ...)
  2018-12-04 20:44 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2018-12-05 23:25 ` Patchwork
  2018-12-05 23:46 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-12-05 23:25 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev3)
URL   : https://patchwork.freedesktop.org/series/53449/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
0f0985d49a1e drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
-:170: WARNING:SYMBOLIC_PERMS: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
#170: FILE: drivers/gpu/drm/i915/i915_debugfs.c:5231:
+		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,

-:199: CHECK:BOOL_MEMBER: Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384
#199: FILE: drivers/gpu/drm/i915/intel_drv.h:1214:
+	bool force_dsc_en;

total: 0 errors, 1 warnings, 1 checks, 142 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev3)
  2018-12-04  3:07 [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable Manasi Navare
                   ` (5 preceding siblings ...)
  2018-12-05 23:25 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev3) Patchwork
@ 2018-12-05 23:46 ` Patchwork
  2018-12-06  1:56 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev4) Patchwork
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-12-05 23:46 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev3)
URL   : https://patchwork.freedesktop.org/series/53449/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5267 -> Patchwork_11026
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53449/revisions/3/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-ivb-3520m:       PASS -> FAIL [fdo#108880]

  * igt@pm_rpm@basic-rte:
    - {fi-icl-u3}:        PASS -> DMESG-WARN [fdo#108654]

  * {igt@runner@aborted}:
    - {fi-icl-u3}:        NOTRUN -> FAIL [fdo#108654] / [fdo#108756]
    - {fi-icl-y}:         NOTRUN -> FAIL [fdo#108070]

  
#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       DMESG-WARN [fdo#102614] -> PASS

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

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#108070]: https://bugs.freedesktop.org/show_bug.cgi?id=108070
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108756]: https://bugs.freedesktop.org/show_bug.cgi?id=108756
  [fdo#108880]: https://bugs.freedesktop.org/show_bug.cgi?id=108880


Participating hosts (49 -> 45)
------------------------------

  Additional (1): fi-icl-y 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


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

    * Linux: CI_DRM_5267 -> Patchwork_11026

  CI_DRM_5267: 680d161b3cf77a94e05dfdedcdeed4f38e4b7fd8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11026: 0f0985d49a1e6a169aa8516418b64d6cf386e540 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0f0985d49a1e drm/i915/dsc: Add Per connector debugfs node for DSC support/enable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11026/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v8] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-05 22:57   ` [PATCH v7] " Manasi Navare
  2018-12-05 23:03     ` Lyude Paul
@ 2018-12-06  0:54     ` Manasi Navare
  2018-12-19  8:08       ` Chris Wilson
  1 sibling, 1 reply; 20+ messages in thread
From: Manasi Navare @ 2018-12-06  0:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

DSC can be supported per DP connector. This patch adds a per connector
debugfs node to expose DSC support capability by the kernel.
The same node can be used from userspace to force DSC enable.

force_dsc_en written through this debugfs node is used to force
DSC even for lower resolutions.

Credits to Ville Syrjala for suggesting the proper locks to be used
and to Lyude Paul for explaining how to use them in this context

v8:
* Add else if (ret) for drm_modeset_lock (Lyude)
v7:
* Get crtc, crtc_state from connector atomic state
and add proper locks and backoff (Ville, Chris Wilson, Lyude)
(Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>)
* Use %zu for printing size_t variable (Lyude)
v6:
* Read fec_capable only for non edp (Manasi)
v5:
* Name it dsc sink support and also add
fec support in the same node (Ville)
v4:
* Add missed connector_status check (Manasi)
* Create i915_dsc_support node only for Gen >=10 (manasi)
* Access intel_dp->dsc_dpcd only if its not NULL (Manasi)
v3:
* Combine Force_dsc_en with this patch (Ville)
v2:
* Use kstrtobool_from_user to avoid explicit error checking (Lyude)
* Rebase on drm-tip (Manasi)

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 106 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c     |   3 +-
 drivers/gpu/drm/i915/intel_drv.h    |   3 +
 3 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 38dcee1ca062..5fe1f44e9bc0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -5088,6 +5088,105 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
 
+static int i915_dsc_fec_support_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_dp *intel_dp;
+	struct drm_modeset_acquire_ctx ctx;
+	struct intel_crtc_state *crtc_state = NULL;
+	int ret = 0;
+	bool try_again = false;
+
+	drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
+
+	do {
+		ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
+				       &ctx);
+		if (ret) {
+			ret = -EINTR;
+			break;
+		}
+		crtc = connector->state->crtc;
+		if (connector->status != connector_status_connected || !crtc) {
+			ret = -ENODEV;
+			break;
+		}
+		ret = drm_modeset_lock(&crtc->mutex, &ctx);
+		if (ret == -EDEADLK) {
+			ret = drm_modeset_backoff(&ctx);
+			if (!ret) {
+				try_again = true;
+				continue;
+			}
+			break;
+		} else if (ret) {
+			break;
+		}
+		intel_dp = enc_to_intel_dp(&intel_attached_encoder(connector)->base);
+		crtc_state = to_intel_crtc_state(crtc->state);
+		seq_printf(m, "DSC_Enabled: %s\n",
+			   yesno(crtc_state->dsc_params.compression_enable));
+		if (intel_dp->dsc_dpcd)
+			seq_printf(m, "DSC_Sink_Support: %s\n",
+				   yesno(drm_dp_sink_supports_dsc(intel_dp->dsc_dpcd)));
+		if (!intel_dp_is_edp(intel_dp))
+			seq_printf(m, "FEC_Sink_Support: %s\n",
+				   yesno(drm_dp_sink_supports_fec(intel_dp->fec_capable)));
+	} while (try_again);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	return ret;
+}
+
+static ssize_t i915_dsc_fec_support_write(struct file *file,
+					  const char __user *ubuf,
+					  size_t len, loff_t *offp)
+{
+	bool dsc_enable = false;
+	int ret;
+	struct drm_connector *connector =
+		((struct seq_file *)file->private_data)->private;
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
+	if (len == 0)
+		return 0;
+
+	DRM_DEBUG_DRIVER("Copied %zu bytes from user to force DSC\n",
+			 len);
+
+	ret = kstrtobool_from_user(ubuf, len, &dsc_enable);
+	if (ret < 0)
+		return ret;
+
+	DRM_DEBUG_DRIVER("Got %s for DSC Enable\n",
+			 (dsc_enable) ? "true" : "false");
+	intel_dp->force_dsc_en = dsc_enable;
+
+	*offp += len;
+	return len;
+}
+
+static int i915_dsc_fec_support_open(struct inode *inode,
+				     struct file *file)
+{
+	return single_open(file, i915_dsc_fec_support_show,
+			   inode->i_private);
+}
+
+static const struct file_operations i915_dsc_fec_support_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_dsc_fec_support_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+	.write = i915_dsc_fec_support_write
+};
+
 /**
  * i915_debugfs_connector_add - add i915 specific connector debugfs files
  * @connector: pointer to a registered drm_connector
@@ -5100,6 +5199,7 @@ DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
 int i915_debugfs_connector_add(struct drm_connector *connector)
 {
 	struct dentry *root = connector->debugfs_entry;
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 
 	/* The connector must have been registered beforehands. */
 	if (!root)
@@ -5124,5 +5224,11 @@ int i915_debugfs_connector_add(struct drm_connector *connector)
 				    connector, &i915_hdcp_sink_capability_fops);
 	}
 
+	if (INTEL_GEN(dev_priv) >= 10 &&
+	    (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+	     connector->connector_type == DRM_MODE_CONNECTOR_eDP))
+		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
+				    connector, &i915_dsc_fec_support_fops);
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index de4219721cbc..8aa6f3c649d3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2051,7 +2051,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 							&limits);
 
 	/* enable compression if the mode doesn't fit available BW */
-	if (!ret) {
+	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
+	if (!ret || intel_dp->force_dsc_en) {
 		if (!intel_dp_dsc_compute_config(intel_dp, pipe_config,
 						 conn_state, &limits))
 			return false;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f94a04b4ad87..0cedce438433 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1209,6 +1209,9 @@ struct intel_dp {
 
 	/* Displayport compliance testing */
 	struct intel_dp_compliance compliance;
+
+	/* Display stream compression testing */
+	bool force_dsc_en;
 };
 
 enum lspcon_vendor {
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev4)
  2018-12-04  3:07 [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable Manasi Navare
                   ` (6 preceding siblings ...)
  2018-12-05 23:46 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-12-06  1:56 ` Patchwork
  2018-12-06  2:19 ` ✓ Fi.CI.BAT: success " Patchwork
  2018-12-06 12:21 ` ✓ Fi.CI.IGT: " Patchwork
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-12-06  1:56 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev4)
URL   : https://patchwork.freedesktop.org/series/53449/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
00393ec00edf drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
-:171: WARNING:SYMBOLIC_PERMS: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
#171: FILE: drivers/gpu/drm/i915/i915_debugfs.c:5230:
+		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,

-:200: CHECK:BOOL_MEMBER: Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384
#200: FILE: drivers/gpu/drm/i915/intel_drv.h:1214:
+	bool force_dsc_en;

total: 0 errors, 1 warnings, 1 checks, 141 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev4)
  2018-12-04  3:07 [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable Manasi Navare
                   ` (7 preceding siblings ...)
  2018-12-06  1:56 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev4) Patchwork
@ 2018-12-06  2:19 ` Patchwork
  2018-12-06 12:21 ` ✓ Fi.CI.IGT: " Patchwork
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-12-06  2:19 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev4)
URL   : https://patchwork.freedesktop.org/series/53449/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5269 -> Patchwork_11028
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/53449/revisions/4/mbox/

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

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

### IGT changes ###

#### Warnings ####

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - {fi-kbl-7567u}:     PASS -> SKIP +33

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_selftest@live_hangcheck:
    - fi-bwr-2160:        PASS -> DMESG-FAIL [fdo#108735]

  * igt@prime_vgem@basic-fence-flip:
    - fi-bdw-gvtdvm:      PASS -> FAIL [fdo#103182]

  
#### Possible fixes ####

  * igt@i915_selftest@live_coherency:
    - fi-gdg-551:         DMESG-FAIL [fdo#107164] -> PASS

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         DMESG-WARN [fdo#108622] -> PASS

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

  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#107164]: https://bugs.freedesktop.org/show_bug.cgi?id=107164
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735


Participating hosts (49 -> 45)
------------------------------

  Additional (1): fi-kbl-7560u 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bdw-samus 


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

    * Linux: CI_DRM_5269 -> Patchwork_11028

  CI_DRM_5269: 4a90a5940e2c7c3fba7af5529e693ba32ccb6d43 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11028: 00393ec00edf942b5cda8f7315c8af5487ae7092 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

00393ec00edf drm/i915/dsc: Add Per connector debugfs node for DSC support/enable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11028/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev4)
  2018-12-04  3:07 [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable Manasi Navare
                   ` (8 preceding siblings ...)
  2018-12-06  2:19 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-12-06 12:21 ` Patchwork
  9 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2018-12-06 12:21 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev4)
URL   : https://patchwork.freedesktop.org/series/53449/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5269_full -> Patchwork_11028_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_11028_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11028_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

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

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

### IGT changes ###

#### Warnings ####

  * igt@kms_draw_crc@draw-method-xrgb8888-blt-xtiled:
    - shard-snb:          PASS -> SKIP

  * igt@tools_test@sysfs_l3_parity:
    - shard-hsw:          SKIP -> PASS

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_suspend@debugfs-reader:
    - {shard-iclb}:       PASS -> DMESG-FAIL [fdo#103375]

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
    - shard-skl:          PASS -> FAIL [fdo#107815] / [fdo#108470]

  * igt@kms_busy@extended-modeset-hang-newfb-render-c:
    - {shard-iclb}:       NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-a-crc-primary-rotation-180:
    - {shard-iclb}:       NOTRUN -> FAIL [fdo#107725]

  * igt@kms_chv_cursor_fail@pipe-c-128x128-left-edge:
    - shard-skl:          NOTRUN -> FAIL [fdo#104671]

  * igt@kms_cursor_crc@cursor-128x128-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-256x85-offscreen:
    - shard-skl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_draw_crc@draw-method-rgb565-mmap-wc-ytiled:
    - shard-skl:          PASS -> FAIL [fdo#103184]

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-untiled:
    - {shard-iclb}:       PASS -> WARN [fdo#108336] +1

  * igt@kms_draw_crc@draw-method-xrgb8888-pwrite-untiled:
    - shard-skl:          PASS -> FAIL [fdo#108472]

  * igt@kms_flip@dpms-off-confusion:
    - {shard-iclb}:       PASS -> DMESG-WARN [fdo#107724] +13

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-glk:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-indfb-scaledprimary:
    - {shard-iclb}:       PASS -> DMESG-FAIL [fdo#107724] +7

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-wc:
    - {shard-iclb}:       PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-render:
    - shard-skl:          PASS -> FAIL [fdo#103167] +3

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-cpu:
    - {shard-iclb}:       PASS -> DMESG-WARN [fdo#107724] / [fdo#108336] +9

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885]

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - {shard-iclb}:       PASS -> DMESG-FAIL [fdo#103166] / [fdo#107724]

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-glk:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-apl:          PASS -> FAIL [fdo#103166]

  * {igt@kms_rotation_crc@multiplane-rotation-cropping-top}:
    - shard-glk:          PASS -> DMESG-WARN [fdo#105763] / [fdo#106538]

  * igt@perf@polling:
    - shard-hsw:          PASS -> FAIL [fdo#102252]

  * igt@pm_rpm@modeset-non-lpsp-stress:
    - shard-skl:          SKIP -> INCOMPLETE [fdo#107807]

  
#### Possible fixes ####

  * igt@gem_exec_create@forked:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-skl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-glk:          FAIL [fdo#103232] -> PASS +2

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          FAIL [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
    - shard-glk:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
    - {shard-iclb}:       FAIL [fdo#105683] -> PASS

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-wc:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +1

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-move:
    - {shard-iclb}:       FAIL [fdo#103167] -> PASS +4

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-apl:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - {shard-iclb}:       FAIL [fdo#103166] -> PASS +3

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
    - shard-glk:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_rmfb@rmfb-ioctl:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] -> PASS +4

  * igt@pm_rpm@pm-tiling:
    - {shard-iclb}:       INCOMPLETE [fdo#107713] / [fdo#108840] -> PASS

  
#### Warnings ####

  * igt@i915_suspend@shrink:
    - shard-skl:          INCOMPLETE [fdo#106886] -> DMESG-WARN [fdo#108784]
    - shard-glk:          DMESG-WARN [fdo#108784] -> INCOMPLETE [fdo#103359] / [fdo#106886] / [k.org#198133]

  * igt@kms_ccs@pipe-c-crc-primary-basic:
    - {shard-iclb}:       FAIL [fdo#107725] -> DMESG-WARN [fdo#107724] / [fdo#108336]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
    - {shard-iclb}:       DMESG-FAIL [fdo#107724] -> FAIL [fdo#103167]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103166]

  * igt@kms_properties@connector-properties-atomic:
    - {shard-iclb}:       FAIL [fdo#108642] -> DMESG-FAIL [fdo#107724]

  * {igt@kms_rotation_crc@multiplane-rotation-cropping-top}:
    - shard-kbl:          DMESG-WARN [fdo#105604] -> DMESG-FAIL [fdo#108950]

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

  [fdo#102252]: https://bugs.freedesktop.org/show_bug.cgi?id=102252
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105604]: https://bugs.freedesktop.org/show_bug.cgi?id=105604
  [fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
  [fdo#108470]: https://bugs.freedesktop.org/show_bug.cgi?id=108470
  [fdo#108472]: https://bugs.freedesktop.org/show_bug.cgi?id=108472
  [fdo#108642]: https://bugs.freedesktop.org/show_bug.cgi?id=108642
  [fdo#108784]: https://bugs.freedesktop.org/show_bug.cgi?id=108784
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


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

    * Linux: CI_DRM_5269 -> Patchwork_11028

  CI_DRM_5269: 4a90a5940e2c7c3fba7af5529e693ba32ccb6d43 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11028: 00393ec00edf942b5cda8f7315c8af5487ae7092 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11028/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-06  0:54     ` [PATCH v8] " Manasi Navare
@ 2018-12-19  8:08       ` Chris Wilson
  2018-12-19 21:39         ` Manasi Navare
  0 siblings, 1 reply; 20+ messages in thread
From: Chris Wilson @ 2018-12-19  8:08 UTC (permalink / raw)
  To: Manasi Navare, intel-gfx; +Cc: Rodrigo Vivi

Quoting Manasi Navare (2018-12-06 00:54:07)
> DSC can be supported per DP connector. This patch adds a per connector
> debugfs node to expose DSC support capability by the kernel.
> The same node can be used from userspace to force DSC enable.

So this has a nice deadlock that is killing icl sporadically...

> +static int i915_dsc_fec_support_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_dp *intel_dp;
> +       struct drm_modeset_acquire_ctx ctx;
> +       struct intel_crtc_state *crtc_state = NULL;
> +       int ret = 0;
> +       bool try_again = false;
> +
> +       drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> +
> +       do {
> +               ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
> +                                      &ctx);
> +               if (ret) {
> +                       ret = -EINTR;
> +                       break;
> +               }
> +               crtc = connector->state->crtc;
> +               if (connector->status != connector_status_connected || !crtc) {
> +                       ret = -ENODEV;
> +                       break;
> +               }
> +               ret = drm_modeset_lock(&crtc->mutex, &ctx);
> +               if (ret == -EDEADLK) {
> +                       ret = drm_modeset_backoff(&ctx);
> +                       if (!ret) {
> +                               try_again = true;

try_again is never cleared on the next loop, so after you hit the
backoff, we never escape on success.

Reported-by: Tomi Sarvela <tomi.p.sarvela@intel.com>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v8] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable
  2018-12-19  8:08       ` Chris Wilson
@ 2018-12-19 21:39         ` Manasi Navare
  0 siblings, 0 replies; 20+ messages in thread
From: Manasi Navare @ 2018-12-19 21:39 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, Rodrigo Vivi

On Wed, Dec 19, 2018 at 08:08:51AM +0000, Chris Wilson wrote:
> Quoting Manasi Navare (2018-12-06 00:54:07)
> > DSC can be supported per DP connector. This patch adds a per connector
> > debugfs node to expose DSC support capability by the kernel.
> > The same node can be used from userspace to force DSC enable.
> 
> So this has a nice deadlock that is killing icl sporadically...
> 
> > +static int i915_dsc_fec_support_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_dp *intel_dp;
> > +       struct drm_modeset_acquire_ctx ctx;
> > +       struct intel_crtc_state *crtc_state = NULL;
> > +       int ret = 0;
> > +       bool try_again = false;
> > +
> > +       drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> > +
> > +       do {
> > +               ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
> > +                                      &ctx);
> > +               if (ret) {
> > +                       ret = -EINTR;
> > +                       break;
> > +               }
> > +               crtc = connector->state->crtc;
> > +               if (connector->status != connector_status_connected || !crtc) {
> > +                       ret = -ENODEV;
> > +                       break;
> > +               }
> > +               ret = drm_modeset_lock(&crtc->mutex, &ctx);
> > +               if (ret == -EDEADLK) {
> > +                       ret = drm_modeset_backoff(&ctx);
> > +                       if (!ret) {
> > +                               try_again = true;
> 
> try_again is never cleared on the next loop, so after you hit the
> backoff, we never escape on success.

Thanks for catching this, teh CI results came green before this patch was merged so
never noticed this.
I will submit a patch to fix this.

Thanks again Chris and Tomi for catching this bug

Manasi

> 
> Reported-by: Tomi Sarvela <tomi.p.sarvela@intel.com>
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-12-19 21:36 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-04  3:07 [PATCH v5] drm/i915/dsc: Add Per connector debugfs node for DSC support/enable Manasi Navare
2018-12-04  3:20 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-12-04  3:45 ` ✗ Fi.CI.BAT: failure " Patchwork
2018-12-04 19:55 ` [PATCH v6] " Manasi Navare
2018-12-05 19:00   ` Lyude Paul
2018-12-05 19:29     ` Manasi Navare
2018-12-05 21:24       ` Lyude Paul
2018-12-05 22:57   ` [PATCH v7] " Manasi Navare
2018-12-05 23:03     ` Lyude Paul
2018-12-05 23:23       ` Manasi Navare
2018-12-06  0:54     ` [PATCH v8] " Manasi Navare
2018-12-19  8:08       ` Chris Wilson
2018-12-19 21:39         ` Manasi Navare
2018-12-04 20:18 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev2) Patchwork
2018-12-04 20:44 ` ✗ Fi.CI.BAT: failure " Patchwork
2018-12-05 23:25 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev3) Patchwork
2018-12-05 23:46 ` ✓ Fi.CI.BAT: success " Patchwork
2018-12-06  1:56 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dsc: Add Per connector debugfs node for DSC support/enable (rev4) Patchwork
2018-12-06  2:19 ` ✓ Fi.CI.BAT: success " Patchwork
2018-12-06 12:21 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.