Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Fri, Apr 29, 2022 at 07:05:59PM +0300, Laurent Pinchart wrote:
> Hi Maxime,
>
> On Fri, Apr 29, 2022 at 05:46:45PM +0200, Maxime Ripard wrote:
> > On Fri, Apr 29, 2022 at 01:17:26AM +0300, Laurent Pinchart wrote:
> > > On Thu, Apr 28, 2022 at 02:09:42PM +0530, Jagan Teki wrote:
> > > > On Wed, Apr 27, 2022 at 8:04 PM Maxime Ripard wrote:
> > > > > On Tue, Apr 26, 2022 at 01:40:31PM +0530, Jagan Teki wrote:
> > > > > > On Tue, Apr 26, 2022 at 1:24 PM Paul Kocialkowski wrote:
> > > > > > > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > > > > > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > > > > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > > > > > > + Linus
> > > > > > > > > > + Marek
> > > > > > > > > > + Laurent
> > > > > > > > > > + Robert
> > > > > > > > > >
> > > > > > > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson wrote:
> > > > > > > > > > >
> > > > > > > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has
> > > > > > > > > > > panel or
> > > > > > > > > > > bridge")' attempted to simplify the case of expressing a
> > > > > > > > > > > simple panel
> > > > > > > > > > > under a DSI controller, by assuming that the first
> > > > > > > > > > > non-graph child node
> > > > > > > > > > > was a panel or bridge.
> > > > > > > > > > >
> > > > > > > > > > > Unfortunately for non-trivial cases the first child node
> > > > > > > > > > > might not be a
> > > > > > > > > > > panel or bridge. Examples of this can be a aux-bus in
> > > > > > > > > > > the case of
> > > > > > > > > > > DisplayPort, or an opp-table represented before the panel
> > > > > > > > > > > node.
> > > > > > > > > > >
> > > > > > > > > > > In these cases the reverted commit prevents the caller
> > > > > > > > > > > from ever finding
> > > > > > > > > > > a reference to the panel.
> > > > > > > > > > >
> > > > > > > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if
> > > > > > > > > > > child node has
> > > > > > > > > > > panel or bridge")', in favor of using an explicit graph
> > > > > > > > > > > reference to the
> > > > > > > > > > > panel in the trivial case as well.
> > > > > > > > > >
> > > > > > > > > > This eventually breaks many child-based
> > > > > > > > > > devm_drm_of_get_bridge
> > > > > > > > > > switched drivers. Do you have any suggestions on how to
> > > > > > > > > > proceed to
> > > > > > > > > > succeed in those use cases as well?
> > > > > > > > >
> > > > > > > > > I guess we could create a new helper for those, like
> > > > > > > > > devm_drm_of_get_bridge_with_panel, or something.
> > > > > > > >
> > > > > > > > Oh wow I feel stupid for not thinking about that.
> > > > > > > >
> > > > > > > > Yeah I agree that it seems like the best option.
> > > > > > >
> > > > > > > Should I prepare a patch with such a new helper?
> > > > > > >
> > > > > > > The idea would be to keep drm_of_find_panel_or_bridge only for
> > > > > > > the of graph
> > > > > > > case and add one for the child node case, maybe:
> > > > > > > drm_of_find_child_panel_or_bridge.
> > > > > > >
> > > > > > > I really don't have a clear idea of which driver would need to be
> > > > > > > switched
> > > > > > > over though. Could someone (Jagan?) let me know where it would be
> > > > > > > needed?
> > > > > >
> > > > > > sun6i_mipi_dsi
> > > > >
> > > > > It doesn't look like sun6i_mipi_dsi is using devm_drm_of_get_bridge
> > > > > at all?
> > > >
> > > > Correct, patch for this on the mailing list.
> > >
> > > I've lost track of how we're solving the fallout of this for v5.18. I
> > > have received a report that the original commit (80253168dbfd) also
> > > broke the rcar-du driver. Could you please provide a git branch (based
> > > on drm-fixes or drm-misc-fixes) with any patch that you plan to get
> > > merged in v5.18, to let me test them locally ?
> >
> > Was that report about 5.18 or drm-misc-next? It appears that all the
> > drivers conversions are in drm-misc-next.
>
> v5.18-rc2. I've double-checked, and it has been bisected to commit
> 67bae5f28c89, which is a fix of the commit this patch reverts
> (80253168dbfd).
We've reverted 67bae5f28c89 in -rc4, so it should work just fine now
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Wed, Apr 27, 2022 at 2:34 AM Paul Kocialkowski wrote: > > Hi Bjorn, > > On Tue 26 Apr 22, 14:10, Bjorn Andersson wrote: > > On Tue 26 Apr 06:50 PDT 2022, Paul Kocialkowski wrote: [..] > > > Bjorn, what do you think? > > > > > > > I'm okay with the idea that it's up the driver to check that the output > > port references an usb-c-connector - either before the call or upon > > drm_of_find_panel_or_bridge() returning an error. > > Actually I'm starting to think might be wrong on this one: there's a > display-connector bridge driver that should register a bridge, so > drm_of_find_panel_or_bridge should work. Did you have that driver enabled? > I don't have this driver enabled, but that seems like it would solve the problem when the remote node is a dp-connector. Unfortunately in my particular case, I have a usb-c-connector. So I don't see that I would be able to reuse this straight off. But I assume that this is trying to say that the USB Type-C code is supposed to provide a bridge for each of the connectors exposed by my USB Type-C controller? I've been building on top of drm_connector_oob_hotplug_event() to achieve this (with the link in the other direction)... Regards, Bjorn
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
Hi Maxime,
On Fri, Apr 29, 2022 at 05:46:45PM +0200, Maxime Ripard wrote:
> On Fri, Apr 29, 2022 at 01:17:26AM +0300, Laurent Pinchart wrote:
> > On Thu, Apr 28, 2022 at 02:09:42PM +0530, Jagan Teki wrote:
> > > On Wed, Apr 27, 2022 at 8:04 PM Maxime Ripard wrote:
> > > > On Tue, Apr 26, 2022 at 01:40:31PM +0530, Jagan Teki wrote:
> > > > > On Tue, Apr 26, 2022 at 1:24 PM Paul Kocialkowski wrote:
> > > > > > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > > > > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > > > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > > > > > + Linus
> > > > > > > > > + Marek
> > > > > > > > > + Laurent
> > > > > > > > > + Robert
> > > > > > > > >
> > > > > > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson wrote:
> > > > > > > > > >
> > > > > > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has
> > > > > > > > > > panel or
> > > > > > > > > > bridge")' attempted to simplify the case of expressing a
> > > > > > > > > > simple panel
> > > > > > > > > > under a DSI controller, by assuming that the first
> > > > > > > > > > non-graph child node
> > > > > > > > > > was a panel or bridge.
> > > > > > > > > >
> > > > > > > > > > Unfortunately for non-trivial cases the first child node
> > > > > > > > > > might not be a
> > > > > > > > > > panel or bridge. Examples of this can be a aux-bus in the
> > > > > > > > > > case of
> > > > > > > > > > DisplayPort, or an opp-table represented before the panel
> > > > > > > > > > node.
> > > > > > > > > >
> > > > > > > > > > In these cases the reverted commit prevents the caller from
> > > > > > > > > > ever finding
> > > > > > > > > > a reference to the panel.
> > > > > > > > > >
> > > > > > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if
> > > > > > > > > > child node has
> > > > > > > > > > panel or bridge")', in favor of using an explicit graph
> > > > > > > > > > reference to the
> > > > > > > > > > panel in the trivial case as well.
> > > > > > > > >
> > > > > > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > > > > > switched drivers. Do you have any suggestions on how to
> > > > > > > > > proceed to
> > > > > > > > > succeed in those use cases as well?
> > > > > > > >
> > > > > > > > I guess we could create a new helper for those, like
> > > > > > > > devm_drm_of_get_bridge_with_panel, or something.
> > > > > > >
> > > > > > > Oh wow I feel stupid for not thinking about that.
> > > > > > >
> > > > > > > Yeah I agree that it seems like the best option.
> > > > > >
> > > > > > Should I prepare a patch with such a new helper?
> > > > > >
> > > > > > The idea would be to keep drm_of_find_panel_or_bridge only for the
> > > > > > of graph
> > > > > > case and add one for the child node case, maybe:
> > > > > > drm_of_find_child_panel_or_bridge.
> > > > > >
> > > > > > I really don't have a clear idea of which driver would need to be
> > > > > > switched
> > > > > > over though. Could someone (Jagan?) let me know where it would be
> > > > > > needed?
> > > > >
> > > > > sun6i_mipi_dsi
> > > >
> > > > It doesn't look like sun6i_mipi_dsi is using devm_drm_of_get_bridge at
> > > > all?
> > >
> > > Correct, patch for this on the mailing list.
> >
> > I've lost track of how we're solving the fallout of this for v5.18. I
> > have received a report that the original commit (80253168dbfd) also
> > broke the rcar-du driver. Could you please provide a git branch (based
> > on drm-fixes or drm-misc-fixes) with any patch that you plan to get
> > merged in v5.18, to let me test them locally ?
>
> Was that report about 5.18 or drm-misc-next? It appears that all the
> drivers conversions are in drm-misc-next.
v5.18-rc2. I've double-checked, and it has been bisected to commit
67bae5f28c89, which is a fix of the commit this patch reverts
(80253168dbfd).
--
Regards,
Laurent Pinchart
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Fri, Apr 29, 2022 at 01:17:26AM +0300, Laurent Pinchart wrote:
> Hi Jagan,
>
> On Thu, Apr 28, 2022 at 02:09:42PM +0530, Jagan Teki wrote:
> > On Wed, Apr 27, 2022 at 8:04 PM Maxime Ripard wrote:
> > > On Tue, Apr 26, 2022 at 01:40:31PM +0530, Jagan Teki wrote:
> > > > On Tue, Apr 26, 2022 at 1:24 PM Paul Kocialkowski wrote:
> > > > > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > > > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > > > > + Linus
> > > > > > > > + Marek
> > > > > > > > + Laurent
> > > > > > > > + Robert
> > > > > > > >
> > > > > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson wrote:
> > > > > > > > >
> > > > > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has
> > > > > > > > > panel or
> > > > > > > > > bridge")' attempted to simplify the case of expressing a
> > > > > > > > > simple panel
> > > > > > > > > under a DSI controller, by assuming that the first non-graph
> > > > > > > > > child node
> > > > > > > > > was a panel or bridge.
> > > > > > > > >
> > > > > > > > > Unfortunately for non-trivial cases the first child node
> > > > > > > > > might not be a
> > > > > > > > > panel or bridge. Examples of this can be a aux-bus in the
> > > > > > > > > case of
> > > > > > > > > DisplayPort, or an opp-table represented before the panel
> > > > > > > > > node.
> > > > > > > > >
> > > > > > > > > In these cases the reverted commit prevents the caller from
> > > > > > > > > ever finding
> > > > > > > > > a reference to the panel.
> > > > > > > > >
> > > > > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child
> > > > > > > > > node has
> > > > > > > > > panel or bridge")', in favor of using an explicit graph
> > > > > > > > > reference to the
> > > > > > > > > panel in the trivial case as well.
> > > > > > > >
> > > > > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > > > > switched drivers. Do you have any suggestions on how to
> > > > > > > > proceed to
> > > > > > > > succeed in those use cases as well?
> > > > > > >
> > > > > > > I guess we could create a new helper for those, like
> > > > > > > devm_drm_of_get_bridge_with_panel, or something.
> > > > > >
> > > > > > Oh wow I feel stupid for not thinking about that.
> > > > > >
> > > > > > Yeah I agree that it seems like the best option.
> > > > >
> > > > > Should I prepare a patch with such a new helper?
> > > > >
> > > > > The idea would be to keep drm_of_find_panel_or_bridge only for the of
> > > > > graph
> > > > > case and add one for the child node case, maybe:
> > > > > drm_of_find_child_panel_or_bridge.
> > > > >
> > > > > I really don't have a clear idea of which driver would need to be
> > > > > switched
> > > > > over though. Could someone (Jagan?) let me know where it would be
> > > > > needed?
> > > >
> > > > sun6i_mipi_dsi
> > >
> > > It doesn't look like sun6i_mipi_dsi is using devm_drm_of_get_bridge at
> > > all?
> >
> > Correct, patch for this on the mailing list.
>
> I've lost track of how we're solving the fallout of this for v5.18. I
> have received a report that the original commit (80253168dbfd) also
> broke the rcar-du driver. Could you please provide a git branch (based
> on drm-fixes or drm-misc-fixes) with any patch that you plan to get
> merged in v5.18, to let me test them locally ?
Was that report about 5.18 or drm-misc-next? It appears that all the
drivers conversions are in drm-misc-next.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
Hi Laurent,
On Fri, Apr 29, 2022 at 3:47 AM Laurent Pinchart
wrote:
>
> Hi Jagan,
>
> On Thu, Apr 28, 2022 at 02:09:42PM +0530, Jagan Teki wrote:
> > On Wed, Apr 27, 2022 at 8:04 PM Maxime Ripard wrote:
> > > On Tue, Apr 26, 2022 at 01:40:31PM +0530, Jagan Teki wrote:
> > > > On Tue, Apr 26, 2022 at 1:24 PM Paul Kocialkowski wrote:
> > > > > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > > > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > > > > + Linus
> > > > > > > > + Marek
> > > > > > > > + Laurent
> > > > > > > > + Robert
> > > > > > > >
> > > > > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson wrote:
> > > > > > > > >
> > > > > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has
> > > > > > > > > panel or
> > > > > > > > > bridge")' attempted to simplify the case of expressing a
> > > > > > > > > simple panel
> > > > > > > > > under a DSI controller, by assuming that the first non-graph
> > > > > > > > > child node
> > > > > > > > > was a panel or bridge.
> > > > > > > > >
> > > > > > > > > Unfortunately for non-trivial cases the first child node
> > > > > > > > > might not be a
> > > > > > > > > panel or bridge. Examples of this can be a aux-bus in the
> > > > > > > > > case of
> > > > > > > > > DisplayPort, or an opp-table represented before the panel
> > > > > > > > > node.
> > > > > > > > >
> > > > > > > > > In these cases the reverted commit prevents the caller from
> > > > > > > > > ever finding
> > > > > > > > > a reference to the panel.
> > > > > > > > >
> > > > > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child
> > > > > > > > > node has
> > > > > > > > > panel or bridge")', in favor of using an explicit graph
> > > > > > > > > reference to the
> > > > > > > > > panel in the trivial case as well.
> > > > > > > >
> > > > > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > > > > switched drivers. Do you have any suggestions on how to
> > > > > > > > proceed to
> > > > > > > > succeed in those use cases as well?
> > > > > > >
> > > > > > > I guess we could create a new helper for those, like
> > > > > > > devm_drm_of_get_bridge_with_panel, or something.
> > > > > >
> > > > > > Oh wow I feel stupid for not thinking about that.
> > > > > >
> > > > > > Yeah I agree that it seems like the best option.
> > > > >
> > > > > Should I prepare a patch with such a new helper?
> > > > >
> > > > > The idea would be to keep drm_of_find_panel_or_bridge only for the of
> > > > > graph
> > > > > case and add one for the child node case, maybe:
> > > > > drm_of_find_child_panel_or_bridge.
> > > > >
> > > > > I really don't have a clear idea of which driver would need to be
> > > > > switched
> > > > > over though. Could someone (Jagan?) let me know where it would be
> > > > > needed?
> > > >
> > > > sun6i_mipi_dsi
> > >
> > > It doesn't look like sun6i_mipi_dsi is using devm_drm_of_get_bridge at
> > > all?
> >
> > Correct, patch for this on the mailing list.
>
> I've lost track of how we're solving the fallout of this for v5.18. I
> have received a report that the original commit (80253168dbfd) also
> broke the rcar-du driver. Could you please provide a git branch (based
> on drm-fixes or drm-misc-fixes) with any patch that you plan to get
> merged in v5.18, to let me test them locally ?
The affected patches for 80253168dbfd revert are
711c7adc4687
3730bc6147b0 and 3d7039e1e649
Both these are not present drm-misc-fixes but there in linux-next.
I've sent a patch for 711c7adc4687
https://patchwork.kernel.org/project/dri-devel/patch/20220428094808.782938-1-ja...@amarulasolutions.com/
This is my repo on top of linux-next
https://github.com/openedev/kernel/tree/linux-next/drm-misc
As I have seen before rcar-du ("155358310f013") is OF-graph and it
doesn't affect the child node lookup was introduced in
("80253168dbfd")
Let me know if you have any further information.
Jagan.
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
Hi Jagan,
On Thu, Apr 28, 2022 at 02:09:42PM +0530, Jagan Teki wrote:
> On Wed, Apr 27, 2022 at 8:04 PM Maxime Ripard wrote:
> > On Tue, Apr 26, 2022 at 01:40:31PM +0530, Jagan Teki wrote:
> > > On Tue, Apr 26, 2022 at 1:24 PM Paul Kocialkowski wrote:
> > > > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > > > + Linus
> > > > > > > + Marek
> > > > > > > + Laurent
> > > > > > > + Robert
> > > > > > >
> > > > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson wrote:
> > > > > > > >
> > > > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel
> > > > > > > > or
> > > > > > > > bridge")' attempted to simplify the case of expressing a simple
> > > > > > > > panel
> > > > > > > > under a DSI controller, by assuming that the first non-graph
> > > > > > > > child node
> > > > > > > > was a panel or bridge.
> > > > > > > >
> > > > > > > > Unfortunately for non-trivial cases the first child node might
> > > > > > > > not be a
> > > > > > > > panel or bridge. Examples of this can be a aux-bus in the case
> > > > > > > > of
> > > > > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > > > > >
> > > > > > > > In these cases the reverted commit prevents the caller from
> > > > > > > > ever finding
> > > > > > > > a reference to the panel.
> > > > > > > >
> > > > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child
> > > > > > > > node has
> > > > > > > > panel or bridge")', in favor of using an explicit graph
> > > > > > > > reference to the
> > > > > > > > panel in the trivial case as well.
> > > > > > >
> > > > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > > > switched drivers. Do you have any suggestions on how to proceed
> > > > > > > to
> > > > > > > succeed in those use cases as well?
> > > > > >
> > > > > > I guess we could create a new helper for those, like
> > > > > > devm_drm_of_get_bridge_with_panel, or something.
> > > > >
> > > > > Oh wow I feel stupid for not thinking about that.
> > > > >
> > > > > Yeah I agree that it seems like the best option.
> > > >
> > > > Should I prepare a patch with such a new helper?
> > > >
> > > > The idea would be to keep drm_of_find_panel_or_bridge only for the of
> > > > graph
> > > > case and add one for the child node case, maybe:
> > > > drm_of_find_child_panel_or_bridge.
> > > >
> > > > I really don't have a clear idea of which driver would need to be
> > > > switched
> > > > over though. Could someone (Jagan?) let me know where it would be
> > > > needed?
> > >
> > > sun6i_mipi_dsi
> >
> > It doesn't look like sun6i_mipi_dsi is using devm_drm_of_get_bridge at all?
>
> Correct, patch for this on the mailing list.
I've lost track of how we're solving the fallout of this for v5.18. I
have received a report that the original commit (80253168dbfd) also
broke the rcar-du driver. Could you please provide a git branch (based
on drm-fixes or drm-misc-fixes) with any patch that you plan to get
merged in v5.18, to let me test them locally ?
> > > exynos_drm_dsi
> >
> > If you reference 711c7adc4687, I don't see why we would need to switch
> > it back to the old behaviour. It wasn't iterating over its child node
> > before, so what does the switch to drm_of_get_bridge broke exactly?
>
> Exynos bindings have a child node (unlike OF-graph), the old code is
> checking panel and bridge individually so it broke once switch to
> devm_drm_of_get_bridge
--
Regards,
Laurent Pinchart
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Wed, Apr 27, 2022 at 8:04 PM Maxime Ripard wrote:
>
> On Tue, Apr 26, 2022 at 01:40:31PM +0530, Jagan Teki wrote:
> > On Tue, Apr 26, 2022 at 1:24 PM Paul Kocialkowski
> > wrote:
> > >
> > > Hi,
> > >
> > > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > > Hi Maxime,
> > > >
> > > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > > + Linus
> > > > > > + Marek
> > > > > > + Laurent
> > > > > > + Robert
> > > > > >
> > > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
> > > > > > wrote:
> > > > > > >
> > > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > > > > > > bridge")' attempted to simplify the case of expressing a simple
> > > > > > > panel
> > > > > > > under a DSI controller, by assuming that the first non-graph
> > > > > > > child node
> > > > > > > was a panel or bridge.
> > > > > > >
> > > > > > > Unfortunately for non-trivial cases the first child node might
> > > > > > > not be a
> > > > > > > panel or bridge. Examples of this can be a aux-bus in the case of
> > > > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > > > >
> > > > > > > In these cases the reverted commit prevents the caller from ever
> > > > > > > finding
> > > > > > > a reference to the panel.
> > > > > > >
> > > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child node
> > > > > > > has
> > > > > > > panel or bridge")', in favor of using an explicit graph reference
> > > > > > > to the
> > > > > > > panel in the trivial case as well.
> > > > > >
> > > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > > switched drivers. Do you have any suggestions on how to proceed to
> > > > > > succeed in those use cases as well?
> > > > >
> > > > > I guess we could create a new helper for those, like
> > > > > devm_drm_of_get_bridge_with_panel, or something.
> > > >
> > > > Oh wow I feel stupid for not thinking about that.
> > > >
> > > > Yeah I agree that it seems like the best option.
> > >
> > > Should I prepare a patch with such a new helper?
> > >
> > > The idea would be to keep drm_of_find_panel_or_bridge only for the of
> > > graph
> > > case and add one for the child node case, maybe:
> > > drm_of_find_child_panel_or_bridge.
> > >
> > > I really don't have a clear idea of which driver would need to be switched
> > > over though. Could someone (Jagan?) let me know where it would be needed?
> >
> > sun6i_mipi_dsi
>
> It doesn't look like sun6i_mipi_dsi is using devm_drm_of_get_bridge at all?
Correct, patch for this on the mailing list.
>
> > exynos_drm_dsi
>
> If you reference 711c7adc4687, I don't see why we would need to switch
> it back to the old behaviour. It wasn't iterating over its child node
> before, so what does the switch to drm_of_get_bridge broke exactly?
Exynos bindings have a child node (unlike OF-graph), the old code is
checking panel and bridge individually so it broke once switch to
devm_drm_of_get_bridge
Jagan.
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
Hi Marek,
On Thu, Apr 28, 2022 at 11:47 AM Marek Szyprowski
wrote:
>
> Hi Maxime,
>
> On 27.04.2022 16:34, Maxime Ripard wrote:
> > On Tue, Apr 26, 2022 at 01:40:31PM +0530, Jagan Teki wrote:
> >> On Tue, Apr 26, 2022 at 1:24 PM Paul Kocialkowski
> >> wrote:
> >>> On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> >> + Linus
> >> + Marek
> >> + Laurent
> >> + Robert
> >>
> >> On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
> >> wrote:
> >>> Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> >>> bridge")' attempted to simplify the case of expressing a simple panel
> >>> under a DSI controller, by assuming that the first non-graph child
> >>> node
> >>> was a panel or bridge.
> >>>
> >>> Unfortunately for non-trivial cases the first child node might not be
> >>> a
> >>> panel or bridge. Examples of this can be a aux-bus in the case of
> >>> DisplayPort, or an opp-table represented before the panel node.
> >>>
> >>> In these cases the reverted commit prevents the caller from ever
> >>> finding
> >>> a reference to the panel.
> >>>
> >>> This reverts commit '80253168dbfd ("drm: of: Lookup if child node has
> >>> panel or bridge")', in favor of using an explicit graph reference to
> >>> the
> >>> panel in the trivial case as well.
> >> This eventually breaks many child-based devm_drm_of_get_bridge
> >> switched drivers. Do you have any suggestions on how to proceed to
> >> succeed in those use cases as well?
> > I guess we could create a new helper for those, like
> > devm_drm_of_get_bridge_with_panel, or something.
> Oh wow I feel stupid for not thinking about that.
>
> Yeah I agree that it seems like the best option.
> >>> Should I prepare a patch with such a new helper?
> >>>
> >>> The idea would be to keep drm_of_find_panel_or_bridge only for the of
> >>> graph
> >>> case and add one for the child node case, maybe:
> >>> drm_of_find_child_panel_or_bridge.
> >>>
> >>> I really don't have a clear idea of which driver would need to be switched
> >>> over though. Could someone (Jagan?) let me know where it would be needed?
> >> sun6i_mipi_dsi
> > It doesn't look like sun6i_mipi_dsi is using devm_drm_of_get_bridge at all?
> >
> >> exynos_drm_dsi
> > If you reference 711c7adc4687, I don't see why we would need to switch
> > it back to the old behaviour. It wasn't iterating over its child node
> > before, so what does the switch to drm_of_get_bridge broke exactly?
>
> It broke getting the panel if it is a direct child of the DSI device
> node. It worked before because it used following code:
>
> dsi->panel = of_drm_find_panel(device->dev.of_node);
>
> which got replaced by devm_drm_of_get_bridge().
Yes, we need to revert that change back to find the individual panel
and bridge. I'm preparing a patch for it.
Jagan.
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
Hi Maxime,
On 27.04.2022 16:34, Maxime Ripard wrote:
> On Tue, Apr 26, 2022 at 01:40:31PM +0530, Jagan Teki wrote:
>> On Tue, Apr 26, 2022 at 1:24 PM Paul Kocialkowski
>> wrote:
>>> On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
>> + Linus
>> + Marek
>> + Laurent
>> + Robert
>>
>> On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
>> wrote:
>>> Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
>>> bridge")' attempted to simplify the case of expressing a simple panel
>>> under a DSI controller, by assuming that the first non-graph child node
>>> was a panel or bridge.
>>>
>>> Unfortunately for non-trivial cases the first child node might not be a
>>> panel or bridge. Examples of this can be a aux-bus in the case of
>>> DisplayPort, or an opp-table represented before the panel node.
>>>
>>> In these cases the reverted commit prevents the caller from ever finding
>>> a reference to the panel.
>>>
>>> This reverts commit '80253168dbfd ("drm: of: Lookup if child node has
>>> panel or bridge")', in favor of using an explicit graph reference to the
>>> panel in the trivial case as well.
>> This eventually breaks many child-based devm_drm_of_get_bridge
>> switched drivers. Do you have any suggestions on how to proceed to
>> succeed in those use cases as well?
> I guess we could create a new helper for those, like
> devm_drm_of_get_bridge_with_panel, or something.
Oh wow I feel stupid for not thinking about that.
Yeah I agree that it seems like the best option.
>>> Should I prepare a patch with such a new helper?
>>>
>>> The idea would be to keep drm_of_find_panel_or_bridge only for the of graph
>>> case and add one for the child node case, maybe:
>>> drm_of_find_child_panel_or_bridge.
>>>
>>> I really don't have a clear idea of which driver would need to be switched
>>> over though. Could someone (Jagan?) let me know where it would be needed?
>> sun6i_mipi_dsi
> It doesn't look like sun6i_mipi_dsi is using devm_drm_of_get_bridge at all?
>
>> exynos_drm_dsi
> If you reference 711c7adc4687, I don't see why we would need to switch
> it back to the old behaviour. It wasn't iterating over its child node
> before, so what does the switch to drm_of_get_bridge broke exactly?
It broke getting the panel if it is a direct child of the DSI device
node. It worked before because it used following code:
dsi->panel = of_drm_find_panel(device->dev.of_node);
which got replaced by devm_drm_of_get_bridge().
>> mcde_dsi (as of now)
> Yeah, we do need to revert 3730bc6147b0 and 3d7039e1e649
>
> Maxime
>
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Tue, Apr 26, 2022 at 01:40:31PM +0530, Jagan Teki wrote:
> On Tue, Apr 26, 2022 at 1:24 PM Paul Kocialkowski
> wrote:
> >
> > Hi,
> >
> > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > Hi Maxime,
> > >
> > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > + Linus
> > > > > + Marek
> > > > > + Laurent
> > > > > + Robert
> > > > >
> > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
> > > > > wrote:
> > > > > >
> > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > > > > > bridge")' attempted to simplify the case of expressing a simple
> > > > > > panel
> > > > > > under a DSI controller, by assuming that the first non-graph child
> > > > > > node
> > > > > > was a panel or bridge.
> > > > > >
> > > > > > Unfortunately for non-trivial cases the first child node might not
> > > > > > be a
> > > > > > panel or bridge. Examples of this can be a aux-bus in the case of
> > > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > > >
> > > > > > In these cases the reverted commit prevents the caller from ever
> > > > > > finding
> > > > > > a reference to the panel.
> > > > > >
> > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child node
> > > > > > has
> > > > > > panel or bridge")', in favor of using an explicit graph reference
> > > > > > to the
> > > > > > panel in the trivial case as well.
> > > > >
> > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > switched drivers. Do you have any suggestions on how to proceed to
> > > > > succeed in those use cases as well?
> > > >
> > > > I guess we could create a new helper for those, like
> > > > devm_drm_of_get_bridge_with_panel, or something.
> > >
> > > Oh wow I feel stupid for not thinking about that.
> > >
> > > Yeah I agree that it seems like the best option.
> >
> > Should I prepare a patch with such a new helper?
> >
> > The idea would be to keep drm_of_find_panel_or_bridge only for the of graph
> > case and add one for the child node case, maybe:
> > drm_of_find_child_panel_or_bridge.
> >
> > I really don't have a clear idea of which driver would need to be switched
> > over though. Could someone (Jagan?) let me know where it would be needed?
>
> sun6i_mipi_dsi
It doesn't look like sun6i_mipi_dsi is using devm_drm_of_get_bridge at all?
> exynos_drm_dsi
If you reference 711c7adc4687, I don't see why we would need to switch
it back to the old behaviour. It wasn't iterating over its child node
before, so what does the switch to drm_of_get_bridge broke exactly?
> mcde_dsi (as of now)
Yeah, we do need to revert 3730bc6147b0 and 3d7039e1e649
Maxime
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Tue, Apr 26, 2022 at 02:54:01PM +0200, Maxime Ripard wrote:
> On Tue, Apr 26, 2022 at 02:41:44PM +0200, Paul Kocialkowski wrote:
> > On Tue 26 Apr 22, 14:33, Laurent Pinchart wrote:
> > > On Tue, Apr 26, 2022 at 09:54:36AM +0200, Paul Kocialkowski wrote:
> > > > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > > > + Linus
> > > > > > > + Marek
> > > > > > > + Laurent
> > > > > > > + Robert
> > > > > > >
> > > > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson wrote:
> > > > > > > >
> > > > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel
> > > > > > > > or
> > > > > > > > bridge")' attempted to simplify the case of expressing a simple
> > > > > > > > panel
> > > > > > > > under a DSI controller, by assuming that the first non-graph
> > > > > > > > child node
> > > > > > > > was a panel or bridge.
> > > > > > > >
> > > > > > > > Unfortunately for non-trivial cases the first child node might
> > > > > > > > not be a
> > > > > > > > panel or bridge. Examples of this can be a aux-bus in the case
> > > > > > > > of
> > > > > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > > > > >
> > > > > > > > In these cases the reverted commit prevents the caller from
> > > > > > > > ever finding
> > > > > > > > a reference to the panel.
> > > > > > > >
> > > > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child
> > > > > > > > node has
> > > > > > > > panel or bridge")', in favor of using an explicit graph
> > > > > > > > reference to the
> > > > > > > > panel in the trivial case as well.
> > > > > > >
> > > > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > > > switched drivers. Do you have any suggestions on how to proceed
> > > > > > > to
> > > > > > > succeed in those use cases as well?
> > > > > >
> > > > > > I guess we could create a new helper for those, like
> > > > > > devm_drm_of_get_bridge_with_panel, or something.
> > > > >
> > > > > Oh wow I feel stupid for not thinking about that.
> > > > >
> > > > > Yeah I agree that it seems like the best option.
> > > >
> > > > Should I prepare a patch with such a new helper?
> > > >
> > > > The idea would be to keep drm_of_find_panel_or_bridge only for the of
> > > > graph
> > > > case and add one for the child node case, maybe:
> > > > drm_of_find_child_panel_or_bridge.
> > > >
> > > > I really don't have a clear idea of which driver would need to be
> > > > switched
> > > > over though. Could someone (Jagan?) let me know where it would be
> > > > needed?
> > > >
> > > > Are there cases where we could both expect of graph and child node?
> > > > (i.e. does the new helper also need to try via of graph?)
> > >
> > > I still think we should use OF graph uncondtionally, even in the DSI
> > > case. We need to ensure backward-compatibility, but I'd like new
> > > bindings (and thus new drivers) to always use OF graph.
> >
> > I just went over the thread on "drm: of: Improve error handling in
> > bridge/panel
> > detection" again and I'm no longer sure there's actually still an issue that
> > stands, with the fix that allows returning -ENODEV when possible.
> >
> > The remaining issue that was brought up was with a connector node, but it
> > should
> > be up to the driver to detect that and avoid calling
> > drm_of_find_panel_or_bridge
> > in such situations.
> >
> > So with that in mind it feels like the child node approach can be viable
> > (and integrated in the same helper).
> >
> > We might still want to favor an explicit OF graph approach, but note that
> > dsi-controller.yaml also specifies extra properties that are specific to
> > MIPI DSI and I'm not sure there are equivalent definitions for the OF graph
> > approach.
> >
> > What do you think?
>
> I don't think Laurent's point was to move the child node away from its
> DSI controller, that part doesn't make much sense. The panel or bridge
> is still accessed through the DSI bus, so it very much belongs there.
>
> What he meant I think was that we mandate the OF graph for all panels,
> so for panels/bridges controlled through DCS, you would still list the
> output through the graph.
That's right. A DCS panel would still be a child of the DSI controller.
--
Regards,
Laurent Pinchart
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Wed, Apr 27, 2022 at 05:22:32PM +0530, Jagan Teki wrote:
> On Wed, Apr 27, 2022 at 12:29 PM Jagan Teki wrote:
> > On Thu, Apr 21, 2022 at 1:54 PM Maxime Ripard wrote:
> > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > + Linus
> > > > + Marek
> > > > + Laurent
> > > > + Robert
> > > >
> > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
> > > > wrote:
> > > > >
> > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > > > > bridge")' attempted to simplify the case of expressing a simple panel
> > > > > under a DSI controller, by assuming that the first non-graph child
> > > > > node
> > > > > was a panel or bridge.
> > > > >
> > > > > Unfortunately for non-trivial cases the first child node might not be
> > > > > a
> > > > > panel or bridge. Examples of this can be a aux-bus in the case of
> > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > >
> > > > > In these cases the reverted commit prevents the caller from ever
> > > > > finding
> > > > > a reference to the panel.
> > > > >
> > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child node has
> > > > > panel or bridge")', in favor of using an explicit graph reference to
> > > > > the
> > > > > panel in the trivial case as well.
> > > >
> > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > switched drivers. Do you have any suggestions on how to proceed to
> > > > succeed in those use cases as well?
> > >
> > > I guess we could create a new helper for those, like
> > > devm_drm_of_get_bridge_with_panel, or something.
> >
> > I think using the same existing helper and updating child support is
> > make sense, as there is a possibility to use the same host for child
> > and OF-graph bindings.
> >
> > I can see two possible solutions (as of now)
> >
> > 1. adding "dcs-child-type" bindings for child-based panel or bridge
> > 2. iterate child and skip those nodes other than panel or bridge. or
> > iterate sub-child to find it has a panel or bridge-like aux-bus (which
> > is indeed hard as this configuration seems not 'standard' i think )
> >
> > Any inputs?
>
> Checking aux-bus with the sub-node panel can be a possible check to
> look at it, any comments?
Can we stop piling hacks and move towards OF graph everywhere, please ?
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -244,6 +244,25 @@ int drm_of_find_panel_or_bridge(const struct
> device_node *np,
> if (panel)
> *panel = NULL;
>
> + /**
> +* 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;
> +
> + if (!(of_node_name_eq(remote, "aux-bus") &&
> + of_get_child_by_name(remote, "panel")))
> + continue;
> +
> + 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,
> @@ -254,6 +273,8 @@ int drm_of_find_panel_or_bridge(const struct
> device_node *np,
> return -ENODEV;
>
> remote = of_graph_get_remote_node(np, port, endpoint);
> +
> +of_find_panel_or_bridge:
> if (!remote)
> return -ENODEV;
--
Regards,
Laurent Pinchart
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
Hi Paul,
On Wed, Apr 27, 2022 at 5:49 PM Paul Kocialkowski
wrote:
>
> Hi Jagan,
>
> On Wed 27 Apr 22, 17:22, Jagan Teki wrote:
> > On Wed, Apr 27, 2022 at 12:29 PM Jagan Teki
> > wrote:
> > >
> > > On Thu, Apr 21, 2022 at 1:54 PM Maxime Ripard wrote:
> > > >
> > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > + Linus
> > > > > + Marek
> > > > > + Laurent
> > > > > + Robert
> > > > >
> > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
> > > > > wrote:
> > > > > >
> > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > > > > > bridge")' attempted to simplify the case of expressing a simple
> > > > > > panel
> > > > > > under a DSI controller, by assuming that the first non-graph child
> > > > > > node
> > > > > > was a panel or bridge.
> > > > > >
> > > > > > Unfortunately for non-trivial cases the first child node might not
> > > > > > be a
> > > > > > panel or bridge. Examples of this can be a aux-bus in the case of
> > > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > > >
> > > > > > In these cases the reverted commit prevents the caller from ever
> > > > > > finding
> > > > > > a reference to the panel.
> > > > > >
> > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child node
> > > > > > has
> > > > > > panel or bridge")', in favor of using an explicit graph reference
> > > > > > to the
> > > > > > panel in the trivial case as well.
> > > > >
> > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > switched drivers. Do you have any suggestions on how to proceed to
> > > > > succeed in those use cases as well?
> > > >
> > > > I guess we could create a new helper for those, like
> > > > devm_drm_of_get_bridge_with_panel, or something.
> > >
> > > I think using the same existing helper and updating child support is
> > > make sense, as there is a possibility to use the same host for child
> > > and OF-graph bindings.
> > >
> > > I can see two possible solutions (as of now)
> > >
> > > 1. adding "dcs-child-type" bindings for child-based panel or bridge
> > > 2. iterate child and skip those nodes other than panel or bridge. or
> > > iterate sub-child to find it has a panel or bridge-like aux-bus (which
> > > is indeed hard as this configuration seems not 'standard' i think )
> > >
> > > Any inputs?
> >
> > Checking aux-bus with the sub-node panel can be a possible check to
> > look at it, any comments?
>
> That looks very fragile and oddly specific. Also why base changes on the
> original patch that you made?
It is just showcased a snippet to check the child's conditions.
>
> With the follow-up fixes, we are checking the of graph first and only
> considering child nodes if the of graph and remote are missing, so there isn't
> really a need to be more specific in the child noise discrimination.
So, does it handle child panel or bridge finding? just keep in mind
the same call from the host need to handle with and without OF-graph
representation.
>
> Actually I should also make a new version of "drm: of: Improve error handling
> in
> bridge/panel detection" to also return -ENODEV if of_graph_get_remote_node
> fails, so that it doesn't try to use the child node when the graph is defined
> but not remote is defined.
Jagan.
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Wed, Apr 27, 2022 at 12:29 PM Jagan Teki wrote:
>
> On Thu, Apr 21, 2022 at 1:54 PM Maxime Ripard wrote:
> >
> > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > + Linus
> > > + Marek
> > > + Laurent
> > > + Robert
> > >
> > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
> > > wrote:
> > > >
> > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > > > bridge")' attempted to simplify the case of expressing a simple panel
> > > > under a DSI controller, by assuming that the first non-graph child node
> > > > was a panel or bridge.
> > > >
> > > > Unfortunately for non-trivial cases the first child node might not be a
> > > > panel or bridge. Examples of this can be a aux-bus in the case of
> > > > DisplayPort, or an opp-table represented before the panel node.
> > > >
> > > > In these cases the reverted commit prevents the caller from ever finding
> > > > a reference to the panel.
> > > >
> > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child node has
> > > > panel or bridge")', in favor of using an explicit graph reference to the
> > > > panel in the trivial case as well.
> > >
> > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > switched drivers. Do you have any suggestions on how to proceed to
> > > succeed in those use cases as well?
> >
> > I guess we could create a new helper for those, like
> > devm_drm_of_get_bridge_with_panel, or something.
>
> I think using the same existing helper and updating child support is
> make sense, as there is a possibility to use the same host for child
> and OF-graph bindings.
>
> I can see two possible solutions (as of now)
>
> 1. adding "dcs-child-type" bindings for child-based panel or bridge
> 2. iterate child and skip those nodes other than panel or bridge. or
> iterate sub-child to find it has a panel or bridge-like aux-bus (which
> is indeed hard as this configuration seems not 'standard' i think )
>
> Any inputs?
Checking aux-bus with the sub-node panel can be a possible check to
look at it, any comments?
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -244,6 +244,25 @@ int drm_of_find_panel_or_bridge(const struct
device_node *np,
if (panel)
*panel = NULL;
+ /**
+* 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;
+
+ if (!(of_node_name_eq(remote, "aux-bus") &&
+ of_get_child_by_name(remote, "panel")))
+ continue;
+
+ 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,
@@ -254,6 +273,8 @@ int drm_of_find_panel_or_bridge(const struct
device_node *np,
return -ENODEV;
remote = of_graph_get_remote_node(np, port, endpoint);
+
+of_find_panel_or_bridge:
if (!remote)
return -ENODEV;
Jagan.
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Thu, Apr 21, 2022 at 1:54 PM Maxime Ripard wrote:
>
> On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > + Linus
> > + Marek
> > + Laurent
> > + Robert
> >
> > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
> > wrote:
> > >
> > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > > bridge")' attempted to simplify the case of expressing a simple panel
> > > under a DSI controller, by assuming that the first non-graph child node
> > > was a panel or bridge.
> > >
> > > Unfortunately for non-trivial cases the first child node might not be a
> > > panel or bridge. Examples of this can be a aux-bus in the case of
> > > DisplayPort, or an opp-table represented before the panel node.
> > >
> > > In these cases the reverted commit prevents the caller from ever finding
> > > a reference to the panel.
> > >
> > > This reverts commit '80253168dbfd ("drm: of: Lookup if child node has
> > > panel or bridge")', in favor of using an explicit graph reference to the
> > > panel in the trivial case as well.
> >
> > This eventually breaks many child-based devm_drm_of_get_bridge
> > switched drivers. Do you have any suggestions on how to proceed to
> > succeed in those use cases as well?
>
> I guess we could create a new helper for those, like
> devm_drm_of_get_bridge_with_panel, or something.
I think using the same existing helper and updating child support is
make sense, as there is a possibility to use the same host for child
and OF-graph bindings.
I can see two possible solutions (as of now)
1. adding "dcs-child-type" bindings for child-based panel or bridge
2. iterate child and skip those nodes other than panel or bridge. or
iterate sub-child to find it has a panel or bridge-like aux-bus (which
is indeed hard as this configuration seems not 'standard' i think )
Any inputs?
Thanks,
Jagan.
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Tue 26 Apr 06:50 PDT 2022, Paul Kocialkowski wrote:
> On Tue 26 Apr 22, 15:19, Maxime Ripard wrote:
> > On Tue, Apr 26, 2022 at 03:04:17PM +0200, Paul Kocialkowski wrote:
> > > On Tue 26 Apr 22, 14:55, Maxime Ripard wrote:
> > > > On Tue, Apr 26, 2022 at 02:54:01PM +0200, Maxime Ripard wrote:
> > > > > On Tue, Apr 26, 2022 at 02:41:44PM +0200, Paul Kocialkowski wrote:
> > > > > > On Tue 26 Apr 22, 14:33, Laurent Pinchart wrote:
> > > > > > > On Tue, Apr 26, 2022 at 09:54:36AM +0200, Paul Kocialkowski wrote:
> > > > > > > > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > > > > > > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > > > > > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > > > > > > > + Linus
> > > > > > > > > > > + Marek
> > > > > > > > > > > + Laurent
> > > > > > > > > > > + Robert
> > > > > > > > > > >
> > > > > > > > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node
> > > > > > > > > > > > has panel or
> > > > > > > > > > > > bridge")' attempted to simplify the case of expressing
> > > > > > > > > > > > a simple panel
> > > > > > > > > > > > under a DSI controller, by assuming that the first
> > > > > > > > > > > > non-graph child node
> > > > > > > > > > > > was a panel or bridge.
> > > > > > > > > > > >
> > > > > > > > > > > > Unfortunately for non-trivial cases the first child
> > > > > > > > > > > > node might not be a
> > > > > > > > > > > > panel or bridge. Examples of this can be a aux-bus in
> > > > > > > > > > > > the case of
> > > > > > > > > > > > DisplayPort, or an opp-table represented before the
> > > > > > > > > > > > panel node.
> > > > > > > > > > > >
> > > > > > > > > > > > In these cases the reverted commit prevents the caller
> > > > > > > > > > > > from ever finding
> > > > > > > > > > > > a reference to the panel.
> > > > > > > > > > > >
> > > > > > > > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if
> > > > > > > > > > > > child node has
> > > > > > > > > > > > panel or bridge")', in favor of using an explicit graph
> > > > > > > > > > > > reference to the
> > > > > > > > > > > > panel in the trivial case as well.
> > > > > > > > > > >
> > > > > > > > > > > This eventually breaks many child-based
> > > > > > > > > > > devm_drm_of_get_bridge
> > > > > > > > > > > switched drivers. Do you have any suggestions on how to
> > > > > > > > > > > proceed to
> > > > > > > > > > > succeed in those use cases as well?
> > > > > > > > > >
> > > > > > > > > > I guess we could create a new helper for those, like
> > > > > > > > > > devm_drm_of_get_bridge_with_panel, or something.
> > > > > > > > >
> > > > > > > > > Oh wow I feel stupid for not thinking about that.
> > > > > > > > >
> > > > > > > > > Yeah I agree that it seems like the best option.
> > > > > > > >
> > > > > > > > Should I prepare a patch with such a new helper?
> > > > > > > >
> > > > > > > > The idea would be to keep drm_of_find_panel_or_bridge only for
> > > > > > > > the of graph
> > > > > > > > case and add one for the child node case, maybe:
> > > > > > > > drm_of_find_child_panel_or_bridge.
> > > > > > > >
> > > > > > > > I really don't have a clear idea of which driver would need to
> > > > > > > > be switched
> > > > > > > > over though. Could someone (Jagan?) let me know where it would
> > > > > > > > be needed?
> > > > > > > >
> > > > > > > > Are there cases where we could both expect of graph and child
> > > > > > > > node?
> > > > > > > > (i.e. does the new helper also need to try via of graph?)
> > > > > > >
> > > > > > > I still think we should use OF graph uncondtionally, even in the
> > > > > > > DSI
> > > > > > > case. We need to ensure backward-compatibility, but I'd like new
> > > > > > > bindings (and thus new drivers) to always use OF graph.
> > > > > >
> > > > > > I just went over the thread on "drm: of: Improve error handling in
> > > > > > bridge/panel
> > > > > > detection" again and I'm no longer sure there's actually still an
> > > > > > issue that
> > > > > > stands, with the fix that allows returning -ENODEV when possible.
> > > > > >
> > > > > > The remaining issue that was brought up was with a connector node,
> > > > > > but it should
> > > > > > be up to the driver to detect that and avoid calling
> > > > > > drm_of_find_panel_or_bridge
> > > > > > in such situations.
> > > > > >
> > > > > > So with that in mind it feels like the child node approach can be
> > > > > > viable
> > > > > > (and integrated in the same helper).
> > > > > >
> > > > > > We might still want to favor an explicit OF graph approach, but
> > > > > > note that
> > > > > > dsi-controller.yaml also specifies extra properties that are
> > > > > > specific to
> > > > > > MIPI DSI and I'm not sure there are equivalent definitions for the
> > > > > > OF graph
> > > > > > approach.
> > > > > >
> > > > > > What do
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Tue, Apr 26, 2022 at 03:04:17PM +0200, Paul Kocialkowski wrote:
> On Tue 26 Apr 22, 14:55, Maxime Ripard wrote:
> > On Tue, Apr 26, 2022 at 02:54:01PM +0200, Maxime Ripard wrote:
> > > On Tue, Apr 26, 2022 at 02:41:44PM +0200, Paul Kocialkowski wrote:
> > > > On Tue 26 Apr 22, 14:33, Laurent Pinchart wrote:
> > > > > On Tue, Apr 26, 2022 at 09:54:36AM +0200, Paul Kocialkowski wrote:
> > > > > > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > > > > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > > > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > > > > > + Linus
> > > > > > > > > + Marek
> > > > > > > > > + Laurent
> > > > > > > > > + Robert
> > > > > > > > >
> > > > > > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson wrote:
> > > > > > > > > >
> > > > > > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has
> > > > > > > > > > panel or
> > > > > > > > > > bridge")' attempted to simplify the case of expressing a
> > > > > > > > > > simple panel
> > > > > > > > > > under a DSI controller, by assuming that the first
> > > > > > > > > > non-graph child node
> > > > > > > > > > was a panel or bridge.
> > > > > > > > > >
> > > > > > > > > > Unfortunately for non-trivial cases the first child node
> > > > > > > > > > might not be a
> > > > > > > > > > panel or bridge. Examples of this can be a aux-bus in the
> > > > > > > > > > case of
> > > > > > > > > > DisplayPort, or an opp-table represented before the panel
> > > > > > > > > > node.
> > > > > > > > > >
> > > > > > > > > > In these cases the reverted commit prevents the caller from
> > > > > > > > > > ever finding
> > > > > > > > > > a reference to the panel.
> > > > > > > > > >
> > > > > > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if
> > > > > > > > > > child node has
> > > > > > > > > > panel or bridge")', in favor of using an explicit graph
> > > > > > > > > > reference to the
> > > > > > > > > > panel in the trivial case as well.
> > > > > > > > >
> > > > > > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > > > > > switched drivers. Do you have any suggestions on how to
> > > > > > > > > proceed to
> > > > > > > > > succeed in those use cases as well?
> > > > > > > >
> > > > > > > > I guess we could create a new helper for those, like
> > > > > > > > devm_drm_of_get_bridge_with_panel, or something.
> > > > > > >
> > > > > > > Oh wow I feel stupid for not thinking about that.
> > > > > > >
> > > > > > > Yeah I agree that it seems like the best option.
> > > > > >
> > > > > > Should I prepare a patch with such a new helper?
> > > > > >
> > > > > > The idea would be to keep drm_of_find_panel_or_bridge only for the
> > > > > > of graph
> > > > > > case and add one for the child node case, maybe:
> > > > > > drm_of_find_child_panel_or_bridge.
> > > > > >
> > > > > > I really don't have a clear idea of which driver would need to be
> > > > > > switched
> > > > > > over though. Could someone (Jagan?) let me know where it would be
> > > > > > needed?
> > > > > >
> > > > > > Are there cases where we could both expect of graph and child node?
> > > > > > (i.e. does the new helper also need to try via of graph?)
> > > > >
> > > > > I still think we should use OF graph uncondtionally, even in the DSI
> > > > > case. We need to ensure backward-compatibility, but I'd like new
> > > > > bindings (and thus new drivers) to always use OF graph.
> > > >
> > > > I just went over the thread on "drm: of: Improve error handling in
> > > > bridge/panel
> > > > detection" again and I'm no longer sure there's actually still an issue
> > > > that
> > > > stands, with the fix that allows returning -ENODEV when possible.
> > > >
> > > > The remaining issue that was brought up was with a connector node, but
> > > > it should
> > > > be up to the driver to detect that and avoid calling
> > > > drm_of_find_panel_or_bridge
> > > > in such situations.
> > > >
> > > > So with that in mind it feels like the child node approach can be viable
> > > > (and integrated in the same helper).
> > > >
> > > > We might still want to favor an explicit OF graph approach, but note
> > > > that
> > > > dsi-controller.yaml also specifies extra properties that are specific to
> > > > MIPI DSI and I'm not sure there are equivalent definitions for the OF
> > > > graph
> > > > approach.
> > > >
> > > > What do you think?
> > >
> > > I don't think Laurent's point was to move the child node away from its
> > > DSI controller, that part doesn't make much sense. The panel or bridge
> > > is still accessed through the DSI bus, so it very much belongs there.
> > >
> > > What he meant I think was that we mandate the OF graph for all panels,
> > > so for panels/bridges controlled through DCS, you would still list the
> > > output through the graph.
> >
> > Also, we're already in a bit of a mess right now. I don't think rushing
> > that kind of patches in a (late) rc is
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Tue, Apr 26, 2022 at 02:54:01PM +0200, Maxime Ripard wrote:
> On Tue, Apr 26, 2022 at 02:41:44PM +0200, Paul Kocialkowski wrote:
> > On Tue 26 Apr 22, 14:33, Laurent Pinchart wrote:
> > > On Tue, Apr 26, 2022 at 09:54:36AM +0200, Paul Kocialkowski wrote:
> > > > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > > > + Linus
> > > > > > > + Marek
> > > > > > > + Laurent
> > > > > > > + Robert
> > > > > > >
> > > > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson wrote:
> > > > > > > >
> > > > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel
> > > > > > > > or
> > > > > > > > bridge")' attempted to simplify the case of expressing a simple
> > > > > > > > panel
> > > > > > > > under a DSI controller, by assuming that the first non-graph
> > > > > > > > child node
> > > > > > > > was a panel or bridge.
> > > > > > > >
> > > > > > > > Unfortunately for non-trivial cases the first child node might
> > > > > > > > not be a
> > > > > > > > panel or bridge. Examples of this can be a aux-bus in the case
> > > > > > > > of
> > > > > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > > > > >
> > > > > > > > In these cases the reverted commit prevents the caller from
> > > > > > > > ever finding
> > > > > > > > a reference to the panel.
> > > > > > > >
> > > > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child
> > > > > > > > node has
> > > > > > > > panel or bridge")', in favor of using an explicit graph
> > > > > > > > reference to the
> > > > > > > > panel in the trivial case as well.
> > > > > > >
> > > > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > > > switched drivers. Do you have any suggestions on how to proceed
> > > > > > > to
> > > > > > > succeed in those use cases as well?
> > > > > >
> > > > > > I guess we could create a new helper for those, like
> > > > > > devm_drm_of_get_bridge_with_panel, or something.
> > > > >
> > > > > Oh wow I feel stupid for not thinking about that.
> > > > >
> > > > > Yeah I agree that it seems like the best option.
> > > >
> > > > Should I prepare a patch with such a new helper?
> > > >
> > > > The idea would be to keep drm_of_find_panel_or_bridge only for the of
> > > > graph
> > > > case and add one for the child node case, maybe:
> > > > drm_of_find_child_panel_or_bridge.
> > > >
> > > > I really don't have a clear idea of which driver would need to be
> > > > switched
> > > > over though. Could someone (Jagan?) let me know where it would be
> > > > needed?
> > > >
> > > > Are there cases where we could both expect of graph and child node?
> > > > (i.e. does the new helper also need to try via of graph?)
> > >
> > > I still think we should use OF graph uncondtionally, even in the DSI
> > > case. We need to ensure backward-compatibility, but I'd like new
> > > bindings (and thus new drivers) to always use OF graph.
> >
> > I just went over the thread on "drm: of: Improve error handling in
> > bridge/panel
> > detection" again and I'm no longer sure there's actually still an issue that
> > stands, with the fix that allows returning -ENODEV when possible.
> >
> > The remaining issue that was brought up was with a connector node, but it
> > should
> > be up to the driver to detect that and avoid calling
> > drm_of_find_panel_or_bridge
> > in such situations.
> >
> > So with that in mind it feels like the child node approach can be viable
> > (and integrated in the same helper).
> >
> > We might still want to favor an explicit OF graph approach, but note that
> > dsi-controller.yaml also specifies extra properties that are specific to
> > MIPI DSI and I'm not sure there are equivalent definitions for the OF graph
> > approach.
> >
> > What do you think?
>
> I don't think Laurent's point was to move the child node away from its
> DSI controller, that part doesn't make much sense. The panel or bridge
> is still accessed through the DSI bus, so it very much belongs there.
>
> What he meant I think was that we mandate the OF graph for all panels,
> so for panels/bridges controlled through DCS, you would still list the
> output through the graph.
Also, we're already in a bit of a mess right now. I don't think rushing
that kind of patches in a (late) rc is making much sense, but as I said,
if you want to start working on this, then I'll take a revert for the
next rc, and then we can work calmly on this.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Tue, Apr 26, 2022 at 02:41:44PM +0200, Paul Kocialkowski wrote:
> On Tue 26 Apr 22, 14:33, Laurent Pinchart wrote:
> > On Tue, Apr 26, 2022 at 09:54:36AM +0200, Paul Kocialkowski wrote:
> > > On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > > > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > > > + Linus
> > > > > > + Marek
> > > > > > + Laurent
> > > > > > + Robert
> > > > > >
> > > > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson wrote:
> > > > > > >
> > > > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > > > > > > bridge")' attempted to simplify the case of expressing a simple
> > > > > > > panel
> > > > > > > under a DSI controller, by assuming that the first non-graph
> > > > > > > child node
> > > > > > > was a panel or bridge.
> > > > > > >
> > > > > > > Unfortunately for non-trivial cases the first child node might
> > > > > > > not be a
> > > > > > > panel or bridge. Examples of this can be a aux-bus in the case of
> > > > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > > > >
> > > > > > > In these cases the reverted commit prevents the caller from ever
> > > > > > > finding
> > > > > > > a reference to the panel.
> > > > > > >
> > > > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child node
> > > > > > > has
> > > > > > > panel or bridge")', in favor of using an explicit graph reference
> > > > > > > to the
> > > > > > > panel in the trivial case as well.
> > > > > >
> > > > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > > > switched drivers. Do you have any suggestions on how to proceed to
> > > > > > succeed in those use cases as well?
> > > > >
> > > > > I guess we could create a new helper for those, like
> > > > > devm_drm_of_get_bridge_with_panel, or something.
> > > >
> > > > Oh wow I feel stupid for not thinking about that.
> > > >
> > > > Yeah I agree that it seems like the best option.
> > >
> > > Should I prepare a patch with such a new helper?
> > >
> > > The idea would be to keep drm_of_find_panel_or_bridge only for the of
> > > graph
> > > case and add one for the child node case, maybe:
> > > drm_of_find_child_panel_or_bridge.
> > >
> > > I really don't have a clear idea of which driver would need to be switched
> > > over though. Could someone (Jagan?) let me know where it would be needed?
> > >
> > > Are there cases where we could both expect of graph and child node?
> > > (i.e. does the new helper also need to try via of graph?)
> >
> > I still think we should use OF graph uncondtionally, even in the DSI
> > case. We need to ensure backward-compatibility, but I'd like new
> > bindings (and thus new drivers) to always use OF graph.
>
> I just went over the thread on "drm: of: Improve error handling in
> bridge/panel
> detection" again and I'm no longer sure there's actually still an issue that
> stands, with the fix that allows returning -ENODEV when possible.
>
> The remaining issue that was brought up was with a connector node, but it
> should
> be up to the driver to detect that and avoid calling
> drm_of_find_panel_or_bridge
> in such situations.
>
> So with that in mind it feels like the child node approach can be viable
> (and integrated in the same helper).
>
> We might still want to favor an explicit OF graph approach, but note that
> dsi-controller.yaml also specifies extra properties that are specific to
> MIPI DSI and I'm not sure there are equivalent definitions for the OF graph
> approach.
>
> What do you think?
I don't think Laurent's point was to move the child node away from its
DSI controller, that part doesn't make much sense. The panel or bridge
is still accessed through the DSI bus, so it very much belongs there.
What he meant I think was that we mandate the OF graph for all panels,
so for panels/bridges controlled through DCS, you would still list the
output through the graph.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Tue, Apr 26, 2022 at 09:54:36AM +0200, Paul Kocialkowski wrote:
> On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > Hi Maxime,
> >
> > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > + Linus
> > > > + Marek
> > > > + Laurent
> > > > + Robert
> > > >
> > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
> > > > wrote:
> > > > >
> > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > > > > bridge")' attempted to simplify the case of expressing a simple panel
> > > > > under a DSI controller, by assuming that the first non-graph child
> > > > > node
> > > > > was a panel or bridge.
> > > > >
> > > > > Unfortunately for non-trivial cases the first child node might not be
> > > > > a
> > > > > panel or bridge. Examples of this can be a aux-bus in the case of
> > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > >
> > > > > In these cases the reverted commit prevents the caller from ever
> > > > > finding
> > > > > a reference to the panel.
> > > > >
> > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child node has
> > > > > panel or bridge")', in favor of using an explicit graph reference to
> > > > > the
> > > > > panel in the trivial case as well.
> > > >
> > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > switched drivers. Do you have any suggestions on how to proceed to
> > > > succeed in those use cases as well?
> > >
> > > I guess we could create a new helper for those, like
> > > devm_drm_of_get_bridge_with_panel, or something.
> >
> > Oh wow I feel stupid for not thinking about that.
> >
> > Yeah I agree that it seems like the best option.
>
> Should I prepare a patch with such a new helper?
>
> The idea would be to keep drm_of_find_panel_or_bridge only for the of graph
> case and add one for the child node case, maybe:
> drm_of_find_child_panel_or_bridge.
>
> I really don't have a clear idea of which driver would need to be switched
> over though. Could someone (Jagan?) let me know where it would be needed?
>
> Are there cases where we could both expect of graph and child node?
> (i.e. does the new helper also need to try via of graph?)
Yeah, we should figure it out this week. I mentioned this to Dave, who
in turn talked about it Linus, so the fastest it's figured out the best.
The helper would probably be best, but if you don't have time to do it
by then, we can always revert those 3 patches until a helper is there.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Tue, Apr 26, 2022 at 09:54:36AM +0200, Paul Kocialkowski wrote:
> On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > + Linus
> > > > + Marek
> > > > + Laurent
> > > > + Robert
> > > >
> > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson wrote:
> > > > >
> > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > > > > bridge")' attempted to simplify the case of expressing a simple panel
> > > > > under a DSI controller, by assuming that the first non-graph child
> > > > > node
> > > > > was a panel or bridge.
> > > > >
> > > > > Unfortunately for non-trivial cases the first child node might not be
> > > > > a
> > > > > panel or bridge. Examples of this can be a aux-bus in the case of
> > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > >
> > > > > In these cases the reverted commit prevents the caller from ever
> > > > > finding
> > > > > a reference to the panel.
> > > > >
> > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child node has
> > > > > panel or bridge")', in favor of using an explicit graph reference to
> > > > > the
> > > > > panel in the trivial case as well.
> > > >
> > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > switched drivers. Do you have any suggestions on how to proceed to
> > > > succeed in those use cases as well?
> > >
> > > I guess we could create a new helper for those, like
> > > devm_drm_of_get_bridge_with_panel, or something.
> >
> > Oh wow I feel stupid for not thinking about that.
> >
> > Yeah I agree that it seems like the best option.
>
> Should I prepare a patch with such a new helper?
>
> The idea would be to keep drm_of_find_panel_or_bridge only for the of graph
> case and add one for the child node case, maybe:
> drm_of_find_child_panel_or_bridge.
>
> I really don't have a clear idea of which driver would need to be switched
> over though. Could someone (Jagan?) let me know where it would be needed?
>
> Are there cases where we could both expect of graph and child node?
> (i.e. does the new helper also need to try via of graph?)
I still think we should use OF graph uncondtionally, even in the DSI
case. We need to ensure backward-compatibility, but I'd like new
bindings (and thus new drivers) to always use OF graph.
--
Regards,
Laurent Pinchart
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Tue, Apr 26, 2022 at 1:24 PM Paul Kocialkowski
wrote:
>
> Hi,
>
> On Thu 21 Apr 22, 10:59, Paul Kocialkowski wrote:
> > Hi Maxime,
> >
> > On Thu 21 Apr 22, 10:23, Maxime Ripard wrote:
> > > On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> > > > + Linus
> > > > + Marek
> > > > + Laurent
> > > > + Robert
> > > >
> > > > On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
> > > > wrote:
> > > > >
> > > > > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > > > > bridge")' attempted to simplify the case of expressing a simple panel
> > > > > under a DSI controller, by assuming that the first non-graph child
> > > > > node
> > > > > was a panel or bridge.
> > > > >
> > > > > Unfortunately for non-trivial cases the first child node might not be
> > > > > a
> > > > > panel or bridge. Examples of this can be a aux-bus in the case of
> > > > > DisplayPort, or an opp-table represented before the panel node.
> > > > >
> > > > > In these cases the reverted commit prevents the caller from ever
> > > > > finding
> > > > > a reference to the panel.
> > > > >
> > > > > This reverts commit '80253168dbfd ("drm: of: Lookup if child node has
> > > > > panel or bridge")', in favor of using an explicit graph reference to
> > > > > the
> > > > > panel in the trivial case as well.
> > > >
> > > > This eventually breaks many child-based devm_drm_of_get_bridge
> > > > switched drivers. Do you have any suggestions on how to proceed to
> > > > succeed in those use cases as well?
> > >
> > > I guess we could create a new helper for those, like
> > > devm_drm_of_get_bridge_with_panel, or something.
> >
> > Oh wow I feel stupid for not thinking about that.
> >
> > Yeah I agree that it seems like the best option.
>
> Should I prepare a patch with such a new helper?
>
> The idea would be to keep drm_of_find_panel_or_bridge only for the of graph
> case and add one for the child node case, maybe:
> drm_of_find_child_panel_or_bridge.
>
> I really don't have a clear idea of which driver would need to be switched
> over though. Could someone (Jagan?) let me know where it would be needed?
sun6i_mipi_dsi, exynos_drm_dsi, mcde_dsi (as of now)
>
> Are there cases where we could both expect of graph and child node?
> (i.e. does the new helper also need to try via of graph?)
One finding so far from my side would be if the check iterates the
child and identify the panel or bridge child irrespective of the
position it has and untouched non-trivial child-like dp, opp-table can
help to use same change what we have it before. Still working on
getting a proper check.
Thanks,
Jagan.
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
On Thu, Apr 21, 2022 at 01:15:54PM +0530, Jagan Teki wrote:
> + Linus
> + Marek
> + Laurent
> + Robert
>
> On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
> wrote:
> >
> > Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> > bridge")' attempted to simplify the case of expressing a simple panel
> > under a DSI controller, by assuming that the first non-graph child node
> > was a panel or bridge.
> >
> > Unfortunately for non-trivial cases the first child node might not be a
> > panel or bridge. Examples of this can be a aux-bus in the case of
> > DisplayPort, or an opp-table represented before the panel node.
> >
> > In these cases the reverted commit prevents the caller from ever finding
> > a reference to the panel.
> >
> > This reverts commit '80253168dbfd ("drm: of: Lookup if child node has
> > panel or bridge")', in favor of using an explicit graph reference to the
> > panel in the trivial case as well.
>
> This eventually breaks many child-based devm_drm_of_get_bridge
> switched drivers. Do you have any suggestions on how to proceed to
> succeed in those use cases as well?
I guess we could create a new helper for those, like
devm_drm_of_get_bridge_with_panel, or something.
Maxime
signature.asc
Description: PGP signature
Re: [PATCH 2/2] Revert "drm: of: Lookup if child node has panel or bridge"
+ Linus
+ Marek
+ Laurent
+ Robert
On Thu, Apr 21, 2022 at 4:40 AM Bjorn Andersson
wrote:
>
> Commit '80253168dbfd ("drm: of: Lookup if child node has panel or
> bridge")' attempted to simplify the case of expressing a simple panel
> under a DSI controller, by assuming that the first non-graph child node
> was a panel or bridge.
>
> Unfortunately for non-trivial cases the first child node might not be a
> panel or bridge. Examples of this can be a aux-bus in the case of
> DisplayPort, or an opp-table represented before the panel node.
>
> In these cases the reverted commit prevents the caller from ever finding
> a reference to the panel.
>
> This reverts commit '80253168dbfd ("drm: of: Lookup if child node has
> panel or bridge")', in favor of using an explicit graph reference to the
> panel in the trivial case as well.
This eventually breaks many child-based devm_drm_of_get_bridge
switched drivers. Do you have any suggestions on how to proceed to
succeed in those use cases as well?
Jagan.
