Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753212AbdLERMM (ORCPT ); Tue, 5 Dec 2017 12:12:12 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:39484 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752357AbdLERMF (ORCPT ); Tue, 5 Dec 2017 12:12:05 -0500 X-Google-Smtp-Source: AGs4zMYXKtoDy5rH2+uL2wiQCx8EsODqgU8deAFkdGYdevZsIJHRfJmi+6UhQrdfW+iloy6KD3hhOg== Date: Tue, 5 Dec 2017 18:12:01 +0100 From: Daniel Vetter To: Sean Paul Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, seanpaul@google.com, David Airlie , Joonas Lahtinen , linux-kernel@vger.kernel.org, Rodrigo Vivi , daniel.vetter@intel.com Subject: Re: [PATCH v3 9/9] drm/i915: Implement HDCP for DisplayPort Message-ID: <20171205171201.baqimw6kujav3z43@phenom.ffwll.local> Mail-Followup-To: Sean Paul , dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, seanpaul@google.com, David Airlie , Joonas Lahtinen , linux-kernel@vger.kernel.org, Rodrigo Vivi , daniel.vetter@intel.com References: <20171205051513.8603-1-seanpaul@chromium.org> <20171205051513.8603-10-seanpaul@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171205051513.8603-10-seanpaul@chromium.org> X-Operating-System: Linux phenom 4.13.0-1-amd64 User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 12059 Lines: 363 On Tue, Dec 05, 2017 at 12:15:08AM -0500, Sean Paul wrote: > This patch adds HDCP support for DisplayPort connectors by implementing > the intel_hdcp_shim. > > Most of this is straightforward read/write from/to DPCD registers. One > thing worth pointing out is the Aksv output bit. It wasn't easily > separable like it's HDMI counterpart, so it's crammed in with the rest > of it. > > Changes in v2: > - Moved intel_hdcp_check_link out of intel_dp_check_link and only call > it on short pulse. Since intel_hdcp_check_link does its own locking, > this ensures we don't deadlock when intel_dp_check_link is called > holding connection_mutex. > - Rebased on drm-intel-next > Changes in v3: > - Initialize new worker > - Move intel_hdcp_check_link further out to avoid calling it while > holding _any_ locks > > Signed-off-by: Sean Paul > --- > drivers/gpu/drm/i915/intel_dp.c | 248 ++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 241 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index c603d4c903e1..dc303e18c1dd 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -36,7 +36,9 @@ > #include > #include > #include > +#include > #include > +#include > #include "intel_drv.h" > #include > #include "i915_drv.h" > @@ -1025,10 +1027,29 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp, > DP_AUX_CH_CTL_SYNC_PULSE_SKL(32); > } > > +static uint32_t intel_dp_get_aux_send_ctl(struct intel_dp *intel_dp, > + bool has_aux_irq, > + int send_bytes, > + uint32_t aux_clock_divider, > + bool aksv_write) > +{ > + uint32_t val = 0; > + > + if (aksv_write) { > + send_bytes += 5; > + val |= DP_AUX_CH_CTL_AUX_AKSV_SELECT; > + } > + > + return val | intel_dp->get_aux_send_ctl(intel_dp, > + has_aux_irq, > + send_bytes, > + aux_clock_divider); > +} > + > static int > intel_dp_aux_ch(struct intel_dp *intel_dp, > const uint8_t *send, int send_bytes, > - uint8_t *recv, int recv_size) > + uint8_t *recv, int recv_size, bool aksv_write) > { > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > struct drm_i915_private *dev_priv = > @@ -1088,10 +1109,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > } > > while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) { > - u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, > - has_aux_irq, > - send_bytes, > - aux_clock_divider); > + u32 send_ctl = intel_dp_get_aux_send_ctl(intel_dp, > + has_aux_irq, > + send_bytes, > + aux_clock_divider, > + aksv_write); > > /* Must try at least 3 times according to DP spec */ > for (try = 0; try < 5; try++) { > @@ -1228,7 +1250,8 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > if (msg->buffer) > memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size); > > - ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); > + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize, > + false); > if (ret > 0) { > msg->reply = rxbuf[0] >> 4; > > @@ -1250,7 +1273,8 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > if (WARN_ON(rxsize > 20)) > return -E2BIG; > > - ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); > + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize, > + false); > if (ret > 0) { > msg->reply = rxbuf[0] >> 4; > /* > @@ -4981,6 +5005,203 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder) > pps_unlock(intel_dp); > } > > +static > +int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port, > + u8 *an) > +{ > + struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base); > + uint8_t txbuf[4], rxbuf[2], reply = 0; > + ssize_t dpcd_ret; > + int ret; > + > + /* Output An first, that's easy */ > + dpcd_ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, DP_AUX_HDCP_AN, > + an, DRM_HDCP_AN_LEN); > + if (dpcd_ret != DRM_HDCP_AN_LEN) { > + DRM_ERROR("Failed to write An over DP/AUX (%ld)\n", dpcd_ret); > + return dpcd_ret >= 0 ? -EIO : dpcd_ret; > + } > + > + /* > + * Since Aksv is Oh-So-Secret, we can't access it in software. So in > + * order to get it on the wire, we need to create the AUX header as if > + * we were writing the data, and then tickle the hardware to output the > + * data once the header is sent out. > + */ > + txbuf[0] = (DP_AUX_NATIVE_WRITE << 4) | > + ((DP_AUX_HDCP_AKSV >> 16) & 0xf); > + txbuf[1] = (DP_AUX_HDCP_AKSV >> 8) & 0xff; > + txbuf[2] = DP_AUX_HDCP_AKSV & 0xff; > + txbuf[3] = DRM_HDCP_KSV_LEN - 1; > + > + ret = intel_dp_aux_ch(intel_dp, txbuf, sizeof(txbuf), rxbuf, > + sizeof(rxbuf), true); > + if (ret < 0) { > + DRM_ERROR("Write Aksv over DP/AUX failed (%d)\n", ret); > + return ret; > + } else if (ret == 0) { > + DRM_ERROR("Aksv write over DP/AUX was empty\n"); > + return -EIO; > + } > + > + reply = (rxbuf[0] >> 4) & DP_AUX_NATIVE_REPLY_MASK; > + return reply == DP_AUX_NATIVE_REPLY_ACK ? 0 : -EIO; > +} > + > +static int intel_dp_hdcp_read_bksv(struct intel_digital_port *intel_dig_port, > + u8 *bksv) > +{ > + ssize_t ret; > + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BKSV, bksv, > + DRM_HDCP_KSV_LEN); > + if (ret != DRM_HDCP_KSV_LEN) { > + DRM_ERROR("Read Bksv from DP/AUX failed (%ld)\n", ret); > + return ret >= 0 ? -EIO : ret; > + } > + return 0; > +} > + > +static int intel_dp_hdcp_read_bstatus(struct intel_digital_port *intel_dig_port, > + u8 *bstatus) > +{ > + ssize_t ret; > + /* > + * For some reason the HDMI and DP HDCP specs call this register > + * definition by different names. In the HDMI spec, it's called BSTATUS, > + * but in DP it's called BINFO. > + */ > + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BINFO, > + bstatus, DRM_HDCP_BSTATUS_LEN); > + if (ret != DRM_HDCP_BSTATUS_LEN) { > + DRM_ERROR("Read bstatus from DP/AUX failed (%ld)\n", ret); > + return ret >= 0 ? -EIO : ret; > + } > + return 0; > +} > + > +static > +int intel_dp_hdcp_repeater_present(struct intel_digital_port *intel_dig_port, > + bool *repeater_present) > +{ > + ssize_t ret; > + u8 bcaps; > + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, DP_AUX_HDCP_BCAPS, > + &bcaps, 1); > + if (ret != 1) { > + DRM_ERROR("Read bcaps from DP/AUX failed (%ld)\n", ret); > + return ret >= 0 ? -EIO : ret; > + } > + *repeater_present = bcaps & DP_BCAPS_REPEATER_PRESENT; > + return 0; > +} > + > +static > +int intel_dp_hdcp_read_ri_prime(struct intel_digital_port *intel_dig_port, > + u8 *ri_prime) > +{ > + ssize_t 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) { > + DRM_ERROR("Read Ri' from DP/AUX failed (%ld)\n", ret); > + return ret >= 0 ? -EIO : ret; > + } > + return 0; > +} > + > +static > +int intel_dp_hdcp_read_ksv_ready(struct intel_digital_port *intel_dig_port, > + bool *ksv_ready) > +{ > + ssize_t ret; > + u8 bstatus; > + 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 (%ld)\n", ret); > + return ret >= 0 ? -EIO : ret; > + } > + *ksv_ready = bstatus & DP_BSTATUS_READY; > + return 0; > +} > + > +static > +int intel_dp_hdcp_read_ksv_fifo(struct intel_digital_port *intel_dig_port, > + int num_downstream, u8 *ksv_fifo) > +{ > + ssize_t ret; > + int i; > + > + // KSV list is read via 15 byte window (3 entries @ 5 bytes each) Spotted the one (I hope, maybe I didn't check stuff perfectly) C++ style comment! > + for (i = 0; i < num_downstream; i += 3) { > + size_t len = min(num_downstream - i, 3) * DRM_HDCP_KSV_LEN; > + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, > + DP_AUX_HDCP_KSV_FIFO, > + ksv_fifo + i * DRM_HDCP_KSV_LEN, > + len); > + if (ret != len) { > + DRM_ERROR("Read ksv[%d] from DP/AUX failed (%ld)\n", i, > + ret); > + return ret >= 0 ? -EIO : ret; > + } > + } > + return 0; > +} > + > +static > +int intel_dp_hdcp_read_v_prime_part(struct intel_digital_port *intel_dig_port, > + int i, u32 *part) > +{ > + ssize_t ret; > + > + if (i >= DRM_HDCP_V_PRIME_NUM_PARTS) > + return -EINVAL; > + > + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, > + DP_AUX_HDCP_V_PRIME(i), part, > + DRM_HDCP_V_PRIME_PART_LEN); > + if (ret != DRM_HDCP_V_PRIME_PART_LEN) { > + DRM_ERROR("Read v'[%d] from DP/AUX failed (%ld)\n", i, ret); > + return ret >= 0 ? -EIO : ret; > + } > + return 0; > +} > + > +static > +int intel_dp_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port, > + bool enable) > +{ > + /* Not used for single stream DisplayPort setups */ > + return 0; Fun. I didn't really spot this behaviour difference compared to earlier paltforms, where you had to (iirc) toggle bit 11 in the DP register. > +} > + > +static > +bool intel_dp_hdcp_check_link(struct intel_digital_port *intel_dig_port) > +{ > + ssize_t ret; > + u8 bstatus; > + 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 (%ld)\n", ret); > + return ret >= 0 ? -EIO : ret; > + } > + return !(bstatus & DP_BSTATUS_LINK_FAILURE); > +} > + > +static const struct intel_hdcp_shim intel_dp_hdcp_shim = { > + .write_an_aksv = intel_dp_hdcp_write_an_aksv, > + .read_bksv = intel_dp_hdcp_read_bksv, > + .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, > + .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, > + .check_link = intel_dp_hdcp_check_link, > +}; > + > static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp) > { > struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp)); > @@ -5146,6 +5367,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) > drm_modeset_acquire_fini(&ctx); > 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 (!handled) { > intel_dp->detect_done = false; > goto put_power; > @@ -6121,6 +6345,16 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > > intel_dp_add_properties(intel_dp, connector); > > + if (INTEL_GEN(dev_priv) >= 9 && !intel_dp_is_edp(intel_dp)) { > + drm_connector_attach_content_protection_property(connector); > + intel_connector->hdcp_shim = &intel_dp_hdcp_shim; > + mutex_init(&intel_connector->hdcp_mutex); > + INIT_DELAYED_WORK(&intel_connector->hdcp_check_work, > + intel_hdcp_check_work); > + INIT_WORK(&intel_connector->hdcp_prop_work, > + intel_hdcp_prop_work); > + } Same refactor request as for the hdmi patch here. Either way: Reviewed-by: Daniel Vetter > + > /* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be written > * 0xd. Failure to do so will result in spurious interrupts being > * generated on the port when a cable is not attached. > -- > 2.15.0.531.g2ccb3012c9-goog > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch