[Intel-gfx] [RFC v1 01/20] drm/hdcp: HDCP bitmask property for connectors

[Ramalingam C]

On Friday 14 July 2017 12:32 AM, Daniel Vetter wrote:
> On Thu, Jul 13, 2017 at 06:29:33PM +0530, Ramalingam C wrote:
>>
>> On Thursday 13 July 2017 04:06 PM, Daniel Vetter wrote:
>>> On Thu, Jul 13, 2017 at 12:15 PM, Ramalingam C <ramalingam.c at intel.com> wrote:
>>>> On Thursday 13 July 2017 02:15 PM, Daniel Vetter wrote:
>>>>
>>>> On Thu, Jul 13, 2017 at 8:54 AM, Ramalingam C <ramalingam.c at intel.com>
>>>> wrote:
>>>>
>>>> On Thursday 13 July 2017 11:39 AM, Daniel Vetter wrote:
>>>>
>>>> On Wed, Jul 12, 2017 at 9:10 PM, Sean Paul <seanpaul at chromium.org> wrote:
>>>>
>>>> Why all these intermediate steps and different failure modes? Either hdcp
>>>> works, or it doesnt (and we can split up with the type 0 or type 1 if
>>>> needed), but I don't know what userspace would do with all the other
>>>> stuff?
>>>>
>>>> enum values  HDCP_ENABLE, HDCP_ENABLE_TYPE1 and HDCP_DISABLE along with
>>>> kobj-uevent
>>>> for HDCP state change, could do the bare minimal HDCP1.4 and HDCP2.2
>>>> configuration.
>>>>
>>>> And without Type info it is not possible for HDCP2.2.
>>>>
>>>> I've had requests from chrome team to expose HDCP version, so I don't think
>>>> this
>>>> is too contentious.
>>>>
>>>> I think it'd still be easier if we start out with the current content
>>>> protection props that CrOS is using, and then figure out how to layer
>>>> the exact version/standard on top? One thing at a time and all that.
>>>> -Daniel
>>>>
>>>> I understand the approach.
>>>>
>>>> But Only problem is current upstreaming effort is for HDCP2.2 support at DRM
>>>> with a design which can
>>>> easily accommodate other versions too. So we  need to stretch current CrOS
>>>> property a bit with
>>>> ENABLE_TYPE1 and UNSUPPORTED etc. Hope that should be fine for all.
>>>>
>>>> Yeah, but if we just go with enable (without specifying the type) we
>>>> could still enable the highest hdcp level (so 2.2 for our case). At
>>>> least I don't see a reason why we need to already have the
>>>> enable_type1 thing. Can you pls explain why you think this is
>>>> necessary?
>>>>
>>>> There seems to be a need to force type1, but I think it's easier to do
>>>> that as an extension. Of course we need to keep it in mind meanwhile.
>>>>
>>>> Background for this need of Type info in HDCP2.2 implementation is as
>>>> follows:
>>> Aside: You're quoting is broken for inline quoting. Either fix the
>>> quoting or top-quote (there's no difference between your text and
>>> mine, mine should be indented with > or | or similar).
>> Sorry for the inconvenience. Hope now it is fine. Just reset the settings on
>> thunderbird
> Yes, looks good now.
>
>>>> HDCP2.2 Spec classify the protected content as Type 0 and Type 1. For
>>>> Example lets say
>>>> - A HDCP2.2 Src is connected to HDCP repeater
>>>> - that repeater is connected to a HDCP2.2 panel
>>>> - that same repeater is also connected to a HDCP1.4 panel.
>>>>
>>>> In this topology, as part of Repeater authentication:
>>>> - HDCP2.2 Source will mention the Content Type to HDCP2.2 Repeater
>>>> - Repeater can transmit this Type 1 content to HDCP2.2 compliant sink only
>>>> (which is HDCP 2.2 panel here).
>>>> - Repeater can transmit any type0 content to any other devices (like HDCP1.4
>>>> panel here).
>>>> - Device with no HDCP support will get Neither of Type 0 or Type 1.
>>>>
>>>> So if we implement HDCP2.2 with HDCP_ENABLE state alone there is no way for
>>>> Userspace
>>>> to request for HDCP2.2 protection only. In this case we wont know the
>>>> content type classification.
>>> Yes, that is the case, but also the point of gradual enabling. Atm
>>> (with the current CrOS usersapce) userspace can ask for "pls give me
>>> content protection, I don't care what level/type". That itself is
>>> already useful, and a good step forward. Allowing to ask for a
>>> specific type is something on top.
>> Ok. When i think over it, that sounds as a  good idea to go gradually for
>> enabling HDCP2.2.
>>>> Even if we force Content type to Type1, in above topology Type 0 content
>>>> that could be rendered to
>>>> HDCP1.4 compliant panel wont be rendered as that has been forcibly
>>>> classified as Type 1 by KMD.
>>> Why? HDCP_ENABLE would just give you the "best" HDCP, so we'd fall
>>> back to type 0 (if that's available).
>> I think i misinterpreted. We could enable the HDCP2.2(if supported on panel)
>> for the Type 0 content. No issue on that
>>>> Forcing type 1 content to Type 0 will break the association of type1 content
>>>> to HDCP2.2 devices only.
>>> I didn't propose to force type1 everywhere. Why do you think this is needed.
>>>> More than that Devices with our indented DRM HDCP2.2 support wont pass the
>>>> HDCP2.2 compliance.
>>>> Considering we could extend the CrOS Userspace for HDCP2.2, I would prefer
>>>> to go ahead with
>>>> HDCP_ENABLE_TYPE1 along with HDCP_ENABLE.
>>> Yes, it's only hdcp1.4, and getting to full hdcp2.2 will take more
>>> work. You can do all of that in one go, but my experience with
>>> upstreaming new uabi is that usually that's not the most effective way
>>> to go about things. But in the end, that's your choice.
>> Agreed. We will go gradually about enabling HDCP2.2.
>>      1. Enable HDCP2.2 for HDCP_ENABLE with Content type as 0.
>>      2. Enable HDCP2.2 for Type 1 content with Enum value HDCP_ENABLE_TYPE1
>>      3. Making HDCP2.2 support as HDCP2.2 spec compliant.
>>
>> But I think i will just add another enum value HDCP_UNSUPPORTED to mark the
>> no HDCP supported on the setup.
>> I hope that is fine.
> Makes sense.
>
> btw I chatted with Shashi today, I think it'd be good if we could have a
> chat next week about some of the design patterns for the implementation. I
> can give you some pointers for kernel best practices (so you understand
> why things are done a bit different).
>
> But I think first priority is to get agreement on the new uapi and make
> sure it works for CrOS, we can polish the implementation in following
> rounds.
>
> Thanks, Daniel
Sure daniel. Now I have prepared v2 of this patch alone. Please have 
review that too.
Based on feedbacks I will spin other patches of this series.

--Ram