On 2019-07-09 at 17:31:10 +0300, Pekka Paalanen wrote: > On Mon, 8 Jul 2019 16:51:11 +0530 > Ramalingam C <ramalinga...@intel.com> 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 <ramalinga...@intel.com> > > Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch> > > --- > > 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. > > > + * > > + * Streams classified as HDCP Type1 can be transmitted on a link which is > > + * encrypted only with HDCP 2.2. In future, HDCP versions >2.2 also might > > + * support Type1 based on their spec. > > + * > > + * HDCP2.2 authentication protocol itself takes the "Content Type" as a > > + * parameter, which is a input for the DP HDCP2.2 encryption algo. > > + * > > + * 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. > > + * Based on how next versions of HDCP specs are defined content Type could > > + * be used for higher versions too. > > Ok, userspace does not have to cope with a "HDCP Content Type" property > that is missing the enum value Type1. I think the Weston patch would > attempt something silly or misbehave if Type1 value was ever missing. > Not having the whole property is fine, of course. If the Type1 is not supported then there is no need for the "HDCP Content Type" property itself. Thats why when a driver has the support for Type1( as of now HDCP2.2) only then "HDCP Content Type" will be exposed. So weston is fine at its current state. -Ram > > > + * > > + * 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". > > + * > > Very good! This wording is much better already. > > > Thanks, > pq > > > > * HDR_OUTPUT_METADATA: > > * Connector property to enable userspace to send HDR Metadata to > > * driver. This metadata is based on the composition and blending > > diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c > > index cd837bd409f7..ce235fd1c844 100644 > > --- a/drivers/gpu/drm/drm_hdcp.c > > +++ b/drivers/gpu/drm/drm_hdcp.c > > @@ -344,23 +344,41 @@ static struct drm_prop_enum_list drm_cp_enum_list[] = > > { > > }; > > DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list) > > > > +static struct drm_prop_enum_list drm_hdcp_content_type_enum_list[] = { > > + { DRM_MODE_HDCP_CONTENT_TYPE0, "HDCP Type0" }, > > + { DRM_MODE_HDCP_CONTENT_TYPE1, "HDCP Type1" }, > > +}; > > +DRM_ENUM_NAME_FN(drm_get_hdcp_content_type_name, > > + drm_hdcp_content_type_enum_list) > > + > > /** > > * drm_connector_attach_content_protection_property - attach content > > protection > > * property > > * > > * @connector: connector to attach CP property on. > > + * @hdcp_content_type: is HDCP Content Type property needed for connector > > * > > * This is used to add support for content protection on select connectors. > > * Content Protection is intentionally vague to allow for different > > underlying > > * technologies, however it is most implemented by HDCP. > > * > > + * When hdcp_content_type is true enum property called HDCP Content Type is > > + * created (if it is not already) and attached to the connector. > > + * > > + * This property is used for sending the protected content's stream type > > + * from userspace to kernel on selected connectors. Protected content > > provider > > + * will decide their type of their content and declare the same to kernel. > > + * > > + * Content type will be used during the HDCP 2.2 authentication. > > + * Content type will be set to &drm_connector_state.hdcp_content_type. > > + * > > * The content protection will be set to > > &drm_connector_state.content_protection > > * > > * Returns: > > * Zero on success, negative errno on failure. > > */ > > int drm_connector_attach_content_protection_property( > > - struct drm_connector *connector) > > + struct drm_connector *connector, bool hdcp_content_type) > > { > > struct drm_device *dev = connector->dev; > > struct drm_property *prop = > > @@ -377,6 +395,22 @@ int drm_connector_attach_content_protection_property( > > DRM_MODE_CONTENT_PROTECTION_UNDESIRED); > > dev->mode_config.content_protection_property = prop; > > > > + if (!hdcp_content_type) > > + return 0; > > + > > + prop = dev->mode_config.hdcp_content_type_property; > > + if (!prop) > > + prop = drm_property_create_enum(dev, 0, "HDCP Content Type", > > + drm_hdcp_content_type_enum_list, > > + ARRAY_SIZE( > > + drm_hdcp_content_type_enum_list)); > > + if (!prop) > > + return -ENOMEM; > > + > > + drm_object_attach_property(&connector->base, prop, > > + DRM_MODE_HDCP_CONTENT_TYPE0); > > + dev->mode_config.hdcp_content_type_property = prop; > > + > > return 0; > > } > > EXPORT_SYMBOL(drm_connector_attach_content_protection_property); > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > index bc3a94d491c4..2a4d10952b74 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > @@ -1829,7 +1829,9 @@ int intel_hdcp_init(struct intel_connector *connector, > > if (!shim) > > return -EINVAL; > > > > - ret = > > drm_connector_attach_content_protection_property(&connector->base); > > + ret = > > + drm_connector_attach_content_protection_property(&connector->base, > > + false); > > if (ret) > > return ret; > > > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > > index 4c30d751487a..d6432967a2a9 100644 > > --- a/include/drm/drm_connector.h > > +++ b/include/drm/drm_connector.h > > @@ -601,6 +601,12 @@ struct drm_connector_state { > > */ > > unsigned int content_type; > > > > + /** > > + * @hdcp_content_type: Connector property to pass the type of > > + * protected content. This is most commonly used for HDCP. > > + */ > > + unsigned int hdcp_content_type; > > + > > /** > > * @scaling_mode: Connector property to control the > > * upscaling, mostly used for built-in panels. > > @@ -1484,6 +1490,7 @@ const char *drm_get_dvi_i_select_name(int val); > > const char *drm_get_tv_subconnector_name(int val); > > const char *drm_get_tv_select_name(int val); > > const char *drm_get_content_protection_name(int val); > > +const char *drm_get_hdcp_content_type_name(int val); > > > > int drm_mode_create_dvi_i_properties(struct drm_device *dev); > > int drm_mode_create_tv_margin_properties(struct drm_device *dev); > > diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h > > index 13771a496e2b..2970abdfaf12 100644 > > --- a/include/drm/drm_hdcp.h > > +++ b/include/drm/drm_hdcp.h > > @@ -291,5 +291,5 @@ struct drm_connector; > > bool drm_hdcp_check_ksvs_revoked(struct drm_device *dev, > > u8 *ksvs, u32 ksv_count); > > int drm_connector_attach_content_protection_property( > > - struct drm_connector *connector); > > + struct drm_connector *connector, bool hdcp_content_type); > > #endif > > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > > index 759d462d028b..6c4b5bc85951 100644 > > --- a/include/drm/drm_mode_config.h > > +++ b/include/drm/drm_mode_config.h > > @@ -849,6 +849,12 @@ struct drm_mode_config { > > */ > > struct drm_property *content_protection_property; > > > > + /** > > + * @hdcp_content_type_property: DRM ENUM property for type of > > + * Protected Content. > > + */ > > + struct drm_property *hdcp_content_type_property; > > + > > /* dumb ioctl parameters */ > > uint32_t preferred_depth, prefer_shadow; > > > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > > index 5ab331e5dc23..5c954394093f 100644 > > --- a/include/uapi/drm/drm_mode.h > > +++ b/include/uapi/drm/drm_mode.h > > @@ -218,6 +218,10 @@ extern "C" { > > #define DRM_MODE_CONTENT_PROTECTION_DESIRED 1 > > #define DRM_MODE_CONTENT_PROTECTION_ENABLED 2 > > > > +/* Content Type classification for HDCP2.2 vs others */ > > +#define DRM_MODE_HDCP_CONTENT_TYPE0 0 > > +#define DRM_MODE_HDCP_CONTENT_TYPE1 1 > > + > > struct drm_mode_modeinfo { > > __u32 clock; > > __u16 hdisplay; > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel