| Message ID | 1530088829-11730-12-git-send-email-ramalingam.c@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 27, 2018 at 02:10:00PM +0530, Ramalingam C wrote: > Considering that HDCP2.2 is more secure than HDCP1.4, When a setup > supports HDCP2.2 and HDCP1.4, HDCP2.2 will be enabled. > > v2: > Included few optimization suggestions [Chris Wilson] > Commit message is updated as per the rebased version. > v3: > No changes. > v4: > Extra comment added and Style issue fixed [Uma] > v5: > Rebased as part of patch reordering. > Flag is added for tracking hdcp2.2 encryption status. > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > --- > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_hdcp.c | 90 +++++++++++++++++++++++++++++++++++---- > 2 files changed, 83 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 2eeb82b04953..7624388eecd5 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -414,6 +414,7 @@ struct intel_hdcp { > > /* HDCP2.2 related definitions */ > bool hdcp2_supported; > + bool hdcp2_in_use; > > /* > * Content Stream Type defined by content owner. TYPE0(0x0) content can > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c > index 32a1a3f39b65..b34e3b1587d6 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -21,6 +21,60 @@ > (enum hdcp_physical_port)(port)) > > static int intel_hdcp2_init(struct intel_connector *connector); > +static int _intel_hdcp2_enable(struct intel_connector *connector); > +static int _intel_hdcp2_disable(struct intel_connector *connector); > +static > +int intel_hdcp_read_valid_bksv(struct intel_digital_port *intel_dig_port, > + const struct intel_hdcp_shim *shim, u8 *bksv); > +static > +struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector); Please avoid forward declarations of static functions. Just place things appropriately in the file. > + > +static bool panel_supports_hdcp(struct intel_connector *connector) > +{ > + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector); > + struct intel_hdcp *hdcp = &connector->hdcp; > + bool capable = false; > + u8 bksv[5]; > + > + if (hdcp->hdcp_shim) { This function can only be called from intel_hdcp_enable() -> intel_hdcp_capable() -> panel_supports_hdcp() Both of those preceding functions check if hdcp_shim is NULL. > + if (hdcp->hdcp_shim->hdcp_capable) { > + hdcp->hdcp_shim->hdcp_capable(intel_dig_port, &capable); > + } else { > + if (!intel_hdcp_read_valid_bksv(intel_dig_port, > + hdcp->hdcp_shim, bksv)) > + capable = true; > + } > + } > + > + return capable; > +} > + > +static inline > +bool panel_supports_hdcp2(struct intel_connector *connector) > +{ > + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector); > + struct intel_hdcp *hdcp = &connector->hdcp; > + bool capable = false; > + > + /* Check the panel's hdcp2.2 compliance if platform supports it. */ > + if (hdcp->hdcp2_supported) > + hdcp->hdcp_shim->hdcp_2_2_capable(intel_dig_port, &capable); > + > + return capable; > +} > + > +/* Is HDCP1.4 capable on Platform and Panel */ > +static inline bool intel_hdcp_capable(struct intel_connector *connector) > +{ > + return (connector->hdcp.hdcp_shim && panel_supports_hdcp(connector)); As mentioned, the hdcp_shim check is already covered by the caller. The way things are setup, the shim checks only need to exist at the entry points (enable/disable/check_link). > +} > + > +/* Is HDCP2.2 capable on Platform and Panel */ > +static inline bool intel_hdcp2_capable(struct intel_connector *connector) > +{ > + return (connector->hdcp.hdcp2_supported && > + panel_supports_hdcp2(connector)); > +} The panel_supports_* functions don't seem necessary, just do the work in the intel_hdcp*_capable functions. > > static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port, > const struct intel_hdcp_shim *shim) > @@ -796,20 +850,27 @@ int intel_hdcp_init(struct intel_connector *connector, > int intel_hdcp_enable(struct intel_connector *connector) > { > struct intel_hdcp *hdcp = &connector->hdcp; > - int ret; > + int ret = -EINVAL; > > if (!hdcp->hdcp_shim) > return -ENOENT; > > mutex_lock(&hdcp->hdcp_mutex); > > - ret = _intel_hdcp_enable(connector); > - if (ret) > - goto out; > + /* > + * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup > + * is capable of HDCP2.2, it is preferred to use HDCP2.2. > + */ > + if (intel_hdcp2_capable(connector)) > + ret = _intel_hdcp2_enable(connector); > + else if (intel_hdcp_capable(connector)) > + ret = _intel_hdcp_enable(connector); > + > + if (!ret) { > + hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED; > + schedule_work(&hdcp->hdcp_prop_work); > + } > > - hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED; > - schedule_work(&hdcp->hdcp_prop_work); > -out: > mutex_unlock(&hdcp->hdcp_mutex); > return ret; > } > @@ -826,7 +887,10 @@ int intel_hdcp_disable(struct intel_connector *connector) > > if (hdcp->hdcp_value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) { > hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED; > - ret = _intel_hdcp_disable(connector); > + if (hdcp->hdcp2_in_use) > + ret = _intel_hdcp2_disable(connector); > + else > + ret = _intel_hdcp_disable(connector); > } > > mutex_unlock(&hdcp->hdcp_mutex); > @@ -928,6 +992,16 @@ int intel_hdcp_check_link(struct intel_connector *connector) > return ret; > } > > +static int _intel_hdcp2_enable(struct intel_connector *connector) > +{ > + return 0; > +} > + > +static int _intel_hdcp2_disable(struct intel_connector *connector) > +{ > + return 0; > +} nit: It'd probably be better to introduce the stubs with an error message, since as you have it, you'll be able to "enable" HDCP2 without doing anything. Sean > + > static int i915_hdcp_component_master_bind(struct device *dev) > { > struct drm_i915_private *dev_priv = kdev_to_i915(dev); > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tuesday 10 July 2018 02:14 AM, Sean Paul wrote: > On Wed, Jun 27, 2018 at 02:10:00PM +0530, Ramalingam C wrote: >> Considering that HDCP2.2 is more secure than HDCP1.4, When a setup >> supports HDCP2.2 and HDCP1.4, HDCP2.2 will be enabled. >> >> v2: >> Included few optimization suggestions [Chris Wilson] >> Commit message is updated as per the rebased version. >> v3: >> No changes. >> v4: >> Extra comment added and Style issue fixed [Uma] >> v5: >> Rebased as part of patch reordering. >> Flag is added for tracking hdcp2.2 encryption status. >> >> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> >> --- >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> drivers/gpu/drm/i915/intel_hdcp.c | 90 +++++++++++++++++++++++++++++++++++---- >> 2 files changed, 83 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 2eeb82b04953..7624388eecd5 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -414,6 +414,7 @@ struct intel_hdcp { >> >> /* HDCP2.2 related definitions */ >> bool hdcp2_supported; >> + bool hdcp2_in_use; >> >> /* >> * Content Stream Type defined by content owner. TYPE0(0x0) content can >> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c >> index 32a1a3f39b65..b34e3b1587d6 100644 >> --- a/drivers/gpu/drm/i915/intel_hdcp.c >> +++ b/drivers/gpu/drm/i915/intel_hdcp.c >> @@ -21,6 +21,60 @@ >> (enum hdcp_physical_port)(port)) >> >> static int intel_hdcp2_init(struct intel_connector *connector); >> +static int _intel_hdcp2_enable(struct intel_connector *connector); >> +static int _intel_hdcp2_disable(struct intel_connector *connector); >> +static >> +int intel_hdcp_read_valid_bksv(struct intel_digital_port *intel_dig_port, >> + const struct intel_hdcp_shim *shim, u8 *bksv); >> +static >> +struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector); > Please avoid forward declarations of static functions. Just place things > appropriately in the file. > >> + >> +static bool panel_supports_hdcp(struct intel_connector *connector) >> +{ >> + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector); >> + struct intel_hdcp *hdcp = &connector->hdcp; >> + bool capable = false; >> + u8 bksv[5]; >> + >> + if (hdcp->hdcp_shim) { > This function can only be called from > intel_hdcp_enable() -> intel_hdcp_capable() -> panel_supports_hdcp() > > Both of those preceding functions check if hdcp_shim is NULL. > >> + if (hdcp->hdcp_shim->hdcp_capable) { >> + hdcp->hdcp_shim->hdcp_capable(intel_dig_port, &capable); >> + } else { >> + if (!intel_hdcp_read_valid_bksv(intel_dig_port, >> + hdcp->hdcp_shim, bksv)) >> + capable = true; >> + } >> + } >> + >> + return capable; >> +} >> + >> +static inline >> +bool panel_supports_hdcp2(struct intel_connector *connector) >> +{ >> + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector); >> + struct intel_hdcp *hdcp = &connector->hdcp; >> + bool capable = false; >> + >> + /* Check the panel's hdcp2.2 compliance if platform supports it. */ >> + if (hdcp->hdcp2_supported) >> + hdcp->hdcp_shim->hdcp_2_2_capable(intel_dig_port, &capable); >> + >> + return capable; >> +} >> + >> +/* Is HDCP1.4 capable on Platform and Panel */ >> +static inline bool intel_hdcp_capable(struct intel_connector *connector) >> +{ >> + return (connector->hdcp.hdcp_shim && panel_supports_hdcp(connector)); > As mentioned, the hdcp_shim check is already covered by the caller. The way > things are setup, the shim checks only need to exist at the entry points > (enable/disable/check_link). > >> +} >> + >> +/* Is HDCP2.2 capable on Platform and Panel */ >> +static inline bool intel_hdcp2_capable(struct intel_connector *connector) >> +{ >> + return (connector->hdcp.hdcp2_supported && >> + panel_supports_hdcp2(connector)); >> +} > The panel_supports_* functions don't seem necessary, just do the work in the > intel_hdcp*_capable functions. > >> >> static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port, >> const struct intel_hdcp_shim *shim) >> @@ -796,20 +850,27 @@ int intel_hdcp_init(struct intel_connector *connector, >> int intel_hdcp_enable(struct intel_connector *connector) >> { >> struct intel_hdcp *hdcp = &connector->hdcp; >> - int ret; >> + int ret = -EINVAL; >> >> if (!hdcp->hdcp_shim) >> return -ENOENT; >> >> mutex_lock(&hdcp->hdcp_mutex); >> >> - ret = _intel_hdcp_enable(connector); >> - if (ret) >> - goto out; >> + /* >> + * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup >> + * is capable of HDCP2.2, it is preferred to use HDCP2.2. >> + */ >> + if (intel_hdcp2_capable(connector)) >> + ret = _intel_hdcp2_enable(connector); >> + else if (intel_hdcp_capable(connector)) >> + ret = _intel_hdcp_enable(connector); >> + >> + if (!ret) { >> + hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED; >> + schedule_work(&hdcp->hdcp_prop_work); >> + } >> >> - hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED; >> - schedule_work(&hdcp->hdcp_prop_work); >> -out: >> mutex_unlock(&hdcp->hdcp_mutex); >> return ret; >> } >> @@ -826,7 +887,10 @@ int intel_hdcp_disable(struct intel_connector *connector) >> >> if (hdcp->hdcp_value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) { >> hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED; >> - ret = _intel_hdcp_disable(connector); >> + if (hdcp->hdcp2_in_use) >> + ret = _intel_hdcp2_disable(connector); >> + else >> + ret = _intel_hdcp_disable(connector); >> } >> >> mutex_unlock(&hdcp->hdcp_mutex); >> @@ -928,6 +992,16 @@ int intel_hdcp_check_link(struct intel_connector *connector) >> return ret; >> } >> >> +static int _intel_hdcp2_enable(struct intel_connector *connector) >> +{ >> + return 0; >> +} >> + >> +static int _intel_hdcp2_disable(struct intel_connector *connector) >> +{ >> + return 0; >> +} > nit: It'd probably be better to introduce the stubs with an error message, since > as you have it, you'll be able to "enable" HDCP2 without doing anything. Thanks Sean. Taken care of all above suggestions in next version. And I have squashed patches 11, 12, 13 and 14. Ram. > > Sean > >> + >> static int i915_hdcp_component_master_bind(struct device *dev) >> { >> struct drm_i915_private *dev_priv = kdev_to_i915(dev); >> -- >> 2.7.4 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 2eeb82b04953..7624388eecd5 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -414,6 +414,7 @@ struct intel_hdcp { /* HDCP2.2 related definitions */ bool hdcp2_supported; + bool hdcp2_in_use; /* * Content Stream Type defined by content owner. TYPE0(0x0) content can diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c index 32a1a3f39b65..b34e3b1587d6 100644 --- a/drivers/gpu/drm/i915/intel_hdcp.c +++ b/drivers/gpu/drm/i915/intel_hdcp.c @@ -21,6 +21,60 @@ (enum hdcp_physical_port)(port)) static int intel_hdcp2_init(struct intel_connector *connector); +static int _intel_hdcp2_enable(struct intel_connector *connector); +static int _intel_hdcp2_disable(struct intel_connector *connector); +static +int intel_hdcp_read_valid_bksv(struct intel_digital_port *intel_dig_port, + const struct intel_hdcp_shim *shim, u8 *bksv); +static +struct intel_digital_port *conn_to_dig_port(struct intel_connector *connector); + +static bool panel_supports_hdcp(struct intel_connector *connector) +{ + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector); + struct intel_hdcp *hdcp = &connector->hdcp; + bool capable = false; + u8 bksv[5]; + + if (hdcp->hdcp_shim) { + if (hdcp->hdcp_shim->hdcp_capable) { + hdcp->hdcp_shim->hdcp_capable(intel_dig_port, &capable); + } else { + if (!intel_hdcp_read_valid_bksv(intel_dig_port, + hdcp->hdcp_shim, bksv)) + capable = true; + } + } + + return capable; +} + +static inline +bool panel_supports_hdcp2(struct intel_connector *connector) +{ + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector); + struct intel_hdcp *hdcp = &connector->hdcp; + bool capable = false; + + /* Check the panel's hdcp2.2 compliance if platform supports it. */ + if (hdcp->hdcp2_supported) + hdcp->hdcp_shim->hdcp_2_2_capable(intel_dig_port, &capable); + + return capable; +} + +/* Is HDCP1.4 capable on Platform and Panel */ +static inline bool intel_hdcp_capable(struct intel_connector *connector) +{ + return (connector->hdcp.hdcp_shim && panel_supports_hdcp(connector)); +} + +/* Is HDCP2.2 capable on Platform and Panel */ +static inline bool intel_hdcp2_capable(struct intel_connector *connector) +{ + return (connector->hdcp.hdcp2_supported && + panel_supports_hdcp2(connector)); +} static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port, const struct intel_hdcp_shim *shim) @@ -796,20 +850,27 @@ int intel_hdcp_init(struct intel_connector *connector, int intel_hdcp_enable(struct intel_connector *connector) { struct intel_hdcp *hdcp = &connector->hdcp; - int ret; + int ret = -EINVAL; if (!hdcp->hdcp_shim) return -ENOENT; mutex_lock(&hdcp->hdcp_mutex); - ret = _intel_hdcp_enable(connector); - if (ret) - goto out; + /* + * Considering that HDCP2.2 is more secure than HDCP1.4, If the setup + * is capable of HDCP2.2, it is preferred to use HDCP2.2. + */ + if (intel_hdcp2_capable(connector)) + ret = _intel_hdcp2_enable(connector); + else if (intel_hdcp_capable(connector)) + ret = _intel_hdcp_enable(connector); + + if (!ret) { + hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED; + schedule_work(&hdcp->hdcp_prop_work); + } - hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_ENABLED; - schedule_work(&hdcp->hdcp_prop_work); -out: mutex_unlock(&hdcp->hdcp_mutex); return ret; } @@ -826,7 +887,10 @@ int intel_hdcp_disable(struct intel_connector *connector) if (hdcp->hdcp_value != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) { hdcp->hdcp_value = DRM_MODE_CONTENT_PROTECTION_UNDESIRED; - ret = _intel_hdcp_disable(connector); + if (hdcp->hdcp2_in_use) + ret = _intel_hdcp2_disable(connector); + else + ret = _intel_hdcp_disable(connector); } mutex_unlock(&hdcp->hdcp_mutex); @@ -928,6 +992,16 @@ int intel_hdcp_check_link(struct intel_connector *connector) return ret; } +static int _intel_hdcp2_enable(struct intel_connector *connector) +{ + return 0; +} + +static int _intel_hdcp2_disable(struct intel_connector *connector) +{ + return 0; +} + static int i915_hdcp_component_master_bind(struct device *dev) { struct drm_i915_private *dev_priv = kdev_to_i915(dev);
Considering that HDCP2.2 is more secure than HDCP1.4, When a setup supports HDCP2.2 and HDCP1.4, HDCP2.2 will be enabled. v2: Included few optimization suggestions [Chris Wilson] Commit message is updated as per the rebased version. v3: No changes. v4: Extra comment added and Style issue fixed [Uma] v5: Rebased as part of patch reordering. Flag is added for tracking hdcp2.2 encryption status. Signed-off-by: Ramalingam C <ramalingam.c@intel.com> --- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_hdcp.c | 90 +++++++++++++++++++++++++++++++++++---- 2 files changed, 83 insertions(+), 8 deletions(-)