On Tue, Feb 28, 2023 at 1:19 AM Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Feb 28, 2023 at 1:11 AM Marek Vasut <marex@xxxxxxx> wrote:
> >
> > On 2/27/23 20:34, Jagan Teki wrote:
> > > On Tue, Feb 28, 2023 at 12:54 AM Marek Vasut <marex@xxxxxxx> wrote:
> > >>
> > >> On 2/27/23 20:15, Jagan Teki wrote:
> > >>> On Tue, Feb 28, 2023 at 12:38 AM Marek Vasut <marex@xxxxxxx> wrote:
> > >>>>
> > >>>> On 2/27/23 20:01, Jagan Teki wrote:
> > >>>>> On Tue, Feb 28, 2023 at 12:25 AM Marek Vasut <marex@xxxxxxx> wrote:
> > >>>>>>
> > >>>>>> On 2/27/23 12:39, Jagan Teki wrote:
> > >>>>>>> drm_of_dsi_find_panel_or_bridge is capable of looking up the
> > >>>>>>> downstream DSI bridge and panel and trying to add a panel bridge
> > >>>>>>> if the panel is found.
> > >>>>>>>
> > >>>>>>> Replace explicit finding calls with drm_of_dsi_find_panel_or_bridge
> > >>>>>>> followed with drmm_panel_bridge_add.
> > >>>>>>>
> > >>>>>>> Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> > >>>>>>> ---
> > >>>>>>> Changes for v13, v12, v11:
> > >>>>>>> - none
> > >>>>>>> Changes for v10:
> > >>>>>>> - new patch
> > >>>>>>>
> > >>>>>>> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 25 +++++++++++++------------
> > >>>>>>> 1 file changed, 13 insertions(+), 12 deletions(-)
> > >>>>>>>
> > >>>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > >>>>>>> index df15501b1075..12a6dd987e8f 100644
> > >>>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > >>>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > >>>>>>> @@ -25,6 +25,7 @@
> > >>>>>>> #include <drm/drm_atomic_helper.h>
> > >>>>>>> #include <drm/drm_bridge.h>
> > >>>>>>> #include <drm/drm_mipi_dsi.h>
> > >>>>>>> +#include <drm/drm_of.h>
> > >>>>>>> #include <drm/drm_panel.h>
> > >>>>>>> #include <drm/drm_print.h>
> > >>>>>>> #include <drm/drm_probe_helper.h>
> > >>>>>>> @@ -1470,24 +1471,26 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
> > >>>>>>> struct device *dev = dsi->dev;
> > >>>>>>> struct drm_encoder *encoder = &dsi->encoder;
> > >>>>>>> struct drm_device *drm = encoder->dev;
> > >>>>>>> + struct drm_bridge *bridge;
> > >>>>>>> struct drm_panel *panel;
> > >>>>>>> int ret;
> > >>>>>>>
> > >>>>>>> - panel = of_drm_find_panel(device->dev.of_node);
> > >>>>>>> - if (!IS_ERR(panel)) {
> > >>>>>>> - dsi->out_bridge = devm_drm_panel_bridge_add(dev, panel);
> > >>>>>>> - } else {
> > >>>>>>> - dsi->out_bridge = of_drm_find_bridge(device->dev.of_node);
> > >>>>>>> - if (!dsi->out_bridge)
> > >>>>>>> - dsi->out_bridge = ERR_PTR(-EINVAL);
> > >>>>>>> - }
> > >>>>>>
> > >>>>>> As far as I understand this from my conversation with Maxime (please put
> > >>>>>> him on CC of V15), the change here should instead perform the panel look
> > >>>>>> up NOT in exynos_dsi_host_attach() , but in exynos_dsi_bind() , i.e. in
> > >>>>>> the component_ops .bind callback . Here is an example of correct
> > >>>>>> implementation:
> > >>>>>>
> > >>>>>> https://cgit.freedesktop.org/drm-misc/tree/drivers/gpu/drm/vc4/vc4_dsi.c#n1805
> > >>>>>
> > >>>>> But, we don't have component_ops.bind for imx8m case, so where do we
> > >>>>> want to place the panel hook?
> > >>>>>
> > >>>>> Exynos drm drivers are component-based but, imx8mm is not.
> > >>>>
> > >>>> In 14/18 patch, the same should be added to generic_dsim_register_host()
> > >>>> , which is called from the driver .probe() callback, but that is OK.
> > >>>>
> > >>>> That's ^ is the iMX part.
> > >>>
> > >>> Ohh. You mean, we need to find the panel hook separately in two places like
> > >>> - bind for exynos
> > >>> - probe for imx8mm
> > >>
> > >> Yes
> > >>
> > >>> If so? how can I find the drm_device pointer in the probe?
> > >>
> > >> What for ? The panel lookup uses OF graph . Where do you need the
> > >> drm_device in it ?
> > >
> > > May I can summarize the whole setback here so that everybody is on the
> > > same page and find the proper solution?
> > >
> > > The key blocker is accessing the DRM pointer in order to use the
> > > DRM-managed action helper.
> > >
> > > 1. If we find the panel hook in Exynos component_ops.bind then we can
> > > use the DRM pointer directly as VC4 does.
> > > 2. if we find the panel hook in Samsung drm_bridge_funcs.attach (for
> > > imx8mm) then we can use the DRM pointer directly via bridge->dev.
> > >
> > > If we make 2) has common across like finding the panel hook in
> > > drm_bridge_funcs.attach the Exynos drm pipeline cannot find the
> > > panels.
> > >
> > > The common solution for both exynos and imx8m is host.attach but if we
> > > do so we cannot get find the DRM pointer.
> >
> > There isn't going to be common solution, you would really have to do the
> > look up in component_ops .bind callback for exynos , and
> > generic_dsim_register_host() for i.MX .
> >
> > > If we go ahead with no need for DRM-managed helper at the moment, then
> > > find the panel hook in host.attach and then drop 2/18.
> >
> > The panel lookup must happen in .bind/.probe for exynos/imx respectively
> > , that's really all that is required here. Then you can drop 1,2,3/18
> > and get this series applied (I hope) .
>
> I'm not quite sure, if the panel hook in .bind work for exynos or not?
> the host. attach has KMS hotplug code after the panel hook and
> bridge_attach as before. I believe that is a working scenario for
> Exynos to be the panel hook in the host. attach.
>
> >
> > Can you implement just this one change ?
> >
> > There is no need to use drmm_* helper for now, that can be improved
> > later if possible.
>
> If this is the case then 1/18 will need otherwise we can drop 1,2,3/18
> by doing the panel hook we did in v7
> https://patchwork.kernel.org/project/dri-devel/patch/20221005151309.7278-3-jagan@xxxxxxxxxxxxxxxxxxxx/
This makes panel hook common a cross bridge by handling in the host.attach
Jagan.
