Re: [PATCH 5/5] drm/i915: Move hdcp msg detection into shim

On Monday 26 February 2018 11:31 PM, Sean Paul wrote:

On Mon, Feb 26, 2018 at 10:42:39PM +0530, Ramalingam C wrote:

...

DP and HDMI HDCP specifications are varying with respect to detecting the R0 and ksv_fifo availability.

DP will produce CP_IRQ and set a bit for indicating the R0 and FIFO_READY status.

I'm not sure what the benefit is? Keeping things common was a deliberate decision on my part to keep complexity down and increase the amount of common code.

I am proposing this expecting that this will address the DP compliance failure incase of regular repeater tests(attempt to read wrong FIFO size, due to wrong device count). Possibly a corner case!?.

And since anyway we have the shim for handling the DP and HDMI specific code, i feel this will do only better but harm.

--Ram

Sean

...

Whereas HDMI will set a bit for FIFO_READY but there is no bit indication for R0 ready. And also polling of READY bit is needed for HDMI as ther is no CP_IRQ.

So Fielding the CP_IRQ for DP and the polling the READY bit for a period and blindly waiting for a fixed time for R0 incase of HDMI are moved into corresponding hdcp_shim.

Signed-off-by: Ramalingam C ramalingam.c@intel.com

drivers/gpu/drm/i915/intel_dp.c | 70 +++++++++++++++++++++++++++++++++++---- drivers/gpu/drm/i915/intel_drv.h | 6 ++-- drivers/gpu/drm/i915/intel_hdcp.c | 39 ++-------------------- drivers/gpu/drm/i915/intel_hdmi.c | 28 +++++++++++++++- 4 files changed, 98 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 2c3eb90b9499..afe310a91d3c 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4436,6 +4436,7 @@ static bool intel_dp_short_pulse(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));

• struct intel_connector *connector = intel_dp->attached_connector; u8 sink_irq_vector = 0; u8 old_sink_count = intel_dp->sink_count; bool ret;

@@ -4470,8 +4471,11 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)

if (sink_irq_vector & DP_AUTOMATED_TEST_REQUEST)
	intel_dp_handle_test_request(intel_dp);

• if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
  

• 	DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
  

• if (sink_irq_vector & DP_CP_IRQ)
  

• 	if (connector->hdcp_shim)
  

• 		complete_all(&connector->cp_irq_recved);
  

• if (sink_irq_vector & DP_SINK_SPECIFIC_IRQ)
  

• 	DRM_DEBUG_DRIVER("Sink specific irq unhandled\n");
  

  }

  intel_dp_check_link_status(intel_dp);

@@ -5083,6 +5087,25 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder) pps_unlock(intel_dp); }

+static int intel_dp_hdcp_wait_for_cp_irq(struct completion *cp_irq_recved,

• 			 int timeout)
  

+{

• long ret;
• if (completion_done(cp_irq_recved))

• reinit_completion(cp_irq_recved);
  

• ret = wait_for_completion_interruptible_timeout(cp_irq_recved,

• 					msecs_to_jiffies(
  

• 					timeout));
  

• reinit_completion(cp_irq_recved);
• if (ret < 0)

• return (int)ret;
  

• else if (!ret)

• return -ETIMEDOUT;
  

• return 0;

+}

• static int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port, u8 *an)

@@ -5188,11 +5211,38 @@ int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port, return 0; }

+static int intel_dp_hdcp_wait_for_r0(struct intel_digital_port *intel_dig_port) +{

• struct intel_dp *dp = &intel_dig_port->dp;
• struct intel_connector *connector = dp->attached_connector;
• int ret;
• u8 bstatus;
• intel_dp_hdcp_wait_for_cp_irq(&connector->cp_irq_recved, 100);
• ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BSTATUS,

• 	       &bstatus, 1);
  

• if (ret != 1) {

• DRM_ERROR("Read bstatus from DP/AUX failed (%d)\n", ret);
  

• return ret >= 0 ? -EIO : ret;
  

• }
• if (!(bstatus & DP_BSTATUS_R0_PRIME_READY))

• return -ETIMEDOUT;
  

• return 0;

+}

• static int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port, u8 *ri_prime) { ssize_t ret;
• ret = intel_dp_hdcp_wait_for_r0(intel_dig_port);
• if (!ret)

• return ret;
  

• ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_RI_PRIME, ri_prime, DRM_HDCP_RI_LEN); if (ret != DRM_HDCP_RI_LEN) {

@@ -5203,18 +5253,26 @@ int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port, }

static -int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port,

• 		 bool *ksv_ready)
  

+int intel_dp_hdcp_ksv_ready(struct intel_digital_port *intel_dig_port) {

• struct intel_dp *dp = &intel_dig_port->dp;
• struct intel_connector *connector = dp->attached_connector; ssize_t ret; u8 bstatus;
• intel_dp_hdcp_wait_for_cp_irq(&connector->cp_irq_recved,

• 		      5 * 1000 * 1000);
  

• ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BSTATUS, &bstatus, 1); if (ret != 1) { DRM_ERROR("Read bstatus from DP/AUX failed (%zd)\n", ret); return ret >= 0 ? -EIO : ret; }

• *ksv_ready = bstatus & DP_BSTATUS_READY;

• if (!(bstatus & DP_BSTATUS_READY))

• return -ETIMEDOUT;
  

• return 0; }

@@ -5305,7 +5363,7 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = { .read_bstatus = intel_dp_hdcp_read_bstatus, .repeater_present = intel_dp_hdcp_repeater_present, .read_ri_prime = intel_dp_hdcp_read_ri_prime,

• .read_ksv_ready = intel_dp_hdcp_read_ksv_ready,

• .wait_for_ksv_ready = intel_dp_hdcp_ksv_ready, .read_ksv_fifo = intel_dp_hdcp_read_ksv_fifo, .read_v_prime_part = intel_dp_hdcp_read_v_prime_part, .toggle_signalling = intel_dp_hdcp_toggle_signalling,

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8f38e584d375..ab975107c978 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -353,8 +353,7 @@ struct intel_hdcp_shim { int (*read_ri_prime)(struct intel_digital_port *intel_dig_port, u8 *ri);

/* Determines if the receiver's KSV FIFO is ready for consumption */

• int (*read_ksv_ready)(struct intel_digital_port *intel_dig_port,

• 	      bool *ksv_ready);
  

• int (*wait_for_ksv_ready)(struct intel_digital_port *intel_dig_port);

  /* Reads the ksv fifo for num_downstream devices */ int (*read_ksv_fifo)(struct intel_digital_port *intel_dig_port,

@@ -413,6 +412,9 @@ struct intel_connector { uint64_t hdcp_value; /* protected by hdcp_mutex */ struct delayed_work hdcp_check_work; struct work_struct hdcp_prop_work;

• /* To indicate the assertion of CP_IRQ */

• struct completion cp_irq_recved; };

  struct intel_digital_connector_state {

diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c index 14be14a45e5a..a077e1333886 100644 --- a/drivers/gpu/drm/i915/intel_hdcp.c +++ b/drivers/gpu/drm/i915/intel_hdcp.c @@ -57,27 +57,6 @@ static bool wait_for_hdcp_port(struct intel_connector *connector) return true; }

-static int intel_hdcp_poll_ksv_fifo(struct intel_digital_port *intel_dig_port,

• 		    const struct intel_hdcp_shim *shim)
  

-{

• int ret, read_ret;
• bool ksv_ready;
• /* Poll for ksv list ready (spec says max time allowed is 5s) */
• ret = __wait_for(read_ret = shim->read_ksv_ready(intel_dig_port,

• 					 &ksv_ready),
  

• 	 read_ret || ksv_ready, 5 * 1000 * 1000, 1000,
  

• 	 100 * 1000);
  

• if (ret)

• return ret;
  

• if (read_ret)

• return read_ret;
  

• if (!ksv_ready)

• return -ETIMEDOUT;
  

• return 0;

-}

• static bool hdcp_key_loadable(struct drm_i915_private *dev_priv) { struct i915_power_domains *power_domains = &dev_priv->power_domains;

@@ -222,7 +201,7 @@ int intel_hdcp_auth_downstream(struct intel_digital_port *intel_dig_port,

dev_priv = intel_dig_port->base.base.dev->dev_private;

• ret = intel_hdcp_poll_ksv_fifo(intel_dig_port, shim);

• ret = shim->wait_for_ksv_ready(intel_dig_port); if (ret) { DRM_ERROR("KSV list failed to become ready (%d)\n", ret); return ret;

@@ -476,7 +455,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port, { struct drm_i915_private *dev_priv; enum port port;

• unsigned long r0_prime_gen_start; int ret, i, tries = 2; union { u32 reg[2];

@@ -531,8 +509,6 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port, if (ret) return ret;

• r0_prime_gen_start = jiffies;

• memset(&bksv, 0, sizeof(bksv));

  /* HDCP spec states that we must retry the bksv if it is invalid */

@@ -572,22 +548,11 @@ static int intel_hdcp_auth(struct intel_digital_port *intel_dig_port, }

/*

• * Wait for R0' to become available. The spec says minimum 100ms from
  

• * Aksv, but some monitors can take longer than this. So we are
  

• * combinely waiting for 300mSec just to be sure in case of HDMI.
  

  • DP HDCP Spec mandates the two more reattempt to read R0, incase
  • of R0 mismatch.

• *
  

• * On DP, there's an R0_READY bit available but no such bit
  

• * exists on HDMI. Since the upper-bound is the same, we'll just do
  

• * the stupid thing instead of polling on one and not the other.
  

  */
• tries = 3; for (i = 0; i < tries; i++) {

• wait_remaining_ms_from_jiffies(r0_prime_gen_start,
  

• 			       100 * (i + 1));
  

• ri.reg = 0; ret = shim->read_ri_prime(intel_dig_port, ri.shim); if (ret)

@@ -749,6 +714,8 @@ int intel_hdcp_init(struct intel_connector *connector, mutex_init(&connector->hdcp_mutex); INIT_DELAYED_WORK(&connector->hdcp_check_work, intel_hdcp_check_work); INIT_WORK(&connector->hdcp_prop_work, intel_hdcp_prop_work);

• init_completion(&connector->cp_irq_recved); return 0; }

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index f5d7bfb43006..532d13f91602 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1007,6 +1007,9 @@ int intel_hdmi_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port, u8 *ri_prime) { int ret;

• wait_remaining_ms_from_jiffies(jiffies, 100);
• ret = intel_hdmi_hdcp_read(intel_dig_port, DRM_HDCP_DDC_RI_PRIME, ri_prime, DRM_HDCP_RI_LEN); if (ret)

@@ -1027,6 +1030,29 @@ int intel_hdmi_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port, return ret; } *ksv_ready = val & DRM_HDCP_DDC_BCAPS_KSV_FIFO_READY;

• return 0;

+}

+static +int intel_hdmi_hdcp_ksv_ready(struct intel_digital_port *intel_dig_port) +{

• int ret, read_ret;
• bool ksv_ready;
• /* Poll for ksv list ready (spec says max time allowed is 5s) */
• ret = __wait_for(read_ret =

• 	 intel_hdmi_hdcp_read_ksv_ready(intel_dig_port,
  

• 					&ksv_ready),
  

• 	 read_ret || ksv_ready, 5 * 1000 * 1000, 1000,
  

• 	 100 * 1000);
  

• if (ret)

• return ret;
  

• if (read_ret)

• return read_ret;
  

• if (!ksv_ready)

• return -ETIMEDOUT;
  

• return 0; }

@@ -1112,7 +1138,7 @@ static const struct intel_hdcp_shim intel_hdmi_hdcp_shim = { .read_bstatus = intel_hdmi_hdcp_read_bstatus, .repeater_present = intel_hdmi_hdcp_repeater_present, .read_ri_prime = intel_hdmi_hdcp_read_ri_prime,

• .read_ksv_ready = intel_hdmi_hdcp_read_ksv_ready,

• .wait_for_ksv_ready = intel_hdmi_hdcp_ksv_ready, .read_ksv_fifo = intel_hdmi_hdcp_read_ksv_fifo, .read_v_prime_part = intel_hdmi_hdcp_read_v_prime_part, .toggle_signalling = intel_hdmi_hdcp_toggle_signalling,

-- 2.7.4