Re: [PATCH v13 04/18] drm: exynos: dsi: Switch to DSI panel or bridge find helper

[Marek Vasut <marex@denx.de>]

On 2/27/23 20:34, Jagan Teki wrote:
> On Tue, Feb 28, 2023 at 12:54 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 2/27/23 20:15, Jagan Teki wrote:
>>> On Tue, Feb 28, 2023 at 12:38 AM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 2/27/23 20:01, Jagan Teki wrote:
>>>>> On Tue, Feb 28, 2023 at 12:25 AM Marek Vasut <marex@denx.de> 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@amarulasolutions.com>
>>>>>>> ---
>>>>>>> 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) .

Can you implement just this one change ?

There is no need to use drmm_* helper for now, that can be improved 
later if possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel