Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752732AbdLEPem (ORCPT ); Tue, 5 Dec 2017 10:34:42 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:46770 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752606AbdLEPek (ORCPT ); Tue, 5 Dec 2017 10:34:40 -0500 X-Google-Smtp-Source: AGs4zMbFShk8k3+SoYYh6o2u33pdLT30vr21T5EKioE+o1vGZ7yhXyyp7Mi+u4v805Tui4sra3kqug== Date: Tue, 5 Dec 2017 16:34:36 +0100 From: Daniel Vetter To: Sean Paul Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, seanpaul@google.com, David Airlie , linux-kernel@vger.kernel.org, daniel.vetter@intel.com Subject: Re: [PATCH v3 3/9] drm: Add Content Protection property Message-ID: <20171205153436.57efslvedeo63guu@phenom.ffwll.local> Mail-Followup-To: Sean Paul , dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, seanpaul@google.com, David Airlie , linux-kernel@vger.kernel.org, daniel.vetter@intel.com References: <20171205051513.8603-1-seanpaul@chromium.org> <20171205051513.8603-4-seanpaul@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171205051513.8603-4-seanpaul@chromium.org> X-Operating-System: Linux phenom 4.13.0-1-amd64 User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10851 Lines: 268 On Tue, Dec 05, 2017 at 12:15:02AM -0500, Sean Paul wrote: > This patch adds a new optional connector property to allow userspace to enable > protection over the content it is displaying. This will typically be implemented > by the driver using HDCP. > > The property is a tri-state with the following values: > - OFF: Self explanatory, no content protection > - DESIRED: Userspace requests that the driver enable protection > - ENABLED: Once the driver has authenticated the link, it sets this value > > The driver is responsible for downgrading ENABLED to DESIRED if the link becomes > unprotected. The driver should also maintain the desiredness of protection > across hotplug/dpms/suspend. > > If this looks familiar, I posted [1] this 3 years ago. We have been using this > in ChromeOS across exynos, mediatek, and rockchip over that time. > > Changes in v2: > - Pimp kerneldoc for content_protection_property (Daniel) > - Drop sysfs attribute > Changes in v3: > - None > > Cc: Daniel Vetter > Signed-off-by: Sean Paul > > [1] https://lists.freedesktop.org/archives/dri-devel/2014-December/073336.html > --- > drivers/gpu/drm/drm_atomic.c | 8 +++++ > drivers/gpu/drm/drm_connector.c | 71 +++++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_sysfs.c | 1 + > include/drm/drm_connector.h | 16 ++++++++++ > include/uapi/drm/drm_mode.h | 4 +++ > 5 files changed, 100 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index c2da5585e201..676025d755b2 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1196,6 +1196,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, > state->picture_aspect_ratio = val; > } else if (property == connector->scaling_mode_property) { > state->scaling_mode = val; > + } else if (property == connector->content_protection_property) { > + if (val == DRM_MODE_CONTENT_PROTECTION_ENABLED) { > + DRM_DEBUG_KMS("only drivers can set CP Enabled\n"); > + return -EINVAL; > + } > + state->content_protection = val; > } else if (connector->funcs->atomic_set_property) { > return connector->funcs->atomic_set_property(connector, > state, property, val); > @@ -1275,6 +1281,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, > *val = state->picture_aspect_ratio; > } else if (property == connector->scaling_mode_property) { > *val = state->scaling_mode; > + } else if (property == connector->content_protection_property) { > + *val = state->content_protection; > } else if (connector->funcs->atomic_get_property) { > return connector->funcs->atomic_get_property(connector, > state, property, val); > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index f14b48e6e839..8626aa8f485e 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -698,6 +698,13 @@ static const struct drm_prop_enum_list drm_tv_subconnector_enum_list[] = { > DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name, > drm_tv_subconnector_enum_list) > > +static struct drm_prop_enum_list drm_cp_enum_list[] = { > + { DRM_MODE_CONTENT_PROTECTION_OFF, "Off" }, > + { DRM_MODE_CONTENT_PROTECTION_DESIRED, "Desired" }, > + { DRM_MODE_CONTENT_PROTECTION_ENABLED, "Enabled" }, > +}; > +DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list) > + > /** > * DOC: standard connector properties > * > @@ -764,6 +771,34 @@ DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name, > * after modeset, the kernel driver may set this to "BAD" and issue a > * hotplug uevent. Drivers should update this value using > * drm_mode_connector_set_link_status_property(). > + * Content Protection: > + * This property is used by userspace to request the kernel protect future > + * content communicated over the link. When requested, kernel will apply > + * the appropriate means of protection (most often HDCP), and use the > + * property to tell userspace the protection is active. > + * > + * The value of this property can be one of the following: > + * > + * - DRM_MODE_CONTENT_PROTECTION_OFF = 0 > + * The link is not protected, content is transmitted in the clear. > + * - DRM_MODE_CONTENT_PROTECTION_DESIRED = 1 > + * Userspace has requested content protection, but the link is not > + * currently protected. When in this state, kernel should enable > + * Content Protection as soon as possible. > + * - DRM_MODE_CONTENT_PROTECTION_ENABLED = 2 > + * Userspace has requested content protection, and the link is > + * protected. Only the driver can set the property to this value. > + * If userspace attempts to set to ENABLED, kernel will return > + * -EINVAL. > + * > + * A few guidelines: > + * > + * - DESIRED state should be preserved until userspace de-asserts it by > + * setting the property to OFF. This means ENABLED should only transition > + * to OFF when the user explicitly requests it. > + * - If the state is DESIRED, kernel should attempt to re-authenticate the > + * link whenever possible. This includes across disable/enable, dpms, > + * hotplug, downstream device changes, link status failures, etc.. Two bits missing imo: - Should explain that userspace should poll this property to detect a change from ENABLED to DESIRED (and take adequate actions and e.g. stop the stream). No uevent will be sent out because the HDCP specs require polling anyway. - One sentence to explain that drivers set this up using drm_connector_attach_content_protection_property(). Also, pls double-check the html output looks decent, you have quite some fancy formatting in the above section :-) With that all addressed: Reviewed-by: Daniel Vetter > * > * Connectors also have one standardized atomic property: > * > @@ -1047,6 +1082,42 @@ int drm_connector_attach_scaling_mode_property(struct drm_connector *connector, > } > EXPORT_SYMBOL(drm_connector_attach_scaling_mode_property); > > +/** > + * drm_connector_attach_content_protection_property - attach content protection > + * property > + * > + * @connector: connector to attach CP property on. > + * > + * 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. > + * > + * 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_device *dev = connector->dev; > + struct drm_property *prop; > + > + prop = drm_property_create_enum(dev, 0, "Content Protection", > + drm_cp_enum_list, > + ARRAY_SIZE(drm_cp_enum_list)); > + if (!prop) > + return -ENOMEM; > + > + drm_object_attach_property(&connector->base, prop, > + DRM_MODE_CONTENT_PROTECTION_OFF); > + > + connector->content_protection_property = prop; > + > + return 0; > +} > +EXPORT_SYMBOL(drm_connector_attach_content_protection_property); > + > /** > * drm_mode_create_aspect_ratio_property - create aspect ratio property > * @dev: DRM device > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > index 1c5b5ce1fd7f..2385c7e0bef5 100644 > --- a/drivers/gpu/drm/drm_sysfs.c > +++ b/drivers/gpu/drm/drm_sysfs.c > @@ -21,6 +21,7 @@ > #include > #include > #include "drm_internal.h" > +#include "drm_crtc_internal.h" > > #define to_drm_minor(d) dev_get_drvdata(d) > #define to_drm_connector(d) dev_get_drvdata(d) > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 7a7140543012..828878addd03 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -370,6 +370,12 @@ struct drm_connector_state { > * upscaling, mostly used for built-in panels. > */ > unsigned int scaling_mode; > + > + /** > + * @content_protection: Connector property to request content > + * protection. This is most commonly used for HDCP. > + */ > + unsigned int content_protection; > }; > > /** > @@ -718,6 +724,7 @@ struct drm_cmdline_mode { > * @tile_h_size: horizontal size of this tile. > * @tile_v_size: vertical size of this tile. > * @scaling_mode_property: Optional atomic property to control the upscaling. > + * @content_protection_property: Optional property to control content protection > * > * Each connector may be connected to one or more CRTCs, or may be clonable by > * another connector if they can share a CRTC. Each connector also has a specific > @@ -808,6 +815,12 @@ struct drm_connector { > > struct drm_property *scaling_mode_property; > > + /** > + * @content_protection_property: DRM ENUM property for content > + * protection > + */ > + struct drm_property *content_protection_property; > + > /** > * @path_blob_ptr: > * > @@ -1002,6 +1015,7 @@ const char *drm_get_dvi_i_subconnector_name(int val); > 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); > > int drm_mode_create_dvi_i_properties(struct drm_device *dev); > int drm_mode_create_tv_properties(struct drm_device *dev, > @@ -1010,6 +1024,8 @@ int drm_mode_create_tv_properties(struct drm_device *dev, > int drm_mode_create_scaling_mode_property(struct drm_device *dev); > int drm_connector_attach_scaling_mode_property(struct drm_connector *connector, > u32 scaling_mode_mask); > +int drm_connector_attach_content_protection_property( > + struct drm_connector *connector); > int drm_mode_create_aspect_ratio_property(struct drm_device *dev); > int drm_mode_create_suggested_offset_properties(struct drm_device *dev); > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index 5597a87154e5..03f4d22305c2 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -173,6 +173,10 @@ extern "C" { > DRM_MODE_REFLECT_X | \ > DRM_MODE_REFLECT_Y) > > +/* Content Protection Flags */ > +#define DRM_MODE_CONTENT_PROTECTION_OFF 0 > +#define DRM_MODE_CONTENT_PROTECTION_DESIRED 1 > +#define DRM_MODE_CONTENT_PROTECTION_ENABLED 2 > > struct drm_mode_modeinfo { > __u32 clock; > -- > 2.15.0.531.g2ccb3012c9-goog > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch