Re: [PATCH v3 4/5] drm: convert drivers to use drm_of_find_panel_or_bridge

[Rob Herring <robh@kernel.org>]

On Tue, May 9, 2017 at 9:28 AM, Leonard Crestez <leonard.crestez@nxp.com> wrote:
> On Wed, Mar 22, 2017 at 5:01 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>> On Wed, 2017-03-22 at 08:26 -0500, Rob Herring wrote:
>> >
>> > Similar to the previous commit, convert drivers open coding OF graph
>> > parsing to use drm_of_find_panel_or_bridge instead.
>> >
>> > This changes some error messages to debug messages (in the graph core).
>> > Graph connections are often "no connects" depending on the particular
>> > board, so we want to avoid spurious messages. Plus the kernel is not a
>> > DT validator.
>> >
>> > Signed-off-by: Rob Herring <robh@kernel.org>
>> > Reviewed-by: Archit Taneja <architt@codeaurora.org>
>> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
>> (imx-ldb on i.MX6)
>
> It seems that this breaks on (at least) imx6qdl-sabreauto. The relevant
> section of the boot log looks like this:
>
> imx-drm display-subsystem: bound imx-ipuv3-crtc.2 (ops ipu_crtc_ops)
> imx-drm display-subsystem: bound imx-ipuv3-crtc.3 (ops ipu_crtc_ops)
> dwhdmi-imx 120000.hdmi: Detected HDMI TX controller v1.31a with HDCP
> (DWC HDMI 3D TX PHY)
> dwhdmi-imx 120000.hdmi: registered DesignWare HDMI I2C bus driver
> imx-drm display-subsystem: bound 120000.hdmi (ops dw_hdmi_imx_ops)
> imx-drm display-subsystem: failed to bind 2000000.aips-bus:ldb (ops
> imx_ldb_ops): -19
> imx-drm display-subsystem: master bind failed: -19
>
> It seems that imx6qdl-sabreauto does not have any panel defined in dts.
> This used to be ignored when of_graph_get_endpoint_by_regs returned
> NULL but now drm_of_find_panel_or_bridge returns -ENODEV and this
> causes imx_ldb_bind to fail altogether. Defining a panel works
> (including showing stuff on a LVDS panel). Ignoring -ENODEV also fixes
> this:
>
> --- drivers/gpu/drm/imx/imx-ldb.c
> +++ drivers/gpu/drm/imx/imx-ldb.c
> @@ -673,7 +673,7 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data)
>                 ret = drm_of_find_panel_or_bridge(child,
>                                                   imx_ldb->lvds_mux ? 4 : 2, 0,
>                                                   &channel->panel, &channel->bridge);
> -               if (ret)
> +               if (ret != -ENODEV)
>                         return ret;
>
>                 /* panel ddc only if there is no bridge */
>
> I don't know much about drm and it's not clear if failing to find a
> panel should be an error here or not and the hack above is likely the
> wrong way to handle it anyway.

That looks like the right fix to me. Ideally, the DT should probably
define an LVDS connector node (if not a panel) in this case like we do
for other cases with DDC, but more importantly we shouldn't require a
DT update to fix things. If you needed power or gpio controls of the
panel you would also need some node in DT.

Do you mind sending a proper patch?

Rob