Message-ID: <43e5b273-d156-beea-bcfb-cc61b190a671@collabora.com>
Date:   Tue, 30 Jun 2020 23:02:35 +0200
From:   Enric Balletbo i Serra <enric.balletbo@...labora.com>
To:     Chun-Kuang Hu <chunkuang.hu@...nel.org>
Cc:     Sam Ravnborg <sam@...nborg.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Collabora Kernel ML <kernel@...labora.com>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Nicolas Boichat <drinkcat@...omium.org>,
        Jonas Karlman <jonas@...boo.se>,
        David Airlie <airlied@...ux.ie>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        Neil Armstrong <narmstrong@...libre.com>,
        Andrzej Hajda <a.hajda@...sung.com>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Hsin-Yi Wang <hsinyi@...omium.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RESEND PATCH v4 0/7] Convert mtk-dsi to drm_bridge API and get
 EDID for ps8640 bridge

Hi Chun-Kuang,

On 30/6/20 18:26, Chun-Kuang Hu wrote:
> Hi, Enric:
> 
> Enric Balletbo i Serra <enric.balletbo@...labora.com> 於 2020年6月30日 週二 下午10:34寫道：
>>
>> Hi Sam, Chun-Kuan,
>>
>> On 20/6/20 23:33, Sam Ravnborg wrote:
>>> Hi Enric
>>>
>>> On Mon, Jun 15, 2020 at 10:31:01PM +0200, Enric Balletbo i Serra wrote:
>>>> (This resend is to fix some trivial conflicts due the merge window)
>>>>
>>>> The PS8640 dsi-to-eDP bridge driver is using the panel bridge API,
>>>> however, not all the components in the chain have been ported to the
>>>> drm_bridge API. Actually, when a panel is attached the default panel's mode
>>>> is used, but in some cases we can't get display up if mode getting from
>>>> eDP control EDID is not chosen.
>>>>
>>>> This series address that problem, first implements the .get_edid()
>>>> callback in the PS8640 driver (which is not used until the conversion is
>>>> done) and then, converts the Mediatek DSI driver to use the drm_bridge
>>>> API.
>>>>
>>>> As far as I know, we're the only users of the mediatek dsi driver in
>>>> mainline, so should be safe to switch to the new chain of drm_bridge API
>>>> unconditionally.
>>>>
>>>> The patches has been tested on a Acer Chromebook R13 (Elm) running a
>>>> Chrome OS userspace and checking that the valid EDID mode reported by
>>>> the bridge is selected.
>>>>
>>>> Changes in v4:
>>>> - Remove double call to drm_encoder_init(). (Chun-Kuang Hu)
>>>> - Cleanup the encoder in mtk_dsi_unbind(). (Chun-Kuang Hu)
>>>>
>>>> Changes in v3:
>>>> - Replace s/bridge/next bridge/ for comment. (Laurent Pinchart)
>>>> - Add the bridge.type. (Laurent Pinchart)
>>>> - Use next_bridge field to store the panel bridge. (Laurent Pinchart)
>>>> - Add the bridge.type field. (Laurent Pinchart)
>>>> - This patch requires https://lkml.org/lkml/2020/4/16/2080 to work
>>>>   properly.
>>>> - Move the bridge.type line to the patch that adds drm_bridge support. (Laurent Pinchart)
>>>>
>>>> Changes in v2:
>>>> - Do not set connector_type for panel here. (Sam Ravnborg)
>>>>
>>>> Enric Balletbo i Serra (7):
>>>>   drm/bridge: ps8640: Get the EDID from eDP control
>>>>   drm/bridge_connector: Set default status connected for eDP connectors
>>>>   drm/mediatek: mtk_dsi: Rename bridge to next_bridge
>>>>   drm/mediatek: mtk_dsi: Convert to bridge driver
>>>>   drm/mediatek: mtk_dsi: Use simple encoder
>>>>   drm/mediatek: mtk_dsi: Use the drm_panel_bridge API
>>>>   drm/mediatek: mtk_dsi: Create connector for bridges
>>>
>>> Patch seems ready to apply. Will they be applied to a mediatek tree
>>> or to drm-misc-next?
>>> Or shall we take the first two patches via drm-misc-next, and the
>>> remaning via a mediatek tree? (I hope not)
>>>
>>
>> I think the only concern is from Chun-Kuan regarding patch 7/7 "drm/mediatek:
>> mtk_dsi: Create connector for bridges" whether we should support the old API or
>> not, but the discussion stalled.
>>
> 
> I get more clear now. In patch 7/7,
> 
> ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL,
>                                         DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> 
> this would call into mtk_dsi_bridge_attach() first, and then call into
> panel_bridge_attach() next. So panel_bridge_attach() would receive
> DRM_BRIDGE_ATTACH_NO_CONNECTOR and it return immediately so it does
> not call drm_panel_attach(). So where do you call drm_panel_attach()?
> 

Why I need to call drm_panel_attach?

I believe drm_panel_attach() was to attach a panel to a connector, but we don't
need to do this with the new API as the connector is already created and
attached to the "dummy" encoder.

Makes that sense to you? What do you think will not work if I don't call
drm_panel_attach?

[1]
https://elixir.bootlin.com/linux/v5.8-rc3/source/drivers/gpu/drm/drm_panel.c#L101

Regards,
 Enric


> Regards,
> Chun-Kuang.
> 
>> Thanks,
>>  Enric
>>
>>
>>
>>>       Sam
>>>
>>>
>>>>
>>>>  drivers/gpu/drm/bridge/parade-ps8640.c |  12 ++
>>>>  drivers/gpu/drm/drm_bridge_connector.c |   1 +
>>>>  drivers/gpu/drm/mediatek/mtk_dsi.c     | 269 ++++++++-----------------
>>>>  3 files changed, 97 insertions(+), 185 deletions(-)
>>>>
>>>> --
>>>> 2.27.0
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@...ts.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
> 

Powered by blists - more mailing lists