Re: [PATCH v9 1/6] drm: Add Content protection type property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
> 
> > + *
> > + *	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;
> 


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux