Re: [RFC v3] drm/hdcp: drm enum property for CP State

[Ramalingam C <ramalingam.c@intel.com>]

On Wednesday 26 July 2017 11:24 PM, Sean Paul wrote:
> On Wed, Jul 26, 2017 at 12:51 PM, C, Ramalingam <ramalingam.c@intel.com> wrote:
>>> -----Original Message-----
>>> From: Sean Paul [mailto:seanpaul@chromium.org]
>>> Sent: Wednesday, July 26, 2017 8:23 PM
>>> To: C, Ramalingam <ramalingam.c@intel.com>
>>> Cc: Sean Paul <seanpaul@chromium.org>; Vetter, Daniel
>>> <daniel.vetter@intel.com>; Intel Graphics Development <intel-
>>> gfx@lists.freedesktop.org>; dri-devel <dri-devel@lists.freedesktop.org>; Daniel
>>> Vetter <daniel@ffwll.ch>
>>> Subject: Re: [RFC v3] drm/hdcp: drm enum property for CP State
>>>
>>> On Wed, Jul 26, 2017 at 03:24:10PM +0530, Ramalingam C wrote:
>>>>
>>>> On Tuesday 25 July 2017 06:04 PM, Sean Paul wrote:
>>>>> On Mon, Jul 24, 2017 at 2:12 PM, Ramalingam C <ramalingam.c@intel.com>
>>> wrote:
>>>>>> DRM connector property is created to represent the content
>>>>>> protection state of the connector and to configure the same.
>>>>>>
>>>>>> Content protection states defined:
>>>>>>           DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED         -
>>> Unsupported
>>>>>>           DRM_MODE_CONTENT_PROTECTION_DISABLE             - Disabled
>>>>>>           DRM_MODE_CONTENT_PROTECTION_ENABLE              - Enabled
>>>>>>
>>>>>> v2: Redesigned the property to match with CP needs of CrOS [Sean].
>>>>>>
>>>>>> v3: Renamed the state names. Header is removed [sean].
>>>>>>
>>>>>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
>>>>>> ---
>>>>>>    drivers/gpu/drm/drm_connector.c | 14 ++++++++++++++
>>>>>>    include/drm/drm_mode_config.h   |  5 +++++
>>>>>>    include/uapi/drm/drm_mode.h     |  5 +++++
>>>>>>    3 files changed, 24 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/drm_connector.c
>>>>>> b/drivers/gpu/drm/drm_connector.c index 5cd61af..d6aaa08 100644
>>>>>> --- a/drivers/gpu/drm/drm_connector.c
>>>>>> +++ b/drivers/gpu/drm/drm_connector.c
>>>>>> @@ -617,6 +617,13 @@ static const struct drm_prop_enum_list
>>> drm_link_status_enum_list[] = {
>>>>>>    };
>>>>>>    DRM_ENUM_NAME_FN(drm_get_link_status_name,
>>>>>> drm_link_status_enum_list)
>>>>>>
>>>>>> +static const struct drm_prop_enum_list drm_cp_enum_list[] = {
>>>>>> +       { DRM_MODE_CONTENT_PROTECTION_UNSUPPORTED,
>>> "Unsupported" },
>>>>> You're still changing the enum names from the original patch/CrOS
>>>>> implementation.
>>>>>
>>>>> https://lists.freedesktop.org/archives/dri-devel/2014-December/07333
>>>>> 6.html
>>>>>
>>>>> https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/gpu/drm_d
>>>>> isplay.cc?l=27
>>>> Sean,
>>>>
>>>> I think we have bit of confusion here.
>>> Agreed :)
>>>
>>>> And in previous discussion you were fine with new state of property
>>>> that is "unsupported" - indicates no common HDCP version is supported
>>>> on HDCP src and Sink combo.
>>>>
>>>> when CP is not possible, if property exist, userspace will interpret
>>>> it as CP is supported and attempt for enabling.
>>>> I prefer indicating Unsupported state than failing such requests blindly.
>>>>
>>>> In that case to interpret the new state, we need to change CrOS User
>>>> space code.
>>>>
>>>> In the RFC you mentioned above, two version of uapi interfaces are
>>>> discussed.
>>>>          V1 uses single property to configure the CP and also for
>>>> status indication.
>>>>          V2 uses two properties one for configuring and another one for
>>>> status of Content protection.
>>>>
>>>> But CrOS user space is currently using the V1 interface.
>>>>
>>>> which will be preferred approach right now (V1/V2)?
>>>> In either way we need to change the CrOS :(
>>> We can't upstream code without a userspace implementation, so V1. Without
>>> any modifications.
>> Based on uAPI rules mentioned by Daniel, what I infer is that once a uAPI is in place,
>> usecase of uAPI cant be changed. It can be only be extended for future needs.
>>
>> So IMHO it is better to shape the userspace as per the uAPI
>> interfaces that community want to have for longer run. And mainly we should adapt to a
>> interface which can be conveniently extended for HDCP2.2 and future specs versions.
>>
>> So IMO it is worth seeing what it takes for CrOS Userspace changes for HDCP2.2 interfaces.
>> And In the interest of having HDCP2.2 in their stack sooner,  if the CrOS community or Google
>>   is interested in providing that userspace support, it will be smoother to put the
>> complete and preferred uAPI for HDCP2.2 and 1.4 in place.
>>
>> Sean, I think you will be better person to comment on this from both sides, CrOS and DRM.
>>
>>
>>> As has been mentioned before in this thread, it's a lot easier to upstream code
>>> that is proven to work, than it is to merge speculative API. Further, if this patch
>>> were merged, it would be CrOS which is currently the only consumer... not a
>>> good look :)
>> Not exactly. CrOS will be the first consumer, Android will follow soon after this.
>> We need to enable this feature on HwC for that purpose. Yup might take required time :)
>>
>>>
>>>> Lets say we need to modify CrOS user space, will there be any help for that?
>>>> I am not aware what it takes though.
>>> You could probably reach out to chromium-dev@chromium.org or the author of
>>> the Chrome hdcp implementation for help.
>> Thank you for the contact.
>> And before reaching out, it is better to conclude the HDCP1.4/2.2 uAPI interfaces that is agreeable for this community.
>>
>> In that perspective, can we consider and review the uAPI interface with two properties discussed for V2 of your earlier patches?
>> Complete use case is discussed in my previous mail.
>>
>> Based on userspace commitment for HDCP2.2, we can add blob property for SRM too.
>>
>>> Sean
>>>
>>>> I am trying to discuss both uapi versions of V1 and V2. I prefer V2 though.
>>>>
>>>> If V1 is preferred we need a single property as below "content
>>>> protection" property  with {"Unsupported", "Undesired", "Desired",
>>>> "Enabled"}
>>>>                          "Type1_desired" and "Type1_Enabled" are needed
>>>> for
>>>> HDCP2.2
>>>>
>>>>
>>>> If V2 is preferred we need two properties as below
>>>>
>>>> "content protection" property with {"Unsupported", "Undesired", "Desired"}
>>>>              ("Type1_desired" - needed For HDCP2.2) "content protection
>>>> status" property with {"Disabled", "Enabled"}
>>>>              ("Type1_enabled" - needed for HDCP2.2)
>> This uAPI interface looks fit to serve the all needs of HDCP1.4 and 2.2 as explained below.
>>
>> Sean and Daniel,
>>          I request you to share your views. Thanks.
> Just going to summarize what I mentioned on IRC for the benefit of the list.
>
> Any change to the original proposal will require a userspace
> implementation to go along with it. We currently only have one
> userspace implementation, which is Chrome. Chrome implements the
> original RFC (linked above). If you change anything from the original
> RFC, you no longer have a userspace implementation.
>
> In order to have a userspace implementation, you can choose to alter
> Chrome or another compositor.
>
> In order to change Chrome, I suspect you'll need to do one of the
> following to avoid breaking devices running old kernels with the RFC
> uapi:
> 1- Make the proposed uapi changes compatible with the original RFC
> 2- Change all CrOS kernels to the new proposal at the same time as
> userspace changes (this is actually possible with CrOS since it's all
> packaged/updated up together)
>
> In my opinion, option 1 is going to be the easiest path forward.
> Others have also suggested same. So before we get too far ahead of
> ourselves with HDCP 2.2, let's figure out how to upstream *something*
> with a userspace implementation. If you want to go for option 2,
> you'll need to do some legwork in Chrome beforehand.
>
> Sean
Based on given options, I will make proposed uapi changes compatible 
with with original RFC
to go with option 1.  I will send the updated patch in some time. Thanks 
Sean.

As per my off line discussion with Daniel, I working on using the DRM 
HDCP helper functions than
abstraction layer in DRM. Once that work is complete, I will re-spine 
the complete series for
review than phase by phase.

--Ram
>
>
>> --Ram
>
> <snip>
>
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>> --
>>> Sean Paul, Software Engineer, Google / Chromium OS

-- 
Regards,
--Ram.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel