On Thu, 11 Jul 2019 11:20:49 +0530
Ramalingam C <ramalingam.c@xxxxxxxxx> wrote:
> On 2019-07-10 at 11:16:24 +0300, Pekka Paalanen wrote:
> > On Tue, 9 Jul 2019 18:17:59 +0530
> > Ramalingam C <ramalingam.c@xxxxxxxxx> wrote:
> >
> > > On 2019-07-09 at 17:31:10 +0300, Pekka Paalanen wrote:
> > > > On Mon, 8 Jul 2019 16:51:11 +0530
> > > > Ramalingam C <ramalingam.c@xxxxxxxxx> wrote:
> > > >
> > > > > This patch adds a DRM ENUM property to the selected connectors.
> > > > > This property is used for mentioning the protected content's type
> > > > > from userspace to kernel HDCP authentication.
> > > > >
> > > > > Type of the stream is decided by the protected content providers.
> > > > > Type 0 content can be rendered on any HDCP protected display wires.
> > > > > But Type 1 content can be rendered only on HDCP2.2 protected paths.
> > > > >
> > > > > So when a userspace sets this property to Type 1 and starts the HDCP
> > > > > enable, kernel will honour it only if HDCP2.2 authentication is through
> > > > > for type 1. Else HDCP enable will be failed.
> > > > >
> > > > > Need ACK for this new conenctor property from userspace consumer.
> > > > >
> > > > > v2:
> > > > > cp_content_type is replaced with content_protection_type [daniel]
> > > > > check at atomic_set_property is removed [Maarten]
> > > > > v3:
> > > > > %s/content_protection_type/hdcp_content_type [Pekka]
> > > > > v4:
> > > > > property is created for the first requested connector and then reused.
> > > > > [Danvet]
> > > > > v5:
> > > > > kernel doc nits addressed [Daniel]
> > > > > Rebased as part of patch reordering.
> > > > > v6:
> > > > > Kernel docs are modified [pekka]
> > > > > v7:
> > > > > More details in Kernel docs. [pekka]
> > > > >
> > > > > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx>
> > > > > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > > > > ---
> > > > > drivers/gpu/drm/drm_atomic_uapi.c | 4 +++
> > > > > drivers/gpu/drm/drm_connector.c | 39 +++++++++++++++++++++++
> > > > > drivers/gpu/drm/drm_hdcp.c | 36 ++++++++++++++++++++-
> > > > > drivers/gpu/drm/i915/display/intel_hdcp.c | 4 ++-
> > > > > include/drm/drm_connector.h | 7 ++++
> > > > > include/drm/drm_hdcp.h | 2 +-
> > > > > include/drm/drm_mode_config.h | 6 ++++
> > > > > include/uapi/drm/drm_mode.h | 4 +++
> > > > > 8 files changed, 99 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > index abe38bdf85ae..19ae119f1a5d 100644
> > > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > > > > @@ -747,6 +747,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> > > > > return -EINVAL;
> > > > > }
> > > > > state->content_protection = val;
> > > > > + } else if (property == config->hdcp_content_type_property) {
> > > > > + state->hdcp_content_type = val;
> > > > > } else if (property == connector->colorspace_property) {
> > > > > state->colorspace = val;
> > > > > } else if (property == config->writeback_fb_id_property) {
> > > > > @@ -831,6 +833,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> > > > > state->hdr_output_metadata->base.id : 0;
> > > > > } else if (property == config->content_protection_property) {
> > > > > *val = state->content_protection;
> > > > > + } else if (property == config->hdcp_content_type_property) {
> > > > > + *val = state->hdcp_content_type;
> > > > > } else if (property == config->writeback_fb_id_property) {
> > > > > /* Writeback framebuffer is one-shot, write and forget */
> > > > > *val = 0;
> > > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > > > index 068d4b05f1be..732f6645643d 100644
> > > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > > @@ -952,6 +952,45 @@ static const struct drm_prop_enum_list hdmi_colorspaces[] = {
> > > > > * is no longer protected and userspace should take appropriate action
> > > > > * (whatever that might be).
> > > > > *
> > > > > + * HDCP Content Type:
> > > > > + * This Enum property is used by the userspace to declare the content type
> > > > > + * of the display stream, to kernel. Here display stream stands for any
> > > > > + * display content that userspace intended to render with HDCP encryption.
> > > >
> > > > Hi,
> > > >
> > > > I'd suggest s/render with/display through/.
> > > >
> > > > As a gfx dev, rendering is something quite different to me.
> > > Ok.
> > > >
> > > > > + *
> > > > > + * Content Type of a stream is decided by the owner of the stream, as
> > > > > + * "HDCP Type0" or "HDCP Type1".
> > > > > + *
> > > > > + * The value of the property can be one the below:
> > > >
> > > > *one of the below
> > > Sure.
> > > >
> > > > > + * - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> > > > > + * - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> > > > > + *
> > > > > + * When kernel starts the HDCP authentication upon the "DESIRED" state of
> > > > > + * the "Content Protection", it refers the "HDCP Content Type" property
> > > > > + * state. And perform the HDCP authentication with the display sink for
> > > > > + * the content type mentioned by "HDCP Content Type".
> > > >
> > > > How about:
> > > >
> > > > When kernel starts the HDCP authentication (see "Content Protection"
> > > > for details), it uses the content type in "HDCP Content Type"
> > > > for performing the HDCP authentication with the display sink.
> > > less confusing :) Thanks.
> > > >
> > > > > + *
> > > > > + * Stream classified as HDCP Type0 can be transmitted on a link which is
> > > > > + * encrypted with HDCP 1.4 or higher versions of HDCP(i.e HDCP2.2
> > > > > + * and more).
> > > >
> > > > This is where I get confused, see my earlier email from today on the
> > > > previous revision of this patch series. Is it necessary to talk about
> > > > HDCP versions here? The only thing that matters for UAPI is the content
> > > > type, right?
> > > >
> > > > Previously you said that the kernel will not use Type1 if userspace
> > > > only asked for Type0, but to me this text reads as quite the opposite.
> > > Simple. HDCP2.2 itself support both Type 0 and Type 1. where as HDCP1.4
> > > by default supports the Type 0 and doesn't support the Type 1.
> > >
> > > I guess you are getting confused by assigning the type to the versions.
> >
> > Hi,
> >
> > yes, I am indeed.
> >
> > Is the HDCP version ever exposed to userspace in any way?
> >
> > If it is, then explaining how the types relate to the versions may well
> > be useful. But if userspace cannot even know what HDCP version is being
> > used or available,
> Hi,
>
> Implicitly content type capability is exposing the HDCP version capability.
> Say if HDCP Type 1 is supported we infer that underlying kernel support
> HDCP2.2.
Hi,
this is kind of a circular argument. ;-)
Do you think that userspace would actually use the HDCP version
information for anything?
E.g. telling the end user that HDCP 2.2 is available if the display
server finds "HDCP Content Type" property? Is this expected use of the
property?
I also understand that this would only mean HDCP 2.2 support in the
display driver and hardware, but it says nothing about the sinks, so
userspace cannot know if HDCP 2.2 is actually being used unless it
configures Type1 encryption and succeeds.
So if the HDCP version is useful to userspace, then this property is
not enough to communicate it. But if it is not useful to userspace,
then why mention it at all?
> And content type is introduced to restrict few catagories of the content
> with few HDCP versions' protection, IMHO it is better to capture the
> relationship between Type and the underlying HDCP versions.
>
> See if the below para will do this task with out any ambiguous.
>
> * HDCP Content Type:
> * This Enum property is used by the userspace to declare the content type
> * of the display stream, to kernel. Here display stream stands for any
> * display content that userspace intended to display through HDCP
> * encryption.
> *
> * Content Type of a stream is decided by the owner of the stream, as
> * "HDCP Type0" or "HDCP Type1".
> *
> * The value of the property can be one of the below:
> * - "HDCP Type0": DRM_MODE_HDCP_CONTENT_TYPE0 = 0
> * - "HDCP Type1": DRM_MODE_HDCP_CONTENT_TYPE1 = 1
> *
> * When kernel starts the HDCP authentication (see "Content Protection"
> * for details), it uses the content type in "HDCP Content Type"
> * for performing the HDCP authentication with the display sink.
> *
> * Please note in HDCP spec versions, a link can be authenticated with
> * HDCP 2.2 for Content Type 0/Content Type 1. Where as a link can be
> * authenticated with HDCP1.4 only for Content Type 0(though it is implicit
> * in nature. As there is no reference for Content Type in HDCP1.4).
Ok, this makes it perfectly clear to me.
> *
> * HDCP2.2 authentication protocol itself takes the "Content Type" as a
> * parameter, which is a input for the DP HDCP2.2 encryption algo.
> *
> * In case of Type 0 content protection request, kernel driver can choose
> * either of HDCP spec versions 1.4 and 2.2. When HDCP2.2 is used for
> * "HDCP Type 0", a HDCP 2.2 capable repeater in the downstream can send
> * that content to a HDCP 1.4 authenticated HDCP sink (Type0 link).
> * But if the content is classified as "HDCP Type 1", above mentioned
> * HDCP 2.2 repeater wont send the content to the HDCP sink as it can't
> * authenticate the HDCP1.4 capable sink for "HDCP Type 1".
> *
Right, but are these details meaningful to the userspace using this
API? I mean, are these the property of this API rather than just
mandated by HDCP spec 2.2 in general?
> * Please note userspace can be ignorant of the HDCP versions used by the
> * kernel driver to achieve the "HDCP Content Type".
> *
> * At current scenario, classifying a content as Type 1 ensures that the
> * content will be displayed only through the HDCP2.2 encrypted link.
> *
> * Note that the HDCP Content Type property is introduced at HDCP 2.2, and
> * defaults to type 0. It is only exposed by drivers supporting HDCP 2.2
> * (hence supporting Type 0 and Type 1). Based on how next versions of
> * HDCP specs are defined content Type could be used for higher versions too.
> *
> * If content type is changed when "Content Protection" is not UNDESIRED,
> * then kernel will disable the HDCP and re-enable with new type in the
> * same atomic commit. And when "Content Protection" is ENABLED, it means
> * that link is HDCP authenticated and encrypted, for the transmission of
> * the Type of stream mentioned at "HDCP Content Type".
>
> Please share your view on this.
That is a big piece of well-written doc. If you believe all these
details can be relevant to userspace using this property, then I am
happy see this in the UAPI doc. It is detailed and clear. My only
objection is whether it is all relevant, and I'm ok giving that up.
Thanks,
pq
Attachment:
pgpE8Ga1GUTA7.pgp
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
