On Thu, 11 Jul 2019 10:18:22 -0400
Sean Paul <sean@poorly.run> wrote:

> On Mon, Jul 08, 2019 at 04:51:11PM +0530, Ramalingam C 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.

...

> > 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.
> > + *
> > + *	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:
> > + *	  - "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".
> > + *
> > + *	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).
> > + *
> > + *	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.
> > + *
> > + *	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".
> > + *
> >   * HDR_OUTPUT_METADATA:
> >   *	Connector property to enable userspace to send HDR Metadata to
> >   *	driver. This metadata is based on the composition and blending  
> 
> Do we actually need an entirely new property? Can't we just add a new
> entry to the existing Content Protection property which is "Desired Type1" or
> similar? The kernel will then either attempt 2.2 auth or it will ignore it the
> request if it's not supported.

Hi,

IMHO the existing "Content Protection" property is already complicated
enough that one should not add anything new to it.

If you added "desired-type-1", the readback of it would become
ambiguous if it was "ENABLED", userspace would not know if the value
written was "DESIRED" or "desired-type-1". Sure, it's not a problem
when a display server knows what it just wrote into it, but shouldn't
we try to keep KMS state readable as well, if for nothing but debugging?

I think using the same property for communicating in both directions
between the kernel and userspace (value can be set by both userspace and
kernel at times) was a mistake to begin with. It has already caused
long discussions on what the readback actually should reflect and
whether there are races for a given userspace implementation.


Thanks,
pq