On Mon, 19 May 2025 at 11:20, Xiangxu Yin <xiangxu....@oss.qualcomm.com> wrote:
>
>
>
> On 3/6/2025 5:14 AM, Dmitry Baryshkov wrote:
> > On Wed, Mar 05, 2025 at 06:16:45PM +0800, Xiangxu Yin wrote:
> >>
> >>
> >> On 12/20/2024 5:45 AM, Dmitry Baryshkov wrote:
> >>> On Thu, Dec 19, 2024 at 06:36:38PM +0800, Xiangxu Yin wrote:
> >>>>
> >>>>
> >>>> On 12/5/2024 7:40 PM, Dmitry Baryshkov wrote:
> >>>>> On Thu, 5 Dec 2024 at 13:28, Xiangxu Yin <quic_xiang...@quicinc.com>
> >>>>> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 12/2/2024 6:46 PM, Dmitry Baryshkov wrote:
> >>>>>>> On Mon, Dec 02, 2024 at 04:40:05PM +0800, Xiangxu Yin wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 11/29/2024 9:50 PM, Dmitry Baryshkov wrote:
> >>>>>>>>> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin
> >>>>>>>>> <quic_xiang...@quicinc.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> Add the ability to configure lane mapping for the DP controller.
> >>>>>>>>>> This is
> >>>>>>>>>> required when the platform's lane mapping does not follow the
> >>>>>>>>>> default
> >>>>>>>>>> order (0, 1, 2, 3). The mapping rules are now configurable via the
> >>>>>>>>>> `data-lane` property in the devicetree. This property defines the
> >>>>>>>>>> logical-to-physical lane mapping sequence, ensuring correct lane
> >>>>>>>>>> assignment for non-default configurations.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Xiangxu Yin <quic_xiang...@quicinc.com>
> >>>>>>>>>> ---
> >>>>>>>>>> drivers/gpu/drm/msm/dp/dp_catalog.c | 11 +++++------
> >>>>>>>>>> drivers/gpu/drm/msm/dp/dp_catalog.h | 2 +-
> >>>>>>>>>> drivers/gpu/drm/msm/dp/dp_ctrl.c | 2 +-
> >>>>>>>>>> drivers/gpu/drm/msm/dp/dp_panel.c | 13 ++++++++++---
> >>>>>>>>>> drivers/gpu/drm/msm/dp/dp_panel.h | 3 +++
> >>>>>>>>>> 5 files changed, 20 insertions(+), 11 deletions(-)
> >>>>>>>>>>
> >>>>>>>
> >>>>>>>>>> @@ -461,6 +460,7 @@ static int msm_dp_panel_parse_dt(struct
> >>>>>>>>>> msm_dp_panel *msm_dp_panel)
> >>>>>>>>>> struct msm_dp_panel_private *panel;
> >>>>>>>>>> struct device_node *of_node;
> >>>>>>>>>> int cnt;
> >>>>>>>>>> + u32 lane_map[DP_MAX_NUM_DP_LANES] = {0, 1, 2, 3};
> >>>>>>>>>>
> >>>>>>>>>> panel = container_of(msm_dp_panel, struct
> >>>>>>>>>> msm_dp_panel_private, msm_dp_panel);
> >>>>>>>>>> of_node = panel->dev->of_node;
> >>>>>>>>>> @@ -474,10 +474,17 @@ static int msm_dp_panel_parse_dt(struct
> >>>>>>>>>> msm_dp_panel *msm_dp_panel)
> >>>>>>>>>> cnt = drm_of_get_data_lanes_count(of_node, 1,
> >>>>>>>>>> DP_MAX_NUM_DP_LANES);
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>> - if (cnt > 0)
> >>>>>>>>>> + if (cnt > 0) {
> >>>>>>>>>> + struct device_node *endpoint;
> >>>>>>>>>> +
> >>>>>>>>>> msm_dp_panel->max_dp_lanes = cnt;
> >>>>>>>>>> - else
> >>>>>>>>>> + endpoint = of_graph_get_endpoint_by_regs(of_node,
> >>>>>>>>>> 1, -1);
> >>>>>>>>>> + of_property_read_u32_array(endpoint, "data-lanes",
> >>>>>>>>>> lane_map, cnt);
> >>>>>>>>>> + } else {
> >>>>>>>>>> msm_dp_panel->max_dp_lanes = DP_MAX_NUM_DP_LANES;
> >>>>>>>>>> /* 4 lanes */
> >>>>>>>>>> + }
> >>>>>>>>>
> >>>>>>>>> Why? This sounds more like dp_catalog or (after the refactoring at
> >>>>>>>>> [1]) dp_ctrl. But not the dp_panel.
> >>>>>>>>>
> >>>>>>>>> [1]
> >>>>>>>>> https://patchwork.freedesktop.org/project/freedreno/series/?ordering=-last_updated
> >>>>>>>>>
> >>>>>>>> We are used the same prop 'data-lanes = <3 2 0 1>' in mdss_dp_out to
> >>>>>>>> keep similar behaviour with dsi_host_parse_lane_data.
> >>>>>>>> From the modules used, catalog seems more appropriate, but since the
> >>>>>>>> max_dp_lanes is parsed at dp_panel, it has been placed here.
> >>>>>>>> Should lane_map parsing in msm_dp_catalog_get, and keep max_dp_lanes
> >>>>>>>> parsing at the dp_panel?
> >>>>>>>
> >>>>>>> msm_dp_catalog_get() is going to be removed. Since the functions that
> >>>>>>> are going to use it are in dp_ctrl module, I thought that dp_ctrl.c is
> >>>>>>> the best place. A better option might be to move max_dp_lanes and
> >>>>>>> max_dp_link_rate to dp_link.c as those are link params. Then
> >>>>>>> lane_mapping also logically becomes a part of dp_link module.
> >>>>>>>
> >>>>>>> But now I have a more important question (triggered by Krishna's email
> >>>>>>> about SAR2130P's USB): if the lanes are swapped, does USB 3 work on
> >>>>>>> that
> >>>>>>> platform? Or is it being demoted to USB 2 with nobody noticing that?
> >>>>>>>
> >>>>>>> If lanes 0/1 and 2/3 are swapped, shouldn't it be handled in the QMP
> >>>>>>> PHY, where we handle lanes and orientation switching?
> >>>>>>>
> >>>>>> I have checked the DP hardware programming guide and also discussed it
> >>>>>> with Krishna.
> >>>>>>
> >>>>>> According to the HPG section '3.4.2 PN and Lane Swap: PHY supports PN
> >>>>>> swap for mainlink and AUX, but it doesn't support lane swap feature.'
> >>>>>>
> >>>>>> The lane swap mainly refers to the logical to physical mapping between
> >>>>>> the DP controller and the DP PHY. The PHY handles polarity inversion,
> >>>>>> and the lane map does not affect USB behavior.
> >>>>>>
> >>>>>> On the QCS615 platform, we have also tested when DP works with lane
> >>>>>> swap, other USB 3.0 ports can works normally at super speed.
> >>>>>
> >>>>> "Other USB 3.0 ports"? What does that mean? Please correct me if I'm
> >>>>> wrong, you should have a USB+DP combo port that is being managed with
> >>>>> combo PHY. Does USB 3 work on that port?
> >>>>>
> >>>>> In other words, where the order of lanes is actually inverted? Between
> >>>>> DP and combo PHY? Within combo PHY? Between the PHY and the pinout?
> >>>>> Granted that SM6150 was supported in msm-4.14 could you possibly point
> >>>>> out a corresponding commit or a set of commits from that kernel?
> >>>>>
> >>>> For "Other USB 3.0 ports", as replied in USBC driver, USB3 primary phy
> >>>> works for other four USB type-A port.
> >>>
> >>> So if that's the USB3 primary, then why do you mention here at all? We
> >>> are taling about the secondary USB3 + DP.
> >>>
> >> OK, sorry for confusing you.
> >>>> The REG_DP_LOGICAL2PHYSICAL_LANE_MAPPING mapping determines how logical
> >>>> lanes (0, 1, 2, 3) map to physical lanes sent to the PHY.
> >>>> This ensures alignment with hardware requirements.
> >>>> The PHY’s polarity inversion only adjusts signal polarity and doesn’t
> >>>> affect lane mapping.
> >>>> Both DP ctrl and PHY lane related config will not affect USB phy.
> >>>
> >>> Probably we misundersand each other. The DP PHY should have orientation
> >>> switch register, which controls whether 2-lane DP uses lanes 0/1 or 2/3.
> >>> Can you use that register?
> >>>
> >> Yes, DP PHY have orientation register as below.
> >> DP_PHY_DP_PHY_CFG_1(0x88e9014) bit(7) SW_PORTSELECT
> >>> Also, could you _please_ answer the question that I have asked? Is the
> >>> order of lanes inverted between the DP controller and DP PHY? Or between
> >>> DP PHY and the DP connector? If one uses USB3 signals coming from this
> >>> port (yes, on the other board, not on the Ride), would they also need to
> >>> switch the order of USB3 lanes? If one uses a DP-over-USB-C, are DP
> >>> lanes are swapped?
> >>>
> >> It's inverted between the DP controller and DP PHY.
> >> If other use USB3 on the other board, will not need switch order of USB3
> >> lanes,
> >> If one use DP-over-USB-C, then need DP lanes swap.
> >
> > Thanks!
> >
> >>>> Without extra Type-C mapping, the DP controller’s mapping indirectly
> >>>> decides how signals are transmitted through Type-C.
> >>>> Mapping ensures proper data transmission and compatibility across
> >>>> interfaces.
> >>>>
> >>>> We only found sm6150 need this lane mapping config,
> >>>> For msm 4.14, please refer these links,
> >>>> https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/arch/arm64/boot/dts/qcom/sm6150-sde.dtsi
> >>>> (qcom,logical2physical-lane-map)
> >>>> https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/drivers/gpu/drm/msm/dp/dp_parser.c
> >>>> (dp_parser_misc)
> >>>> https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/drivers/gpu/drm/msm/dp/dp_catalog_v200.c
> >>>> (dp_catalog_ctrl_lane_mapping_v200)
> >>>>
> >>>> If need process orientation info like dp_catalog_ctrl_lane_mapping_v200,
> >>>> then
> >>>> if implement in DP phy, then we need config dp_link register in PHY,
> >>>> if implement in DP link, then we need pass orientation info to DP
> >>>> driver, perhaps we could add a new attribute to the
> >>>> phy_configure_opts_dp structure to pass this.
> >>>> Do you have any suggestions?
> >>>
> >>> Does SW_PORTSEL_VAL affect the DP lanes on this platform?
> >>>
> >> SW_PORTSEL_VAL for USB3PHY_PCS_MISC_TYPEC_CTRL will not affect DP lanes in
> >> this DP or USB3 chip series.
> >> USB3 will use USB3PHY_PCS_MISC_TYPEC_CTRL(SW_PORTSEL_VAL BIT_0) and DP
> >> will use DP_PHY_DP_PHY_CFG_1(SW_PORTSELECT BIT_7)
> >
> > Is it possible to set this bit from the PHY driver rather than remapping
> > the lanes in the DP driver?
> >
> I have verified and confirmed with chip verification team.
>
> We configured the logical2physical mapping primarily to correct the PHY
> output mapping.
> Currently, the logical2physical mapping defines the input-to-output mapping
> for the DP controller,
> while the SW_PORTSELECT in PHY determines the swapping between PHY input
> ports 0↔3 and 1↔2.
> When the DP controller input to PHY output mapping is correctly configured,
> PHY's SW_PORTSELECT can be used to implement flip operations.
> However, due to the improper mapping implementation on Talos platforms, using
> SW_PORTSELECT would require additional modifications to the logical2physical
> mapping.
>
> For example, other platform except Talos implementations the data-lanes
> mapping follows <0 1 2 3> sequence.
> A proper flip operation should produce <3 2 1 0>, which can be equivalently
> achieved either through DP driver configuration or PHY portselect.
> But in the Talos where the initial mapping is arranged as <3 2 0 1>, the
> expected post-flip sequence should be <0 1 3 2>.
> then when applying PHY SW_PORTSELECT setting 1, the PHY output becomes <1 0 2
> 3> which mismatches the expected pattern.
Ack. Thanks for the detailed explanation. Please add similar text to
the commit message, with the only change: s/Talos/SM6150/
>
> To maintain cross-platform compatibility between Talos and other platforms,
> recommend the flip handling at the DP driver level such like
> dp_catalog_ctrl_lane_mapping_v200 in sm6150.
> >>>>
> >>>>>>
> >>>>>> Additionally, if it were placed on the PHY side, the PHY would need
> >>>>>> access to dp_link’s domain which can access
> >>>>>> REG_DP_LOGICAL2PHYSICAL_LANE_MAPPING.
> >>>>>
> >>>>> I was thinking about inverting the SW_PORTSEL_VAL bit.
> >>>>>
> >>>>>> Therefore, we believe that the max_dp_link_rate,max_dp_lanes and
> >>>>>> lane_map move to dp_link side is better.
--
With best wishes
Dmitry
