Re: [PATCH] drm/mediatek/dp: Add the HDCP feature for DisplayPort

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

Hi, Mac:

On Fri, 2023-11-24 at 16:53 +0800, mac.shen wrote:
> Add tee client application, HDCP 1.x and 2.x authentication for
> DisplayPort
> to support the HDCP feature.
> 
> Signed-off-by: mac.shen <mac.shen@mediatek.com>
> ---

[snip]

> +
> +int tee_hdcp_enable_encrypt(struct mtk_hdcp_info *hdcp_info, bool
> enable, u8 version)
> +{
> +	int rc;
> +	struct dp_tee_private *dp_tee_priv = hdcp_info->g_dp_tee_priv;
> +	struct tci_t *tci = (struct tci_t *)dp_tee_priv->shm->kaddr;
> +	u8 *share_buffer = (u8 *)dp_tee_priv->shm->kaddr;
> +
> +	/* Copy parameters */
> +	memset(tci, 0, TCI_LENGTH);

You don't need to clear this structure. The field you do not write in
this command would not be access in tee. So it's not necessary to clear
the field that does not write in this command.

> +	tci->command_id = CMD_ENABLE_ENCRYPT;
> +	if (enable)
> +		tci->cmd_body.cmd_hdcp_write_val.type =
> TYPE_HDCP_ENABLE_ENCRYPT;
> +	else
> +		tci->cmd_body.cmd_hdcp_write_val.type =
> TYPE_HDCP_DISABLE_ENCRYPT;
> +
> +	/* Set HDCP version supportted by device */
> +	tci->cmd_body.cmd_hdcp_write_val.len = 1;
> +	memset(share_buffer + TCI_LENGTH, version, 1);

It's weird that some data in the range of share_buffer + 0 ~
share_buffer + TCI_LENGTH - 1, and some data is after share_buffer +
TCI_LENGTH. Do not separate them and code would be more clear.

Regards,
CK



> +
> +	rc = dp_tee_op_send(dp_tee_priv, (u8 *)tci, TCI_LENGTH + 1,
> CMD_ENABLE_ENCRYPT);
> +	if (rc != 0) {
> +		TLCERR("tee_op_send failed, error=%x\n", rc);
> +		return rc;
> +	}
> +
> +	return rc;
> +}
> +