[Intel-gfx] [RFC 0/2] i915: Add debugfs for requesting HDCP version

[Intel-gfx] [RFC 0/2] i915: Add debugfs for requesting HDCP version

[Ankit Nautiyal]

Currently, for a given content-protection request, the kernel selects
the highest version of HDCP supported by the panel and the platform.

This makes the testing/debugging difficult for lower versions of HDCP.
E.g. In case both the lower and the higher HDCP versions are supported 
then the higher version of HDCP will always be selected and the
lower HDCP version cannot be tested without changing the setup.

A solution for this was proposed in an IGT patch [1] by removing
"mei_hdcp" module, but a need for a generic future-proof solution was
identified.

As suggested by the community members, this patch attempts to add a
new debugfs per connector for requesting a specific version of HDCP
for debug/testing environment.

The test can request for a specific HDCP version and set the
appropriate content-protection connector properties to test the
required version. The kernel will consider the request if the HDCP
version is sufficient for the requested content-protection.

To make the design simpler, the debugfs for requesting the HDCP
version can be only set when the content-protection is disabled.
In case, the test tries to write the debugfs while a request for
content-protectiont is ongoing, the write will be failed.
So the test needs to make sure that the content-protection is disabled
before requesting for an HDCP version via debugfs.


[1] https://patchwork.freedesktop.org/patch/358240/

Ankit Nautiyal (2):
  drm/i915: Add support for considering HDCP ver requested via debugfs
  drm/i915: Add a new debugfs to request HDCP version

 .../drm/i915/display/intel_display_debugfs.c  | 98 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    | 10 ++
 drivers/gpu/drm/i915/display/intel_hdcp.c     |  8 +-
 3 files changed, 114 insertions(+), 2 deletions(-)

-- 
2.17.1

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

[Intel-gfx] [RFC 1/2] drm/i915: Add support for considering HDCP ver requested via debugfs

[Ankit Nautiyal]

For testing and debugging each HDCP version separately, a debugfs
entry for requesting a specific version is required. The vesion
requested via debugfs needs to be stored in hdcp structure. This can
then be considered while enabling HDCP, provided the platform and the
display supports the requested version.

This patch adds the support for storing the version requested as a 32bit
flag. The default value is set to signify that any of the HDCP version
can be used.

If a specific HDCP version is requested through the debugfs, the driver
chooses that version, instead of policy of choosing the highest HDCP
version supported.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_types.h | 10 ++++++++++
 drivers/gpu/drm/i915/display/intel_hdcp.c          |  8 ++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 9488449e4b94..cfa641c70717 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -408,6 +408,16 @@ struct intel_hdcp {
 	 * Hence caching the transcoder here.
 	 */
 	enum transcoder cpu_transcoder;
+
+	/*
+	 * HDCP version requested from debugfs i915_hdcp_ver_request.
+	 * Kernel will read these bits and entertain the request, as per
+	 * the HDCP capability of the panel and platform.
+	 */
+#define HDCP_VERSION_1_4	0x01
+#define HDCP_VERSION_2_2	0x02
+#define HDCP_VERSION_MASK	0x03
+	u32 debugfs_ver_request;
 };
 
 struct intel_connector {
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index 2cbc4619b4ce..951401046804 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -1977,6 +1977,8 @@ int intel_hdcp_init(struct intel_connector *connector,
 	if (!shim)
 		return -EINVAL;
 
+	hdcp->debugfs_ver_request = HDCP_VERSION_MASK;
+
 	if (is_hdcp2_supported(dev_priv))
 		intel_hdcp2_init(connector, shim);
 
@@ -2023,7 +2025,8 @@ int intel_hdcp_enable(struct intel_connector *connector,
 	 * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
 	 * is capable of HDCP2.2, it is preferred to use HDCP2.2.
 	 */
-	if (intel_hdcp2_capable(connector)) {
+	if ((hdcp->debugfs_ver_request & HDCP_VERSION_2_2) &&
+	    intel_hdcp2_capable(connector)) {
 		ret = _intel_hdcp2_enable(connector);
 		if (!ret)
 			check_link_interval = DRM_HDCP2_CHECK_PERIOD_MS;
@@ -2033,7 +2036,8 @@ int intel_hdcp_enable(struct intel_connector *connector,
 	 * When HDCP2.2 fails and Content Type is not Type1, HDCP1.4 will
 	 * be attempted.
 	 */
-	if (ret && intel_hdcp_capable(connector) &&
+	if (ret && (hdcp->debugfs_ver_request & HDCP_VERSION_1_4) &&
+	    intel_hdcp_capable(connector) &&
 	    hdcp->content_type != DRM_MODE_HDCP_CONTENT_TYPE1) {
 		ret = _intel_hdcp_enable(connector);
 	}
-- 
2.17.1

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

[Intel-gfx] [RFC 2/2] drm/i915: Add a new debugfs to request HDCP version

[Ankit Nautiyal]

As per the current HDCP design, the driver selects the highest
version of HDCP that can be used to satisfy the content-protection
requirements of the user. Due to this, the content-protection
tests cannot test a lower version of HDCP, if the platform and the
display panel, both support higher HDCP version.

To provide some support for testing and debugging, a per-connector
debugfs is required to set the HDCP version via debugfs that the
kernel can consider, while enabling HDCP.

This patch adds a new debugfs entry for each connector that supports
HDCP. For enforcing a particular HDCP version for a connector, the user
can write into the debugfs for that connector.

To make design simple, the HDCP version request can only be made via
debugfs, if there is no ongoing request for Content-Protection for
the connector. The tests are expected to make sure that HDCP is disabled
before making HDCP version request via the debugfs for the connector.
Otherwise, the write request to the debugfs will be failed.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 98 +++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 70525623bcdf..e65abca1a1fa 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -2185,6 +2185,102 @@ static const struct file_operations i915_dsc_fec_support_fops = {
 	.write = i915_dsc_fec_support_write
 };
 
+static int i915_hdcp_ver_request_show(struct seq_file *m, void *data)
+{
+
+	struct drm_connector *connector = m->private;
+	struct intel_connector *intel_connector = to_intel_connector(connector);
+	u64 hdcp_ver_flag;
+
+	if (connector->status != connector_status_connected)
+		return -ENODEV;
+
+	/* HDCP is supported by connector */
+	if (!intel_connector->hdcp.shim)
+		return -EINVAL;
+
+	hdcp_ver_flag = intel_connector->hdcp.debugfs_ver_request;
+	seq_printf(m, "HDCP_VER_FLAGS: %llu\n", hdcp_ver_flag);
+
+	seq_printf(m, "Requested Versions:\n");
+	if (hdcp_ver_flag & HDCP_VERSION_1_4)
+		seq_printf(m, "HDCP1.4\n");
+	if (hdcp_ver_flag & HDCP_VERSION_2_2)
+		seq_printf(m, "HDCP2.2\n");
+
+	return 0;
+}
+
+static int i915_hdcp_ver_request_open(struct inode *inode,
+				     struct file *file)
+{
+	return single_open(file, i915_hdcp_ver_request_show,
+			   inode->i_private);
+}
+
+static int intel_hdcp_debugfs_ver_set(struct intel_connector *connector, u64 val)
+{
+	struct intel_hdcp *hdcp = &connector->hdcp;
+
+	if (!hdcp->shim || val > HDCP_VERSION_MASK)
+		return -EINVAL;
+
+	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
+		return -EINVAL;
+
+	hdcp->debugfs_ver_request = val;
+	return 0;
+}
+
+static ssize_t i915_hdcp_ver_request_write(struct file *file,
+					  const char __user *ubuf,
+					  size_t len, loff_t *offp)
+{
+	unsigned int hdcp_ver = 0;
+	int ret;
+	struct drm_connector *connector =
+		((struct seq_file *)file->private_data)->private;
+	struct intel_connector *intel_con = to_intel_connector(connector);
+	struct drm_i915_private *i915 = to_i915(connector->dev);
+	char tmp[16];
+
+	if (len == 0)
+		return 0;
+
+	if (len >= sizeof(tmp))
+		return -EINVAL;
+
+	if (copy_from_user(tmp, ubuf, len))
+		return -EFAULT;
+
+	tmp[len] = '\0';
+
+
+	drm_dbg(&i915->drm,
+		"Copied %zu bytes from user to request hdcp ver\n", len);
+
+	ret = kstrtouint(tmp, 10, &hdcp_ver);
+	if (ret < 0)
+		return ret;
+
+	drm_dbg(&i915->drm, "Got %u for HDCP version\n", hdcp_ver);
+	ret = intel_hdcp_debugfs_ver_set(intel_con, hdcp_ver);
+	if (ret < 0)
+		return ret;
+
+	*offp += len;
+	return len;
+}
+
+static const struct file_operations i915_hdcp_ver_request_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_hdcp_ver_request_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+	.write = i915_hdcp_ver_request_write
+};
+
 /**
  * intel_connector_debugfs_add - add i915 specific connector debugfs files
  * @connector: pointer to a registered drm_connector
@@ -2215,6 +2311,8 @@ int intel_connector_debugfs_add(struct drm_connector *connector)
 	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
 		debugfs_create_file("i915_hdcp_sink_capability", S_IRUGO, root,
 				    connector, &i915_hdcp_sink_capability_fops);
+		debugfs_create_file("i915_hdcp_version_request", S_IRUGO, root,
+				    connector, &i915_hdcp_ver_request_fops);
 	}
 
 	if (INTEL_GEN(dev_priv) >= 10 &&
-- 
2.17.1

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

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for i915: Add debugfs for requesting HDCP version

[Patchwork]

== Series Details ==

Series: i915: Add debugfs for requesting HDCP version
URL   : https://patchwork.freedesktop.org/series/77693/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ee34d42eee76 drm/i915: Add support for considering HDCP ver requested via debugfs
a44896d4d874 drm/i915: Add a new debugfs to request HDCP version
-:38: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#38: FILE: drivers/gpu/drm/i915/display/intel_display_debugfs.c:2190:
+{
+

-:53: WARNING:PREFER_SEQ_PUTS: Prefer seq_puts to seq_printf
#53: FILE: drivers/gpu/drm/i915/display/intel_display_debugfs.c:2205:
+	seq_printf(m, "Requested Versions:\n");

-:55: WARNING:PREFER_SEQ_PUTS: Prefer seq_puts to seq_printf
#55: FILE: drivers/gpu/drm/i915/display/intel_display_debugfs.c:2207:
+		seq_printf(m, "HDCP1.4\n");

-:57: WARNING:PREFER_SEQ_PUTS: Prefer seq_puts to seq_printf
#57: FILE: drivers/gpu/drm/i915/display/intel_display_debugfs.c:2209:
+		seq_printf(m, "HDCP2.2\n");

-:63: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#63: FILE: drivers/gpu/drm/i915/display/intel_display_debugfs.c:2215:
+static int i915_hdcp_ver_request_open(struct inode *inode,
+				     struct file *file)

-:84: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#84: FILE: drivers/gpu/drm/i915/display/intel_display_debugfs.c:2236:
+static ssize_t i915_hdcp_ver_request_write(struct file *file,
+					  const char __user *ubuf,

-:106: CHECK:LINE_SPACING: Please don't use multiple blank lines
#106: FILE: drivers/gpu/drm/i915/display/intel_display_debugfs.c:2258:
+
+

-:139: WARNING:SYMBOLIC_PERMS: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
#139: FILE: drivers/gpu/drm/i915/display/intel_display_debugfs.c:2314:
+		debugfs_create_file("i915_hdcp_version_request", S_IRUGO, root,

total: 0 errors, 4 warnings, 4 checks, 110 lines checked

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

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for i915: Add debugfs for requesting HDCP version

[Patchwork]

== Series Details ==

Series: i915: Add debugfs for requesting HDCP version
URL   : https://patchwork.freedesktop.org/series/77693/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.0
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/display/intel_display.c:1222:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/display/intel_display.c:1225:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/display/intel_display.c:1228:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/display/intel_display.c:1231:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2274:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2275:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2276:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2277:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2278:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2279:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gt/intel_reset.c:1310:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block

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

[Intel-gfx] ✓ Fi.CI.BAT: success for i915: Add debugfs for requesting HDCP version

[Patchwork]

== Series Details ==

Series: i915: Add debugfs for requesting HDCP version
URL   : https://patchwork.freedesktop.org/series/77693/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8542 -> Patchwork_17785
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Possible fixes ####

  * igt@i915_selftest@live@active:
    - fi-kbl-soraka:      [DMESG-FAIL][1] ([i915#666]) -> [PASS][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/fi-kbl-soraka/igt@i915_selftest@live@active.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/fi-kbl-soraka/igt@i915_selftest@live@active.html

  
  [i915#666]: https://gitlab.freedesktop.org/drm/intel/issues/666


Participating hosts (51 -> 43)
------------------------------

  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-kbl-7560u fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_8542 -> Patchwork_17785

  CI-20190529: 20190529
  CI_DRM_8542: 547d23ff9379bfa0f65c53721f63e688c5356b68 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5680: f7e3772175c53f0c910f4513831791cb5bdcab04 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17785: a44896d4d87446f7ff4ffe16e31b13e3de07b79d @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a44896d4d874 drm/i915: Add a new debugfs to request HDCP version
ee34d42eee76 drm/i915: Add support for considering HDCP ver requested via debugfs

== Logs ==

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

Re: [Intel-gfx] [RFC 2/2] drm/i915: Add a new debugfs to request HDCP version

[Jani Nikula]

On Wed, 27 May 2020, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
> As per the current HDCP design, the driver selects the highest
> version of HDCP that can be used to satisfy the content-protection
> requirements of the user. Due to this, the content-protection
> tests cannot test a lower version of HDCP, if the platform and the
> display panel, both support higher HDCP version.
>
> To provide some support for testing and debugging, a per-connector
> debugfs is required to set the HDCP version via debugfs that the
> kernel can consider, while enabling HDCP.
>
> This patch adds a new debugfs entry for each connector that supports
> HDCP. For enforcing a particular HDCP version for a connector, the user
> can write into the debugfs for that connector.
>
> To make design simple, the HDCP version request can only be made via
> debugfs, if there is no ongoing request for Content-Protection for
> the connector. The tests are expected to make sure that HDCP is disabled
> before making HDCP version request via the debugfs for the connector.
> Otherwise, the write request to the debugfs will be failed.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 98 +++++++++++++++++++
>  1 file changed, 98 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 70525623bcdf..e65abca1a1fa 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -2185,6 +2185,102 @@ static const struct file_operations i915_dsc_fec_support_fops = {
>  	.write = i915_dsc_fec_support_write
>  };
>  
> +static int i915_hdcp_ver_request_show(struct seq_file *m, void *data)
> +{
> +
> +	struct drm_connector *connector = m->private;
> +	struct intel_connector *intel_connector = to_intel_connector(connector);
> +	u64 hdcp_ver_flag;

u64 seems a little excessive for something that needs 2 bits.

> +
> +	if (connector->status != connector_status_connected)
> +		return -ENODEV;
> +
> +	/* HDCP is supported by connector */
> +	if (!intel_connector->hdcp.shim)
> +		return -EINVAL;

Why do you need to check these? The version request is valid regardless
of connection or hdcp, no?

> +
> +	hdcp_ver_flag = intel_connector->hdcp.debugfs_ver_request;
> +	seq_printf(m, "HDCP_VER_FLAGS: %llu\n", hdcp_ver_flag);
> +
> +	seq_printf(m, "Requested Versions:\n");
> +	if (hdcp_ver_flag & HDCP_VERSION_1_4)
> +		seq_printf(m, "HDCP1.4\n");
> +	if (hdcp_ver_flag & HDCP_VERSION_2_2)
> +		seq_printf(m, "HDCP2.2\n");

Why do you need to print duplicated information? One or the other, not
both. Simplify, don't complicate.

> +
> +	return 0;
> +}
> +
> +static int i915_hdcp_ver_request_open(struct inode *inode,
> +				     struct file *file)
> +{
> +	return single_open(file, i915_hdcp_ver_request_show,
> +			   inode->i_private);
> +}
> +
> +static int intel_hdcp_debugfs_ver_set(struct intel_connector *connector, u64 val)
> +{
> +	struct intel_hdcp *hdcp = &connector->hdcp;
> +
> +	if (!hdcp->shim || val > HDCP_VERSION_MASK)
> +		return -EINVAL;
> +
> +	if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
> +		return -EINVAL;

What does it matter if you can request the version independent of what's
currently going on? I think it's just extra code that can go wrong here.

> +
> +	hdcp->debugfs_ver_request = val;

Usually there's a blank line before the return.

> +	return 0;
> +}

Perhaps even the helper is excessive here.

> +
> +static ssize_t i915_hdcp_ver_request_write(struct file *file,
> +					  const char __user *ubuf,
> +					  size_t len, loff_t *offp)
> +{
> +	unsigned int hdcp_ver = 0;
> +	int ret;
> +	struct drm_connector *connector =
> +		((struct seq_file *)file->private_data)->private;
> +	struct intel_connector *intel_con = to_intel_connector(connector);

It's *never* intel_con. It's either intel_connector or just connector.

> +	struct drm_i915_private *i915 = to_i915(connector->dev);
> +	char tmp[16];
> +
> +	if (len == 0)
> +		return 0;
> +
> +	if (len >= sizeof(tmp))
> +		return -EINVAL;
> +
> +	if (copy_from_user(tmp, ubuf, len))
> +		return -EFAULT;
> +
> +	tmp[len] = '\0';
> +
> +
> +	drm_dbg(&i915->drm,
> +		"Copied %zu bytes from user to request hdcp ver\n", len);
> +
> +	ret = kstrtouint(tmp, 10, &hdcp_ver);
> +	if (ret < 0)
> +		return ret;

Replace most of the above with val = kstrtouint_from_user(...).

> +
> +	drm_dbg(&i915->drm, "Got %u for HDCP version\n", hdcp_ver);

Useless.

> +	ret = intel_hdcp_debugfs_ver_set(intel_con, hdcp_ver);
> +	if (ret < 0)
> +		return ret;
> +
> +	*offp += len;

Usually there's a blank line before return.

> +	return len;
> +}
> +
> +static const struct file_operations i915_hdcp_ver_request_fops = {
> +	.owner = THIS_MODULE,
> +	.open = i915_hdcp_ver_request_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +	.write = i915_hdcp_ver_request_write
> +};
> +
>  /**
>   * intel_connector_debugfs_add - add i915 specific connector debugfs files
>   * @connector: pointer to a registered drm_connector
> @@ -2215,6 +2311,8 @@ int intel_connector_debugfs_add(struct drm_connector *connector)
>  	    connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
>  		debugfs_create_file("i915_hdcp_sink_capability", S_IRUGO, root,
>  				    connector, &i915_hdcp_sink_capability_fops);
> +		debugfs_create_file("i915_hdcp_version_request", S_IRUGO, root,
> +				    connector, &i915_hdcp_ver_request_fops);
>  	}
>  
>  	if (INTEL_GEN(dev_priv) >= 10 &&

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 1/2] drm/i915: Add support for considering HDCP ver requested via debugfs

[Jani Nikula]

On Wed, 27 May 2020, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
> For testing and debugging each HDCP version separately, a debugfs
> entry for requesting a specific version is required. The vesion
> requested via debugfs needs to be stored in hdcp structure. This can
> then be considered while enabling HDCP, provided the platform and the
> display supports the requested version.
>
> This patch adds the support for storing the version requested as a 32bit
> flag. The default value is set to signify that any of the HDCP version
> can be used.
>
> If a specific HDCP version is requested through the debugfs, the driver
> chooses that version, instead of policy of choosing the highest HDCP
> version supported.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h | 10 ++++++++++
>  drivers/gpu/drm/i915/display/intel_hdcp.c          |  8 ++++++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 9488449e4b94..cfa641c70717 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -408,6 +408,16 @@ struct intel_hdcp {
>  	 * Hence caching the transcoder here.
>  	 */
>  	enum transcoder cpu_transcoder;
> +
> +	/*
> +	 * HDCP version requested from debugfs i915_hdcp_ver_request.
> +	 * Kernel will read these bits and entertain the request, as per
> +	 * the HDCP capability of the panel and platform.
> +	 */
> +#define HDCP_VERSION_1_4	0x01
> +#define HDCP_VERSION_2_2	0x02
> +#define HDCP_VERSION_MASK	0x03
> +	u32 debugfs_ver_request;
>  };
>  
>  struct intel_connector {
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index 2cbc4619b4ce..951401046804 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -1977,6 +1977,8 @@ int intel_hdcp_init(struct intel_connector *connector,
>  	if (!shim)
>  		return -EINVAL;
>  
> +	hdcp->debugfs_ver_request = HDCP_VERSION_MASK;

It's almost always better to keep 0 as the default or initial value. If
the member is non-zero, you do as requested.

> +
>  	if (is_hdcp2_supported(dev_priv))
>  		intel_hdcp2_init(connector, shim);
>  
> @@ -2023,7 +2025,8 @@ int intel_hdcp_enable(struct intel_connector *connector,
>  	 * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
>  	 * is capable of HDCP2.2, it is preferred to use HDCP2.2.
>  	 */
> -	if (intel_hdcp2_capable(connector)) {
> +	if ((hdcp->debugfs_ver_request & HDCP_VERSION_2_2) &&
> +	    intel_hdcp2_capable(connector)) {
>  		ret = _intel_hdcp2_enable(connector);
>  		if (!ret)
>  			check_link_interval = DRM_HDCP2_CHECK_PERIOD_MS;
> @@ -2033,7 +2036,8 @@ int intel_hdcp_enable(struct intel_connector *connector,
>  	 * When HDCP2.2 fails and Content Type is not Type1, HDCP1.4 will
>  	 * be attempted.
>  	 */
> -	if (ret && intel_hdcp_capable(connector) &&
> +	if (ret && (hdcp->debugfs_ver_request & HDCP_VERSION_1_4) &&
> +	    intel_hdcp_capable(connector) &&
>  	    hdcp->content_type != DRM_MODE_HDCP_CONTENT_TYPE1) {
>  		ret = _intel_hdcp_enable(connector);
>  	}

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.IGT: failure for i915: Add debugfs for requesting HDCP version

[Patchwork]

== Series Details ==

Series: i915: Add debugfs for requesting HDCP version
URL   : https://patchwork.freedesktop.org/series/77693/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8542_full -> Patchwork_17785_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_17785_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_17785_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_17785_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_render_copy@yf-tiled-ccs-to-y-tiled-ccs:
    - shard-glk:          NOTRUN -> [TIMEOUT][1] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-glk1/igt@gem_render_copy@yf-tiled-ccs-to-y-tiled-ccs.html

  * igt@runner@aborted:
    - shard-hsw:          NOTRUN -> [FAIL][2]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-hsw8/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@system-suspend-modeset:
    - shard-skl:          [PASS][3] -> [INCOMPLETE][4] ([i915#151] / [i915#69])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl2/igt@i915_pm_rpm@system-suspend-modeset.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl2/igt@i915_pm_rpm@system-suspend-modeset.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [PASS][5] -> [DMESG-WARN][6] ([i915#180]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-apl8/igt@i915_suspend@sysfs-reader.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-apl6/igt@i915_suspend@sysfs-reader.html

  * igt@kms_cursor_crc@pipe-c-cursor-64x64-offscreen:
    - shard-hsw:          [PASS][7] -> [DMESG-WARN][8] ([i915#1927])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-hsw6/igt@kms_cursor_crc@pipe-c-cursor-64x64-offscreen.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-hsw8/igt@kms_cursor_crc@pipe-c-cursor-64x64-offscreen.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [PASS][9] -> [DMESG-WARN][10] ([i915#180]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-kbl3/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-kbl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_frontbuffer_tracking@psr-suspend:
    - shard-skl:          [PASS][11] -> [INCOMPLETE][12] ([i915#123] / [i915#69])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl8/igt@kms_frontbuffer_tracking@psr-suspend.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl7/igt@kms_frontbuffer_tracking@psr-suspend.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([i915#1188])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl4/igt@kms_hdr@bpc-switch-dpms.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane@plane-panning-bottom-right-pipe-b-planes:
    - shard-skl:          [PASS][15] -> [FAIL][16] ([i915#1036])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl2/igt@kms_plane@plane-panning-bottom-right-pipe-b-planes.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl2/igt@kms_plane@plane-panning-bottom-right-pipe-b-planes.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-kbl:          [PASS][17] -> [DMESG-WARN][18] ([i915#180] / [i915#93] / [i915#95])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([fdo#108145] / [i915#265])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@psr2_sprite_mmap_cpu:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109441])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_cpu.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-iclb4/igt@kms_psr@psr2_sprite_mmap_cpu.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][23] -> [FAIL][24] ([i915#31])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-apl1/igt@kms_setmode@basic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-apl7/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_ctx_persistence@legacy-engines-mixed-process@bsd:
    - shard-skl:          [FAIL][25] ([i915#1528]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl9/igt@gem_ctx_persistence@legacy-engines-mixed-process@bsd.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl5/igt@gem_ctx_persistence@legacy-engines-mixed-process@bsd.html

  * {igt@gem_exec_reloc@basic-concurrent0}:
    - shard-glk:          [FAIL][27] ([i915#1930]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-glk1/igt@gem_exec_reloc@basic-concurrent0.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-glk9/igt@gem_exec_reloc@basic-concurrent0.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-skl:          [FAIL][29] ([i915#644]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl1/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl1/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-apl:          [DMESG-WARN][31] ([i915#1436] / [i915#716]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-apl6/igt@gen9_exec_parse@allowed-all.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-apl3/igt@gen9_exec_parse@allowed-all.html

  * {igt@i915_selftest@perf@request}:
    - shard-tglb:         [INCOMPLETE][33] ([i915#1823]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-tglb5/igt@i915_selftest@perf@request.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-tglb6/igt@i915_selftest@perf@request.html

  * igt@i915_suspend@forcewake:
    - shard-skl:          [INCOMPLETE][35] ([i915#69]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl4/igt@i915_suspend@forcewake.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl3/igt@i915_suspend@forcewake.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-180:
    - shard-glk:          [FAIL][37] ([i915#1119] / [i915#118] / [i915#95]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-glk8/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-glk6/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html

  * {igt@kms_flip@flip-vs-suspend@c-dp1}:
    - shard-kbl:          [DMESG-WARN][39] ([i915#180]) -> [PASS][40] +4 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-kbl7/igt@kms_flip@flip-vs-suspend@c-dp1.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-kbl4/igt@kms_flip@flip-vs-suspend@c-dp1.html

  * igt@kms_hdr@bpc-switch:
    - shard-skl:          [FAIL][41] ([i915#1188]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl5/igt@kms_hdr@bpc-switch.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl3/igt@kms_hdr@bpc-switch.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
    - shard-skl:          [FAIL][43] ([i915#53]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl4/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl7/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][45] ([i915#180]) -> [PASS][46] +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][47] ([fdo#108145] / [i915#265]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][49] ([fdo#109642] / [fdo#111068]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-iclb7/igt@kms_psr2_su@frontbuffer.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [SKIP][51] ([fdo#109441]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-iclb7/igt@kms_psr@psr2_no_drrs.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-iclb2/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][53] ([i915#31]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-kbl2/igt@kms_setmode@basic.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-kbl6/igt@kms_setmode@basic.html

  * igt@perf@short-reads:
    - shard-skl:          [FAIL][55] ([i915#51]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl7/igt@perf@short-reads.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl7/igt@perf@short-reads.html

  * {igt@sysfs_heartbeat_interval@mixed@rcs0}:
    - shard-skl:          [FAIL][57] ([i915#1731]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-skl8/igt@sysfs_heartbeat_interval@mixed@rcs0.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-skl4/igt@sysfs_heartbeat_interval@mixed@rcs0.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][59] ([i915#658]) -> [SKIP][60] ([i915#588])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-iclb7/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-tglb:         [FAIL][61] ([i915#454]) -> [SKIP][62] ([i915#468])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-tglb7/igt@i915_pm_dc@dc6-psr.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-tglb2/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_content_protection@atomic-dpms:
    - shard-apl:          [FAIL][63] ([fdo#110321] / [fdo#110336] / [i915#95]) -> [TIMEOUT][64] ([i915#1319])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-apl4/igt@kms_content_protection@atomic-dpms.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-apl4/igt@kms_content_protection@atomic-dpms.html

  * igt@kms_content_protection@lic:
    - shard-apl:          [FAIL][65] ([fdo#110321]) -> [TIMEOUT][66] ([i915#1319] / [i915#1635])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-apl6/igt@kms_content_protection@lic.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-apl3/igt@kms_content_protection@lic.html

  * igt@kms_cursor_legacy@cursora-vs-flipb-toggle:
    - shard-glk:          [DMESG-WARN][67] ([i915#1926]) -> [DMESG-FAIL][68] ([i915#1925])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-glk9/igt@kms_cursor_legacy@cursora-vs-flipb-toggle.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-glk4/igt@kms_cursor_legacy@cursora-vs-flipb-toggle.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [FAIL][69] ([i915#608]) -> [SKIP][70] ([fdo#109642] / [fdo#111068])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8542/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17785/shard-iclb3/igt@kms_psr2_su@page_flip.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [i915#1036]: https://gitlab.freedesktop.org/drm/intel/issues/1036
  [i915#1119]: https://gitlab.freedesktop.org/drm/intel/issues/1119
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#123]: https://gitlab.freedesktop.org/drm/intel/issues/123
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#151]: https://gitlab.freedesktop.org/drm/intel/issues/151
  [i915#1528]: https://gitlab.freedesktop.org/drm/intel/issues/1528
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#1731]: https://gitlab.freedesktop.org/drm/intel/issues/1731
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1823]: https://gitlab.freedesktop.org/drm/intel/issues/1823
  [i915#1925]: https://gitlab.freedesktop.org/drm/intel/issues/1925
  [i915#1926]: https://gitlab.freedesktop.org/drm/intel/issues/1926
  [i915#1927]: https://gitlab.freedesktop.org/drm/intel/issues/1927
  [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#51]: https://gitlab.freedesktop.org/drm/intel/issues/51
  [i915#53]: https://gitlab.freedesktop.org/drm/intel/issues/53
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#608]: https://gitlab.freedesktop.org/drm/intel/issues/608
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


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

  * Linux: CI_DRM_8542 -> Patchwork_17785

  CI-20190529: 20190529
  CI_DRM_8542: 547d23ff9379bfa0f65c53721f63e688c5356b68 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5680: f7e3772175c53f0c910f4513831791cb5bdcab04 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17785: a44896d4d87446f7ff4ffe16e31b13e3de07b79d @ 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_17785/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 2/2] drm/i915: Add a new debugfs to request HDCP version

[Nautiyal, Ankit K]

Hi Jani,

Thanks for the comments and suggestions. Please find my response inline.

On 5/27/2020 7:44 PM, Jani Nikula wrote:
> On Wed, 27 May 2020, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>> As per the current HDCP design, the driver selects the highest
>> version of HDCP that can be used to satisfy the content-protection
>> requirements of the user. Due to this, the content-protection
>> tests cannot test a lower version of HDCP, if the platform and the
>> display panel, both support higher HDCP version.
>>
>> To provide some support for testing and debugging, a per-connector
>> debugfs is required to set the HDCP version via debugfs that the
>> kernel can consider, while enabling HDCP.
>>
>> This patch adds a new debugfs entry for each connector that supports
>> HDCP. For enforcing a particular HDCP version for a connector, the user
>> can write into the debugfs for that connector.
>>
>> To make design simple, the HDCP version request can only be made via
>> debugfs, if there is no ongoing request for Content-Protection for
>> the connector. The tests are expected to make sure that HDCP is disabled
>> before making HDCP version request via the debugfs for the connector.
>> Otherwise, the write request to the debugfs will be failed.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   .../drm/i915/display/intel_display_debugfs.c  | 98 +++++++++++++++++++
>>   1 file changed, 98 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> index 70525623bcdf..e65abca1a1fa 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> @@ -2185,6 +2185,102 @@ static const struct file_operations i915_dsc_fec_support_fops = {
>>        .write = i915_dsc_fec_support_write
>>   };
>>
>> +static int i915_hdcp_ver_request_show(struct seq_file *m, void *data)
>> +{
>> +
>> +     struct drm_connector *connector = m->private;
>> +     struct intel_connector *intel_connector = to_intel_connector(connector);
>> +     u64 hdcp_ver_flag;
> u64 seems a little excessive for something that needs 2 bits.
Agreed. Will change this in next version.
>> +
>> +     if (connector->status != connector_status_connected)
>> +             return -ENODEV;
>> +
>> +     /* HDCP is supported by connector */
>> +     if (!intel_connector->hdcp.shim)
>> +             return -EINVAL;
> Why do you need to check these? The version request is valid regardless
> of connection or hdcp, no?
Hmm, for connectors that are unconnected or do not support hdcp, the 
member `hdcp` will not have any useful value.
The `debugfs_ver_request` is initialized to dafault value only if 
hdcp.shim exists.
It might show perhaps incorrect flag.

>> +
>> +     hdcp_ver_flag = intel_connector->hdcp.debugfs_ver_request;
>> +     seq_printf(m, "HDCP_VER_FLAGS: %llu\n", hdcp_ver_flag);
>> +
>> +     seq_printf(m, "Requested Versions:\n");
>> +     if (hdcp_ver_flag & HDCP_VERSION_1_4)
>> +             seq_printf(m, "HDCP1.4\n");
>> +     if (hdcp_ver_flag & HDCP_VERSION_2_2)
>> +             seq_printf(m, "HDCP2.2\n");
> Why do you need to print duplicated information? One or the other, not
> both. Simplify, don't complicate.

Alright, I will just keep the print with HDCP_VER_FLAGS, that should be 
sufficient.

>> +
>> +     return 0;
>> +}
>> +
>> +static int i915_hdcp_ver_request_open(struct inode *inode,
>> +                                  struct file *file)
>> +{
>> +     return single_open(file, i915_hdcp_ver_request_show,
>> +                        inode->i_private);
>> +}
>> +
>> +static int intel_hdcp_debugfs_ver_set(struct intel_connector *connector, u64 val)
>> +{
>> +     struct intel_hdcp *hdcp = &connector->hdcp;
>> +
>> +     if (!hdcp->shim || val > HDCP_VERSION_MASK)
>> +             return -EINVAL;
>> +
>> +     if (hdcp->value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED)
>> +             return -EINVAL;
> What does it matter if you can request the version independent of what's
> currently going on? I think it's just extra code that can go wrong here.
I was a little skeptical about the behavior if the authentication is 
getting retried, and in between
there is a change in the requested version. But again since this is a 
debug environment, the
test should be knowing what it is doing. I can do away with this code.

>> +
>> +     hdcp->debugfs_ver_request = val;
> Usually there's a blank line before the return.

Noted. Will take care in the next version.
>> +     return 0;
>> +}
> Perhaps even the helper is excessive here.

Alright, will get rid of this in next version.

>> +
>> +static ssize_t i915_hdcp_ver_request_write(struct file *file,
>> +                                       const char __user *ubuf,
>> +                                       size_t len, loff_t *offp)
>> +{
>> +     unsigned int hdcp_ver = 0;
>> +     int ret;
>> +     struct drm_connector *connector =
>> +             ((struct seq_file *)file->private_data)->private;
>> +     struct intel_connector *intel_con = to_intel_connector(connector);
> It's *never* intel_con. It's either intel_connector or just connector.
Noted. I think I was trying to squeeze into 80 chars.
I will conform to the existing norms for better readability.

>> +     struct drm_i915_private *i915 = to_i915(connector->dev);
>> +     char tmp[16];
>> +
>> +     if (len == 0)
>> +             return 0;
>> +
>> +     if (len >= sizeof(tmp))
>> +             return -EINVAL;
>> +
>> +     if (copy_from_user(tmp, ubuf, len))
>> +             return -EFAULT;
>> +
>> +     tmp[len] = '\0';
>> +
>> +
>> +     drm_dbg(&i915->drm,
>> +             "Copied %zu bytes from user to request hdcp ver\n", len);
>> +
>> +     ret = kstrtouint(tmp, 10, &hdcp_ver);
>> +     if (ret < 0)
>> +             return ret;
> Replace most of the above with val = kstrtouint_from_user(...).
Will use kstrtouint_from_user(...) to avoid copying and read directly 
from user buffer instead.

>> +
>> +     drm_dbg(&i915->drm, "Got %u for HDCP version\n", hdcp_ver);
> Useless.

I thought it was good to have a debug print.
But the same can be seen by reading the debugfs, so we can do away with it.

>> +     ret = intel_hdcp_debugfs_ver_set(intel_con, hdcp_ver);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     *offp += len;
> Usually there's a blank line before return.

Will add the blank line here.

Thanks,
Ankit

>> +     return len;
>> +}
>> +
>> +static const struct file_operations i915_hdcp_ver_request_fops = {
>> +     .owner = THIS_MODULE,
>> +     .open = i915_hdcp_ver_request_open,
>> +     .read = seq_read,
>> +     .llseek = seq_lseek,
>> +     .release = single_release,
>> +     .write = i915_hdcp_ver_request_write
>> +};
>> +
>>   /**
>>    * intel_connector_debugfs_add - add i915 specific connector debugfs files
>>    * @connector: pointer to a registered drm_connector
>> @@ -2215,6 +2311,8 @@ int intel_connector_debugfs_add(struct drm_connector *connector)
>>            connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
>>                debugfs_create_file("i915_hdcp_sink_capability", S_IRUGO, root,
>>                                    connector, &i915_hdcp_sink_capability_fops);
>> +             debugfs_create_file("i915_hdcp_version_request", S_IRUGO, root,
>> +                                 connector, &i915_hdcp_ver_request_fops);
>>        }
>>
>>        if (INTEL_GEN(dev_priv) >= 10 &&
> --
> Jani Nikula, Intel Open Source Graphics Center

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

Re: [Intel-gfx] [RFC 1/2] drm/i915: Add support for considering HDCP ver requested via debugfs

[Nautiyal, Ankit K]

On 5/27/2020 7:48 PM, Jani Nikula wrote:
> On Wed, 27 May 2020, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>> For testing and debugging each HDCP version separately, a debugfs
>> entry for requesting a specific version is required. The vesion
>> requested via debugfs needs to be stored in hdcp structure. This can
>> then be considered while enabling HDCP, provided the platform and the
>> display supports the requested version.
>>
>> This patch adds the support for storing the version requested as a 32bit
>> flag. The default value is set to signify that any of the HDCP version
>> can be used.
>>
>> If a specific HDCP version is requested through the debugfs, the driver
>> chooses that version, instead of policy of choosing the highest HDCP
>> version supported.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display_types.h | 10 ++++++++++
>>   drivers/gpu/drm/i915/display/intel_hdcp.c          |  8 ++++++--
>>   2 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index 9488449e4b94..cfa641c70717 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -408,6 +408,16 @@ struct intel_hdcp {
>>         * Hence caching the transcoder here.
>>         */
>>        enum transcoder cpu_transcoder;
>> +
>> +     /*
>> +      * HDCP version requested from debugfs i915_hdcp_ver_request.
>> +      * Kernel will read these bits and entertain the request, as per
>> +      * the HDCP capability of the panel and platform.
>> +      */
>> +#define HDCP_VERSION_1_4     0x01
>> +#define HDCP_VERSION_2_2     0x02
>> +#define HDCP_VERSION_MASK    0x03
>> +     u32 debugfs_ver_request;
>>   };
>>
>>   struct intel_connector {
>> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> index 2cbc4619b4ce..951401046804 100644
>> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> @@ -1977,6 +1977,8 @@ int intel_hdcp_init(struct intel_connector *connector,
>>        if (!shim)
>>                return -EINVAL;
>>
>> +     hdcp->debugfs_ver_request = HDCP_VERSION_MASK;
> It's almost always better to keep 0 as the default or initial value. If
> the member is non-zero, you do as requested.

I was thinking if we can have a request for "neither of the versions" to 
be represented as 0.
But that seems to be silly now, without any actual use case, when I look 
closely.

I will have this set to 0 by default and only use the member if non-zero.

Thanks,
Ankit

>
>> +
>>        if (is_hdcp2_supported(dev_priv))
>>                intel_hdcp2_init(connector, shim);
>>
>> @@ -2023,7 +2025,8 @@ int intel_hdcp_enable(struct intel_connector *connector,
>>         * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup
>>         * is capable of HDCP2.2, it is preferred to use HDCP2.2.
>>         */
>> -     if (intel_hdcp2_capable(connector)) {
>> +     if ((hdcp->debugfs_ver_request & HDCP_VERSION_2_2) &&
>> +         intel_hdcp2_capable(connector)) {
>>                ret = _intel_hdcp2_enable(connector);
>>                if (!ret)
>>                        check_link_interval = DRM_HDCP2_CHECK_PERIOD_MS;
>> @@ -2033,7 +2036,8 @@ int intel_hdcp_enable(struct intel_connector *connector,
>>         * When HDCP2.2 fails and Content Type is not Type1, HDCP1.4 will
>>         * be attempted.
>>         */
>> -     if (ret && intel_hdcp_capable(connector) &&
>> +     if (ret && (hdcp->debugfs_ver_request & HDCP_VERSION_1_4) &&
>> +         intel_hdcp_capable(connector) &&
>>            hdcp->content_type != DRM_MODE_HDCP_CONTENT_TYPE1) {
>>                ret = _intel_hdcp_enable(connector);
>>        }
> --
> Jani Nikula, Intel Open Source Graphics Center

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