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.
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 <ramalinga...@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
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
