Re: [PATCH v3 5/7] platform/chrome: cros_ec_typec: Displayport support

[Benson Leung <bleung@google.com>]

[-- Attachment #1: Type: text/plain, Size: 14586 bytes --]

Hi Abhishek,

On Thu, Nov 07, 2024 at 11:29:58AM -0800, Abhishek Pandit-Subedi wrote:
> Add support for entering and exiting displayport alt-mode on systems
> using AP driven alt-mode.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> ---
> 
> Changes in v3:
> - Refactored typec_altmode_dp_data per review request
> - Removed unused vdm operations during altmode registration
> 
> Changes in v2:
> - Refactored displayport into cros_typec_altmode.c to extract common
>   implementation between altmodes
> 
>  MAINTAINERS                                  |   3 +
>  drivers/platform/chrome/Makefile             |   4 +
>  drivers/platform/chrome/cros_ec_typec.c      |  12 +-
>  drivers/platform/chrome/cros_ec_typec.h      |   1 +
>  drivers/platform/chrome/cros_typec_altmode.c | 275 +++++++++++++++++++
>  drivers/platform/chrome/cros_typec_altmode.h |  34 +++
>  6 files changed, 326 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/platform/chrome/cros_typec_altmode.c
>  create mode 100644 drivers/platform/chrome/cros_typec_altmode.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cd6aa609deba..5f9d8b8f1cb3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5369,9 +5369,12 @@ F:	include/linux/platform_data/cros_usbpd_notify.h
>  
>  CHROMEOS EC USB TYPE-C DRIVER
>  M:	Prashant Malani <pmalani@chromium.org>
> +M:	Benson Leung <bleung@chromium.org>
> +M:	Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
>  L:	chrome-platform@lists.linux.dev
>  S:	Maintained
>  F:	drivers/platform/chrome/cros_ec_typec.*
> +F:	drivers/platform/chrome/cros_typec_altmode.*
>  F:	drivers/platform/chrome/cros_typec_switch.c
>  F:	drivers/platform/chrome/cros_typec_vdm.*
>  
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index 2dcc6ccc2302..2f90d4db8099 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -18,7 +18,11 @@ obj-$(CONFIG_CROS_EC_SPI)		+= cros_ec_spi.o
>  obj-$(CONFIG_CROS_EC_UART)		+= cros_ec_uart.o
>  cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_mec.o
>  cros-ec-typec-objs			:= cros_ec_typec.o cros_typec_vdm.o
> +ifneq ($(CONFIG_TYPEC_DP_ALTMODE),)
> +	cros-ec-typec-objs		+= cros_typec_altmode.o
> +endif
>  obj-$(CONFIG_CROS_EC_TYPEC)		+= cros-ec-typec.o
> +
>  obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
>  obj-$(CONFIG_CROS_EC_PROTO)		+= cros_ec_proto.o cros_ec_trace.o
>  obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)	+= cros_kbd_led_backlight.o
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index e3eabe5e42ac..3a6f5f2717b9 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -18,6 +18,7 @@
>  
>  #include "cros_ec_typec.h"
>  #include "cros_typec_vdm.h"
> +#include "cros_typec_altmode.h"
>  
>  #define DRV_NAME "cros-ec-typec"
>  
> @@ -293,12 +294,11 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec,

As we debugged late last week, this desc here needs to be initialized ahead
of  the register, as it had some junk from the stack that was causing the
"active" property to be sometimes set to no.

memset(&desc, 0, sizeof(desc));

This worked for me when testing your series.

>  	desc.svid = USB_TYPEC_DP_SID;
>  	desc.mode = USB_TYPEC_DP_MODE;
>  	desc.vdo = DP_PORT_VDO;
> -	amode = typec_port_register_altmode(port->port, &desc);
> +	amode = cros_typec_register_displayport(port, &desc,
> +						typec->ap_driven_altmode);
>  	if (IS_ERR(amode))
>  		return PTR_ERR(amode);
>  	port->port_altmode[CROS_EC_ALTMODE_DP] = amode;
> -	typec_altmode_set_drvdata(amode, port);
> -	amode->ops = &port_amode_ops;
>  
>  	/*
>  	 * Register TBT compatibility alt mode. The EC will not enter the mode
> @@ -575,6 +575,10 @@ static int cros_typec_enable_dp(struct cros_typec_data *typec,
>  	if (!ret)
>  		ret = typec_mux_set(port->mux, &port->state);
>  
> +	if (!ret)
> +		cros_typec_displayport_status_update(port->state.alt,
> +						     port->state.data);
> +
>  	return ret;
>  }
>  
> @@ -1254,6 +1258,8 @@ static int cros_typec_probe(struct platform_device *pdev)
>  
>  	typec->typec_cmd_supported = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_CMD);
>  	typec->needs_mux_ack = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_MUX_REQUIRE_AP_ACK);
> +	typec->ap_driven_altmode = cros_ec_check_features(
> +		ec_dev, EC_FEATURE_TYPEC_REQUIRE_AP_MODE_ENTRY);
>  
>  	ret = cros_ec_cmd(typec->ec, 0, EC_CMD_USB_PD_PORTS, NULL, 0,
>  			  &resp, sizeof(resp));
> diff --git a/drivers/platform/chrome/cros_ec_typec.h b/drivers/platform/chrome/cros_ec_typec.h
> index deda180a646f..9fd5342bb0ad 100644
> --- a/drivers/platform/chrome/cros_ec_typec.h
> +++ b/drivers/platform/chrome/cros_ec_typec.h
> @@ -39,6 +39,7 @@ struct cros_typec_data {
>  	struct work_struct port_work;
>  	bool typec_cmd_supported;
>  	bool needs_mux_ack;
> +	bool ap_driven_altmode;
>  };
>  
>  /* Per port data. */
> diff --git a/drivers/platform/chrome/cros_typec_altmode.c b/drivers/platform/chrome/cros_typec_altmode.c
> new file mode 100644
> index 000000000000..3598b8a6ceee
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_typec_altmode.c
> @@ -0,0 +1,275 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Alt-mode implementation on ChromeOS EC.
> + *
> + * Copyright 2024 Google LLC
> + * Author: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> + */
> +#include "cros_ec_typec.h"
> +
> +#include <linux/usb/typec_dp.h>
> +#include <linux/usb/pd_vdo.h>
> +
> +#include "cros_typec_altmode.h"
> +
> +struct cros_typec_altmode_data {
> +	struct work_struct work;
> +	struct cros_typec_port *port;
> +	struct typec_altmode *alt;
> +	bool ap_mode_entry;
> +
> +	u32 header;
> +	u32 *vdo_data;
> +	u8 vdo_size;
> +
> +	u16 sid;
> +	u8 mode;
> +};
> +
> +struct cros_typec_dp_data {
> +	struct cros_typec_altmode_data adata;
> +	struct typec_displayport_data data;
> +	bool configured;
> +	bool pending_status_update;
> +};
> +
> +static void cros_typec_altmode_work(struct work_struct *work)
> +{
> +	struct cros_typec_altmode_data *data =
> +		container_of(work, struct cros_typec_altmode_data, work);
> +
> +	if (typec_altmode_vdm(data->alt, data->header, data->vdo_data,
> +			      data->vdo_size))
> +		dev_err(&data->alt->dev, "VDM 0x%x failed", data->header);
> +
> +	data->header = 0;
> +	data->vdo_data = NULL;
> +	data->vdo_size = 0;
> +}
> +
> +static int cros_typec_altmode_enter(struct typec_altmode *alt, u32 *vdo)
> +{
> +	struct cros_typec_altmode_data *data = typec_altmode_get_drvdata(alt);
> +	struct ec_params_typec_control req = {
> +		.port = data->port->port_num,
> +		.command = TYPEC_CONTROL_COMMAND_ENTER_MODE,
> +	};
> +	int svdm_version;
> +	int ret;
> +
> +	if (!data->ap_mode_entry) {
> +		const struct typec_altmode *partner =
> +			typec_altmode_get_partner(alt);
> +		dev_warn(&partner->dev,
> +			 "EC does not support ap driven mode entry\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (data->sid == USB_TYPEC_DP_SID)
> +		req.mode_to_enter = CROS_EC_ALTMODE_DP;
> +	else
> +		return -EOPNOTSUPP;
> +
> +	ret = cros_ec_cmd(data->port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL,
> +			  &req, sizeof(req), NULL, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	svdm_version = typec_altmode_get_svdm_version(alt);
> +	if (svdm_version < 0)
> +		return svdm_version;
> +
> +	data->header = VDO(data->sid, 1, svdm_version, CMD_ENTER_MODE);
> +	data->header |= VDO_OPOS(data->mode);
> +	data->header |= VDO_CMDT(CMDT_RSP_ACK);
> +
> +	data->vdo_data = NULL;
> +	data->vdo_size = 1;
> +
> +	schedule_work(&data->work);
> +
> +	return ret;
> +}
> +
> +static int cros_typec_altmode_exit(struct typec_altmode *alt)
> +{
> +	struct cros_typec_altmode_data *data = typec_altmode_get_drvdata(alt);
> +	struct ec_params_typec_control req = {
> +		.port = data->port->port_num,
> +		.command = TYPEC_CONTROL_COMMAND_EXIT_MODES,
> +	};
> +	int svdm_version;
> +	int ret;
> +
> +	if (!data->ap_mode_entry) {
> +		const struct typec_altmode *partner =
> +			typec_altmode_get_partner(alt);
> +		dev_warn(&partner->dev,
> +			 "EC does not support ap driven mode entry\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	ret = cros_ec_cmd(data->port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL,
> +			  &req, sizeof(req), NULL, 0);
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	svdm_version = typec_altmode_get_svdm_version(alt);
> +	if (svdm_version < 0)
> +		return svdm_version;
> +
> +	data->header = VDO(data->sid, 1, svdm_version, CMD_EXIT_MODE);
> +	data->header |= VDO_OPOS(data->mode);
> +	data->header |= VDO_CMDT(CMDT_RSP_ACK);
> +
> +	data->vdo_data = NULL;
> +	data->vdo_size = 1;
> +
> +	schedule_work(&data->work);
> +
> +	return ret;
> +}
> +
> +static int cros_typec_displayport_vdm(struct typec_altmode *alt, u32 header,
> +				      const u32 *data, int count)
> +{
> +	struct cros_typec_dp_data *dp_data = typec_altmode_get_drvdata(alt);
> +	struct cros_typec_altmode_data *adata = &dp_data->adata;
> +
> +
> +	int cmd_type = PD_VDO_CMDT(header);
> +	int cmd = PD_VDO_CMD(header);
> +	int svdm_version;
> +
> +	if (!adata->ap_mode_entry) {
> +		const struct typec_altmode *partner =
> +			typec_altmode_get_partner(alt);
> +		dev_warn(&partner->dev,
> +			 "EC does not support ap driven mode entry\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	svdm_version = typec_altmode_get_svdm_version(alt);
> +	if (svdm_version < 0)
> +		return svdm_version;
> +
> +	switch (cmd_type) {
> +	case CMDT_INIT:
> +		if (PD_VDO_SVDM_VER(header) < svdm_version) {
> +			typec_partner_set_svdm_version(adata->port->partner,
> +						       PD_VDO_SVDM_VER(header));
> +			svdm_version = PD_VDO_SVDM_VER(header);
> +		}
> +
> +		adata->header = VDO(adata->sid, 1, svdm_version, cmd);
> +		adata->header |= VDO_OPOS(adata->mode);
> +
> +		/*
> +		 * DP_CMD_CONFIGURE: We can't actually do anything with the
> +		 * provided VDO yet so just send back an ACK.
> +		 *
> +		 * DP_CMD_STATUS_UPDATE: We wait for Mux changes to send
> +		 * DPStatus Acks.
> +		 */
> +		switch (cmd) {
> +		case DP_CMD_CONFIGURE:
> +			dp_data->data.conf = *data;
> +			adata->header |= VDO_CMDT(CMDT_RSP_ACK);
> +			dp_data->configured = true;
> +			schedule_work(&adata->work);
> +			break;
> +		case DP_CMD_STATUS_UPDATE:
> +			dp_data->pending_status_update = true;
> +			break;
> +		default:
> +			adata->header |= VDO_CMDT(CMDT_RSP_ACK);
> +			schedule_work(&adata->work);
> +			break;
> +		}
> +
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int cros_typec_altmode_vdm(struct typec_altmode *alt, u32 header,
> +				      const u32 *data, int count)
> +{
> +	struct cros_typec_altmode_data *adata = typec_altmode_get_drvdata(alt);
> +
> +	if (adata->sid == USB_TYPEC_DP_SID)
> +		return cros_typec_displayport_vdm(alt, header, data, count);
> +
> +	return -EINVAL;
> +}
> +
> +static const struct typec_altmode_ops cros_typec_altmode_ops = {
> +	.enter = cros_typec_altmode_enter,
> +	.exit = cros_typec_altmode_exit,
> +	.vdm = cros_typec_altmode_vdm,
> +};
> +
> +#if IS_ENABLED(CONFIG_TYPEC_DP_ALTMODE)
> +int cros_typec_displayport_status_update(struct typec_altmode *altmode,
> +					 struct typec_displayport_data *data)
> +{
> +	struct cros_typec_dp_data *dp_data =
> +		typec_altmode_get_drvdata(altmode);
> +	struct cros_typec_altmode_data *adata = &dp_data->adata;
> +
> +	if (!dp_data->pending_status_update) {
> +		dev_dbg(&altmode->dev,
> +			"Got DPStatus without a pending request");
> +		return 0;
> +	}
> +
> +	if (dp_data->configured && dp_data->data.conf != data->conf)
> +		dev_dbg(&altmode->dev,
> +			"DP Conf doesn't match. Requested 0x%04x, Actual 0x%04x",
> +			dp_data->data.conf, data->conf);
> +
> +	dp_data->data = *data;
> +	dp_data->pending_status_update = false;
> +	adata->header |= VDO_CMDT(CMDT_RSP_ACK);
> +	adata->vdo_data = &dp_data->data.status;
> +	adata->vdo_size = 2;
> +
> +	schedule_work(&adata->work);
> +	return 0;
> +}
> +
> +struct typec_altmode *
> +cros_typec_register_displayport(struct cros_typec_port *port,
> +				struct typec_altmode_desc *desc,
> +				bool ap_mode_entry)
> +{
> +	struct typec_altmode *alt;
> +	struct cros_typec_altmode_data *data;
> +
> +	alt = typec_port_register_altmode(port->port, desc);
> +	if (IS_ERR(alt))
> +		return alt;
> +
> +	data = devm_kzalloc(&alt->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data) {
> +		typec_unregister_altmode(alt);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	INIT_WORK(&data->work, cros_typec_altmode_work);
> +	data->alt = alt;
> +	data->port = port;
> +	data->ap_mode_entry = ap_mode_entry;
> +	data->sid = desc->svid;
> +	data->mode = desc->mode;
> +
> +	typec_altmode_set_ops(alt, &cros_typec_altmode_ops);
> +	typec_altmode_set_drvdata(alt, data);
> +
> +	return alt;
> +}
> +#endif
> diff --git a/drivers/platform/chrome/cros_typec_altmode.h b/drivers/platform/chrome/cros_typec_altmode.h
> new file mode 100644
> index 000000000000..c6f8fb02c99c
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_typec_altmode.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __CROS_TYPEC_ALTMODE_H__
> +#define __CROS_TYPEC_ALTMODE_H__
> +
> +struct cros_typec_port;
> +struct typec_altmode;
> +struct typec_altmode_desc;
> +struct typec_displayport_data;
> +
> +#if IS_ENABLED(CONFIG_TYPEC_DP_ALTMODE)
> +struct typec_altmode *
> +cros_typec_register_displayport(struct cros_typec_port *port,
> +				struct typec_altmode_desc *desc,
> +				bool ap_mode_entry);
> +
> +int cros_typec_displayport_status_update(struct typec_altmode *altmode,
> +					 struct typec_displayport_data *data);
> +#else
> +static inline struct typec_altmode *
> +cros_typec_register_displayport(struct cros_typec_port *port,
> +				struct typec_altmode_desc *desc,
> +				bool ap_mode_entry)
> +{
> +	return typec_port_register_altmode(port->port, desc);
> +}
> +
> +static inline int cros_typec_displayport_status_update(struct typec_altmode *altmode,
> +					 struct typec_displayport_data *data)
> +{
> +	return 0;
> +}
> +#endif
> +#endif /* __CROS_TYPEC_ALTMODE_H__ */
> -- 
> 2.47.0.277.g8800431eea-goog
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]