Re: [PATCH V2] drm/bridge: adv7533: Fix adv7533_mode_valid for adv7533 and adv7535

[Adam Ford]

On Tue, Jul 4, 2023 at 11:05 AM Yongqin Liu <yongqin.liu@xxxxxxxxxx> wrote:
>
> Hi, Adam, All
>
> On Sun, 19 Mar 2023 at 20:55, Adam Ford <aford173@xxxxxxxxx> 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@xxxxxxxxxx>
> > Signed-off-by: Adam Ford <aford173@xxxxxxxxx>
> > ---
> >
> > 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.

I'd be curious to know the desired clock rate and the number of lanes
you're trying to use.

adam
>
> 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@xxxxxxxxxxxxxxxx
> http://lists.linaro.org/mailman/listinfo/linaro-android