On Wed, Oct 11, 2023 at 01:52:29AM +0300, Dmitry Baryshkov wrote:
> As it was pointed out by Simon Ser, the DRM_MODE_CONNECTOR_USB connector
> is reserved for the GUD devices. Other drivers (i915, amdgpu) use
> DRM_MODE_CONNECTOR_DisplayPort even if the DP stream is handled by the
> USB-C altmode. While we are still working on implementing the proper way
> to let userspace know that the DP is wrapped into USB-C, change
> connector type to be DRM_MODE_CONNECTOR_DisplayPort.
>
> Fixes: 080b4e24852b ("soc: qcom: pmic_glink: Introduce altmode support")
> Cc: Simon Ser <[email protected]>
> Signed-off-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/soc/qcom/pmic_glink_altmode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
> index 9569d999391d..6f8b2f7ae3cc 100644
> --- a/drivers/soc/qcom/pmic_glink_altmode.c
> +++ b/drivers/soc/qcom/pmic_glink_altmode.c
> @@ -467,7 +467,7 @@ static int pmic_glink_altmode_probe(struct auxiliary_device *adev,
> alt_port->bridge.funcs = &pmic_glink_altmode_bridge_funcs;
> alt_port->bridge.of_node = to_of_node(fwnode);
> alt_port->bridge.ops = DRM_BRIDGE_OP_HPD;
> - alt_port->bridge.type = DRM_MODE_CONNECTOR_USB;
> + alt_port->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
>
> ret = devm_drm_bridge_add(dev, &alt_port->bridge);
> if (ret) {
I was just going to post a patch fixing this after finally investigating
why the DisplayPort outputs on the X13s were annoyingly identified as
"Unknown20-1" and "Unknown20-2" instead of the expected "DP-1" and
"DP-2".
A lore search just before posting led me to this fix from two weeks ago.
I think the commit message should have mentioned something about the how
this change affects user space. My patch also had a CC stable, but I
guess we can ping the stable team once it hits mainline:
commit e5f55bf5ad4effdd59d4d06c839a0ac553a73c7d (HEAD -> work)
Author: Johan Hovold <[email protected]>
Date: Wed Oct 25 11:54:09 2023 +0200
soc: qcom: pmic_glink_altmode: fix DP alt mode connector type
The PMIC glink altmode bridge connector type should be "DisplayPort"
rather than "USB", which is intended for custom USB display protocols
(e.g. see 40e1a70b4aed ("drm: Add GUD USB Display driver")).
This specifically makes the DisplayPort outputs on the Lenovo ThinkPad
X13s show up as "DP-1" and "DP-2" rather than "Unknown20-1" and
"Unknown20-2" with xrandr as expected (by users and tools):
Screen 0: minimum 320 x 200, current 1920 x 1200, maximum 5120 x 4096
eDP-1 connected primary 1920x1200+0+0 (normal left inverted right x axis y axis) 286mm x 178mm
1920x1200 60.03*+
1600x1200 60.00
DP-1 disconnected (normal left inverted right x axis y axis)
DP-2 connected (normal left inverted right x axis y axis)
1920x1200 59.95 +
...
Fixes: 080b4e24852b ("soc: qcom: pmic_glink: Introduce altmode support")
Cc: [email protected] # 6.3
Signed-off-by: Johan Hovold <[email protected]>
Johan
On Wednesday, October 25th, 2023 at 14:22, Johan Hovold <[email protected]> wrote:
> I was just going to post a patch fixing this after finally investigating
> why the DisplayPort outputs on the X13s were annoyingly identified as
> "Unknown20-1" and "Unknown20-2" instead of the expected "DP-1" and
> "DP-2".
Note, ideally userspace should use drmModeGetConnectorTypeName() from
libdrm to figure out the proper name for a connector type. That way we
only need to update a single spot when adding a new connector type,
instead of patching a whole bunch of programs.
On Wed, Oct 25, 2023 at 12:29:26PM +0000, Simon Ser wrote:
> On Wednesday, October 25th, 2023 at 14:22, Johan Hovold <[email protected]> wrote:
>
> > I was just going to post a patch fixing this after finally investigating
> > why the DisplayPort outputs on the X13s were annoyingly identified as
> > "Unknown20-1" and "Unknown20-2" instead of the expected "DP-1" and
> > "DP-2".
>
> Note, ideally userspace should use drmModeGetConnectorTypeName() from
> libdrm to figure out the proper name for a connector type. That way we
> only need to update a single spot when adding a new connector type,
> instead of patching a whole bunch of programs.
Yeah, I only skimmed the xrandr code and these strings appear to
originate from some X library. So hopefully not that many places to
update.
Scripts and other tools may still be using these strings directly
however (e.g. as did my custom script to enable external displays).
Johan
On 25/10/2023 15:29, Simon Ser wrote:
> On Wednesday, October 25th, 2023 at 14:22, Johan Hovold <[email protected]> wrote:
>
>> I was just going to post a patch fixing this after finally investigating
>> why the DisplayPort outputs on the X13s were annoyingly identified as
>> "Unknown20-1" and "Unknown20-2" instead of the expected "DP-1" and
>> "DP-2".
>
> Note, ideally userspace should use drmModeGetConnectorTypeName() from
> libdrm to figure out the proper name for a connector type. That way we
> only need to update a single spot when adding a new connector type,
> instead of patching a whole bunch of programs.
X11 does its own thing. It further "renames" DP MST connectors. So on my
laptop I end up with DP-1-1 in xrandr, but DP-3 in kernel.
--
With best wishes
Dmitry
On 25/10/2023 15:23, Johan Hovold wrote:
> On Wed, Oct 11, 2023 at 01:52:29AM +0300, Dmitry Baryshkov wrote:
>> As it was pointed out by Simon Ser, the DRM_MODE_CONNECTOR_USB connector
>> is reserved for the GUD devices. Other drivers (i915, amdgpu) use
>> DRM_MODE_CONNECTOR_DisplayPort even if the DP stream is handled by the
>> USB-C altmode. While we are still working on implementing the proper way
>> to let userspace know that the DP is wrapped into USB-C, change
>> connector type to be DRM_MODE_CONNECTOR_DisplayPort.
>>
>> Fixes: 080b4e24852b ("soc: qcom: pmic_glink: Introduce altmode support")
>> Cc: Simon Ser <[email protected]>
>> Signed-off-by: Dmitry Baryshkov <[email protected]>
>> ---
>> drivers/soc/qcom/pmic_glink_altmode.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
>> index 9569d999391d..6f8b2f7ae3cc 100644
>> --- a/drivers/soc/qcom/pmic_glink_altmode.c
>> +++ b/drivers/soc/qcom/pmic_glink_altmode.c
>> @@ -467,7 +467,7 @@ static int pmic_glink_altmode_probe(struct auxiliary_device *adev,
>> alt_port->bridge.funcs = &pmic_glink_altmode_bridge_funcs;
>> alt_port->bridge.of_node = to_of_node(fwnode);
>> alt_port->bridge.ops = DRM_BRIDGE_OP_HPD;
>> - alt_port->bridge.type = DRM_MODE_CONNECTOR_USB;
>> + alt_port->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
>>
>> ret = devm_drm_bridge_add(dev, &alt_port->bridge);
>> if (ret) {
>
> I was just going to post a patch fixing this after finally investigating
> why the DisplayPort outputs on the X13s were annoyingly identified as
> "Unknown20-1" and "Unknown20-2" instead of the expected "DP-1" and
> "DP-2".
Well, that depends on the userspace being updated to know about USB
connectors or not. But you are right, we should probably mention that in
the commit message.
>
> A lore search just before posting led me to this fix from two weeks ago.
>
> I think the commit message should have mentioned something about the how
> this change affects user space. My patch also had a CC stable, but I
> guess we can ping the stable team once it hits mainline:
>
> commit e5f55bf5ad4effdd59d4d06c839a0ac553a73c7d (HEAD -> work)
> Author: Johan Hovold <[email protected]>
> Date: Wed Oct 25 11:54:09 2023 +0200
>
> soc: qcom: pmic_glink_altmode: fix DP alt mode connector type
>
> The PMIC glink altmode bridge connector type should be "DisplayPort"
> rather than "USB", which is intended for custom USB display protocols
> (e.g. see 40e1a70b4aed ("drm: Add GUD USB Display driver")).
>
> This specifically makes the DisplayPort outputs on the Lenovo ThinkPad
> X13s show up as "DP-1" and "DP-2" rather than "Unknown20-1" and
> "Unknown20-2" with xrandr as expected (by users and tools):
>
> Screen 0: minimum 320 x 200, current 1920 x 1200, maximum 5120 x 4096
> eDP-1 connected primary 1920x1200+0+0 (normal left inverted right x axis y axis) 286mm x 178mm
> 1920x1200 60.03*+
> 1600x1200 60.00
> DP-1 disconnected (normal left inverted right x axis y axis)
> DP-2 connected (normal left inverted right x axis y axis)
> 1920x1200 59.95 +
> ...
>
> Fixes: 080b4e24852b ("soc: qcom: pmic_glink: Introduce altmode support")
> Cc: [email protected] # 6.3
> Signed-off-by: Johan Hovold <[email protected]>
>
> Johan
--
With best wishes
Dmitry
On Wednesday, October 25th, 2023 at 14:45, Dmitry Baryshkov <[email protected]> wrote:
> On 25/10/2023 15:29, Simon Ser wrote:
>
> > On Wednesday, October 25th, 2023 at 14:22, Johan Hovold [email protected] wrote:
> >
> > > I was just going to post a patch fixing this after finally investigating
> > > why the DisplayPort outputs on the X13s were annoyingly identified as
> > > "Unknown20-1" and "Unknown20-2" instead of the expected "DP-1" and
> > > "DP-2".
> >
> > Note, ideally userspace should use drmModeGetConnectorTypeName() from
> > libdrm to figure out the proper name for a connector type. That way we
> > only need to update a single spot when adding a new connector type,
> > instead of patching a whole bunch of programs.
>
> X11 does its own thing. It further "renames" DP MST connectors. So on my
> laptop I end up with DP-1-1 in xrandr, but DP-3 in kernel.
Right. But that function only takes in a connector type enum as input,
and returns a string for the type only. It doesn't include the suffix
with the number.
IOW: xserver could use drmModeGetConnectorTypeName() and then append
"-2" or "-1-1" or whatever.
But with the current appetite for xserver development this is probably
not going to happen (and doesn't matter very much).