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