Quoting Sean Paul (2021-09-15 13:38:32) > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > index c8921e2d6480..3ae6fc7a2c01 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > @@ -3088,7 +3088,9 @@ mdss_dp: displayport-controller@ae90000 { > compatible = "qcom,sc7180-dp"; > status = "disabled"; > > - reg = <0 0x0ae90000 0 0x1400>; > + reg = <0 0x0ae90000 0 0x1400>, > + <0 0x0aed1000 0 0x174>, > + <0 0x0aee1000 0 0x2c>; > I suspect we'll still want this hunk of the patch to be split off and go through arm-soc tree. > interrupt-parent = <&mdss>; > interrupts = <12>; > diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c b/drivers/gpu/drm/msm/dp/dp_debug.c > index 2f6247e80e9d..de16fca8782a 100644 > --- a/drivers/gpu/drm/msm/dp/dp_debug.c > +++ b/drivers/gpu/drm/msm/dp/dp_debug.c > @@ -8,6 +8,7 @@ > #include <linux/debugfs.h> > #include <drm/drm_connector.h> > #include <drm/drm_file.h> > +#include <drm/drm_hdcp.h> > > #include "dp_parser.h" > #include "dp_catalog.h" > @@ -15,6 +16,7 @@ > #include "dp_ctrl.h" > #include "dp_debug.h" > #include "dp_display.h" > +#include "dp_hdcp.h" > > #define DEBUG_NAME "msm_dp" > > @@ -24,6 +26,7 @@ struct dp_debug_private { > struct dp_usbpd *usbpd; > struct dp_link *link; > struct dp_panel *panel; > + struct dp_hdcp *hdcp; > struct drm_connector **connector; > struct device *dev; > struct drm_device *drm_dev; > @@ -349,6 +352,38 @@ static int dp_test_active_open(struct inode *inode, > inode->i_private); > } > > +static ssize_t dp_hdcp_key_write(struct file *file, const char __user *ubuf, Is this the API that userspace is going to use to set the key? Or a simple debug interface that's used to test this code out? I hope it's a debugging aid and not the normal flow given that it's through debugfs. > + size_t len, loff_t *offp) > +{ > + char *input_buffer; > + int ret = 0; Please don't assign variables and then overwrite without testing the variable. > + struct dp_debug_private *debug = file->private_data; > + struct drm_device *dev; > + > + dev = debug->drm_dev; > + > + if (len != (DRM_HDCP_KSV_LEN + DP_HDCP_NUM_KEYS * DP_HDCP_KEY_LEN)) > + return -EINVAL; > + > + if (!debug->hdcp) > + return -ENOENT; > + > + input_buffer = memdup_user_nul(ubuf, len); > + if (IS_ERR(input_buffer)) > + return PTR_ERR(input_buffer); > + > + ret = dp_hdcp_ingest_key(debug->hdcp, input_buffer, len); > + > + kfree(input_buffer); > + if (ret < 0) { > + DRM_ERROR("Could not ingest HDCP key, ret=%d\n", ret); > + return ret; > + } > + > + *offp += len; > + return len; > +} > + > static const struct file_operations dp_debug_fops = { > .open = simple_open, > .read = dp_debug_read_info, > @@ -363,6 +398,12 @@ static const struct file_operations test_active_fops = { > .write = dp_test_active_write > }; > > +static const struct file_operations dp_hdcp_key_fops = { > + .owner = THIS_MODULE, > + .open = simple_open, > + .write = dp_hdcp_key_write, > +}; > + > static int dp_debug_init(struct dp_debug *dp_debug, struct drm_minor *minor) > { > int rc = 0; > @@ -384,6 +425,10 @@ static int dp_debug_init(struct dp_debug *dp_debug, struct drm_minor *minor) > minor->debugfs_root, > debug, &dp_test_type_fops); > > + debugfs_create_file("msm_dp_hdcp_key", 0222, > + minor->debugfs_root, > + debug, &dp_hdcp_key_fops); > + > debug->root = minor->debugfs_root; > > return rc; > @@ -391,7 +436,8 @@ static int dp_debug_init(struct dp_debug *dp_debug, struct drm_minor *minor) > > struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel, > struct dp_usbpd *usbpd, struct dp_link *link, > - struct drm_connector **connector, struct drm_minor *minor) > + struct dp_hdcp *hdcp, struct drm_connector **connector, > + struct drm_minor *minor) > { > int rc = 0; > struct dp_debug_private *debug; > @@ -413,6 +459,7 @@ struct dp_debug *dp_debug_get(struct device *dev, struct dp_panel *panel, > debug->usbpd = usbpd; > debug->link = link; > debug->panel = panel; > + debug->hdcp = hdcp; > debug->dev = dev; > debug->drm_dev = minor->dev; > debug->connector = connector; > diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h > index 8b47cdabb67e..421268e47f30 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.h > +++ b/drivers/gpu/drm/msm/dp/dp_display.h > @@ -27,8 +27,13 @@ struct msm_dp { > struct dp_audio *dp_audio; > }; > > +struct drm_atomic_state; > + > int dp_display_set_plugged_cb(struct msm_dp *dp_display, > hdmi_codec_plugged_cb fn, struct device *codec_dev); > +struct dp_hdcp *dp_display_connector_to_hdcp(struct drm_connector *connector); > +void dp_display_hdcp_commit(struct msm_dp *dp_display, > + struct drm_atomic_state *state); > int dp_display_validate_mode(struct msm_dp *dp_display, u32 mode_pclk_khz); > int dp_display_get_modes(struct msm_dp *dp_display, > struct dp_display_mode *dp_mode); > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c > index 764f4b81017e..8e62558b4fc3 100644 > --- a/drivers/gpu/drm/msm/dp/dp_drm.c > +++ b/drivers/gpu/drm/msm/dp/dp_drm.c > @@ -5,11 +5,20 @@ > > #include <drm/drm_atomic_helper.h> > #include <drm/drm_atomic.h> > +#include <drm/drm_connector.h> > #include <drm/drm_crtc.h> > +#include <drm/drm_hdcp.h> > > #include "msm_drv.h" > #include "msm_kms.h" > #include "dp_drm.h" > +#include "dp_hdcp.h" > + > +struct dp_connector_state { > + struct drm_connector_state base; > + bool hdcp_transition; > +}; > +#define to_dp_connector_state(x) container_of(x, struct dp_connector_state, base) > > struct dp_connector { > struct drm_connector base; > @@ -17,6 +26,11 @@ struct dp_connector { > }; > #define to_dp_connector(x) container_of(x, struct dp_connector, base) > > +struct msm_dp *msm_dp_from_connector(struct drm_connector *connector) > +{ > + return to_dp_connector(connector)->dp_display; > +} > + > /** > * dp_connector_detect - callback to determine if connector is connected > * @conn: Pointer to drm connector structure > @@ -114,20 +128,72 @@ static enum drm_mode_status dp_connector_mode_valid( > return dp_display_validate_mode(dp_disp, mode->clock); > } > > +static int dp_connector_atomic_check(struct drm_connector *connector, > + struct drm_atomic_state *state) > +{ > + struct drm_connector_state *conn_state; > + struct dp_connector_state *dp_state; > + > + conn_state = drm_atomic_get_new_connector_state(state, connector); > + dp_state = to_dp_connector_state(conn_state); > + > + dp_state->hdcp_transition = drm_hdcp_atomic_check(connector, state); > + > + return 0; > +} > + > +static struct drm_connector_state * > +dp_connector_atomic_duplicate_state(struct drm_connector *connector) > +{ > + struct dp_connector_state *state; > + > + state = kzalloc(sizeof(*state), GFP_KERNEL); > + if (!state) > + return NULL; > + > + state->hdcp_transition = false; > + > + __drm_atomic_helper_connector_duplicate_state(connector, &state->base); > + return &state->base; > +} > + > static const struct drm_connector_funcs dp_connector_funcs = { > .detect = dp_connector_detect, > .fill_modes = drm_helper_probe_single_connector_modes, > .destroy = drm_connector_cleanup, > .reset = drm_atomic_helper_connector_reset, > - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > + .atomic_duplicate_state = dp_connector_atomic_duplicate_state, > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > }; > > static const struct drm_connector_helper_funcs dp_connector_helper_funcs = { > .get_modes = dp_connector_get_modes, > .mode_valid = dp_connector_mode_valid, > + .atomic_check = dp_connector_atomic_check, > }; > > +bool dp_drm_is_connector_msm_dp(struct drm_connector *connector) > +{ > + return connector->funcs == &dp_connector_funcs; > +} > + > +void dp_drm_atomic_commit(struct drm_connector *connector, > + struct drm_connector_state *conn_state, > + struct drm_atomic_state *state) > +{ > + struct dp_connector_state *dp_state; > + struct msm_dp *dp_disp; > + > + dp_state = to_dp_connector_state(conn_state); > + > + if (!dp_state->hdcp_transition) > + return; > + > + dp_disp = msm_dp_from_connector(connector); > + > + dp_display_hdcp_commit(dp_disp, state); > +} > + > /* connector initialization */ > struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display) > { > diff --git a/drivers/gpu/drm/msm/dp/dp_drm.h b/drivers/gpu/drm/msm/dp/dp_drm.h > index c27bfceefdf0..a5d95c6acd67 100644 > --- a/drivers/gpu/drm/msm/dp/dp_drm.h > +++ b/drivers/gpu/drm/msm/dp/dp_drm.h > @@ -14,5 +14,10 @@ > #include "dp_display.h" > > struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display); > +struct msm_dp *msm_dp_from_connector(struct drm_connector *connector); > +bool dp_drm_is_connector_msm_dp(struct drm_connector *connector); > +void dp_drm_atomic_commit(struct drm_connector *connector, > + struct drm_connector_state *conn_state, > + struct drm_atomic_state *state); > > #endif /* _DP_DRM_H_ */ > diff --git a/drivers/gpu/drm/msm/dp/dp_hdcp.c b/drivers/gpu/drm/msm/dp/dp_hdcp.c > new file mode 100644 > index 000000000000..07d2a1f04d97 > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/dp_hdcp.c > @@ -0,0 +1,433 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright (C) 2021 Google, Inc. > + * > + * Authors: > + * Sean Paul <seanpaul@xxxxxxxxxxxx> > + */ > + > +#include "dp_display.h" > +#include "dp_drm.h" > +#include "dp_hdcp.h" > +#include "dp_reg.h" > + > +#include <drm/drm_connector.h> > +#include <drm/drm_device.h> > +#include <drm/drm_dp_helper.h> > +#include <drm/drm_hdcp.h> > +#include <drm/drm_print.h> > + > +#include <linux/bitfield.h> > +#include <linux/bits.h> > +#include <linux/iopoll.h> > +#include <linux/mutex.h> > +#include <linux/random.h> > +#include <linux/slab.h> > + > +/* Offsets based on hdcp_ksv mmio */ > +#define DP_HDCP_KSV_AN_LSB 0x0 > +#define DP_HDCP_KSV_AN_MSB 0x4 > +#define DP_HDCP_KSV_AKSV_MSB 0x1D8 > +#define DP_HDCP_KSV_AKSV_LSB 0x1DC > + > +/* Key offsets based on hdcp_key mmio */ > +#define DP_HDCP_KEY_BASE 0x30 > +#define DP_HDCP_KEY_MSB(x) (DP_HDCP_KEY_BASE + (x * 8)) > +#define DP_HDCP_KEY_LSB(x) (DP_HDCP_KEY_MSB(x) + 4) > +#define DP_HDCP_KEY_VALID 0x170 > +#define DP_HDCP_SW_KEY_VALID BIT(0) > + > +/* > + * dp_hdcp_key - structure which contains an HDCP key set > + * @ksv: The key selection vector > + * @keys: Contains 40 keys > + */ > +struct dp_hdcp_key { > + struct drm_hdcp_ksv ksv; > + union { > + u32 words[2]; > + u8 bytes[DP_HDCP_KEY_LEN]; > + } keys[DP_HDCP_NUM_KEYS]; > + bool valid; > +}; > + > +struct dp_hdcp { > + struct drm_device *dev; > + struct drm_connector *connector; > + > + struct drm_dp_aux *aux; > + struct dp_parser *parser; > + > + struct drm_hdcp_helper_data *helper_data; > + > + struct mutex key_lock; > + struct dp_hdcp_key *key; Is there a reason this is a pointer vs. a plain struct member? > +}; > + > +static inline void dp_hdcp_write_dp(struct dp_hdcp *hdcp, u32 offset, u32 val) > +{ > + writel(val, hdcp->parser->io.dp_controller.base + offset); > +} > + > +static inline u32 dp_hdcp_read_dp(struct dp_hdcp *hdcp, u32 offset) > +{ > + return readl(hdcp->parser->io.dp_controller.base + offset); > +} > + > +static inline void dp_hdcp_write_hdcp(struct dp_hdcp *hdcp, u32 offset, u32 val) > +{ > + writel(val, hdcp->parser->io.hdcp_key.base + offset); > +} > + > +static inline u32 dp_hdcp_read_hdcp(struct dp_hdcp *hdcp, u32 offset) > +{ > + return readl(hdcp->parser->io.hdcp_key.base + offset); > +} > + > +static inline void dp_hdcp_write_tz(struct dp_hdcp *hdcp, u32 offset, u32 val) > +{ > + writel(val, hdcp->parser->io.hdcp_tz.base + offset); > +} > + > +static inline u32 dp_hdcp_read_tz(struct dp_hdcp *hdcp, u32 offset) > +{ > + return readl(hdcp->parser->io.hdcp_tz.base + offset); > +} > + > +int dp_hdcp_ingest_key(struct dp_hdcp *hdcp, const u8 *raw_key, int raw_len) > +{ > + struct dp_hdcp_key *key; > + const u8 *ptr = raw_key; Why have the local variable when raw_key will do? > + unsigned int ksv_weight; > + int i, ret = 0; > + > + mutex_lock(&hdcp->key_lock); This can move after the length check? > + > + if (raw_len != (DRM_HDCP_KSV_LEN + DP_HDCP_NUM_KEYS * DP_HDCP_KEY_LEN)) { > + DRM_ERROR("Invalid HDCP key length expected=%d actual=%d\n", > + (DRM_HDCP_KSV_LEN + DP_HDCP_NUM_KEYS * DP_HDCP_KEY_LEN), > + raw_len); > + ret = -EINVAL; > + goto out; > + } > + > + key = hdcp->key; > + > + memcpy(key->ksv.bytes, ptr, DRM_HDCP_KSV_LEN); > + ksv_weight = hweight32(key->ksv.words[0]) + hweight32(key->ksv.words[1]); > + if (ksv_weight != 20) { > + DRM_ERROR("Invalid ksv weight, expected=20 actual=%d\n", > + ksv_weight); > + ret = -EINVAL; > + goto out; > + } > + > + ptr += DRM_HDCP_KSV_LEN; > + for (i = 0; i < DP_HDCP_NUM_KEYS; i++) { > + memcpy(key->keys[i].bytes, ptr, DP_HDCP_KEY_LEN); > + ptr += DP_HDCP_KEY_LEN; > + } > + > + DRM_DEBUG_DRIVER("Successfully ingested HDCP key\n"); > + hdcp->key->valid = true; > + > +out: > + mutex_unlock(&hdcp->key_lock); > + return ret; > +} > + > +static bool dp_hdcp_are_keys_valid(struct drm_connector *connector) > +{ > + struct dp_hdcp *hdcp = dp_display_connector_to_hdcp(connector); > + u32 val; > + > + val = dp_hdcp_read_dp(hdcp, DP_HDCP_STATUS); > + return FIELD_GET(DP_HDCP_KEY_STATUS, val) == DP_HDCP_KEY_STATUS_VALID; > +} > + > +static int dp_hdcp_load_keys(struct drm_connector *connector) > +{ > + struct dp_hdcp *hdcp = dp_display_connector_to_hdcp(connector); > + struct dp_hdcp_key *key; > + int i, ret = 0; > + > + mutex_lock(&hdcp->key_lock); > + > + key = hdcp->key; > + > + if (!key->valid) { > + ret = -ENOENT; > + goto out; > + } > + > + dp_hdcp_write_dp(hdcp, DP_HDCP_SW_LOWER_AKSV, key->ksv.words[0]); > + dp_hdcp_write_dp(hdcp, DP_HDCP_SW_UPPER_AKSV, key->ksv.words[1]); > + > + for (i = 0; i < DP_HDCP_NUM_KEYS; i++) { > + dp_hdcp_write_hdcp(hdcp, DP_HDCP_KEY_LSB(i), > + key->keys[i].words[0]); > + dp_hdcp_write_hdcp(hdcp, DP_HDCP_KEY_MSB(i), > + key->keys[i].words[1]); > + } > + > + dp_hdcp_write_hdcp(hdcp, DP_HDCP_KEY_VALID, DP_HDCP_SW_KEY_VALID); > + wmb(); What are the wmb()s for? Can you add a comment indicating what we're trying to fix by having them? > + > + dp_hdcp_write_dp(hdcp, DP_HDCP_ENTROPY_CTRL0, get_random_u32()); > + dp_hdcp_write_dp(hdcp, DP_HDCP_ENTROPY_CTRL1, get_random_u32()); Can we call get_random_u64() at the start of this function outside the mutex lock and then use the upper and lower halves for these two lines above? > + wmb(); > + > +out: > + mutex_unlock(&hdcp->key_lock); > + return ret; > +} > + > +static int dp_hdcp_hdcp2_capable(struct drm_connector *connector, bool *capable) > +{ > + *capable = false; > + return 0; > +} > + > +static int dp_hdcp_hdcp1_read_an_aksv(struct drm_connector *connector, > + u32 *an, u32 *aksv) > +{ > + struct dp_hdcp *hdcp = dp_display_connector_to_hdcp(connector); > + bool keys_valid; > + int ret; > + u32 val; > + > + dp_hdcp_write_dp(hdcp, DP_HDCP_CTRL, 1); > + > + ret = read_poll_timeout(dp_hdcp_are_keys_valid, keys_valid, keys_valid, > + 20 * 1000, 10 * 1000, false, connector); > + if (ret) { > + drm_err(hdcp->dev, "HDCP keys invalid %d\n", ret); > + return ret; > + } > + > + /* Clear AInfo */ > + dp_hdcp_write_dp(hdcp, DP_HDCP_RCVPORT_DATA4, 0); > + > + aksv[0] = dp_hdcp_read_dp(hdcp, DP_HDCP_RCVPORT_DATA3); > + aksv[1] = GENMASK(7, 0) & dp_hdcp_read_dp(hdcp, DP_HDCP_RCVPORT_DATA4); > + > + ret = read_poll_timeout(dp_hdcp_read_dp, val, > + (val & DP_HDCP_AN_READY_MASK) == DP_HDCP_AN_READY_MASK, > + 100, 10 * 1000, false, hdcp, DP_HDCP_STATUS); > + if (ret) { > + drm_err(hdcp->dev, "AN failed to become ready %x/%d\n", val, ret); > + return ret; > + } > + > + /* > + * Get An from hardware, for unknown reasons we need to read the reg > + * twice to get valid data. That's annoying. > + */ > + dp_hdcp_read_dp(hdcp, DP_HDCP_RCVPORT_DATA5); > + an[0] = dp_hdcp_read_dp(hdcp, DP_HDCP_RCVPORT_DATA5); > + > + dp_hdcp_read_dp(hdcp, DP_HDCP_RCVPORT_DATA6); > + an[1] = dp_hdcp_read_dp(hdcp, DP_HDCP_RCVPORT_DATA6); > + > + return 0; > +} > + > +static int dp_hdcp_hdcp1_store_receiver_info(struct drm_connector *connector, > + u32 *ksv, u32 status, u8 bcaps, > + bool is_repeater) > +{ > + struct dp_hdcp *hdcp = dp_display_connector_to_hdcp(connector); > + u32 val; > + > + dp_hdcp_write_tz(hdcp, HDCP_SEC_DP_TZ_HV_HLOS_HDCP_RCVPORT_DATA0, > + ksv[0]); > + dp_hdcp_write_tz(hdcp, HDCP_SEC_DP_TZ_HV_HLOS_HDCP_RCVPORT_DATA1, > + ksv[1]); > + > + val = ((status & GENMASK(15, 0)) << 8) | (bcaps & GENMASK(7, 0)); Nitpick: Can this use FIELD_PREP() too? > + > + dp_hdcp_write_tz(hdcp, HDCP_SEC_DP_TZ_HV_HLOS_HDCP_RCVPORT_DATA12, val); > + > + return 0; > +} > + > +static int dp_hdcp_hdcp1_enable_encryption(struct drm_connector *connector) > +{ > + return 0; > +} > + > +static int dp_hdcp_hdcp1_wait_for_r0(struct drm_connector *connector) > +{ > + struct dp_hdcp *hdcp = dp_display_connector_to_hdcp(connector); > + int ret; > + u32 val; > + > + ret = read_poll_timeout(dp_hdcp_read_dp, val, (val & DP_HDCP_R0_READY), > + 100, 1000, false, hdcp, > + DP_HDCP_STATUS); > + if (ret) { > + drm_err(hdcp->dev, "HDCP R0 not ready %x/%d\n", val, ret); > + return ret; > + } > + > + return 0; > +} > + > +static int dp_hdcp_hdcp1_match_ri(struct drm_connector *connector, u32 ri_prime) > +{ > + struct dp_hdcp *hdcp = dp_display_connector_to_hdcp(connector); > + int ret; > + u32 val; > + > + dp_hdcp_write_dp(hdcp, DP_HDCP_RCVPORT_DATA2_0, ri_prime); > + > + ret = read_poll_timeout(dp_hdcp_read_dp, val, (val & DP_HDCP_RI_MATCH), > + 20 * 1000, 100 * 1000, false, hdcp, Maybe 20 * 1000 and 100 * 1000 should be some defines at the top of this file? > + DP_HDCP_STATUS); > + if (ret) { > + drm_err(hdcp->dev, "Failed to match Ri and Ri` (%08x) %08x/%d\n", > + ri_prime, val, ret); > + return ret; > + } > + return 0; > +} > + > +static int dp_hdcp_hdcp1_store_ksv_fifo(struct drm_connector *connector, > + u8 *ksv_fifo, u8 num_downstream, > + u8 *bstatus, u32 *vprime) > +{ > + struct dp_hdcp *hdcp = dp_display_connector_to_hdcp(connector); > + int num_bytes = (num_downstream * DRM_HDCP_KSV_LEN); Nitpick: Why the parenthesis? > + int ret, i; > + u32 val; > + > + /* Reset the SHA computation block */ > + dp_hdcp_write_tz(hdcp, HDCP_SEC_DP_TZ_HV_HLOS_HDCP_SHA_CTRL, > + DP_HDCP_SHA_CTRL_RESET); > + dp_hdcp_write_tz(hdcp, HDCP_SEC_DP_TZ_HV_HLOS_HDCP_SHA_CTRL, 0); > + > + /* > + * KSV info gets written a byte at a time in the same order it was > + * received. Every 64 bytes, we need to wait for the SHA_BLOCK_DONE > + * bit to be set in SHA_CTRL. > + */ > + for (i = 0; i < num_bytes; i++) { > + val = FIELD_PREP(DP_HDCP_SHA_DATA_MASK, ksv_fifo[i]); > + > + if (i == (num_bytes - 1)) > + val |= DP_HDCP_SHA_DATA_DONE; > + > + dp_hdcp_write_tz(hdcp, HDCP_SEC_DP_TZ_HV_HLOS_HDCP_SHA_DATA, > + val); > + > + if (((i + 1) % 64) != 0) > + continue; > + > + ret = read_poll_timeout(dp_hdcp_read_dp, val, > + (val & DP_HDCP_SHA_DONE), 100, > + 100 * 1000, false, hdcp, > + DP_HDCP_SHA_STATUS); > + if (ret) { > + drm_err(hdcp->dev, "SHA block incomplete %d\n", ret); > + return ret; > + } > + } > + > + ret = read_poll_timeout(dp_hdcp_read_dp, val, > + (val & DP_HDCP_SHA_COMP_DONE), 100, 100 * 1000, > + false, hdcp, DP_HDCP_SHA_STATUS); > + if (ret) { > + drm_err(hdcp->dev, "SHA computation incomplete %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +static int dp_hdcp_hdcp1_disable(struct drm_connector *connector) > +{ > + struct dp_hdcp *hdcp = dp_display_connector_to_hdcp(connector); > + u32 val; > + > + val = dp_hdcp_read_dp(hdcp, REG_DP_SW_RESET); > + dp_hdcp_write_dp(hdcp, REG_DP_SW_RESET, val | DP_HDCP_SW_RESET); > + > + /* Disable encryption and disable the HDCP block */ > + dp_hdcp_write_dp(hdcp, DP_HDCP_CTRL, 0); > + > + dp_hdcp_write_dp(hdcp, REG_DP_SW_RESET, val); > + > + return 0; > +} > + > +void dp_hdcp_commit(struct dp_hdcp *hdcp, struct drm_atomic_state *state) > +{ > + drm_hdcp_helper_atomic_commit(hdcp->helper_data, state, NULL); > +} > + > +static const struct drm_hdcp_helper_funcs dp_hdcp_funcs = { > + .are_keys_valid = dp_hdcp_are_keys_valid, > + .load_keys = dp_hdcp_load_keys, > + .hdcp2_capable = dp_hdcp_hdcp2_capable, > + .hdcp1_read_an_aksv = dp_hdcp_hdcp1_read_an_aksv, > + .hdcp1_store_receiver_info = dp_hdcp_hdcp1_store_receiver_info, > + .hdcp1_enable_encryption = dp_hdcp_hdcp1_enable_encryption, > + .hdcp1_wait_for_r0 = dp_hdcp_hdcp1_wait_for_r0, > + .hdcp1_match_ri = dp_hdcp_hdcp1_match_ri, > + .hdcp1_store_ksv_fifo = dp_hdcp_hdcp1_store_ksv_fifo, > + .hdcp1_disable = dp_hdcp_hdcp1_disable, > +}; > + > +int dp_hdcp_attach(struct dp_hdcp *hdcp, struct drm_connector *connector) > +{ > + struct drm_device *dev = connector->dev; > + struct drm_hdcp_helper_data *helper_data; > + int ret; > + > + /* HDCP is not configured for this device */ > + if (!hdcp || !hdcp->parser || hdcp->parser->io.hdcp_key.len == 0) > + return 0; > + > + helper_data = drm_hdcp_helper_initialize_dp(connector, hdcp->aux, > + &dp_hdcp_funcs, false); > + if (IS_ERR_OR_NULL(helper_data)) > + return PTR_ERR(helper_data); > + > + ret = drm_connector_attach_content_protection_property(connector, false); > + if (ret) { > + drm_hdcp_helper_destroy(helper_data); > + drm_err(dev, "Failed to attach content protection prop %d\n", ret); > + return ret; > + } > + > + hdcp->dev = connector->dev; > + hdcp->connector = connector; > + hdcp->helper_data = helper_data; > + > + return 0; > +} > + > +struct dp_hdcp *dp_hdcp_get(struct dp_parser *parser, struct drm_dp_aux *aux) > +{ > + struct dp_hdcp *hdcp; > + > + hdcp = devm_kzalloc(&parser->pdev->dev, sizeof(*hdcp), GFP_KERNEL); > + if (!hdcp) > + return ERR_PTR(-ENOMEM); > + > + hdcp->key = devm_kzalloc(&parser->pdev->dev, sizeof(*hdcp->key), GFP_KERNEL); > + if (!hdcp->key) > + return ERR_PTR(-ENOMEM); > + > + hdcp->parser = parser; > + hdcp->aux = aux; > + > + mutex_init(&hdcp->key_lock); > + > + return hdcp; > +} > + > +void dp_hdcp_put(struct dp_hdcp *hdcp) > +{ > + drm_hdcp_helper_destroy(hdcp->helper_data); > +} > diff --git a/drivers/gpu/drm/msm/dp/dp_hdcp.h b/drivers/gpu/drm/msm/dp/dp_hdcp.h > new file mode 100644 > index 000000000000..5637a9b0dea2 > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/dp_hdcp.h > @@ -0,0 +1,27 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright (C) 2021 Google, Inc. > + * > + * Authors: > + * Sean Paul <seanpaul@xxxxxxxxxxxx> > + */ > + > +#ifndef DP_HDCP_H_ > +#define DP_HDCP_H_ > + > +#define DP_HDCP_KEY_LEN 7 > +#define DP_HDCP_NUM_KEYS 40 > + > +struct dp_hdcp; > +struct dp_parser; > +struct drm_atomic_state; > +struct drm_dp_aux; > + > +struct dp_hdcp *dp_hdcp_get(struct dp_parser *parser, struct drm_dp_aux *aux); > +void dp_hdcp_put(struct dp_hdcp *hdcp); > + > +int dp_hdcp_attach(struct dp_hdcp *hdcp, struct drm_connector *connector); > +int dp_hdcp_ingest_key(struct dp_hdcp *hdcp, const u8 *raw_key, int raw_len); > +void dp_hdcp_commit(struct dp_hdcp *hdcp, struct drm_atomic_state *state); > + > +#endif > diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c > index 0519dd3ac3c3..75a163b0b5af 100644 > --- a/drivers/gpu/drm/msm/dp/dp_parser.c > +++ b/drivers/gpu/drm/msm/dp/dp_parser.c > @@ -20,11 +20,11 @@ static const struct dp_regulator_cfg sdm845_dp_reg_cfg = { > }; > > static int msm_dss_ioremap(struct platform_device *pdev, > - struct dss_io_data *io_data) > + struct dss_io_data *io_data, int idx) > { > struct resource *res = NULL; > > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + res = platform_get_resource(pdev, IORESOURCE_MEM, idx); > if (!res) { > DRM_ERROR("%pS->%s: msm_dss_get_res failed\n", > __builtin_return_address(0), __func__); We should remove this error message. It's confusing now that some resources are optional. > @@ -55,6 +55,8 @@ static void dp_parser_unmap_io_resources(struct dp_parser *parser) > { > struct dp_io *io = &parser->io; > > + msm_dss_iounmap(&io->hdcp_tz); > + msm_dss_iounmap(&io->hdcp_key); > msm_dss_iounmap(&io->dp_controller); > } > > @@ -64,10 +66,20 @@ static int dp_parser_ctrl_res(struct dp_parser *parser) > struct platform_device *pdev = parser->pdev; > struct dp_io *io = &parser->io; > > - rc = msm_dss_ioremap(pdev, &io->dp_controller); > - if (rc) { > - DRM_ERROR("unable to remap dp io resources, rc=%d\n", rc); > + rc = msm_dss_ioremap(pdev, &io->dp_controller, 0); > + if (rc) > goto err; > + > + rc = msm_dss_ioremap(pdev, &io->hdcp_key, 1); > + if (rc) { > + io->hdcp_key.base = NULL; > + io->hdcp_key.len = 0; > + } > + > + rc = msm_dss_ioremap(pdev, &io->hdcp_tz, 2); > + if (rc) { > + io->hdcp_tz.base = NULL; > + io->hdcp_tz.len = 0; Bjorn is trying to split the single io region apart into 4 different regions[1]. This would add two more io regions. Maybe this should come after those patches and be indexed later? I worry about needing to add more register properties later on though. Maybe a better approach would be to make them mandatory for certain compatible strings instead. [1] https://lore.kernel.org/r/20210825222557.1499104-6-bjorn.andersson@xxxxxxxxxx > } > > io->phy = devm_phy_get(&pdev->dev, "dp");