| Message ID | 20200218231520.12208-1-ajayg@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: typec: ucsi_ccg: workaround for NVIDIA test device | expand |
Hi Ajay, On Tue, Feb 18, 2020 at 03:15:20PM -0800, Ajay Gupta wrote: > From: Ajay Gupta <ajayg@nvidia.com> > > NVIDIA VirtualLink (svid 0x955) has two altmode, vdo=0x1 for > VirtualLink DP mode and vdo=0x3 for NVIDIA test mode. NVIDIA > test device FTB (Function Test Board) reports altmode list with > vdo=0x3 first and then vdo=0x1. The list is: > SVID VDO > 0xff01 0xc05 > 0x28de 0x8085 > 0x955 0x3 > 0x955 0x1 > > Current logic to assign mode value is based on order > in altmode list. This causes a mismatch of CON and SOP altmodes > since NVIDIA GPU connector has order of vdo=0x1 first and then > vdo=0x3. Fixing this by changing the order of vdo values > reported by NVIDIA test device. the new list will be: > > SVID VDO > 0xff01 0xc05 > 0x28de 0x8085 > 0x955 0x1085 > 0x955 0x3 > > Also NVIDIA VirtualLink (svid 0x955) uses pin E for display mode. > NVIDIA test device reports vdo of 0x1 so make sure vdo values > always have pin E assignement. > > Signed-off-by: Ajay Gupta <ajayg@nvidia.com> > --- > drivers/usb/typec/ucsi/ucsi.h | 2 ++ > drivers/usb/typec/ucsi/ucsi_ccg.c | 27 +++++++++++++++++++++++++++ > 2 files changed, 29 insertions(+) > > diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h > index a89112b69cd5..8e831108f481 100644 > --- a/drivers/usb/typec/ucsi/ucsi.h > +++ b/drivers/usb/typec/ucsi/ucsi.h > @@ -119,12 +119,14 @@ void ucsi_connector_change(struct ucsi *ucsi, u8 num); > #define UCSI_SET_PDR_ACCEPT_ROLE_SWAPS BIT(25) > > /* GET_ALTERNATE_MODES command bits */ > +#define UCSI_ALTMODE_RECIPIENT(_r_) (((_r_) >> 16) & 0x7) > #define UCSI_GET_ALTMODE_RECIPIENT(_r_) ((u64)(_r_) << 16) > #define UCSI_RECIPIENT_CON 0 > #define UCSI_RECIPIENT_SOP 1 > #define UCSI_RECIPIENT_SOP_P 2 > #define UCSI_RECIPIENT_SOP_PP 3 > #define UCSI_GET_ALTMODE_CONNECTOR_NUMBER(_r_) ((u64)(_r_) << 24) > +#define UCSI_ALTMODE_OFFSET(_r_) (((_r_) >> 32) & 0xff) > #define UCSI_GET_ALTMODE_OFFSET(_r_) ((u64)(_r_) << 32) > #define UCSI_GET_ALTMODE_NUM_ALTMODES(_r_) ((u64)(_r_) << 40) > > diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c > index 2658cda5da11..b421b0045448 100644 > --- a/drivers/usb/typec/ucsi/ucsi_ccg.c > +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c > @@ -125,6 +125,10 @@ struct version_format { > #define CCG_FW_BUILD_NVIDIA (('n' << 8) | 'v') > #define CCG_OLD_FW_VERSION (CCG_VERSION(0x31) | CCG_VERSION_PATCH(10)) > > +/* Altmode offset for NVIDIA Function Test Board (FTB) */ > +#define USB_TYPEC_NVIDIA_FTB_DP_OFFSET (2) > +#define USB_TYPEC_NVIDIA_FTB_DBG_OFFSET (3) > + > struct version_info { > struct version_format base; > struct version_format app; > @@ -489,6 +493,29 @@ static int ucsi_ccg_read(struct ucsi *ucsi, unsigned int offset, > return ret; > > if (offset == UCSI_MESSAGE_IN) { > + if (UCSI_COMMAND(uc->last_cmd_sent) == > + UCSI_GET_ALTERNATE_MODES && > + UCSI_ALTMODE_RECIPIENT(uc->last_cmd_sent) == > + UCSI_RECIPIENT_SOP) { > + struct ucsi_altmode *alt = (struct ucsi_altmode *)val; > + > + if (alt[0].svid == USB_TYPEC_NVIDIA_VLINK_SID && > + UCSI_ALTMODE_OFFSET(uc->last_cmd_sent) == > + USB_TYPEC_NVIDIA_FTB_DP_OFFSET && > + alt[0].mid == USB_TYPEC_NVIDIA_VLINK_DBG_VDO) { > + alt[0].mid = USB_TYPEC_NVIDIA_VLINK_DP_VDO | > + DP_CAP_DP_SIGNALING | > + DP_CAP_USB | > + DP_CONF_SET_PIN_ASSIGN(BIT > + (DP_PIN_ASSIGN_E)); > + } > + if (alt[0].svid == USB_TYPEC_NVIDIA_VLINK_SID && > + UCSI_ALTMODE_OFFSET(uc->last_cmd_sent) == > + USB_TYPEC_NVIDIA_FTB_DBG_OFFSET && > + alt[0].mid == USB_TYPEC_NVIDIA_VLINK_DP_VDO) { > + alt[0].mid = USB_TYPEC_NVIDIA_VLINK_DBG_VDO; > + } > + } > if (UCSI_COMMAND(uc->last_cmd_sent) == UCSI_GET_CURRENT_CAM && > uc->has_multiple_dp) { > ucsi_ccg_update_get_current_cam_cmd(uc, (u8 *)val); This did not compile: drivers/usb/typec/ucsi/ucsi_ccg.c: In function ‘ucsi_ccg_read’: drivers/usb/typec/ucsi/ucsi_ccg.c:505:22: error: ‘USB_TYPEC_NVIDIA_VLINK_DBG_VDO’ undeclared (first use in this function); did you mean ‘USB_TYPEC_NVIDIA_VLINK_SID’? 505 | alt[0].mid == USB_TYPEC_NVIDIA_VLINK_DBG_VDO) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | USB_TYPEC_NVIDIA_VLINK_SID drivers/usb/typec/ucsi/ucsi_ccg.c:505:22: note: each undeclared identifier is reported only once for each function it appears in drivers/usb/typec/ucsi/ucsi_ccg.c:506:18: error: ‘USB_TYPEC_NVIDIA_VLINK_DP_VDO’ undeclared (first use in this function); did you mean ‘USB_TYPEC_NVIDIA_VLINK_SID’? 506 | alt[0].mid = USB_TYPEC_NVIDIA_VLINK_DP_VDO | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | USB_TYPEC_NVIDIA_VLINK_SID All those nested conditions make the code a bit difficult to read for me. You could handle the NVIDIA alt mode in its own function. I'm attaching a diff that you should be able to apply on top of your patch that should make the code a bit easier to read. thanks,
On Thu, Feb 27, 2020 at 04:02:07PM +0200, Heikki Krogerus wrote: > This did not compile: > > drivers/usb/typec/ucsi/ucsi_ccg.c: In function ‘ucsi_ccg_read’: > drivers/usb/typec/ucsi/ucsi_ccg.c:505:22: error: ‘USB_TYPEC_NVIDIA_VLINK_DBG_VDO’ undeclared (first use in this function); did you mean ‘USB_TYPEC_NVIDIA_VLINK_SID’? > 505 | alt[0].mid == USB_TYPEC_NVIDIA_VLINK_DBG_VDO) { > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | USB_TYPEC_NVIDIA_VLINK_SID > drivers/usb/typec/ucsi/ucsi_ccg.c:505:22: note: each undeclared identifier is reported only once for each function it appears in > drivers/usb/typec/ucsi/ucsi_ccg.c:506:18: error: ‘USB_TYPEC_NVIDIA_VLINK_DP_VDO’ undeclared (first use in this function); did you mean ‘USB_TYPEC_NVIDIA_VLINK_SID’? > 506 | alt[0].mid = USB_TYPEC_NVIDIA_VLINK_DP_VDO | > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | USB_TYPEC_NVIDIA_VLINK_SID Sorry, I was working on top of the wrong branch. The patch does compile fine... > All those nested conditions make the code a bit difficult to read for > me. You could handle the NVIDIA alt mode in its own function. I'm > attaching a diff that you should be able to apply on top of your patch > that should make the code a bit easier to read. I attached a fixed version of the diff. thanks,
Hi Heikki > -----Original Message----- > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Sent: Thursday, February 27, 2020 6:16 AM > To: Ajay Gupta <ajaykuee@gmail.com> > Cc: linux-usb@vger.kernel.org; Ajay Gupta <ajayg@nvidia.com> > Subject: Re: [PATCH] usb: typec: ucsi_ccg: workaround for NVIDIA test device > > > On Thu, Feb 27, 2020 at 04:02:07PM +0200, Heikki Krogerus wrote: > > This did not compile: > > > > drivers/usb/typec/ucsi/ucsi_ccg.c: In function ‘ucsi_ccg_read’: > > drivers/usb/typec/ucsi/ucsi_ccg.c:505:22: error: > ‘USB_TYPEC_NVIDIA_VLINK_DBG_VDO’ undeclared (first use in this function); > did you mean ‘USB_TYPEC_NVIDIA_VLINK_SID’? > > 505 | alt[0].mid == USB_TYPEC_NVIDIA_VLINK_DBG_VDO) { > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > | USB_TYPEC_NVIDIA_VLINK_SID > > drivers/usb/typec/ucsi/ucsi_ccg.c:505:22: note: each undeclared > > identifier is reported only once for each function it appears in > > drivers/usb/typec/ucsi/ucsi_ccg.c:506:18: error: > ‘USB_TYPEC_NVIDIA_VLINK_DP_VDO’ undeclared (first use in this function); > did you mean ‘USB_TYPEC_NVIDIA_VLINK_SID’? > > 506 | alt[0].mid = USB_TYPEC_NVIDIA_VLINK_DP_VDO | > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > | USB_TYPEC_NVIDIA_VLINK_SID > > Sorry, I was working on top of the wrong branch. The patch does compile > fine... > > > All those nested conditions make the code a bit difficult to read for > > me. You could handle the NVIDIA alt mode in its own function. I'm > > attaching a diff that you should be able to apply on top of your patch > > that should make the code a bit easier to read. > > I attached a fixed version of the diff. Thanks for review. I have updated the change and posted v2. thanks > nvpublic > > thanks, > > -- > heikki
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h index a89112b69cd5..8e831108f481 100644 --- a/drivers/usb/typec/ucsi/ucsi.h +++ b/drivers/usb/typec/ucsi/ucsi.h @@ -119,12 +119,14 @@ void ucsi_connector_change(struct ucsi *ucsi, u8 num); #define UCSI_SET_PDR_ACCEPT_ROLE_SWAPS BIT(25) /* GET_ALTERNATE_MODES command bits */ +#define UCSI_ALTMODE_RECIPIENT(_r_) (((_r_) >> 16) & 0x7) #define UCSI_GET_ALTMODE_RECIPIENT(_r_) ((u64)(_r_) << 16) #define UCSI_RECIPIENT_CON 0 #define UCSI_RECIPIENT_SOP 1 #define UCSI_RECIPIENT_SOP_P 2 #define UCSI_RECIPIENT_SOP_PP 3 #define UCSI_GET_ALTMODE_CONNECTOR_NUMBER(_r_) ((u64)(_r_) << 24) +#define UCSI_ALTMODE_OFFSET(_r_) (((_r_) >> 32) & 0xff) #define UCSI_GET_ALTMODE_OFFSET(_r_) ((u64)(_r_) << 32) #define UCSI_GET_ALTMODE_NUM_ALTMODES(_r_) ((u64)(_r_) << 40) diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c index 2658cda5da11..b421b0045448 100644 --- a/drivers/usb/typec/ucsi/ucsi_ccg.c +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c @@ -125,6 +125,10 @@ struct version_format { #define CCG_FW_BUILD_NVIDIA (('n' << 8) | 'v') #define CCG_OLD_FW_VERSION (CCG_VERSION(0x31) | CCG_VERSION_PATCH(10)) +/* Altmode offset for NVIDIA Function Test Board (FTB) */ +#define USB_TYPEC_NVIDIA_FTB_DP_OFFSET (2) +#define USB_TYPEC_NVIDIA_FTB_DBG_OFFSET (3) + struct version_info { struct version_format base; struct version_format app; @@ -489,6 +493,29 @@ static int ucsi_ccg_read(struct ucsi *ucsi, unsigned int offset, return ret; if (offset == UCSI_MESSAGE_IN) { + if (UCSI_COMMAND(uc->last_cmd_sent) == + UCSI_GET_ALTERNATE_MODES && + UCSI_ALTMODE_RECIPIENT(uc->last_cmd_sent) == + UCSI_RECIPIENT_SOP) { + struct ucsi_altmode *alt = (struct ucsi_altmode *)val; + + if (alt[0].svid == USB_TYPEC_NVIDIA_VLINK_SID && + UCSI_ALTMODE_OFFSET(uc->last_cmd_sent) == + USB_TYPEC_NVIDIA_FTB_DP_OFFSET && + alt[0].mid == USB_TYPEC_NVIDIA_VLINK_DBG_VDO) { + alt[0].mid = USB_TYPEC_NVIDIA_VLINK_DP_VDO | + DP_CAP_DP_SIGNALING | + DP_CAP_USB | + DP_CONF_SET_PIN_ASSIGN(BIT + (DP_PIN_ASSIGN_E)); + } + if (alt[0].svid == USB_TYPEC_NVIDIA_VLINK_SID && + UCSI_ALTMODE_OFFSET(uc->last_cmd_sent) == + USB_TYPEC_NVIDIA_FTB_DBG_OFFSET && + alt[0].mid == USB_TYPEC_NVIDIA_VLINK_DP_VDO) { + alt[0].mid = USB_TYPEC_NVIDIA_VLINK_DBG_VDO; + } + } if (UCSI_COMMAND(uc->last_cmd_sent) == UCSI_GET_CURRENT_CAM && uc->has_multiple_dp) { ucsi_ccg_update_get_current_cam_cmd(uc, (u8 *)val);