| Message ID | 1526907240-17639-39-git-send-email-ramalingam.c@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Ramalingam,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20180517]
[cannot apply to v4.17-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ramalingam-C/drm-i915-Implement-HDCP2-2/20180523-031938
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x000-201820 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
In file included from include/drm/drm_mm.h:49:0,
from include/drm/drmP.h:73,
from drivers/gpu//drm/i915/intel_dp.c:36:
drivers/gpu//drm/i915/intel_dp.c: In function 'intel_dp_hdcp2_read_rx_status':
>> drivers/gpu//drm/i915/intel_dp.c:5396:13: error: format '%ld' expects argument of type 'long int', but argument 2 has type 'ssize_t {aka int}' [-Werror=format=]
DRM_ERROR("Read bstatus from DP/AUX failed (%ld)\n", ret);
^
include/drm/drm_print.h:239:10: note: in definition of macro 'DRM_ERROR'
drm_err(fmt, ##__VA_ARGS__)
^~~
Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size
Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size
Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_read
Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_write
Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:ffs
Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls
Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u32
Cyclomatic Complexity 3 include/linux/log2.h:is_power_of_2
Cyclomatic Complexity 1 include/linux/list.h:INIT_LIST_HEAD
Cyclomatic Complexity 1 include/linux/err.h:ERR_PTR
Cyclomatic Complexity 1 include/linux/err.h:IS_ERR
Cyclomatic Complexity 3 include/linux/err.h:IS_ERR_OR_NULL
Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_read
Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_set
Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_read
Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_set
Cyclomatic Complexity 1 include/asm-generic/atomic-long.h:atomic_long_read
Cyclomatic Complexity 1 include/linux/lockdep.h:lock_is_held
Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order
Cyclomatic Complexity 1 include/linux/mutex.h:__mutex_owner
Cyclomatic Complexity 1 include/linux/mutex.h:mutex_is_locked
Cyclomatic Complexity 1 include/linux/jiffies.h:_msecs_to_jiffies
Cyclomatic Complexity 3 include/linux/jiffies.h:msecs_to_jiffies
Cyclomatic Complexity 3 include/linux/ktime.h:ktime_compare
Cyclomatic Complexity 1 include/linux/ktime.h:ktime_after
Cyclomatic Complexity 70 include/linux/ktime.h:ktime_divns
Cyclomatic Complexity 1 include/linux/ktime.h:ktime_to_ms
Cyclomatic Complexity 1 include/linux/ktime.h:ktime_ms_delta
Cyclomatic Complexity 1 include/linux/timekeeping.h:ktime_get_boottime
Cyclomatic Complexity 2 include/linux/workqueue.h:to_delayed_work
Cyclomatic Complexity 1 include/linux/workqueue.h:queue_delayed_work
Cyclomatic Complexity 1 include/linux/workqueue.h:schedule_delayed_work
Cyclomatic Complexity 67 include/linux/slab.h:kmalloc_large
Cyclomatic Complexity 3 include/linux/slab.h:kmalloc
Cyclomatic Complexity 1 include/linux/slab.h:kzalloc
Cyclomatic Complexity 1 include/linux/ww_mutex.h:ww_mutex_is_locked
Cyclomatic Complexity 1 include/drm/drm_modeset_lock.h:drm_modeset_is_locked
Cyclomatic Complexity 1 include/drm/drm_modeset_helper_vtables.h:drm_connector_helper_add
Cyclomatic Complexity 1 include/drm/drm_dp_helper.h:drm_dp_max_lane_count
Cyclomatic Complexity 3 include/drm/drm_dp_helper.h:drm_dp_enhanced_frame_cap
Cyclomatic Complexity 1 include/drm/drm_dp_helper.h:drm_dp_is_branch
Cyclomatic Complexity 1 include/drm/drm_dp_helper.h:drm_dp_dpcd_readb
Cyclomatic Complexity 1 include/drm/drm_dp_helper.h:drm_dp_dpcd_writeb
Cyclomatic Complexity 1 include/drm/drm_dp_helper.h:drm_dp_has_quirk
Cyclomatic Complexity 1 drivers/gpu//drm/i915/i915_reg.h:i915_mmio_reg_offset
Cyclomatic Complexity 1 drivers/gpu//drm/i915/i915_reg.h:i915_mmio_reg_equal
Cyclomatic Complexity 1 drivers/gpu//drm/i915/i915_reg.h:i915_mmio_reg_valid
Cyclomatic Complexity 2 drivers/gpu//drm/i915/i915_utils.h:onoff
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_uncore.h:intel_wait_for_register
Cyclomatic Complexity 2 drivers/gpu//drm/i915/i915_drv.h:to_i915
Cyclomatic Complexity 1 drivers/gpu//drm/i915/i915_drv.h:intel_info
Cyclomatic Complexity 1 drivers/gpu//drm/i915/i915_drv.h:msecs_to_jiffies_timeout
Cyclomatic Complexity 5 drivers/gpu//drm/i915/i915_drv.h:wait_remaining_ms_from_jiffies
Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_drv.h:vlv_pipe_to_channel
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_drv.h:intel_get_crtc_for_pipe
Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_drv.h:intel_attached_encoder
Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_drv.h:enc_to_dig_port
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_drv.h:enc_to_intel_dp
Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_drv.h:dp_to_dig_port
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_drv.h:dp_to_lspcon
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_drv.h:intel_crtc_has_type
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_drv.h:intel_crtc_has_dp_encoder
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_drv.h:intel_wait_for_vblank
Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_drv.h:intel_wait_for_vblank_if_active
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_drv.h:intel_dp_unused_lane_mask
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:intel_dp_to_dev
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:intel_attached_dp
Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_dp.c:intel_dp_rate_limit_len
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:intel_dp_common_len_rate_limit
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:intel_dp_max_common_rate
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:intel_dp_max_common_lane_count
Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_dp.c:cnl_max_source_rate
Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_dp.c:intel_dp_rate_index
Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_dp.c:intel_dp_unpack_aux
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:vlv_pipe_has_pp_on
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:vlv_pipe_has_vdd_on
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:vlv_pipe_any
Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_dp.c:vlv_initial_pps_pipe
Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_dp.c:g4x_get_aux_clock_divider
Cyclomatic Complexity 3 drivers/gpu//drm/i915/intel_dp.c:ilk_get_aux_clock_divider
Cyclomatic Complexity 6 drivers/gpu//drm/i915/intel_dp.c:hsw_get_aux_clock_divider
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:skl_get_aux_clock_divider
Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_dp.c:g4x_get_aux_send_ctl
Cyclomatic Complexity 2 drivers/gpu//drm/i915/intel_dp.c:skl_get_aux_send_ctl
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:intel_dp_aux_header
Cyclomatic Complexity 9 drivers/gpu//drm/i915/intel_dp.c:intel_dp_set_clock
Cyclomatic Complexity 11 drivers/gpu//drm/i915/intel_dp.c:intel_edp_compare_alt_mode
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:wait_backlight_on
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:edp_wait_backlight_off
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:edp_panel_vdd_schedule_off
Cyclomatic Complexity 4 drivers/gpu//drm/i915/intel_dp.c:downstream_hpd_needs_d0
Cyclomatic Complexity 7 drivers/gpu//drm/i915/intel_dp.c:gen4_signal_levels
Cyclomatic Complexity 5 drivers/gpu//drm/i915/intel_dp.c:intel_dp_can_mst
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:intel_dp_get_sink_irq
Cyclomatic Complexity 1 drivers/gpu//drm/i915/intel_dp.c:intel_dp_autotest_phy_pattern
vim +5396 drivers/gpu//drm/i915/intel_dp.c
5385
5386 static inline
5387 int intel_dp_hdcp2_read_rx_status(struct intel_digital_port *intel_dig_port,
5388 uint8_t *rx_status)
5389 {
5390 ssize_t ret;
5391
5392 ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux,
5393 DP_HDCP_2_2_REG_RXSTATUS_OFFSET, rx_status,
5394 HDCP_2_2_DP_RXSTATUS_LEN);
5395 if (ret != HDCP_2_2_DP_RXSTATUS_LEN) {
> 5396 DRM_ERROR("Read bstatus from DP/AUX failed (%ld)\n", ret);
5397 return ret >= 0 ? -EIO : ret;
5398 }
5399
5400 return 0;
5401 }
5402
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Ramalingam,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20180517]
[cannot apply to v4.17-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ramalingam-C/drm-i915-Implement-HDCP2-2/20180523-031938
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x012-201820 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/drm/drm_mm.h:49:0,
from include/drm/drmP.h:73,
from drivers/gpu/drm/i915/intel_dp.c:36:
drivers/gpu/drm/i915/intel_dp.c: In function 'intel_dp_hdcp2_read_rx_status':
>> drivers/gpu/drm/i915/intel_dp.c:5396:13: warning: format '%ld' expects argument of type 'long int', but argument 2 has type 'ssize_t {aka int}' [-Wformat=]
DRM_ERROR("Read bstatus from DP/AUX failed (%ld)\n", ret);
^
include/drm/drm_print.h:239:10: note: in definition of macro 'DRM_ERROR'
drm_err(fmt, ##__VA_ARGS__)
^~~
vim +5396 drivers/gpu/drm/i915/intel_dp.c
5385
5386 static inline
5387 int intel_dp_hdcp2_read_rx_status(struct intel_digital_port *intel_dig_port,
5388 uint8_t *rx_status)
5389 {
5390 ssize_t ret;
5391
5392 ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux,
5393 DP_HDCP_2_2_REG_RXSTATUS_OFFSET, rx_status,
5394 HDCP_2_2_DP_RXSTATUS_LEN);
5395 if (ret != HDCP_2_2_DP_RXSTATUS_LEN) {
> 5396 DRM_ERROR("Read bstatus from DP/AUX failed (%ld)\n", ret);
5397 return ret >= 0 ? -EIO : ret;
5398 }
5399
5400 return 0;
5401 }
5402
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Mon, May 21, 2018 at 06:23:57PM +0530, Ramalingam C wrote: > Implements the DP adaptation specific HDCP2.2 functions. > > These functions perform the DPCD read and write for communicating the > HDCP2.2 auth message back and forth. > > Note: Chris Wilson suggested alternate method for waiting for CP_IRQ, > than completions concept. WIP to understand and implement that, > if needed. Just to unblock the review of other changes, v2 still > continues with completions. > > v2: > wait for cp_irq is merged with this patch. Rebased. > v3: > wait_queue is used for wait for cp_irq [Chris Wilson] > v4: > Style fixed. > %s/PARING/PAIRING > Few style fixes [Uma] > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 358 ++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 7 + > drivers/gpu/drm/i915/intel_hdcp.c | 5 + > 3 files changed, 370 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 528407d608c8..ee71a26ec23f 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -31,6 +31,7 @@ > #include <linux/types.h> > #include <linux/notifier.h> > #include <linux/reboot.h> > +#include <linux/mei_hdcp.h> > #include <asm/byteorder.h> > #include <drm/drmP.h> > #include <drm/drm_atomic_helper.h> > @@ -5057,6 +5058,28 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder) > pps_unlock(intel_dp); > } > > +static int intel_dp_hdcp_wait_for_cp_irq(struct intel_hdcp *hdcp, > + int timeout) > +{ > + long ret; > + > + /* Reinit */ > + atomic_set(&hdcp->cp_irq_recved, 0); > + > +#define C (atomic_read(&hdcp->cp_irq_recved) > 0) > + ret = wait_event_interruptible_timeout(hdcp->cp_irq_queue, C, > + msecs_to_jiffies(timeout)); > + > + if (ret > 0) { > + atomic_set(&hdcp->cp_irq_recved, 0); > + return 0; > + } else if (!ret) { > + return -ETIMEDOUT; > + } > + return (int)ret; > +} > + > + > static > int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port, > u8 *an) > @@ -5275,6 +5298,335 @@ int intel_dp_hdcp_capable(struct intel_digital_port *intel_dig_port, > return 0; > } > > +static > +int intel_dpcd_offset_for_hdcp2_msgid(uint8_t byte, unsigned int *offset) Ugh, this is annoying that we have to map things around like that. But looking at the numbers the standards really don't seem to match at all. Instead of open-coding this I suggested you use a table with a struct like: const static struct hdcp_dp { int hdcp_msg; int dpcd_offset; int timeout; /* whatever else you might need */ } hdcp_2_dp_table[] = { { HDCP_2_2_AKE_INIT, DP_HDCP_2_2_AKE_INIT_OFFSET, ... }, ... }; Then just search that table in the code instead of the huge switch table below. > +{ > + switch (byte) { > + case HDCP_2_2_AKE_INIT: > + *offset = DP_HDCP_2_2_AKE_INIT_OFFSET; > + break; > + case HDCP_2_2_AKE_SEND_CERT: > + *offset = DP_HDCP_2_2_AKE_SEND_CERT_OFFSET; > + break; > + case HDCP_2_2_AKE_NO_STORED_KM: > + *offset = DP_HDCP_2_2_AKE_NO_STORED_KM_OFFSET; > + break; > + case HDCP_2_2_AKE_STORED_KM: > + *offset = DP_HDCP_2_2_AKE_STORED_KM_OFFSET; > + break; > + case HDCP_2_2_AKE_SEND_HPRIME: > + *offset = DP_HDCP_2_2_AKE_SEND_HPRIME_OFFSET; > + break; > + case HDCP_2_2_AKE_SEND_PAIRING_INFO: > + *offset = DP_HDCP_2_2_AKE_SEND_PAIRING_INFO_OFFSET; > + break; > + case HDCP_2_2_LC_INIT: > + *offset = DP_HDCP_2_2_LC_INIT_OFFSET; > + break; > + case HDCP_2_2_LC_SEND_LPRIME: > + *offset = DP_HDCP_2_2_LC_SEND_LPRIME_OFFSET; > + break; > + case HDCP_2_2_SKE_SEND_EKS: > + *offset = DP_HDCP_2_2_SKE_SEND_EKS_OFFSET; > + break; > + case HDCP_2_2_REP_SEND_RECVID_LIST: > + *offset = DP_HDCP_2_2_REP_SEND_RECVID_LIST_OFFSET; > + break; > + case HDCP_2_2_REP_SEND_ACK: > + *offset = DP_HDCP_2_2_REP_SEND_ACK_OFFSET; > + break; > + case HDCP_2_2_REP_STREAM_MANAGE: > + *offset = DP_HDCP_2_2_REP_STREAM_MANAGE_OFFSET; > + break; > + case HDCP_2_2_REP_STREAM_READY: > + *offset = DP_HDCP_2_2_REP_STREAM_READY_OFFSET; > + break; > + case HDCP_2_2_ERRATA_DP_STREAM_TYPE: > + *offset = DP_HDCP_2_2_REG_STREAM_TYPE_OFFSET; > + break; > + default: > + DRM_ERROR("Unrecognized Msg ID\n"); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static inline > +int intel_dp_hdcp2_read_rx_status(struct intel_digital_port *intel_dig_port, > + uint8_t *rx_status) > +{ > + ssize_t ret; > + > + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, > + DP_HDCP_2_2_REG_RXSTATUS_OFFSET, rx_status, > + HDCP_2_2_DP_RXSTATUS_LEN); > + if (ret != HDCP_2_2_DP_RXSTATUS_LEN) { > + DRM_ERROR("Read bstatus from DP/AUX failed (%ld)\n", ret); > + return ret >= 0 ? -EIO : ret; > + } > + > + return 0; > +} > + > +static > +int intel_dp_hdcp2_timeout_for_msg(uint8_t msg_id, bool paired) For the timeout and the availability check below, is that not the same for hdmi? Might be good to set the timeout/check values once, and perhaps pass that then down to the dp/hdmi implementation, instead of computing it here. All these functions here look like a huge layering violation. Imo all this timeout stuff should be handled higher up, or at least more tightly grouped together. Remapping from the HDCP_2_2 defines to the dpcd offsets is imo different than all the stuff below here. -Daniel > +{ > + int timeout; > + > + switch (msg_id) { > + case HDCP_2_2_AKE_SEND_CERT: > + timeout = HDCP_2_2_CERT_TIMEOUT; > + break; > + case HDCP_2_2_AKE_SEND_HPRIME: > + if (paired) > + timeout = HDCP_2_2_HPRIME_PAIRED_TIMEOUT; > + else > + timeout = HDCP_2_2_HPRIME_NO_PAIRED_TIMEOUT; > + break; > + case HDCP_2_2_AKE_SEND_PAIRING_INFO: > + timeout = HDCP_2_2_PAIRING_TIMEOUT; > + break; > + case HDCP_2_2_LC_SEND_LPRIME: > + timeout = HDCP_2_2_DP_LPRIME_TIMEOUT; > + break; > + case HDCP_2_2_REP_SEND_RECVID_LIST: > + timeout = HDCP_2_2_RECVID_LIST_TIMEOUT; > + break; > + case HDCP_2_2_REP_STREAM_READY: > + timeout = HDCP_2_2_STREAM_READY_TIMEOUT; > + break; > + default: > + timeout = -EINVAL; > + DRM_ERROR("Unsupported msg_id: %d\n", (int)msg_id); > + } > + > + return timeout; > +} > + > +static > +int hdcp2_detect_msg_availability(struct intel_digital_port *intel_dig_port, > + uint8_t msg_id, bool *msg_ready) > +{ > + uint8_t rx_status; > + int ret; > + > + *msg_ready = false; > + ret = intel_dp_hdcp2_read_rx_status(intel_dig_port, &rx_status); > + if (ret < 0) > + return ret; > + > + switch (msg_id) { > + case HDCP_2_2_AKE_SEND_HPRIME: > + if (HDCP_2_2_DP_RXSTATUS_H_PRIME(rx_status)) > + *msg_ready = true; > + break; > + case HDCP_2_2_AKE_SEND_PAIRING_INFO: > + if (HDCP_2_2_DP_RXSTATUS_PAIRING(rx_status)) > + *msg_ready = true; > + break; > + case HDCP_2_2_REP_SEND_RECVID_LIST: > + if (HDCP_2_2_DP_RXSTATUS_READY(rx_status)) > + *msg_ready = true; > + break; > + default: > + DRM_DEBUG_KMS("Unidentified msg_id: %d\n", (int)msg_id); > + return -EINVAL; > + } > + > + return 0; > +} > + > + > +static ssize_t > +intel_dp_hdcp2_wait_for_msg(struct intel_digital_port *intel_dig_port, > + uint8_t msg_id) > +{ > + struct intel_dp *dp = &intel_dig_port->dp; > + struct intel_hdcp *hdcp = &dp->attached_connector->hdcp; > + int ret, timeout; > + bool msg_ready = false; > + > + timeout = intel_dp_hdcp2_timeout_for_msg(msg_id, hdcp->is_paired); > + switch (msg_id) { > + > + /* > + * There is no way to detect the CERT, LPRIME and STREAM_READY > + * availability. So Wait for timeout and read the msg. > + */ > + case HDCP_2_2_AKE_SEND_CERT: > + case HDCP_2_2_LC_SEND_LPRIME: > + case HDCP_2_2_REP_STREAM_READY: > + mdelay(timeout); > + ret = 0; > + break; > + case HDCP_2_2_AKE_SEND_HPRIME: > + case HDCP_2_2_AKE_SEND_PAIRING_INFO: > + case HDCP_2_2_REP_SEND_RECVID_LIST: > + intel_dp_hdcp_wait_for_cp_irq(hdcp, timeout); > + ret = hdcp2_detect_msg_availability(intel_dig_port, msg_id, > + &msg_ready); > + if (!msg_ready) > + ret = -ETIMEDOUT; > + break; > + default: > + DRM_DEBUG_KMS("Unidentified msg_id: %d\n", (int)msg_id); > + return -EINVAL; > + } > + if (ret) > + DRM_ERROR("msg_id %d, ret %d, timeout(mSec): %d\n", msg_id, ret, > + timeout); > + > + return ret; > +} > + > +static > +int intel_dp_hdcp2_write_msg(struct intel_digital_port *intel_dig_port, > + void *buf, size_t size) > +{ > + unsigned int offset; > + uint8_t *byte = buf; > + ssize_t ret, bytes_to_write, len; > + > + if (intel_dpcd_offset_for_hdcp2_msgid(*byte, &offset) < 0) > + return -EINVAL; > + > + /* No msg_id in DP HDCP2.2 msgs */ > + bytes_to_write = size - 1; > + byte++; > + > + while (bytes_to_write) { > + len = bytes_to_write > DP_AUX_MAX_PAYLOAD_BYTES ? > + DP_AUX_MAX_PAYLOAD_BYTES : bytes_to_write; > + > + ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, offset, > + (void *)byte, len); > + if (ret < 0) > + return ret; > + > + bytes_to_write -= ret; > + byte += ret; > + offset += ret; > + } > + > + return size; > +} > + > +static > +int intel_dp_hdcp2_read_msg(struct intel_digital_port *intel_dig_port, > + uint8_t msg_id, void *buf, size_t size) > +{ > + unsigned int offset, dev_cnt; > + uint8_t *byte = buf; > + uint8_t rx_info[HDCP_2_2_RXINFO_LEN]; > + ssize_t ret, bytes_to_recv, len; > + > + if (intel_dpcd_offset_for_hdcp2_msgid(msg_id, &offset) < 0) > + return -EINVAL; > + > + ret = intel_dp_hdcp2_wait_for_msg(intel_dig_port, msg_id); > + if (ret < 0) > + return ret; > + > + /* Finding the ReceiverID List size */ > + if (msg_id == HDCP_2_2_REP_SEND_RECVID_LIST) { > + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, > + DP_HDCP_2_2_REG_RXINFO_OFFSET, > + (void *)rx_info, HDCP_2_2_RXINFO_LEN); > + if (ret != HDCP_2_2_RXINFO_LEN) > + return ret >= 0 ? -EIO : ret; > + > + dev_cnt = (HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 | > + HDCP_2_2_DEV_COUNT_LO(rx_info[1])); > + > + if (dev_cnt > HDCP_2_2_MAX_DEVICE_COUNT) > + dev_cnt = HDCP_2_2_MAX_DEVICE_COUNT; > + > + size = sizeof(struct hdcp2_rep_send_receiverid_list) - > + HDCP_2_2_RECEIVER_IDS_MAX_LEN + > + (dev_cnt * HDCP_2_2_RECEIVER_ID_LEN); > + } > + > + bytes_to_recv = size - 1; > + > + /* To skip the msg_id, as msgs in DP adaptation has no msg_id */ > + byte++; > + > + while (bytes_to_recv) { > + len = bytes_to_recv > DP_AUX_MAX_PAYLOAD_BYTES ? > + DP_AUX_MAX_PAYLOAD_BYTES : bytes_to_recv; > + > + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, offset, > + (void *)byte, len); > + if (ret < 0) { > + DRM_DEBUG_KMS("msg_id %d, ret %d\n", msg_id, (int)ret); > + return ret; > + } > + > + bytes_to_recv -= ret; > + byte += ret; > + offset += ret; > + } > + byte = buf; > + *byte = msg_id; > + > + return size; > +} > + > +static > +int intel_dp_hdcp2_config_stream_type(struct intel_digital_port *intel_dig_port, > + void *buf, size_t size) > +{ > + return intel_dp_hdcp2_write_msg(intel_dig_port, buf, size); > +} > + > +static > +int intel_dp_hdcp2_check_link(struct intel_digital_port *intel_dig_port) > +{ > + uint8_t rx_status; > + int ret; > + > + ret = intel_dp_hdcp2_read_rx_status(intel_dig_port, &rx_status); > + if (ret) > + return ret; > + > + if (HDCP_2_2_DP_RXSTATUS_REAUTH_REQ(rx_status)) > + ret = DRM_HDCP_REAUTH_REQUEST; > + else if (HDCP_2_2_DP_RXSTATUS_LINK_FAILED(rx_status)) > + ret = DRM_HDCP_LINK_INTEGRITY_FAILURE; > + else if (HDCP_2_2_DP_RXSTATUS_READY(rx_status)) > + ret = DRM_HDCP_TOPOLOGY_CHANGE; > + > + return ret; > +} > + > +static > +int intel_dp_hdcp2_capable(struct intel_digital_port *intel_dig_port, > + bool *capable) > +{ > + uint8_t rx_caps[3]; > + int ret; > + > + *capable = false; > + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, > + DP_HDCP_2_2_REG_RX_CAPS_OFFSET, > + rx_caps, HDCP_2_2_RXCAPS_LEN); > + if (ret != HDCP_2_2_RXCAPS_LEN) > + return ret >= 0 ? -EIO : ret; > + > + if (rx_caps[0] == HDCP_2_2_RXCAPS_VERSION_VAL && > + HDCP_2_2_DP_HDCP_CAPABLE(rx_caps[2])) > + *capable = true; > + > + return 0; > +} > + > +static > +enum hdcp_protocol intel_dp_hdcp2_protocol(void) > +{ > + return HDCP_PROTOCOL_DP; > +} > + > 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, > @@ -5287,6 +5639,12 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = { > .toggle_signalling = intel_dp_hdcp_toggle_signalling, > .check_link = intel_dp_hdcp_check_link, > .hdcp_capable = intel_dp_hdcp_capable, > + .write_2_2_msg = intel_dp_hdcp2_write_msg, > + .read_2_2_msg = intel_dp_hdcp2_read_msg, > + .config_stream_type = intel_dp_hdcp2_config_stream_type, > + .check_2_2_link = intel_dp_hdcp2_check_link, > + .hdcp_2_2_capable = intel_dp_hdcp2_capable, > + .hdcp_protocol = intel_dp_hdcp2_protocol, > }; > > static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index def8e3255742..0496ebec287b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -444,6 +444,13 @@ struct intel_hdcp { > struct mei_hdcp_data mei_data; > struct notifier_block mei_cldev_nb; > struct delayed_work hdcp2_check_work; > + > + /* > + * Work queue to signal the CP_IRQ. Used for the waiters to read the > + * available information from HDCP DP sink. > + */ > + wait_queue_head_t cp_irq_queue; > + atomic_t cp_irq_recved; > }; > > struct intel_connector { > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c > index d060d7f2e13b..d502bdb380d2 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -870,6 +870,8 @@ int intel_hdcp_init(struct intel_connector *connector, > if (hdcp2_supported) > intel_hdcp2_init(connector); > > + init_waitqueue_head(&hdcp->cp_irq_queue); > + atomic_set(&hdcp->cp_irq_recved, 0); > return 0; > } > > @@ -1849,4 +1851,7 @@ void intel_hdcp_handle_cp_irq(struct intel_connector *connector) > intel_hdcp_check_link(connector); > else if (intel_hdcp2_in_force(connector)) > intel_hdcp2_check_link(connector); > + > + atomic_set(&connector->hdcp.cp_irq_recved, 1); > + wake_up_all(&connector->hdcp.cp_irq_queue); > } > -- > 2.7.4 >
On Thursday 31 May 2018 12:52 PM, Daniel Vetter wrote: > On Mon, May 21, 2018 at 06:23:57PM +0530, Ramalingam C wrote: >> Implements the DP adaptation specific HDCP2.2 functions. >> >> These functions perform the DPCD read and write for communicating the >> HDCP2.2 auth message back and forth. >> >> Note: Chris Wilson suggested alternate method for waiting for CP_IRQ, >> than completions concept. WIP to understand and implement that, >> if needed. Just to unblock the review of other changes, v2 still >> continues with completions. >> >> v2: >> wait for cp_irq is merged with this patch. Rebased. >> v3: >> wait_queue is used for wait for cp_irq [Chris Wilson] >> v4: >> Style fixed. >> %s/PARING/PAIRING >> Few style fixes [Uma] >> >> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 358 ++++++++++++++++++++++++++++++++++++++ >> drivers/gpu/drm/i915/intel_drv.h | 7 + >> drivers/gpu/drm/i915/intel_hdcp.c | 5 + >> 3 files changed, 370 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 528407d608c8..ee71a26ec23f 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -31,6 +31,7 @@ >> #include <linux/types.h> >> #include <linux/notifier.h> >> #include <linux/reboot.h> >> +#include <linux/mei_hdcp.h> >> #include <asm/byteorder.h> >> #include <drm/drmP.h> >> #include <drm/drm_atomic_helper.h> >> @@ -5057,6 +5058,28 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder) >> pps_unlock(intel_dp); >> } >> >> +static int intel_dp_hdcp_wait_for_cp_irq(struct intel_hdcp *hdcp, >> + int timeout) >> +{ >> + long ret; >> + >> + /* Reinit */ >> + atomic_set(&hdcp->cp_irq_recved, 0); >> + >> +#define C (atomic_read(&hdcp->cp_irq_recved) > 0) >> + ret = wait_event_interruptible_timeout(hdcp->cp_irq_queue, C, >> + msecs_to_jiffies(timeout)); >> + >> + if (ret > 0) { >> + atomic_set(&hdcp->cp_irq_recved, 0); >> + return 0; >> + } else if (!ret) { >> + return -ETIMEDOUT; >> + } >> + return (int)ret; >> +} >> + >> + >> static >> int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port, >> u8 *an) >> @@ -5275,6 +5298,335 @@ int intel_dp_hdcp_capable(struct intel_digital_port *intel_dig_port, >> return 0; >> } >> >> +static >> +int intel_dpcd_offset_for_hdcp2_msgid(uint8_t byte, unsigned int *offset) > Ugh, this is annoying that we have to map things around like that. But > looking at the numbers the standards really don't seem to match at all. Sorry i am not getting about not matching part. You mean some offsets and timeouts are not matching with spec? > > Instead of open-coding this I suggested you use a table with a struct > like: > > const static struct hdcp_dp { > int hdcp_msg; > int dpcd_offset; > int timeout; > /* whatever else you might need */ > } hdcp_2_dp_table[] = { > { HDCP_2_2_AKE_INIT, DP_HDCP_2_2_AKE_INIT_OFFSET, ... }, > ... > }; > > Then just search that table in the code instead of the huge switch table > below. Suggesting this for cpu optimization or for coding style? > >> +{ >> + switch (byte) { >> + case HDCP_2_2_AKE_INIT: >> + *offset = DP_HDCP_2_2_AKE_INIT_OFFSET; >> + break; >> + case HDCP_2_2_AKE_SEND_CERT: >> + *offset = DP_HDCP_2_2_AKE_SEND_CERT_OFFSET; >> + break; >> + case HDCP_2_2_AKE_NO_STORED_KM: >> + *offset = DP_HDCP_2_2_AKE_NO_STORED_KM_OFFSET; >> + break; >> + case HDCP_2_2_AKE_STORED_KM: >> + *offset = DP_HDCP_2_2_AKE_STORED_KM_OFFSET; >> + break; >> + case HDCP_2_2_AKE_SEND_HPRIME: >> + *offset = DP_HDCP_2_2_AKE_SEND_HPRIME_OFFSET; >> + break; >> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: >> + *offset = DP_HDCP_2_2_AKE_SEND_PAIRING_INFO_OFFSET; >> + break; >> + case HDCP_2_2_LC_INIT: >> + *offset = DP_HDCP_2_2_LC_INIT_OFFSET; >> + break; >> + case HDCP_2_2_LC_SEND_LPRIME: >> + *offset = DP_HDCP_2_2_LC_SEND_LPRIME_OFFSET; >> + break; >> + case HDCP_2_2_SKE_SEND_EKS: >> + *offset = DP_HDCP_2_2_SKE_SEND_EKS_OFFSET; >> + break; >> + case HDCP_2_2_REP_SEND_RECVID_LIST: >> + *offset = DP_HDCP_2_2_REP_SEND_RECVID_LIST_OFFSET; >> + break; >> + case HDCP_2_2_REP_SEND_ACK: >> + *offset = DP_HDCP_2_2_REP_SEND_ACK_OFFSET; >> + break; >> + case HDCP_2_2_REP_STREAM_MANAGE: >> + *offset = DP_HDCP_2_2_REP_STREAM_MANAGE_OFFSET; >> + break; >> + case HDCP_2_2_REP_STREAM_READY: >> + *offset = DP_HDCP_2_2_REP_STREAM_READY_OFFSET; >> + break; >> + case HDCP_2_2_ERRATA_DP_STREAM_TYPE: >> + *offset = DP_HDCP_2_2_REG_STREAM_TYPE_OFFSET; >> + break; >> + default: >> + DRM_ERROR("Unrecognized Msg ID\n"); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +static inline >> +int intel_dp_hdcp2_read_rx_status(struct intel_digital_port *intel_dig_port, >> + uint8_t *rx_status) >> +{ >> + ssize_t ret; >> + >> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, >> + DP_HDCP_2_2_REG_RXSTATUS_OFFSET, rx_status, >> + HDCP_2_2_DP_RXSTATUS_LEN); >> + if (ret != HDCP_2_2_DP_RXSTATUS_LEN) { >> + DRM_ERROR("Read bstatus from DP/AUX failed (%ld)\n", ret); >> + return ret >= 0 ? -EIO : ret; >> + } >> + >> + return 0; >> +} >> + >> +static >> +int intel_dp_hdcp2_timeout_for_msg(uint8_t msg_id, bool paired) > For the timeout and the availability check below, is that not the same for > hdmi? Might be good to set the timeout/check values once, and perhaps pass > that then down to the dp/hdmi implementation, instead of computing it > here. > > All these functions here look like a huge layering violation. Imo all this > timeout stuff should be handled higher up, or at least more tightly > grouped together. Between HDMI and DP adaptation differences are there but very minimal. Message avialbility check is completely different between HDMI and DP. DP uses the cp_irq. And timeout also varies for a DP HDCP msg. Since we are adapting the HDCP1.4 design where all encoder specific operations are pushed into hdcp_shim, I have implemented all HDMI and DP related operation to their respective shims. So that we need not do more if(dp/hdmi) in common code intel_hdcp.c atleast. The first design I had shared using the drm helper functions, dont have different flows for HDMI and DP. Where ever different treatment is required based on the adaptation, that was taken care then and there. -Ram > Remapping from the HDCP_2_2 defines to the dpcd offsets is imo different > than all the stuff below here. > -Daniel > >> +{ >> + int timeout; >> + >> + switch (msg_id) { >> + case HDCP_2_2_AKE_SEND_CERT: >> + timeout = HDCP_2_2_CERT_TIMEOUT; >> + break; >> + case HDCP_2_2_AKE_SEND_HPRIME: >> + if (paired) >> + timeout = HDCP_2_2_HPRIME_PAIRED_TIMEOUT; >> + else >> + timeout = HDCP_2_2_HPRIME_NO_PAIRED_TIMEOUT; >> + break; >> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: >> + timeout = HDCP_2_2_PAIRING_TIMEOUT; >> + break; >> + case HDCP_2_2_LC_SEND_LPRIME: >> + timeout = HDCP_2_2_DP_LPRIME_TIMEOUT; >> + break; >> + case HDCP_2_2_REP_SEND_RECVID_LIST: >> + timeout = HDCP_2_2_RECVID_LIST_TIMEOUT; >> + break; >> + case HDCP_2_2_REP_STREAM_READY: >> + timeout = HDCP_2_2_STREAM_READY_TIMEOUT; >> + break; >> + default: >> + timeout = -EINVAL; >> + DRM_ERROR("Unsupported msg_id: %d\n", (int)msg_id); >> + } >> + >> + return timeout; >> +} >> + >> +static >> +int hdcp2_detect_msg_availability(struct intel_digital_port *intel_dig_port, >> + uint8_t msg_id, bool *msg_ready) >> +{ >> + uint8_t rx_status; >> + int ret; >> + >> + *msg_ready = false; >> + ret = intel_dp_hdcp2_read_rx_status(intel_dig_port, &rx_status); >> + if (ret < 0) >> + return ret; >> + >> + switch (msg_id) { >> + case HDCP_2_2_AKE_SEND_HPRIME: >> + if (HDCP_2_2_DP_RXSTATUS_H_PRIME(rx_status)) >> + *msg_ready = true; >> + break; >> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: >> + if (HDCP_2_2_DP_RXSTATUS_PAIRING(rx_status)) >> + *msg_ready = true; >> + break; >> + case HDCP_2_2_REP_SEND_RECVID_LIST: >> + if (HDCP_2_2_DP_RXSTATUS_READY(rx_status)) >> + *msg_ready = true; >> + break; >> + default: >> + DRM_DEBUG_KMS("Unidentified msg_id: %d\n", (int)msg_id); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> + >> +static ssize_t >> +intel_dp_hdcp2_wait_for_msg(struct intel_digital_port *intel_dig_port, >> + uint8_t msg_id) >> +{ >> + struct intel_dp *dp = &intel_dig_port->dp; >> + struct intel_hdcp *hdcp = &dp->attached_connector->hdcp; >> + int ret, timeout; >> + bool msg_ready = false; >> + >> + timeout = intel_dp_hdcp2_timeout_for_msg(msg_id, hdcp->is_paired); >> + switch (msg_id) { >> + >> + /* >> + * There is no way to detect the CERT, LPRIME and STREAM_READY >> + * availability. So Wait for timeout and read the msg. >> + */ >> + case HDCP_2_2_AKE_SEND_CERT: >> + case HDCP_2_2_LC_SEND_LPRIME: >> + case HDCP_2_2_REP_STREAM_READY: >> + mdelay(timeout); >> + ret = 0; >> + break; >> + case HDCP_2_2_AKE_SEND_HPRIME: >> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: >> + case HDCP_2_2_REP_SEND_RECVID_LIST: >> + intel_dp_hdcp_wait_for_cp_irq(hdcp, timeout); >> + ret = hdcp2_detect_msg_availability(intel_dig_port, msg_id, >> + &msg_ready); >> + if (!msg_ready) >> + ret = -ETIMEDOUT; >> + break; >> + default: >> + DRM_DEBUG_KMS("Unidentified msg_id: %d\n", (int)msg_id); >> + return -EINVAL; >> + } >> + if (ret) >> + DRM_ERROR("msg_id %d, ret %d, timeout(mSec): %d\n", msg_id, ret, >> + timeout); >> + >> + return ret; >> +} >> + >> +static >> +int intel_dp_hdcp2_write_msg(struct intel_digital_port *intel_dig_port, >> + void *buf, size_t size) >> +{ >> + unsigned int offset; >> + uint8_t *byte = buf; >> + ssize_t ret, bytes_to_write, len; >> + >> + if (intel_dpcd_offset_for_hdcp2_msgid(*byte, &offset) < 0) >> + return -EINVAL; >> + >> + /* No msg_id in DP HDCP2.2 msgs */ >> + bytes_to_write = size - 1; >> + byte++; >> + >> + while (bytes_to_write) { >> + len = bytes_to_write > DP_AUX_MAX_PAYLOAD_BYTES ? >> + DP_AUX_MAX_PAYLOAD_BYTES : bytes_to_write; >> + >> + ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, offset, >> + (void *)byte, len); >> + if (ret < 0) >> + return ret; >> + >> + bytes_to_write -= ret; >> + byte += ret; >> + offset += ret; >> + } >> + >> + return size; >> +} >> + >> +static >> +int intel_dp_hdcp2_read_msg(struct intel_digital_port *intel_dig_port, >> + uint8_t msg_id, void *buf, size_t size) >> +{ >> + unsigned int offset, dev_cnt; >> + uint8_t *byte = buf; >> + uint8_t rx_info[HDCP_2_2_RXINFO_LEN]; >> + ssize_t ret, bytes_to_recv, len; >> + >> + if (intel_dpcd_offset_for_hdcp2_msgid(msg_id, &offset) < 0) >> + return -EINVAL; >> + >> + ret = intel_dp_hdcp2_wait_for_msg(intel_dig_port, msg_id); >> + if (ret < 0) >> + return ret; >> + >> + /* Finding the ReceiverID List size */ >> + if (msg_id == HDCP_2_2_REP_SEND_RECVID_LIST) { >> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, >> + DP_HDCP_2_2_REG_RXINFO_OFFSET, >> + (void *)rx_info, HDCP_2_2_RXINFO_LEN); >> + if (ret != HDCP_2_2_RXINFO_LEN) >> + return ret >= 0 ? -EIO : ret; >> + >> + dev_cnt = (HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 | >> + HDCP_2_2_DEV_COUNT_LO(rx_info[1])); >> + >> + if (dev_cnt > HDCP_2_2_MAX_DEVICE_COUNT) >> + dev_cnt = HDCP_2_2_MAX_DEVICE_COUNT; >> + >> + size = sizeof(struct hdcp2_rep_send_receiverid_list) - >> + HDCP_2_2_RECEIVER_IDS_MAX_LEN + >> + (dev_cnt * HDCP_2_2_RECEIVER_ID_LEN); >> + } >> + >> + bytes_to_recv = size - 1; >> + >> + /* To skip the msg_id, as msgs in DP adaptation has no msg_id */ >> + byte++; >> + >> + while (bytes_to_recv) { >> + len = bytes_to_recv > DP_AUX_MAX_PAYLOAD_BYTES ? >> + DP_AUX_MAX_PAYLOAD_BYTES : bytes_to_recv; >> + >> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, offset, >> + (void *)byte, len); >> + if (ret < 0) { >> + DRM_DEBUG_KMS("msg_id %d, ret %d\n", msg_id, (int)ret); >> + return ret; >> + } >> + >> + bytes_to_recv -= ret; >> + byte += ret; >> + offset += ret; >> + } >> + byte = buf; >> + *byte = msg_id; >> + >> + return size; >> +} >> + >> +static >> +int intel_dp_hdcp2_config_stream_type(struct intel_digital_port *intel_dig_port, >> + void *buf, size_t size) >> +{ >> + return intel_dp_hdcp2_write_msg(intel_dig_port, buf, size); >> +} >> + >> +static >> +int intel_dp_hdcp2_check_link(struct intel_digital_port *intel_dig_port) >> +{ >> + uint8_t rx_status; >> + int ret; >> + >> + ret = intel_dp_hdcp2_read_rx_status(intel_dig_port, &rx_status); >> + if (ret) >> + return ret; >> + >> + if (HDCP_2_2_DP_RXSTATUS_REAUTH_REQ(rx_status)) >> + ret = DRM_HDCP_REAUTH_REQUEST; >> + else if (HDCP_2_2_DP_RXSTATUS_LINK_FAILED(rx_status)) >> + ret = DRM_HDCP_LINK_INTEGRITY_FAILURE; >> + else if (HDCP_2_2_DP_RXSTATUS_READY(rx_status)) >> + ret = DRM_HDCP_TOPOLOGY_CHANGE; >> + >> + return ret; >> +} >> + >> +static >> +int intel_dp_hdcp2_capable(struct intel_digital_port *intel_dig_port, >> + bool *capable) >> +{ >> + uint8_t rx_caps[3]; >> + int ret; >> + >> + *capable = false; >> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, >> + DP_HDCP_2_2_REG_RX_CAPS_OFFSET, >> + rx_caps, HDCP_2_2_RXCAPS_LEN); >> + if (ret != HDCP_2_2_RXCAPS_LEN) >> + return ret >= 0 ? -EIO : ret; >> + >> + if (rx_caps[0] == HDCP_2_2_RXCAPS_VERSION_VAL && >> + HDCP_2_2_DP_HDCP_CAPABLE(rx_caps[2])) >> + *capable = true; >> + >> + return 0; >> +} >> + >> +static >> +enum hdcp_protocol intel_dp_hdcp2_protocol(void) >> +{ >> + return HDCP_PROTOCOL_DP; >> +} >> + >> 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, >> @@ -5287,6 +5639,12 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = { >> .toggle_signalling = intel_dp_hdcp_toggle_signalling, >> .check_link = intel_dp_hdcp_check_link, >> .hdcp_capable = intel_dp_hdcp_capable, >> + .write_2_2_msg = intel_dp_hdcp2_write_msg, >> + .read_2_2_msg = intel_dp_hdcp2_read_msg, >> + .config_stream_type = intel_dp_hdcp2_config_stream_type, >> + .check_2_2_link = intel_dp_hdcp2_check_link, >> + .hdcp_2_2_capable = intel_dp_hdcp2_capable, >> + .hdcp_protocol = intel_dp_hdcp2_protocol, >> }; >> >> static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp) >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index def8e3255742..0496ebec287b 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -444,6 +444,13 @@ struct intel_hdcp { >> struct mei_hdcp_data mei_data; >> struct notifier_block mei_cldev_nb; >> struct delayed_work hdcp2_check_work; >> + >> + /* >> + * Work queue to signal the CP_IRQ. Used for the waiters to read the >> + * available information from HDCP DP sink. >> + */ >> + wait_queue_head_t cp_irq_queue; >> + atomic_t cp_irq_recved; >> }; >> >> struct intel_connector { >> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c >> index d060d7f2e13b..d502bdb380d2 100644 >> --- a/drivers/gpu/drm/i915/intel_hdcp.c >> +++ b/drivers/gpu/drm/i915/intel_hdcp.c >> @@ -870,6 +870,8 @@ int intel_hdcp_init(struct intel_connector *connector, >> if (hdcp2_supported) >> intel_hdcp2_init(connector); >> >> + init_waitqueue_head(&hdcp->cp_irq_queue); >> + atomic_set(&hdcp->cp_irq_recved, 0); >> return 0; >> } >> >> @@ -1849,4 +1851,7 @@ void intel_hdcp_handle_cp_irq(struct intel_connector *connector) >> intel_hdcp_check_link(connector); >> else if (intel_hdcp2_in_force(connector)) >> intel_hdcp2_check_link(connector); >> + >> + atomic_set(&connector->hdcp.cp_irq_recved, 1); >> + wake_up_all(&connector->hdcp.cp_irq_queue); >> } >> -- >> 2.7.4 >>
On Wed, Jun 20, 2018 at 12:19 PM, Ramalingam C <ramalingam.c@intel.com> wrote: > > > On Thursday 31 May 2018 12:52 PM, Daniel Vetter wrote: >> >> On Mon, May 21, 2018 at 06:23:57PM +0530, Ramalingam C wrote: >>> >>> Implements the DP adaptation specific HDCP2.2 functions. >>> >>> These functions perform the DPCD read and write for communicating the >>> HDCP2.2 auth message back and forth. >>> >>> Note: Chris Wilson suggested alternate method for waiting for CP_IRQ, >>> than completions concept. WIP to understand and implement that, >>> if needed. Just to unblock the review of other changes, v2 still >>> continues with completions. >>> >>> v2: >>> wait for cp_irq is merged with this patch. Rebased. >>> v3: >>> wait_queue is used for wait for cp_irq [Chris Wilson] >>> v4: >>> Style fixed. >>> %s/PARING/PAIRING >>> Few style fixes [Uma] >>> >>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_dp.c | 358 >>> ++++++++++++++++++++++++++++++++++++++ >>> drivers/gpu/drm/i915/intel_drv.h | 7 + >>> drivers/gpu/drm/i915/intel_hdcp.c | 5 + >>> 3 files changed, 370 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c >>> b/drivers/gpu/drm/i915/intel_dp.c >>> index 528407d608c8..ee71a26ec23f 100644 >>> --- a/drivers/gpu/drm/i915/intel_dp.c >>> +++ b/drivers/gpu/drm/i915/intel_dp.c >>> @@ -31,6 +31,7 @@ >>> #include <linux/types.h> >>> #include <linux/notifier.h> >>> #include <linux/reboot.h> >>> +#include <linux/mei_hdcp.h> >>> #include <asm/byteorder.h> >>> #include <drm/drmP.h> >>> #include <drm/drm_atomic_helper.h> >>> @@ -5057,6 +5058,28 @@ void intel_dp_encoder_suspend(struct intel_encoder >>> *intel_encoder) >>> pps_unlock(intel_dp); >>> } >>> +static int intel_dp_hdcp_wait_for_cp_irq(struct intel_hdcp *hdcp, >>> + int timeout) >>> +{ >>> + long ret; >>> + >>> + /* Reinit */ >>> + atomic_set(&hdcp->cp_irq_recved, 0); >>> + >>> +#define C (atomic_read(&hdcp->cp_irq_recved) > 0) >>> + ret = wait_event_interruptible_timeout(hdcp->cp_irq_queue, C, >>> + >>> msecs_to_jiffies(timeout)); >>> + >>> + if (ret > 0) { >>> + atomic_set(&hdcp->cp_irq_recved, 0); >>> + return 0; >>> + } else if (!ret) { >>> + return -ETIMEDOUT; >>> + } >>> + return (int)ret; >>> +} >>> + >>> + >>> static >>> int intel_dp_hdcp_write_an_aksv(struct intel_digital_port >>> *intel_dig_port, >>> u8 *an) >>> @@ -5275,6 +5298,335 @@ int intel_dp_hdcp_capable(struct >>> intel_digital_port *intel_dig_port, >>> return 0; >>> } >>> +static >>> +int intel_dpcd_offset_for_hdcp2_msgid(uint8_t byte, unsigned int >>> *offset) >> >> Ugh, this is annoying that we have to map things around like that. But >> looking at the numbers the standards really don't seem to match at all. > > Sorry i am not getting about not matching part. > You mean some offsets and timeouts are not matching with spec? match as in you can't just compute them using a base_offset + hdcp_msgid trick, you do have to use the lookup table. The numbers itself match the spec, it's just that the specs are all inconsistent with each another for no real good reason. >> >> >> Instead of open-coding this I suggested you use a table with a struct >> like: >> >> const static struct hdcp_dp { >> int hdcp_msg; >> int dpcd_offset; >> int timeout; >> /* whatever else you might need */ >> } hdcp_2_dp_table[] = { >> { HDCP_2_2_AKE_INIT, DP_HDCP_2_2_AKE_INIT_OFFSET, ... }, >> ... >> }; >> >> Then just search that table in the code instead of the huge switch table >> below. > > Suggesting this for cpu optimization or for coding style? Just coding style, having to check a table is easier than checking huge case statements. None of this code matters from a performance pov. > > >> >>> +{ >>> + switch (byte) { >>> + case HDCP_2_2_AKE_INIT: >>> + *offset = DP_HDCP_2_2_AKE_INIT_OFFSET; >>> + break; >>> + case HDCP_2_2_AKE_SEND_CERT: >>> + *offset = DP_HDCP_2_2_AKE_SEND_CERT_OFFSET; >>> + break; >>> + case HDCP_2_2_AKE_NO_STORED_KM: >>> + *offset = DP_HDCP_2_2_AKE_NO_STORED_KM_OFFSET; >>> + break; >>> + case HDCP_2_2_AKE_STORED_KM: >>> + *offset = DP_HDCP_2_2_AKE_STORED_KM_OFFSET; >>> + break; >>> + case HDCP_2_2_AKE_SEND_HPRIME: >>> + *offset = DP_HDCP_2_2_AKE_SEND_HPRIME_OFFSET; >>> + break; >>> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: >>> + *offset = DP_HDCP_2_2_AKE_SEND_PAIRING_INFO_OFFSET; >>> + break; >>> + case HDCP_2_2_LC_INIT: >>> + *offset = DP_HDCP_2_2_LC_INIT_OFFSET; >>> + break; >>> + case HDCP_2_2_LC_SEND_LPRIME: >>> + *offset = DP_HDCP_2_2_LC_SEND_LPRIME_OFFSET; >>> + break; >>> + case HDCP_2_2_SKE_SEND_EKS: >>> + *offset = DP_HDCP_2_2_SKE_SEND_EKS_OFFSET; >>> + break; >>> + case HDCP_2_2_REP_SEND_RECVID_LIST: >>> + *offset = DP_HDCP_2_2_REP_SEND_RECVID_LIST_OFFSET; >>> + break; >>> + case HDCP_2_2_REP_SEND_ACK: >>> + *offset = DP_HDCP_2_2_REP_SEND_ACK_OFFSET; >>> + break; >>> + case HDCP_2_2_REP_STREAM_MANAGE: >>> + *offset = DP_HDCP_2_2_REP_STREAM_MANAGE_OFFSET; >>> + break; >>> + case HDCP_2_2_REP_STREAM_READY: >>> + *offset = DP_HDCP_2_2_REP_STREAM_READY_OFFSET; >>> + break; >>> + case HDCP_2_2_ERRATA_DP_STREAM_TYPE: >>> + *offset = DP_HDCP_2_2_REG_STREAM_TYPE_OFFSET; >>> + break; >>> + default: >>> + DRM_ERROR("Unrecognized Msg ID\n"); >>> + return -EINVAL; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static inline >>> +int intel_dp_hdcp2_read_rx_status(struct intel_digital_port >>> *intel_dig_port, >>> + uint8_t *rx_status) >>> +{ >>> + ssize_t ret; >>> + >>> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, >>> + DP_HDCP_2_2_REG_RXSTATUS_OFFSET, >>> rx_status, >>> + HDCP_2_2_DP_RXSTATUS_LEN); >>> + if (ret != HDCP_2_2_DP_RXSTATUS_LEN) { >>> + DRM_ERROR("Read bstatus from DP/AUX failed (%ld)\n", >>> ret); >>> + return ret >= 0 ? -EIO : ret; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static >>> +int intel_dp_hdcp2_timeout_for_msg(uint8_t msg_id, bool paired) >> >> For the timeout and the availability check below, is that not the same for >> hdmi? Might be good to set the timeout/check values once, and perhaps pass >> that then down to the dp/hdmi implementation, instead of computing it >> here. >> >> All these functions here look like a huge layering violation. Imo all this >> timeout stuff should be handled higher up, or at least more tightly >> grouped together. > > Between HDMI and DP adaptation differences are there but very minimal. > Message avialbility check is completely different between HDMI and DP. DP > uses the cp_irq. > And timeout also varies for a DP HDCP msg. > > Since we are adapting the HDCP1.4 design where all encoder specific > operations are pushed into hdcp_shim, > I have implemented all HDMI and DP related operation to their respective > shims. > So that we need not do more if(dp/hdmi) in common code intel_hdcp.c atleast. > > The first design I had shared using the drm helper functions, dont have > different flows for HDMI and DP. > Where ever different treatment is required based on the adaptation, that was > taken care then and there. I'm not against having dp/hdmi specific functions, I'm against having to spread that specific stuff all around in different places. If you need msgid specific timeouts between DP and HDMI then maybe stuff them into the lookup table, but from a quick look it seemed like that's not the case. And if the hdcp1.4 shim design is getting in the way, we need to fix that. The hdcp1.4 shim is indeed a bit looking like a midlayer, so it getting in the way is no surprise really. -Daniel > > -Ram > > >> Remapping from the HDCP_2_2 defines to the dpcd offsets is imo different >> than all the stuff below here. >> -Daniel >> >>> +{ >>> + int timeout; >>> + >>> + switch (msg_id) { >>> + case HDCP_2_2_AKE_SEND_CERT: >>> + timeout = HDCP_2_2_CERT_TIMEOUT; >>> + break; >>> + case HDCP_2_2_AKE_SEND_HPRIME: >>> + if (paired) >>> + timeout = HDCP_2_2_HPRIME_PAIRED_TIMEOUT; >>> + else >>> + timeout = HDCP_2_2_HPRIME_NO_PAIRED_TIMEOUT; >>> + break; >>> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: >>> + timeout = HDCP_2_2_PAIRING_TIMEOUT; >>> + break; >>> + case HDCP_2_2_LC_SEND_LPRIME: >>> + timeout = HDCP_2_2_DP_LPRIME_TIMEOUT; >>> + break; >>> + case HDCP_2_2_REP_SEND_RECVID_LIST: >>> + timeout = HDCP_2_2_RECVID_LIST_TIMEOUT; >>> + break; >>> + case HDCP_2_2_REP_STREAM_READY: >>> + timeout = HDCP_2_2_STREAM_READY_TIMEOUT; >>> + break; >>> + default: >>> + timeout = -EINVAL; >>> + DRM_ERROR("Unsupported msg_id: %d\n", (int)msg_id); >>> + } >>> + >>> + return timeout; >>> +} >>> + >>> +static >>> +int hdcp2_detect_msg_availability(struct intel_digital_port >>> *intel_dig_port, >>> + uint8_t msg_id, bool *msg_ready) >>> +{ >>> + uint8_t rx_status; >>> + int ret; >>> + >>> + *msg_ready = false; >>> + ret = intel_dp_hdcp2_read_rx_status(intel_dig_port, &rx_status); >>> + if (ret < 0) >>> + return ret; >>> + >>> + switch (msg_id) { >>> + case HDCP_2_2_AKE_SEND_HPRIME: >>> + if (HDCP_2_2_DP_RXSTATUS_H_PRIME(rx_status)) >>> + *msg_ready = true; >>> + break; >>> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: >>> + if (HDCP_2_2_DP_RXSTATUS_PAIRING(rx_status)) >>> + *msg_ready = true; >>> + break; >>> + case HDCP_2_2_REP_SEND_RECVID_LIST: >>> + if (HDCP_2_2_DP_RXSTATUS_READY(rx_status)) >>> + *msg_ready = true; >>> + break; >>> + default: >>> + DRM_DEBUG_KMS("Unidentified msg_id: %d\n", (int)msg_id); >>> + return -EINVAL; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> + >>> +static ssize_t >>> +intel_dp_hdcp2_wait_for_msg(struct intel_digital_port *intel_dig_port, >>> + uint8_t msg_id) >>> +{ >>> + struct intel_dp *dp = &intel_dig_port->dp; >>> + struct intel_hdcp *hdcp = &dp->attached_connector->hdcp; >>> + int ret, timeout; >>> + bool msg_ready = false; >>> + >>> + timeout = intel_dp_hdcp2_timeout_for_msg(msg_id, >>> hdcp->is_paired); >>> + switch (msg_id) { >>> + >>> + /* >>> + * There is no way to detect the CERT, LPRIME and STREAM_READY >>> + * availability. So Wait for timeout and read the msg. >>> + */ >>> + case HDCP_2_2_AKE_SEND_CERT: >>> + case HDCP_2_2_LC_SEND_LPRIME: >>> + case HDCP_2_2_REP_STREAM_READY: >>> + mdelay(timeout); >>> + ret = 0; >>> + break; >>> + case HDCP_2_2_AKE_SEND_HPRIME: >>> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: >>> + case HDCP_2_2_REP_SEND_RECVID_LIST: >>> + intel_dp_hdcp_wait_for_cp_irq(hdcp, timeout); >>> + ret = hdcp2_detect_msg_availability(intel_dig_port, >>> msg_id, >>> + &msg_ready); >>> + if (!msg_ready) >>> + ret = -ETIMEDOUT; >>> + break; >>> + default: >>> + DRM_DEBUG_KMS("Unidentified msg_id: %d\n", (int)msg_id); >>> + return -EINVAL; >>> + } >>> + if (ret) >>> + DRM_ERROR("msg_id %d, ret %d, timeout(mSec): %d\n", >>> msg_id, ret, >>> + timeout); >>> + >>> + return ret; >>> +} >>> + >>> +static >>> +int intel_dp_hdcp2_write_msg(struct intel_digital_port *intel_dig_port, >>> + void *buf, size_t size) >>> +{ >>> + unsigned int offset; >>> + uint8_t *byte = buf; >>> + ssize_t ret, bytes_to_write, len; >>> + >>> + if (intel_dpcd_offset_for_hdcp2_msgid(*byte, &offset) < 0) >>> + return -EINVAL; >>> + >>> + /* No msg_id in DP HDCP2.2 msgs */ >>> + bytes_to_write = size - 1; >>> + byte++; >>> + >>> + while (bytes_to_write) { >>> + len = bytes_to_write > DP_AUX_MAX_PAYLOAD_BYTES ? >>> + DP_AUX_MAX_PAYLOAD_BYTES : >>> bytes_to_write; >>> + >>> + ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, offset, >>> + (void *)byte, len); >>> + if (ret < 0) >>> + return ret; >>> + >>> + bytes_to_write -= ret; >>> + byte += ret; >>> + offset += ret; >>> + } >>> + >>> + return size; >>> +} >>> + >>> +static >>> +int intel_dp_hdcp2_read_msg(struct intel_digital_port *intel_dig_port, >>> + uint8_t msg_id, void *buf, size_t size) >>> +{ >>> + unsigned int offset, dev_cnt; >>> + uint8_t *byte = buf; >>> + uint8_t rx_info[HDCP_2_2_RXINFO_LEN]; >>> + ssize_t ret, bytes_to_recv, len; >>> + >>> + if (intel_dpcd_offset_for_hdcp2_msgid(msg_id, &offset) < 0) >>> + return -EINVAL; >>> + >>> + ret = intel_dp_hdcp2_wait_for_msg(intel_dig_port, msg_id); >>> + if (ret < 0) >>> + return ret; >>> + >>> + /* Finding the ReceiverID List size */ >>> + if (msg_id == HDCP_2_2_REP_SEND_RECVID_LIST) { >>> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, >>> + DP_HDCP_2_2_REG_RXINFO_OFFSET, >>> + (void *)rx_info, >>> HDCP_2_2_RXINFO_LEN); >>> + if (ret != HDCP_2_2_RXINFO_LEN) >>> + return ret >= 0 ? -EIO : ret; >>> + >>> + dev_cnt = (HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 | >>> + HDCP_2_2_DEV_COUNT_LO(rx_info[1])); >>> + >>> + if (dev_cnt > HDCP_2_2_MAX_DEVICE_COUNT) >>> + dev_cnt = HDCP_2_2_MAX_DEVICE_COUNT; >>> + >>> + size = sizeof(struct hdcp2_rep_send_receiverid_list) - >>> + HDCP_2_2_RECEIVER_IDS_MAX_LEN + >>> + (dev_cnt * HDCP_2_2_RECEIVER_ID_LEN); >>> + } >>> + >>> + bytes_to_recv = size - 1; >>> + >>> + /* To skip the msg_id, as msgs in DP adaptation has no msg_id */ >>> + byte++; >>> + >>> + while (bytes_to_recv) { >>> + len = bytes_to_recv > DP_AUX_MAX_PAYLOAD_BYTES ? >>> + DP_AUX_MAX_PAYLOAD_BYTES : bytes_to_recv; >>> + >>> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, offset, >>> + (void *)byte, len); >>> + if (ret < 0) { >>> + DRM_DEBUG_KMS("msg_id %d, ret %d\n", msg_id, >>> (int)ret); >>> + return ret; >>> + } >>> + >>> + bytes_to_recv -= ret; >>> + byte += ret; >>> + offset += ret; >>> + } >>> + byte = buf; >>> + *byte = msg_id; >>> + >>> + return size; >>> +} >>> + >>> +static >>> +int intel_dp_hdcp2_config_stream_type(struct intel_digital_port >>> *intel_dig_port, >>> + void *buf, size_t size) >>> +{ >>> + return intel_dp_hdcp2_write_msg(intel_dig_port, buf, size); >>> +} >>> + >>> +static >>> +int intel_dp_hdcp2_check_link(struct intel_digital_port *intel_dig_port) >>> +{ >>> + uint8_t rx_status; >>> + int ret; >>> + >>> + ret = intel_dp_hdcp2_read_rx_status(intel_dig_port, &rx_status); >>> + if (ret) >>> + return ret; >>> + >>> + if (HDCP_2_2_DP_RXSTATUS_REAUTH_REQ(rx_status)) >>> + ret = DRM_HDCP_REAUTH_REQUEST; >>> + else if (HDCP_2_2_DP_RXSTATUS_LINK_FAILED(rx_status)) >>> + ret = DRM_HDCP_LINK_INTEGRITY_FAILURE; >>> + else if (HDCP_2_2_DP_RXSTATUS_READY(rx_status)) >>> + ret = DRM_HDCP_TOPOLOGY_CHANGE; >>> + >>> + return ret; >>> +} >>> + >>> +static >>> +int intel_dp_hdcp2_capable(struct intel_digital_port *intel_dig_port, >>> + bool *capable) >>> +{ >>> + uint8_t rx_caps[3]; >>> + int ret; >>> + >>> + *capable = false; >>> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, >>> + DP_HDCP_2_2_REG_RX_CAPS_OFFSET, >>> + rx_caps, HDCP_2_2_RXCAPS_LEN); >>> + if (ret != HDCP_2_2_RXCAPS_LEN) >>> + return ret >= 0 ? -EIO : ret; >>> + >>> + if (rx_caps[0] == HDCP_2_2_RXCAPS_VERSION_VAL && >>> + HDCP_2_2_DP_HDCP_CAPABLE(rx_caps[2])) >>> + *capable = true; >>> + >>> + return 0; >>> +} >>> + >>> +static >>> +enum hdcp_protocol intel_dp_hdcp2_protocol(void) >>> +{ >>> + return HDCP_PROTOCOL_DP; >>> +} >>> + >>> 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, >>> @@ -5287,6 +5639,12 @@ static const struct intel_hdcp_shim >>> intel_dp_hdcp_shim = { >>> .toggle_signalling = intel_dp_hdcp_toggle_signalling, >>> .check_link = intel_dp_hdcp_check_link, >>> .hdcp_capable = intel_dp_hdcp_capable, >>> + .write_2_2_msg = intel_dp_hdcp2_write_msg, >>> + .read_2_2_msg = intel_dp_hdcp2_read_msg, >>> + .config_stream_type = intel_dp_hdcp2_config_stream_type, >>> + .check_2_2_link = intel_dp_hdcp2_check_link, >>> + .hdcp_2_2_capable = intel_dp_hdcp2_capable, >>> + .hdcp_protocol = intel_dp_hdcp2_protocol, >>> }; >>> static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp) >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h >>> b/drivers/gpu/drm/i915/intel_drv.h >>> index def8e3255742..0496ebec287b 100644 >>> --- a/drivers/gpu/drm/i915/intel_drv.h >>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>> @@ -444,6 +444,13 @@ struct intel_hdcp { >>> struct mei_hdcp_data mei_data; >>> struct notifier_block mei_cldev_nb; >>> struct delayed_work hdcp2_check_work; >>> + >>> + /* >>> + * Work queue to signal the CP_IRQ. Used for the waiters to read >>> the >>> + * available information from HDCP DP sink. >>> + */ >>> + wait_queue_head_t cp_irq_queue; >>> + atomic_t cp_irq_recved; >>> }; >>> struct intel_connector { >>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c >>> b/drivers/gpu/drm/i915/intel_hdcp.c >>> index d060d7f2e13b..d502bdb380d2 100644 >>> --- a/drivers/gpu/drm/i915/intel_hdcp.c >>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c >>> @@ -870,6 +870,8 @@ int intel_hdcp_init(struct intel_connector >>> *connector, >>> if (hdcp2_supported) >>> intel_hdcp2_init(connector); >>> + init_waitqueue_head(&hdcp->cp_irq_queue); >>> + atomic_set(&hdcp->cp_irq_recved, 0); >>> return 0; >>> } >>> @@ -1849,4 +1851,7 @@ void intel_hdcp_handle_cp_irq(struct >>> intel_connector *connector) >>> intel_hdcp_check_link(connector); >>> else if (intel_hdcp2_in_force(connector)) >>> intel_hdcp2_check_link(connector); >>> + >>> + atomic_set(&connector->hdcp.cp_irq_recved, 1); >>> + wake_up_all(&connector->hdcp.cp_irq_queue); >>> } >>> -- >>> 2.7.4 >>> >
Best Regards, Ramalingam C > -----Original Message----- > From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On Behalf Of > Daniel Vetter > Sent: Wednesday, June 20, 2018 5:14 PM > To: C, Ramalingam <ramalingam.c@intel.com> > Cc: intel-gfx <intel-gfx@lists.freedesktop.org>; dri-devel <dri- > devel@lists.freedesktop.org>; Sean Paul <seanpaul@chromium.org>; Chris > Wilson <chris@chris-wilson.co.uk>; Nikula, Jani <jani.nikula@linux.intel.com>; > Winkler, Tomas <tomas.winkler@intel.com>; Usyskin, Alexander > <alexander.usyskin@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Sharma, Shashank <shashank.sharma@intel.com> > Subject: Re: [PATCH v4 38/41] drm/i915: Implement the HDCP2.2 support for DP > > On Wed, Jun 20, 2018 at 12:19 PM, Ramalingam C <ramalingam.c@intel.com> > wrote: > > > > > > On Thursday 31 May 2018 12:52 PM, Daniel Vetter wrote: > >> > >> On Mon, May 21, 2018 at 06:23:57PM +0530, Ramalingam C wrote: > >>> > >>> Implements the DP adaptation specific HDCP2.2 functions. > >>> > >>> These functions perform the DPCD read and write for communicating > >>> the > >>> HDCP2.2 auth message back and forth. > >>> > >>> Note: Chris Wilson suggested alternate method for waiting for > >>> CP_IRQ, than completions concept. WIP to understand and implement > >>> that, if needed. Just to unblock the review of other changes, v2 > >>> still continues with completions. > >>> > >>> v2: > >>> wait for cp_irq is merged with this patch. Rebased. > >>> v3: > >>> wait_queue is used for wait for cp_irq [Chris Wilson] > >>> v4: > >>> Style fixed. > >>> %s/PARING/PAIRING > >>> Few style fixes [Uma] > >>> > >>> Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > >>> --- > >>> drivers/gpu/drm/i915/intel_dp.c | 358 > >>> ++++++++++++++++++++++++++++++++++++++ > >>> drivers/gpu/drm/i915/intel_drv.h | 7 + > >>> drivers/gpu/drm/i915/intel_hdcp.c | 5 + > >>> 3 files changed, 370 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c > >>> b/drivers/gpu/drm/i915/intel_dp.c index 528407d608c8..ee71a26ec23f > >>> 100644 > >>> --- a/drivers/gpu/drm/i915/intel_dp.c > >>> +++ b/drivers/gpu/drm/i915/intel_dp.c > >>> @@ -31,6 +31,7 @@ > >>> #include <linux/types.h> > >>> #include <linux/notifier.h> > >>> #include <linux/reboot.h> > >>> +#include <linux/mei_hdcp.h> > >>> #include <asm/byteorder.h> > >>> #include <drm/drmP.h> > >>> #include <drm/drm_atomic_helper.h> @@ -5057,6 +5058,28 @@ void > >>> intel_dp_encoder_suspend(struct intel_encoder > >>> *intel_encoder) > >>> pps_unlock(intel_dp); > >>> } > >>> +static int intel_dp_hdcp_wait_for_cp_irq(struct intel_hdcp *hdcp, > >>> + int timeout) { > >>> + long ret; > >>> + > >>> + /* Reinit */ > >>> + atomic_set(&hdcp->cp_irq_recved, 0); > >>> + > >>> +#define C (atomic_read(&hdcp->cp_irq_recved) > 0) > >>> + ret = wait_event_interruptible_timeout(hdcp->cp_irq_queue, > >>> +C, > >>> + > >>> msecs_to_jiffies(timeout)); > >>> + > >>> + if (ret > 0) { > >>> + atomic_set(&hdcp->cp_irq_recved, 0); > >>> + return 0; > >>> + } else if (!ret) { > >>> + return -ETIMEDOUT; > >>> + } > >>> + return (int)ret; > >>> +} > >>> + > >>> + > >>> static > >>> int intel_dp_hdcp_write_an_aksv(struct intel_digital_port > >>> *intel_dig_port, > >>> u8 *an) @@ -5275,6 +5298,335 @@ int > >>> intel_dp_hdcp_capable(struct intel_digital_port *intel_dig_port, > >>> return 0; > >>> } > >>> +static > >>> +int intel_dpcd_offset_for_hdcp2_msgid(uint8_t byte, unsigned int > >>> *offset) > >> > >> Ugh, this is annoying that we have to map things around like that. > >> But looking at the numbers the standards really don't seem to match at all. > > > > Sorry i am not getting about not matching part. > > You mean some offsets and timeouts are not matching with spec? > > match as in you can't just compute them using a base_offset + hdcp_msgid trick, > you do have to use the lookup table. The numbers itself match the spec, it's just > that the specs are all inconsistent with each another for no real good reason. > > >> > >> > >> Instead of open-coding this I suggested you use a table with a struct > >> like: > >> > >> const static struct hdcp_dp { > >> int hdcp_msg; > >> int dpcd_offset; > >> int timeout; > >> /* whatever else you might need */ } hdcp_2_dp_table[] = { > >> { HDCP_2_2_AKE_INIT, DP_HDCP_2_2_AKE_INIT_OFFSET, ... }, > >> ... > >> }; > >> > >> Then just search that table in the code instead of the huge switch > >> table below. > > > > Suggesting this for cpu optimization or for coding style? > > Just coding style, having to check a table is easier than checking huge case > statements. None of this code matters from a performance pov. Sure Daniel. I will use array of struct here. Thanks -Ram > > > > > > >> > >>> +{ > >>> + switch (byte) { > >>> + case HDCP_2_2_AKE_INIT: > >>> + *offset = DP_HDCP_2_2_AKE_INIT_OFFSET; > >>> + break; > >>> + case HDCP_2_2_AKE_SEND_CERT: > >>> + *offset = DP_HDCP_2_2_AKE_SEND_CERT_OFFSET; > >>> + break; > >>> + case HDCP_2_2_AKE_NO_STORED_KM: > >>> + *offset = DP_HDCP_2_2_AKE_NO_STORED_KM_OFFSET; > >>> + break; > >>> + case HDCP_2_2_AKE_STORED_KM: > >>> + *offset = DP_HDCP_2_2_AKE_STORED_KM_OFFSET; > >>> + break; > >>> + case HDCP_2_2_AKE_SEND_HPRIME: > >>> + *offset = DP_HDCP_2_2_AKE_SEND_HPRIME_OFFSET; > >>> + break; > >>> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: > >>> + *offset = DP_HDCP_2_2_AKE_SEND_PAIRING_INFO_OFFSET; > >>> + break; > >>> + case HDCP_2_2_LC_INIT: > >>> + *offset = DP_HDCP_2_2_LC_INIT_OFFSET; > >>> + break; > >>> + case HDCP_2_2_LC_SEND_LPRIME: > >>> + *offset = DP_HDCP_2_2_LC_SEND_LPRIME_OFFSET; > >>> + break; > >>> + case HDCP_2_2_SKE_SEND_EKS: > >>> + *offset = DP_HDCP_2_2_SKE_SEND_EKS_OFFSET; > >>> + break; > >>> + case HDCP_2_2_REP_SEND_RECVID_LIST: > >>> + *offset = DP_HDCP_2_2_REP_SEND_RECVID_LIST_OFFSET; > >>> + break; > >>> + case HDCP_2_2_REP_SEND_ACK: > >>> + *offset = DP_HDCP_2_2_REP_SEND_ACK_OFFSET; > >>> + break; > >>> + case HDCP_2_2_REP_STREAM_MANAGE: > >>> + *offset = DP_HDCP_2_2_REP_STREAM_MANAGE_OFFSET; > >>> + break; > >>> + case HDCP_2_2_REP_STREAM_READY: > >>> + *offset = DP_HDCP_2_2_REP_STREAM_READY_OFFSET; > >>> + break; > >>> + case HDCP_2_2_ERRATA_DP_STREAM_TYPE: > >>> + *offset = DP_HDCP_2_2_REG_STREAM_TYPE_OFFSET; > >>> + break; > >>> + default: > >>> + DRM_ERROR("Unrecognized Msg ID\n"); > >>> + return -EINVAL; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static inline > >>> +int intel_dp_hdcp2_read_rx_status(struct intel_digital_port > >>> *intel_dig_port, > >>> + uint8_t *rx_status) { > >>> + ssize_t ret; > >>> + > >>> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, > >>> + DP_HDCP_2_2_REG_RXSTATUS_OFFSET, > >>> rx_status, > >>> + HDCP_2_2_DP_RXSTATUS_LEN); > >>> + if (ret != HDCP_2_2_DP_RXSTATUS_LEN) { > >>> + DRM_ERROR("Read bstatus from DP/AUX failed (%ld)\n", > >>> ret); > >>> + return ret >= 0 ? -EIO : ret; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static > >>> +int intel_dp_hdcp2_timeout_for_msg(uint8_t msg_id, bool paired) > >> > >> For the timeout and the availability check below, is that not the > >> same for hdmi? Might be good to set the timeout/check values once, > >> and perhaps pass that then down to the dp/hdmi implementation, > >> instead of computing it here. > >> > >> All these functions here look like a huge layering violation. Imo all > >> this timeout stuff should be handled higher up, or at least more > >> tightly grouped together. > > > > Between HDMI and DP adaptation differences are there but very minimal. > > Message avialbility check is completely different between HDMI and DP. > > DP uses the cp_irq. > > And timeout also varies for a DP HDCP msg. > > > > Since we are adapting the HDCP1.4 design where all encoder specific > > operations are pushed into hdcp_shim, I have implemented all HDMI and > > DP related operation to their respective shims. > > So that we need not do more if(dp/hdmi) in common code intel_hdcp.c > atleast. > > > > The first design I had shared using the drm helper functions, dont > > have different flows for HDMI and DP. > > Where ever different treatment is required based on the adaptation, > > that was taken care then and there. > > I'm not against having dp/hdmi specific functions, I'm against having to spread > that specific stuff all around in different places. If you need msgid specific > timeouts between DP and HDMI then maybe stuff them into the lookup table, > but from a quick look it seemed like that's not the case. > > And if the hdcp1.4 shim design is getting in the way, we need to fix that. The > hdcp1.4 shim is indeed a bit looking like a midlayer, so it getting in the way is no > surprise really. > -Daniel > > > > > -Ram > > > > > >> Remapping from the HDCP_2_2 defines to the dpcd offsets is imo > >> different than all the stuff below here. > >> -Daniel > >> > >>> +{ > >>> + int timeout; > >>> + > >>> + switch (msg_id) { > >>> + case HDCP_2_2_AKE_SEND_CERT: > >>> + timeout = HDCP_2_2_CERT_TIMEOUT; > >>> + break; > >>> + case HDCP_2_2_AKE_SEND_HPRIME: > >>> + if (paired) > >>> + timeout = HDCP_2_2_HPRIME_PAIRED_TIMEOUT; > >>> + else > >>> + timeout = HDCP_2_2_HPRIME_NO_PAIRED_TIMEOUT; > >>> + break; > >>> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: > >>> + timeout = HDCP_2_2_PAIRING_TIMEOUT; > >>> + break; > >>> + case HDCP_2_2_LC_SEND_LPRIME: > >>> + timeout = HDCP_2_2_DP_LPRIME_TIMEOUT; > >>> + break; > >>> + case HDCP_2_2_REP_SEND_RECVID_LIST: > >>> + timeout = HDCP_2_2_RECVID_LIST_TIMEOUT; > >>> + break; > >>> + case HDCP_2_2_REP_STREAM_READY: > >>> + timeout = HDCP_2_2_STREAM_READY_TIMEOUT; > >>> + break; > >>> + default: > >>> + timeout = -EINVAL; > >>> + DRM_ERROR("Unsupported msg_id: %d\n", (int)msg_id); > >>> + } > >>> + > >>> + return timeout; > >>> +} > >>> + > >>> +static > >>> +int hdcp2_detect_msg_availability(struct intel_digital_port > >>> *intel_dig_port, > >>> + uint8_t msg_id, bool *msg_ready) { > >>> + uint8_t rx_status; > >>> + int ret; > >>> + > >>> + *msg_ready = false; > >>> + ret = intel_dp_hdcp2_read_rx_status(intel_dig_port, &rx_status); > >>> + if (ret < 0) > >>> + return ret; > >>> + > >>> + switch (msg_id) { > >>> + case HDCP_2_2_AKE_SEND_HPRIME: > >>> + if (HDCP_2_2_DP_RXSTATUS_H_PRIME(rx_status)) > >>> + *msg_ready = true; > >>> + break; > >>> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: > >>> + if (HDCP_2_2_DP_RXSTATUS_PAIRING(rx_status)) > >>> + *msg_ready = true; > >>> + break; > >>> + case HDCP_2_2_REP_SEND_RECVID_LIST: > >>> + if (HDCP_2_2_DP_RXSTATUS_READY(rx_status)) > >>> + *msg_ready = true; > >>> + break; > >>> + default: > >>> + DRM_DEBUG_KMS("Unidentified msg_id: %d\n", (int)msg_id); > >>> + return -EINVAL; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> + > >>> +static ssize_t > >>> +intel_dp_hdcp2_wait_for_msg(struct intel_digital_port *intel_dig_port, > >>> + uint8_t msg_id) { > >>> + struct intel_dp *dp = &intel_dig_port->dp; > >>> + struct intel_hdcp *hdcp = &dp->attached_connector->hdcp; > >>> + int ret, timeout; > >>> + bool msg_ready = false; > >>> + > >>> + timeout = intel_dp_hdcp2_timeout_for_msg(msg_id, > >>> hdcp->is_paired); > >>> + switch (msg_id) { > >>> + > >>> + /* > >>> + * There is no way to detect the CERT, LPRIME and STREAM_READY > >>> + * availability. So Wait for timeout and read the msg. > >>> + */ > >>> + case HDCP_2_2_AKE_SEND_CERT: > >>> + case HDCP_2_2_LC_SEND_LPRIME: > >>> + case HDCP_2_2_REP_STREAM_READY: > >>> + mdelay(timeout); > >>> + ret = 0; > >>> + break; > >>> + case HDCP_2_2_AKE_SEND_HPRIME: > >>> + case HDCP_2_2_AKE_SEND_PAIRING_INFO: > >>> + case HDCP_2_2_REP_SEND_RECVID_LIST: > >>> + intel_dp_hdcp_wait_for_cp_irq(hdcp, timeout); > >>> + ret = hdcp2_detect_msg_availability(intel_dig_port, > >>> msg_id, > >>> + &msg_ready); > >>> + if (!msg_ready) > >>> + ret = -ETIMEDOUT; > >>> + break; > >>> + default: > >>> + DRM_DEBUG_KMS("Unidentified msg_id: %d\n", (int)msg_id); > >>> + return -EINVAL; > >>> + } > >>> + if (ret) > >>> + DRM_ERROR("msg_id %d, ret %d, timeout(mSec): %d\n", > >>> msg_id, ret, > >>> + timeout); > >>> + > >>> + return ret; > >>> +} > >>> + > >>> +static > >>> +int intel_dp_hdcp2_write_msg(struct intel_digital_port *intel_dig_port, > >>> + void *buf, size_t size) { > >>> + unsigned int offset; > >>> + uint8_t *byte = buf; > >>> + ssize_t ret, bytes_to_write, len; > >>> + > >>> + if (intel_dpcd_offset_for_hdcp2_msgid(*byte, &offset) < 0) > >>> + return -EINVAL; > >>> + > >>> + /* No msg_id in DP HDCP2.2 msgs */ > >>> + bytes_to_write = size - 1; > >>> + byte++; > >>> + > >>> + while (bytes_to_write) { > >>> + len = bytes_to_write > DP_AUX_MAX_PAYLOAD_BYTES ? > >>> + DP_AUX_MAX_PAYLOAD_BYTES : > >>> bytes_to_write; > >>> + > >>> + ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, offset, > >>> + (void *)byte, len); > >>> + if (ret < 0) > >>> + return ret; > >>> + > >>> + bytes_to_write -= ret; > >>> + byte += ret; > >>> + offset += ret; > >>> + } > >>> + > >>> + return size; > >>> +} > >>> + > >>> +static > >>> +int intel_dp_hdcp2_read_msg(struct intel_digital_port *intel_dig_port, > >>> + uint8_t msg_id, void *buf, size_t size) > >>> +{ > >>> + unsigned int offset, dev_cnt; > >>> + uint8_t *byte = buf; > >>> + uint8_t rx_info[HDCP_2_2_RXINFO_LEN]; > >>> + ssize_t ret, bytes_to_recv, len; > >>> + > >>> + if (intel_dpcd_offset_for_hdcp2_msgid(msg_id, &offset) < 0) > >>> + return -EINVAL; > >>> + > >>> + ret = intel_dp_hdcp2_wait_for_msg(intel_dig_port, msg_id); > >>> + if (ret < 0) > >>> + return ret; > >>> + > >>> + /* Finding the ReceiverID List size */ > >>> + if (msg_id == HDCP_2_2_REP_SEND_RECVID_LIST) { > >>> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, > >>> + DP_HDCP_2_2_REG_RXINFO_OFFSET, > >>> + (void *)rx_info, > >>> HDCP_2_2_RXINFO_LEN); > >>> + if (ret != HDCP_2_2_RXINFO_LEN) > >>> + return ret >= 0 ? -EIO : ret; > >>> + > >>> + dev_cnt = (HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 | > >>> + HDCP_2_2_DEV_COUNT_LO(rx_info[1])); > >>> + > >>> + if (dev_cnt > HDCP_2_2_MAX_DEVICE_COUNT) > >>> + dev_cnt = HDCP_2_2_MAX_DEVICE_COUNT; > >>> + > >>> + size = sizeof(struct hdcp2_rep_send_receiverid_list) - > >>> + HDCP_2_2_RECEIVER_IDS_MAX_LEN + > >>> + (dev_cnt * HDCP_2_2_RECEIVER_ID_LEN); > >>> + } > >>> + > >>> + bytes_to_recv = size - 1; > >>> + > >>> + /* To skip the msg_id, as msgs in DP adaptation has no msg_id */ > >>> + byte++; > >>> + > >>> + while (bytes_to_recv) { > >>> + len = bytes_to_recv > DP_AUX_MAX_PAYLOAD_BYTES ? > >>> + DP_AUX_MAX_PAYLOAD_BYTES : bytes_to_recv; > >>> + > >>> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, offset, > >>> + (void *)byte, len); > >>> + if (ret < 0) { > >>> + DRM_DEBUG_KMS("msg_id %d, ret %d\n", msg_id, > >>> (int)ret); > >>> + return ret; > >>> + } > >>> + > >>> + bytes_to_recv -= ret; > >>> + byte += ret; > >>> + offset += ret; > >>> + } > >>> + byte = buf; > >>> + *byte = msg_id; > >>> + > >>> + return size; > >>> +} > >>> + > >>> +static > >>> +int intel_dp_hdcp2_config_stream_type(struct intel_digital_port > >>> *intel_dig_port, > >>> + void *buf, size_t size) { > >>> + return intel_dp_hdcp2_write_msg(intel_dig_port, buf, size); > >>> +} > >>> + > >>> +static > >>> +int intel_dp_hdcp2_check_link(struct intel_digital_port > >>> +*intel_dig_port) { > >>> + uint8_t rx_status; > >>> + int ret; > >>> + > >>> + ret = intel_dp_hdcp2_read_rx_status(intel_dig_port, &rx_status); > >>> + if (ret) > >>> + return ret; > >>> + > >>> + if (HDCP_2_2_DP_RXSTATUS_REAUTH_REQ(rx_status)) > >>> + ret = DRM_HDCP_REAUTH_REQUEST; > >>> + else if (HDCP_2_2_DP_RXSTATUS_LINK_FAILED(rx_status)) > >>> + ret = DRM_HDCP_LINK_INTEGRITY_FAILURE; > >>> + else if (HDCP_2_2_DP_RXSTATUS_READY(rx_status)) > >>> + ret = DRM_HDCP_TOPOLOGY_CHANGE; > >>> + > >>> + return ret; > >>> +} > >>> + > >>> +static > >>> +int intel_dp_hdcp2_capable(struct intel_digital_port *intel_dig_port, > >>> + bool *capable) { > >>> + uint8_t rx_caps[3]; > >>> + int ret; > >>> + > >>> + *capable = false; > >>> + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, > >>> + DP_HDCP_2_2_REG_RX_CAPS_OFFSET, > >>> + rx_caps, HDCP_2_2_RXCAPS_LEN); > >>> + if (ret != HDCP_2_2_RXCAPS_LEN) > >>> + return ret >= 0 ? -EIO : ret; > >>> + > >>> + if (rx_caps[0] == HDCP_2_2_RXCAPS_VERSION_VAL && > >>> + HDCP_2_2_DP_HDCP_CAPABLE(rx_caps[2])) > >>> + *capable = true; > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static > >>> +enum hdcp_protocol intel_dp_hdcp2_protocol(void) { > >>> + return HDCP_PROTOCOL_DP; > >>> +} > >>> + > >>> 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, @@ -5287,6 +5639,12 @@ > >>> static const struct intel_hdcp_shim intel_dp_hdcp_shim = { > >>> .toggle_signalling = intel_dp_hdcp_toggle_signalling, > >>> .check_link = intel_dp_hdcp_check_link, > >>> .hdcp_capable = intel_dp_hdcp_capable, > >>> + .write_2_2_msg = intel_dp_hdcp2_write_msg, > >>> + .read_2_2_msg = intel_dp_hdcp2_read_msg, > >>> + .config_stream_type = intel_dp_hdcp2_config_stream_type, > >>> + .check_2_2_link = intel_dp_hdcp2_check_link, > >>> + .hdcp_2_2_capable = intel_dp_hdcp2_capable, > >>> + .hdcp_protocol = intel_dp_hdcp2_protocol, > >>> }; > >>> static void intel_edp_panel_vdd_sanitize(struct intel_dp > >>> *intel_dp) diff --git a/drivers/gpu/drm/i915/intel_drv.h > >>> b/drivers/gpu/drm/i915/intel_drv.h > >>> index def8e3255742..0496ebec287b 100644 > >>> --- a/drivers/gpu/drm/i915/intel_drv.h > >>> +++ b/drivers/gpu/drm/i915/intel_drv.h > >>> @@ -444,6 +444,13 @@ struct intel_hdcp { > >>> struct mei_hdcp_data mei_data; > >>> struct notifier_block mei_cldev_nb; > >>> struct delayed_work hdcp2_check_work; > >>> + > >>> + /* > >>> + * Work queue to signal the CP_IRQ. Used for the waiters to > >>> + read > >>> the > >>> + * available information from HDCP DP sink. > >>> + */ > >>> + wait_queue_head_t cp_irq_queue; > >>> + atomic_t cp_irq_recved; > >>> }; > >>> struct intel_connector { > >>> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > >>> b/drivers/gpu/drm/i915/intel_hdcp.c > >>> index d060d7f2e13b..d502bdb380d2 100644 > >>> --- a/drivers/gpu/drm/i915/intel_hdcp.c > >>> +++ b/drivers/gpu/drm/i915/intel_hdcp.c > >>> @@ -870,6 +870,8 @@ int intel_hdcp_init(struct intel_connector > >>> *connector, > >>> if (hdcp2_supported) > >>> intel_hdcp2_init(connector); > >>> + init_waitqueue_head(&hdcp->cp_irq_queue); > >>> + atomic_set(&hdcp->cp_irq_recved, 0); > >>> return 0; > >>> } > >>> @@ -1849,4 +1851,7 @@ void intel_hdcp_handle_cp_irq(struct > >>> intel_connector *connector) > >>> intel_hdcp_check_link(connector); > >>> else if (intel_hdcp2_in_force(connector)) > >>> intel_hdcp2_check_link(connector); > >>> + > >>> + atomic_set(&connector->hdcp.cp_irq_recved, 1); > >>> + wake_up_all(&connector->hdcp.cp_irq_queue); > >>> } > >>> -- > >>> 2.7.4 > >>> > > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 528407d608c8..ee71a26ec23f 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -31,6 +31,7 @@ #include <linux/types.h> #include <linux/notifier.h> #include <linux/reboot.h> +#include <linux/mei_hdcp.h> #include <asm/byteorder.h> #include <drm/drmP.h> #include <drm/drm_atomic_helper.h> @@ -5057,6 +5058,28 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder) pps_unlock(intel_dp); } +static int intel_dp_hdcp_wait_for_cp_irq(struct intel_hdcp *hdcp, + int timeout) +{ + long ret; + + /* Reinit */ + atomic_set(&hdcp->cp_irq_recved, 0); + +#define C (atomic_read(&hdcp->cp_irq_recved) > 0) + ret = wait_event_interruptible_timeout(hdcp->cp_irq_queue, C, + msecs_to_jiffies(timeout)); + + if (ret > 0) { + atomic_set(&hdcp->cp_irq_recved, 0); + return 0; + } else if (!ret) { + return -ETIMEDOUT; + } + return (int)ret; +} + + static int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port, u8 *an) @@ -5275,6 +5298,335 @@ int intel_dp_hdcp_capable(struct intel_digital_port *intel_dig_port, return 0; } +static +int intel_dpcd_offset_for_hdcp2_msgid(uint8_t byte, unsigned int *offset) +{ + switch (byte) { + case HDCP_2_2_AKE_INIT: + *offset = DP_HDCP_2_2_AKE_INIT_OFFSET; + break; + case HDCP_2_2_AKE_SEND_CERT: + *offset = DP_HDCP_2_2_AKE_SEND_CERT_OFFSET; + break; + case HDCP_2_2_AKE_NO_STORED_KM: + *offset = DP_HDCP_2_2_AKE_NO_STORED_KM_OFFSET; + break; + case HDCP_2_2_AKE_STORED_KM: + *offset = DP_HDCP_2_2_AKE_STORED_KM_OFFSET; + break; + case HDCP_2_2_AKE_SEND_HPRIME: + *offset = DP_HDCP_2_2_AKE_SEND_HPRIME_OFFSET; + break; + case HDCP_2_2_AKE_SEND_PAIRING_INFO: + *offset = DP_HDCP_2_2_AKE_SEND_PAIRING_INFO_OFFSET; + break; + case HDCP_2_2_LC_INIT: + *offset = DP_HDCP_2_2_LC_INIT_OFFSET; + break; + case HDCP_2_2_LC_SEND_LPRIME: + *offset = DP_HDCP_2_2_LC_SEND_LPRIME_OFFSET; + break; + case HDCP_2_2_SKE_SEND_EKS: + *offset = DP_HDCP_2_2_SKE_SEND_EKS_OFFSET; + break; + case HDCP_2_2_REP_SEND_RECVID_LIST: + *offset = DP_HDCP_2_2_REP_SEND_RECVID_LIST_OFFSET; + break; + case HDCP_2_2_REP_SEND_ACK: + *offset = DP_HDCP_2_2_REP_SEND_ACK_OFFSET; + break; + case HDCP_2_2_REP_STREAM_MANAGE: + *offset = DP_HDCP_2_2_REP_STREAM_MANAGE_OFFSET; + break; + case HDCP_2_2_REP_STREAM_READY: + *offset = DP_HDCP_2_2_REP_STREAM_READY_OFFSET; + break; + case HDCP_2_2_ERRATA_DP_STREAM_TYPE: + *offset = DP_HDCP_2_2_REG_STREAM_TYPE_OFFSET; + break; + default: + DRM_ERROR("Unrecognized Msg ID\n"); + return -EINVAL; + } + + return 0; +} + +static inline +int intel_dp_hdcp2_read_rx_status(struct intel_digital_port *intel_dig_port, + uint8_t *rx_status) +{ + ssize_t ret; + + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, + DP_HDCP_2_2_REG_RXSTATUS_OFFSET, rx_status, + HDCP_2_2_DP_RXSTATUS_LEN); + if (ret != HDCP_2_2_DP_RXSTATUS_LEN) { + DRM_ERROR("Read bstatus from DP/AUX failed (%ld)\n", ret); + return ret >= 0 ? -EIO : ret; + } + + return 0; +} + +static +int intel_dp_hdcp2_timeout_for_msg(uint8_t msg_id, bool paired) +{ + int timeout; + + switch (msg_id) { + case HDCP_2_2_AKE_SEND_CERT: + timeout = HDCP_2_2_CERT_TIMEOUT; + break; + case HDCP_2_2_AKE_SEND_HPRIME: + if (paired) + timeout = HDCP_2_2_HPRIME_PAIRED_TIMEOUT; + else + timeout = HDCP_2_2_HPRIME_NO_PAIRED_TIMEOUT; + break; + case HDCP_2_2_AKE_SEND_PAIRING_INFO: + timeout = HDCP_2_2_PAIRING_TIMEOUT; + break; + case HDCP_2_2_LC_SEND_LPRIME: + timeout = HDCP_2_2_DP_LPRIME_TIMEOUT; + break; + case HDCP_2_2_REP_SEND_RECVID_LIST: + timeout = HDCP_2_2_RECVID_LIST_TIMEOUT; + break; + case HDCP_2_2_REP_STREAM_READY: + timeout = HDCP_2_2_STREAM_READY_TIMEOUT; + break; + default: + timeout = -EINVAL; + DRM_ERROR("Unsupported msg_id: %d\n", (int)msg_id); + } + + return timeout; +} + +static +int hdcp2_detect_msg_availability(struct intel_digital_port *intel_dig_port, + uint8_t msg_id, bool *msg_ready) +{ + uint8_t rx_status; + int ret; + + *msg_ready = false; + ret = intel_dp_hdcp2_read_rx_status(intel_dig_port, &rx_status); + if (ret < 0) + return ret; + + switch (msg_id) { + case HDCP_2_2_AKE_SEND_HPRIME: + if (HDCP_2_2_DP_RXSTATUS_H_PRIME(rx_status)) + *msg_ready = true; + break; + case HDCP_2_2_AKE_SEND_PAIRING_INFO: + if (HDCP_2_2_DP_RXSTATUS_PAIRING(rx_status)) + *msg_ready = true; + break; + case HDCP_2_2_REP_SEND_RECVID_LIST: + if (HDCP_2_2_DP_RXSTATUS_READY(rx_status)) + *msg_ready = true; + break; + default: + DRM_DEBUG_KMS("Unidentified msg_id: %d\n", (int)msg_id); + return -EINVAL; + } + + return 0; +} + + +static ssize_t +intel_dp_hdcp2_wait_for_msg(struct intel_digital_port *intel_dig_port, + uint8_t msg_id) +{ + struct intel_dp *dp = &intel_dig_port->dp; + struct intel_hdcp *hdcp = &dp->attached_connector->hdcp; + int ret, timeout; + bool msg_ready = false; + + timeout = intel_dp_hdcp2_timeout_for_msg(msg_id, hdcp->is_paired); + switch (msg_id) { + + /* + * There is no way to detect the CERT, LPRIME and STREAM_READY + * availability. So Wait for timeout and read the msg. + */ + case HDCP_2_2_AKE_SEND_CERT: + case HDCP_2_2_LC_SEND_LPRIME: + case HDCP_2_2_REP_STREAM_READY: + mdelay(timeout); + ret = 0; + break; + case HDCP_2_2_AKE_SEND_HPRIME: + case HDCP_2_2_AKE_SEND_PAIRING_INFO: + case HDCP_2_2_REP_SEND_RECVID_LIST: + intel_dp_hdcp_wait_for_cp_irq(hdcp, timeout); + ret = hdcp2_detect_msg_availability(intel_dig_port, msg_id, + &msg_ready); + if (!msg_ready) + ret = -ETIMEDOUT; + break; + default: + DRM_DEBUG_KMS("Unidentified msg_id: %d\n", (int)msg_id); + return -EINVAL; + } + if (ret) + DRM_ERROR("msg_id %d, ret %d, timeout(mSec): %d\n", msg_id, ret, + timeout); + + return ret; +} + +static +int intel_dp_hdcp2_write_msg(struct intel_digital_port *intel_dig_port, + void *buf, size_t size) +{ + unsigned int offset; + uint8_t *byte = buf; + ssize_t ret, bytes_to_write, len; + + if (intel_dpcd_offset_for_hdcp2_msgid(*byte, &offset) < 0) + return -EINVAL; + + /* No msg_id in DP HDCP2.2 msgs */ + bytes_to_write = size - 1; + byte++; + + while (bytes_to_write) { + len = bytes_to_write > DP_AUX_MAX_PAYLOAD_BYTES ? + DP_AUX_MAX_PAYLOAD_BYTES : bytes_to_write; + + ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, offset, + (void *)byte, len); + if (ret < 0) + return ret; + + bytes_to_write -= ret; + byte += ret; + offset += ret; + } + + return size; +} + +static +int intel_dp_hdcp2_read_msg(struct intel_digital_port *intel_dig_port, + uint8_t msg_id, void *buf, size_t size) +{ + unsigned int offset, dev_cnt; + uint8_t *byte = buf; + uint8_t rx_info[HDCP_2_2_RXINFO_LEN]; + ssize_t ret, bytes_to_recv, len; + + if (intel_dpcd_offset_for_hdcp2_msgid(msg_id, &offset) < 0) + return -EINVAL; + + ret = intel_dp_hdcp2_wait_for_msg(intel_dig_port, msg_id); + if (ret < 0) + return ret; + + /* Finding the ReceiverID List size */ + if (msg_id == HDCP_2_2_REP_SEND_RECVID_LIST) { + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, + DP_HDCP_2_2_REG_RXINFO_OFFSET, + (void *)rx_info, HDCP_2_2_RXINFO_LEN); + if (ret != HDCP_2_2_RXINFO_LEN) + return ret >= 0 ? -EIO : ret; + + dev_cnt = (HDCP_2_2_DEV_COUNT_HI(rx_info[0]) << 4 | + HDCP_2_2_DEV_COUNT_LO(rx_info[1])); + + if (dev_cnt > HDCP_2_2_MAX_DEVICE_COUNT) + dev_cnt = HDCP_2_2_MAX_DEVICE_COUNT; + + size = sizeof(struct hdcp2_rep_send_receiverid_list) - + HDCP_2_2_RECEIVER_IDS_MAX_LEN + + (dev_cnt * HDCP_2_2_RECEIVER_ID_LEN); + } + + bytes_to_recv = size - 1; + + /* To skip the msg_id, as msgs in DP adaptation has no msg_id */ + byte++; + + while (bytes_to_recv) { + len = bytes_to_recv > DP_AUX_MAX_PAYLOAD_BYTES ? + DP_AUX_MAX_PAYLOAD_BYTES : bytes_to_recv; + + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, offset, + (void *)byte, len); + if (ret < 0) { + DRM_DEBUG_KMS("msg_id %d, ret %d\n", msg_id, (int)ret); + return ret; + } + + bytes_to_recv -= ret; + byte += ret; + offset += ret; + } + byte = buf; + *byte = msg_id; + + return size; +} + +static +int intel_dp_hdcp2_config_stream_type(struct intel_digital_port *intel_dig_port, + void *buf, size_t size) +{ + return intel_dp_hdcp2_write_msg(intel_dig_port, buf, size); +} + +static +int intel_dp_hdcp2_check_link(struct intel_digital_port *intel_dig_port) +{ + uint8_t rx_status; + int ret; + + ret = intel_dp_hdcp2_read_rx_status(intel_dig_port, &rx_status); + if (ret) + return ret; + + if (HDCP_2_2_DP_RXSTATUS_REAUTH_REQ(rx_status)) + ret = DRM_HDCP_REAUTH_REQUEST; + else if (HDCP_2_2_DP_RXSTATUS_LINK_FAILED(rx_status)) + ret = DRM_HDCP_LINK_INTEGRITY_FAILURE; + else if (HDCP_2_2_DP_RXSTATUS_READY(rx_status)) + ret = DRM_HDCP_TOPOLOGY_CHANGE; + + return ret; +} + +static +int intel_dp_hdcp2_capable(struct intel_digital_port *intel_dig_port, + bool *capable) +{ + uint8_t rx_caps[3]; + int ret; + + *capable = false; + ret = drm_dp_dpcd_read(&intel_dig_port->dp.aux, + DP_HDCP_2_2_REG_RX_CAPS_OFFSET, + rx_caps, HDCP_2_2_RXCAPS_LEN); + if (ret != HDCP_2_2_RXCAPS_LEN) + return ret >= 0 ? -EIO : ret; + + if (rx_caps[0] == HDCP_2_2_RXCAPS_VERSION_VAL && + HDCP_2_2_DP_HDCP_CAPABLE(rx_caps[2])) + *capable = true; + + return 0; +} + +static +enum hdcp_protocol intel_dp_hdcp2_protocol(void) +{ + return HDCP_PROTOCOL_DP; +} + 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, @@ -5287,6 +5639,12 @@ static const struct intel_hdcp_shim intel_dp_hdcp_shim = { .toggle_signalling = intel_dp_hdcp_toggle_signalling, .check_link = intel_dp_hdcp_check_link, .hdcp_capable = intel_dp_hdcp_capable, + .write_2_2_msg = intel_dp_hdcp2_write_msg, + .read_2_2_msg = intel_dp_hdcp2_read_msg, + .config_stream_type = intel_dp_hdcp2_config_stream_type, + .check_2_2_link = intel_dp_hdcp2_check_link, + .hdcp_2_2_capable = intel_dp_hdcp2_capable, + .hdcp_protocol = intel_dp_hdcp2_protocol, }; static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index def8e3255742..0496ebec287b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -444,6 +444,13 @@ struct intel_hdcp { struct mei_hdcp_data mei_data; struct notifier_block mei_cldev_nb; struct delayed_work hdcp2_check_work; + + /* + * Work queue to signal the CP_IRQ. Used for the waiters to read the + * available information from HDCP DP sink. + */ + wait_queue_head_t cp_irq_queue; + atomic_t cp_irq_recved; }; struct intel_connector { diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c index d060d7f2e13b..d502bdb380d2 100644 --- a/drivers/gpu/drm/i915/intel_hdcp.c +++ b/drivers/gpu/drm/i915/intel_hdcp.c @@ -870,6 +870,8 @@ int intel_hdcp_init(struct intel_connector *connector, if (hdcp2_supported) intel_hdcp2_init(connector); + init_waitqueue_head(&hdcp->cp_irq_queue); + atomic_set(&hdcp->cp_irq_recved, 0); return 0; } @@ -1849,4 +1851,7 @@ void intel_hdcp_handle_cp_irq(struct intel_connector *connector) intel_hdcp_check_link(connector); else if (intel_hdcp2_in_force(connector)) intel_hdcp2_check_link(connector); + + atomic_set(&connector->hdcp.cp_irq_recved, 1); + wake_up_all(&connector->hdcp.cp_irq_queue); }
Implements the DP adaptation specific HDCP2.2 functions. These functions perform the DPCD read and write for communicating the HDCP2.2 auth message back and forth. Note: Chris Wilson suggested alternate method for waiting for CP_IRQ, than completions concept. WIP to understand and implement that, if needed. Just to unblock the review of other changes, v2 still continues with completions. v2: wait for cp_irq is merged with this patch. Rebased. v3: wait_queue is used for wait for cp_irq [Chris Wilson] v4: Style fixed. %s/PARING/PAIRING Few style fixes [Uma] Signed-off-by: Ramalingam C <ramalingam.c@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 358 ++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 7 + drivers/gpu/drm/i915/intel_hdcp.c | 5 + 3 files changed, 370 insertions(+)