Quoting Sean Paul (2021-11-04 20:04:31) > From: Sean Paul <seanpaul@xxxxxxxxxxxx> > > This patch adds HDCP 1.x support to msm DP connectors using the new HDCP $ git grep "This patch" -- Documentation/process/ > helpers. > > Cc: Stephen Boyd <swboyd@xxxxxxxxxxxx> > Cc: Abhinav Kumar <abhinavk@xxxxxxxxxxxxxx> > Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx> > Link: https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-15-sean@xxxxxxxxxx #v1 > Link: https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-14-sean@xxxxxxxxxx #v2 > Link: https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-15-sean@xxxxxxxxxx #v3 > > Changes in v2: > -Squash [1] into this patch with the following changes (Stephen) > -Update the sc7180 dtsi file > -Remove resource names and just use index (Stephen) > Changes in v3: > -Split out the dtsi change from v2 (Stephen) > -Fix set-but-unused warning identified by 0-day > -Fix up a couple of style nits (Stephen) > -Store HDCP key directly in dp_hdcp struct (Stephen) > -Remove wmb in HDCP key initialization, move an_seed (Stephen) > -Use FIELD_PREP for bstatus/bcaps (Stephen) > -#define read_poll_timeout values (Stephen) > -Remove unnecessary parentheses in dp_hdcp_store_ksv_fifo (Stephen) > -Add compatible string for hdcp (Stephen) > -Rename dp_hdcp_write_* functions (Abhinav) > -Add 1us delay between An reads (Abhinav) > -Delete unused dp_hdcp_read_* functions > Changes in v4: > -Rebase on Bjorn's multi-dp patchset > > [1] https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-14-sean@xxxxxxxxxx Looks mostly ok to me. One nit below but otherwise you can have my Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx> > diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c b/drivers/gpu/drm/msm/dp/dp_debug.c > index da4323556ef3..c16fce17d096 100644 > --- a/drivers/gpu/drm/msm/dp/dp_debug.c > +++ b/drivers/gpu/drm/msm/dp/dp_debug.c > @@ -198,6 +201,35 @@ 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, > + size_t len, loff_t *offp) I deem this API through debugfs no good, but I can see that opening the can of worms that is programming the key other ways is worse, so alright. > +{ > + char *input_buffer; > + int ret; > + struct dp_debug_private *debug = file->private_data; > + > + if (len != (DRM_HDCP_KSV_LEN + DP_HDCP_NUM_KEYS * DP_HDCP_KEY_LEN)) > + return -EINVAL; > + [....] > 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..03ea3a974576 > --- /dev/null > +++ b/drivers/gpu/drm/msm/dp/dp_hdcp.c > @@ -0,0 +1,462 @@ [...] > + > +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->parser->io.dp_controller.hdcp_key.base) > + return 0; > + > + helper_data = drm_hdcp_helper_initialize_dp(connector, hdcp->aux, > + &dp_hdcp_funcs, false); > + if (IS_ERR_OR_NULL(helper_data)) Just IS_ERR()? > + return PTR_ERR(helper_data); Because PTR_ERR() on NULL is zero. Maybe return PTR_ERR_OR_ZERO() is supposed to be here? Or I don't understand why drm_hdcp_helper_initialize_dp() would return NULL. > + > + 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; > + }