Message-ID: <CAMSo37Whxvp82i9hr-L-O4Qo9Pxkh5L+hFXqw9hNQ+asrD6oyA@mail.gmail.com>
Date:   Wed, 5 Jul 2023 00:05:13 +0800
From:   Yongqin Liu <yongqin.liu@...aro.org>
To:     Adam Ford <aford173@...il.com>
Cc:     dri-devel@...ts.freedesktop.org, dmitry.baryshkov@...aro.org,
        aford@...conembedded.com, Robert Foss <rfoss@...nel.org>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        linux-kernel@...r.kernel.org,
        Sumit Semwal <sumit.semwal@...aro.org>,
        John Stultz <jstultz@...gle.com>
Subject: Re: [PATCH V2] drm/bridge: adv7533: Fix adv7533_mode_valid for
 adv7533 and adv7535

Hi, Adam, All

On Sun, 19 Mar 2023 at 20:55, Adam Ford <aford173@...il.com> wrote:
>
> When dynamically switching lanes was removed, the intent of the code
> was to check to make sure that higher speed items used 4 lanes, but
> it had the unintended consequence of removing the slower speeds for
> 4-lane users.
>
> This attempts to remedy this by doing a check to see that the
> max frequency doesn't exceed the chip limit, and a second
> check to make sure that the max bit-rate doesn't exceed the
> number of lanes * max bit rate / lane.
>
> Fixes: 9a0cdcd6649b ("drm/bridge: adv7533: remove dynamic lane switching from adv7533 bridge")
> Reviewed-by: Robert Foss <rfoss@...nel.org>
> Signed-off-by: Adam Ford <aford173@...il.com>
> ---
>
> V2:  Fix whitespace in comment
>      Remove TODO comment
>      Add R-B from Robert.

With this change, the ACK android-mainline based hikey960 build failed
to show UI on the HDMI
monitor connected, but it works if I revert this change.
Here is the serial console output: http://ix.io/4zK8

Not sure if you have any idea what the problem is there,
and how to have it fixed.

Please let me know if you need any other information.

Thanks,
Yongqin Liu

> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> index fdfeadcefe80..7e3e56441aed 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> @@ -103,22 +103,19 @@ void adv7533_dsi_power_off(struct adv7511 *adv)
>  enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
>                                         const struct drm_display_mode *mode)
>  {
> -       int lanes;
> +       unsigned long max_lane_freq;
>         struct mipi_dsi_device *dsi = adv->dsi;
> +       u8 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
>
> -       if (mode->clock > 80000)
> -               lanes = 4;
> -       else
> -               lanes = 3;
> -
> -       /*
> -        * TODO: add support for dynamic switching of lanes
> -        * by using the bridge pre_enable() op . Till then filter
> -        * out the modes which shall need different number of lanes
> -        * than what was configured in the device tree.
> -        */
> -       if (lanes != dsi->lanes)
> -               return MODE_BAD;
> +       /* Check max clock for either 7533 or 7535 */
> +       if (mode->clock > (adv->type == ADV7533 ? 80000 : 148500))
> +               return MODE_CLOCK_HIGH;
> +
> +       /* Check max clock for each lane */
> +       max_lane_freq = (adv->type == ADV7533 ? 800000 : 891000);
> +
> +       if (mode->clock * bpp > max_lane_freq * adv->num_dsi_lanes)
> +               return MODE_CLOCK_HIGH;
>
>         return MODE_OK;
>  }
> --
> 2.34.1
>


--
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@...ts.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android

Powered by blists - more mailing lists