Hi
On 4/21/22 09:13, Paul Kocialkowski wrote:
> Hi,
>
> On Wed 20 Apr 22, 16:19, Bjorn Andersson wrote:
>> On Wed 20 Apr 16:12 PDT 2022, Bjorn Andersson wrote:
>>
>> Sorry, I missed Jagan and Linus, author and reviewer of the reverted
>> patch 2, among the recipients.
> I'd be curious to have Jagan's feedback on why the change was needed in the
> first place and whether an accepted dt binding relies on it.
>
> We might be able to just keep the whole thing reverted (forever).
This patch was merged within the last mainline kernel and was breaking our STM32MP15 platforms.
Switching back to drm-based kernel (drm-next-fixes branch) made me realize this patch was faulty.
I'm glad it was reverted.
Thanks,
Raphaël
>
> Paul
>
>> Regards,
>> Bjorn
>>
>>> Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
>>> bridge")' introduced the ability to describe a panel under a display
>>> controller without having to use a graph to connect the controller to
>>> its single child panel (or bridge).
>>>
>>> The implementation of this would find the first non-graph node and
>>> attempt to acquire the related panel or bridge. This prevents cases
>>> where any other child node, such as a aux bus for a DisplayPort
>>> controller, or an opp-table to find the referenced panel.
>>>
>>> Commit '67bae5f28c89 ("drm: of: Properly try all possible cases for
>>> bridge/panel detection")' attempted to solve this problem by not
>>> bypassing the graph reference lookup before attempting to find the panel
>>> or bridge.
>>>
>>> While this does solve the case where a proper graph reference is
>>> present, it does not allow the caller to distinguish between a
>>> yet-to-be-probed panel or bridge and the absence of a reference to a
>>> panel.
>>>
>>> One such case is a DisplayPort controller that on some boards have an
>>> explicitly described reference to a panel, but on others have a
>>> discoverable DisplayPort display attached (which doesn't need to be
>>> expressed in DeviceTree).
>>>
>>> This reverts commit '67bae5f28c89 ("drm: of: Properly try all possible
>>> cases for bridge/panel detection")', as a step towards reverting commit
>>> '80253168dbfd ("drm: of: Lookup if child node has panel or bridge")'.
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
>>> ---
>>> drivers/gpu/drm/drm_of.c | 99 ++++++++++++++++++++--------------------
>>> 1 file changed, 49 insertions(+), 50 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
>>> index f4df344509a8..026e4e29a0f3 100644
>>> --- a/drivers/gpu/drm/drm_of.c
>>> +++ b/drivers/gpu/drm/drm_of.c
>>> @@ -214,29 +214,6 @@ int drm_of_encoder_active_endpoint(struct device_node *node,
>>> }
>>> EXPORT_SYMBOL_GPL(drm_of_encoder_active_endpoint);
>>>
>>> -static int find_panel_or_bridge(struct device_node *node,
>>> - struct drm_panel **panel,
>>> - struct drm_bridge **bridge)
>>> -{
>>> - if (panel) {
>>> - *panel = of_drm_find_panel(node);
>>> - if (!IS_ERR(*panel))
>>> - return 0;
>>> -
>>> - /* Clear the panel pointer in case of error. */
>>> - *panel = NULL;
>>> - }
>>> -
>>> - /* No panel found yet, check for a bridge next. */
>>> - if (bridge) {
>>> - *bridge = of_drm_find_bridge(node);
>>> - if (*bridge)
>>> - return 0;
>>> - }
>>> -
>>> - return -EPROBE_DEFER;
>>> -}
>>> -
>>> /**
>>> * drm_of_find_panel_or_bridge - return connected panel or bridge device
>>> * @np: device tree node containing encoder output ports
>>> @@ -259,44 +236,66 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
>>> struct drm_panel **panel,
>>> struct drm_bridge **bridge)
>>> {
>>> - struct device_node *node;
>>> - int ret;
>>> + int ret = -EPROBE_DEFER;
>>> + struct device_node *remote;
>>>
>>> if (!panel && !bridge)
>>> return -EINVAL;
>>> -
>>> if (panel)
>>> *panel = NULL;
>>> - if (bridge)
>>> - *bridge = NULL;
>>> -
>>> - /* Check for a graph on the device node first. */
>>> - if (of_graph_is_present(np)) {
>>> - node = of_graph_get_remote_node(np, port, endpoint);
>>> - if (node) {
>>> - ret = find_panel_or_bridge(node, panel, bridge);
>>> - of_node_put(node);
>>> -
>>> - if (!ret)
>>> - return 0;
>>> - }
>>> - }
>>>
>>> - /* Otherwise check for any child node other than port/ports. */
>>> - for_each_available_child_of_node(np, node) {
>>> - if (of_node_name_eq(node, "port") ||
>>> - of_node_name_eq(node, "ports"))
>>> + /**
>>> + * Devices can also be child nodes when we also control that device
>>> + * through the upstream device (ie, MIPI-DCS for a MIPI-DSI device).
>>> + *
>>> + * Lookup for a child node of the given parent that isn't either port
>>> + * or ports.
>>> + */
>>> + for_each_available_child_of_node(np, remote) {
>>> + if (of_node_name_eq(remote, "port") ||
>>> + of_node_name_eq(remote, "ports"))
>>> continue;
>>>
>>> - ret = find_panel_or_bridge(node, panel, bridge);
>>> - of_node_put(node);
>>> + goto of_find_panel_or_bridge;
>>> + }
>>> +
>>> + /*
>>> + * of_graph_get_remote_node() produces a noisy error message if port
>>> + * node isn't found and the absence of the port is a legit case here,
>>> + * so at first we silently check whether graph presents in the
>>> + * device-tree node.
>>> + */
>>> + if (!of_graph_is_present(np))
>>> + return -ENODEV;
>>> +
>>> + remote = of_graph_get_remote_node(np, port, endpoint);
>>> +
>>> +of_find_panel_or_bridge:
>>> + if (!remote)
>>> + return -ENODEV;
>>> +
>>> + if (panel) {
>>> + *panel = of_drm_find_panel(remote);
>>> + if (!IS_ERR(*panel))
>>> + ret = 0;
>>> + else
>>> + *panel = NULL;
>>> + }
>>> +
>>> + /* No panel found yet, check for a bridge next. */
>>> + if (bridge) {
>>> + if (ret) {
>>> + *bridge = of_drm_find_bridge(remote);
>>> + if (*bridge)
>>> + ret = 0;
>>> + } else {
>>> + *bridge = NULL;
>>> + }
>>>
>>> - /* Stop at the first found occurrence. */
>>> - if (!ret)
>>> - return 0;
>>> }
>>>
>>> - return -EPROBE_DEFER;
>>> + of_node_put(remote);
>>> + return ret;
>>> }
>>> EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge);
>>>
>>> --
>>> 2.35.1
>>>
