Re: [Intel-gfx] [PATCH 23/43] drm/i915: wrapping all hdcp var into intel_hdcp

[Ramalingam C]

On Thursday 22 February 2018 08:17 PM, Sean Paul wrote:

On Wed, Feb 14, 2018 at 07:43:38PM +0530, Ramalingam C wrote:

Considering the upcoming significant no HDCP2.2 variables, it will
be clean to have separate struct fo HDCP.

New structure called intel_hdcp is introduced.

Signed-off-by: Ramalingam C 
---
  drivers/gpu/drm/i915/intel_display.c |   9 ++-
  drivers/gpu/drm/i915/intel_dp.c  |   4 +-
  drivers/gpu/drm/i915/intel_drv.h |  20 +++--
  drivers/gpu/drm/i915/intel_hdcp.c| 141 ---
  4 files changed, 100 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 048d60b5143b..22097349529b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15314,10 +15314,11 @@ static void intel_hpd_poll_fini(struct drm_device 
*dev)
for_each_intel_connector_iter(connector, _iter) {
if (connector->modeset_retry_work.func)
cancel_work_sync(>modeset_retry_work);
-   if (connector->hdcp_shim) {
-   cancel_delayed_work_sync(>hdcp_check_work);
-   cancel_work_sync(>hdcp_prop_work);
-   cancel_work_sync(>hdcp_enable_work);
+   if (connector->hdcp && connector->hdcp->hdcp_shim) {
+   cancel_delayed_work_sync(
+   >hdcp->hdcp_check_work);
+   cancel_work_sync(>hdcp->hdcp_prop_work);
+   cancel_work_sync(>hdcp->hdcp_enable_work);
}
}
drm_connector_list_iter_end(_iter);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f20b25f98e5a..80476689754f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5445,7 +5445,9 @@ intel_dp_hpd_pulse(struct intel_digital_port 
*intel_dig_port, bool long_hpd)
WARN(iret, "Acquiring modeset locks failed with %i\n", iret);
  
  		/* Short pulse can signify loss of hdcp authentication */

-   intel_hdcp_check_link(intel_dp->attached_connector);
+   if (intel_dp->attached_connector->hdcp)
+   intel_hdcp_check_link(
+   intel_dp->attached_connector->hdcp);
  
  		if (!handled) {

intel_dp->detect_done = false;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7b9e5f70826f..5b170ff7ec14 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -375,6 +375,17 @@ struct intel_hdcp_shim {
bool *hdcp_capable);
  };
  
+struct intel_hdcp {

+   struct intel_connector *connector;

You don't need a backpointer, use container_of if necessary.

ok sure.



+
+   const struct intel_hdcp_shim *hdcp_shim;
+   struct mutex hdcp_mutex;
+   uint64_t hdcp_value; /* protected by hdcp_mutex */
+   struct delayed_work hdcp_check_work;
+   struct work_struct hdcp_prop_work;
+   struct work_struct hdcp_enable_work;
+};
+
  struct intel_connector {
struct drm_connector base;
/*
@@ -407,12 +418,7 @@ struct intel_connector {
/* Work struct to schedule a uevent on link train failure */
struct work_struct modeset_retry_work;
  
-	const struct intel_hdcp_shim *hdcp_shim;

-   struct mutex hdcp_mutex;
-   uint64_t hdcp_value; /* protected by hdcp_mutex */
-   struct delayed_work hdcp_check_work;
-   struct work_struct hdcp_prop_work;
-   struct work_struct hdcp_enable_work;
+   struct intel_hdcp *hdcp;
  };
  
  struct intel_digital_connector_state {

@@ -1853,7 +1859,7 @@ int intel_hdcp_init(struct intel_connector *connector,
const struct intel_hdcp_shim *hdcp_shim);
  int intel_hdcp_enable(struct intel_connector *connector);
  int intel_hdcp_disable(struct intel_connector *connector);
-int intel_hdcp_check_link(struct intel_connector *connector);
+int intel_hdcp_check_link(struct intel_hdcp *hdcp);
  bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
  
  /* intel_psr.c */

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c 
b/drivers/gpu/drm/i915/intel_hdcp.c
index e03bd376d92c..9147fb17a9fc 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -545,8 +545,9 @@ struct intel_digital_port *conn_to_dig_port(struct 
intel_connector *connector)
return enc_to_dig_port(_attached_encoder(>base)->base);
  }
  
-static int _intel_hdcp_disable(struct intel_connector *connector)

+static int _intel_hdcp_disable(struct intel_hdcp *hdcp)
  {
+   struct intel_connector *connector = hdcp->connector;

I'm not sure why we need to change all of the function arguments to intel_hdcp
if the first thing we do is fetch the connector.


struct 

Re: [Intel-gfx] [PATCH 23/43] drm/i915: wrapping all hdcp var into intel_hdcp

[Sean Paul]

On Wed, Feb 14, 2018 at 07:43:38PM +0530, Ramalingam C wrote:
> Considering the upcoming significant no HDCP2.2 variables, it will
> be clean to have separate struct fo HDCP.
> 
> New structure called intel_hdcp is introduced.
> 
> Signed-off-by: Ramalingam C 
> ---
>  drivers/gpu/drm/i915/intel_display.c |   9 ++-
>  drivers/gpu/drm/i915/intel_dp.c  |   4 +-
>  drivers/gpu/drm/i915/intel_drv.h |  20 +++--
>  drivers/gpu/drm/i915/intel_hdcp.c| 141 
> ---
>  4 files changed, 100 insertions(+), 74 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 048d60b5143b..22097349529b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15314,10 +15314,11 @@ static void intel_hpd_poll_fini(struct drm_device 
> *dev)
>   for_each_intel_connector_iter(connector, _iter) {
>   if (connector->modeset_retry_work.func)
>   cancel_work_sync(>modeset_retry_work);
> - if (connector->hdcp_shim) {
> - cancel_delayed_work_sync(>hdcp_check_work);
> - cancel_work_sync(>hdcp_prop_work);
> - cancel_work_sync(>hdcp_enable_work);
> + if (connector->hdcp && connector->hdcp->hdcp_shim) {
> + cancel_delayed_work_sync(
> + >hdcp->hdcp_check_work);
> + cancel_work_sync(>hdcp->hdcp_prop_work);
> + cancel_work_sync(>hdcp->hdcp_enable_work);
>   }
>   }
>   drm_connector_list_iter_end(_iter);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f20b25f98e5a..80476689754f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5445,7 +5445,9 @@ intel_dp_hpd_pulse(struct intel_digital_port 
> *intel_dig_port, bool long_hpd)
>   WARN(iret, "Acquiring modeset locks failed with %i\n", iret);
>  
>   /* Short pulse can signify loss of hdcp authentication */
> - intel_hdcp_check_link(intel_dp->attached_connector);
> + if (intel_dp->attached_connector->hdcp)
> + intel_hdcp_check_link(
> + intel_dp->attached_connector->hdcp);
>  
>   if (!handled) {
>   intel_dp->detect_done = false;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 7b9e5f70826f..5b170ff7ec14 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -375,6 +375,17 @@ struct intel_hdcp_shim {
>   bool *hdcp_capable);
>  };
>  
> +struct intel_hdcp {
> + struct intel_connector *connector;

You don't need a backpointer, use container_of if necessary.

> +
> + const struct intel_hdcp_shim *hdcp_shim;
> + struct mutex hdcp_mutex;
> + uint64_t hdcp_value; /* protected by hdcp_mutex */
> + struct delayed_work hdcp_check_work;
> + struct work_struct hdcp_prop_work;
> + struct work_struct hdcp_enable_work;
> +};
> +
>  struct intel_connector {
>   struct drm_connector base;
>   /*
> @@ -407,12 +418,7 @@ struct intel_connector {
>   /* Work struct to schedule a uevent on link train failure */
>   struct work_struct modeset_retry_work;
>  
> - const struct intel_hdcp_shim *hdcp_shim;
> - struct mutex hdcp_mutex;
> - uint64_t hdcp_value; /* protected by hdcp_mutex */
> - struct delayed_work hdcp_check_work;
> - struct work_struct hdcp_prop_work;
> - struct work_struct hdcp_enable_work;
> + struct intel_hdcp *hdcp;
>  };
>  
>  struct intel_digital_connector_state {
> @@ -1853,7 +1859,7 @@ int intel_hdcp_init(struct intel_connector *connector,
>   const struct intel_hdcp_shim *hdcp_shim);
>  int intel_hdcp_enable(struct intel_connector *connector);
>  int intel_hdcp_disable(struct intel_connector *connector);
> -int intel_hdcp_check_link(struct intel_connector *connector);
> +int intel_hdcp_check_link(struct intel_hdcp *hdcp);
>  bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
>  
>  /* intel_psr.c */
> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c 
> b/drivers/gpu/drm/i915/intel_hdcp.c
> index e03bd376d92c..9147fb17a9fc 100644
> --- a/drivers/gpu/drm/i915/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/intel_hdcp.c
> @@ -545,8 +545,9 @@ struct intel_digital_port *conn_to_dig_port(struct 
> intel_connector *connector)
>   return enc_to_dig_port(_attached_encoder(>base)->base);
>  }
>  
> -static int _intel_hdcp_disable(struct intel_connector *connector)
> +static int _intel_hdcp_disable(struct intel_hdcp *hdcp)
>  {
> + struct intel_connector *connector = hdcp->connector;

I'm not sure why we need to change all of the function arguments to intel_hdcp
if the first 

[Intel-gfx] [PATCH 23/43] drm/i915: wrapping all hdcp var into intel_hdcp

[Ramalingam C]

Considering the upcoming significant no HDCP2.2 variables, it will
be clean to have separate struct fo HDCP.

New structure called intel_hdcp is introduced.

Signed-off-by: Ramalingam C 
---
 drivers/gpu/drm/i915/intel_display.c |   9 ++-
 drivers/gpu/drm/i915/intel_dp.c  |   4 +-
 drivers/gpu/drm/i915/intel_drv.h |  20 +++--
 drivers/gpu/drm/i915/intel_hdcp.c| 141 ---
 4 files changed, 100 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 048d60b5143b..22097349529b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15314,10 +15314,11 @@ static void intel_hpd_poll_fini(struct drm_device 
*dev)
for_each_intel_connector_iter(connector, _iter) {
if (connector->modeset_retry_work.func)
cancel_work_sync(>modeset_retry_work);
-   if (connector->hdcp_shim) {
-   cancel_delayed_work_sync(>hdcp_check_work);
-   cancel_work_sync(>hdcp_prop_work);
-   cancel_work_sync(>hdcp_enable_work);
+   if (connector->hdcp && connector->hdcp->hdcp_shim) {
+   cancel_delayed_work_sync(
+   >hdcp->hdcp_check_work);
+   cancel_work_sync(>hdcp->hdcp_prop_work);
+   cancel_work_sync(>hdcp->hdcp_enable_work);
}
}
drm_connector_list_iter_end(_iter);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f20b25f98e5a..80476689754f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5445,7 +5445,9 @@ intel_dp_hpd_pulse(struct intel_digital_port 
*intel_dig_port, bool long_hpd)
WARN(iret, "Acquiring modeset locks failed with %i\n", iret);
 
/* Short pulse can signify loss of hdcp authentication */
-   intel_hdcp_check_link(intel_dp->attached_connector);
+   if (intel_dp->attached_connector->hdcp)
+   intel_hdcp_check_link(
+   intel_dp->attached_connector->hdcp);
 
if (!handled) {
intel_dp->detect_done = false;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7b9e5f70826f..5b170ff7ec14 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -375,6 +375,17 @@ struct intel_hdcp_shim {
bool *hdcp_capable);
 };
 
+struct intel_hdcp {
+   struct intel_connector *connector;
+
+   const struct intel_hdcp_shim *hdcp_shim;
+   struct mutex hdcp_mutex;
+   uint64_t hdcp_value; /* protected by hdcp_mutex */
+   struct delayed_work hdcp_check_work;
+   struct work_struct hdcp_prop_work;
+   struct work_struct hdcp_enable_work;
+};
+
 struct intel_connector {
struct drm_connector base;
/*
@@ -407,12 +418,7 @@ struct intel_connector {
/* Work struct to schedule a uevent on link train failure */
struct work_struct modeset_retry_work;
 
-   const struct intel_hdcp_shim *hdcp_shim;
-   struct mutex hdcp_mutex;
-   uint64_t hdcp_value; /* protected by hdcp_mutex */
-   struct delayed_work hdcp_check_work;
-   struct work_struct hdcp_prop_work;
-   struct work_struct hdcp_enable_work;
+   struct intel_hdcp *hdcp;
 };
 
 struct intel_digital_connector_state {
@@ -1853,7 +1859,7 @@ int intel_hdcp_init(struct intel_connector *connector,
const struct intel_hdcp_shim *hdcp_shim);
 int intel_hdcp_enable(struct intel_connector *connector);
 int intel_hdcp_disable(struct intel_connector *connector);
-int intel_hdcp_check_link(struct intel_connector *connector);
+int intel_hdcp_check_link(struct intel_hdcp *hdcp);
 bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
 
 /* intel_psr.c */
diff --git a/drivers/gpu/drm/i915/intel_hdcp.c 
b/drivers/gpu/drm/i915/intel_hdcp.c
index e03bd376d92c..9147fb17a9fc 100644
--- a/drivers/gpu/drm/i915/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/intel_hdcp.c
@@ -545,8 +545,9 @@ struct intel_digital_port *conn_to_dig_port(struct 
intel_connector *connector)
return enc_to_dig_port(_attached_encoder(>base)->base);
 }
 
-static int _intel_hdcp_disable(struct intel_connector *connector)
+static int _intel_hdcp_disable(struct intel_hdcp *hdcp)
 {
+   struct intel_connector *connector = hdcp->connector;
struct drm_i915_private *dev_priv = connector->base.dev->dev_private;
struct intel_digital_port *intel_dig_port = conn_to_dig_port(connector);
enum port port = intel_dig_port->base.port;
@@ -562,7 +563,7 @@ static int _intel_hdcp_disable(struct intel_connector 
*connector)
return -ETIMEDOUT;
}
 
-