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?
> >>
> >>>>
> >>>> 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.
> >>>>
> >>>>>>>> +
> >>>>>>>> + memcpy(msm_dp_panel->lane_map, lane_map,
> >>>>>>>> msm_dp_panel->max_dp_lanes * sizeof(u32));
> >>>>>>>>
> >>>>>>>> msm_dp_panel->max_dp_link_rate =
> >>>>>>>> msm_dp_panel_link_frequencies(of_node);
> >>>>>>>> if (!msm_dp_panel->max_dp_link_rate)
> >>>>>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.h
> >>>>>>>> b/drivers/gpu/drm/msm/dp/dp_panel.h
> >>>>>>>> index
> >>>>>>>> 0e944db3adf2f187f313664fe80cf540ec7a19f2..7603b92c32902bd3d4485539bd6308537ff75a2c
> >>>>>>>> 100644
> >>>>>>>> --- a/drivers/gpu/drm/msm/dp/dp_panel.h
> >>>>>>>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.h
> >>>>>>>> @@ -11,6 +11,8 @@
> >>>>>>>> #include "dp_aux.h"
> >>>>>>>> #include "dp_link.h"
> >>>>>>>>
> >>>>>>>> +#define DP_MAX_NUM_DP_LANES 4
> >>>>>>>> +
> >>>>>>>> struct edid;
> >>>>>>>>
> >>>>>>>> struct msm_dp_display_mode {
> >>>>>>>> @@ -46,6 +48,7 @@ struct msm_dp_panel {
> >>>>>>>> bool video_test;
> >>>>>>>> bool vsc_sdp_supported;
> >>>>>>>>
> >>>>>>>> + u32 lane_map[DP_MAX_NUM_DP_LANES];
> >>>>>>>> u32 max_dp_lanes;
> >>>>>>>> u32 max_dp_link_rate;
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> --
> >>>>>>>> 2.25.1
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> linux-phy mailing list
> >>>>>> linux-...@lists.infradead.org
> >>>>>> https://lists.infradead.org/mailman/listinfo/linux-phy
> >>>>>
> >>>>
> >>>
> >>>
> >>
> >
>
--
With best wishes
Dmitry
