| Message ID | 20240925092505.8.Ic14738918e3d026fa2d85e95fb68f8e07a0828d0@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Thunderbolt and DP altmode support for cros-ec-typec | expand |
On Wed, Sep 25, 2024 at 09:25:09AM GMT, Abhishek Pandit-Subedi wrote: > Altmodes with cros_ec are either automatically entered by the EC or > entered by typecd in userspace so we should not auto enter from the > kernel. This makes policy decision for the whole platform. Consider somebody running normal Linux distro on chromebooks. Can this be configured by the userspace itself? > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > --- > > drivers/platform/chrome/cros_ec_typec.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c > index ec13d84d11b8..e06a0f2712ce 100644 > --- a/drivers/platform/chrome/cros_ec_typec.c > +++ b/drivers/platform/chrome/cros_ec_typec.c > @@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, > desc.svid = USB_TYPEC_DP_SID; > desc.mode = USB_TYPEC_DP_MODE; > desc.vdo = DP_PORT_VDO; > + desc.no_auto_enter = true; > amode = cros_typec_register_displayport(port, &desc, > typec->ap_driven_altmode); > if (IS_ERR(amode)) > @@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, > memset(&desc, 0, sizeof(desc)); > desc.svid = USB_TYPEC_TBT_SID; > desc.mode = TYPEC_ANY_MODE; > + desc.no_auto_enter = true; > amode = cros_typec_register_thunderbolt(port, &desc); > if (IS_ERR(amode)) > return PTR_ERR(amode); > -- > 2.46.0.792.g87dc391469-goog >
On Wed, Sep 25, 2024 at 10:03 AM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Wed, Sep 25, 2024 at 09:25:09AM GMT, Abhishek Pandit-Subedi wrote: > > Altmodes with cros_ec are either automatically entered by the EC or > > entered by typecd in userspace so we should not auto enter from the > > kernel. > > This makes policy decision for the whole platform. Consider somebody > running normal Linux distro on chromebooks. Can this be configured by > the userspace itself? This is just the default for when the driver registers. You will then find that there is now a sysfs entry for auto_enter that you can control from userspace to allow you to auto-enter the mode on partner attach (which you could probably write a udev rule for). I still need to send a follow-up for how to handle auto-enter when there are multiple modes available on the partner that are all enabled. This first patch series is mostly about supporting existing use-case for ChromeOS. > > > > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > > --- > > > > drivers/platform/chrome/cros_ec_typec.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c > > index ec13d84d11b8..e06a0f2712ce 100644 > > --- a/drivers/platform/chrome/cros_ec_typec.c > > +++ b/drivers/platform/chrome/cros_ec_typec.c > > @@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, > > desc.svid = USB_TYPEC_DP_SID; > > desc.mode = USB_TYPEC_DP_MODE; > > desc.vdo = DP_PORT_VDO; > > + desc.no_auto_enter = true; > > amode = cros_typec_register_displayport(port, &desc, > > typec->ap_driven_altmode); > > if (IS_ERR(amode)) > > @@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, > > memset(&desc, 0, sizeof(desc)); > > desc.svid = USB_TYPEC_TBT_SID; > > desc.mode = TYPEC_ANY_MODE; > > + desc.no_auto_enter = true; > > amode = cros_typec_register_thunderbolt(port, &desc); > > if (IS_ERR(amode)) > > return PTR_ERR(amode); > > -- > > 2.46.0.792.g87dc391469-goog > > > > -- > With best wishes > Dmitry
On Wed, Sep 25, 2024 at 10:39:00AM GMT, Abhishek Pandit-Subedi wrote: > On Wed, Sep 25, 2024 at 10:03 AM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > On Wed, Sep 25, 2024 at 09:25:09AM GMT, Abhishek Pandit-Subedi wrote: > > > Altmodes with cros_ec are either automatically entered by the EC or > > > entered by typecd in userspace so we should not auto enter from the > > > kernel. > > > > This makes policy decision for the whole platform. Consider somebody > > running normal Linux distro on chromebooks. Can this be configured by > > the userspace itself? > > This is just the default for when the driver registers. You will then > find that there is now a sysfs entry for auto_enter that you can > control from userspace to allow you to auto-enter the mode on partner > attach (which you could probably write a udev rule for). I think, a usual policy is to handle everything automatically, unless userspace configures it in a different way. Otherwise it might be really surprising to users, if the kernel expects an action from the non-existing userspace agent. > I still need to send a follow-up for how to handle auto-enter when > there are multiple modes available on the partner that are all > enabled. This first patch series is mostly about supporting existing > use-case for ChromeOS. Doesn't EC decide that, which AltMode to select? > > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > > > --- > > > > > > drivers/platform/chrome/cros_ec_typec.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c > > > index ec13d84d11b8..e06a0f2712ce 100644 > > > --- a/drivers/platform/chrome/cros_ec_typec.c > > > +++ b/drivers/platform/chrome/cros_ec_typec.c > > > @@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, > > > desc.svid = USB_TYPEC_DP_SID; > > > desc.mode = USB_TYPEC_DP_MODE; > > > desc.vdo = DP_PORT_VDO; > > > + desc.no_auto_enter = true; > > > amode = cros_typec_register_displayport(port, &desc, > > > typec->ap_driven_altmode); > > > if (IS_ERR(amode)) > > > @@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, > > > memset(&desc, 0, sizeof(desc)); > > > desc.svid = USB_TYPEC_TBT_SID; > > > desc.mode = TYPEC_ANY_MODE; > > > + desc.no_auto_enter = true; > > > amode = cros_typec_register_thunderbolt(port, &desc); > > > if (IS_ERR(amode)) > > > return PTR_ERR(amode); > > > -- > > > 2.46.0.792.g87dc391469-goog > > > > > > > -- > > With best wishes > > Dmitry
On Wed, Sep 25, 2024 at 2:24 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Wed, Sep 25, 2024 at 10:39:00AM GMT, Abhishek Pandit-Subedi wrote: > > On Wed, Sep 25, 2024 at 10:03 AM Dmitry Baryshkov > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > On Wed, Sep 25, 2024 at 09:25:09AM GMT, Abhishek Pandit-Subedi wrote: > > > > Altmodes with cros_ec are either automatically entered by the EC or > > > > entered by typecd in userspace so we should not auto enter from the > > > > kernel. > > > > > > This makes policy decision for the whole platform. Consider somebody > > > running normal Linux distro on chromebooks. Can this be configured by > > > the userspace itself? > > > > This is just the default for when the driver registers. You will then > > find that there is now a sysfs entry for auto_enter that you can > > control from userspace to allow you to auto-enter the mode on partner > > attach (which you could probably write a udev rule for). > > I think, a usual policy is to handle everything automatically, unless > userspace configures it in a different way. Otherwise it might be really > surprising to users, if the kernel expects an action from the > non-existing userspace agent. > > > I still need to send a follow-up for how to handle auto-enter when > > there are multiple modes available on the partner that are all > > enabled. This first patch series is mostly about supporting existing > > use-case for ChromeOS. > > Doesn't EC decide that, which AltMode to select? On ChromeOS systems with only DisplayPort supported, ChromeOS EC automatically enters DP altmode. However, systems with Thunderbolt + USB4 have the AP_DRIVEN_MODE feature enabled which expects the AP to enter/exit modes (due to the security implication of TBT PCIE tunnels). ChromeOS has a userspace daemon, typecd (https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/typecd), which handles the mode entry policy for ChromeOS. Currently, it talks to the EC directly (via ectool) and this patch series is moving this support to the kernel so we can use sysfs instead. Currently, if you put a normal Linux distro on a ChromeOS device with AP_DRIVEN_MODE enabled from the EC, it will not automatically enter any alternate modes and there's no mechanism to do so via sysfs either. You will need to either port typecd or compile + deploy ectool to make the mode requests directly. > > > > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > > > > --- > > > > > > > > drivers/platform/chrome/cros_ec_typec.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c > > > > index ec13d84d11b8..e06a0f2712ce 100644 > > > > --- a/drivers/platform/chrome/cros_ec_typec.c > > > > +++ b/drivers/platform/chrome/cros_ec_typec.c > > > > @@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, > > > > desc.svid = USB_TYPEC_DP_SID; > > > > desc.mode = USB_TYPEC_DP_MODE; > > > > desc.vdo = DP_PORT_VDO; > > > > + desc.no_auto_enter = true; > > > > amode = cros_typec_register_displayport(port, &desc, > > > > typec->ap_driven_altmode); > > > > if (IS_ERR(amode)) > > > > @@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, > > > > memset(&desc, 0, sizeof(desc)); > > > > desc.svid = USB_TYPEC_TBT_SID; > > > > desc.mode = TYPEC_ANY_MODE; > > > > + desc.no_auto_enter = true; > > > > amode = cros_typec_register_thunderbolt(port, &desc); > > > > if (IS_ERR(amode)) > > > > return PTR_ERR(amode); > > > > -- > > > > 2.46.0.792.g87dc391469-goog > > > > > > > > > > -- > > > With best wishes > > > Dmitry > > -- > With best wishes > Dmitry
On Wed, 25 Sept 2024 at 23:35, Abhishek Pandit-Subedi <abhishekpandit@chromium.org> wrote: > > On Wed, Sep 25, 2024 at 2:24 PM Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: > > > > On Wed, Sep 25, 2024 at 10:39:00AM GMT, Abhishek Pandit-Subedi wrote: > > > On Wed, Sep 25, 2024 at 10:03 AM Dmitry Baryshkov > > > <dmitry.baryshkov@linaro.org> wrote: > > > > > > > > On Wed, Sep 25, 2024 at 09:25:09AM GMT, Abhishek Pandit-Subedi wrote: > > > > > Altmodes with cros_ec are either automatically entered by the EC or > > > > > entered by typecd in userspace so we should not auto enter from the > > > > > kernel. > > > > > > > > This makes policy decision for the whole platform. Consider somebody > > > > running normal Linux distro on chromebooks. Can this be configured by > > > > the userspace itself? > > > > > > This is just the default for when the driver registers. You will then > > > find that there is now a sysfs entry for auto_enter that you can > > > control from userspace to allow you to auto-enter the mode on partner > > > attach (which you could probably write a udev rule for). > > > > I think, a usual policy is to handle everything automatically, unless > > userspace configures it in a different way. Otherwise it might be really > > surprising to users, if the kernel expects an action from the > > non-existing userspace agent. > > > > > > > > I still need to send a follow-up for how to handle auto-enter when > > > there are multiple modes available on the partner that are all > > > enabled. This first patch series is mostly about supporting existing > > > use-case for ChromeOS. > > > > Doesn't EC decide that, which AltMode to select? > > On ChromeOS systems with only DisplayPort supported, ChromeOS EC > automatically enters DP altmode. > > However, systems with Thunderbolt + USB4 have the AP_DRIVEN_MODE > feature enabled which expects the AP to enter/exit modes (due to the > security implication of TBT PCIE tunnels). ChromeOS has a userspace > daemon, typecd (https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/typecd), > which handles the mode entry policy for ChromeOS. Currently, it talks > to the EC directly (via ectool) and this patch series is moving this > support to the kernel so we can use sysfs instead. > > Currently, if you put a normal Linux distro on a ChromeOS device with > AP_DRIVEN_MODE enabled from the EC, it will not automatically enter > any alternate modes and there's no mechanism to do so via sysfs > either. You will need to either port typecd or compile + deploy ectool > to make the mode requests directly. Well, I'd say that without a proper sysfs interface the feature is incomplete. At the very least it must be documented in the commit messages, so that anybody who tries to get this up and running can find necessary userspace bits (and then can implement necessary driver support). Also consider extending bolt, a usual ThunderBolt daemon used on Linux, to support new sysfs attributes. > > > > > > > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > > > > > --- > > > > > > > > > > drivers/platform/chrome/cros_ec_typec.c | 2 ++ > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c > > > > > index ec13d84d11b8..e06a0f2712ce 100644 > > > > > --- a/drivers/platform/chrome/cros_ec_typec.c > > > > > +++ b/drivers/platform/chrome/cros_ec_typec.c > > > > > @@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, > > > > > desc.svid = USB_TYPEC_DP_SID; > > > > > desc.mode = USB_TYPEC_DP_MODE; > > > > > desc.vdo = DP_PORT_VDO; > > > > > + desc.no_auto_enter = true; > > > > > amode = cros_typec_register_displayport(port, &desc, > > > > > typec->ap_driven_altmode); > > > > > if (IS_ERR(amode)) > > > > > @@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, > > > > > memset(&desc, 0, sizeof(desc)); > > > > > desc.svid = USB_TYPEC_TBT_SID; > > > > > desc.mode = TYPEC_ANY_MODE; > > > > > + desc.no_auto_enter = true; > > > > > amode = cros_typec_register_thunderbolt(port, &desc); > > > > > if (IS_ERR(amode)) > > > > > return PTR_ERR(amode); > > > > > -- > > > > > 2.46.0.792.g87dc391469-goog > > > > > > > > > > > > > -- > > > > With best wishes > > > > Dmitry > > > > -- > > With best wishes > > Dmitry
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c index ec13d84d11b8..e06a0f2712ce 100644 --- a/drivers/platform/chrome/cros_ec_typec.c +++ b/drivers/platform/chrome/cros_ec_typec.c @@ -294,6 +294,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, desc.svid = USB_TYPEC_DP_SID; desc.mode = USB_TYPEC_DP_MODE; desc.vdo = DP_PORT_VDO; + desc.no_auto_enter = true; amode = cros_typec_register_displayport(port, &desc, typec->ap_driven_altmode); if (IS_ERR(amode)) @@ -314,6 +315,7 @@ static int cros_typec_register_port_altmodes(struct cros_typec_data *typec, memset(&desc, 0, sizeof(desc)); desc.svid = USB_TYPEC_TBT_SID; desc.mode = TYPEC_ANY_MODE; + desc.no_auto_enter = true; amode = cros_typec_register_thunderbolt(port, &desc); if (IS_ERR(amode)) return PTR_ERR(amode);
Altmodes with cros_ec are either automatically entered by the EC or entered by typecd in userspace so we should not auto enter from the kernel. Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> --- drivers/platform/chrome/cros_ec_typec.c | 2 ++ 1 file changed, 2 insertions(+)