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