[RFC PATCH 2/7] drm/msm/dp: support attaching bridges to the DP encoder

[Dmitry Baryshkov]

On 07/01/2022 06:42, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2022-01-06 18:01:27)
>> Currently DP driver will allocate panel bridge for eDP panels.
>> Simplify this code to just check if there is any next bridge in the
>> chain (be it a panel bridge or regular bridge). Rename panel_bridge
>> field to next_bridge accordingly.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> ---
>>   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
>>   drivers/gpu/drm/msm/dp/dp_display.h |  2 +-
>>   drivers/gpu/drm/msm/dp/dp_drm.c     |  4 ++--
>>   drivers/gpu/drm/msm/dp/dp_parser.c  | 26 ++++++++------------------
>>   drivers/gpu/drm/msm/dp/dp_parser.h  |  2 +-
>>   5 files changed, 13 insertions(+), 23 deletions(-)
> 
> I like this one, it certainly makes it easier to understand.
> 
>> diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c b/drivers/gpu/drm/msm/dp/dp_parser.c
>> index a7acc23f742b..5de21f3d0812 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_parser.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_parser.c
>> @@ -307,11 +299,9 @@ static int dp_parser_parse(struct dp_parser *parser, int connector_type)
>>          if (rc)
>>                  return rc;
>>
>> -       if (connector_type == DRM_MODE_CONNECTOR_eDP) {
> 
> It feels like this is on purpose, but I don't see any comment so I have
> no idea. I think qcom folks are concerned about changing how not eDP
> works. I'll have to test it out locally.

Ah, another thing that should go into the commit message.

Current situation:
- DP: no external bridges supported.
- eDP: only a drm_panel wrapped into the panel bridge

After this patch:
- both DP and eDP support any chain of bridges attached.


While the change means nothing for the DP (IIUC, it will not have any 
bridges), it simplifies the code path, lowering the amount of checks.

And for eDP this means that we can attach any eDP-to-something bridges 
(e.g. NXP PTN3460).


Well... After re-checking the source code for 
devm_drm_of_get_bridge/drm_of_find_panel_or_bridge I should probably 
revert removal of the check. The function will return -ENODEV if neither 
bridge nor panel are specified.

> 
>> -               rc = dp_parser_find_panel(parser);
>> -               if (rc)
>> -                       return rc;
>> -       }
>> +       rc = dp_parser_find_next_bridge(parser);
>> +       if (rc)
>> +               return rc;
>>
>>          /* Map the corresponding regulator information according to
>>           * version. Currently, since we only have one supported platform,


-- 
With best wishes
Dmitry