| Message ID | 1555085131-8716-1-git-send-email-sunpeng.li@amd.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add AUX device entries for DP MST devices | expand |
On Fri, Apr 12, 2019 at 12:05:29PM -0400, sunpeng.li@amd.com wrote: > From: Leo Li <sunpeng.li@amd.com> > > Hi all, > > This is a follup to this change made by Ville to add MST aux nodes: > https://github.com/vsyrjala/linux/commit/cac63f799ee2f5fbbe4f0a375383f13e03d640a5 > Patch 2/2 describes what I added on top. Cool. I take you got it to actually work? IIRC I never managed that :P > > Sending as an RFC since there are some items I'm not certain on: > > 1) Only expose aux devices for physical ports. FWICT, only DPTX and DPRX > can handle AUX transactions, leaving logical ports out. Hmm. My MST-foo is admittedly weak so I'm not sure. A quick trawl through the spec didn't provide any solid explanations either :( However eg. "Figure 2-83: Example Multi-function MST Branch-Sink Device Enumeration" in the DP 1.4 spec does appear to show kind of virtual DPCD thing behind a logical port. But I'm not really sure what than means. > 2) Naming of exposed AUX devices. I'm not sure if the scheme implemented > here is the best approach. I'm not sure magic naming schemes are the best. I believe we do have the sysfs hierarchy which allows you to find the right aux dev for the connector, but I do admit that can be a bit cumbersome to use. Also I'm not sure if all the things we might want to talk to are even represented by a connector, so maybe we do need something else. > > Let me know your thoughts, > Leo > > Leo Li (1): > drm/dp_mst: Use non-cyclic idr, add suffix option for aux device > > Ville Syrjälä (1): > drm/dp_mst: Register aux-dev nodes for MST ports > > drivers/gpu/drm/drm_crtc_helper_internal.h | 5 +- > drivers/gpu/drm/drm_dp_aux_dev.c | 21 ++++-- > drivers/gpu/drm/drm_dp_helper.c | 2 +- > drivers/gpu/drm/drm_dp_mst_topology.c | 109 +++++++++++++++++++++++++---- > include/drm/drm_dp_helper.h | 4 ++ > include/drm/drm_dp_mst_helper.h | 6 ++ > 6 files changed, 125 insertions(+), 22 deletions(-) > > -- > 2.7.4
On 2019-04-12 1:30 p.m., Ville Syrjälä wrote: > On Fri, Apr 12, 2019 at 12:05:29PM -0400, sunpeng.li@amd.com wrote: >> From: Leo Li <sunpeng.li@amd.com> >> >> Hi all, >> >> This is a follup to this change made by Ville to add MST aux nodes: >> https://github.com/vsyrjala/linux/commit/cac63f799ee2f5fbbe4f0a375383f13e03d640a5 >> Patch 2/2 describes what I added on top. > > Cool. I take you got it to actually work? IIRC I never managed that :P Yeah, I got it to work on my system :) The diagram in Patch2/2 is what I was testing with. Both reading and writing worked with the help of Lyude's auxrw.py script: https://gitlab.freedesktop.org/lyudess/auxrw/blob/master/auxrw.py > >> >> Sending as an RFC since there are some items I'm not certain on: >> >> 1) Only expose aux devices for physical ports. FWICT, only DPTX and DPRX >> can handle AUX transactions, leaving logical ports out. > > Hmm. My MST-foo is admittedly weak so I'm not sure. A quick trawl through > the spec didn't provide any solid explanations either :( However eg. > "Figure 2-83: Example Multi-function MST Branch-Sink Device Enumeration" > in the DP 1.4 spec does appear to show kind of virtual DPCD thing behind > a logical port. But I'm not really sure what than means. Good point, I'm gonna dig more into that. It sounds like we should be able to access that with the relative addressing as defined by the spec (2.11.5). I'll have to see why that's currently getting nacked though. >> 2) Naming of exposed AUX devices. I'm not sure if the scheme implemented >> here is the best approach. > > I'm not sure magic naming schemes are the best. I believe we do have > the sysfs hierarchy which allows you to find the right aux dev for > the connector, but I do admit that can be a bit cumbersome to use. > Also I'm not sure if all the things we might want to talk to are > even represented by a connector, so maybe we do need something else. > The spec does define two methods of identifying devices (GUID or RAD) in section 2.5.3. I think the method we choose should make use of those. FWIW the scheme used here is based upon the RAD as generated by build_mst_prop_path(). Thanks, Leo >> >> Let me know your thoughts, >> Leo >> >> Leo Li (1): >> drm/dp_mst: Use non-cyclic idr, add suffix option for aux device >> >> Ville Syrjälä (1): >> drm/dp_mst: Register aux-dev nodes for MST ports >> >> drivers/gpu/drm/drm_crtc_helper_internal.h | 5 +- >> drivers/gpu/drm/drm_dp_aux_dev.c | 21 ++++-- >> drivers/gpu/drm/drm_dp_helper.c | 2 +- >> drivers/gpu/drm/drm_dp_mst_topology.c | 109 +++++++++++++++++++++++++---- >> include/drm/drm_dp_helper.h | 4 ++ >> include/drm/drm_dp_mst_helper.h | 6 ++ >> 6 files changed, 125 insertions(+), 22 deletions(-) >> >> -- >> 2.7.4 >
>> Hmm. My MST-foo is admittedly weak so I'm not sure. A quick trawl through >> the spec didn't provide any solid explanations either :( However eg. >> "Figure 2-83: Example Multi-function MST Branch-Sink Device Enumeration" >> in the DP 1.4 spec does appear to show kind of virtual DPCD thing behind >> a logical port. But I'm not really sure what than means. > > Good point, I'm gonna dig more into that. It sounds like we should be > able to access that with the relative addressing as defined by the spec > (2.11.5). I'll have to see why that's currently getting nacked though. > It looks like DPCD reads on logical ports work on some devices, and not others... I swapped my MST display out with a different one, and it read just fine. More specifically - in a daisy chain setup with 2 MST displays - with the one that works at the end: # auxrw.py read /dev/drm_dp_aux4_mst\:0-1-8 0x30-0x3f -> ACK # auxrw.py read /dev/drm_dp_aux6_mst\:0-1 0x30-0x3f -> ACK (The GUIDs returned are identical) With the one that doesn't at the end: # auxrw.py read /dev/drm_dp_aux4_mst\:0-1-8 0x30-0x3f -> *NAK* # auxrw.py read /dev/drm_dp_aux6_mst\:0-1 0x30-0x3f -> ACK So it seems there's some device dependent behavior here. I'm not sure if there's a better way of handling this besides exposing all the downstream ports: If it works, great. If not, just don't use it? Leo
On Tue, Apr 16, 2019 at 03:28:24PM +0000, Li, Sun peng (Leo) wrote: > >> Hmm. My MST-foo is admittedly weak so I'm not sure. A quick trawl through > >> the spec didn't provide any solid explanations either :( However eg. > >> "Figure 2-83: Example Multi-function MST Branch-Sink Device Enumeration" > >> in the DP 1.4 spec does appear to show kind of virtual DPCD thing behind > >> a logical port. But I'm not really sure what than means. > > > > Good point, I'm gonna dig more into that. It sounds like we should be > > able to access that with the relative addressing as defined by the spec > > (2.11.5). I'll have to see why that's currently getting nacked though. > > > > It looks like DPCD reads on logical ports work on some devices, and not > others... I swapped my MST display out with a different one, and it read > just fine. > > More specifically - in a daisy chain setup with 2 MST displays - with > the one that works at the end: > > # auxrw.py read /dev/drm_dp_aux4_mst\:0-1-8 0x30-0x3f -> ACK > # auxrw.py read /dev/drm_dp_aux6_mst\:0-1 0x30-0x3f -> ACK > (The GUIDs returned are identical) > > With the one that doesn't at the end: > > # auxrw.py read /dev/drm_dp_aux4_mst\:0-1-8 0x30-0x3f -> *NAK* > # auxrw.py read /dev/drm_dp_aux6_mst\:0-1 0x30-0x3f -> ACK > > So it seems there's some device dependent behavior here. I'm not sure if > there's a better way of handling this besides exposing all the > downstream ports: If it works, great. If not, just don't use it? Yeah, I think that's fine. It's really meant for debugging anyway so doesn't really matter if we expose something that's not guaranteed to work.
From: Leo Li <sunpeng.li@amd.com> Hi all, This is a follup to this change made by Ville to add MST aux nodes: https://github.com/vsyrjala/linux/commit/cac63f799ee2f5fbbe4f0a375383f13e03d640a5 Patch 2/2 describes what I added on top. Sending as an RFC since there are some items I'm not certain on: 1) Only expose aux devices for physical ports. FWICT, only DPTX and DPRX can handle AUX transactions, leaving logical ports out. 2) Naming of exposed AUX devices. I'm not sure if the scheme implemented here is the best approach. Let me know your thoughts, Leo Leo Li (1): drm/dp_mst: Use non-cyclic idr, add suffix option for aux device Ville Syrjälä (1): drm/dp_mst: Register aux-dev nodes for MST ports drivers/gpu/drm/drm_crtc_helper_internal.h | 5 +- drivers/gpu/drm/drm_dp_aux_dev.c | 21 ++++-- drivers/gpu/drm/drm_dp_helper.c | 2 +- drivers/gpu/drm/drm_dp_mst_topology.c | 109 +++++++++++++++++++++++++---- include/drm/drm_dp_helper.h | 4 ++ include/drm/drm_dp_mst_helper.h | 6 ++ 6 files changed, 125 insertions(+), 22 deletions(-)