Re: [PATCH 5/8] drm/msm/dp: Add support for lane mapping configuration

2025-05-20 Thread Xiangxu Yin



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  
> 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  
> 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 
>> ---
>>  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 m

Re: [PATCH 5/8] drm/msm/dp: Add support for lane mapping configuration

2025-05-19 Thread Dmitry Baryshkov
On Mon, 19 May 2025 at 11:20, Xiangxu Yin  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  
> > 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 
> >  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 
> >> ---
> >>  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 P

Re: [PATCH 5/8] drm/msm/dp: Add support for lane mapping configuration

2025-03-05 Thread Dmitry Baryshkov
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  
> >>> 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  
> >>> 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 
>  ---
>   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 shou

Re: [PATCH 5/8] drm/msm/dp: Add support for lane mapping configuration

2025-03-05 Thread Xiangxu Yin



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  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  
>>> 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 
 ---
  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 suppor

Re: [PATCH 5/8] drm/msm/dp: Add support for lane mapping configuration

2024-12-19 Thread Dmitry Baryshkov
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  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  
> > 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 
> >> ---
> >>  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 

Re: [PATCH 5/8] drm/msm/dp: Add support for lane mapping configuration

2024-12-19 Thread Xiangxu Yin



On 12/5/2024 7:40 PM, Dmitry Baryshkov wrote:
> On Thu, 5 Dec 2024 at 13:28, Xiangxu Yin  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  
> 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 
>> ---
>>  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.

The REG_DP_LOGICAL2PHYSICAL_LANE_MAPPING mapping determines how logical lanes 
(0, 1, 2, 3) map to physical lanes sent to the

Re: [PATCH 5/8] drm/msm/dp: Add support for lane mapping configuration

2024-12-05 Thread Dmitry Baryshkov
On Thu, 5 Dec 2024 at 13:28, Xiangxu Yin  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  
> >>> 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 
>  ---
>   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?

>
> 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 bette

Re: [PATCH 5/8] drm/msm/dp: Add support for lane mapping configuration

2024-12-05 Thread Xiangxu Yin



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

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.
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_LANES4
 +
  struct edid;

  struct msm_dp_display_mode {
 @@ -46,6 

Re: [PATCH 5/8] drm/msm/dp: Add support for lane mapping configuration

2024-12-02 Thread Dmitry Baryshkov
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  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 
> >> ---
> >>  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?

> >> +
> >> +   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_LANES4
> >> +
> >>  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
> [email protected]
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
With best wishes
Dmitry


Re: [PATCH 5/8] drm/msm/dp: Add support for lane mapping configuration

2024-12-02 Thread Xiangxu Yin



On 11/29/2024 9:50 PM, Dmitry Baryshkov wrote:
> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin  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 
>> ---
>>  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(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c 
>> b/drivers/gpu/drm/msm/dp/dp_catalog.c
>> index 
>> b4c8856fb25d01dd1b30c5ec33ce821aafa9551d..34439d0709d2e1437e5669fd0b995936420ee16f
>>  100644
>> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
>> @@ -361,17 +361,16 @@ void msm_dp_catalog_ctrl_config_ctrl(struct 
>> msm_dp_catalog *msm_dp_catalog, u32
>> msm_dp_write_link(catalog, REG_DP_CONFIGURATION_CTRL, cfg);
>>  }
>>
>> -void msm_dp_catalog_ctrl_lane_mapping(struct msm_dp_catalog *msm_dp_catalog)
>> +void msm_dp_catalog_ctrl_lane_mapping(struct msm_dp_catalog 
>> *msm_dp_catalog, u32 *l_map)
> 
> lane_map, not l_map.
> 
Ok, will update in next patch.
>>  {
>> struct msm_dp_catalog_private *catalog = container_of(msm_dp_catalog,
>> struct msm_dp_catalog_private, 
>> msm_dp_catalog);
>> -   u32 ln_0 = 0, ln_1 = 1, ln_2 = 2, ln_3 = 3; /* One-to-One mapping */
>> u32 ln_mapping;
>>
>> -   ln_mapping = ln_0 << LANE0_MAPPING_SHIFT;
>> -   ln_mapping |= ln_1 << LANE1_MAPPING_SHIFT;
>> -   ln_mapping |= ln_2 << LANE2_MAPPING_SHIFT;
>> -   ln_mapping |= ln_3 << LANE3_MAPPING_SHIFT;
>> +   ln_mapping = l_map[0] << LANE0_MAPPING_SHIFT;
>> +   ln_mapping |= l_map[1] << LANE1_MAPPING_SHIFT;
>> +   ln_mapping |= l_map[2] << LANE2_MAPPING_SHIFT;
>> +   ln_mapping |= l_map[3] << LANE3_MAPPING_SHIFT;
>>
>> msm_dp_write_link(catalog, REG_DP_LOGICAL2PHYSICAL_LANE_MAPPING,
>> ln_mapping);
>> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h 
>> b/drivers/gpu/drm/msm/dp/dp_catalog.h
>> index 
>> e932b17eecbf514070cd8cd0b98ca0fefbe81ab7..8b8de2a7d3ad561c1901e1bdaad92d4fab12e808
>>  100644
>> --- a/drivers/gpu/drm/msm/dp/dp_catalog.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
>> @@ -69,7 +69,7 @@ u32 msm_dp_catalog_aux_get_irq(struct msm_dp_catalog 
>> *msm_dp_catalog);
>>  /* DP Controller APIs */
>>  void msm_dp_catalog_ctrl_state_ctrl(struct msm_dp_catalog *msm_dp_catalog, 
>> u32 state);
>>  void msm_dp_catalog_ctrl_config_ctrl(struct msm_dp_catalog *msm_dp_catalog, 
>> u32 config);
>> -void msm_dp_catalog_ctrl_lane_mapping(struct msm_dp_catalog 
>> *msm_dp_catalog);
>> +void msm_dp_catalog_ctrl_lane_mapping(struct msm_dp_catalog 
>> *msm_dp_catalog, u32 *l_map);
>>  void msm_dp_catalog_ctrl_mainlink_ctrl(struct msm_dp_catalog 
>> *msm_dp_catalog, bool enable);
>>  void msm_dp_catalog_ctrl_psr_mainlink_enable(struct msm_dp_catalog 
>> *msm_dp_catalog, bool enable);
>>  void msm_dp_catalog_setup_peripheral_flush(struct msm_dp_catalog 
>> *msm_dp_catalog);
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c 
>> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> index 
>> bc2ca8133b790fc049e18ab3b37a629558664dd4..49c8ce9b2d0e57a613e50865be3fe98e814d425a
>>  100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> @@ -177,7 +177,7 @@ static void msm_dp_ctrl_configure_source_params(struct 
>> msm_dp_ctrl_private *ctrl
>>  {
>> u32 cc, tb;
>>
>> -   msm_dp_catalog_ctrl_lane_mapping(ctrl->catalog);
>> +   msm_dp_catalog_ctrl_lane_mapping(ctrl->catalog, 
>> ctrl->panel->lane_map);
>> msm_dp_catalog_ctrl_mainlink_ctrl(ctrl->catalog, true);
>> msm_dp_catalog_setup_peripheral_flush(ctrl->catalog);
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c 
>> b/drivers/gpu/drm/msm/dp/dp_panel.c
>> index 
>> 5d7eaa31bf3176566f40f01ff636bee64e81c64f..8654180aa259234bbd41f4f88c13c485f9791b1d
>>  100644
>> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
>> @@ -11,7 +11,6 @@
>>  #include 
>>  #include 
>>
>> -#define DP_MAX_NUM_DP_LANES4
>>  #define DP_LINK_RATE_HBR2  54 /* kbytes */
>>
>>  struct msm_dp_panel_private {
>> @@ -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,

Re: [PATCH 5/8] drm/msm/dp: Add support for lane mapping configuration

2024-11-29 Thread Dmitry Baryshkov
On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin  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 
> ---
>  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(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c 
> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index 
> b4c8856fb25d01dd1b30c5ec33ce821aafa9551d..34439d0709d2e1437e5669fd0b995936420ee16f
>  100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -361,17 +361,16 @@ void msm_dp_catalog_ctrl_config_ctrl(struct 
> msm_dp_catalog *msm_dp_catalog, u32
> msm_dp_write_link(catalog, REG_DP_CONFIGURATION_CTRL, cfg);
>  }
>
> -void msm_dp_catalog_ctrl_lane_mapping(struct msm_dp_catalog *msm_dp_catalog)
> +void msm_dp_catalog_ctrl_lane_mapping(struct msm_dp_catalog *msm_dp_catalog, 
> u32 *l_map)

lane_map, not l_map.

>  {
> struct msm_dp_catalog_private *catalog = container_of(msm_dp_catalog,
> struct msm_dp_catalog_private, 
> msm_dp_catalog);
> -   u32 ln_0 = 0, ln_1 = 1, ln_2 = 2, ln_3 = 3; /* One-to-One mapping */
> u32 ln_mapping;
>
> -   ln_mapping = ln_0 << LANE0_MAPPING_SHIFT;
> -   ln_mapping |= ln_1 << LANE1_MAPPING_SHIFT;
> -   ln_mapping |= ln_2 << LANE2_MAPPING_SHIFT;
> -   ln_mapping |= ln_3 << LANE3_MAPPING_SHIFT;
> +   ln_mapping = l_map[0] << LANE0_MAPPING_SHIFT;
> +   ln_mapping |= l_map[1] << LANE1_MAPPING_SHIFT;
> +   ln_mapping |= l_map[2] << LANE2_MAPPING_SHIFT;
> +   ln_mapping |= l_map[3] << LANE3_MAPPING_SHIFT;
>
> msm_dp_write_link(catalog, REG_DP_LOGICAL2PHYSICAL_LANE_MAPPING,
> ln_mapping);
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h 
> b/drivers/gpu/drm/msm/dp/dp_catalog.h
> index 
> e932b17eecbf514070cd8cd0b98ca0fefbe81ab7..8b8de2a7d3ad561c1901e1bdaad92d4fab12e808
>  100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.h
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
> @@ -69,7 +69,7 @@ u32 msm_dp_catalog_aux_get_irq(struct msm_dp_catalog 
> *msm_dp_catalog);
>  /* DP Controller APIs */
>  void msm_dp_catalog_ctrl_state_ctrl(struct msm_dp_catalog *msm_dp_catalog, 
> u32 state);
>  void msm_dp_catalog_ctrl_config_ctrl(struct msm_dp_catalog *msm_dp_catalog, 
> u32 config);
> -void msm_dp_catalog_ctrl_lane_mapping(struct msm_dp_catalog *msm_dp_catalog);
> +void msm_dp_catalog_ctrl_lane_mapping(struct msm_dp_catalog *msm_dp_catalog, 
> u32 *l_map);
>  void msm_dp_catalog_ctrl_mainlink_ctrl(struct msm_dp_catalog 
> *msm_dp_catalog, bool enable);
>  void msm_dp_catalog_ctrl_psr_mainlink_enable(struct msm_dp_catalog 
> *msm_dp_catalog, bool enable);
>  void msm_dp_catalog_setup_peripheral_flush(struct msm_dp_catalog 
> *msm_dp_catalog);
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c 
> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 
> bc2ca8133b790fc049e18ab3b37a629558664dd4..49c8ce9b2d0e57a613e50865be3fe98e814d425a
>  100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -177,7 +177,7 @@ static void msm_dp_ctrl_configure_source_params(struct 
> msm_dp_ctrl_private *ctrl
>  {
> u32 cc, tb;
>
> -   msm_dp_catalog_ctrl_lane_mapping(ctrl->catalog);
> +   msm_dp_catalog_ctrl_lane_mapping(ctrl->catalog, 
> ctrl->panel->lane_map);
> msm_dp_catalog_ctrl_mainlink_ctrl(ctrl->catalog, true);
> msm_dp_catalog_setup_peripheral_flush(ctrl->catalog);
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c 
> b/drivers/gpu/drm/msm/dp/dp_panel.c
> index 
> 5d7eaa31bf3176566f40f01ff636bee64e81c64f..8654180aa259234bbd41f4f88c13c485f9791b1d
>  100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -11,7 +11,6 @@
>  #include 
>  #include 
>
> -#define DP_MAX_NUM_DP_LANES4
>  #define DP_LINK_RATE_HBR2  54 /* kbytes */
>
>  struct msm_dp_panel_private {
> @@ -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 =