Re: [PATCH v8 06/35] drm/i915: Enable and Disable of HDCP2.2
On Fri, Dec 07, 2018 at 11:52:21AM +0530, C, Ramalingam wrote:
>
> On 12/6/2018 4:00 PM, Daniel Vetter wrote:
> > On Tue, Nov 27, 2018 at 04:13:04PM +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.
> > >
> > > When HDCP2.2 enabling fails and HDCP1.4 is supported, HDCP1.4 is
> > > enabled.
> > >
> > > This change implements a sequence of enabling and disabling of
> > > HDCP2.2 authentication and HDCP2.2 port encryption.
> > Patch series suggestion for next time around: First build out the helper
> > functions, then time them into the big picture like here. Personally I
> > think that makes it easier to understand, but it's kinda personal choice.
>
> Tried that already. But bisecting will give warnings as "defined but unused
> functions"
>
> Hence moved to this approach.
Ah right, that's the annoying part with that approach. I tend to just
ignore that until the series is built up (it's just a warning), but good
point. As mentioned, there's pro/cons to each approach.
-Daniel
>
> > I guess this here works too.
> >
> > > v2:
> > >Included few optimization suggestions [Chris Wilson]
> > >Commit message is updated as per the rebased version.
> > >intel_wait_for_register is used instead of wait_for. [Chris Wilson]
> > > v3:
> > >No changes.
> > > v4:
> > >Extra comment added and Style issue fixed [Uma]
> > > v5:
> > >Rebased as part of patch reordering.
> > >HDCP2 encryption status is tracked.
> > >HW state check is moved into WARN_ON [Daniel]
> > > v6:
> > >Redefined the mei service functions as per comp redesign.
> > >Merged patches related to hdcp2.2 enabling and disabling [Sean Paul].
> > >Required shim functionality is defined [Sean Paul]
> > > v7:
> > >Return values are handles [Uma]
> > >Realigned the code.
> > >Check for comp_master is removed.
> > > v8:
> > >HDCP2.2 is attempted only if mei interface is up.
> > >Adjust to the new interface
> > >Avoid bool usage in struct [Tomas]
> > >
> > > Signed-off-by: Ramalingam C
> > > ---
> > > drivers/gpu/drm/i915/intel_drv.h | 5 +
> > > drivers/gpu/drm/i915/intel_hdcp.c | 223
> > > +++---
> > > 2 files changed, 214 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index bde82f3ada85..3e9f21d23442 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -383,6 +383,10 @@ struct intel_hdcp_shim {
> > > /* Detects the HDCP protocol(DP/HDMI) required on the port */
> > > enum mei_hdcp_wired_protocol (*hdcp_protocol)(void);
> > > +
> > > + /* Detects whether Panel is HDCP2.2 capable */
> > > + int (*hdcp_2_2_capable)(struct intel_digital_port *intel_dig_port,
> > > + bool *capable);
> > > };
> > > struct intel_hdcp {
> > > @@ -396,6 +400,7 @@ struct intel_hdcp {
> > > /* HDCP2.2 related definitions */
> > > /* Flag indicates whether this connector supports HDCP2.2 or
> > > not. */
> > > u8 hdcp2_supported;
> > > + u8 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 760780f1105c..c1bd1ccd47cd 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdcp.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> > > @@ -79,6 +79,43 @@ bool intel_hdcp_capable(struct intel_connector
> > > *connector)
> > > return capable;
> > > }
> > > +/* At present whether mei_hdcp component is binded with i915 master
> > > component */
> > > +static bool intel_hdcp2_mei_binded(struct intel_connector *connector)
> > > +{
> > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > > + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp;
> > > +
> > > + mutex_lock(>mutex);
> > > + if (!comp->ops || !comp->dev) {
> > > + mutex_unlock(>mutex);
> > > + return false;
> > > + }
> > > + mutex_unlock(>mutex);
> > > +
> > > + return true;
> > > +}
> > > +
> > > +/* Is HDCP2.2 capable on Platform and Sink */
> > > +static bool intel_hdcp2_capable(struct intel_connector *connector)
> > > +{
> > > + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> > > + struct intel_hdcp *hdcp = >hdcp;
> > > + bool capable = false;
> > > +
> > > + /* I915 support for HDCP2.2 */
> > > + if (!hdcp->hdcp2_supported)
> > > + return false;
> > > +
> > > + /* MEI services for HDCP2.2 */
> > > + if (!intel_hdcp2_mei_binded(connector))
> > > + return false;
> > Why do we still need this with component? Driver load should be stalled
> > out until it's all there, that was kinda the entire point of component,
> > so we don't
Re: [PATCH v8 06/35] drm/i915: Enable and Disable of HDCP2.2
On 12/6/2018 4:00 PM, Daniel Vetter wrote:
On Tue, Nov 27, 2018 at 04:13:04PM +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.
When HDCP2.2 enabling fails and HDCP1.4 is supported, HDCP1.4 is
enabled.
This change implements a sequence of enabling and disabling of
HDCP2.2 authentication and HDCP2.2 port encryption.
Patch series suggestion for next time around: First build out the helper
functions, then time them into the big picture like here. Personally I
think that makes it easier to understand, but it's kinda personal choice.
Tried that already. But bisecting will give warnings as "defined but
unused functions"
Hence moved to this approach.
I guess this here works too.
v2:
Included few optimization suggestions [Chris Wilson]
Commit message is updated as per the rebased version.
intel_wait_for_register is used instead of wait_for. [Chris Wilson]
v3:
No changes.
v4:
Extra comment added and Style issue fixed [Uma]
v5:
Rebased as part of patch reordering.
HDCP2 encryption status is tracked.
HW state check is moved into WARN_ON [Daniel]
v6:
Redefined the mei service functions as per comp redesign.
Merged patches related to hdcp2.2 enabling and disabling [Sean Paul].
Required shim functionality is defined [Sean Paul]
v7:
Return values are handles [Uma]
Realigned the code.
Check for comp_master is removed.
v8:
HDCP2.2 is attempted only if mei interface is up.
Adjust to the new interface
Avoid bool usage in struct [Tomas]
Signed-off-by: Ramalingam C
---
drivers/gpu/drm/i915/intel_drv.h | 5 +
drivers/gpu/drm/i915/intel_hdcp.c | 223 +++---
2 files changed, 214 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bde82f3ada85..3e9f21d23442 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -383,6 +383,10 @@ struct intel_hdcp_shim {
/* Detects the HDCP protocol(DP/HDMI) required on the port */
enum mei_hdcp_wired_protocol (*hdcp_protocol)(void);
+
+ /* Detects whether Panel is HDCP2.2 capable */
+ int (*hdcp_2_2_capable)(struct intel_digital_port *intel_dig_port,
+ bool *capable);
};
struct intel_hdcp {
@@ -396,6 +400,7 @@ struct intel_hdcp {
/* HDCP2.2 related definitions */
/* Flag indicates whether this connector supports HDCP2.2 or not. */
u8 hdcp2_supported;
+ u8 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 760780f1105c..c1bd1ccd47cd 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -79,6 +79,43 @@ bool intel_hdcp_capable(struct intel_connector *connector)
return capable;
}
+/* At present whether mei_hdcp component is binded with i915 master component */
+static bool intel_hdcp2_mei_binded(struct intel_connector *connector)
+{
+ struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
+ struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp;
+
+ mutex_lock(>mutex);
+ if (!comp->ops || !comp->dev) {
+ mutex_unlock(>mutex);
+ return false;
+ }
+ mutex_unlock(>mutex);
+
+ return true;
+}
+
+/* Is HDCP2.2 capable on Platform and Sink */
+static bool intel_hdcp2_capable(struct intel_connector *connector)
+{
+ struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
+ struct intel_hdcp *hdcp = >hdcp;
+ bool capable = false;
+
+ /* I915 support for HDCP2.2 */
+ if (!hdcp->hdcp2_supported)
+ return false;
+
+ /* MEI services for HDCP2.2 */
+ if (!intel_hdcp2_mei_binded(connector))
+ return false;
Why do we still need this with component? Driver load should be stalled
out until it's all there, that was kinda the entire point of component,
so we don't have to recheck all the time whether hdcp2 is still there or
not.
We discussed this in previous patches. Lets decide whether this approach
is good enough or not.
--Ram
+
+ /* Sink's capability for HDCP2.2 */
+ hdcp->shim->hdcp_2_2_capable(intel_dig_port, );
+
+ return capable;
+}
+
static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,
const struct intel_hdcp_shim *shim)
{
@@ -1114,8 +1151,7 @@ int hdcp2_authenticate_port(struct intel_connector
*connector)
return ret;
}
-static __attribute__((unused))
-int hdcp2_close_mei_session(struct intel_connector *connector)
+static int hdcp2_close_mei_session(struct intel_connector *connector)
{
struct mei_hdcp_data *data = >hdcp.mei_data;
Re: [PATCH v8 06/35] drm/i915: Enable and Disable of HDCP2.2
On Tue, Nov 27, 2018 at 04:13:04PM +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.
>
> When HDCP2.2 enabling fails and HDCP1.4 is supported, HDCP1.4 is
> enabled.
>
> This change implements a sequence of enabling and disabling of
> HDCP2.2 authentication and HDCP2.2 port encryption.
Patch series suggestion for next time around: First build out the helper
functions, then time them into the big picture like here. Personally I
think that makes it easier to understand, but it's kinda personal choice.
I guess this here works too.
>
> v2:
> Included few optimization suggestions [Chris Wilson]
> Commit message is updated as per the rebased version.
> intel_wait_for_register is used instead of wait_for. [Chris Wilson]
> v3:
> No changes.
> v4:
> Extra comment added and Style issue fixed [Uma]
> v5:
> Rebased as part of patch reordering.
> HDCP2 encryption status is tracked.
> HW state check is moved into WARN_ON [Daniel]
> v6:
> Redefined the mei service functions as per comp redesign.
> Merged patches related to hdcp2.2 enabling and disabling [Sean Paul].
> Required shim functionality is defined [Sean Paul]
> v7:
> Return values are handles [Uma]
> Realigned the code.
> Check for comp_master is removed.
> v8:
> HDCP2.2 is attempted only if mei interface is up.
> Adjust to the new interface
> Avoid bool usage in struct [Tomas]
>
> Signed-off-by: Ramalingam C
> ---
> drivers/gpu/drm/i915/intel_drv.h | 5 +
> drivers/gpu/drm/i915/intel_hdcp.c | 223
> +++---
> 2 files changed, 214 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index bde82f3ada85..3e9f21d23442 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -383,6 +383,10 @@ struct intel_hdcp_shim {
>
> /* Detects the HDCP protocol(DP/HDMI) required on the port */
> enum mei_hdcp_wired_protocol (*hdcp_protocol)(void);
> +
> + /* Detects whether Panel is HDCP2.2 capable */
> + int (*hdcp_2_2_capable)(struct intel_digital_port *intel_dig_port,
> + bool *capable);
> };
>
> struct intel_hdcp {
> @@ -396,6 +400,7 @@ struct intel_hdcp {
> /* HDCP2.2 related definitions */
> /* Flag indicates whether this connector supports HDCP2.2 or not. */
> u8 hdcp2_supported;
> + u8 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 760780f1105c..c1bd1ccd47cd 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -79,6 +79,43 @@ bool intel_hdcp_capable(struct intel_connector *connector)
> return capable;
> }
>
> +/* At present whether mei_hdcp component is binded with i915 master
> component */
> +static bool intel_hdcp2_mei_binded(struct intel_connector *connector)
> +{
> + struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> + struct i915_hdcp_component_master *comp = dev_priv->hdcp_comp;
> +
> + mutex_lock(>mutex);
> + if (!comp->ops || !comp->dev) {
> + mutex_unlock(>mutex);
> + return false;
> + }
> + mutex_unlock(>mutex);
> +
> + return true;
> +}
> +
> +/* Is HDCP2.2 capable on Platform and Sink */
> +static bool intel_hdcp2_capable(struct intel_connector *connector)
> +{
> + struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
> + struct intel_hdcp *hdcp = >hdcp;
> + bool capable = false;
> +
> + /* I915 support for HDCP2.2 */
> + if (!hdcp->hdcp2_supported)
> + return false;
> +
> + /* MEI services for HDCP2.2 */
> + if (!intel_hdcp2_mei_binded(connector))
> + return false;
Why do we still need this with component? Driver load should be stalled
out until it's all there, that was kinda the entire point of component,
so we don't have to recheck all the time whether hdcp2 is still there or
not.
> +
> + /* Sink's capability for HDCP2.2 */
> + hdcp->shim->hdcp_2_2_capable(intel_dig_port, );
> +
> + return capable;
> +}
> +
> static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port
> *intel_dig_port,
> const struct intel_hdcp_shim *shim)
> {
> @@ -1114,8 +1151,7 @@ int hdcp2_authenticate_port(struct intel_connector
> *connector)
> return ret;
> }
>
> -static __attribute__((unused))
> -int hdcp2_close_mei_session(struct intel_connector *connector)
> +static int hdcp2_close_mei_session(struct intel_connector *connector)
> {
> struct mei_hdcp_data *data = >hdcp.mei_data;
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> @@ -1137,12 +1173,157 @@ int
