[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXv+5GHcigEahCp+UefxR+AboF7i-kYJjiNftz8fPDLAZVMow@mail.gmail.com>
Date: Mon, 24 Apr 2023 14:12:33 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Adam Ford <aford173@...il.com>
Cc: dri-devel@...ts.freedesktop.org, Marek Vasut <marex@...x.de>,
Neil Armstrong <neil.armstrong@...aro.org>,
Robert Foss <rfoss@...nel.org>,
Jonas Karlman <jonas@...boo.se>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
aford@...conembedded.com,
Jernej Skrabec <jernej.skrabec@...il.com>,
linux-kernel@...r.kernel.org,
Jagan Teki <jagan@...rulasolutions.com>,
Andrzej Hajda <andrzej.hajda@...el.com>,
m.szyprowski@...sung.com, dario.binacchi@...rulasolutions.com
Subject: Re: [PATCH V2 4/6] drm: bridge: samsung-dsim: Dynamically configure
DPHY timing
On Sun, Apr 23, 2023 at 8:13 PM Adam Ford <aford173@...il.com> wrote:
>
> The DPHY timings are currently hard coded. Since the input
> clock can be variable, the phy timings need to be variable
> too. Add an additional variable to the driver data to enable
> this feature to prevent breaking boards that don't support it.
>
> The phy_mipi_dphy_get_default_config function configures the
> DPHY timings in pico-seconds, and a small macro converts those
> timings into clock cycles based on the pixel clock rate.
>
> Signed-off-by: Adam Ford <aford173@...il.com>
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 79 +++++++++++++++++++++++----
> include/drm/bridge/samsung-dsim.h | 1 +
> 2 files changed, 70 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 5b6e7825b92f..f165483d5044 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -18,9 +18,7 @@
> #include <linux/media-bus-format.h>
> #include <linux/of_device.h>
> #include <linux/phy/phy.h>
> -
> #include <video/mipi_display.h>
> -
> #include <drm/bridge/samsung-dsim.h>
> #include <drm/drm_panel.h>
> #include <drm/drm_print.h>
> @@ -218,6 +216,8 @@
>
> #define OLD_SCLK_MIPI_CLK_NAME "pll_clk"
>
> +#define PS_TO_CYCLE(PS, MHz) DIV64_U64_ROUND_CLOSEST(((PS) * (MHz)), 1000000000000ULL)
> +
> static const char *const clk_names[5] = {
> "bus_clk",
> "sclk_mipi",
> @@ -487,6 +487,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
> .m_min = 64,
> .m_max = 1023,
> .vco_min = 1050,
> + .dynamic_dphy = 1,
> };
>
> static const struct samsung_dsim_driver_data *
> @@ -698,13 +699,50 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
> const unsigned int *reg_values = driver_data->reg_values;
> u32 reg;
> + struct drm_display_mode *m = &dsi->mode;
> + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> + struct phy_configure_opts_mipi_dphy cfg;
> + int clk_prepare, lpx, clk_zero, clk_post, clk_trail;
> + int hs_exit, hs_prepare, hs_zero, hs_trail;
> + unsigned long long clock_in_hz = m->clock * 1000;
>
> if (driver_data->has_freqband)
> return;
>
> + /* The dynamic_phy has the ability to adjust PHY Timing settings */
> + if (driver_data->dynamic_dphy) {
> + phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->lanes, &cfg);
This requires adding "select GENERIC_PHY_MIPI_DPHY" to DRM_SAMSUNG_DSIM,
otherwise with CONFIG_DRM_SAMSUNG_DSIM=m:
ERROR: modpost: "phy_mipi_dphy_get_default_config"
[drivers/gpu/drm/bridge/samsung-dsim.ko] undefined!
make[5]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1
make[4]: *** [Makefile:1978: modpost] Error 2
make[3]: *** [Makefile:357: __build_one_by_one] Error 2
I'm sure there'll be a similar error if CONFIG_DRM_SAMSUNG_DSIM=y.
> +
> + /*
> + * TODO:
> + * The tech reference manual for i.MX8M Mini/Nano/Plus
> + * doesn't state what the definition of the PHYTIMING
> + * bits are beyond their address and bit position.
> + * After reviewing NXP's downstream code, it appears
> + * that the various PHYTIMING registers take the number
> + * of cycles and use various dividers on them. This
> + * calculation does not result in an exact match to the
> + * downstream code, but it is very close, and it appears
> + * to sync at a variety of resolutions. If someone
> + * can get a more accurate mathematical equation needed
> + * for these registers, this should be updated.
> + */
> +
> + lpx = PS_TO_CYCLE(cfg.lpx, clock_in_hz);
> + hs_exit = PS_TO_CYCLE(cfg.hs_exit, clock_in_hz);
> + clk_prepare = PS_TO_CYCLE(cfg.clk_prepare, clock_in_hz);
> + clk_zero = PS_TO_CYCLE(cfg.clk_zero, clock_in_hz);
> + clk_post = PS_TO_CYCLE(cfg.clk_post, clock_in_hz);
> + clk_trail = PS_TO_CYCLE(cfg.clk_trail, clock_in_hz);
> + hs_prepare = PS_TO_CYCLE(cfg.hs_prepare, clock_in_hz);
> + hs_zero = PS_TO_CYCLE(cfg.hs_zero, clock_in_hz);
> + hs_trail = PS_TO_CYCLE(cfg.hs_trail, clock_in_hz);
> + }
> +
> /* B D-PHY: D-PHY Master & Slave Analog Block control */
> reg = reg_values[PHYCTRL_ULPS_EXIT] | reg_values[PHYCTRL_VREG_LP] |
> reg_values[PHYCTRL_SLEW_UP];
> +
> samsung_dsim_write(dsi, DSIM_PHYCTRL_REG, reg);
>
> /*
> @@ -712,7 +750,11 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> * T HS-EXIT: Time that the transmitter drives LP-11 following a HS
> * burst
> */
> - reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT];
> + if (driver_data->dynamic_dphy)
> + reg = DSIM_PHYTIMING_LPX(lpx) | DSIM_PHYTIMING_HS_EXIT(hs_exit);
> + else
> + reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT];
> +
> samsung_dsim_write(dsi, DSIM_PHYTIMING_REG, reg);
>
> /*
> @@ -728,10 +770,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> * T CLK-TRAIL: Time that the transmitter drives the HS-0 state after
> * the last payload clock bit of a HS transmission burst
> */
> - reg = reg_values[PHYTIMING_CLK_PREPARE] |
> - reg_values[PHYTIMING_CLK_ZERO] |
> - reg_values[PHYTIMING_CLK_POST] |
> - reg_values[PHYTIMING_CLK_TRAIL];
> + if (driver_data->dynamic_dphy) {
> + reg = DSIM_PHYTIMING1_CLK_PREPARE(clk_prepare) |
> + DSIM_PHYTIMING1_CLK_ZERO(clk_zero) |
> + DSIM_PHYTIMING1_CLK_POST(clk_post) |
> + DSIM_PHYTIMING1_CLK_TRAIL(clk_trail);
> + } else {
> + reg = reg_values[PHYTIMING_CLK_PREPARE] |
> + reg_values[PHYTIMING_CLK_ZERO] |
> + reg_values[PHYTIMING_CLK_POST] |
> + reg_values[PHYTIMING_CLK_TRAIL];
> + }
>
> samsung_dsim_write(dsi, DSIM_PHYTIMING1_REG, reg);
>
> @@ -744,8 +793,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> * T HS-TRAIL: Time that the transmitter drives the flipped differential
> * state after last payload data bit of a HS transmission burst
> */
> - reg = reg_values[PHYTIMING_HS_PREPARE] | reg_values[PHYTIMING_HS_ZERO] |
> - reg_values[PHYTIMING_HS_TRAIL];
> +
> + if (driver_data->dynamic_dphy) {
> + reg = DSIM_PHYTIMING2_HS_PREPARE(hs_prepare) |
> + DSIM_PHYTIMING2_HS_ZERO(hs_zero) |
> + DSIM_PHYTIMING2_HS_TRAIL(hs_trail);
> + } else {
> + reg = reg_values[PHYTIMING_HS_PREPARE] |
> + reg_values[PHYTIMING_HS_ZERO] |
> + reg_values[PHYTIMING_HS_TRAIL];
> + }
> +
> samsung_dsim_write(dsi, DSIM_PHYTIMING2_REG, reg);
> }
>
> @@ -1337,7 +1395,8 @@ static int samsung_dsim_init(struct samsung_dsim *dsi)
> samsung_dsim_enable_clock(dsi);
> if (driver_data->wait_for_reset)
> samsung_dsim_wait_for_reset(dsi);
> - samsung_dsim_set_phy_ctrl(dsi);
> + if (!driver_data->has_freqband)
> + samsung_dsim_set_phy_ctrl(dsi);
> samsung_dsim_init_link(dsi);
>
> dsi->state |= DSIM_STATE_INITIALIZED;
> diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
> index a088d84579bc..25475d78adb3 100644
> --- a/include/drm/bridge/samsung-dsim.h
> +++ b/include/drm/bridge/samsung-dsim.h
> @@ -62,6 +62,7 @@ struct samsung_dsim_driver_data {
> u16 m_min;
> u16 m_max;
> u64 vco_min;
> + bool dynamic_dphy;
> };
>
> struct samsung_dsim_host_ops {
> --
> 2.39.2
>
Powered by blists - more mailing lists