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.
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.
>>>>>>
>>>>>>>>>> +
>>>>>>>>>> + 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
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
