On 2/27/23 20:49, Jagan Teki wrote:
> On Tue, Feb 28, 2023 at 1:11 AM Marek Vasut <marex at denx.de> wrote:
>>
>> On 2/27/23 20:34, Jagan Teki wrote:
>>> On Tue, Feb 28, 2023 at 12:54 AM Marek Vasut <marex at denx.de> wrote:
>>>>
>>>> On 2/27/23 20:15, Jagan Teki wrote:
>>>>> On Tue, Feb 28, 2023 at 12:38 AM Marek Vasut <marex at denx.de> wrote:
>>>>>>
>>>>>> On 2/27/23 20:01, Jagan Teki wrote:
>>>>>>> On Tue, Feb 28, 2023 at 12:25 AM Marek Vasut <marex at 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 at 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) .
>
> I'm not quite sure, if the panel hook in .bind work for exynos or not?
Marek should be able to test exynos for you one more time I hope.
> 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.
As far as I understand it, the look up has to be in .bind callback (and
generic_dsim_register_host() for imx). Can you try if/how that works ?
>> 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
Ah yes, 1/18 will be needed indeed. It should just be called from .bind
callback or generic_dsim_register_host() (i.e. probe callback).
> 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@amarulasolutions.com/
[...]