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

[Xiangxu Yin <quic_xiangxuy@xxxxxxxxxxx>]

On 11/29/2024 9:50 PM, Dmitry Baryshkov wrote:
> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin <quic_xiangxuy@xxxxxxxxxxx> 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_xiangxuy@xxxxxxxxxxx>
>> ---
>>  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 <drm/drm_of.h>
>>  #include <drm/drm_print.h>
>>
>> -#define DP_MAX_NUM_DP_LANES    4
>>  #define DP_LINK_RATE_HBR2      540000 /* 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 = 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.