Re: [PATCH] drm/bridge: tc358767: Enable FRMSYNC timing generator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 13, 2024 at 4:16 AM Marek Vasut <marex@xxxxxxx> wrote:
>
> TC9595 datasheet Video Path0 Control (VPCTRL0) Register bit FRMSYNC description
> says "This bit should be disabled only in video mode transmission where Host
> transmits video timing together with video data and where pixel clock source
> is from DSI clock." . This driver always sources pixel clock from external xtal,
> therefore the FRMSYNC bit must always be enabled, enable it.
>
> This fixes an actual issue with DSI-to-DPI mode, where the display would
> randomly show subtle pixel flickering, or wobble, or shimmering. This is
> visible on solid gray color, but the degree of the shimmering differs
> between boots, which makes it hard to debug.
>
> There is a caveat to the FRMSYNC and this bridge pixel PLL, which can only
> generate pixel clock with limited accuracy, it may therefore be necessary
> to reduce the HFP to fit into line length of input pixel data, to avoid any
> possible overflows, which make the output video look striped horizontally.
>
> Signed-off-by: Marek Vasut <marex@xxxxxxx>
> ---
> Cc: Adam Ford <aford173@xxxxxxxxx>
> Cc: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
> Cc: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> Cc: David Airlie <airlied@xxxxxxxxx>
> Cc: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>
> Cc: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
> Cc: Jonas Karlman <jonas@xxxxxxxxx>
> Cc: Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Maxime Ripard <mripard@xxxxxxxxxx>
> Cc: Michael Walle <mwalle@xxxxxxxxxx>
> Cc: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> Cc: Robert Foss <rfoss@xxxxxxxxxx>
> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: kernel@xxxxxxxxxxxxxxxxxx
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 166f9a3e9622d..fe2b93546eaef 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -382,6 +382,9 @@ struct tc_data {
>
>         /* HPD pin number (0 or 1) or -ENODEV */
>         int                     hpd_pin;
> +
> +       /* Number of pixels to subtract from a line due to pixel clock delta */
> +       u32                     line_pixel_subtract;
>  };
>
>  static inline struct tc_data *aux_to_tc(struct drm_dp_aux *a)
> @@ -577,6 +580,11 @@ static int tc_pllupdate(struct tc_data *tc, unsigned int pllctrl)
>         return 0;
>  }
>
> +static u32 div64_round_up(u64 v, u32 d)
> +{
> +       return div_u64(v + d - 1, d);
> +}
> +

Could the DIV_ROUND_UP macro in math,h replace this function?

>  static int tc_pxl_pll_en(struct tc_data *tc, u32 refclk, u32 pixelclock)
>  {
>         int ret;
> @@ -658,8 +666,11 @@ static int tc_pxl_pll_en(struct tc_data *tc, u32 refclk, u32 pixelclock)
>                 return -EINVAL;
>         }
>
> -       dev_dbg(tc->dev, "PLL: got %d, delta %d\n", best_pixelclock,
> -               best_delta);
> +       tc->line_pixel_subtract = tc->mode.htotal -
> +               div64_round_up(tc->mode.htotal * (u64)best_pixelclock, pixelclock);
> +
> +       dev_dbg(tc->dev, "PLL: got %d, delta %d (subtract %d px)\n", best_pixelclock,
> +               best_delta, tc->line_pixel_subtract);
>         dev_dbg(tc->dev, "PLL: %d / %d / %d * %d / %d\n", refclk,
>                 ext_div[best_pre], best_div, best_mul, ext_div[best_post]);
>
> @@ -885,6 +896,12 @@ static int tc_set_common_video_mode(struct tc_data *tc,
>                 upper_margin, lower_margin, vsync_len);
>         dev_dbg(tc->dev, "total: %dx%d\n", mode->htotal, mode->vtotal);
>
> +       if (right_margin > tc->line_pixel_subtract) {
> +               right_margin -= tc->line_pixel_subtract;
> +       } else {
> +               dev_err(tc->dev, "Bridge pixel clock too slow for mode\n");
> +               right_margin = 0;
> +       }
>
>         /*
>          * LCD Ctl Frame Size
> @@ -894,7 +911,7 @@ static int tc_set_common_video_mode(struct tc_data *tc,
>          */
>         ret = regmap_write(tc->regmap, VPCTRL0,
>                            FIELD_PREP(VSDELAY, right_margin + 10) |
> -                          OPXLFMT_RGB888 | FRMSYNC_DISABLED | MSF_DISABLED);
> +                          OPXLFMT_RGB888 | FRMSYNC_ENABLED | MSF_DISABLED);
>         if (ret)
>                 return ret;
>
> --
> 2.43.0
>

With the above question resolved, feel free to add my r-b. I will
snooze this for a week before looking at applying this.

Reviewed-by: Robert Foss <rfoss@xxxxxxxxxx>




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux