[PATCH 2/7] drm/i915/hdcp: HDCP Capability for the downstream device
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Tue Jan 30 08:45:29 UTC 2024
On 1/27/2024 12:14 PM, Kandpal, Suraj wrote:
> =
>> On 1/24/2024 6:50 PM, Nautiyal, Ankit K wrote:
>>> On 1/12/2024 1:11 PM, Suraj Kandpal wrote:
>>>> Currently we are only checking capability of remote device and not
>>>> immediate downstream device but during capability check we need are
>>>> concerned with only the HDCP capability of downstream device.
>>>> During i915_display_info reporting we need HDCP Capability for both
>>>> the monitors and downstream branch device if any this patch adds that.
>>>
>>> I agree cases where MST hub/docker and sink are of different
>>> capabilities, this creates a confusion.
>>>
>>> with this change, perhaps kms_content_protection IGT can also be
>>> changed to check for MST hub's capability.
>>>
>>> Only thing is that for hdmi the 'remote_req' doesnt make sense.
>>>
>> Instead of changing the hdcp_2_2_capable can we just have a separate
>> function for intel_dp_remote_hdcp2_capable(), which uses aux =
>> &connector->port->aux.
> Yes I agree about the hdmi has a argument it wont use but I went with the lesser
> Of the two evils . If I went with the approach suggested the problem would be that
> debug fs calls intel_hdcp2_capable which has other checks for mei, gsc and If HW supports
> hdcp2 or not so all this would also have to be put in this function intel_dp_remote_hdcp2_capable()
> which I feel would create much more of a mess
>
> Regards,
> Suraj Kandpal
I see for HDCP1.4 also we need to have a separate functions if we want
to get correct HDCP capability for remote sinks.
I think lets not change existing hdcp_2_capable function. IMHO it would
be better to have another shim function for hdcp capabilities of remote
sink.
Regards,
Ankit
>> The common code for reading HDCP2_2 Rx caps can be pulled out in a
>> separate function, which we can call only in case of MST when we read
>> remote.
>>
>> Also we might need to have similar thing for HDCP1.4.
>>
>>
>> Regards,
>>
>> Ankit
>>
>>
>>>> Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
>>>> ---
>>>> .../drm/i915/display/intel_display_debugfs.c | 19
>>>> +++++++++++++++----
>>>> .../drm/i915/display/intel_display_types.h | 2 +-
>>>> drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 4 ++--
>>>> drivers/gpu/drm/i915/display/intel_hdcp.c | 6 +++---
>>>> drivers/gpu/drm/i915/display/intel_hdcp.h | 2 +-
>>>> drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
>>>> 6 files changed, 23 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>>>> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>>>> index d951edb36687..457f13357fad 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>>>> @@ -210,7 +210,8 @@ static void intel_panel_info(struct seq_file *m,
>>>> }
>>>> static void intel_hdcp_info(struct seq_file *m,
>>>> - struct intel_connector *intel_connector)
>>>> + struct intel_connector *intel_connector,
>>>> + bool remote_req)
>>>> {
>>>> bool hdcp_cap, hdcp2_cap;
>>>> @@ -220,7 +221,7 @@ static void intel_hdcp_info(struct seq_file *m,
>>>> }
>>>> hdcp_cap = intel_hdcp_capable(intel_connector);
>>>> - hdcp2_cap = intel_hdcp2_capable(intel_connector);
>>>> + hdcp2_cap = intel_hdcp2_capable(intel_connector, remote_req);
>>>> if (hdcp_cap)
>>>> seq_puts(m, "HDCP1.4 ");
>>>> @@ -307,7 +308,12 @@ static void intel_connector_info(struct seq_file
>>>> *m,
>>>> }
>>>> seq_puts(m, "\tHDCP version: ");
>>>> - intel_hdcp_info(m, intel_connector);
>>>> + intel_hdcp_info(m, intel_connector, true);
>>>> +
>>>> + if (intel_encoder_is_mst(encoder)) {
>>>> + seq_puts(m, "\tHDCP Branch Device version: ");
>>>> + intel_hdcp_info(m, intel_connector, false);
>>>> + }
>>>> seq_printf(m, "\tmax bpc: %u\n",
>>>> connector->display_info.bpc);
>>>> @@ -1153,7 +1159,12 @@ static int
>>>> i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
>>>> seq_printf(m, "%s:%d HDCP version: ", connector->base.name,
>>>> connector->base.base.id);
>>>> - intel_hdcp_info(m, connector);
>>>> + intel_hdcp_info(m, connector, true);
>>>> +
>>>> + if (intel_encoder_is_mst(connector->encoder)) {
>>>> + seq_puts(m, "\tHDCP Branch Device version: ");
>>>
>>> Perhaps MST HUB HDCP version?
>>>
>>>
>>>> + intel_hdcp_info(m, connector, false);
>>>> + }
>>>> out:
>>>> drm_modeset_unlock(&i915->drm.mode_config.connection_mutex);
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>>>> b/drivers/gpu/drm/i915/display/intel_display_types.h
>>>> index ae2e8cff9d69..aa559598f049 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>>>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>>>> @@ -507,7 +507,7 @@ struct intel_hdcp_shim {
>>>> /* Detects whether sink is HDCP2.2 capable */
>>>> int (*hdcp_2_2_capable)(struct intel_connector *connector,
>>>> - bool *capable);
>>>> + bool *capable, bool remote_req);
>>>> /* Write HDCP2.2 messages */
>>>> int (*write_2_2_msg)(struct intel_connector *connector, diff
>>>> --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
>>>> b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
>>>> index bec49061b2e1..90b027ba3302 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c
>>>> @@ -649,13 +649,13 @@ int intel_dp_hdcp2_check_link(struct
>>>> intel_digital_port *dig_port,
>>>> static
>>>> int intel_dp_hdcp2_capable(struct intel_connector *connector,
>>>> - bool *capable)
>>>> + bool *capable, bool remote_req)
>>>> {
>>>> struct drm_dp_aux *aux;
>>>> u8 rx_caps[3];
>>>> int ret;
>>>> - aux = intel_dp_hdcp_get_aux(connector, true);
>>>> + aux = intel_dp_hdcp_get_aux(connector, remote_req);
>>> Inline with the comments on the previous patch, this would also be
>>> required to change.
>>>
>>> Otherwise the patch looks good to me.
>>>
>>>
>>> Regards,
>>>
>>> Ankit
>>>
>>>> *capable = false;
>>>> ret = drm_dp_dpcd_read(aux,
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
>>>> b/drivers/gpu/drm/i915/display/intel_hdcp.c
>>>> index c3e692e7f790..b88a4713e6a8 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
>>>> @@ -161,7 +161,7 @@ bool intel_hdcp_capable(struct intel_connector
>>>> *connector)
>>>> }
>>>> /* Is HDCP2.2 capable on Platform and Sink */ -bool
>>>> intel_hdcp2_capable(struct intel_connector *connector)
>>>> +bool intel_hdcp2_capable(struct intel_connector *connector, bool
>>>> remote_req)
>>>> {
>>>> struct drm_i915_private *i915 = to_i915(connector->base.dev);
>>>> struct intel_hdcp *hdcp = &connector->hdcp; @@ -186,7 +186,7 @@
>>>> bool intel_hdcp2_capable(struct intel_connector
>>>> *connector)
>>>> mutex_unlock(&i915->display.hdcp.hdcp_mutex);
>>>> /* Sink's capability for HDCP2.2 */
>>>> - hdcp->shim->hdcp_2_2_capable(connector, &capable);
>>>> + hdcp->shim->hdcp_2_2_capable(connector, &capable, remote_req);
>>>> return capable;
>>>> }
>>>> @@ -2374,7 +2374,7 @@ static int _intel_hdcp_enable(struct
>>>> intel_atomic_state *state,
>>>> * 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 (intel_hdcp2_capable(connector, false)) {
>>>> ret = intel_hdcp_set_streams(dig_port, state);
>>>> if (!ret) {
>>>> ret = _intel_hdcp2_enable(connector); diff --git
>>>> a/drivers/gpu/drm/i915/display/intel_hdcp.h
>>>> b/drivers/gpu/drm/i915/display/intel_hdcp.h
>>>> index a9c784fd9ba5..72268e593cec 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_hdcp.h
>>>> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h
>>>> @@ -39,7 +39,7 @@ void intel_hdcp_update_pipe(struct
>>>> intel_atomic_state *state,
>>>> const struct drm_connector_state *conn_state);
>>>> bool is_hdcp_supported(struct drm_i915_private *i915, enum port
>>>> port);
>>>> bool intel_hdcp_capable(struct intel_connector *connector); -bool
>>>> intel_hdcp2_capable(struct intel_connector *connector);
>>>> +bool intel_hdcp2_capable(struct intel_connector *connector, bool
>>>> remote_req);
>>>> void intel_hdcp_component_init(struct drm_i915_private *i915);
>>>> void intel_hdcp_component_fini(struct drm_i915_private *i915);
>>>> void intel_hdcp_cleanup(struct intel_connector *connector); diff
>>>> --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
>>>> b/drivers/gpu/drm/i915/display/intel_hdmi.c
>>>> index 7020e5806109..d7feef05bc47 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>>>> @@ -1733,7 +1733,7 @@ int intel_hdmi_hdcp2_check_link(struct
>>>> intel_digital_port *dig_port,
>>>> static
>>>> int intel_hdmi_hdcp2_capable(struct intel_connector *connector,
>>>> - bool *capable)
>>>> + bool *capable, bool remote_req)
>>>
>>>> {
>>>> struct intel_digital_port *dig_port =
>>>> intel_attached_dig_port(connector);
>>>> u8 hdcp2_version;
More information about the Intel-gfx
mailing list